fix(#269): bucket My Trades filter by live status, not the stale snapshot - #299
fix(#269): bucket My Trades filter by live status, not the stale snapshot#299codaMW wants to merge 2 commits into
Conversation
…tale snapshot The status filter bucketed each trade using the status loaded once from the DB snapshot (rawTradesProvider), while the row chip shows the live status from tradeStatusProvider. The snapshot only refreshes on pull-to-refresh / retry / after create-take, so status changes arriving via gift wrap / 38383 left the filter stale: a taken order stayed in the Pending bucket while its chip read Waiting Invoice. Bucket the filter with the same live status the chip uses, making it the single source of truth. filteredTradesWithOrderStateProvider now reads tradeStatusProvider per trade and buckets on that, falling back to the snapshot status only until the live status has loaded (so nothing briefly escapes the active filter). The list re-buckets live, so a trade moving Pending to Waiting Invoice leaves the Pending filter in real time, matching its chip. Tests: a trade whose snapshot is Pending but whose live status is Waiting Invoice is excluded from the Pending bucket and included in the Waiting Invoice bucket; the snapshot fallback is covered too. Verified on a physical device (Nokia C31). Note: the persisted DB row can still be stale for own orders (the 38383 ingest skips syncing Pending back, orders.rs:2873). This change fixes the user-visible mismatch regardless; the DB write-back is a separate data-correctness follow-up. Closes MostroP2P#269.
|
Warning Review limit reached
Next review available in: 45 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughTrade filtering now uses each trade’s live status when available. It falls back to the persisted snapshot status while live status loads. Tests cover stale snapshots, waiting-invoice classification, and fallback behavior. ChangesLive status filtering
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant FilteredTradesProvider
participant tradeStatusProvider
participant _tradeInfoToItem
FilteredTradesProvider->>tradeStatusProvider: Read live trade status
tradeStatusProvider-->>FilteredTradesProvider: Return live status or loading state
FilteredTradesProvider->>_tradeInfoToItem: Pass live or persisted status bucket
_tradeInfoToItem-->>FilteredTradesProvider: Return filtered trade item
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@test/features/trades/filtered_trades_provider_test.dart`:
- Around line 100-154: Extend the live-status tests around
staleSnapshotContainer and primeLiveStatus with a controllable status stream.
Assert the trade initially appears in its first live-status bucket, emit a
second status, then verify it is removed from the old bucket and appears in the
new bucket, covering re-bucketing after a live transition.
🪄 Autofix
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 Plus
Run ID: 873404f7-36a0-4805-aa6c-c4b15707920b
📒 Files selected for processing (2)
lib/features/trades/providers/trades_providers.darttest/features/trades/filtered_trades_provider_test.dart
Add a test that drives a trade's live status with a controllable stream: assert it sits in the Pending bucket, emit waitingBuyerInvoice, then assert it leaves Pending and enters the Waiting Invoice bucket. Covers the live re-bucketing the filter fix relies on, beyond the initial-status and snapshot-fallback cases.
|
@Catrya this is ready for review. What it does: filteredTradesWithOrderStateProvider now buckets each trade by the live status from tradeStatusProvider (the same source the row chip uses) instead of the DB snapshot, so the chip and the filter share one source of truth. _tradeInfoToItem takes an optional statusOverride so the provider supplies the live bucket without duplicating the mapping. Because it watches each trade's live status, the list re-buckets in real time, and it falls back to the snapshot bucket only until the live status has loaded. Tests: stale-snapshot excluded from the old bucket, present in the live bucket, snapshot fallback before first poll, and a live-transition test (Pending -> Waiting Invoice) covering the re-bucketing. Verified on a physical device (Nokia C31). Scope note: I kept the orders.rs:2873 write-back out of this PR. The persisted row can still be stale for own orders, but the UI is correct either way now since it reads live status. I think that write-back is worth doing for data correctness, but it's a distinct Rust change with its own testing (taker-timeout republish), so it felt cleaner as a follow-up. Happy to pick it up next or file it separately, whichever you prefer. |
1 participant
Problem
Each row's status chip shows the live status (
tradeStatusProvider), but the filter dropdown bucketed trades using the status loaded once from the DB snapshot (rawTradesProvider->filteredTradesWithOrderStateProvider). The snapshot only refreshes on pull-to-refresh / retry / after create-take, so changing the filter did not reload it, and status changes arriving via gift wrap / 38383 did not invalidate it.Result: chip-vs-filter mismatches. A taken order stayed in the Pending bucket while its chip read Waiting Invoice.
Fix
Bucket the filter with the same live status the chip uses, making it the single source of truth.
filteredTradesWithOrderStateProvidernow readstradeStatusProviderper trade and buckets on that, falling back to the snapshot status only until the live status has loaded (so nothing briefly escapes the active filter).Because the provider watches each trade's live status, the list re-buckets in real time: a trade moving Pending to Waiting Invoice leaves the Pending filter immediately, matching its chip.
_tradeInfoToItemgained an optionalstatusOverrideso the provider can supply the live bucket without duplicating the mapping.Testing
flutter analyzeclean; verified on a physical device (Nokia C31): whatever a trade's chip shows, selecting that status in the filter shows it and selecting a different status hides it.Note on the DB layer
For own orders the 38383 ingest skips syncing
Pendingback (orders.rs:2873), so the persisted row can remain stale. This change fixes the user-visible mismatch regardless (the live status bypasses the stale snapshot). The persisted-status write-back is a separate data-correctness item and would be a good follow-up; happy to take it on if you'd like it in-scope.Closes #269.
Summary by CodeRabbit
Bug Fixes
Tests