Filter history: order list integration - #16422
Conversation
|
|
cf50664 to
87091f0
Compare
There was a problem hiding this comment.
AI Code Review - Found 1 potential issue
The feature is well structured: the shared FilterHistoryRepository/FilterHistoryViewModel are reused cleanly, the payload encoding is canonicalized for reliable dedup, the save runs fire-and-forget on the app scope with failures swallowed, and the test coverage (mapper round-trip, save use case, both ViewModels) is thorough. One correctness issue on the custom-date-range round-trip is called out inline.
PR housekeeping
- Applied repo
AGENTS.md/CLAUDE.mdguidance: store-app MVVM patterns (ScopedViewModel, triggerEvent/MultiLiveEvent, repository-mediated data access) are followed correctly.
Automatic review · claude-opus-4-8 · Workflow run
How to reply to a finding
Reply on this review (or inline at the line the finding refers to) with one of:
@claude addressed- I made the change. Bot verifies against the next diff before marking resolved.@claude rejected: <reason>- Will not fix; reason gets quoted on the next review.@claude not-applicable- Finding does not apply (wrong file, already covered elsewhere, etc.).
The bot honours these on the next review pass.
| category.orderFilterOptions.any { it.isSelected && it.key != DEFAULT_ALL_KEY } | ||
| } | ||
| if (!hasSelection) return | ||
| val customDateRange = orderFiltersRepository.getCustomDateRangeFilter() |
There was a problem hiding this comment.
AI Code Review [fix here]
Issue: Custom date range is saved in the wrong unit. getCustomDateRangeFilter() returns the range converted to milliseconds (startDay.toDateAtStartOfDay().time), and that value is stored in the payload as customDateRangeStart/customDateRangeEnd. On restore, OrderFilterCategoriesViewModel.onPastFilterSelected() passes those numbers straight into orderFilterRepository.setCustomDateRange(...), which expects epoch days (same contract as the picker: OrderFilterOptionsViewModel.onCustomDateRangeChanged(startDay, endDay) → setCustomDateRange). Applying a saved filter that has a custom date range therefore writes a millis value into the days pref, yielding a corrupted, far-future range. The existing tests don't catch this because both sides are mocked.
Suggestion: Store epoch days in the payload so the round-trip matches setCustomDateRange. Use getCustomDateRangeDays() instead of getCustomDateRangeFilter() here:
val customDateRange = orderFiltersRepository.getCustomDateRangeDays()Consider adding a round-trip test that goes through the real OrderFiltersRepository (not a mock) to lock the units down.
71ec830 to
ed405c6
Compare
Adds a flag-gated clock entry point on the order filter screens, saves the current selection to history on Show Orders (via SaveOrderFilterToHistory on the app scope), and applies a picked past filter. Part 3 of WOOMOB-3326 (WOOMOB-3826).
ed405c6 to
19457e2
Compare

Part of WOOMOB-3326 — WOOMOB-3826
Description
Third PR in the filter history stack: wires the shared history screen (#16421) into the order list filters. Behind the
FILTER_HISTORYflag.source=ordersand tracksfilter_history_button_tapped.SaveOrderFilterToHistoryuse case that reads the full selection fromOrderFiltersRepository(the source of truth) and runs fire-and-forget on the app scope, so navigation isn't blocked and a rare failure can't crash the app.OrderFilterHistoryMapperproduces a canonical JSON payload (stable dedup) and a readable label.Test Steps
Enable Filter History in the developer feature-flag screen, then on the Orders tab:
source=orders.Stacked PRs
PR 3 of 4. Base branch:
issue/WOOMOB-3825-filter-history-screen(#16421).