Conversation
Implements the Friends screen with three internal tabs (Friends, Search, Requests), sign-in gate for unauthenticated users, email search with Add/Friends/Sent states, incoming and outgoing request lists.
- removeFriend now also deletes both directions of the friendRequests document so re-requests after unfriending don't hit a permission error - Add onSnapshot error callbacks for all three listeners in useSocialListeners - Clear social store on sign-out to prevent previous user's data persisting - handleSearch in SearchTab now catches errors, reports to Bugsnag, and shows an error message to the user - FriendRequestItem mutually disables Accept/Decline buttons while either mutation is in-flight via isBusy flag - Remove unused borders prop from FriendsTab call site and interface - Update sendFriendRequest comment to use RNFirebase API name (.set())
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (20)
hooks/usePublishedWorkoutIdsQuery.ts (1)
10-10: ⚡ Quick winInclude
user.uidin the query key to avoid cross-account cache bleed.The key
["publishedWorkoutIds"]is static, so a logout/login as a different user can serve the previous account's cached IDs until the entry goes stale or refetches. The friend query hooks already scope their keys by uid; mirror that here.♻️ Suggested change
- queryKey: ["publishedWorkoutIds"], + queryKey: ["publishedWorkoutIds", user?.uid],🤖 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 `@hooks/usePublishedWorkoutIdsQuery.ts` at line 10, The query key for the hook usePublishedWorkoutIdsQuery is currently static ("publishedWorkoutIds") which causes cross-account cache bleed; update the queryKey in that hook to include the current user's uid (e.g., ["publishedWorkoutIds", user.uid] or ["publishedWorkoutIds", user?.uid] if user may be null) so cached results are scoped per user, and ensure any conditional logic (refetching or enabled flags) uses the same uid presence check to avoid running the query for anonymous/undefined users.hooks/useSocialListeners.ts (1)
40-57: ⚖️ Poor tradeoffN+1 Firestore reads per snapshot.
Each snapshot triggers one extra
users/{uid}read per document. With many friends/requests this multiplies read costs and latency on every realtime update. If profile fields (displayName/photoURL) can be denormalised onto the request/friend docs at write time, the listener could avoid these per-doc lookups entirely.🤖 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 `@hooks/useSocialListeners.ts` around lines 40 - 57, The snapshot mapping currently performs an N+1 read by calling db.collection("users").doc(...).get() per doc (see snapshot.docs map, PendingRequest creation), so change the listener to first use denormalized profile fields on the request/friend document (use data.displayName/data.email/data.photoURL when present) and avoid per-doc lookups; if denormalization cannot be applied immediately, replace the per-doc get with a single batched lookup by collecting unique from UIDs and fetching them once (use a single query with where(FieldPath.documentId(), 'in', uids) or getAll) then map results into the PendingRequest objects to populate displayName/email/photoURL, eliminating the per-document db.collection("users").doc(...).get() calls.components/PlanList.tsx (1)
63-66: 💤 Low valueConsider extracting the duplicated
isPublishedcomputation.The same expression is repeated across the three render modes. A small memoised helper (ideally a
Setfor O(1) lookups) keeps the logic in one place and avoids future divergence.♻️ Example
const { width: screenWidth } = useWindowDimensions(); const gridCardWidth = Math.floor((screenWidth - 22) / 2) - 10; + + const publishedSet = useMemo( + () => new Set(publishedPlanIds ?? []), + [publishedPlanIds], + ); + const isPlanPublished = (item: Plan) => + !!sharePlansEnabled && publishedSet.has(String(item.id));Then use
isPublished={isPlanPublished(item)}in each render mode.Also applies to: 86-89, 112-115
🤖 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 `@components/PlanList.tsx` around lines 63 - 66, Extract the duplicated isPublished logic into a small memoised helper: inside the PlanList component create a useMemo that builds a Set from publishedPlanIds (e.g., const publishedPlanIdSet = useMemo(() => new Set((publishedPlanIds || []).map(String)), [publishedPlanIds]) and then expose a function like isPlanPublished = (plan) => !!sharePlansEnabled && publishedPlanIdSet.has(String(plan.id)); replace the three inline expressions (the isPublished prop at the locations using publishedPlanIds and sharePlansEnabled) with isPublished={isPlanPublished(item)} to ensure O(1) lookups and avoid duplication.hooks/usePrivacySettingsMutation.ts (2)
13-13: 💤 Low valueConsider a more descriptive error message.
The generic "Not authenticated" error could be improved for better debugging and user feedback.
💡 Proposed improvement
- if (!user) throw new Error("Not authenticated"); + if (!user) throw new Error("User must be authenticated to update privacy settings");🤖 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 `@hooks/usePrivacySettingsMutation.ts` at line 13, The thrown error "Not authenticated" in usePrivacySettingsMutation is too generic—replace it with a more descriptive message that includes the context/action (e.g., "User not authenticated in usePrivacySettingsMutation: cannot update privacy settings") so callers and logs can identify where and why authentication failed; update the throw in the if (!user) check inside the usePrivacySettingsMutation function accordingly.
11-24: ⚡ Quick winConsider invalidating privacy settings queries on success.
After updating privacy settings, related queries should be invalidated to reflect the changes. The mutation doesn't include an
onSuccesshandler to invalidate cached privacy settings.♻️ Proposed improvement
+ const queryClient = useQueryClient(); + return useMutation({ mutationFn: async (patch: Partial<FirestorePrivateSettings>) => { if (!user) throw new Error("Not authenticated"); await firestore() .collection("users") .doc(user.uid) .collection("private") .doc("settings") .update(patch); }, + onSuccess: () => { + queryClient.invalidateQueries({ queryKey: ["privacySettings"] }); + }, onError: (error: Error) => { Bugsnag.notify(error); }, });🤖 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 `@hooks/usePrivacySettingsMutation.ts` around lines 11 - 24, Add an onSuccess handler to usePrivacySettingsMutation that, after the mutation completes, obtains a react-query QueryClient (via useQueryClient) and calls queryClient.invalidateQueries for the privacy settings query key so cached privacy settings are refreshed; modify usePrivacySettingsMutation to import/use useQueryClient, add onSuccess alongside onError, and call queryClient.invalidateQueries with the same key used by the privacy-settings query (e.g., 'privacySettings' or ['privacySettings', user.uid] or the helper key function if one exists).app/(app)/settings.tsx (1)
84-107: ⚡ Quick winConsider providing user feedback after successful deletion.
The deletion handler shows a loading state but doesn't inform the user when the operation completes successfully. Consider showing a success message or snackbar.
💡 Suggested enhancement
try { await deleteAllSharedData(user.uid); + Alert.alert( + t`Success`, + t`All shared data has been deleted.`, + ); } catch (error: any) { Bugsnag.notify(error); + Alert.alert( + t`Error`, + t`Failed to delete shared data. Please try again.`, + ); } finally { setIsDeletingSharedData(false); }🤖 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 `@app/`(app)/settings.tsx around lines 84 - 107, The handleDeleteAllSharedData flow currently sets isDeletingSharedData and calls deleteAllSharedData but never informs the user on success; after the await deleteAllSharedData(user.uid) completes, add user feedback (e.g., Alert.alert or a Snackbar) indicating successful deletion, and only clear isDeletingSharedData in finally as currently done; update the onPress success branch inside handleDeleteAllSharedData to call the chosen feedback API (e.g., Alert.alert("Deleted", "All shared data removed") or showSnackbar) and keep Bugsnag.notify(error) in the catch for failures.hooks/useFriendSharedStandaloneWorkoutsQuery.ts (1)
20-20: ⚖️ Poor tradeoffType casting assumes valid Firestore data structure.
The cast
d.data() as SharedStandaloneWorkoutassumes the Firestore document matches the TypeScript type. If the data structure changes or is malformed, this could cause runtime issues. Consider adding runtime validation or at least documenting the assumption.🤖 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 `@hooks/useFriendSharedStandaloneWorkoutsQuery.ts` at line 20, The current map call uses an unsafe cast (d.data() as SharedStandaloneWorkout) which can hide malformed Firestore documents; add runtime validation instead of direct casting by introducing a type-guard or schema validator (e.g., isSharedStandaloneWorkout or a Zod/IO-TS schema) and use it inside the useFriendSharedStandaloneWorkoutsQuery mapping: for each d.data() validate required fields and types, skip or log/collect invalid documents, and only return objects that pass validation as SharedStandaloneWorkout so consumers no longer rely on unchecked casts.app/(app)/friend-exercise.tsx (2)
135-140: ⚡ Quick winConsider adding error feedback for failed imports.
The mutation doesn't handle
onError, so users won't receive feedback if the import fails. TheimportExercise.isPendingstate provides loading feedback, but errors are silent.💡 Suggested improvement
onPress={() => importExercise.mutate(exercise, { onSuccess: () => setImported(true), + onError: (error) => { + Alert.alert( + t`Error`, + t`Failed to import exercise. Please try again.`, + ); + }, }) }🤖 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 `@app/`(app)/friend-exercise.tsx around lines 135 - 140, The import button’s mutation lacks error handling so failures are silent; update the call to importExercise.mutate (used where setImported and importExercise.isPending are referenced) to include an onError callback that surfaces the error to the user (e.g., show a toast/alert or set an error state) and optionally resets any loading/disabled state so the user can retry; ensure onSuccess still sets setImported(true) and that importExercise.isPending/ imported are updated appropriately.
31-31: ⚖️ Poor tradeoffThe imported state resets on component remount.
Using
useState(false)means the "imported" flag resets if the user navigates away and returns. Consider checking against the user's existing exercises or using a more persistent approach.🤖 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 `@app/`(app)/friend-exercise.tsx at line 31, The imported flag initialized with useState(false) in the FriendExercise component resets on remount; instead derive its initial value from a persistent source (e.g., the current user's exercises or localStorage) and set it on mount. Replace useState(false) with useState(() => checkIfImported()) where checkIfImported reads from the user's existing exercises prop/selector (or localStorage key) or call an effect like useEffect(() => { if (userExercises.includes(exerciseId)) setImported(true) }, [userExercises, exerciseId]) so the flag reflects persisted/imported state rather than always defaulting to false; update setImported use sites accordingly. Ensure any server-side truth (userExercises) is preferred over client-only storage if available.hooks/useWorkoutPublishQuery.ts (1)
19-19: ⚡ Quick winImprove type safety for Firestore document exists check.
The type assertion
(doc as any).existsbypasses TypeScript's type checking. React Native Firebase's document snapshot should have a properly typedexistsproperty.♻️ Suggested improvement
- return !!(doc as any).exists; + return !!doc.exists;Ensure you're using the correct types from
@react-native-firebase/firestore.🤖 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 `@hooks/useWorkoutPublishQuery.ts` at line 19, The current existence check uses an unsafe cast `(doc as any).exists`; instead type the Firestore snapshot properly and use its typed exists property. Import the DocumentSnapshot type from `@react-native-firebase/firestore`, annotate the `doc` parameter/variable in `useWorkoutPublishQuery` (or the function that receives the snapshot) as DocumentSnapshot<T> (or appropriate generic), and replace `(doc as any).exists` with `doc.exists`; update any related return/type annotations so TypeScript infers a boolean without using `any`.hooks/useFriendSharedCompletedWorkoutsQuery.ts (1)
20-20: ⚡ Quick winConsider validating Firestore data before type assertion.
The direct type assertion
d.data() as SharedCompletedWorkoutassumes the Firestore document structure always matches the TypeScript type. If the data schema diverges or is malformed, this could lead to runtime errors or type mismatches that TypeScript cannot catch.Consider adding runtime validation or at minimum document the expected schema to prevent future issues.
🤖 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 `@hooks/useFriendSharedCompletedWorkoutsQuery.ts` at line 20, The map over Firestore docs in useFriendSharedCompletedWorkoutsQuery.ts currently does a blind cast (d.data() as SharedCompletedWorkout); replace this with runtime validation: implement or import a validator (e.g., validateSharedCompletedWorkout or a zod/schema function) and for each doc call the validator on d.data(), filter out or log invalid docs, and only return validated SharedCompletedWorkout objects (or throw/handle errors as appropriate) so you never assert the type without checking the document shape first.hooks/useCreatePlan.ts (2)
73-73: ⚡ Quick winImprove type safety for Firestore document exists check.
The type assertion
(snap as any).existsbypasses TypeScript's type checking. React Native Firebase'sFirebaseFirestoreTypes.DocumentSnapshotshould have anexistsproperty that can be accessed with proper typing.♻️ Suggested improvement
- if ((snap as any).exists) { + if (snap.exists) {Ensure you're importing the correct types from
@react-native-firebase/firestore.🤖 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 `@hooks/useCreatePlan.ts` at line 73, The check uses an unsafe cast `(snap as any).exists`; update the snapshot typing so you can use `snap.exists` directly: import the correct `FirebaseFirestoreTypes` from `@react-native-firebase/firestore`, type the snapshot variable (e.g., as `FirebaseFirestoreTypes.DocumentSnapshot` or `DocumentSnapshot<YourDocType>`) in the `useCreatePlan` logic, and replace `(snap as any).exists` with `snap.exists`; ensure any helper functions that receive the snapshot also accept the typed `DocumentSnapshot`.
64-76: ⚖️ Poor tradeoffConsider notifying users about automatic republishing.
The plan is silently republished when a user updates an already-shared plan. If the republish fails (error caught at line 74), the user receives no feedback, which could lead to confusion about whether their shared plan is up to date.
Consider:
- Showing a toast/alert when republishing succeeds or fails
- Adding a visual indicator that republishing is in progress
- Documenting this auto-republish behaviour in user-facing help text
🤖 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 `@hooks/useCreatePlan.ts` around lines 64 - 76, The auto-republish block (in hooks/useCreatePlan.ts) currently fires publishPlan(user.uid, planId).catch(Bugsnag.notify) silently; change it to set a local republishing state (e.g., isRepublishing) before calling publishPlan, await the promise, then clear the state and show a user-facing success toast on resolve or an error toast plus Bugsnag.notify(err) on reject, so users see progress and outcome; also add/update the relevant user-facing help text to document the auto-republish behaviour.hooks/useFriendSharedMeasurementsQuery.ts (1)
18-18: ⚡ Quick winConsider validating Firestore data before type assertion.
The direct type assertion
d.data() as SharedMeasurementassumes the Firestore document structure always matches the TypeScript type. If the data schema diverges or is malformed, this could lead to runtime errors or type mismatches that TypeScript cannot catch.Consider adding runtime validation (e.g., using a library like Zod or a simple validation function) or at minimum add a comment documenting the expected schema.
🤖 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 `@hooks/useFriendSharedMeasurementsQuery.ts` at line 18, The current mapping in useFriendSharedMeasurementsQuery uses snap.docs.map((d) => d.data() as SharedMeasurement) which unsafely asserts Firestore data to the SharedMeasurement type; replace this with runtime validation: parse each d.data() using a validator (e.g., a Zod schema for SharedMeasurement or a small validateSharedMeasurement(obj) function) inside the map/filter pipeline, skip or log invalid docs, and only return objects that pass validation so consumers of useFriendSharedMeasurementsQuery receive guaranteed-valid SharedMeasurement instances; update or add a short comment describing the expected Firestore schema near the validation logic.hooks/useImportCustomExerciseMutation.ts (1)
11-33: ⚡ Quick winDuplicated custom-exercise resolution logic that has already diverged.
The lookup-by-name (
app_exercise_id IS NULL) plus insert flow here is the same asresolveExerciseIdinutils/importUtils.ts(lines 27-48), but the two have already drifted: this hook persistsdescriptionwhileresolveExerciseIddoes not. That divergence is a maintenance hazard — a future schema change has to be applied in two places. Consider extracting a single shared helper (e.g. accepting an optionaldescription) and having both call sites use it.Want me to draft a shared helper that both this hook and
resolveExerciseIdcan use?🤖 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 `@hooks/useImportCustomExerciseMutation.ts` around lines 11 - 33, The hook duplicates the "lookup by name (app_exercise_id IS NULL) then INSERT" flow already implemented in resolveExerciseId in utils/importUtils.ts and has diverged by persisting description; extract a single shared helper (e.g., resolveOrCreateCustomExercise) that implements the SELECT ... WHERE app_exercise_id IS NULL lookup and the INSERT of the same columns, accepting an optional description parameter and the fields currently passed from useImportCustomExerciseMutation (name, bodyPart, targetMuscle, equipment, secondaryMuscles, trackingType, isUnilateral, doubleWeight, animatedUrl, description), replace the logic in useImportCustomExerciseMutation to call that helper and update resolveExerciseId to call the same helper (omitting description when not available) so all creation logic is centralized.utils/sharing.ts (3)
77-113: 💤 Low valueConsider propagating errors for operations that may need retry logic.
The
publishPlanfunction catches errors and reports to Bugsnag but doesn't re-throw. This means callers cannot detect failures or implement retry logic. If this is intentional (fire-and-forget), consider adding a brief comment to document this design choice.The same pattern applies to
unpublishPlan,publishStandaloneWorkout,pushCustomExercise, and similar functions in this file.🤖 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 `@utils/sharing.ts` around lines 77 - 113, The try/catch in publishPlan swallows errors after reporting to Bugsnag, preventing callers from detecting failures or retrying; update publishPlan (and similar functions like unpublishPlan, publishStandaloneWorkout, pushCustomExercise) to either re-throw the caught error after Bugsnag.notify(error) or explicitly document the fire-and-forget behavior with a comment, so callers know they cannot rely on success—choose one approach and apply consistently across those named functions.
314-358: 💤 Low valueBatch size is appropriate but consider documenting the assumption.
Firestore batches are limited to 500 operations. The strength PR push doesn't enforce this limit, relying on the assumption that a single workout won't contain 500+ exercises. This is reasonable, but a brief comment or a check would make the code more defensive.
🤖 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 `@utils/sharing.ts` around lines 314 - 358, pushStrengthPRs assumes fewer than 500 batched writes but doesn’t enforce Firestore’s 500-operation limit; update pushStrengthPRs to either document this assumption and early-return when prData is empty or, better, split prData into chunks of <=500 and create/commit a new firestore().batch() for each chunk (use fetchPRDataForExercises result length to drive chunking), building each batch with the same per-PR logic and awaiting each batch.commit() in sequence; also add a brief inline comment referencing the 500-op limit to make the assumption explicit.
93-96: 💤 Low valueMinor: Avoid type assertion for DocumentSnapshot.
The
(existing as any).existscast can be replaced with proper typing. The@react-native-firebase/firestoreDocumentSnapshottype has anexistsproperty.Suggested improvement
const existing = await ref.get(); - const publishedAt = (existing as any).exists + const publishedAt = existing.exists ? existing.data()?.publishedAt : now;🤖 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 `@utils/sharing.ts` around lines 93 - 96, The code uses a type assertion "(existing as any).exists" when reading the DocumentSnapshot from "const existing = await ref.get();": instead, import and use the proper DocumentSnapshot type from '`@react-native-firebase/firestore`' (e.g., FirebaseFirestoreTypes.DocumentSnapshot) and annotate the variable (const existing: FirebaseFirestoreTypes.DocumentSnapshot = await ref.get();) so you can use "existing.exists" directly when computing "publishedAt"; remove the any cast and ensure the firestore types import is added if missing.utils/database.ts (1)
3094-3174: 💤 Low valueComplex PR calculation query looks correct but consider performance for large datasets.
The SQL query uses window functions and a subquery for
bodyWeightthat executes per row. For typical workout sizes this is fine, but if users have extensive exercise history, consider:
- Moving the bodyWeight lookup outside the main query and passing it as a parameter.
- Adding an index on
completed_exercises(exercise_id)if not present.Otherwise, the logic correctly handles different tracking types and the Epley-style formula for weight-based exercises.
🤖 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 `@utils/database.ts` around lines 3094 - 3174, The SQL in fetchPRDataForExercises uses a per-row subquery for settings (bodyWeight) inside pmExpr which can be slow on large histories; extract the bodyWeight once before building pmExpr (e.g., query settings for key 'bodyWeight' and pass the numeric value into the SQL as a bound parameter) and replace the subquery CAST((SELECT value FROM settings WHERE key = 'bodyWeight') AS REAL) in pmExpr with that parameter; also ensure there is an index on completed_exercises(exercise_id) (or create one) to speed the JOINs referenced in the same function.firestore.rules (1)
59-72: 💤 Low valueConsider restricting friend request deletion to prevent re-request abuse.
The current rules allow sender to create (which enables re-request after decline), but there's no explicit delete rule. By default, delete is denied, which is correct. However, the comment on line 60 mentions "enables re-request after decline" - verify the application handles declined requests appropriately (e.g., by updating status rather than creating new documents).
🤖 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 `@firestore.rules` around lines 59 - 72, The friendRequests rules currently allow create/overwrite and lack an explicit delete rule, which can enable re-request abuse; update the match /friendRequests/{requestId} block by (1) changing the allow create condition on request.auth.uid == request.resource.data.from to also require that no existing request is being overwritten (e.g., resource == null) or that the existing resource.data.status is not 'declined' so senders cannot recreate a request immediately after a decline, and (2) add an explicit allow delete rule (e.g., allow delete: if false) to make deletion intent explicit and prevent accidental removals; refer to the existing allow create, allow update, and resource/request.resource symbols when applying these checks.
🤖 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 `@app/`(app)/(tabs)/(plans)/standalone-workout.tsx:
- Around line 269-301: The publish toggle currently calls
publishMutation.mutate(!isPublished) but lacks user-facing error feedback;
update the mutation used to create publishMutation to include an onError handler
that surfaces failures (e.g., show an Alert or toast with a localized message)
and/or render inline feedback in standalone-workout.tsx by checking
publishMutation.isError or publishMutation.error and displaying a ThemedText or
error UI near the Switch; ensure the UI still disables during
publishMutation.isPending or isPublishLoading and that the onError clears any
local loading state so the toggle accurately reflects the actual isPublished
state.
In `@app/`(app)/friend-plan.tsx:
- Around line 124-127: The JSX currently only checks exercise.sets[0]?.repsMin
before rendering the range but still interpolates exercise.sets[0].repsMax,
causing "undefined reps" when repsMax is missing; update the conditional around
the template that emits ` ·
${exercise.sets[0].repsMin}–${exercise.sets[0].repsMax} reps` to require both
exercise.sets[0]?.repsMin != null and exercise.sets[0]?.repsMax != null (or
otherwise render a single value/omit the range when repsMax is null), targeting
the JSX that reads exercise.sets and the expression using repsMin/repsMax.
In `@app/`(app)/friend-workout.tsx:
- Around line 83-86: The reps suffix is hardcoded and not localized; update the
rendering in friend-workout.tsx where you build the reps string for
exercise.sets[0] (inside the AppText block) to use the i18n translation API or
<Trans> instead of the literal "reps" (e.g., call t('reps') or wrap the whole "·
{min}–{max} reps" segment in <Trans> with interpolation) so the label for reps
is translated across locales.
- Around line 62-68: Guard against a null/pending updatedAt before calling
toDate() by checking workout.updatedAt (e.g. if (!workout.updatedAt) use a
fallback like new Date() or render a placeholder) when calling
formatDistanceToNow so formatDistanceToNow(workout.updatedAt?.toDate() ?? new
Date(), { addSuffix: true }) won’t throw; also replace the hardcoded reps string
with a localized string (wrap the reps output in <Trans> or use the existing
i18n helper consistent with how sets are rendered) so the reps range is
internationalized alongside sets.
In `@app/`(app)/friends.tsx:
- Around line 174-179: The search card logic only checks friends and
sentRequests, so incoming pendingRequests are ignored; update the component to
also read pendingRequests from useSocialStore and compute a hasIncoming flag
(e.g., hasIncoming = result ? pendingRequests.some(p => p.fromUid === result.uid
&& p.toUid === myUid) : false), then use hasIncoming to suppress the "Add"
affordance (or render an "Accept" affordance) before rendering the Add button;
adjust the existing isFriend and hasPendingSent branches to consider hasIncoming
so you don't create a reverse request when an incoming request already exists.
In `@app/`(app)/settings.tsx:
- Around line 98-99: Replace the loose any typing in the catch and guard before
passing to Bugsnag: change the catch parameter from any to unknown (catch
(error: unknown)), then inside the catch use a type guard (e.g., check error
instanceof Error) and call Bugsnag.notify with the Error when true, otherwise
convert the unknown to an Error (e.g., new Error(String(error))) before
notifying; target the catch block that currently calls Bugsnag.notify(error) so
you update that path safely.
In `@firestore.rules`:
- Around line 25-30: The rule for match /friends/{friendUid} currently allows
writes if request.auth.uid == friendUid; change the write condition so that when
the writer is the friend (request.auth.uid == friendUid) it additionally
requires a pending friend request document at friendsRequests/{friendUid + '_' +
uid} with status == "pending" (and optionally enforce allowed field updates),
while preserving the existing allowance for the owner (request.auth.uid == uid);
update the allow write expression inside match /friends/{friendUid} to check
existence and status of that friendRequests document before permitting friendUid
to create/update the friendship record.
In `@hooks/usePlanPublishQuery.ts`:
- Line 19: The return currently casts doc to any to access exists; instead rely
on Firestore's typed DocumentSnapshot.exists property—remove the cast and return
the boolean using doc.exists directly (ensure the parameter/variable `doc` in
the `usePlanPublishQuery` function is typed as a Firestore DocumentSnapshot or
compatible type so TypeScript recognizes `.exists`).
In `@hooks/usePublishedPlanIdsQuery.ts`:
- Around line 9-22: The query key for useQuery in usePublishedPlanIdsQuery is
currently static (["publishedPlanIds"]) which can cause cache collisions across
users; update the queryKey to include the current user's id (e.g.,
["publishedPlanIds", user?.uid] or ["publishedPlanIds", user.uid]) so cached
results are scoped per account, and ensure the enabled flag still uses !!user so
the query only runs when user is present (refer to usePublishedPlanIdsQuery and
the queryKey property).
In `@utils/userProfile.ts`:
- Around line 33-40: The snapshot existence checks in utils/userProfile.ts use
the old property form (userDoc.exists and settingsDoc.exists); update those to
call the method form userDoc.exists() and settingsDoc.exists() so the
initial-creation branch runs correctly (ensuring profileData.createdAt is set
and DEFAULT_PRIVACY_SETTINGS is written); keep the rest of the logic intact
around userRef.set(profileData, { merge: true }) and
privateSettingsRef.set(DEFAULT_PRIVACY_SETTINGS).
---
Nitpick comments:
In `@app/`(app)/friend-exercise.tsx:
- Around line 135-140: The import button’s mutation lacks error handling so
failures are silent; update the call to importExercise.mutate (used where
setImported and importExercise.isPending are referenced) to include an onError
callback that surfaces the error to the user (e.g., show a toast/alert or set an
error state) and optionally resets any loading/disabled state so the user can
retry; ensure onSuccess still sets setImported(true) and that
importExercise.isPending/ imported are updated appropriately.
- Line 31: The imported flag initialized with useState(false) in the
FriendExercise component resets on remount; instead derive its initial value
from a persistent source (e.g., the current user's exercises or localStorage)
and set it on mount. Replace useState(false) with useState(() =>
checkIfImported()) where checkIfImported reads from the user's existing
exercises prop/selector (or localStorage key) or call an effect like
useEffect(() => { if (userExercises.includes(exerciseId)) setImported(true) },
[userExercises, exerciseId]) so the flag reflects persisted/imported state
rather than always defaulting to false; update setImported use sites
accordingly. Ensure any server-side truth (userExercises) is preferred over
client-only storage if available.
In `@app/`(app)/settings.tsx:
- Around line 84-107: The handleDeleteAllSharedData flow currently sets
isDeletingSharedData and calls deleteAllSharedData but never informs the user on
success; after the await deleteAllSharedData(user.uid) completes, add user
feedback (e.g., Alert.alert or a Snackbar) indicating successful deletion, and
only clear isDeletingSharedData in finally as currently done; update the onPress
success branch inside handleDeleteAllSharedData to call the chosen feedback API
(e.g., Alert.alert("Deleted", "All shared data removed") or showSnackbar) and
keep Bugsnag.notify(error) in the catch for failures.
In `@components/PlanList.tsx`:
- Around line 63-66: Extract the duplicated isPublished logic into a small
memoised helper: inside the PlanList component create a useMemo that builds a
Set from publishedPlanIds (e.g., const publishedPlanIdSet = useMemo(() => new
Set((publishedPlanIds || []).map(String)), [publishedPlanIds]) and then expose a
function like isPlanPublished = (plan) => !!sharePlansEnabled &&
publishedPlanIdSet.has(String(plan.id)); replace the three inline expressions
(the isPublished prop at the locations using publishedPlanIds and
sharePlansEnabled) with isPublished={isPlanPublished(item)} to ensure O(1)
lookups and avoid duplication.
In `@firestore.rules`:
- Around line 59-72: The friendRequests rules currently allow create/overwrite
and lack an explicit delete rule, which can enable re-request abuse; update the
match /friendRequests/{requestId} block by (1) changing the allow create
condition on request.auth.uid == request.resource.data.from to also require that
no existing request is being overwritten (e.g., resource == null) or that the
existing resource.data.status is not 'declined' so senders cannot recreate a
request immediately after a decline, and (2) add an explicit allow delete rule
(e.g., allow delete: if false) to make deletion intent explicit and prevent
accidental removals; refer to the existing allow create, allow update, and
resource/request.resource symbols when applying these checks.
In `@hooks/useCreatePlan.ts`:
- Line 73: The check uses an unsafe cast `(snap as any).exists`; update the
snapshot typing so you can use `snap.exists` directly: import the correct
`FirebaseFirestoreTypes` from `@react-native-firebase/firestore`, type the
snapshot variable (e.g., as `FirebaseFirestoreTypes.DocumentSnapshot` or
`DocumentSnapshot<YourDocType>`) in the `useCreatePlan` logic, and replace
`(snap as any).exists` with `snap.exists`; ensure any helper functions that
receive the snapshot also accept the typed `DocumentSnapshot`.
- Around line 64-76: The auto-republish block (in hooks/useCreatePlan.ts)
currently fires publishPlan(user.uid, planId).catch(Bugsnag.notify) silently;
change it to set a local republishing state (e.g., isRepublishing) before
calling publishPlan, await the promise, then clear the state and show a
user-facing success toast on resolve or an error toast plus Bugsnag.notify(err)
on reject, so users see progress and outcome; also add/update the relevant
user-facing help text to document the auto-republish behaviour.
In `@hooks/useFriendSharedCompletedWorkoutsQuery.ts`:
- Line 20: The map over Firestore docs in
useFriendSharedCompletedWorkoutsQuery.ts currently does a blind cast (d.data()
as SharedCompletedWorkout); replace this with runtime validation: implement or
import a validator (e.g., validateSharedCompletedWorkout or a zod/schema
function) and for each doc call the validator on d.data(), filter out or log
invalid docs, and only return validated SharedCompletedWorkout objects (or
throw/handle errors as appropriate) so you never assert the type without
checking the document shape first.
In `@hooks/useFriendSharedMeasurementsQuery.ts`:
- Line 18: The current mapping in useFriendSharedMeasurementsQuery uses
snap.docs.map((d) => d.data() as SharedMeasurement) which unsafely asserts
Firestore data to the SharedMeasurement type; replace this with runtime
validation: parse each d.data() using a validator (e.g., a Zod schema for
SharedMeasurement or a small validateSharedMeasurement(obj) function) inside the
map/filter pipeline, skip or log invalid docs, and only return objects that pass
validation so consumers of useFriendSharedMeasurementsQuery receive
guaranteed-valid SharedMeasurement instances; update or add a short comment
describing the expected Firestore schema near the validation logic.
In `@hooks/useFriendSharedStandaloneWorkoutsQuery.ts`:
- Line 20: The current map call uses an unsafe cast (d.data() as
SharedStandaloneWorkout) which can hide malformed Firestore documents; add
runtime validation instead of direct casting by introducing a type-guard or
schema validator (e.g., isSharedStandaloneWorkout or a Zod/IO-TS schema) and use
it inside the useFriendSharedStandaloneWorkoutsQuery mapping: for each d.data()
validate required fields and types, skip or log/collect invalid documents, and
only return objects that pass validation as SharedStandaloneWorkout so consumers
no longer rely on unchecked casts.
In `@hooks/useImportCustomExerciseMutation.ts`:
- Around line 11-33: The hook duplicates the "lookup by name (app_exercise_id IS
NULL) then INSERT" flow already implemented in resolveExerciseId in
utils/importUtils.ts and has diverged by persisting description; extract a
single shared helper (e.g., resolveOrCreateCustomExercise) that implements the
SELECT ... WHERE app_exercise_id IS NULL lookup and the INSERT of the same
columns, accepting an optional description parameter and the fields currently
passed from useImportCustomExerciseMutation (name, bodyPart, targetMuscle,
equipment, secondaryMuscles, trackingType, isUnilateral, doubleWeight,
animatedUrl, description), replace the logic in useImportCustomExerciseMutation
to call that helper and update resolveExerciseId to call the same helper
(omitting description when not available) so all creation logic is centralized.
In `@hooks/usePrivacySettingsMutation.ts`:
- Line 13: The thrown error "Not authenticated" in usePrivacySettingsMutation is
too generic—replace it with a more descriptive message that includes the
context/action (e.g., "User not authenticated in usePrivacySettingsMutation:
cannot update privacy settings") so callers and logs can identify where and why
authentication failed; update the throw in the if (!user) check inside the
usePrivacySettingsMutation function accordingly.
- Around line 11-24: Add an onSuccess handler to usePrivacySettingsMutation
that, after the mutation completes, obtains a react-query QueryClient (via
useQueryClient) and calls queryClient.invalidateQueries for the privacy settings
query key so cached privacy settings are refreshed; modify
usePrivacySettingsMutation to import/use useQueryClient, add onSuccess alongside
onError, and call queryClient.invalidateQueries with the same key used by the
privacy-settings query (e.g., 'privacySettings' or ['privacySettings', user.uid]
or the helper key function if one exists).
In `@hooks/usePublishedWorkoutIdsQuery.ts`:
- Line 10: The query key for the hook usePublishedWorkoutIdsQuery is currently
static ("publishedWorkoutIds") which causes cross-account cache bleed; update
the queryKey in that hook to include the current user's uid (e.g.,
["publishedWorkoutIds", user.uid] or ["publishedWorkoutIds", user?.uid] if user
may be null) so cached results are scoped per user, and ensure any conditional
logic (refetching or enabled flags) uses the same uid presence check to avoid
running the query for anonymous/undefined users.
In `@hooks/useSocialListeners.ts`:
- Around line 40-57: The snapshot mapping currently performs an N+1 read by
calling db.collection("users").doc(...).get() per doc (see snapshot.docs map,
PendingRequest creation), so change the listener to first use denormalized
profile fields on the request/friend document (use
data.displayName/data.email/data.photoURL when present) and avoid per-doc
lookups; if denormalization cannot be applied immediately, replace the per-doc
get with a single batched lookup by collecting unique from UIDs and fetching
them once (use a single query with where(FieldPath.documentId(), 'in', uids) or
getAll) then map results into the PendingRequest objects to populate
displayName/email/photoURL, eliminating the per-document
db.collection("users").doc(...).get() calls.
In `@hooks/useWorkoutPublishQuery.ts`:
- Line 19: The current existence check uses an unsafe cast `(doc as
any).exists`; instead type the Firestore snapshot properly and use its typed
exists property. Import the DocumentSnapshot type from
`@react-native-firebase/firestore`, annotate the `doc` parameter/variable in
`useWorkoutPublishQuery` (or the function that receives the snapshot) as
DocumentSnapshot<T> (or appropriate generic), and replace `(doc as any).exists`
with `doc.exists`; update any related return/type annotations so TypeScript
infers a boolean without using `any`.
In `@utils/database.ts`:
- Around line 3094-3174: The SQL in fetchPRDataForExercises uses a per-row
subquery for settings (bodyWeight) inside pmExpr which can be slow on large
histories; extract the bodyWeight once before building pmExpr (e.g., query
settings for key 'bodyWeight' and pass the numeric value into the SQL as a bound
parameter) and replace the subquery CAST((SELECT value FROM settings WHERE key =
'bodyWeight') AS REAL) in pmExpr with that parameter; also ensure there is an
index on completed_exercises(exercise_id) (or create one) to speed the JOINs
referenced in the same function.
In `@utils/sharing.ts`:
- Around line 77-113: The try/catch in publishPlan swallows errors after
reporting to Bugsnag, preventing callers from detecting failures or retrying;
update publishPlan (and similar functions like unpublishPlan,
publishStandaloneWorkout, pushCustomExercise) to either re-throw the caught
error after Bugsnag.notify(error) or explicitly document the fire-and-forget
behavior with a comment, so callers know they cannot rely on success—choose one
approach and apply consistently across those named functions.
- Around line 314-358: pushStrengthPRs assumes fewer than 500 batched writes but
doesn’t enforce Firestore’s 500-operation limit; update pushStrengthPRs to
either document this assumption and early-return when prData is empty or,
better, split prData into chunks of <=500 and create/commit a new
firestore().batch() for each chunk (use fetchPRDataForExercises result length to
drive chunking), building each batch with the same per-PR logic and awaiting
each batch.commit() in sequence; also add a brief inline comment referencing the
500-op limit to make the assumption explicit.
- Around line 93-96: The code uses a type assertion "(existing as any).exists"
when reading the DocumentSnapshot from "const existing = await ref.get();":
instead, import and use the proper DocumentSnapshot type from
'`@react-native-firebase/firestore`' (e.g.,
FirebaseFirestoreTypes.DocumentSnapshot) and annotate the variable (const
existing: FirebaseFirestoreTypes.DocumentSnapshot = await ref.get();) so you can
use "existing.exists" directly when computing "publishedAt"; remove the any cast
and ensure the firestore types import is added if missing.
🪄 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
Run ID: 2ff2659b-2089-4294-b2d7-3ca0e8cce88d
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (73)
.gitignoreapp/(app)/(tabs)/(plans)/index.tsxapp/(app)/(tabs)/(plans)/overview.tsxapp/(app)/(tabs)/(plans)/standalone-workout.tsxapp/(app)/_layout.tsxapp/(app)/custom-exercise.tsxapp/(app)/friend-exercise.tsxapp/(app)/friend-plan.tsxapp/(app)/friend-profile.tsxapp/(app)/friend-workout.tsxapp/(app)/friends.tsxapp/(app)/settings.tsxapp/_layout.tsxcomponents/AppMenu.tsxcomponents/PlanList.tsxcomponents/TrainingPlanCard.tsxcomponents/TrainingPlanListItem.tsxcomponents/friends/FriendListItem.tsxcomponents/friends/FriendRequestItem.tsxconstants/WhatsNew.tscontext/AuthProvider.tsxfirebase.jsonfirestore.indexes.jsonfirestore.ruleshooks/__tests__/useAnimatedImageQuery.test.tshooks/__tests__/useBodyMeasurementMutations.test.tshooks/__tests__/useCreateStandaloneWorkout.test.tshooks/__tests__/useExerciseDetailQuery.test.tshooks/__tests__/useSaveCompletedWorkoutMutation.test.tshooks/__tests__/useTrackedExercisesQuery.test.tshooks/useAcceptFriendRequestMutation.tshooks/useBodyMeasurementMutations.tshooks/useCreatePlan.tshooks/useCreateStandaloneWorkout.tshooks/useDeclineFriendRequestMutation.tshooks/useFriendSharedCompletedWorkoutsQuery.tshooks/useFriendSharedCustomExercisesQuery.tshooks/useFriendSharedMeasurementsQuery.tshooks/useFriendSharedPlansQuery.tshooks/useFriendSharedStandaloneWorkoutsQuery.tshooks/useFriendSharedStrengthQuery.tshooks/useImportCustomExerciseMutation.tshooks/useImportPlanMutation.tshooks/useImportStandaloneWorkoutMutation.tshooks/usePlanPublishMutation.tshooks/usePlanPublishQuery.tshooks/usePrivacySettingsMutation.tshooks/usePublishedPlanIdsQuery.tshooks/usePublishedWorkoutIdsQuery.tshooks/useRemoveFriendMutation.tshooks/useSaveCompletedWorkoutMutation.tshooks/useSendFriendRequestMutation.tshooks/useSocialListeners.tshooks/useWorkoutPublishMutation.tshooks/useWorkoutPublishQuery.tslocales/de/messages.jslocales/de/messages.polocales/en/messages.jslocales/en/messages.polocales/es/messages.jslocales/es/messages.polocales/fr/messages.jslocales/fr/messages.popackage.jsonstore/socialStore.tstypes/firestore.tsutils/__tests__/importUtils.test.tsutils/database.tsutils/friends.tsutils/importUtils.tsutils/loadPremadePlans.tsutils/sharing.tsutils/userProfile.ts
…andalone workouts
…and mutual removal
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
utils/sharing.ts (1)
178-181:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInconsistent document existence check.
Lines 93 and 139 use
existing.exists()(method call), but here(existing as any).exists(property access with type cast) is used. This inconsistency may cause incorrect behaviour depending on the API version—and the cast hides any type errors.🔧 Suggested fix
const existing = await ref.get(); - const publishedAt = (existing as any).exists + const publishedAt = existing.exists() ? existing.data()?.publishedAt : now;🤖 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 `@utils/sharing.ts` around lines 178 - 181, The code uses a property access cast "(existing as any).exists" when computing publishedAt which is inconsistent with other uses of existing.exists() and hides type errors; change this to call the existence method (existing.exists()) and remove the any cast so publishedAt is set via existing.exists() ? existing.data()?.publishedAt : now, and ensure the variable holding the result of ref.get() is typed as a DocumentSnapshot (or appropriate Firestore snapshot type) to keep type safety for functions like existing.data() and existing.exists().
🧹 Nitpick comments (1)
utils/sharing.ts (1)
294-338: 💤 Low valueConsider chunking if
exerciseIdscould exceed 500 entries.Firestore batched writes are limited to 500 operations. If
prDataever exceeds this (e.g. bulk sync of a power-user's library),batch.commit()will reject. Currently unlikely in a fitness app, but worth a defensive check or chunk loop if the caller surface grows.🤖 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 `@utils/sharing.ts` around lines 294 - 338, The pushStrengthPRs function may exceed Firestore's 500-operation batched write limit when prData is large; modify pushStrengthPRs to chunk prData (from fetchPRDataForExercises) into windows of at most 500 documents, create a new db.batch() per chunk, add set operations for each pr in that chunk (using the same docId logic and fields), commit each batch sequentially (await batch.commit()) and handle/notify errors as currently done (Bugsnag.notify). Ensure you reference pushStrengthPRs, fetchPRDataForExercises, prData, and batch.commit when implementing the chunk-and-commit loop.
🤖 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 `@firestore.rules`:
- Around line 31-35: The create rule for match /users/{uid}/friends/{friendUid}
currently allows creation if any friendRequests doc exists; change it to require
that the corresponding friendRequests document (either uid_friendUid or
friendUid_uid) exists AND its status == "pending" before allowing create. Update
the friendRequests rules (match /friendRequests/{requestId}) to enforce allowed
transitions: allow create by the requester, allow updates only by the receiver
and only permit status to change from "pending" to either "accepted" or
"declined" (disallow changing back to "pending" or from "accepted"/"declined" to
any other value), and ensure server-visible immutable fields (requester/receiver
IDs and createdAt) cannot be arbitrarily modified; use these checks to prevent a
client from flipping status to bypass the pending-only check in match
/users/{uid}/friends/{friendUid}.
---
Outside diff comments:
In `@utils/sharing.ts`:
- Around line 178-181: The code uses a property access cast "(existing as
any).exists" when computing publishedAt which is inconsistent with other uses of
existing.exists() and hides type errors; change this to call the existence
method (existing.exists()) and remove the any cast so publishedAt is set via
existing.exists() ? existing.data()?.publishedAt : now, and ensure the variable
holding the result of ref.get() is typed as a DocumentSnapshot (or appropriate
Firestore snapshot type) to keep type safety for functions like existing.data()
and existing.exists().
---
Nitpick comments:
In `@utils/sharing.ts`:
- Around line 294-338: The pushStrengthPRs function may exceed Firestore's
500-operation batched write limit when prData is large; modify pushStrengthPRs
to chunk prData (from fetchPRDataForExercises) into windows of at most 500
documents, create a new db.batch() per chunk, add set operations for each pr in
that chunk (using the same docId logic and fields), commit each batch
sequentially (await batch.commit()) and handle/notify errors as currently done
(Bugsnag.notify). Ensure you reference pushStrengthPRs, fetchPRDataForExercises,
prData, and batch.commit when implementing the chunk-and-commit loop.
🪄 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
Run ID: 4ee2f134-e5ec-4525-8860-13b300b3e1e9
📒 Files selected for processing (15)
app/(app)/(tabs)/(plans)/standalone-workout.tsxapp/(app)/friend-exercise.tsxapp/(app)/friend-plan.tsxapp/(app)/friend-workout.tsxapp/(app)/friends.tsxapp/(app)/settings.tsxfirestore.ruleshooks/useCreatePlan.tshooks/usePlanPublishQuery.tshooks/usePrivacySettingsMutation.tshooks/usePublishedPlanIdsQuery.tshooks/usePublishedWorkoutIdsQuery.tshooks/useWorkoutPublishQuery.tsutils/sharing.tsutils/userProfile.ts
🚧 Files skipped from review as they are similar to previous changes (13)
- hooks/usePublishedWorkoutIdsQuery.ts
- hooks/usePrivacySettingsMutation.ts
- app/(app)/friend-workout.tsx
- app/(app)/(tabs)/(plans)/standalone-workout.tsx
- hooks/usePlanPublishQuery.ts
- app/(app)/friends.tsx
- hooks/useWorkoutPublishQuery.ts
- app/(app)/friend-exercise.tsx
- utils/userProfile.ts
- hooks/usePublishedPlanIdsQuery.ts
- app/(app)/settings.tsx
- hooks/useCreatePlan.ts
- app/(app)/friend-plan.tsx
…nd updating permissions
…fying publishedAt check
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 `@firestore.rules`:
- Around line 37-39: The create rule for friend documents currently allows
either side to accept via the OR clause; update the rule in the allow create
block to remove the "|| hasPendingRequest(friendUid, uid)" branch so that
creation is only allowed when request.auth.uid == friendUid AND
hasPendingRequest(uid, friendUid) is true (i.e., only the recipient can accept a
request); locate the rule containing allow create and the hasPendingRequest(...)
checks and delete the second hasPendingRequest check to enforce receiver-only
acceptance.
🪄 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
Run ID: 858249be-b7dd-4774-9bc6-7d22cfc6686a
📒 Files selected for processing (2)
firestore.rulesutils/sharing.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- utils/sharing.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
firestore.rules (2)
71-78:⚠️ Potential issue | 🟠 Major | ⚡ Quick winResend contract is broken: sender can’t overwrite an existing declined/accepted friendRequests document.
firestore.rulesonly allows the sender tocreatefriendRequests/{requestId}(auth.uid == resource.data.from), while the onlyupdateis for the receiver (auth.uid == resource.data.to) and only allows apending->accepted/declinedstatus change. There’s nodeleteforfriendRequests, so retrying with the same{fromUid}_${toUid}id afterdeclined/acceptedis rejected—contradicting the “create/overwrite” comment and the app’ssendFriendRequestwhich uses.set()to re-request.
File:firestore.rules(lines 70-88),utils/friends.ts(lines 10-22)🤖 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 `@firestore.rules` around lines 71 - 78, The rules block for match /friendRequests/{requestId} only permits create by request.auth.uid == request.resource.data.from and permits updates only by the receiver (auth.uid == resource.data.to) from "pending" to "accepted"/"declined", so a sender calling sendFriendRequest (which uses .set() to overwrite an existing {fromUid}_{toUid} doc) is rejected after a prior "accepted"/"declined"; update the security rules to allow the sender to overwrite their own request when they are the author (auth.uid == request.resource.data.from OR auth.uid == resource.data.from) and the existing resource.status is not "pending" (or allow delete by the sender), or alternatively add a rule that permits create/overwrite when request.auth.uid == request.resource.data.from and resource.data.from == request.auth.uid to match .set() behavior used in utils/friends.ts (sendFriendRequest), ensuring you restrict overwrites only when existing.status != "pending".
10-12:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winHarden friend-request authorization: bind
friendRequests/{requestId}to its payload
hasPendingRequest(fromUid, toUid)only checks thatfriendRequests/{fromUid}_{toUid}exists and hasdata.status == 'pending'. Sincematch /friendRequests/{requestId}allow createonly requiresrequest.auth.uid == request.resource.data.fromand does not validaterequestIdagainst{fromUid}_{toUid}(nordata.to/data.status), an attacker can writefriendRequests/{victimUid}_{attackerUid}withfrom == attackerUidandstatus == 'pending', makinghasPendingRequest(victimUid, attackerUid)true and then allowingusers/{victimUid}/friends/{attackerUid}creation even without a real request from the victim. (Affects 37-39, 77-78)🔒 Minimal hardening
function hasPendingRequest(fromUid, toUid) { return exists(/databases/$(database)/documents/friendRequests/$(fromUid + '_' + toUid)) && + get(/databases/$(database)/documents/friendRequests/$(fromUid + '_' + toUid)).data.from == fromUid && + get(/databases/$(database)/documents/friendRequests/$(fromUid + '_' + toUid)).data.to == toUid && get(/databases/$(database)/documents/friendRequests/$(fromUid + '_' + toUid)).data.status == 'pending'; } ... match /friendRequests/{requestId} { allow create: if request.auth != null && - request.auth.uid == request.resource.data.from; + request.auth.uid == request.resource.data.from && + requestId == request.resource.data.from + '_' + request.resource.data.to && + request.resource.data.status == 'pending';🤖 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 `@firestore.rules` around lines 10 - 12, hasPendingRequest and the friendRequests create rule are vulnerable because creators can choose arbitrary requestId; require the document id to match its payload and validate payload fields. In match /friendRequests/{requestId} update the allow create rule to require requestId == (request.resource.data.from + '_' + request.resource.data.to), request.resource.data.from == request.auth.uid, request.resource.data.to is a valid UID, and request.resource.data.status == 'pending'. Also harden hasPendingRequest(fromUid, toUid) to not only check existence and status but to verify get(...).data.from == fromUid and get(...).data.to == toUid (i.e., confirm the stored payload matches the expected from/to before treating it as a pending request).
🤖 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.
Outside diff comments:
In `@firestore.rules`:
- Around line 71-78: The rules block for match /friendRequests/{requestId} only
permits create by request.auth.uid == request.resource.data.from and permits
updates only by the receiver (auth.uid == resource.data.to) from "pending" to
"accepted"/"declined", so a sender calling sendFriendRequest (which uses .set()
to overwrite an existing {fromUid}_{toUid} doc) is rejected after a prior
"accepted"/"declined"; update the security rules to allow the sender to
overwrite their own request when they are the author (auth.uid ==
request.resource.data.from OR auth.uid == resource.data.from) and the existing
resource.status is not "pending" (or allow delete by the sender), or
alternatively add a rule that permits create/overwrite when request.auth.uid ==
request.resource.data.from and resource.data.from == request.auth.uid to match
.set() behavior used in utils/friends.ts (sendFriendRequest), ensuring you
restrict overwrites only when existing.status != "pending".
- Around line 10-12: hasPendingRequest and the friendRequests create rule are
vulnerable because creators can choose arbitrary requestId; require the document
id to match its payload and validate payload fields. In match
/friendRequests/{requestId} update the allow create rule to require requestId ==
(request.resource.data.from + '_' + request.resource.data.to),
request.resource.data.from == request.auth.uid, request.resource.data.to is a
valid UID, and request.resource.data.status == 'pending'. Also harden
hasPendingRequest(fromUid, toUid) to not only check existence and status but to
verify get(...).data.from == fromUid and get(...).data.to == toUid (i.e.,
confirm the stored payload matches the expected from/to before treating it as a
pending request).
- Changed the eas-build-on-success script to use npx for better compatibility. - Updated expo version from ^54.0.0 to ~54.0.35 for more consistent builds. - Bumped versions of several expo-related packages: - expo-file-system: ~19.0.22 to ~19.0.23 - expo-font: ~14.0.11 to ~14.0.12 - expo-localization: ~17.0.8 to ~17.0.9 - expo-router: ~6.0.23 to ~6.0.24 - expo-updates: ~29.0.17 to ~29.0.18 - Updated @bugsnag/plugin-expo-eas-sourcemaps from ^54.0.0 to ^54.1.0.
…ing status handling
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
firestore.rules (1)
11-16: ⚡ Quick winConsolidate repeated lookups in
hasPendingRequest
hasPendingRequest(fromUid, toUid)currently performs 1exists()plus 3 separateget()calls against the samefriendRequests/{from_to}document; sincerules_version = '2'you can useletto fetch once and null-check, reducing document read calls and simplifying the logic.♻️ Proposed refactor
function hasPendingRequest(fromUid, toUid) { - return exists(/databases/$(database)/documents/friendRequests/$(fromUid + '_' + toUid)) && - get(/databases/$(database)/documents/friendRequests/$(fromUid + '_' + toUid)).data.status == 'pending' && - get(/databases/$(database)/documents/friendRequests/$(fromUid + '_' + toUid)).data.from == fromUid && - get(/databases/$(database)/documents/friendRequests/$(fromUid + '_' + toUid)).data.to == toUid; + let req = get(/databases/$(database)/documents/friendRequests/$(fromUid + '_' + toUid)); + return req != null && + req.data.status == 'pending' && + req.data.from == fromUid && + req.data.to == toUid; }🤖 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 `@firestore.rules` around lines 11 - 16, The hasPendingRequest(fromUid, toUid) function currently calls exists() and three separate get()s for the same friendRequests/$(fromUid + '_' + toUid) document; replace these with a single let doc = get(...) (using the same path expression) and then null-check doc before accessing doc.data.status, doc.data.from, and doc.data.to to determine pending state, eliminating redundant reads and simplifying the logic.
🤖 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 `@app.config.js`:
- Line 7: Update the misleading version format comment around the version and
versionCode constants: change the comment from "MM.mm.pp" and "MMmmpp" to
accurately reflect the actual scheme used by the file (e.g., "M.mm.pp" for
version and "Mmmpp" for versionCode) or add a brief explanatory note that
documents how version (version) maps to numeric versionCode (versionCode) so
future maintainers won't be confused; adjust the comment near the version and
versionCode declarations to match that explanation.
---
Nitpick comments:
In `@firestore.rules`:
- Around line 11-16: The hasPendingRequest(fromUid, toUid) function currently
calls exists() and three separate get()s for the same friendRequests/$(fromUid +
'_' + toUid) document; replace these with a single let doc = get(...) (using the
same path expression) and then null-check doc before accessing doc.data.status,
doc.data.from, and doc.data.to to determine pending state, eliminating redundant
reads and simplifying the logic.
🪄 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
Run ID: 813d6996-f80b-4f6b-9273-da8943ce6192
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
app.config.jsconstants/HelpData.tsconstants/WhatsNew.tsfirestore.ruleslocales/de/messages.jslocales/de/messages.polocales/en/messages.jslocales/en/messages.polocales/es/messages.jslocales/es/messages.polocales/fr/messages.jslocales/fr/messages.popackage.json
✅ Files skipped from review due to trivial changes (2)
- constants/HelpData.ts
- constants/WhatsNew.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- locales/en/messages.po
- locales/de/messages.po
This was referenced Jun 2, 2026
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.
Summary by CodeRabbit
New Features
Chores
Localisation