Skip to content

Commit 034309b

Browse files
takenagaintakenagain
committed
fix(market-metrics): fix profit/loss index issue (coderabbitai)
1 parent c9046d9 commit 034309b

File tree

4 files changed

+79
-120
lines changed

4 files changed

+79
-120
lines changed

‎lib/bloc/cex_market_data/portfolio_growth/portfolio_growth_bloc.dart

+25-26
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import 'package:komodo_defi_sdk/komodo_defi_sdk.dart';
77
import 'package:logging/logging.dart';
88
import 'package:web_dex/bloc/cex_market_data/charts.dart';
99
import 'package:web_dex/bloc/cex_market_data/portfolio_growth/portfolio_growth_repository.dart';
10-
import 'package:web_dex/bloc/coins_bloc/coins_repo.dart';
1110
import 'package:web_dex/mm2/mm2_api/rpc/base.dart';
1211
import 'package:web_dex/model/coin.dart';
1312
import 'package:web_dex/model/text_error.dart';
@@ -19,7 +18,6 @@ class PortfolioGrowthBloc
1918
extends Bloc<PortfolioGrowthEvent, PortfolioGrowthState> {
2019
PortfolioGrowthBloc({
2120
required this.portfolioGrowthRepository,
22-
required this.coinsRepository,
2321
required this.sdk,
2422
}) : super(const PortfolioGrowthInitial()) {
2523
// Use the restartable transformer for period change events to avoid
@@ -37,9 +35,8 @@ class PortfolioGrowthBloc
3735
}
3836

3937
final PortfolioGrowthRepository portfolioGrowthRepository;
40-
final CoinsRepo coinsRepository;
4138
final KomodoDefiSdk sdk;
42-
final _log = Logger('portfolio-growth-bloc');
39+
final _log = Logger('PortfolioGrowthBloc');
4340

4441
void _onClearPortfolioGrowth(
4542
PortfolioGrowthClearRequested event,
@@ -88,11 +85,13 @@ class PortfolioGrowthBloc
8885

8986
await _loadChart(coins, event, useCache: true)
9087
.then(emit.call)
91-
.catchError((e, _) {
88+
.catchError((Object error, StackTrace stackTrace) {
89+
const errorMessage = 'Failed to load cached chart';
90+
_log.warning(errorMessage, error, stackTrace);
9291
if (state is! PortfolioGrowthChartLoadSuccess) {
9392
emit(
9493
GrowthChartLoadFailure(
95-
error: TextError(error: e.toString()),
94+
error: TextError(error: errorMessage),
9695
selectedPeriod: event.selectedPeriod,
9796
),
9897
);
@@ -105,15 +104,15 @@ class PortfolioGrowthBloc
105104
if (activeCoins.isNotEmpty) {
106105
await _loadChart(activeCoins, event, useCache: false)
107106
.then(emit.call)
108-
.catchError((_, __) {
109-
// Ignore un-cached errors, as a transaction loading exception should not
110-
// make the graph disappear with a load failure emit, as the cached data
111-
// is already displayed. The periodic updates will still try to fetch the
112-
// data and update the graph.
107+
.catchError((Object error, StackTrace stackTrace) {
108+
_log.shout('Failed to load chart', error, stackTrace);
109+
// Don't emit an error state here. If cached and uncached attempts
110+
// both fail, the periodic refresh attempts should recovery
111+
// at the cost of a longer first loading time.
113112
});
114113
}
115114
} catch (error, stackTrace) {
116-
_log.severe('Failed to load portfolio growth', error, stackTrace);
115+
_log.shout('Failed to load CACHED portfolio growth', error, stackTrace);
117116
}
118117

119118
await emit.forEach(
@@ -124,7 +123,7 @@ class PortfolioGrowthBloc
124123
onData: (data) =>
125124
_handlePortfolioGrowthUpdate(data, event.selectedPeriod),
126125
onError: (error, stackTrace) {
127-
_log.severe('Failed to load portfolio growth', error, stackTrace);
126+
_log.shout('Failed to load portfolio growth', error, stackTrace);
128127
return GrowthChartLoadFailure(
129128
error: TextError(error: 'Failed to load portfolio growth'),
130129
selectedPeriod: event.selectedPeriod,
@@ -147,18 +146,6 @@ class PortfolioGrowthBloc
147146
return coins;
148147
}
149148

150-
Future<List<Coin>> _removeInactiveCoins(List<Coin> coins) async {
151-
final coinsCopy = List<Coin>.of(coins);
152-
final activeCoins = await sdk.assets.getActivatedAssets();
153-
final activeCoinsMap = activeCoins.map((e) => e.id).toSet();
154-
for (final coin in coins) {
155-
if (!activeCoinsMap.contains(coin.id)) {
156-
coinsCopy.remove(coin);
157-
}
158-
}
159-
return coinsCopy;
160-
}
161-
162149
Future<PortfolioGrowthState> _loadChart(
163150
List<Coin> coins,
164151
PortfolioGrowthLoadRequested event, {
@@ -207,11 +194,23 @@ class PortfolioGrowthBloc
207194
isHdWallet: currentUser.isHd,
208195
);
209196
} catch (error, stackTrace) {
210-
_log.severe('Empty growth chart on periodic update', error, stackTrace);
197+
_log.shout('Empty growth chart on periodic update', error, stackTrace);
211198
return ChartData.empty();
212199
}
213200
}
214201

202+
Future<List<Coin>> _removeInactiveCoins(List<Coin> coins) async {
203+
final coinsCopy = List<Coin>.of(coins);
204+
final activeCoins = await sdk.assets.getActivatedAssets();
205+
final activeCoinsMap = activeCoins.map((e) => e.id).toSet();
206+
for (final coin in coins) {
207+
if (!activeCoinsMap.contains(coin.id)) {
208+
coinsCopy.remove(coin);
209+
}
210+
}
211+
return coinsCopy;
212+
}
213+
215214
PortfolioGrowthState _handlePortfolioGrowthUpdate(
216215
ChartData growthChart,
217216
Duration selectedPeriod,

‎lib/bloc/cex_market_data/profit_loss/profit_loss_bloc.dart

+50-51
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,12 @@ import 'package:bloc/bloc.dart';
55
import 'package:bloc_concurrency/bloc_concurrency.dart';
66
import 'package:equatable/equatable.dart';
77
import 'package:komodo_defi_sdk/komodo_defi_sdk.dart';
8+
import 'package:logging/logging.dart';
89
import 'package:web_dex/bloc/cex_market_data/charts.dart';
910
import 'package:web_dex/bloc/cex_market_data/profit_loss/profit_loss_repository.dart';
1011
import 'package:web_dex/mm2/mm2_api/rpc/base.dart';
1112
import 'package:web_dex/model/coin.dart';
1213
import 'package:web_dex/model/text_error.dart';
13-
import 'package:web_dex/shared/utils/utils.dart' as logger;
1414

1515
part 'profit_loss_event.dart';
1616
part 'profit_loss_state.dart';
@@ -32,6 +32,8 @@ class ProfitLossBloc extends Bloc<ProfitLossEvent, ProfitLossState> {
3232
final ProfitLossRepository _profitLossRepository;
3333
final KomodoDefiSdk _sdk;
3434

35+
final _log = Logger('ProfitLossBloc');
36+
3537
void _onClearPortfolioProfitLoss(
3638
ProfitLossPortfolioChartClearRequested event,
3739
Emitter<ProfitLossState> emit,
@@ -57,21 +59,22 @@ class ProfitLossBloc extends Bloc<ProfitLossEvent, ProfitLossState> {
5759

5860
await _getProfitLossChart(event, coins, useCache: true)
5961
.then(emit.call)
60-
.catchError((e, _) {
61-
logger.log('Failed to load portfolio profit/loss: $e', isError: true);
62+
.catchError((Object error, StackTrace stackTrace) {
63+
const errorMessage = 'Failed to load CACHED portfolio profit/loss';
64+
// log warning here, because cached
65+
_log.warning(errorMessage, error, stackTrace);
6266
if (state is! PortfolioProfitLossChartLoadSuccess) {
6367
emit(
6468
ProfitLossLoadFailure(
65-
error:
66-
TextError(error: 'Failed to load portfolio profit/loss: $e'),
69+
error: TextError(error: errorMessage),
6770
selectedPeriod: event.selectedPeriod,
6871
),
6972
);
7073
}
7174
});
7275

7376
// Fetch the un-cached version of the chart to update the cache.
74-
coins = await _removeUnsupportedCons(event, allowInactiveCoins: false);
77+
coins = await _removeUnsupportedCons(event);
7578
if (coins.isNotEmpty) {
7679
await _getProfitLossChart(event, coins, useCache: false)
7780
.then(emit.call)
@@ -82,20 +85,18 @@ class ProfitLossBloc extends Bloc<ProfitLossEvent, ProfitLossState> {
8285
// data and update the graph.
8386
});
8487
}
85-
} catch (e) {
86-
logger
87-
.log('Failed to load portfolio profit/loss: $e', isError: true)
88-
.ignore();
88+
} catch (error, stackTrace) {
89+
_log.shout('Failed to load portfolio profit/loss', e, stackTrace);
8990
emit(
9091
ProfitLossLoadFailure(
91-
error: TextError(error: 'Failed to load portfolio profit/loss: $e'),
92+
error: TextError(error: 'Failed to load portfolio profit/loss'),
9293
selectedPeriod: event.selectedPeriod,
9394
),
9495
);
9596
}
9697

9798
await emit.forEach(
98-
// computation is omitted, so null-valued events are emitted on a set
99+
// computation is omitted, so null-valued events are emitted on a set
99100
// interval.
100101
Stream<Object?>.periodic(event.updateFrequency).asyncMap((_) async {
101102
return _getSortedProfitLossChartForCoins(
@@ -121,15 +122,9 @@ class ProfitLossBloc extends Bloc<ProfitLossEvent, ProfitLossState> {
121122
);
122123
},
123124
onError: (e, s) {
124-
logger
125-
.log(
126-
'Failed to load portfolio profit/loss: $e',
127-
isError: true,
128-
trace: s,
129-
)
130-
.ignore();
125+
_log.shout('Failed to load portfolio profit/loss', e, s);
131126
return ProfitLossLoadFailure(
132-
error: TextError(error: 'Failed to load portfolio profit/loss: $e'),
127+
error: TextError(error: 'Failed to load portfolio profit/loss'),
133128
selectedPeriod: event.selectedPeriod,
134129
);
135130
},
@@ -159,21 +154,19 @@ class ProfitLossBloc extends Bloc<ProfitLossEvent, ProfitLossState> {
159154
}
160155

161156
Future<List<Coin>> _removeUnsupportedCons(
162-
ProfitLossPortfolioChartLoadRequested event, {
163-
bool allowInactiveCoins = true,
164-
}) async {
157+
ProfitLossPortfolioChartLoadRequested event,
158+
) async {
165159
final List<Coin> coins = List.from(event.coins);
166160
for (final coin in event.coins) {
167161
final isCoinSupported = await _profitLossRepository.isCoinChartSupported(
168162
coin.id,
169163
event.fiatCoinId,
170-
allowInactiveCoins: allowInactiveCoins,
171164
);
172165
if (coin.isTestCoin || !isCoinSupported) {
173166
coins.remove(coin);
174167
}
175168
}
176-
return coins;
169+
return _removeInactiveCoins(coins);
177170
}
178171

179172
Future<void> _onPortfolioPeriodChanged(
@@ -182,26 +175,18 @@ class ProfitLossBloc extends Bloc<ProfitLossEvent, ProfitLossState> {
182175
) async {
183176
final eventState = state;
184177
if (eventState is! PortfolioProfitLossChartLoadSuccess) {
185-
emit(
178+
return emit(
186179
PortfolioProfitLossChartLoadInProgress(
187180
selectedPeriod: event.selectedPeriod,
188181
),
189182
);
190183
}
191-
192-
assert(
193-
eventState is PortfolioProfitLossChartLoadSuccess,
194-
'Selected period can only be changed when '
195-
'the state is PortfolioProfitLossChartLoadSuccess',
196-
);
197-
198-
final successState = eventState as PortfolioProfitLossChartLoadSuccess;
199184
add(
200185
ProfitLossPortfolioChartLoadRequested(
201-
coins: successState.coins,
202-
fiatCoinId: successState.fiatCurrency,
186+
coins: eventState.coins,
187+
fiatCoinId: eventState.fiatCurrency,
203188
selectedPeriod: event.selectedPeriod,
204-
walletId: successState.walletId,
189+
walletId: eventState.walletId,
205190
),
206191
);
207192
}
@@ -227,22 +212,24 @@ class ProfitLossBloc extends Bloc<ProfitLossEvent, ProfitLossState> {
227212
useCache: useCache,
228213
);
229214

230-
final startIndex = profitLosses.indexOf(
231-
profitLosses.firstWhere((element) => element.profitLoss != 0),
232-
);
233-
234-
if (startIndex == -1) {
235-
profitLosses.removeRange(0, startIndex);
215+
final firstNonZeroProfitLossIndex =
216+
profitLosses.indexWhere((element) => element.profitLoss != 0);
217+
if (firstNonZeroProfitLossIndex == -1) {
218+
_log.info(
219+
'No non-zero profit/loss data found for coin ${coin.abbr}',
220+
);
221+
return ChartData.empty();
236222
}
237223

238-
return profitLosses.toChartData();
239-
} catch (e) {
240-
logger
241-
.log(
242-
'Failed to load cached profit/loss for coin ${coin.abbr}: $e',
243-
isError: true,
244-
)
245-
.ignore();
224+
final nonZeroProfitLosses =
225+
profitLosses.sublist(firstNonZeroProfitLossIndex);
226+
return nonZeroProfitLosses.toChartData();
227+
} catch (e, s) {
228+
_log.severe(
229+
'Failed to load cached profit/loss for coin ${coin.abbr}',
230+
e,
231+
s,
232+
);
246233
return ChartData.empty();
247234
}
248235
}),
@@ -251,4 +238,16 @@ class ProfitLossBloc extends Bloc<ProfitLossEvent, ProfitLossState> {
251238
chartsList.removeWhere((element) => element.isEmpty);
252239
return Charts.merge(chartsList)..sort((a, b) => a.x.compareTo(b.x));
253240
}
241+
242+
Future<List<Coin>> _removeInactiveCoins(List<Coin> coins) async {
243+
final coinsCopy = List<Coin>.of(coins);
244+
final activeCoins = await _sdk.assets.getActivatedAssets();
245+
final activeCoinsMap = activeCoins.map((e) => e.id).toSet();
246+
for (final coin in coins) {
247+
if (!activeCoinsMap.contains(coin.id)) {
248+
coinsCopy.remove(coin);
249+
}
250+
}
251+
return coinsCopy;
252+
}
254253
}

0 commit comments

Comments
 (0)