feat(restore): differentiate restore outcomes by cant-do reason#588
feat(restore): differentiate restore outcomes by cant-do reason#588Catrya wants to merge 3 commits into
Conversation
Map cant-do responses from Action.lastTradeIndex to specific user-facing
messages instead of a single generic error:
- not_found: complete restore as new user with informative subtext
("No previous data for this user in this Mostro"). Same green icon
as a normal successful restore.
- invalid_trade_index: show the error overlay with a specific subtext
("Invalid trade index").
- Any other cant-do reason or timeout: keep the existing generic error
message.
Restore behavior (key indices, sessions, navigation) is unchanged.
syncTradeIndex remains silent.
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
WalkthroughThe PR adds a ChangesRestore "No History Found" Flow
Sequence DiagramsequenceDiagram
participant Server as Mostro Server
participant Manager as RestoreManager
participant State as RestoreProgressState
participant Notifier as RestoreProgressNotifier
participant UI as RestoreOverlay
Server->>Manager: Respond with cant-do (notFound / invalid_trade_index)
Manager->>Manager: Call _extractCantDoReason(payload)
alt cant-do == notFound
Manager->>Manager: return LastTradeIndexResponse(noHistoryFound: true)
Manager->>State: set noHistoryFound = true
State->>Notifier: update state (completed + noHistoryFound)
Notifier->>UI: emit completed + noHistoryFound
UI->>UI: show "restore no previous data"
else cant-do == invalid_trade_index
Manager->>Manager: throw RestoreInvalidTradeIndexException
Manager->>Notifier: show error with key "invalid_trade_index"
UI->>UI: show "restore invalid trade index"
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
lib/features/restore/restore_manager.dart (1)
965-970: ⚡ Quick winUse typed restore error codes instead of raw string literals.
'invalid_trade_index'and'restore_error'are string-coupled with UI checks; a typo silently changes behavior. Consider a small shared enum/const set for restore error codes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/restore/restore_manager.dart` around lines 965 - 970, Replace the raw string literals used for restore error keys with a typed set (enum or const) and update usage in restore_manager.dart: define a RestoreErrorCode (e.g., invalidTradeIndex, genericRestoreError) and use it when computing errorKey instead of 'invalid_trade_index'/'restore_error'; update the call site restoreProgressProvider.notifier.showError to accept the typed RestoreErrorCode (or map the enum to the existing string inside a single conversion helper) and adjust any UI overlay mapping to use the new enum/const values so typos no longer silently change behavior (refer to RestoreInvalidTradeIndexException, errorKey, and restoreProgressProvider.notifier.showError).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/l10n/intl_fr.arb`:
- Line 1249: Update the French translation for the key restoreInvalidTradeIndex
in lib/l10n/intl_fr.arb by replacing the English word "trade" with the French
term "échange" (e.g., change the value to "Index d'échange invalide") so it
matches existing French terminology; ensure you only modify the string value for
restoreInvalidTradeIndex and preserve JSON/ARB syntax and quoting.
In `@lib/l10n/intl_it.arb`:
- Line 1290: The translation for the key restoreInvalidTradeIndex uses "Indice
di trade non valido"; update the value to use the app's consistent Italian
terminology by replacing it with "Indice di scambio non valido" so it matches
other entries using "indice di scambio" (key: restoreInvalidTradeIndex).
---
Nitpick comments:
In `@lib/features/restore/restore_manager.dart`:
- Around line 965-970: Replace the raw string literals used for restore error
keys with a typed set (enum or const) and update usage in restore_manager.dart:
define a RestoreErrorCode (e.g., invalidTradeIndex, genericRestoreError) and use
it when computing errorKey instead of 'invalid_trade_index'/'restore_error';
update the call site restoreProgressProvider.notifier.showError to accept the
typed RestoreErrorCode (or map the enum to the existing string inside a single
conversion helper) and adjust any UI overlay mapping to use the new enum/const
values so typos no longer silently change behavior (refer to
RestoreInvalidTradeIndexException, errorKey, and
restoreProgressProvider.notifier.showError).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9d370874-99b1-4fd9-91a9-c83d8ab07aef
📒 Files selected for processing (10)
lib/data/models/last_trade_index_response.dartlib/features/restore/restore_manager.dartlib/features/restore/restore_overlay.dartlib/features/restore/restore_progress_notifier.dartlib/features/restore/restore_progress_state.dartlib/l10n/intl_de.arblib/l10n/intl_en.arblib/l10n/intl_es.arblib/l10n/intl_fr.arblib/l10n/intl_it.arb
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
BraCR10
left a comment
BraCR10
left a comment
There was a problem hiding this comment.
tACK
While I was testing, I noticed an unusual behavior. However, I think it is outside the scope of this PR.
First, I created an order in one Mostro instance. Then, I connected to another node where my user did not have any orders. Finally, I tried to reconnect to the original Mostro instance where my orders existed.
I tried more than three times and it did not work. However, after each failed restore attempt while reconnecting to the instance, I manually refreshed the user, and the restore worked properly afterwards.
2 participants
Map cant-do responses from Action.lastTradeIndex to specific user-facing messages instead of a single generic error:
Restore behavior (key indices, sessions, navigation) is unchanged. syncTradeIndex remains silent.
Summary by CodeRabbit
New Features
Documentation