Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds a privacy settings system (AUTO_SHARE, LOCATION_SHARE) with DB-backed persistence and a usePrivacySettings hook; introduces friend blocking (new 'blocked' status, block/unblock APIs, UI, and a Blocked Users screen); integrates privacy defaults into capture details and refactors device-location to React Query. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FriendItem as FriendItem UI
participant FriendsHook as useFriends
participant Mutation as updateFriendshipMutation
participant Supabase
participant QueryCache as Query Cache
User->>FriendItem: Tap "Block"
FriendItem->>User: Show confirmation
User->>FriendItem: Confirm
FriendItem->>FriendsHook: blockFriend(friendshipId)
FriendsHook->>QueryCache: Optimistic update (mark blocked)
FriendsHook->>Mutation: mutate(status=BLOCKED)
Mutation->>Supabase: Upsert/update friendship status
alt Success
Supabase-->>Mutation: OK
Mutation->>QueryCache: Invalidate/refresh friends queries
FriendsHook-->>User: show success toast
else Error
Supabase-->>Mutation: Error
Mutation->>QueryCache: Revert optimistic update
FriendsHook-->>User: show error toast
end
sequenceDiagram
participant User
participant PrivacyUI as Privacy Settings UI
participant Hook as usePrivacySettings
participant Mutation as savePrivacySettings
participant Supabase
participant QueryCache as Query Cache
User->>PrivacyUI: Toggle AUTO_SHARE / LOCATION_SHARE
PrivacyUI->>Hook: toggleSetting(key)
Hook->>QueryCache: Optimistically update cached settings
Hook->>Mutation: upsert privacy_settings row
Mutation->>Supabase: Upsert
alt Success
Supabase-->>Mutation: OK
Mutation->>QueryCache: Confirm cache
Hook-->>PrivacyUI: isSaving = false
else Error
Supabase-->>Mutation: Error
Mutation->>QueryCache: Revert cache
Hook-->>PrivacyUI: error state
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
frontend/app/capture/details.tsx (1)
134-143: Duplicate toast message in error handling.The toast
'Cannot save entry'is displayed twice whenhasSelectedSharing()is false - once at line 137 and again at line 141.if (!capture || !user || !hasSelectedSharing()) { if (!hasSelectedSharing()) { toast('Please select who to share this entry with', 'error'); } else { toast('Cannot save entry', 'error'); } - toast('Cannot save entry', 'error'); return; }frontend/hooks/use-friends.ts (3)
42-104: Fix possible runtime crash:data.map(...)runs before checkingerror/ nulldata.
If Supabase returnserrorwithdata = null, this will throw before you surface the real error.const { data, error } = await supabase .from(TABLES.FRIENDSHIPS) .select(` *, user_profile:profiles!friendships_user_id_fkey( id, full_name, avatar_url, username ), friend_profile:profiles!friendships_friend_id_fkey( id, full_name, avatar_url, username ) `) .or(`user_id.eq.${userId},friend_id.eq.${userId}`) .order('created_at', { ascending: false }) as { data: any[], error: any, }; + if (error) { + throw new Error(error.message); + } + + if (!data) return []; + const friends: FriendWithProfile[] = data.map(friend => { const { user_profile: user, friend_profile: friend_, ...friend_record } = friend; const profile = friend.friend_id === userId ? user : friend_; return { ...friend_record, friend_profile: profile } }) - - if (error) { - throw new Error(error.message); - }
108-113:blockedFriendslikely needs directionality (blocked-by) to match “Blocked Users” UX.
friendships.filter(status === BLOCKED)will include rows blocked by either party unless your schema encodes who initiated the block. If the screen is “users you have blocked”, consider adding ablocked_bycolumn (or separate table) and filtering on it; otherwise rename this collection to something likeblockedRelationshipsto avoid misleading UI.
141-165: Mutation may report success when 0 rows were updated (no.select()).
Withupdate(...).eq('id', id)and no.select(), you don’t verify that a row was actually updated (wrong id, RLS denial, already-in-state, etc.). Prefer selecting the updated row (or checkingcount) and failing if nothing changed.const updateFriendshipMutation = useMutation({ mutationFn: async ({ id, status }: { id: string; status: typeof FRIENDSHIP_STATUS.ACCEPTED | typeof FRIENDSHIP_STATUS.DECLINED | typeof FRIENDSHIP_STATUS.BLOCKED }) => { if (__DEV__) console.log('Updating friendship:', { id, status }); - const { error } = await supabase + const { data, error } = await supabase .from(TABLES.FRIENDSHIPS) .update({ status } as never) - .eq('id', id); + .eq('id', id) + .select('id,status') + .single(); if (error) { if (__DEV__) console.error('Error updating friendship:', error); throw new Error(error.message); } if (__DEV__) console.log('Updated friendship status successfully'); - return { id, status }; + if (!data) throw new Error('Friendship not found or not authorized'); + return data; },
🧹 Nitpick comments (12)
frontend/hooks/use-device-location.ts (3)
57-57: Unusedrefetchdestructuring.
refetchis destructured but never used or returned. Either remove it to avoid dead code, or expose it in the return value (and updateUseDeviceLocationResultinterface) to allow consumers to manually trigger location refresh.const { data, isLoading, error, - refetch, isFetching, } = useQuery<LocationData | null, Error>({Or if manual refetch is desired:
interface UseDeviceLocationResult { location: LocationData | null; isLoading: boolean; error: string | null; clearLocation: () => void; + refetch: () => void; }Then return
refetchfrom the hook.
59-68: Consider addingstaleTimeandretryconfiguration.Location queries are expensive (battery, network, GPS). Without
staleTime, React Query will refetch on every mount and window focus. Additionally, default retry behavior (3 retries) is wasteful when permission is denied or services are unavailable.} = useQuery<LocationData | null, Error>({ queryKey: ['device-location'], queryFn: async () => { // ... }, + staleTime: 5 * 60 * 1000, // 5 minutes - adjust based on use case + retry: (failureCount, error) => { + // Don't retry permission or service availability errors + if (error.message.includes('unavailable') || error.message.includes('permission')) { + return false; + } + return failureCount < 2; + }, });
65-68: Permission denial returnsnullsilently.When permission is not granted, returning
nullmakes it indistinguishable from "location not yet fetched" or "no address found". Consider throwing a specific error or adding apermissionDeniedflag to the return type so consumers can inform users appropriately.if (status !== 'granted') { - return null; + throw new Error('Location permission not granted'); }Alternatively, return a distinct state flag if you want to handle this case without triggering an error state.
frontend/types/database.ts (1)
291-316:privacy_settings.idbigint ↔ TSnumbercan be lossy depending on int8 parsing
Your migration definesid bigserial(int8). If your Supabase client returns int8 as string (or if IDs grow beyond JS safe integer),numberwill be incorrect at runtime.Suggested direction (pick one and align service/hook usage accordingly):
- id: number + id: string ... - id?: number + id?: stringfrontend/app/friends.tsx (1)
20-31: Blocking flow: consider refreshing lists after success
IfblockFriend()doesn’t optimistically update cachedfriends/pendingRequests, the blocked user may remain visible until a manual refresh/reopen. Considerawait refetch()(or local removal) on success.Example:
const handleBlockFriend = async (friendshipId: string) => { const result = await blockFriend(friendshipId); if (result.success) { showToast('Friend blocked successfully', 'success'); + await refetch(); } else { showToast(result.error || 'Failed to block friend', 'error'); } };Also applies to: 50-57
frontend/app/settings/blocked-users.tsx (2)
15-19: Prefer static import over inlinerequire()for expo-router.Using
require()inside a component function is non-standard and prevents tree-shaking. The router should be imported statically at the top of the file like other screens in this codebase.Add this import at the top of the file:
import React from 'react'; import { View, Text, StyleSheet, ScrollView, Image, TouchableOpacity, Alert } from 'react-native'; import { SafeAreaView } from 'react-native-safe-area-context'; +import { router } from 'expo-router'; import { ArrowLeft, UserX } from 'lucide-react-native';Then simplify the handler:
const handleBack = () => { - // Using router from expo-router via global import to keep consistent with other screens - const { router } = require('expo-router'); router.back(); };
117-119: Remove unused style definition.The
backstyle is defined but never used in the component.- back: { - width: 40, - },frontend/hooks/use-privacy-settings.ts (1)
27-30:useMemofor queryKey is unnecessary here.React Query internally handles queryKey comparison via deep equality. A simple inline array
['privacy-settings', user?.id]would work identically and reduce cognitive overhead.- const queryKey = useMemo( - () => ['privacy-settings', user?.id], - [user?.id], - ); + const queryKey = ['privacy-settings', user?.id];frontend/components/friend-item.tsx (1)
99-109: Consider adding explicit color for 'blocked' status.The
getStatusColorfunction doesn't handle the new 'blocked' status explicitly; it falls through to the default gray. If blocked users should have a distinct visual indicator (e.g., red), add a case for it.const getStatusColor = () => { switch (friend.status) { case 'connected': return '#10B981'; case 'pending': return '#F59E0B'; case 'invited': return '#6B7280'; + case 'blocked': + return '#EF4444'; // Red for blocked default: return '#6B7280'; } };frontend/app/capture/details.tsx (1)
157-158: Location tag may have trailing comma when region/country are missing.If
location.cityexists but bothregionandcountryare undefined, the result would be"CityName, "with a trailing comma and space.- const locationTag = showLocation && location ? `${location?.city}, ${location?.region ?? location?.country ?? ""}` : null; + const locationTag = showLocation && location?.city + ? [location.city, location.region ?? location.country].filter(Boolean).join(', ') + : null;frontend/app/settings/privacy.tsx (2)
38-45: Consider handlingisLoading/isSavingto avoid confusing toggles.
Right now switches can be toggled even while the hook is loading/saving; recommend disabling switches (or showing a saving indicator) during those states.
89-128: “Blocked Users” navigation tile is good; consider basic accessibility metadata.
For thePressable, addingaccessibilityRole="button"(and optionally anaccessibilityLabel/testID) will help testing and accessibility.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
frontend/app/capture/details.tsx(4 hunks)frontend/app/friends.tsx(4 hunks)frontend/app/settings/blocked-users.tsx(1 hunks)frontend/app/settings/notifications.tsx(2 hunks)frontend/app/settings/privacy.tsx(4 hunks)frontend/app/settings/profile.tsx(0 hunks)frontend/components/friend-item.tsx(3 hunks)frontend/components/friends-section.tsx(4 hunks)frontend/constants/supabase.ts(3 hunks)frontend/hooks/use-device-location.ts(3 hunks)frontend/hooks/use-friends.ts(5 hunks)frontend/hooks/use-privacy-settings.ts(1 hunks)frontend/services/privacy-settings-service.ts(1 hunks)frontend/supabase/migrations/20250825200450_misty_boat.sql(1 hunks)frontend/supabase/migrations/20251212020000_privacy_settings.sql(1 hunks)frontend/types/database.ts(2 hunks)frontend/types/privacy.ts(1 hunks)
💤 Files with no reviewable changes (1)
- frontend/app/settings/profile.tsx
🧰 Additional context used
🧬 Code graph analysis (5)
frontend/app/settings/blocked-users.tsx (2)
frontend/providers/auth-provider.tsx (1)
useAuthContext(69-75)frontend/hooks/use-toast.ts (1)
useToast(3-11)
frontend/hooks/use-friends.ts (2)
frontend/constants/supabase.ts (2)
FRIENDSHIP_STATUS(31-36)TABLES(4-15)frontend/lib/supabase.ts (1)
supabase(14-30)
frontend/services/privacy-settings-service.ts (3)
frontend/services/device-storage.ts (1)
deviceStorage(170-170)frontend/lib/supabase.ts (1)
supabase(14-30)frontend/lib/logger.ts (1)
error(32-35)
frontend/app/settings/privacy.tsx (1)
frontend/hooks/use-privacy-settings.ts (1)
usePrivacySettings(23-91)
frontend/app/capture/details.tsx (3)
frontend/hooks/use-privacy-settings.ts (1)
usePrivacySettings(23-91)frontend/hooks/use-device-location.ts (1)
useDeviceLocation(20-126)frontend/lib/utils.ts (1)
getDefaultAvatarUrl(6-8)
🔇 Additional comments (18)
frontend/hooks/use-device-location.ts (1)
116-125: LGTM!The
clearLocationimplementation correctly resets the query cache, and the return values properly derive state from React Query. UsingisLoading || isFetchingappropriately covers both initial load and refetch scenarios.frontend/types/database.ts (1)
108-117: Friendship status typing updated consistently (incl.blocked)
Row/Insert/Update all include'blocked', which avoids “impossible state” typing drift across CRUD paths.Also applies to: 124-125
frontend/types/privacy.ts (1)
1-4: Enum maps cleanly to column names (auto_share,location_share)frontend/app/friends.tsx (1)
170-196: Prop wiring foronBlockFriendis consistent across both viewsfrontend/app/settings/notifications.tsx (1)
4-9: Sizing changes look consistent (scaled header padding) + import cleanupAlso applies to: 126-132
frontend/components/friends-section.tsx (1)
8-17:onBlockFriendplumbed through cleanly to itemsAlso applies to: 19-28, 77-87, 102-112
frontend/constants/supabase.ts (1)
4-15: Supabase constants updated consistently for privacy settings + blocked statusAlso applies to: 31-36, 106-123
frontend/app/settings/blocked-users.tsx (1)
56-96: Well-structured unblock flow with proper UX.The implementation correctly handles the unblock action with a confirmation dialog, async operation, and appropriate toast feedback for both success and error states.
frontend/hooks/use-privacy-settings.ts (1)
47-69: Solid optimistic update implementation.The mutation correctly implements the optimistic update pattern: cancels in-flight queries, caches previous state for rollback, updates optimistically, reverts on error, and invalidates on settle for eventual consistency.
frontend/services/privacy-settings-service.ts (2)
17-59: Local-first caching may serve stale data in multi-device scenarios.The current strategy returns local cache immediately without checking freshness. If a user updates settings on another device, this device will serve stale data until the local cache is cleared.
For privacy settings that change infrequently, this is likely acceptable. If cross-device consistency becomes important, consider adding a TTL or always fetching remote and using local as fallback only when offline.
84-91: Theas nevertype casts are a consistent pattern across the codebase and are not masking a type mismatch. Therowobject structure (user_id, auto_share, location_share) correctly matches theprivacy_settingstable's Insert type. Even services that properly import Database types (e.g.,use-auth.ts) use the sameas neverworkaround on upsert operations, suggesting this is either a known Supabase TypeScript client limitation or an intentional pattern. Removing these casts without understanding the underlying cause could break the code. If improved type safety is desired, consider importing the Database type and defining a type alias like other services do, but be aware this may not eliminate the need for theas nevercast.frontend/components/friend-item.tsx (1)
31-59: Good UX pattern: two-step confirmation for destructive actions.The separate
confirmRemoveandconfirmBlockfunctions with explicit confirmation dialogs prevent accidental destructive actions. The block confirmation appropriately warns users about the consequences.frontend/app/capture/details.tsx (1)
54-58: Clean integration of privacy settings.The privacy settings are properly consumed and used to derive sensible defaults for the sharing UI. The
AUTO_SHAREandLOCATION_SHAREpreferences are applied appropriately.frontend/hooks/use-friends.ts (2)
17-31: Public API addition looks good; ensure “blocked” semantics are well-defined.
AddingblockedFriends,blockFriend,unblockFriendis a nice, simple surface—just make sure product rules cover “who blocked whom” and what “unblock” restores to (see notes below).
282-296: Return shape update looks consistent.
Hook returnsblockedFriends,blockFriend,unblockFriendas expected.frontend/app/settings/privacy.tsx (3)
1-8: Hook-based settings integration is a solid direction.
Moving persistence/state out of the screen and intousePrivacySettingsis a maintainability win.
19-36: Confirm privacy defaults—LOCATION_SHAREdefaulting totruemay be surprising.
If location sharing is meant to be opt-in, consider defaulting tofalse(and/or gating behind explicit consent).
165-183: Scaling update looks fine.
Usingscale()/verticalScale()for padding is consistent with responsive sizing.
| const blockFriend = useCallback(async (friendshipId: string) => { | ||
| try { | ||
| await updateFriendshipMutation.mutateAsync({ id: friendshipId, status: FRIENDSHIP_STATUS.BLOCKED }); | ||
| return { success: true }; | ||
| } catch (error) { | ||
| return { | ||
| success: false, | ||
| error: error instanceof Error ? error.message : 'Failed to block friend', | ||
| }; | ||
| } | ||
| }, [updateFriendshipMutation]); | ||
|
|
||
| const unblockFriend = useCallback(async (friendshipId: string) => { | ||
| try { | ||
| await updateFriendshipMutation.mutateAsync({ id: friendshipId, status: FRIENDSHIP_STATUS.ACCEPTED }); | ||
| return { success: true }; | ||
| } catch (error) { | ||
| return { | ||
| success: false, | ||
| error: error instanceof Error ? error.message : 'Failed to unblock friend', | ||
| }; | ||
| } | ||
| }, [updateFriendshipMutation]); | ||
|
|
There was a problem hiding this comment.
unblockFriend always restoring to ACCEPTED is risky.
If a user blocks a pending request, “unblock” probably shouldn’t auto-accept it; also if the other user initiated the block, you likely shouldn’t be able to “unblock” it. This ties back to needing “blocked_by” (or similar) plus a stored “previous_status”, or making unblock explicitly choose the target state.
🤖 Prompt for AI Agents
In frontend/hooks/use-friends.ts around lines 223 to 246, unblockFriend
unconditionally sets status to FRIENDSHIP_STATUS.ACCEPTED which can incorrectly
accept pending requests or allow unblocking when the other user performed the
block; change the logic so unblock is explicit about the target status and
enforces blocker ownership: either (A) extend the friendship model to store
blocked_by and previous_status and have unblock restore previous_status only if
current user is the blocker, or (B) change unblockFriend to accept a
targetStatus param (or derive target from stored previous_status) and validate
that the current user is allowed to change from BLOCKED to that target before
calling updateFriendshipMutation; implement checks and update types/usage
accordingly.
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/components/friend-item.tsx (2)
125-133: Online indicator condition likely wrong (ACCEPTEDvs'connected')
IfFRIENDSHIP_STATUS.ACCEPTEDdoesn’t equal'connected', online dots will never render. Prefer a single source of truth. If this component’s status is'connected', simplify the condition:- {friend.status === (FRIENDSHIP_STATUS.ACCEPTED as string) && ( + {friend.status === 'connected' && ( <View style={[ styles.statusIndicator, { backgroundColor: friend.isOnline ? '#10B981' : '#6B7280' } ]} /> )}
8-18: UpdateFriend.statusunion to match database schema: use 'accepted'/'declined' instead of 'connected'/'invited'The
Friend.statustype definition is out of sync with the actual data flow. At line 105 infrontend/app/friends.tsx,friendship.status(which contains values like'accepted','pending','declined','blocked'from the database) is directly assigned to the Friend object. However, the Friend interface defines status as'connected' | 'pending' | 'invited' | 'blocked', creating a type violation.This causes the online indicator check at line 125 (
friend.status === FRIENDSHIP_STATUS.ACCEPTED) to never match, sincefriend.statuswill actually be the string'accepted'but the type union declares it should be'connected'.Change line 14 to:
status: 'accepted' | 'pending' | 'declined' | 'blocked';And update the
getStatusColor()switch statement (lines 99-112) to use the correct status values:
- Replace
case 'connected':withcase 'accepted':- Replace
case 'invited':withcase 'declined':
♻️ Duplicate comments (1)
frontend/app/capture/details.tsx (1)
77-81: This effect addresses the earlier async-friends defaulting issue.
🧹 Nitpick comments (1)
frontend/hooks/use-privacy-settings.ts (1)
27-80: Compute toggles from the query cache to avoid stalecurrentSettingson rapid toggles.Right now
toggleSetting()derivesnextfrom the render-timecurrentSettings. If the user toggles quickly while mutations are in-flight, you can drop intermediate state.const toggleSetting = (id: PrivacySettings) => { if (!user?.id) return; - const next: PrivacySettingsMap = { - ...currentSettings, - [id]: !currentSettings[id], - }; + const base = + queryClient.getQueryData<PrivacySettingsMap>(queryKey) ?? + DEFAULT_PRIVACY_SETTINGS; + const next: PrivacySettingsMap = { ...base, [id]: !base[id] }; saveSettings(next); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
frontend/app/capture/details.tsx(5 hunks)frontend/app/friends.tsx(4 hunks)frontend/app/settings/blocked-users.tsx(1 hunks)frontend/app/settings/privacy.tsx(4 hunks)frontend/components/friend-item.tsx(4 hunks)frontend/hooks/use-device-location.ts(3 hunks)frontend/hooks/use-privacy-settings.ts(1 hunks)frontend/supabase/migrations/20251212020000_privacy_settings.sql(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/app/settings/blocked-users.tsx
- frontend/supabase/migrations/20251212020000_privacy_settings.sql
- frontend/app/friends.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/hooks/use-privacy-settings.ts (2)
frontend/services/privacy-settings-service.ts (2)
PrivacySettingsMap(5-5)PrivacySettingsService(10-93)frontend/providers/auth-provider.tsx (1)
useAuthContext(69-75)
🔇 Additional comments (4)
frontend/components/friend-item.tsx (2)
99-112: Blocked status color mapping looks good
Nice, localized UI affordance for the new state.
20-60: No action needed —friend.idcorrectly contains the friendship record ID (not user ID). TheconvertToFriendFormatfunction infriends.tsx(line 100) explicitly mapsfriendship.idtofriend.id, ensuring all callbacks (onRemove,onAccept,onDecline,onBlock) receive the correct friendshipId. The data flow from theuseFriendshook throughFriendsSectiontoFriendItemis consistent and correct.frontend/app/settings/privacy.tsx (2)
19-45: Centralized privacy settings wiring looks clean (defaults + server-backed map).
88-134: Blocked Users navigation tile: good accessibility/test hooks.
| const { settings: privacySettings } = usePrivacySettings(); | ||
| const { location } = useDeviceLocation(); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the full context around lines 54-56 in details.tsx
cat -n frontend/app/capture/details.tsx | sed -n '40,70p'Repository: fortune710/keepsafe
Length of output: 1382
🏁 Script executed:
# Find the use-device-location.ts file to understand the enabled option
find . -name "use-device-location.ts" -o -name "use-device-location.js" -o -name "*device-location*"Repository: fortune710/keepsafe
Length of output: 102
🏁 Script executed:
# Search for useDeviceLocation hook usage and definition
rg "useDeviceLocation" --type typescript --type tsx -A 2 -B 2Repository: fortune710/keepsafe
Length of output: 97
🏁 Script executed:
# Search for LOCATION_SHARE and showLocation usage
rg "LOCATION_SHARE|showLocation" --type typescript --type tsx -B 3 -A 3Repository: fortune710/keepsafe
Length of output: 97
🏁 Script executed:
# Read the use-device-location hook to understand its signature and enabled option
cat -n ./frontend/hooks/use-device-location.tsRepository: fortune710/keepsafe
Length of output: 4997
🏁 Script executed:
# Search for LOCATION_SHARE and showLocation without type restrictions
rg "LOCATION_SHARE|showLocation" -B 3 -A 3Repository: fortune710/keepsafe
Length of output: 2802
🏁 Script executed:
# Search for PrivacySettings enum definition
rg "enum PrivacySettings|LOCATION_SHARE" -B 2 -A 10Repository: fortune710/keepsafe
Length of output: 4950
🏁 Script executed:
# Get more context from details.tsx to see the full picture
cat -n frontend/app/capture/details.tsx | head -100Repository: fortune710/keepsafe
Length of output: 4665
🏁 Script executed:
# Check React Query documentation in code and verify enabled option
rg "enabled.*:" frontend/hooks --type tsx --type ts -B 2 -A 2Repository: fortune710/keepsafe
Length of output: 90
🏁 Script executed:
# Check how useQuery is used elsewhere in the codebase with conditional logic
rg "useQuery" frontend/hooks --type tsx --type ts -A 5 | head -60Repository: fortune710/keepsafe
Length of output: 90
🏁 Script executed:
# Verify the exact behavior - check if there are any comments in use-device-location.ts about enabled
cat -n ./frontend/hooks/use-device-location.ts | sed -n '53,65p'Repository: fortune710/keepsafe
Length of output: 522
🏁 Script executed:
# Search for any comments or documentation about this issue in the codebase
rg "permission.*prompt|Location.*permission" --type tsx --type ts -B 2 -A 2Repository: fortune710/keepsafe
Length of output: 90
🏁 Script executed:
# Check what showLocation depends on - is it purely privacySettings?
cat -n frontend/app/capture/details.tsx | sed -n '50,80p'Repository: fortune710/keepsafe
Length of output: 1600
🏁 Script executed:
# Search for any other uses of useDeviceLocation to see if enabled pattern is used elsewhere
rg "useDeviceLocation" -A 10Repository: fortune710/keepsafe
Length of output: 4642
🏁 Script executed:
# Check if there's any documentation or TODOs about this in the codebase
rg "enabled.*option|gate.*location|permission.*flow" -iRepository: fortune710/keepsafe
Length of output: 45
🏁 Script executed:
# Check the exact import and usage in location-tab.tsx
cat -n frontend/components/capture/editor/location-tab.tsx | head -50Repository: fortune710/keepsafe
Length of output: 2519
🏁 Script executed:
# Search for all exports from use-device-location.ts
rg "export" frontend/hooks/use-device-location.tsRepository: fortune710/keepsafe
Length of output: 125
🏁 Script executed:
# Check if there are multiple versions or overloads of useDeviceLocation
fd use-device-locationRepository: fortune710/keepsafe
Length of output: 100
🏁 Script executed:
# Read the entire use-device-location.ts file
cat -n frontend/hooks/use-device-location.tsRepository: fortune710/keepsafe
Length of output: 4997
🏁 Script executed:
# Check if showLocation could be computed before line 55
cat -n frontend/app/capture/details.tsx | sed -n '50,65p'Repository: fortune710/keepsafe
Length of output: 900
🏁 Script executed:
# Check what dependencies showLocation has - is it PURELY based on privacySettings?
rg "showLocation" frontend/app/capture/details.tsx -B 5 -A 5Repository: fortune710/keepsafe
Length of output: 490
🏁 Script executed:
# Check if there's any indication this is recent code or pending review
git log --oneline frontend/app/capture/details.tsx 2>/dev/null | head -5 || echo "Git not available"Repository: fortune710/keepsafe
Length of output: 95
🏁 Script executed:
# Check React Query documentation/usage - does useQuery support enabled in this codebase?
rg "enabled:" frontend/hooks -B 2 -A 2 | head -30Repository: fortune710/keepsafe
Length of output: 1660
Gate location fetch behind LOCATION_SHARE privacy setting.
useDeviceLocation() executes unconditionally at line 55 and immediately triggers Location.requestForegroundPermissionsAsync() regardless of the PrivacySettings.LOCATION_SHARE setting. Users with location sharing disabled will still receive a permission prompt. Modify useDeviceLocation() to accept an enabled parameter (following the pattern used in other hooks like use-privacy-settings.ts), then compute showLocation before the hook call and pass it as enabled: showLocation.
🤖 Prompt for AI Agents
In frontend/app/capture/details.tsx around lines 54 to 56, useDeviceLocation()
is called unconditionally causing Location.requestForegroundPermissionsAsync()
even when LOCATION_SHARE is disabled; compute a showLocation boolean from
privacySettings (e.g. privacySettings[PrivacySettings.LOCATION_SHARE] === true)
before calling the hook and call useDeviceLocation({ enabled: showLocation })
instead; update the useDeviceLocation hook signature to accept an options object
with an enabled flag and short-circuit any permission requests and location
fetch when enabled is false so no permission prompt appears.
| const showLocation = privacySettings[PrivacySettings.LOCATION_SHARE] ?? false; | ||
| const locationTag = showLocation && location?.city | ||
| ? [location.city, location.region ?? location.country].filter(Boolean).join(', ') | ||
| : null; | ||
|
|
There was a problem hiding this comment.
region ?? country won’t fall back when region is ''; use ||.
Because region is often '', region ?? country picks '' and you lose the country.
-const locationTag = showLocation && location?.city
- ? [location.city, location.region ?? location.country].filter(Boolean).join(', ')
+const locationTag = showLocation && location?.city
+ ? [location.city, location.region || location.country].filter(Boolean).join(', ')
: null;📝 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 showLocation = privacySettings[PrivacySettings.LOCATION_SHARE] ?? false; | |
| const locationTag = showLocation && location?.city | |
| ? [location.city, location.region ?? location.country].filter(Boolean).join(', ') | |
| : null; | |
| const showLocation = privacySettings[PrivacySettings.LOCATION_SHARE] ?? false; | |
| const locationTag = showLocation && location?.city | |
| ? [location.city, location.region || location.country].filter(Boolean).join(', ') | |
| : null; |
🤖 Prompt for AI Agents
In frontend/app/capture/details.tsx around lines 163 to 167, the expression
"region ?? country" incorrectly retains an empty string for region and thus
drops the country; change the fallback to use logical OR (region ||
location.country) so an empty string will fall through to the country (or
alternatively normalize with .trim() before checking), and update the
locationTag construction accordingly.
| export default function FriendItem({ friend, onRemove, onPress, onAccept, onDecline, onBlock, index = 0 }: FriendItemProps) { | ||
| const confirmRemove = () => { | ||
| Alert.alert( | ||
| 'Remove Friend', | ||
| `Are you sure you want to remove ${friend.name} from your friends?`, | ||
| [ | ||
| { text: 'Cancel', style: 'cancel' }, | ||
| { | ||
| text: 'Remove', | ||
| style: 'destructive', | ||
| onPress: () => onRemove(friend.id) | ||
| }, | ||
| ] | ||
| ); | ||
| }; | ||
|
|
||
| const confirmBlock = () => { | ||
| Alert.alert( | ||
| 'Block Friend', | ||
| `Are you sure you want to block ${friend.name}? They will no longer be able to interact with you.`, | ||
| [ | ||
| { text: 'Cancel', style: 'cancel' }, | ||
| { | ||
| text: 'Block', | ||
| style: 'destructive', | ||
| onPress: () => onBlock?.(friend.id), | ||
| }, | ||
| ] | ||
| ); | ||
| }; | ||
|
|
There was a problem hiding this comment.
Don’t show a “Block Friend” path if onBlock isn’t wired (currently becomes a no-op)
Because onBlock is optional, users can confirm “Block” and nothing happens. Gate the option on onBlock (or make it required when friend.status === 'connected').
} else {
// For connected friends, show options to remove or block
- Alert.alert(
- 'Friend Options',
- `What would you like to do with ${friend.name}?`,
- [
- { text: 'Cancel', style: 'cancel' },
- {
- text: 'Remove Friend',
- onPress: confirmRemove,
- },
- {
- text: 'Block Friend',
- style: 'destructive',
- onPress: confirmBlock,
- },
- ]
- );
+ const buttons = [
+ { text: 'Cancel', style: 'cancel' as const },
+ { text: 'Remove Friend', onPress: confirmRemove },
+ ...(onBlock
+ ? [{ text: 'Block Friend', style: 'destructive' as const, onPress: confirmBlock }]
+ : []),
+ ];
+ Alert.alert('Friend Options', `What would you like to do with ${friend.name}?`, buttons);
}
};Also applies to: 61-91
🤖 Prompt for AI Agents
frontend/components/friend-item.tsx lines 30-60 and 61-91: the Alert shown for
blocking includes a "Block" action even when the optional onBlock prop is
undefined, resulting in a no-op; update both places to only include the
destructive "Block" action (and call onBlock) when onBlock is truthy (or when
friend.status indicates blocking should be allowed), otherwise omit that action
from the Alert options (and ensure any UI element that triggers confirmBlock is
hidden/disabled when onBlock is not provided).
| const { | ||
| data, | ||
| isLoading, | ||
| error, | ||
| getCurrentLocation, | ||
| isFetching, | ||
| } = useQuery<LocationData | null, Error>({ | ||
| queryKey: ['device-location'], | ||
| queryFn: async () => { | ||
| // Request permissions | ||
| const { status } = await Location.requestForegroundPermissionsAsync(); | ||
|
|
||
| // If permission not granted, return null (no error) | ||
| if (status !== 'granted') { | ||
| return null; | ||
| } | ||
|
|
||
| try { | ||
| const position = await Location.getCurrentPositionAsync({ | ||
| accuracy: Location.Accuracy.Balanced, | ||
| }); | ||
|
|
||
| const reverseGeocode = await Location.reverseGeocodeAsync({ | ||
| latitude: position.coords.latitude, | ||
| longitude: position.coords.longitude, | ||
| }); | ||
|
|
||
| if (!reverseGeocode || reverseGeocode.length === 0) { | ||
| throw new Error('Unable to determine location address'); | ||
| } | ||
|
|
||
| const address = reverseGeocode[0]; | ||
| const locationData: LocationData = { | ||
| address: `${address.streetNumber || ''} ${address.street || ''}`.trim(), | ||
| city: address.city || address.subregion || '', | ||
| region: address.region || '', | ||
| country: address.country || '', | ||
| formattedAddress: formatAddress(address), | ||
| }; | ||
|
|
||
| return locationData; | ||
| } catch (err: any) { | ||
| console.error('Location error:', err); | ||
|
|
||
| // Translate known errors into a user-friendly message | ||
| if (err instanceof Error) { | ||
| if (err.message.includes('Location request failed') || err.message.includes('timeout')) { | ||
| throw new Error('Location request failed. Please try again.'); | ||
| } | ||
| if (err.message.includes('Location provider is unavailable')) { | ||
| throw new Error('Location services are unavailable. Please check your settings.'); | ||
| } | ||
| if (err.message.includes('Network')) { | ||
| throw new Error('Network error while getting location. Please check your connection.'); | ||
| } | ||
| throw err; | ||
| } | ||
|
|
||
| throw new Error('An unexpected error occurred while getting location.'); | ||
| } | ||
| }, | ||
| staleTime: 5 * 60 * 1000, // 5 minutes - adjust based on use case | ||
| retry: (failureCount, error) => { | ||
| // Don't retry permission or service availability errors | ||
| if (error.message.includes('unavailable') || error.message.includes('permission')) { | ||
| return false; | ||
| } | ||
| return failureCount < 2; | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Don’t auto-request location permission on mount; gate the query (privacy/UX regression).
useQuery runs immediately and calls requestForegroundPermissionsAsync(), so any screen that mounts this hook can trigger a permission prompt even if the user has LOCATION_SHARE disabled (or the UI never needs location).
Suggested pattern: add an enabled option to the hook and wire callers to privacy/UI intent.
interface UseDeviceLocationResult {
location: LocationData | null;
isLoading: boolean;
error: string | null;
clearLocation: () => void;
}
-export function useDeviceLocation(): UseDeviceLocationResult {
+export function useDeviceLocation(options?: { enabled?: boolean }): UseDeviceLocationResult {
const queryClient = useQueryClient();
+ const enabled = options?.enabled ?? true;
const {
data,
isLoading,
error,
isFetching,
} = useQuery<LocationData | null, Error>({
queryKey: ['device-location'],
+ enabled,
queryFn: async () => {
// Request permissions
const { status } = await Location.requestForegroundPermissionsAsync();
// ...
},Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In frontend/hooks/use-device-location.ts around lines 53 to 121, the hook
currently requests foreground location permission immediately because useQuery
runs on mount; change the hook to accept (or derive) an enabled flag and pass it
to useQuery's enabled option so the permission prompt only happens when callers
explicitly opt-in (e.g., user toggles LOCATION_SHARE or UI intent). Update the
queryFn to assume permissions are already requested only when enabled, keep the
same error handling, and ensure callers are updated to call the hook with
enabled: boolean (or expose a trigger function) so permission requests are gated
behind user action or explicit consent.
| const clearLocation = useCallback(() => { | ||
| queryClient.setQueryData(['device-location'], null); | ||
| }, [queryClient]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd frontend && find . -name "use-device-location.ts" -type fRepository: fortune710/keepsafe
Length of output: 93
🏁 Script executed:
cat -n frontend/hooks/use-device-location.tsRepository: fortune710/keepsafe
Length of output: 4997
🌐 Web query:
react-query setQueryData race condition cancelQueries
💡 Result:
Short answer: this is a common race — an in-flight query can resolve after you call queryClient.setQueryData (optimistic update) and overwrite it. Fix: before setQueryData, cancel the in-flight queries for that key, snapshot getQueryData, setQueryData, then on error restore the snapshot and onSettled invalidate/refetch. Note cancelQueries by default may "revert" the query state — await it before mutating. See docs/examples below.
Pattern (concise):
- await queryClient.cancelQueries({ queryKey }) to stop/refreeze in-flight refetches. [2][3]
- const previous = queryClient.getQueryData(queryKey)
- queryClient.setQueryData(queryKey, updater) // synchronous optimistic update. [2]
- onError => queryClient.setQueryData(queryKey, previous)
- onSettled => queryClient.invalidateQueries({ queryKey }) to reconcile with server. [1][2]
Example (useMutation onMutate):
const previous = queryClient.getQueryData(['todos'])
await queryClient.cancelQueries({ queryKey: ['todos'] })
queryClient.setQueryData(['todos'], old => [...(old||[]), newTodo])
return { previous } // rollback on error; invalidate onSettled. [1]
Notes:
- setQueryData is synchronous and will update cache immediately; it doesn't block in-flight fetches unless you cancel them first. [2]
- cancelQueries options include revert and silent; be aware revert can restore prior state — typically you await cancelQueries() then setQueryData. [2][3]
Sources:
- Optimistic updates guide (examples with cancelQueries + setQueryData + rollback). [1]
- QueryClient reference: setQueryData / cancelQueries / invalidateQueries details. [2]
- Query cancellation guide (behavior, AbortSignal, cancel options). [3]
🌐 Web query:
TanStack React Query cancelQueries await best practice
💡 Result:
Short best-practices summary
-
Why await cancelQueries: awaiting ensures any in-flight fetches that React Query can cancel are cancelled and that query state has been reverted (if revert:true). This avoids an in-flight refetch accidentally overwriting an optimistic update. Use await before reading or mutating cache. [1][2][3]
-
Typical pattern for optimistic updates
- await queryClient.cancelQueries({ queryKey }); // stop in-flight requests.
- const previous = queryClient.getQueryData(queryKey);
- queryClient.setQueryData(queryKey, optimisticValue);
- perform mutation; on error rollback with setQueryData(queryKey, previous); on success invalidate/refetch.
Example (TSX):
const previous = queryClient.getQueryData(['posts']);
await queryClient.cancelQueries({ queryKey: ['posts'] });
queryClient.setQueryData(['posts'], old => /* optimistic change */);
try { await api.update(...) ; await queryClient.invalidateQueries({ queryKey: ['posts'] }) }
catch { queryClient.setQueryData(['posts'], previous) }.
[2][3]
-
cancelQueries options to know
- revert (default true): when true, restores previous data and fetchStatus; when you want only to stop the request but keep current data, set revert:false.
- silent: suppresses CancelledError propagation (useful inside try/catch or when you don't want error callbacks).
Choose { revert: false } if you want to keep current UI data and only stop the network request. [3][4]
-
Cancellation relies on your queryFn consuming the provided AbortSignal (or returning a cancellable promise). If your fetch ignores the signal, cancellation will only affect query state, not the underlying network call. Use fetch(..., { signal }) or a client that supports AbortController. [1][3]
References
- Query cancellation guide (TanStack Query docs). [1] [3]
- QueryClient.cancelQueries and options (reference). [2] [4]
Sources
[1] https://tanstack.com/query/v3/docs/framework/react/guides/query-cancellation
[2] https://tanstack.com/query/v3/docs/framework/react/reference/QueryClient
[3] https://tanstack.com/query/latest/docs/framework/react/guides/query-cancellation
[4] https://tanstack.com/query/latest/docs/reference/QueryClient/
🌐 Web query:
react-query removeQueries vs cancelQueries clearLocation
💡 Result:
Short answer — key differences:
-
cancelQueries: cancels any matching in‑flight requests (uses AbortSignal / promise cancel) and can revert query state; useful before optimistic updates. Supports cancel options like silent and revert. [1][2]
-
removeQueries: removes matching queries from the cache (and unsubscribes observers). It does not, by itself, perform the same cancellation/revert behavior for in‑flight fetches. Use this to evict cached queries. [2]
-
clear: removes/clears all connected caches (global wipe). Use with care — it clears everything. [3]
References:
[1] TanStack Query — Query cancellation guide.
[2] TanStack Query — QueryClient reference (cancelQueries / removeQueries).
[3] TanStack Query — QueryClient.clear documentation.
Add await cancelQueries() before setQueryData() to prevent race condition.
setQueryData(null) can be overwritten by an in-flight query resolving after clearLocation(). Cancel in-flight requests first:
const clearLocation = useCallback(() => {
+ await queryClient.cancelQueries({ queryKey: ['device-location'] });
queryClient.setQueryData(['device-location'], null);
}, [queryClient]);The callback must be async to support the await.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In frontend/hooks/use-device-location.ts around lines 123 to 125, the
clearLocation callback should cancel any in-flight queries for
['device-location'] before calling setQueryData to avoid a race where an
inflight query overwrites the cleared value; make the callback async, call await
queryClient.cancelQueries(['device-location']) first, then call
queryClient.setQueryData(['device-location'], null), and keep queryClient in the
dependency array.
1 participant
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.