feat: extend forceToTop feature for addons#613
feat: extend forceToTop feature for addons#613goodgpa wants to merge 2 commits intoViren070:mainfrom
Conversation
- Updated schemas to allow forceToTop to accept multiple types: boolean, 'streams', 'catalogs', 'both', 'none'. - Enhanced AIOStreams class to manage catalogs that should be forced to the top during resource fetching. - Modified CustomPreset to provide a select option for forceToTop with appropriate labels. - Updated StreamSorter to prioritize streams marked with forceToTop. - Adjusted config validation to handle legacy boolean values for forceToTop.
WalkthroughForce-to-top behaviour is expanded from a boolean to a multi-value setting ('none','streams','catalogs','both'), with schema and type updates, preset normalisation, runtime tracking and reordering in the core, stream-sorting adjustments, and a UI field for merged catalogs. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as User/UI
participant Frontend as MergedCatalogsCard
participant Preset as CustomPreset
participant DB as Schemas/Storage
participant Core as AIOStreams (main)
participant Sorter as StreamSorter
Note over User,Frontend: User sets "Force to Top" in UI
User->>Frontend: toggle ForceToTop
Frontend->>DB: save merged catalog (forceToTop flag)
Frontend->>Preset: (when generating addon) include forceToTop option
Preset->>Preset: normalizeForceToTop(value) -- `#a3be8c`
Preset->>DB: store Addon.forceToTop (enum or boolean)
DB->>Core: fetchResources reads addons and their forceToTop
Core->>Core: compute catalogsForcedToTop keys
Core->>Sorter: ask which streams/catalogs to force
Sorter->>Core: returns shouldForce... flags
Core->>Core: mark entries and reorder finalCatalogs so forced entries appear first
Core->>Frontend: expose ordered catalogs
Note over Core,Frontend: preserving relative order within forced/non-forced groups
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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: 0
🧹 Nitpick comments (1)
packages/core/src/main.ts (1)
1784-1808: Consider a single-pass partition for minor optimisation.The current double-filtering approach is correct and preserves relative order within forced and non-forced groups. For typical catalog counts, the performance impact is negligible and the code is clear.
🔎 Optional single-pass refactor
If you prefer to avoid two passes, you can partition in one loop:
if (this.catalogsForcedToTop.size > 0) { - const forcedCatalogs = this.finalCatalogs.filter((catalog) => - this.isCatalogForcedToTop(catalog) - ); - const remainingCatalogs = this.finalCatalogs.filter( - (catalog) => !this.isCatalogForcedToTop(catalog) - ); + const [forcedCatalogs, remainingCatalogs] = this.finalCatalogs.reduce( + ([forced, remaining], catalog) => { + return this.isCatalogForcedToTop(catalog) + ? [[...forced, catalog], remaining] + : [forced, [...remaining, catalog]]; + }, + [[] as typeof this.finalCatalogs, [] as typeof this.finalCatalogs] + ); this.finalCatalogs = [...forcedCatalogs, ...remainingCatalogs]; } if (this.catalogsForcedToTop.size > 0 && this.finalAddonCatalogs?.length) { - const forcedAddonCatalogs = this.finalAddonCatalogs.filter((catalog) => - this.isCatalogForcedToTop(catalog) - ); - const remainingAddonCatalogs = this.finalAddonCatalogs.filter( - (catalog) => !this.isCatalogForcedToTop(catalog) - ); + const [forcedAddonCatalogs, remainingAddonCatalogs] = this.finalAddonCatalogs.reduce( + ([forced, remaining], catalog) => { + return this.isCatalogForcedToTop(catalog) + ? [[...forced, catalog], remaining] + : [forced, [...remaining, catalog]]; + }, + [[] as typeof this.finalAddonCatalogs, [] as typeof this.finalAddonCatalogs] + ); this.finalAddonCatalogs = [ ...forcedAddonCatalogs, ...remainingAddonCatalogs, ]; }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/core/src/db/schemas.tspackages/core/src/main.tspackages/frontend/src/components/menu/addons.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/src/db/schemas.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/core/src/main.ts (1)
packages/core/src/db/schemas.ts (1)
Addon(164-164)
🔇 Additional comments (13)
packages/frontend/src/components/menu/addons.tsx (6)
1812-1812: LGTM!State declaration follows the established pattern in this component and has a sensible default value.
1919-1930: LGTM!The
forceToTopstate is correctly reset alongside other form fields when opening the add modal for a new merged catalog.
1932-1943: LGTM!The
forceToTopstate is correctly initialised from the existing merged catalog data with a sensible fallback tofalsefor backwards compatibility.
1978-1992: LGTM!The
forceToTopproperty is correctly included when updating an existing merged catalog.
1994-2015: LGTM!The
forceToTopproperty is correctly included when creating a new merged catalog.
2448-2456: LGTM!The UI block for "Force to Top" follows the established styling patterns and provides clear, descriptive labelling. The Switch component is properly bound to the state.
packages/core/src/main.ts (7)
94-94: LGTM!The
Setdata structure is appropriate for tracking unique catalog keys, and the field is correctly scoped as private.
1412-1412: LGTM!Clearing the Set at the start of
fetchResources()ensures a clean state for each fetch cycle and prevents stale force-to-top markers from previous calls.
1463-1465: LGTM!The determination of whether to force catalogs to the top is correctly placed before catalog processing and delegates appropriately to the helper method.
1545-1559: LGTM!The catalog mapping correctly prefixes IDs, marks catalogs for force-to-top when applicable, and adds them to
finalCatalogs. The logic flow is sound.
1563-1579: LGTM!The addon catalog mapping follows the same pattern as regular catalogs, maintaining consistency and correctness.
1652-1666: LGTM!The merged catalog handling correctly checks the
forceToTopflag and adds merged catalogs to the forced set when applicable. The truthy check is appropriate for the boolean flag.
1930-1940: LGTM!The helper methods are clear, concise, and correctly implemented:
getCatalogForceKeyuses::as a separator, which is a good choice to avoid conflicts with catalog IDs or types.shouldForceCatalogsToTopcorrectly checks for'catalogs'or'both'values, aligning with the PR's intent to support multi-value settings.
1 participant
Summary by CodeRabbit
New Features
Behaviour
✏️ Tip: You can customize this high-level summary in your review settings.