Added SheetId filter to the content list providers#188
Conversation
Introduced a `listOfSheetIdsProvider` to get a list of sheet IDs the current user can access. Updated the data providers for tasks, polls, and events to use this list, filtering their Firestore queries to only fetch documents related to these specific sheets. This optimizes data fetching by ensuring only relevant information is streamed to the client.
Modified the `allTextProvider` and `allBulletsProvider` to fetch data more efficiently. The providers now use `listOfSheetIdsProvider` to filter Firestore queries, ensuring that only texts and bullets belonging to the user's accessible sheets are retrieved. This change optimizes data loading by reducing the amount of data fetched from the database.
Modified the `listStreamProvider` to filter lists based on a list of sheet IDs obtained from `listOfSheetIdsProvider`. This ensures that only lists associated with the specified sheets are fetched and listened to.
Conditionally apply the `whereIn` filter for `sheetId` only when the `sheetIds` list is not empty. This prevents a Firestore error that occurs when an empty list is passed to a `whereIn` query.
|
Caution Review failedThe pull request is closed. WalkthroughAdds a new Riverpod provider exposing sheet ID lists and updates multiple feature providers to build Firestore queries that conditionally apply a whereIn filter on Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI (Consumer)
participant RP as Riverpod Provider
participant QB as Query Builder (local)
participant FS as Firestore
UI->>RP: subscribe to feature provider
RP->>RP: ref.watch(listOfSheetIdsProvider) -> sheetIds
RP->>QB: build Query (collection + conditional whereIn with sheetIds)
QB->>FS: query.snapshots()
FS-->>RP: snapshot stream (documents)
RP->>RP: map docs -> models, update state
RP-->>UI: emit new state
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
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 |
This commit updates the auto-generated provider hash values across various feature modules, including text, list, task, polls, bullets, and events.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
ios/Podfile.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
lib/features/bullets/providers/bullet_providers.dart(2 hunks)lib/features/bullets/providers/bullet_providers.g.dart(1 hunks)lib/features/events/providers/event_providers.dart(3 hunks)lib/features/events/providers/event_providers.g.dart(1 hunks)lib/features/list/providers/list_providers.dart(2 hunks)lib/features/list/providers/list_providers.g.dart(1 hunks)lib/features/polls/providers/poll_providers.dart(3 hunks)lib/features/polls/providers/poll_providers.g.dart(1 hunks)lib/features/sheet/providers/sheet_providers.dart(1 hunks)lib/features/sheet/providers/sheet_providers.g.dart(1 hunks)lib/features/task/providers/task_providers.dart(2 hunks)lib/features/task/providers/task_providers.g.dart(1 hunks)lib/features/text/providers/text_providers.dart(2 hunks)lib/features/text/providers/text_providers.g.dart(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: linux
- GitHub Check: ios
- GitHub Check: windows
- GitHub Check: test
- GitHub Check: android
- GitHub Check: macos
- GitHub Check: Test zoe_native plugin
🔇 Additional comments (18)
lib/features/events/providers/event_providers.g.dart (1)
47-47: LGTM - Generated hash update reflects source changes.The updated hash value is auto-generated by Riverpod's code generator to reflect changes in the source
event_providers.dartfile.lib/features/text/providers/text_providers.g.dart (1)
44-44: LGTM - Generated hash update.Auto-generated hash reflecting source changes in
text_providers.dart.lib/features/polls/providers/poll_providers.g.dart (1)
44-44: LGTM - Generated hash update.Auto-generated hash reflecting source changes in
poll_providers.dart.lib/features/bullets/providers/bullet_providers.g.dart (1)
47-47: LGTM - Generated hash update.Auto-generated hash reflecting source changes in
bullet_providers.dart.lib/features/sheet/providers/sheet_providers.g.dart (1)
202-246: LGTM - New generated provider for sheet ID filtering.The generated
ListOfSheetIdsProviderfollows the standard Riverpod 3.0 patterns correctly:
- Proper functional provider structure with
$FunctionalProvidermixin- Auto-dispose enabled (appropriate for derived data)
- Standard
overrideWithValuesupport for testing- Hash function for debugging
This provider enables the sheet-based filtering feature across other providers (bullets, events, lists, polls, tasks, texts).
lib/features/task/providers/task_providers.dart (1)
10-10: LGTM: Import added for sheet filtering.The import of
sheet_providers.dartis necessary to accesslistOfSheetIdsProviderfor the new filtering functionality.lib/features/text/providers/text_providers.dart (2)
9-9: LGTM: Import added for sheet filtering.
28-36: Sheet-based filtering implemented consistently.The filtering logic matches the pattern used in other providers. Note that the Firestore
whereInlimit of 10 values (flagged intask_providers.dart) applies here as well.lib/features/list/providers/list_providers.dart (2)
11-11: LGTM: Import added for sheet filtering.
28-36: Sheet-based filtering implemented consistently.The filtering logic matches the pattern used in other providers. The Firestore
whereInlimit concern flagged intask_providers.dartapplies here as well.lib/features/polls/providers/poll_providers.dart (2)
11-11: LGTM: Import added for sheet filtering.
30-50: Sheet-based filtering implemented consistently with disposal cleanup.The filtering logic matches the pattern used in other providers, and the disposal callback has been appropriately simplified to an arrow function. The Firestore
whereInlimit concern flagged intask_providers.dartapplies here as well.lib/features/bullets/providers/bullet_providers.dart (2)
9-9: LGTM: Import added for sheet filtering.
29-37: Sheet-based filtering implemented consistently.The filtering logic matches the pattern used in other providers. The Firestore
whereInlimit concern flagged intask_providers.dartapplies here as well.lib/features/sheet/providers/sheet_providers.dart (1)
200-204: LGTM: New provider for sheet ID filtering.The
listOfSheetIdsprovider cleanly exposes the list of sheet IDs from thesheetListProvider. This is consumed by other feature providers to filter their Firestore queries.However, note that this provider's values are used with Firestore's
whereInoperator (as seen intask_providers.dart,text_providers.dart, etc.), which has a hard limit of 10 values. If users can have more than 10 sheets, the filtering queries will fail at runtime. Please verify whether the application design constrains the sheet count to 10 or fewer, or implement query pagination/chunking if higher counts are expected.lib/features/events/providers/event_providers.dart (3)
12-12: LGTM!The import is necessary for accessing
listOfSheetIdsProviderused in the sheet filtering logic below.
38-38: LGTM!The change from
collection.snapshots()toquery.snapshots()correctly applies the conditional sheet-based filtering while preserving the existing snapshot handling logic.
50-50: LGTM!The simplified arrow function syntax is more concise while maintaining the same cleanup behavior.
Replaced the direct use of `Filter` with the `whereInFilter` helper function across multiple feature providers (`text`, `list`, `task`, `polls`, `events`, `bullets`) to standardize and simplify Firestore query construction for `whereIn` clauses.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
lib/features/task/providers/task_providers.dart (1)
30-38: Verify thatwhereInFilterhandles Firestore's 30-item limit.The filtering logic correctly conditionally applies the
whereInfilter when sheet IDs are present. However, as flagged in a previous review, Firestore'swhereInoperator has a maximum of 30 equality clauses.Please verify whether the
whereInFilterhelper function (fromfirebase_utils.dart) already implements chunking for lists exceeding 30 items:#!/bin/bash # Check if whereInFilter already implements chunking logic echo "=== Searching for whereInFilter implementation ===" rg -n "whereInFilter" --type=dart -A 20 -B 2 echo -e "\n=== Checking for any existing chunking patterns in firebase_utils ===" rg -n "chunk|batch.*30|whereIn.*30" --type=dart lib/common/utils/firebase_utils.dart -A 5 -B 2If
whereInFilterdoes not handle chunking, the previous review comment's suggested implementation should be applied to prevent runtime failures when users accumulate more than 30 sheets.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
lib/features/bullets/providers/bullet_providers.dart(2 hunks)lib/features/events/providers/event_providers.dart(3 hunks)lib/features/list/providers/list_providers.dart(2 hunks)lib/features/polls/providers/poll_providers.dart(3 hunks)lib/features/task/providers/task_providers.dart(2 hunks)lib/features/text/providers/text_providers.dart(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/features/list/providers/list_providers.dart
- lib/features/events/providers/event_providers.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: ios
- GitHub Check: macos
- GitHub Check: test
- GitHub Check: android
- GitHub Check: windows
- GitHub Check: Test zoe_native plugin
- GitHub Check: format
- GitHub Check: linux
🔇 Additional comments (6)
lib/features/task/providers/task_providers.dart (1)
10-10: LGTM!The import is necessary for accessing
listOfSheetIdsProviderand is correctly added.lib/features/bullets/providers/bullet_providers.dart (2)
9-9: LGTM!The import is necessary for the
listOfSheetIdsProviderused in the build method.
37-37: LGTM!The subscription correctly uses the filtered query, and the reactive dependency on
listOfSheetIdsProviderwill properly rebuild the subscription when sheet IDs change. The subscription cleanup is also handled correctly.lib/features/polls/providers/poll_providers.dart (3)
11-11: LGTM! Import is necessary for the new sheet ID filtering.The import of
sheet_providersis required to accesslistOfSheetIdsProviderfor the dynamic filtering functionality.
38-38: LGTM! Query usage is correct.The change from
_collection.snapshots()toquery.snapshots()properly applies the conditional filter built in the preceding lines.
50-50: LGTM! Cleaner disposal syntax.The arrow function syntax is more concise while maintaining the same functionality for canceling the subscription on disposal.
Introduces `whereInFilter`, a utility function to create Firestore-safe `whereIn` queries for lists of any size. This function handles Firestore's limit of 30 values for a `whereIn` clause by automatically splitting larger lists into multiple `Filter.or` conditions.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
lib/common/utils/firebase_utils.dart (2)
51-52: Consider throwing an exception instead of usingassertfor input validation.The
assertstatement only runs in debug mode. In release builds, passing an empty list will causeFilter(field, whereIn: [])to be created, which may lead to unexpected Firestore behavior or errors.Filter whereInFilter(String field, List<Object?> values) { - assert(values.isNotEmpty, 'values must not be empty'); + if (values.isEmpty) { + throw ArgumentError('values must not be empty'); + }
65-67: Minor: Simplify sublist bounds usingmin.+import 'dart:math' show min; + // In the loop: - whereIn: values.sublist( - i, - i + _whereInLimit > values.length ? values.length : i + _whereInLimit, - ), + whereIn: values.sublist(i, min(i + _whereInLimit, values.length)),
…nstant The 'and' filter for combining Firestore queries has been corrected to 'or' to ensure proper query logic. Additionally, a 'sheetId' constant has been added to the Firestore field constants.
1 participant
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.