feat(filters): saved filter views - #2193
Open
s0up4200 wants to merge 5 commits into
Open
Conversation
Named snapshots of the sidebar TorrentFilters blob, stored per user in a new filter_views table (SQLite 080 + Postgres 082). The filters payload is opaque TEXT the backend never interprets. No FK to user(id) since OIDC-only and auth-disabled installs never populate the user table. Implements #964
Views accordion at the top of the filter sidebar: click applies the saved snapshot (replace, not merge), kebab menu for rename/delete, save dialog that survives name conflicts. Active view highlighted by deep equality with expanded* UI state stripped. i18n across all nine locales.
s0up4200
added
enhancement
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds saved torrent filter views with database persistence, authenticated CRUD API endpoints, OpenAPI definitions, React Query integration, sidebar controls, normalization utilities, persisted accordion behavior, tests, and localized UI strings. ChangesSaved filter views
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant FilterViewsSection
participant useFilterViews
participant ApiClient
participant FilterViewHandler
participant FilterViewStore
FilterViewsSection->>useFilterViews: request saved-view operation
useFilterViews->>ApiClient: call filter-view API method
ApiClient->>FilterViewHandler: send HTTP request
FilterViewHandler->>FilterViewStore: validate and persist view
FilterViewStore-->>FilterViewHandler: return view or error
FilterViewHandler-->>ApiClient: return HTTP response
ApiClient-->>useFilterViews: resolve operation
useFilterViews-->>FilterViewsSection: refresh displayed views
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@internal/models/filter_view.go`:
- Around line 127-129: Update the RowsAffected handling in the deletion flow to
return or propagate the error from res.RowsAffected() before evaluating the
affected-row count. Preserve returning sql.ErrNoRows when the call succeeds with
zero rows, and avoid treating a RowsAffected failure as successful completion.
In `@web/src/components/torrents/FilterViewsSection.tsx`:
- Around line 83-96: Update submitName to return immediately when either
createView or updateView mutation is pending, before trimming or submitting the
name, while preserving the existing success callback and mutation-specific
behavior.
In `@web/src/hooks/usePersistedAccordion.ts`:
- Around line 12-26: Update the usePersistedAccordion state initializer to
derive the initial “views” expansion without writing VIEWS_SEEDED_KEY during
render. Move persistence of both “qui-accordion” and VIEWS_SEEDED_KEY into the
expandedItems effect, ensuring the marker is written together with the expanded
state.
In `@web/src/types/torrents.ts`:
- Around line 220-234: Introduce a FilterViewFilters transport type as
Partial<TorrentFilters> and use it for the filters fields in FilterView and
FilterViewInput. Update the view-application path to normalize these partial
filters into the complete TorrentFilters shape before consumers use them,
preserving safe defaults for omitted arrays.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 88c2be65-4d30-440c-b539-c7b2a71cc6b6
📒 Files selected for processing (27)
cmd/qui/main.gointernal/api/handlers/filter_views.gointernal/api/handlers/filter_views_test.gointernal/api/server.gointernal/api/server_test.gointernal/database/migrations/080_add_filter_views.sqlinternal/database/postgres_migrations/082_add_filter_views.sqlinternal/models/filter_view.gointernal/models/filter_view_test.gointernal/web/swagger/openapi.yamlweb/src/components/torrents/FilterSidebar.tsxweb/src/components/torrents/FilterViewsSection.tsxweb/src/hooks/useFilterViews.tsweb/src/hooks/usePersistedAccordion.tsweb/src/i18n/locales/cs/torrents.jsonweb/src/i18n/locales/de/torrents.jsonweb/src/i18n/locales/en/torrents.jsonweb/src/i18n/locales/fr/torrents.jsonweb/src/i18n/locales/it/torrents.jsonweb/src/i18n/locales/ko/torrents.jsonweb/src/i18n/locales/pt-BR/torrents.jsonweb/src/i18n/locales/uk/torrents.jsonweb/src/i18n/locales/zh-CN/torrents.jsonweb/src/lib/api.tsweb/src/lib/filter-views.test.tsweb/src/lib/filter-views.tsweb/src/types/torrents.ts
Comment on lines
+220
to
+234
| /** A named snapshot of TorrentFilters, saved server-side and shared across instances. */ | ||
| export interface FilterView { | ||
| id: number | ||
| name: string | ||
| filters: TorrentFilters | ||
| sortOrder: number | ||
| createdAt: string | ||
| updatedAt: string | ||
| } | ||
|
|
||
| export interface FilterViewInput { | ||
| name: string | ||
| filters: TorrentFilters | ||
| sortOrder?: number | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Model persisted filters as partial data.
The API accepts and returns opaque sparse objects (for example, the backend CRUD test returns {"tags":["hd"]} and updates with {}). Typing both fields as complete TorrentFilters falsely guarantees every array exists and can mask runtime failures in consumers. Introduce a FilterViewFilters = Partial<TorrentFilters> transport type, then normalize before applying a view.
🤖 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 `@web/src/types/torrents.ts` around lines 220 - 234, Introduce a
FilterViewFilters transport type as Partial<TorrentFilters> and use it for the
filters fields in FilterView and FilterViewInput. Update the view-application
path to normalize these partial filters into the complete TorrentFilters shape
before consumers use them, preserving safe defaults for omitted arrays.
Propagate RowsAffected errors in FilterViewStore.Delete, guard Enter-key resubmission while a save is pending, type stored view filters as Partial<TorrentFilters> (backend stores an opaque blob), and move the accordion seed-marker write out of the render-phase state initializer.
Contributor
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 `@web/src/hooks/usePersistedAccordion.ts`:
- Around line 24-27: Update the useEffect in usePersistedAccordion so it writes
the serialized expandedItems to "qui-accordion" before setting VIEWS_SEEDED_KEY.
Preserve the existing values and dependency behavior while ensuring the seeding
marker is only written after accordion-state persistence succeeds.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a3295847-97b5-4b51-85d2-f9f9201b2423
📒 Files selected for processing (4)
internal/models/filter_view.goweb/src/components/torrents/FilterViewsSection.tsxweb/src/hooks/usePersistedAccordion.tsweb/src/types/torrents.ts
|
SummaryCoverage spans saved-view creation and editing, filter persistence and normalization, localization, deterministic storage behavior, validation, concurrency, and responsive sidebar interactions. The tested product paths are broadly healthy across normal workflows and adversarial input cases, with one persisted-state rendering edge case affecting access to saved views. Merge with caution — a PR-attributable medium-severity defect causes the application to fail to render the sidebar and saved views when persisted UI state is malformed, affecting both desktop and mobile users until that state is cleared. Tests run by Ito
Tip Reply with @itoqa to send us feedback on this test run. |
![itoqa[bot]](https://avatars.githubusercontent.com/in/2622614?s=60&v=4){kind=small}
A corrupt qui-accordion localStorage value threw during the useState initializer and took the whole torrent page down to the error boundary (pre-existing, surfaced by Ito QA). Fall back to defaults like the sibling persistence hooks do; the persist effect then repairs the stored value. Also write the views seed marker after the accordion state so a failed write retries seeding.
Contributor
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 `@web/src/hooks/usePersistedAccordion.ts`:
- Around line 13-25: Update the persisted accordion parsing in the hook
initializer to validate that the parsed value is an array and every element is a
string before returning or spreading it. Fall back to DEFAULT_ITEMS for invalid
scalars, objects, or mixed-type arrays, while preserving the existing
VIEWS_SEEDED_KEY and “views” seeding behavior for valid string arrays.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 93fe0407-33dd-463f-9d3f-1bc25dc4e573
📒 Files selected for processing (2)
web/src/hooks/usePersistedAccordion.test.tsweb/src/hooks/usePersistedAccordion.ts
Valid JSON of the wrong shape (e.g. a bare number) slipped past the try/catch on the already-seeded path and crashed inside Radix instead.
|
Diff SummaryCoverage spans saved-view and filter-sidebar behavior, including normal save and persistence flows, localization across supported languages, desktop/mobile rendering, recovery from malformed or invalid stored settings, and storage access edge cases. The exercised behavior is broadly healthy across UI rendering, state persistence, input handling, and resilience scenarios. Safe to merge — the only failure is a medium-severity storage-denial resilience issue explicitly determined not to be caused by this PR, with no regressions or PR-attributable failures identified. It is a worthwhile flag for later hardening but is not a merge blocker for this change. Tests run by Ito
Additional Findings DetailsThese findings are unrelated to the current changes but were observed during testing. 🟡 Denied localStorage writes crash the filter sidebar
Evidence PackageTip Reply with @itoqa to send us feedback on this test run. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds saved filter views (#964): named snapshots of the sidebar filter selections (status/category/tag/tracker includes+excludes plus expr), stored server-side and applied with one click from a new Views section at the top of the filter sidebar. Applying a view replaces the current selections; search, sort, and column filters are untouched. Views are global across instances, and the snapshot strips the expanded* UI state so the active view highlights by deep equality. Backend is a small CRUD (
/api/filter-views) over a newfilter_viewstable (SQLite 080 + Postgres 082, no FK touser(id)so OIDC-only and auth-disabled installs work), with the filters payload kept as an opaque JSON blob.Deliberately out of scope: an expression editor, and/or filter builder, per-instance scoping, and drag reordering (
sort_orderis in the schema for later).Testing
go test -race -count=1 ./internal/models/ ./internal/api/...make test-openapipnpm test(incl. newfilter-views.test.ts)make precommitandmake buildScreenshots to follow.
Summary by CodeRabbit