FINDIT-160 Collaborative list & visualization interaction#19
Conversation
Co-authored-by: Violeta Ramos <VioletaR@users.noreply.github.com>
Co-authored-by: Violeta Ramos <VioletaR@users.noreply.github.com>
Co-authored-by: Violeta Ramos <VioletaR@users.noreply.github.com>
Co-authored-by: Violeta Ramos <VioletaR@users.noreply.github.com>
Co-authored-by: Violeta Ramos <VioletaR@users.noreply.github.com>
Co-authored-by: Violeta Ramos <VioletaR@users.noreply.github.com>
Co-authored-by: Violeta Ramos <VioletaR@users.noreply.github.com>
Co-authored-by: Violeta Ramos <VioletaR@users.noreply.github.com>
Co-authored-by: Violeta Ramos <VioletaR@users.noreply.github.com>
Co-authored-by: Violeta Ramos <VioletaR@users.noreply.github.com>
Co-authored-by: Violeta Ramos <VioletaR@users.noreply.github.com>
…ive-list-visualization-interaction
Co-authored-by: Violeta Ramos <VioletaR@users.noreply.github.com>
WalkthroughThis update introduces extensive enhancements to list and user management, focusing on collaborative features, improved GraphQL queries/mutations, and modular UI components. The authentication context now tracks the current list ID. New bottom sheets enable list selection, sharing, and collaborator management. Product recommendation and list operations are streamlined, and the GraphQL schema is expanded for user and list-centric queries. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant AuthContext
participant GraphQLAPI
User->>App: Opens Shopping List
App->>AuthContext: Get currentListId
App->>GraphQLAPI: Query list details with currentListId
GraphQLAPI-->>App: Return list/products/collaborators
App->>User: Display list UI
User->>App: Presses "Add Collaborator"
App->>GraphQLAPI: Fetch users (excluding current collaborators)
User->>App: Selects user to add
App->>GraphQLAPI: Share list with selected user
GraphQLAPI-->>App: Confirm collaborator added
App->>User: Update collaborators UI
User->>App: Presses "Recommend Products"
App->>GraphQLAPI: Query recommendations
GraphQLAPI-->>App: Return recommended products
App->>GraphQLAPI: Upsert recommended products to list
GraphQLAPI-->>App: Confirm products added
App->>User: Show updated list
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
app/(tabs)/index.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-import". (The package "eslint-plugin-import" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-import" was referenced from the config file in ".eslintrc.js » eslint-config-expo » ./utils/core.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. app/(tabs)/navigation.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-import". (The package "eslint-plugin-import" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-import" was referenced from the config file in ".eslintrc.js » eslint-config-expo » ./utils/core.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. components/AddCollaboratorBottomSheet.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-import". (The package "eslint-plugin-import" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-import" was referenced from the config file in ".eslintrc.js » eslint-config-expo » ./utils/core.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.
Note Free review on us!CodeRabbit is offering free reviews until Wed May 28 2025 to showcase some of the refinements we've made. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
🚀 Expo preview is ready!
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
♻️ Duplicate comments (1)
components/AvailableListBottomSheet.tsx (1)
135-137: Same.catch()misuse with collaborator selectionReplicate the fix above – remove the promise-style chaining around the state setter.
🧹 Nitpick comments (12)
components/MoreBottomSheet.tsx (1)
66-85: Consider handling empty actions array gracefully.The component doesn't handle the case where an empty actions array is passed, which could result in an empty FlatList.
Add a check for empty actions:
+ {actions.length === 0 ? ( + <ThemedText type="body" color="variant"> + No actions available + </ThemedText> + ) : ( <FlatList data={actions} contentContainerStyle={{ gap: 16 }} keyExtractor={(item) => item.label} renderItem={({ item }) => ( <Pressable className="w-full flex-row items-center gap-4 rounded-2xl p-4 transition-opacity duration-150 active:opacity-70" onPress={item.onPress} style={{ backgroundColor: backgroundColor, boxShadow: themeConfig.theme.boxShadow.DEFAULT, }} > <ThemedIcon Icon={item.icon} color={"gradient"} /> <ThemedText>{item.label}</ThemedText> </Pressable> )} scrollEnabled={false} className={"w-full overflow-visible px-4"} /> + )}components/RecommendationButton.tsx (1)
77-82: Simplify conditional logic for better readability.The current structure with multiple toast calls could be simplified.
Consider combining the error handling:
- if (result.error) { - toast({ - title: "Oops! Something went wrong", - text: result.error.message, - }); - } - if (!result.data?.rawListByUserAndAi) { - toast({ - title: "Oops! Something went wrong", - text: "No recommendations found", - }); - } + if (result.error || !result.data?.rawListByUserAndAi) { + toast({ + title: "Oops! Something went wrong", + text: result.error?.message || "No recommendations found", + }); + setBackground(false); + lottieRef.current?.pause(); + lottieRef.current?.reset(); + return; + }components/AddCollaboratorBottomSheet.tsx (1)
103-107: Debounce the search input to avoid flooding the serverEvery keystroke triggers a network request. Consider using
lodash.debounceoruseDeferredValueto wait ~300 ms after the user stops typing before updatingsearchQuery.
This keeps the UI snappy and reduces server load.components/Header.tsx (1)
78-85: Improve accessibility for pressable titleWhen
titleOnPressis provided, the text should haveaccessibilityRole="button"and perhapsaccessibilityLabel.
This helps screen-reader users understand the element is interactive.app/(tabs)/shopping-list.tsx (2)
122-127: GuardrefetchListagainst race conditions
refetch()will throw if the query is still skipped. You already checkcurrentListId, but consider early-returning when the component is unmounted to avoid setting state on an unmounted component (common withuseFocusEffect). A cancellable flag oruseEffectcleanup would suffice.
200-205:ShoppingListSkeleton/ErrorComponentshould honour safe-area insetsMinor UX: these full-screen placeholders don’t include
paddingTop={insets.top}like the main content, causing a visual jump. Wrap them in aViewwith the same padding for consistency.components/NoListAvailable.tsx (2)
31-38: Type-safety is lost – add generics touseMutationThe generated types
CreateListMutation/CreateListMutationVariablesexist and give you strong typings for the mutation result and variables:-const [createList] = useMutation(CREATE_LIST, { ... }); +const [createList] = useMutation< + CreateListMutation, + CreateListMutationVariables +>(CREATE_LIST, { ... });This prevents accidental misspellings of variable names (as seen above) from compiling in the first place.
84-89: “Generate” button is wired to an empty handler
onPress={() => {}}silently does nothing; users get no feedback.
Either remove the button until the feature is ready or display a toast / disabled state with a TODO.components/AvailableListBottomSheet.tsx (1)
50-52: Unused variablecolorScheme– dead code / lint warning
colorSchemeis created but never referenced.
Remove it or use it for dynamic skeleton colouring to avoid eslint warnings and keep bundle size minimal.graphql/gql.ts (1)
41-45: Duplicate document keys indocumentsmap overwrite each other
"mutation AddProductsToList"appears twice (lines ~41 and ~44).
In plain JS objects the last entry wins, so the first definition is dead code and increases bundle size without effect.While this file is generated, ensuring the codegen configuration avoids duplicate operation names will prevent silent overwrites.
graphql/graphql.ts (2)
677-711: Heavy payload risk inCurrentSupermarketListWithProductsFor every list product you fetch:
•
images– full array (often dozens of URLs)
•blurhash,brandName,categoryName, etc.On a medium-sized list this easily becomes a multi-hundred-KB response and slows down the bottom-sheet opening on low-end devices.
Recommendation:
- Request only the first image (server side slice) or add a dedicated
thumbnailfield.- Consider a dedicated fragment that is reused across product cards so you can widen the set later without touching all documents.
If detailed product information is needed later (e.g., product sheet), fetch it lazily per item.
1-20: Generated artefact committed – consider excluding from VCSThis file looks like pure GraphQL-Codegen output (≈ 8 000 loc,
/* eslint-disable */, deterministic content). Keeping it under version control:• Bloats diffs & PR reviews (as we see here)
• Creates frequent merge conflicts on every schema change
• Slows down repository clonesA common practice is to generate it at build/test time and add
graphql/graphql.ts(or the wholegraphql/__generated__) to.gitignore, committing only the.graphqldocuments and the codegen config.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
app/(tabs)/index.tsx(4 hunks)app/(tabs)/navigation.tsx(4 hunks)app/(tabs)/shopping-list.tsx(8 hunks)app/productSearch/index.tsx(3 hunks)app/profile/index.tsx(0 hunks)components/AddCollaboratorBottomSheet.tsx(1 hunks)components/AvailableListBottomSheet.tsx(1 hunks)components/BSItem.tsx(1 hunks)components/CollaboratorsBottomSheet.tsx(1 hunks)components/Header.tsx(4 hunks)components/MoreBottomSheet.tsx(1 hunks)components/NoListAvailable.tsx(1 hunks)components/RawBottomSheet.tsx(1 hunks)components/RecommendationButton.tsx(1 hunks)components/VoiceBottomSheet.tsx(2 hunks)components/skeletons/ShoppingListSkeleton.tsx(1 hunks)contexts/AuthContext.tsx(2 hunks)graphql/gql.ts(7 hunks)graphql/graphql.ts(18 hunks)graphql/schema.graphql(2 hunks)hooks/useToast.ts(2 hunks)
💤 Files with no reviewable changes (1)
- app/profile/index.tsx
🧰 Additional context used
🧬 Code Graph Analysis (10)
app/productSearch/index.tsx (1)
hooks/useAuth.ts (1)
useAuth(4-10)
components/AddCollaboratorBottomSheet.tsx (5)
graphql/gql.ts (1)
gql(297-299)hooks/useThemeColor.ts (1)
useThemeColor(9-15)hooks/useAuth.ts (1)
useAuth(4-10)hooks/useToast.ts (1)
useToast(11-50)components/ThemedText.tsx (1)
ThemedText(20-56)
components/RecommendationButton.tsx (6)
graphql/gql.ts (1)
gql(297-299)graphql/graphql.ts (3)
UpsertProductFromListMutation(717-724)Exact(5-7)ListProductInput(114-120)hooks/useAuth.ts (1)
useAuth(4-10)hooks/useToast.ts (1)
useToast(11-50)hooks/useBackground.ts (1)
useBackground(4-10)components/Button.tsx (1)
Button(35-102)
components/CollaboratorsBottomSheet.tsx (5)
graphql/gql.ts (1)
gql(297-299)hooks/useAuth.ts (1)
useAuth(4-10)hooks/useThemeColor.ts (1)
useThemeColor(9-15)components/ThemedText.tsx (1)
ThemedText(20-56)components/Button.tsx (1)
Button(35-102)
app/(tabs)/index.tsx (1)
hooks/useAuth.ts (1)
useAuth(4-10)
components/BSItem.tsx (4)
hooks/useThemeColor.ts (1)
useThemeColor(9-15)components/LinearGradient.tsx (1)
LinearGradient(15-44)components/ThemedIcon.tsx (1)
ThemedIcon(13-31)components/ThemedText.tsx (1)
ThemedText(20-56)
components/NoListAvailable.tsx (8)
graphql/gql.ts (1)
gql(297-299)graphql/graphql.ts (1)
CurrentSupermarketListWithProductsQuery(681-711)hooks/useAuth.ts (1)
useAuth(4-10)hooks/useToast.ts (1)
useToast(11-50)app/(tabs)/shopping-list.tsx (1)
GET_PRODUCTS_LIST(36-63)components/ThemedText.tsx (1)
ThemedText(20-56)components/ThemedIcon.tsx (1)
ThemedIcon(13-31)components/Button.tsx (1)
Button(35-102)
components/AvailableListBottomSheet.tsx (5)
graphql/gql.ts (1)
gql(297-299)hooks/useAuth.ts (1)
useAuth(4-10)hooks/useToast.ts (1)
useToast(11-50)hooks/useThemeColor.ts (1)
useThemeColor(9-15)components/ThemedText.tsx (1)
ThemedText(20-56)
components/MoreBottomSheet.tsx (3)
hooks/useThemeColor.ts (1)
useThemeColor(9-15)components/ThemedText.tsx (1)
ThemedText(20-56)components/ThemedIcon.tsx (1)
ThemedIcon(13-31)
app/(tabs)/shopping-list.tsx (5)
graphql/gql.ts (1)
gql(297-299)hooks/useAuth.ts (1)
useAuth(4-10)hooks/useToast.ts (1)
useToast(11-50)components/VoiceBottomSheet.tsx (1)
VoiceBottomSheet(64-274)components/RawBottomSheet.tsx (1)
RawBottomSheet(74-226)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build and analyze with SonarQube
- GitHub Check: EAS Update
🔇 Additional comments (34)
hooks/useToast.ts (2)
3-3: Good addition for performance optimization.Adding the
useCallbackimport is necessary for the memoization changes below.
14-47: Excellent memoization for hook stability.The
useCallbackwrappers for bothtoastandtoastOnErrorfunctions are well-implemented:
toastcorrectly depends oninsets.bottomandinsets.toptoastOnErrorcorrectly depends on the memoizedtoastfunction- This prevents unnecessary re-renders in components that use these functions as dependencies
This optimization aligns well with the collaborative features where these toast functions will be used across multiple new components.
components/RawBottomSheet.tsx (1)
161-161: Good UI improvement for modal management.Adding
stackBehavior={"replace"}ensures that when multiple bottom sheets are opened, they replace each other rather than stack. This provides a cleaner user experience, especially important for the collaborative features being introduced.components/VoiceBottomSheet.tsx (2)
3-3: Clean import consolidation.Consolidating the multiline imports to single lines improves readability without any functional changes.
Also applies to: 10-10, 15-15
182-182: Consistent modal behavior across the app.Adding
stackBehavior={"replace"}maintains consistency with other bottom sheet components and ensures proper modal management for the collaborative features.graphql/schema.graphql (2)
270-270: Essential query for list-centric operations.The
list(id: String!): SupermarketListquery enables direct list fetching by ID, supporting the shift from user-centric to list-centric operations mentioned in the PR objectives. This is fundamental for the collaborative features.
297-297: Well-designed user search for collaboration.The
users(excludedIds: [String!] = null, name: String!): [User!]!query is thoughtfully designed:
- Required
nameparameter enables user search by name- Optional
excludedIdsparameter allows filtering out existing collaborators- Perfect for "add collaborator" functionality where you want to search users but exclude those already on the list
components/skeletons/ShoppingListSkeleton.tsx (1)
1-40: LGTM! Well-structured skeleton component.The implementation correctly uses safe area insets and adapts to the color scheme. The skeleton structure appropriately mimics the shopping list layout with categories and products.
app/productSearch/index.tsx (3)
70-70: LGTM! Appropriate use of auth context.Good refactoring to use
currentListIddirectly from the auth context instead of querying for it separately.
105-111: LGTM! Improved error handling and user feedback.The check for
currentListIdpresence and the toast notification provide clear feedback to users when no list is available.
131-131: LGTM! Consistent with the architectural changes.Using
currentListIddirectly aligns with the shift to list-based operations and simplifies the data flow.app/(tabs)/index.tsx (5)
81-81: LGTM! Consistent with the new auth context structure.Properly destructuring the new
currentListIdandsetCurrentListIdfrom the auth context.
108-108: LGTM! Simplified list ID management.Using
currentListIddirectly eliminates the need to check user'sactualListstatus, simplifying the logic.
124-124: LGTM! Proper error handling for async operation.The
.catch(toastOnError)ensures that any errors when setting the current list ID are properly handled and displayed to the user.
131-131: Good use of nullish coalescing operator.Using
??instead of||ensures that onlynullorundefinedvalues are replaced with the empty string, which is more precise than logical OR.
251-259: Good improvement using nullish coalescing.Using
??for default values is more precise than||as it only handlesnullandundefinedcases.components/BSItem.tsx (5)
10-16: LGTM! Well-defined interface.The props interface is clean and uses appropriate types, with optional properties properly marked.
22-22: LGTM! Proper async handling.The async/await pattern in the onPress handler correctly handles both sync and async callback functions.
24-27: LGTM! Consistent theming approach.Using
useThemeColorfor background and the theme config for box shadow ensures consistent styling across the app.
32-38: LGTM! Good conditional rendering logic.The error state properly shows a warning icon, while the normal state displays the first letter of the title in uppercase. The
ignoreDarkMode={true}ensures consistent appearance in the gradient background.
44-51: LGTM! Clear selected state indication.The selected state with a secondary gradient and check icon provides clear visual feedback to users.
components/MoreBottomSheet.tsx (1)
1-14: LGTM! Well-structured component with proper theming.The component demonstrates good practices:
- Proper use of forwardRef for BottomSheetModal integration
- Consistent theming with custom hooks
- Safe area insets handling
- Clean interface definitions
Also applies to: 26-48, 52-65
contexts/AuthContext.tsx (3)
15-16: LGTM! Proper extension of AuthContext interface and state management.The addition of
currentListIdandsetCurrentListIdto the context is well-implemented with proper state initialization.Also applies to: 26-28
39-53: LGTM! Robust currentListId setter with proper error handling.The
setCurrentListIdfunction correctly:
- Handles both setting and clearing the list ID
- Uses secure storage for persistence
- Includes proper error handling with toast notifications
- Follows the same pattern as the existing
setIdfunction
62-62: LGTM! Proper cleanup and dependency management.Good addition of clearing
currentListIdwhen user logs out and proper inclusion ofsetCurrentListIdin the dependency array.Also applies to: 69-69
components/RecommendationButton.tsx (1)
21-37: LGTM! Well-structured GraphQL integration and component design.The component demonstrates good practices:
- Clean GraphQL query definition
- Proper use of hooks for state management
- Good integration with Lottie animations
- Appropriate use of lazy query for user-triggered actions
Also applies to: 51-61, 116-124
app/(tabs)/navigation.tsx (4)
24-25: LGTM! Proper migration to list-centric GraphQL query.The query update correctly changes from user-based to list-based data fetching, aligning with the broader architectural shift in the application.
Also applies to: 58-59
50-51: LGTM! Proper use of updated auth context and toast hook.Good use of
currentListIdfrom the auth context and addition oftoastOnErrorfor improved error handling.
70-70: LGTM! Correct skip condition for the updated query.The skip condition properly checks for both
userLocationandcurrentListId, ensuring the query only runs when both required parameters are available.
102-103: LGTM! Improved error handling and data access pattern.The async error handling with
toastOnErroris a good improvement, and the data access pattern correctly reflects the new query structure (data.list.productsinstead ofdata.user.actualList.products).Also applies to: 109-109
components/Header.tsx (1)
34-36:isReactFCmay mis-detect functions that are not React componentsA plain function satisfies
typeof item === "function", so a non-component could be rendered accidentally.
A safer check is to look for$$typeofor.prototype?.isReactComponent.-const isReactFC = (item: HeaderAction | FC): item is FC => { - return typeof item === "function"; -}; +const isReactFC = (item: HeaderAction | FC): item is FC => + typeof item === "function" && + // heuristics for React component + ("$$typeof" in item || "propTypes" in item || "defaultProps" in item);graphql/graphql.ts (3)
357-376: Validate newlistandusersroot-queries against the backend schemaClient typing now exposes
Query.listandQuery.usersplus their correspondingQueryListArgs/QueryUsersArgs. Before merging, please make sure:
- The API gateway / server has these fields deployed with the exact argument names and nullability (
id: String!,name: String!,excludedIds: [String!]).- Any BFF layer or persisted-query store is upgraded; otherwise runtime GraphQL validation will fail and the whole request batch will be rejected with
Cannot query field "list"errors.If you haven’t rolled out the server change yet, consider guarding the client calls with a feature flag or delaying the code-gen commit.
Also applies to: 394-396, 451-456
2465-2535: Optional variableexcludedIdsis declared, but the list type forces non-null elements
excludedIdsis typed as[String!], which means the list elements cannot be null (good) but the variable itself is optional (no!on the list).
Just double-check the server expects the same; some implementations model this as[ID](allowing nullable elements) for flexibility.No action required if the schema is aligned; mentioning to avoid subtle 400s.
2610-2726:⚠️ Potential issue
UserListsquery still relies onactualList– might break when moving to list-centric modelPR description says we are “shifting from a user-centric to a list-centric model” and that the
User.actualListfield was removed, yet this document still queries it.
If the field is indeed being deprecated server-side, this will raise validation errors.Action items:
- Confirm whether
actualListremains in the schema.- If it is deprecated, refactor the UI that consumes
UserListsto rely solely oncurrentListIdobtained from the auth context.Likely an incorrect or invalid review comment.
| style={{ | ||
| borderRadius: 32, | ||
| overflow: "hidden", | ||
| boxShadow: themeConfig.theme.boxShadow.lg, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Platform compatibility concern with boxShadow styling.
Using boxShadow in React Native style objects may not work consistently across platforms, particularly on Android where it's not supported.
Consider using the elevation property for Android compatibility or platform-specific styling:
- style={{
- borderRadius: 32,
- overflow: "hidden",
- boxShadow: themeConfig.theme.boxShadow.lg,
- }}
+ style={{
+ borderRadius: 32,
+ overflow: "hidden",
+ ...Platform.select({
+ ios: { boxShadow: themeConfig.theme.boxShadow.lg },
+ android: { elevation: 8 },
+ }),
+ }}And import Platform:
+import { Platform } from "react-native";Apply similar changes to line 76.
Also applies to: 76-76
🤖 Prompt for AI Agents
In components/MoreBottomSheet.tsx at lines 49 and 76, the boxShadow style is
used, which is not consistently supported on Android in React Native. To fix
this, import Platform from 'react-native' and apply conditional styling: use
boxShadow for iOS and elevation for Android. Update the style objects at these
lines to include elevation when Platform.OS is 'android' and keep boxShadow for
other platforms.
|
|
||
| const result = await getRecommendations({ | ||
| variables: { | ||
| userId: id!, |
There was a problem hiding this comment.
Potential runtime error with non-null assertion.
Using id! assumes the user ID is always available, but this could cause a runtime error if the user is not authenticated.
Add a safety check:
+ if (!id) {
+ toast({
+ title: "Authentication required",
+ text: "Please log in to get recommendations",
+ });
+ setBackground(false);
+ lottieRef.current?.pause();
+ lottieRef.current?.reset();
+ return;
+ }
+
const result = await getRecommendations({
variables: {
- userId: id!,
+ userId: id,
},
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| userId: id!, | |
| // early-return if there's no authenticated user | |
| if (!id) { | |
| toast({ | |
| title: "Authentication required", | |
| text: "Please log in to get recommendations", | |
| }); | |
| setBackground(false); | |
| lottieRef.current?.pause(); | |
| lottieRef.current?.reset(); | |
| return; | |
| } | |
| const result = await getRecommendations({ | |
| variables: { | |
| userId: id, | |
| }, | |
| }); |
🤖 Prompt for AI Agents
In components/RecommendationButton.tsx at line 68, the use of the non-null
assertion operator on `id!` assumes the user ID is always defined, which can
cause a runtime error if the user is unauthenticated. To fix this, add a safety
check to ensure `id` is defined before using it, such as conditionally handling
the case when `id` is undefined or providing a fallback value, thereby
preventing potential runtime crashes.
| supermarket_id: product?.supermarkets?.reduce( | ||
| (min, current) => (current.price < min.price ? current : min), | ||
| product?.supermarkets?.[0], | ||
| ).id, |
There was a problem hiding this comment.
Potential runtime error in supermarket selection logic.
The reduce operation could fail if product.supermarkets is empty or undefined, causing a runtime error.
Add safety checks for the supermarket selection:
- supermarket_id: product?.supermarkets?.reduce(
- (min, current) => (current.price < min.price ? current : min),
- product?.supermarkets?.[0],
- ).id,
+ supermarket_id: product?.supermarkets?.length > 0
+ ? product.supermarkets.reduce(
+ (min, current) => (current.price < min.price ? current : min),
+ product.supermarkets[0],
+ ).id
+ : 0, // or handle missing supermarket appropriately🤖 Prompt for AI Agents
In components/RecommendationButton.tsx around lines 95 to 98, the reduce
operation on product.supermarkets can cause a runtime error if supermarkets is
empty or undefined. Add a safety check to ensure product.supermarkets is a
non-empty array before calling reduce. If it is empty or undefined, handle the
case gracefully by providing a default value or skipping the assignment to avoid
runtime errors.
| const { data, error, loading } = useQuery(GET_USERS, { | ||
| variables: { name: searchQuery, excludedIds }, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Skip the GET_USERS query until the user actually types something
Right now the query fires on every render with an empty searchQuery, causing an unnecessary round-trip and wasted bandwidth.
Guard the query with skip or debounce the search string before triggering the query.
-const { data, error, loading } = useQuery(GET_USERS, {
- variables: { name: searchQuery, excludedIds },
-});
+const { data, error, loading } = useQuery(GET_USERS, {
+ variables: { name: searchQuery, excludedIds },
+ skip: searchQuery.trim().length === 0,
+});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { data, error, loading } = useQuery(GET_USERS, { | |
| variables: { name: searchQuery, excludedIds }, | |
| }); | |
| const { data, error, loading } = useQuery(GET_USERS, { | |
| variables: { name: searchQuery, excludedIds }, | |
| skip: searchQuery.trim().length === 0, | |
| }); |
🤖 Prompt for AI Agents
In components/AddCollaboratorBottomSheet.tsx around lines 65 to 67, the
GET_USERS query runs on every render even when searchQuery is empty, causing
unnecessary network requests. Fix this by adding a skip condition to the
useQuery hook that prevents the query from running if searchQuery is empty or
only whitespace. Alternatively, implement a debounce on searchQuery so the query
triggers only after the user stops typing for a short delay.
| const [shareList] = useMutation(SHARE_LIST, { | ||
| refetchQueries: ["ShareUsers", "GetUsers"], | ||
| awaitRefetchQueries: true, | ||
| }); | ||
|
|
There was a problem hiding this comment.
Handle missing currentListId before calling shareList
currentListId ?? "" risks sending an empty string to the server which will likely 404/500.
Early-return or disable the action when the ID is undefined.
-const [shareList] = useMutation(SHARE_LIST, {
+const [shareList, { loading: shareLoading }] = useMutation(SHARE_LIST, {
refetchQueries: ["ShareUsers", "GetUsers"],
awaitRefetchQueries: true,
});and inside the render:
-onPress={async () => {
- const result = await shareList({
- variables: {
- listId: currentListId ?? "",
- userId: item._id,
- },
- onError: toastOnError,
- });
- if (result) internalRef.current?.close();
-}}
+onPress={async () => {
+ if (!currentListId) return toastOnError(new Error("No list selected"));
+ const { data } = await shareList({
+ variables: { listId: currentListId, userId: item._id },
+ onError: toastOnError,
+ });
+ if (data) internalRef.current?.close();
+}}
+disabled={!currentListId || shareLoading}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [shareList] = useMutation(SHARE_LIST, { | |
| refetchQueries: ["ShareUsers", "GetUsers"], | |
| awaitRefetchQueries: true, | |
| }); | |
| // Hook – include loading state from the mutation | |
| const [shareList, { loading: shareLoading }] = useMutation(SHARE_LIST, { | |
| refetchQueries: ["ShareUsers", "GetUsers"], | |
| awaitRefetchQueries: true, | |
| }); | |
| // … later, in your JSX where you render the “Share” action … | |
| <YourButtonOrTouchable | |
| onPress={async () => { | |
| if (!currentListId) { | |
| // early‐return if no list is selected | |
| return toastOnError(new Error("No list selected")); | |
| } | |
| const { data } = await shareList({ | |
| variables: { | |
| listId: currentListId, | |
| userId: item._id, | |
| }, | |
| onError: toastOnError, | |
| }); | |
| if (data) { | |
| internalRef.current?.close(); | |
| } | |
| }} | |
| disabled={!currentListId || shareLoading} | |
| > | |
| Share | |
| </YourButtonOrTouchable> |
🤖 Prompt for AI Agents
In components/AddCollaboratorBottomSheet.tsx around lines 69 to 73, the code
uses currentListId ?? "" when calling shareList, which can send an empty string
to the server causing errors. To fix this, add a check before calling shareList
to ensure currentListId is defined; if it is undefined, either return early from
the function or disable the UI action that triggers shareList to prevent invalid
requests.
| excludedIds={[ | ||
| data?.list?.owner?._id ?? "", | ||
| ...(data?.list?.collaborators?.map?.( | ||
| (collaborator) => collaborator._id, | ||
| ) ?? []), | ||
| ]} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Filter empty strings from excludedIds to avoid leaking empty values
When owner?._id is undefined the array will contain "", which the backend may interpret as a valid ID.
excludedIds={[
- data?.list?.owner?._id ?? "",
- ...(data?.list?.collaborators?.map?.((c) => c._id) ?? []),
+ ...[
+ data?.list?.owner?._id,
+ ...(data?.list?.collaborators?.map((c) => c._id) ?? []),
+ ].filter(Boolean),
]}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| excludedIds={[ | |
| data?.list?.owner?._id ?? "", | |
| ...(data?.list?.collaborators?.map?.( | |
| (collaborator) => collaborator._id, | |
| ) ?? []), | |
| ]} | |
| excludedIds={[ | |
| ...[ | |
| data?.list?.owner?._id, | |
| ...(data?.list?.collaborators?.map((c) => c._id) ?? []), | |
| ].filter(Boolean), | |
| ]} |
🤖 Prompt for AI Agents
In components/CollaboratorsBottomSheet.tsx around lines 142 to 147, the
excludedIds array includes an empty string when owner._id is undefined, which
can cause backend issues. Update the code to filter out any empty strings from
the excludedIds array after constructing it, ensuring only valid IDs are passed.
| listId: currentListId || "", | ||
| productEan: item.ean, |
There was a problem hiding this comment.
Use the definitive listId instead of falling back to an empty string
If currentListId is undefined here the mutation will send "", while data.list._id is guaranteed to exist (otherwise we wouldn’t render this screen).
- ids: {
- listId: currentListId || "",
+ ids: {
+ listId: data.list._id,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| listId: currentListId || "", | |
| productEan: item.ean, | |
| listId: data.list._id, | |
| productEan: item.ean, |
🤖 Prompt for AI Agents
In app/(tabs)/shopping-list.tsx around lines 322 to 323, replace the fallback
empty string for listId with the definitive listId from data.list._id. Since
data.list._id is guaranteed to exist when rendering this screen, use it directly
instead of currentListId || "" to ensure the mutation sends the correct listId.
| const [createList] = useMutation(CREATE_LIST, { | ||
| refetchQueries: [ | ||
| { | ||
| query: GET_PRODUCTS_LIST, | ||
| variables: { userId: id }, | ||
| }, | ||
| ], | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
refetchQueries uses the wrong variable – this will break the query at runtime
GET_PRODUCTS_LIST expects a listId, but the refetch object sends { userId: id }.
Apollo will therefore complain about a missing required variable and the refetch will never execute.
Two safe fixes:
- const [createList] = useMutation(CREATE_LIST, {
- refetchQueries: [
- {
- query: GET_PRODUCTS_LIST,
- variables: { userId: id },
- },
- ],
- });
+ const [createList] = useMutation(CREATE_LIST /* no eager refetch */);and, inside onCompleted, trigger the refetch once you know the new list id:
onCompleted: ({ createList }) => {
if (createList?._id) {
setCurrentListId(createList._id);
client.refetchQueries({
include: [GET_PRODUCTS_LIST],
variables: { listId: createList._id },
});
}
}Alternatively, if the surrounding screen already reacts to currentListId, you can drop refetchQueries altogether.
🤖 Prompt for AI Agents
In components/NoListAvailable.tsx around lines 45 to 52, the refetchQueries
option in the useMutation hook incorrectly passes { userId: id } as variables to
GET_PRODUCTS_LIST, which expects a listId. To fix this, remove the
refetchQueries option and instead add an onCompleted callback that checks if
createList._id exists, then sets the current list ID state and triggers
client.refetchQueries with the correct variables { listId: createList._id }.
Alternatively, if the UI already updates based on currentListId, you can remove
refetchQueries entirely.
| error | ||
| ? undefined | ||
| : () => { | ||
| setCurrentListId( | ||
| data?.user?.actualList?._id ?? null, | ||
| ).catch(toastOnError); | ||
| } | ||
| } |
There was a problem hiding this comment.
setCurrentListId(...).catch() is a no-op and can crash in production
setCurrentListId is a React state setter (synchronous, returns void).
Calling .catch() on void throws TypeError: undefined is not a function in JS.
-() => {
- setCurrentListId(data?.user?.actualList?._id ?? null).catch(toastOnError);
-}
+() => {
+ try {
+ setCurrentListId(data?.user?.actualList?._id ?? null);
+ } catch (err) {
+ toastOnError(err as Error);
+ }
+}Apply the same change to the collaborator list handler below.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| error | |
| ? undefined | |
| : () => { | |
| setCurrentListId( | |
| data?.user?.actualList?._id ?? null, | |
| ).catch(toastOnError); | |
| } | |
| } | |
| error | |
| ? undefined | |
| : () => { | |
| try { | |
| setCurrentListId( | |
| data?.user?.actualList?._id ?? null, | |
| ); | |
| } catch (err) { | |
| toastOnError(err as Error); | |
| } | |
| } |
🤖 Prompt for AI Agents
In components/AvailableListBottomSheet.tsx around lines 110 to 117, the code
incorrectly calls .catch() on setCurrentListId, which is a React state setter
returning void, causing a runtime error. Remove the .catch(toastOnError) call
after setCurrentListId to fix this. Also, locate the similar usage in the
collaborator list handler below and apply the same fix by removing the .catch()
call there as well.
| export type GetFilteredSupermarketsQueryVariables = Exact<{ | ||
| userId: Scalars["String"]["input"]; | ||
| listId: Scalars["String"]["input"]; | ||
| coordinateFilter: CoordinateFilter; | ||
| }>; | ||
|
|
||
| export type GetFilteredSupermarketsQuery = { | ||
| __typename?: "Query"; | ||
| user?: { | ||
| __typename?: "User"; | ||
| actualList?: { | ||
| __typename?: "SupermarketList"; | ||
| products: Array<{ | ||
| __typename?: "ListProduct"; | ||
| quantity: number; | ||
| supermarketInfo: { __typename?: "SupermarketWithPrice"; price: number }; | ||
| }>; | ||
| } | null; | ||
| list?: { | ||
| __typename?: "SupermarketList"; | ||
| products: Array<{ | ||
| __typename?: "ListProduct"; | ||
| quantity: number; | ||
| supermarketInfo: { __typename?: "SupermarketWithPrice"; price: number }; | ||
| }>; | ||
| } | null; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
GetFilteredSupermarkets query omits product identifiers
The selection set under list → products only includes quantity and supermarketInfo.price.
Down-stream code that needs to reconcile quantities with specific products (e.g., subtotal calculations or “tick off item” UI) won’t have a stable key (ean or composite id) to index by, forcing fragile positional joins.
- selections: [
- ... quantity & price only ...
+ selections: [
+ { name: { value: "product" } selectionSet: {
+ selections: [
+ { name: { value: "ean" } }
+ ]
+ }},
+ ...quantity & price...
]Adding at least product.ean (or id_composite) keeps the payload minimal while enabling robust look-ups.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In graphql/graphql.ts around lines 645 to 659, the GetFilteredSupermarkets
query's selection set under list → products lacks a product identifier like
product.ean or id_composite. To fix this, add the product identifier field (ean
or id_composite) to the products selection set so downstream code can reliably
index products for operations like subtotal calculations or UI updates without
relying on fragile positional joins.
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Documentation
Style