Completed Friends Page UI Update and Daily Notifications#41
Completed Friends Page UI Update and Daily Notifications#41fortune710 merged 13 commits intomainfrom
Conversation
Co-authored-by: alebiosuf0802 <alebiosuf0802@students.bowiestate.edu>
Co-authored-by: alebiosuf0802 <alebiosuf0802@students.bowiestate.edu>
…tings-persistence-02a2 Notification settings persistence
|
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis PR filters entry owners from notification recipients, enhances push notification permission handling with per-user settings persistence, introduces LocalNotificationService for scheduling notifications, updates typography and responsive scaling across UI components, and bumps dependencies. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant App as Frontend App
participant PushHook as usePushNotifications
participant PushSvc as PushNotificationService
participant DB as Backend/Supabase
participant OS as OS Notification System
User->>App: Launch or initialize
App->>PushHook: usePushNotifications(userId)
PushHook->>OS: Request push permission
OS-->>PushHook: Permission result
PushHook->>PushSvc: registerAndGetToken()
PushSvc->>OS: Get device token
OS-->>PushSvc: token + permission status
PushSvc->>DB: savePushToken(token, userId)
DB-->>PushSvc: success
alt Permission Granted & userId Available
PushHook->>PushSvc: initializeNotificationSettingsFromPermission(userId, true)
PushSvc->>DB: Check existing notification_settings
alt Settings Exist
DB-->>PushSvc: return existing row
else Settings Missing
PushSvc->>DB: Create settings with ALL_NOTIFICATIONS_ON
DB-->>PushSvc: success
end
else Permission Denied
PushHook->>PushSvc: initializeNotificationSettingsFromPermission(userId, false)
PushSvc->>DB: Create/update settings with ALL_NOTIFICATIONS_OFF
DB-->>PushSvc: success
end
PushHook-->>App: Ready with configured tokens
sequenceDiagram
participant Streak as Streak Service
participant Cache as Query Cache
participant LocalNotifSvc as LocalNotificationService
participant Supabase as Supabase
participant OS as OS Notification System
Streak->>Streak: updateStreak(data)
Streak->>Supabase: Update streak row
Supabase-->>Streak: Updated data
Streak->>LocalNotifSvc: cancelAllScheduledNotifications()
LocalNotifSvc->>OS: Cancel all pending
OS-->>LocalNotifSvc: Cancelled
alt Current Streak >= 1
Streak->>LocalNotifSvc: scheduleNotification(midday reminder)
LocalNotifSvc->>OS: Schedule with trigger time
OS-->>LocalNotifSvc: Scheduled identifier
else Current Streak < 1
Streak->>LocalNotifSvc: scheduleNotification(general reminder)
LocalNotifSvc->>OS: Schedule with trigger time
OS-->>LocalNotifSvc: Scheduled identifier
end
Streak->>Cache: Update local cache
Cache-->>Streak: Success
Streak-->>Streak: Notification lifecycle complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
frontend/hooks/use-invite-acceptance.ts (2)
137-142: Analytics event fires even for already-connected users.
invite_acceptedis captured regardless of whether this was a new friendship or an existing one. This could inflate acceptance metrics. Consider only tracking when a new connection is actually created.
160-180:declineInviteis a no-op that always returns success.The function body is empty — no database operation actually declines or removes the invite. Returning
success: trueis misleading. Either implement the decline logic or return an explicit "not implemented" response.frontend/components/friend-item.tsx (1)
119-151: Root element changed toViewbreaksonPressfunctionality.The component accepts an
onPressprop and has ahandlePressfunction (lines 94-98), but replacingTouchableOpacitywithViewmeans tapping the item no longer triggers any action. Either:
- Remove the
onPressprop andhandlePressfunction if touch-on-item is no longer needed, or- Restore
TouchableOpacityif users should be able to tap the friend item.🐛 Option 1: Remove dead code if touch not needed
interface FriendItemProps { friend: Friend; onRemove: (friendId: string) => void; - onPress?: (friend: Friend) => void; onAccept?: (friendshipId: string) => void; onDecline?: (friendshipId: string) => void; onBlock?: (friendshipId: string) => void; index?: number; } -export default function FriendItem({ friend, onRemove, onPress, onAccept, onDecline, onBlock, index = 0 }: FriendItemProps) { +export default function FriendItem({ friend, onRemove, onAccept, onDecline, onBlock, index = 0 }: FriendItemProps) { // ... remove handlePress function (lines 94-98)🐛 Option 2: Restore TouchableOpacity if touch needed
- <View + <TouchableOpacity style={styles.container} + onPress={handlePress} + activeOpacity={0.7} > {/* ... content ... */} - </View> + </TouchableOpacity>frontend/components/friends/suggested-friends-list.tsx (1)
26-34: Bug: Renderingfriendsinstead offilteredFriends.Line 15 correctly filters out the current user into
filteredFriends, and line 17 checks iffilteredFriendsis empty. However, line 27 maps over the originalfriendsarray, which means the current user could still appear in the suggested friends list.🐛 Proposed fix
{ - friends.map((friend, index) => ( + filteredFriends.map((friend, index) => ( <SuggestedFriendItem key={friend.id} friend={friend} index={index} /> )) }frontend/components/friends-section.tsx (1)
77-87: Animation order is inverted relative to visual order.Pending friends are rendered first in the ScrollView but receive higher indices (
connectedFriends.length + index), while connected friends below receive lower indices starting at 0. SinceFriendItemusesFadeInDown.delay(index * 50), the bottom section will animate before the top section, creating a jarring UX.🛠️ Proposed fix to correct animation sequencing
{pendingFriends.map((friend, index) => ( <FriendItem key={friend.id} friend={friend} onRemove={onRemoveFriend} onPress={onFriendPress} onAccept={onAcceptRequest} onDecline={onDeclineRequest} onBlock={onBlockFriend} - index={connectedFriends.length + index} + index={index} /> ))}{connectedFriends.map((friend, index) => ( <FriendItem key={friend.id} friend={friend} onRemove={onRemoveFriend} onPress={onFriendPress} onAccept={onAcceptRequest} onDecline={onDeclineRequest} onBlock={onBlockFriend} - index={index} + index={pendingFriends.length + index} /> ))}Also applies to: 101-111
frontend/app.json (1)
58-64: Avoid WRITE_CONTACTS unless you truly modify contacts.
WRITE_CONTACTSis a high‑sensitivity permission and can trigger additional Play Store review and data‑safety obligations. If the app only reads contacts, remove it; if you do write/update contacts, ensure runtime requests and disclosures are in place.🧹 Optional cleanup if write access isn’t required
"android.permission.READ_CONTACTS", "android.permission.MODIFY_AUDIO_SETTINGS", - "android.permission.WRITE_CONTACTS"
🤖 Fix all issues with AI agents
In `@frontend/components/friends/suggested-friend-item.tsx`:
- Around line 26-28: The useFriends hook is being called without the required
userId so checkFriendStatus(friend.id) always sees empty arrays; update the call
in suggested-friend-item.tsx to pass the current user's id into useFriends
(e.g., useFriends(currentUserId) or the appropriate auth/user context value)
before calling checkFriendStatus, ensuring the friends, pendingRequests, and
blockedFriends arrays are populated so checkFriendStatus(friend.id) can return
the correct status.
In `@frontend/hooks/use-friends.ts`:
- Around line 59-71: The checkFriendStatus function is comparing the wrong
property (f.id) against the user friendId parameter so it always misses; update
the three .find calls inside checkFriendStatus to compare f.friend_id ===
friendId (for friends, pendingRequests and blockedFriends) so the function
correctly detects ACCEPTED, PENDING, or BLOCKED statuses based on the friend's
user ID.
In `@frontend/package.json`:
- Line 4: The package.json "version" field was changed to 0.9.5 which is a
downgrade; confirm this is intentional and either revert to the previous
(higher) version or set it to the correct next semver (and update any
changelog/release notes) so tooling that relies on monotonic versions isn't
broken; locate and update the "version" entry in package.json accordingly and
add a short commit/PR comment noting the deliberate version choice.
- Around line 97-105: Update the `@types/jest` devDependency to match Jest's minor
version: change the "@types/jest" entry in devDependencies from "29.5.14" to a
version aligned with jest@29.7.0 (for example "~29.7.0" or "~29.7.x"); this
ensures type definitions are in sync with "jest" and avoids type drift
(alternatively you can remove "@types/jest" and adopt "@jest/globals" for
Jest-provided typings).
In `@frontend/services/streak-service.ts`:
- Around line 187-226: The else branch is unreachable because
newStreakData.currentStreak is always >= 1 in updateStreak; remove the dead else
block and simplify to a single notification scheduling path: after awaiting
LocalNotificationService.cancelAllScheduledNotifications(), always call
LocalNotificationService.scheduleNotification(...) with the "active streak"
payload (title 'How are you doing today?', body including
newStreakData.currentStreak, data.page '/capture', the same trigger and
identifier `streak_${userId}`). Update the code around
updateStreak/newStreakData to delete the else branch and its
scheduleNotification call so only the active-streak notification remains.
🧹 Nitpick comments (18)
frontend/hooks/use-search.ts (1)
89-92: Consider wrapping the analytics call in a try-catch.If the PostHog SDK throws an unexpected error (e.g., network serialization issues, SDK bugs), it could break the search flow for users. Wrapping analytics in a try-catch ensures telemetry failures never impact core functionality.
🛡️ Proposed defensive wrapper
try { + try { + posthog.capture('ai_search', { + query: userMessage.content, + platform: Platform.OS + }); + } catch { + // Silently ignore analytics failures + } - posthog.capture('ai_search', { - query: userMessage.content, - platform: Platform.OS - }) await SearchService.streamSearch({frontend/hooks/use-invite-acceptance.ts (2)
52-57: "Already connected" status is silently swallowed by the caller.The mutation now returns a structured object for the existing-friendship case, but
acceptInvite(lines 144-148) unconditionally returnssuccess: truewith "Invitation accepted successfully!" — themessagefield from this return is never propagated to the caller.If callers need to distinguish between "newly connected" and "already connected", consider either:
- Adding a status/flag to the mutation result (e.g.,
alreadyConnected: boolean)- Propagating the message to
InviteResult♻️ Suggested approach
if (existingFriendship) { return { friendshipId: (existingFriendship as any).id, - message: 'You are already connected with this user', + alreadyConnected: true, }; }Then in
acceptInvite:return { success: true, - message: 'Invitation accepted successfully!', + message: result.alreadyConnected + ? 'You are already connected with this user' + : 'Invitation accepted successfully!', friendId: result.friendshipId, };
54-54: Consider typing the Supabase query results instead of usingas any.Multiple
(... as any).idcasts bypass type safety. You could type the select result or use a helper type to avoid this pattern.Also applies to: 76-76
frontend/hooks/use-friends.ts (1)
59-71: Consider memoizingcheckFriendStatuswithuseCallback.Since
checkFriendStatusdepends onfriends,pendingRequests, andblockedFriends, it should be wrapped inuseCallbackto maintain referential stability when these dependencies don't change. This prevents unnecessary re-renders in consumers that use this function in their dependency arrays.♻️ Proposed refactor
- const checkFriendStatus = ( - friendId: string - ): typeof FRIENDSHIP_STATUS.ACCEPTED | typeof FRIENDSHIP_STATUS.PENDING | typeof FRIENDSHIP_STATUS.BLOCKED | null => { - if (friends.find(f => f.id === friendId)) { - return FRIENDSHIP_STATUS.ACCEPTED; - } else if (pendingRequests.find(f => f.id === friendId)) { - return FRIENDSHIP_STATUS.PENDING; - } else if (blockedFriends.find(f => f.id === friendId)) { - return FRIENDSHIP_STATUS.BLOCKED; - } else { - return null; - } - }; + const checkFriendStatus = useCallback(( + friendId: string + ): typeof FRIENDSHIP_STATUS.ACCEPTED | typeof FRIENDSHIP_STATUS.PENDING | typeof FRIENDSHIP_STATUS.BLOCKED | null => { + if (friends.find(f => f.id === friendId)) { + return FRIENDSHIP_STATUS.ACCEPTED; + } else if (pendingRequests.find(f => f.id === friendId)) { + return FRIENDSHIP_STATUS.PENDING; + } else if (blockedFriends.find(f => f.id === friendId)) { + return FRIENDSHIP_STATUS.BLOCKED; + } else { + return null; + } + }, [friends, pendingRequests, blockedFriends]);frontend/components/friend-item.tsx (3)
158-163: Remove commented-out code.Commented-out styles (
backgroundColor,borderWidth) should be removed to keep the codebase clean. If these styles might be needed later, they can be retrieved from version control.♻️ Proposed fix
const styles = StyleSheet.create({ container: { - //backgroundColor: 'white', paddingHorizontal: verticalScale(7), marginBottom: verticalScale(10), flexDirection: 'row', alignItems: 'center', - //borderWidth: 1, },
199-201: RedundantfontWeightwhen using weightedfontFamily.When using a font family that includes the weight in its name (e.g.,
Outfit-Bold), specifyingfontWeight: '600'is redundant and may cause inconsistent rendering on some platforms.♻️ Proposed fix
friendName: { fontSize: moderateScale(14), fontFamily: 'Outfit-Bold', - fontWeight: '600', color: '#1E293B', marginBottom: 2, },
100-113: Consider usingColorsconstants for consistency.The
getStatusColorfunction uses hardcoded color values that are already defined inColors(fromfrontend/lib/constants.ts). Using the constants improves maintainability.♻️ Proposed fix
const getStatusColor = () => { switch (friend.status) { case 'connected': - return '#10B981'; + return Colors.positive; // or Colors.success case 'pending': - return '#F59E0B'; + return Colors.warning; case 'invited': return '#6B7280'; case 'blocked': - return '#EF4444'; + return Colors.danger; default: return '#6B7280'; } };frontend/components/friends/add-friends-section.tsx (1)
1-61: New component looks good overall.The component is well-structured with good accessibility practices (minimum touch target height). A few minor suggestions:
- Redundant
fontWeighton lines 40 and 58 when using weighted font families- Hardcoded colors on lines 15 and 41 could use
Colors.primaryandColors.textMutedfor consistency♻️ Optional improvements
<View style={styles.sectionHeader}> - <UserPlus color="#8B5CF6" size={16} /> + <UserPlus color={Colors.primary} size={16} /> <Text style={styles.sectionTitle}>Find More Friends</Text> </View>sectionTitle: { fontSize: moderateScale(14), fontFamily: 'Outfit-SemiBold', - fontWeight: '600', - color: '#64748B', + color: Colors.textMuted, marginLeft: 8, }, // ... shareButtonText: { color: Colors.white, fontSize: moderateScale(14), fontFamily: 'Outfit-Bold', - fontWeight: '600', marginLeft: 8, },frontend/components/friends/suggested-friends-list.tsx (1)
4-4: Unused import:Sparkle.The
Sparkleicon is imported but never used in this component.♻️ Proposed fix
-import { Contact, Sparkle } from "lucide-react-native"; +import { Contact } from "lucide-react-native";frontend/hooks/use-suggested-friends.ts (1)
36-50: Consider handling potential cache/storage inconsistency.The local storage update is fire-and-forget with only a warning on failure. If
deviceStorage.setSuggestedFriendsfails, the TanStack Query cache will be updated but local storage will retain stale data, causing inconsistency on app restart.💡 Suggested improvement to handle storage sync failure
const removeContactFromList = useCallback((friendId: string) => { + const previousData = queryClient.getQueryData<SuggestedFriend[]>(['suggested-friends']); + // Optimistically update TanStack Query cache queryClient.setQueryData<SuggestedFriend[]>(['suggested-friends'], (oldData) => { if (!oldData) return []; return oldData.filter(contact => contact.id !== friendId); }); // Optimistically update local storage deviceStorage.getSuggestedFriends().then((storedContacts) => { const updatedContacts = storedContacts.filter(contact => contact.id !== friendId); deviceStorage.setSuggestedFriends(updatedContacts); }).catch((error) => { - logger.warn('Failed to update local storage:', error); + logger.warn('Failed to update local storage, rolling back cache:', error); + // Rollback cache to maintain consistency + if (previousData) { + queryClient.setQueryData<SuggestedFriend[]>(['suggested-friends'], previousData); + } }); }, [queryClient]);frontend/app/friends.tsx (1)
101-113: Consider adding proper type annotation.Using
any[]loses type safety. If the friendship data structure is well-defined, consider typing it.💡 Optional type improvement
- const convertToFriendFormat = (friendships: any[]) => { + const convertToFriendFormat = (friendships: Array<{ + id: string; + status: string; + updated_at: string; + created_at: string; + friend_profile?: { + full_name?: string; + email?: string; + username?: string; + avatar_url?: string; + }; + }>) => {frontend/components/friends/suggested-friend-item.tsx (2)
74-81: UX: ACCEPTED friends show "Add" button label.When
friendStatus === FRIENDSHIP_STATUS.ACCEPTED, showing a disabled "Add" button is misleading. Consider displaying "Friends" or a checkmark icon instead.💡 Suggested improvement for accepted state
friendStatus === FRIENDSHIP_STATUS.PENDING || friendStatus === FRIENDSHIP_STATUS.ACCEPTED ? ( <TouchableOpacity - style={styles.pendingButton} + style={friendStatus === FRIENDSHIP_STATUS.PENDING ? styles.pendingButton : styles.acceptedButton} disabled={true} > - <Plus color={Colors.white} strokeWidth={3} size={20} /> - <Text style={styles.addButtonText}>Add</Text> + <Plus color={Colors.white} strokeWidth={3} size={20} /> + <Text style={styles.addButtonText}> + {friendStatus === FRIENDSHIP_STATUS.PENDING ? 'Pending' : 'Friends'} + </Text> </TouchableOpacity> ) : (Then add an
acceptedButtonstyle similar topendingButtonwith an appropriate color (e.g., green for connected).
98-107: Clean up commented-out style properties.Commented-out code like
//backgroundColorand//marginBottomshould be removed to keep the stylesheet clean.🧹 Remove commented code
container: { - //backgroundColor: 'white', borderRadius: 16, paddingHorizontal: verticalScale(7), marginBottom: verticalScale(10), - //marginBottom: verticalScale(0), flexDirection: 'row', alignItems: 'center', },frontend/services/streak-service.ts (1)
257-277: Consider extracting repeated notification scheduling logic.The same notification scheduling pattern (cancel all → schedule daily at 12:00) is duplicated across
updateStreak,checkAndUpdateStreak, andresetStreak. Consider extracting this into a private helper method to reduce duplication and centralize the notification content/trigger configuration.♻️ Example helper extraction
private static async scheduleStreakReminder( userId: string, hasActiveStreak: boolean, streakCount?: number ): Promise<void> { await LocalNotificationService.cancelAllScheduledNotifications(); const content = hasActiveStreak ? { title: 'How are you doing today?', body: `Capture your day to keep your ${streakCount} day streak alive!`, data: { page: '/capture' } } : { title: 'Come back to Keepsafe', body: 'Capture a moment from your day and start building your streak!', data: { page: '/capture' } }; await LocalNotificationService.scheduleNotification({ content, trigger: { type: Notifications.SchedulableTriggerInputTypes.DAILY, hour: 12, minute: 0, }, identifier: `streak_${userId}`, }); }Also applies to: 295-313, 337-355
frontend/services/local-notification-service.ts (1)
119-122: Android channel is reconfigured on every notification.
configureAndroidChannel()is called in bothsendNotificationandscheduleNotificationon every invocation. While this is safe (the call is idempotent), it's slightly inefficient. Consider calling it once during app initialization viaconfigureNotificationHandler()instead.♻️ Proposed optimization
static async configureNotificationHandler(): Promise<void> { try { Notifications.setNotificationHandler({ handleNotification: async () => ({ shouldShowAlert: true, shouldPlaySound: true, shouldSetBadge: false, shouldShowBanner: true, shouldShowList: false }), }); + + // Configure Android channel during initialization + if (Platform.OS === 'android') { + await this.configureAndroidChannel(); + } + logger.info('Notification handler configured'); } catch (error) { logger.error('Error configuring notification handler:', error); } }Then remove the
configureAndroidChannel()calls fromsendNotificationandscheduleNotification.Also applies to: 161-164
frontend/hooks/use-push-notifications.ts (1)
65-73: Potential double initialization call.When
userIdis available during registration,initializeNotificationSettingsFromPermissionmay be called twice:
- At line 104 during
registerForPushNotifications- When the effect on lines 65-73 runs (triggered by
permissionStatuschange at line 102)While this is safe because the function is idempotent (it checks for existing rows), it results in an unnecessary Supabase query. Consider guarding the effect to skip if we already called initialization during registration.
♻️ One approach using a ref to track initialization
+ const initializedRef = useRef(false); + useEffect(() => { // If we learned permission status before the userId was available, persist // the initial notification settings row once we have the userId. - if (!userId || !permissionStatus) return; + if (!userId || !permissionStatus || initializedRef.current) return; void PushNotificationService.initializeNotificationSettingsFromPermission( userId, permissionStatus === 'granted', ); + initializedRef.current = true; }, [userId, permissionStatus]);And set
initializedRef.current = trueafter the call inregisterForPushNotificationswhenuserIdis present.Also applies to: 102-108
frontend/hooks/use-notification-settings.ts (2)
104-159: Async IIFE withvoid- consider error feedback to user.The async permission flow uses
void (async () => { ... })()which discards any errors that might occur during permission handling or settings persistence. WhilesaveSettingshandles its own error state via the mutation, errors inNotifications.getPermissionsAsync()orinitializeNotificationSettingsFromPermissionwill be silently swallowed.Consider wrapping the async block in a try-catch to surface errors to the user:
♻️ Proposed error handling improvement
void (async () => { + try { // If we can't support push tokens on this device/platform, keep disabled. if (!Device.isDevice || (Platform.OS !== 'ios' && Platform.OS !== 'android')) { // ... existing code ... } // ... rest of async logic ... + } catch (error) { + console.error('Error enabling push notifications:', error); + // Optionally show user feedback or revert state + } })();
152-155: Potential stale closure withcurrentSettings.When the permission flow completes and calls
saveSettings({ ...currentSettings, ... }), thecurrentSettingsvalue is captured from whentoggleSettingwas called, not when the async operation completes. If settings were modified by another action during the async permission flow, those changes would be overwritten.This is a low-probability edge case since the user is unlikely to modify other settings while the permission dialog is showing, but worth noting for awareness.
| const { checkFriendStatus } = useFriends(); | ||
|
|
||
| const friendStatus = checkFriendStatus(friend.id); |
There was a problem hiding this comment.
Critical: useFriends() called without userId - status check will always fail.
The useFriends hook requires userId to enable the query and populate the friendship arrays. Without it, friends, pendingRequests, and blockedFriends remain empty, causing checkFriendStatus to always return null. The friend status logic will never detect existing friendships.
🐛 Proposed fix
const { profile } = useAuthContext();
const { acceptInvite: sendFriendRequest, isProcessing } = useInviteAcceptance();
const { toast: showToast } = useToast();
const { removeContactFromList } = useSuggestedFriends();
- const { checkFriendStatus } = useFriends();
+ const { checkFriendStatus } = useFriends(profile?.id);
const friendStatus = checkFriendStatus(friend.id);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { checkFriendStatus } = useFriends(); | |
| const friendStatus = checkFriendStatus(friend.id); | |
| const { checkFriendStatus } = useFriends(profile?.id); | |
| const friendStatus = checkFriendStatus(friend.id); |
🤖 Prompt for AI Agents
In `@frontend/components/friends/suggested-friend-item.tsx` around lines 26 - 28,
The useFriends hook is being called without the required userId so
checkFriendStatus(friend.id) always sees empty arrays; update the call in
suggested-friend-item.tsx to pass the current user's id into useFriends (e.g.,
useFriends(currentUserId) or the appropriate auth/user context value) before
calling checkFriendStatus, ensuring the friends, pendingRequests, and
blockedFriends arrays are populated so checkFriendStatus(friend.id) can return
the correct status.
| const checkFriendStatus = ( | ||
| friendId: string | ||
| ): typeof FRIENDSHIP_STATUS.ACCEPTED | typeof FRIENDSHIP_STATUS.PENDING | typeof FRIENDSHIP_STATUS.BLOCKED | null => { | ||
| if (friends.find(f => f.id === friendId)) { | ||
| return FRIENDSHIP_STATUS.ACCEPTED; | ||
| } else if (pendingRequests.find(f => f.id === friendId)) { | ||
| return FRIENDSHIP_STATUS.PENDING; | ||
| } else if (blockedFriends.find(f => f.id === friendId)) { | ||
| return FRIENDSHIP_STATUS.BLOCKED; | ||
| } else { | ||
| return null; | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find FriendWithProfile type definition
rg -n "interface FriendWithProfile|type FriendWithProfile" --type=ts -A 10Repository: fortune710/keepsafe
Length of output: 234
🏁 Script executed:
# Find FriendService.getFriends to understand the data shape
fd "friend.*service" --type=ts | head -5Repository: fortune710/keepsafe
Length of output: 232
🏁 Script executed:
# Check use-friends.ts for how friends/pendingRequests/blockedFriends are populated
cat -n frontend/hooks/use-friends.ts | head -60Repository: fortune710/keepsafe
Length of output: 2860
🏁 Script executed:
# Find usages of checkFriendStatus in the codebase
rg -n "checkFriendStatus" --type=ts --type=tsxRepository: fortune710/keepsafe
Length of output: 90
🏁 Script executed:
# Find the Friendship interface definition
rg -n "interface Friendship|type Friendship" --type=typescript -A 15 | head -40Repository: fortune710/keepsafe
Length of output: 97
🏁 Script executed:
# Search for checkFriendStatus usage in the codebase
rg -n "checkFriendStatus" -B 2 -A 2Repository: fortune710/keepsafe
Length of output: 1871
🏁 Script executed:
# Also check database types to understand the Friendship structure
rg -n "friendship" frontend/types/database.ts -A 5Repository: fortune710/keepsafe
Length of output: 263
🏁 Script executed:
# Get the complete checkFriendStatus function implementation
sed -n '59,71p' frontend/hooks/use-friends.tsRepository: fortune710/keepsafe
Length of output: 571
checkFriendStatus is comparing the wrong ID property and will never find a match.
The function compares f.id (the friendship record ID) against the friendId parameter (a user ID), but should compare f.friend_id (the friend's user ID). Since these are different values, the function will always return null regardless of the friendship status.
Fix by changing all three .find() comparisons from f.id === friendId to f.friend_id === friendId.
🤖 Prompt for AI Agents
In `@frontend/hooks/use-friends.ts` around lines 59 - 71, The checkFriendStatus
function is comparing the wrong property (f.id) against the user friendId
parameter so it always misses; update the three .find calls inside
checkFriendStatus to compare f.friend_id === friendId (for friends,
pendingRequests and blockedFriends) so the function correctly detects ACCEPTED,
PENDING, or BLOCKED statuses based on the friend's user ID.
| "name": "keepsafe", | ||
| "main": "expo-router/entry", | ||
| "version": "1.0.0", | ||
| "version": "0.9.5", |
There was a problem hiding this comment.
Confirm the version downgrade is intentional.
The package version moved to 0.9.5. If any tooling uses package.json for tagging, release gating, or analytics, a downgrade can break monotonic versioning. Please confirm this is deliberate.
🤖 Prompt for AI Agents
In `@frontend/package.json` at line 4, The package.json "version" field was
changed to 0.9.5 which is a downgrade; confirm this is intentional and either
revert to the previous (higher) version or set it to the correct next semver
(and update any changelog/release notes) so tooling that relies on monotonic
versions isn't broken; locate and update the "version" entry in package.json
accordingly and add a short commit/PR comment noting the deliberate version
choice.
| "devDependencies": { | ||
| "@babel/core": "^7.25.2", | ||
| "@testing-library/react-native": "^13.3.3", | ||
| "@types/jest": "^30.0.0", | ||
| "@types/jest": "29.5.14", | ||
| "@types/react": "~19.1.10", | ||
| "babel-plugin-module-resolver": "^5.0.2", | ||
| "jest": "~29.7.0", | ||
| "jest-expo": "~54.0.12", | ||
| "jest-expo": "~54.0.16", | ||
| "supabase": "^2.39.2", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
jest-expo 54.0.16 peer dependencies jest version requirements
💡 Result:
jest-expo@54.0.16 does not declare jest as a peer dependency (so there are no peer-dependency version requirements for jest).
Its peerDependencies are: expo: "*", react-native: "*", and (optional) react-server-dom-webpack: "~19.0.3 || ~19.1.4 || ~19.2.3"; it includes Jest-related packages as regular dependencies (for example @jest/globals, babel-jest, jest-snapshot, etc., all ^29.2.1). [1]
🌐 Web query:
jest 29.7.0 @types/jest 29.5.14 compatibility
💡 Result:
- They’ll generally work together, because
@types/jestis meant to track Jest by major.minor, and its patch version is independent of Jest’s patch version. (tkcnn.com) - But it’s not an ideal match:
jest@29.7.0is 29.7, while@types/jest@29.5.14targets 29.5 (and@types/jestcan lag behind Jest), so you may miss/incorrectly type newer APIs added after 29.5. (jestjs.io)
Best options:
- Prefer Jest’s maintained typings via
@jest/globals(recommended by Jest docs) instead of relying on@types/jest. (jestjs.io) - If you want DefinitelyTyped typings, match major.minor more closely (i.e., use
jest@29.5.xwith@types/jest@29.5.x). (jestjs.io) - If you upgrade Jest to v30, use
@types/jest@30.0.0(latest major). (jestjs.io)
Citations:
- 1: https://www.tkcnn.com/github/DefinitelyTyped/DefinitelyTyped.html?utm_source=openai
- 2: https://jestjs.io/docs/29.7/getting-started?utm_source=openai
- 3: https://jestjs.io/docs/29.7/getting-started?utm_source=openai
- 4: https://jestjs.io/docs/29.7/getting-started?utm_source=openai
- 5: https://jestjs.io/versions?utm_source=openai
🌐 Web query:
Expo SDK 54 jest-expo jest recommended versions
💡 Result:
For Expo SDK 54, you should stay on the jest-expo 54.0.x line (it’s versioned to match the SDK). The current latest 54.x release is jest-expo@54.0.16. [1]
Recommended install (lets Expo pick the compatible pair):
npx expo install jest-expo jest[2]
If you need to pin versions manually for SDK 54:
jest-expo:~54.0.16(or generally~54.0.x) [1]jest: 29.x (becausejest-expo@54.0.16is built around Jest 29.2.1-era packages likebabel-jest,@jest/globals, etc.). [3]
Sources:
[1] https://cdn.jsdelivr.net/npm/jest-expo/
[2] https://www.npmjs.com/package/jest-expo/v/54.0.1
[3] https://cdn.jsdelivr.net/npm/jest-expo%4054.0.16/package.json
Update @types/jest to match Jest's minor version.
The trio (jest@29.7.0, jest-expo@54.0.16, @types/jest@29.5.14) works with Expo SDK 54, but @types/jest@29.5.14 lags behind jest@29.7.0. This minor version mismatch may result in type drift for APIs added between 29.5 and 29.7. Update @types/jest to ~29.7.x to align with Jest's version and get complete type coverage, or alternatively consider using @jest/globals for Jest-maintained typings.
🤖 Prompt for AI Agents
In `@frontend/package.json` around lines 97 - 105, Update the `@types/jest`
devDependency to match Jest's minor version: change the "@types/jest" entry in
devDependencies from "29.5.14" to a version aligned with jest@29.7.0 (for
example "~29.7.0" or "~29.7.x"); this ensures type definitions are in sync with
"jest" and avoids type drift (alternatively you can remove "@types/jest" and
adopt "@jest/globals" for Jest-provided typings).
|
|
||
| // Always schedule a notification, content depends on streak status | ||
| // Cancel any existing notifications first to avoid duplicates | ||
| await LocalNotificationService.cancelAllScheduledNotifications(); | ||
|
|
||
| if (newStreakData.currentStreak >= 1) { | ||
| // Active streak - remind about keeping streak alive | ||
| await LocalNotificationService.scheduleNotification({ | ||
| content: { | ||
| title: 'How are you doing today?', | ||
| body: `Capture your day to keep your ${newStreakData.currentStreak} day streak alive!`, | ||
| data: { | ||
| page: '/capture' | ||
| } | ||
| }, | ||
| trigger: { | ||
| type: Notifications.SchedulableTriggerInputTypes.DAILY, | ||
| hour: 12, | ||
| minute: 0, | ||
| }, | ||
| identifier: `streak_${userId}`, | ||
| }); | ||
| } else { | ||
| // No active streak - general app reminder | ||
| await LocalNotificationService.scheduleNotification({ | ||
| content: { | ||
| title: 'Come back to Keepsafe', | ||
| body: 'Capture a moment from your day and start building your streak!', | ||
| data: { | ||
| page: '/capture' | ||
| } | ||
| }, | ||
| trigger: { | ||
| type: Notifications.SchedulableTriggerInputTypes.DAILY, | ||
| hour: 12, | ||
| minute: 0, | ||
| }, | ||
| identifier: `streak_${userId}`, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Unreachable else branch - currentStreak is always ≥ 1 at this point.
In updateStreak, after processing the entry, newStreakData.currentStreak is set to at least 1 in all code paths:
- First entry ever →
currentStreak = 1(line 148) - Same day → no change, but previous streak was ≥ 1 to have a
lastEntryDate - Consecutive day → incremented from ≥ 1
- Gap → reset to 1 (line 179)
The condition on line 192 (currentStreak >= 1) will always be true, making the else branch (lines 209-226) dead code.
🧹 Proposed fix to remove unreachable code
- if (newStreakData.currentStreak >= 1) {
- // Active streak - remind about keeping streak alive
- await LocalNotificationService.scheduleNotification({
- content: {
- title: 'How are you doing today?',
- body: `Capture your day to keep your ${newStreakData.currentStreak} day streak alive!`,
- data: {
- page: '/capture'
- }
- },
- trigger: {
- type: Notifications.SchedulableTriggerInputTypes.DAILY,
- hour: 12,
- minute: 0,
- },
- identifier: `streak_${userId}`,
- });
- } else {
- // No active streak - general app reminder
- await LocalNotificationService.scheduleNotification({
- content: {
- title: 'Come back to Keepsafe',
- body: 'Capture a moment from your day and start building your streak!',
- data: {
- page: '/capture'
- }
- },
- trigger: {
- type: Notifications.SchedulableTriggerInputTypes.DAILY,
- hour: 12,
- minute: 0,
- },
- identifier: `streak_${userId}`,
- });
- }
+ // Active streak - remind about keeping streak alive
+ await LocalNotificationService.scheduleNotification({
+ content: {
+ title: 'How are you doing today?',
+ body: `Capture your day to keep your ${newStreakData.currentStreak} day streak alive!`,
+ data: {
+ page: '/capture'
+ }
+ },
+ trigger: {
+ type: Notifications.SchedulableTriggerInputTypes.DAILY,
+ hour: 12,
+ minute: 0,
+ },
+ identifier: `streak_${userId}`,
+ });📝 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.
| // Always schedule a notification, content depends on streak status | |
| // Cancel any existing notifications first to avoid duplicates | |
| await LocalNotificationService.cancelAllScheduledNotifications(); | |
| if (newStreakData.currentStreak >= 1) { | |
| // Active streak - remind about keeping streak alive | |
| await LocalNotificationService.scheduleNotification({ | |
| content: { | |
| title: 'How are you doing today?', | |
| body: `Capture your day to keep your ${newStreakData.currentStreak} day streak alive!`, | |
| data: { | |
| page: '/capture' | |
| } | |
| }, | |
| trigger: { | |
| type: Notifications.SchedulableTriggerInputTypes.DAILY, | |
| hour: 12, | |
| minute: 0, | |
| }, | |
| identifier: `streak_${userId}`, | |
| }); | |
| } else { | |
| // No active streak - general app reminder | |
| await LocalNotificationService.scheduleNotification({ | |
| content: { | |
| title: 'Come back to Keepsafe', | |
| body: 'Capture a moment from your day and start building your streak!', | |
| data: { | |
| page: '/capture' | |
| } | |
| }, | |
| trigger: { | |
| type: Notifications.SchedulableTriggerInputTypes.DAILY, | |
| hour: 12, | |
| minute: 0, | |
| }, | |
| identifier: `streak_${userId}`, | |
| }); | |
| } | |
| // Always schedule a notification, content depends on streak status | |
| // Cancel any existing notifications first to avoid duplicates | |
| await LocalNotificationService.cancelAllScheduledNotifications(); | |
| // Active streak - remind about keeping streak alive | |
| await LocalNotificationService.scheduleNotification({ | |
| content: { | |
| title: 'How are you doing today?', | |
| body: `Capture your day to keep your ${newStreakData.currentStreak} day streak alive!`, | |
| data: { | |
| page: '/capture' | |
| } | |
| }, | |
| trigger: { | |
| type: Notifications.SchedulableTriggerInputTypes.DAILY, | |
| hour: 12, | |
| minute: 0, | |
| }, | |
| identifier: `streak_${userId}`, | |
| }); |
🤖 Prompt for AI Agents
In `@frontend/services/streak-service.ts` around lines 187 - 226, The else branch
is unreachable because newStreakData.currentStreak is always >= 1 in
updateStreak; remove the dead else block and simplify to a single notification
scheduling path: after awaiting
LocalNotificationService.cancelAllScheduledNotifications(), always call
LocalNotificationService.scheduleNotification(...) with the "active streak"
payload (title 'How are you doing today?', body including
newStreakData.currentStreak, data.page '/capture', the same trigger and
identifier `streak_${userId}`). Update the code around
updateStreak/newStreakData to delete the else branch and its
scheduleNotification call so only the active-streak notification remains.
2 participants
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.