-
Notifications
You must be signed in to change notification settings - Fork 24
Feature/google signin #139
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Redesigned the group list and group details screens with a modern and bold UI. - Introduced a new color palette and typography theme for consistency. - Updated the layout of the group list and group details screens to be more intuitive and visually appealing. - Added animations and transitions to improve the user experience. - The new design is inspired by modern aesthetics to appeal to a younger audience.
- Applied the new modern theme to all screens for a consistent look and feel. - Refactored the home screen to a grid view for active groups. - Added an expander to the home screen to show/hide settled groups. - Updated the styling of all components to match the new design system.
- Updated EditProfileScreen to use centralized avatar logic for displaying user images or initials. - Refactored FriendsScreen to utilize new avatar utility functions for friend images and initials. - Enhanced GroupDetailsScreen with improved imports and structure. - Cleaned up GroupSettingsScreen by consolidating image handling and member rendering logic. - Improved HomeScreen to leverage avatar utilities for consistent display of group images and initials. - Added avatar utility functions: getInitial for generating initials from names and isValidImageUri for validating image URIs.
… functions" This reverts commit 6d75613.
I've added the functionality for you to sign in or sign up using your Google account. - Added a 'Sign in with Google' button to the Login and Signup screens. - Implemented the Google authentication flow using Expo's `expo-auth-session`. - Added a new API endpoint for Google login in the backend. - Updated the AuthContext to handle the Google sign-in state. - Added environment variable handling for Google Client IDs.
WalkthroughAdds Expo Google sign-in (API client, AuthContext flow, Login/Signup UI), centralizes a theme, refactors many frontend screens to use the theme and new layouts/logic (notably AddExpense, Friends, Groups), adds Expo app identifiers and dependencies, and updates CI to create/ignore Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant App as App (Login/Signup)
participant AuthCtx as AuthContext (Expo Auth)
participant Google as Google OAuth
participant Backend as API (/auth/login/google)
participant Storage as AsyncStorage
User->>App: Tap "Sign in with Google"
App->>AuthCtx: loginWithGoogle()
AuthCtx->>Google: Start OAuth flow (expo-auth-session)
Google-->>AuthCtx: id_token (on success)
AuthCtx->>Backend: POST /auth/login/google { id_token }
Backend-->>AuthCtx: { accessToken, refreshToken, user }
AuthCtx->>Storage: Save tokens and user
AuthCtx-->>App: Auth state updated
App-->>User: Signed in
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #139 +/- ##
=======================================
Coverage ? 72.79%
=======================================
Files ? 17
Lines ? 1669
Branches ? 154
=======================================
Hits ? 1215
Misses ? 400
Partials ? 54
🚀 New features to boost your workflow:
|
|
🚀 Expo preview is ready!
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 14
🔭 Outside diff range comments (1)
frontend/screens/GroupDetailsScreen.js (1)
78-81: Harden getMemberName against missing fields; or reuse existing helper.Directly accessing member.user.name can throw. Prefer optional chaining or reuse the shared util (frontend/utils/balanceCalculator.js:getMemberName).
Option A (minimal hardening):
-const getMemberName = (userId) => { - const member = members.find((m) => m.userId === userId); - return member ? member.user.name : "Unknown"; -}; +const getMemberName = (userId) => { + const member = members.find((m) => m.userId === userId); + return member?.user?.name ?? "Unknown"; +};Option B (reuse util):
+import { getMemberName as getMemberNameUtil } from "../utils/balanceCalculator"; @@ -const getMemberName = (userId) => { - const member = members.find((m) => m.userId === userId); - return member ? member.user.name : "Unknown"; -}; +const getMemberName = (userId) => getMemberNameUtil(members, userId);Also applies to: 109-110
🧹 Nitpick comments (7)
frontend/.env.example (1)
1-4: Consider alphabetical ordering of environment variablesFor better maintainability, consider ordering the variables alphabetically:
-EXPO_PUBLIC_GOOGLE_EXPO_CLIENT_ID= -EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID= EXPO_PUBLIC_GOOGLE_ANDROID_CLIENT_ID= +EXPO_PUBLIC_GOOGLE_EXPO_CLIENT_ID= +EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID= EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID=frontend/screens/GroupDetailsScreen.js (2)
58-64: Add accessibility props to the header settings button.Small a11y improvement: describe the action for screen readers.
-<TouchableOpacity +<TouchableOpacity + accessibilityRole="button" + accessibilityLabel="Open group settings" onPress={() => navigation.navigate("GroupSettings", { groupId })} style={{ marginRight: spacing.md }} >
211-219: Optional: Add pull-to-refresh to FlatList.Improves UX for retrying failed fetches and refreshing.
<FlatList data={expenses} renderItem={renderExpense} keyExtractor={(item) => item._id} ListHeaderComponent={renderHeader} ListEmptyComponent={ <Text style={styles.emptyText}>No expenses recorded yet.</Text> } - contentContainerStyle={styles.contentContainer} + contentContainerStyle={styles.contentContainer} + refreshing={isLoading} + onRefresh={fetchData} />frontend/screens/SignupScreen.js (2)
96-105: Show loading on the Google button for consistent UX.The primary button shows loading; mirror on the Google button.
-<Button +<Button mode="contained" onPress={handleGoogleSignIn} style={[styles.button, styles.googleButton]} labelStyle={styles.buttonLabel} icon="google" - disabled={isLoading} + disabled={isLoading} + loading={isLoading} > Sign up with Google </Button>
53-85: Optional: Centralize TextInput theme instead of per-input prop.Repeating theme on each input adds noise; consider a Paper Provider with a theme or a local wrapper.
frontend/screens/LoginScreen.js (1)
111-113: Move Google brand color to theme constantsThe Google brand color is hardcoded. Consider moving it to the theme file for consistency.
googleButton: { - backgroundColor: '#4285F4', // Google's brand color + backgroundColor: colors.googleBrand, // Move to theme.js },Also add to
frontend/styles/theme.js:export const colors = { // ... existing colors googleBrand: '#4285F4', };frontend/screens/AddExpenseScreen.js (1)
186-199: Add validation for shares inputThe shares calculation should validate for negative values and very small numbers that could cause precision issues.
} else if (splitMethod === "shares") { + // Validate shares + const hasNegative = Object.values(shares).some( + val => parseFloat(val || "0") < 0 + ); + if (hasNegative) { + throw new Error("Shares cannot be negative."); + } + const totalShares = Object.values(shares).reduce( (sum, val) => sum + parseFloat(val || "0"), 0 ); if (totalShares === 0) { throw new Error("Total shares cannot be zero."); } + if (totalShares < 0.01) { + throw new Error("Total shares too small for accurate calculation."); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (19)
.github/workflows/preview.yml(1 hunks).gitignore(1 hunks)frontend/.env.example(1 hunks)frontend/api/auth.js(1 hunks)frontend/app.json(2 hunks)frontend/context/AuthContext.js(4 hunks)frontend/navigation/GroupsStackNavigator.js(1 hunks)frontend/package.json(1 hunks)frontend/screens/AccountScreen.js(3 hunks)frontend/screens/AddExpenseScreen.js(7 hunks)frontend/screens/EditProfileScreen.js(5 hunks)frontend/screens/FriendsScreen.js(2 hunks)frontend/screens/GroupDetailsScreen.js(6 hunks)frontend/screens/GroupSettingsScreen.js(5 hunks)frontend/screens/HomeScreen.js(7 hunks)frontend/screens/JoinGroupScreen.js(4 hunks)frontend/screens/LoginScreen.js(3 hunks)frontend/screens/SignupScreen.js(4 hunks)frontend/styles/theme.js(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (13)
frontend/api/auth.js (3)
frontend/context/AuthContext.js (1)
loginWithGoogle(191-193)frontend/api/client.js (4)
apiClient(28-31)apiClient(28-31)refreshToken(54-75)fn(11-13)backend/app/auth/routes.py (1)
login_with_google(139-168)
frontend/navigation/GroupsStackNavigator.js (4)
frontend/navigation/AccountStackNavigator.js (2)
Stack(6-6)Stack(8-15)frontend/navigation/AuthNavigator.js (1)
Stack(6-6)frontend/screens/AddExpenseScreen.js (1)
AddExpenseScreen(60-377)frontend/screens/GroupSettingsScreen.js (1)
GroupSettingsScreen(36-339)
frontend/screens/SignupScreen.js (5)
frontend/screens/LoginScreen.js (2)
isLoading(10-10)useContext(11-11)frontend/context/AuthContext.js (4)
isLoading(20-20)AuthContext(14-14)AuthContext(14-14)loginWithGoogle(191-193)frontend/navigation/AppNavigator.js (1)
useContext(9-9)frontend/api/auth.js (2)
loginWithGoogle(11-13)loginWithGoogle(11-13)frontend/navigation/AuthNavigator.js (1)
Stack(8-15)
frontend/screens/AccountScreen.js (2)
frontend/context/AuthContext.js (1)
user(17-17)frontend/navigation/AccountStackNavigator.js (1)
Stack(8-15)
frontend/screens/LoginScreen.js (2)
frontend/context/AuthContext.js (4)
isLoading(20-20)AuthContext(14-14)AuthContext(14-14)loginWithGoogle(191-193)frontend/api/auth.js (2)
loginWithGoogle(11-13)loginWithGoogle(11-13)
frontend/screens/JoinGroupScreen.js (4)
frontend/screens/LoginScreen.js (1)
styles(89-125)frontend/screens/SignupScreen.js (1)
styles(118-154)frontend/screens/GroupDetailsScreen.js (1)
styles(230-359)frontend/screens/EditProfileScreen.js (1)
styles(125-166)
frontend/context/AuthContext.js (5)
frontend/screens/LoginScreen.js (1)
handleGoogleSignIn(26-36)frontend/screens/SignupScreen.js (1)
handleGoogleSignIn(38-48)backend/app/auth/routes.py (1)
refresh_token(172-212)frontend/api/client.js (2)
setAuthTokens(15-18)setAuthTokens(15-18)frontend/api/auth.js (2)
loginWithGoogle(11-13)loginWithGoogle(11-13)
frontend/screens/EditProfileScreen.js (9)
frontend/screens/LoginScreen.js (1)
useContext(11-11)frontend/screens/SignupScreen.js (1)
useContext(13-13)frontend/screens/GroupDetailsScreen.js (1)
useContext(24-24)frontend/screens/FriendsScreen.js (1)
useContext(85-85)frontend/screens/AccountScreen.js (1)
useContext(9-9)frontend/screens/GroupSettingsScreen.js (2)
useContext(38-38)pickImage(153-167)frontend/screens/HomeScreen.js (1)
useContext(36-36)frontend/navigation/AppNavigator.js (1)
useContext(9-9)frontend/context/AuthContext.js (3)
AuthContext(14-14)AuthContext(14-14)user(17-17)
frontend/screens/AddExpenseScreen.js (3)
frontend/navigation/AppNavigator.js (1)
useContext(9-9)frontend/context/AuthContext.js (3)
AuthContext(14-14)AuthContext(14-14)isLoading(20-20)frontend/api/groups.js (2)
getGroupMembers(15-16)getGroupMembers(15-16)
frontend/screens/HomeScreen.js (5)
frontend/navigation/AppNavigator.js (2)
useContext(9-9)styles(26-32)frontend/context/AuthContext.js (3)
AuthContext(14-14)AuthContext(14-14)isLoading(20-20)frontend/api/groups.js (4)
getGroups(3-3)getGroups(3-3)getOptimizedSettlements(5-6)getOptimizedSettlements(5-6)frontend/styles/theme.js (6)
typography(13-34)typography(13-34)colors(1-11)colors(1-11)spacing(36-42)spacing(36-42)frontend/utils/currency.js (2)
formatCurrency(8-16)formatCurrency(8-16)
frontend/screens/FriendsScreen.js (3)
frontend/utils/currency.js (2)
formatCurrency(8-16)formatCurrency(8-16)frontend/context/AuthContext.js (2)
AuthContext(14-14)AuthContext(14-14)frontend/api/groups.js (4)
getFriendsBalance(29-30)getFriendsBalance(29-30)getGroups(3-3)getGroups(3-3)
frontend/screens/GroupDetailsScreen.js (5)
frontend/styles/theme.js (6)
spacing(36-42)spacing(36-42)colors(1-11)colors(1-11)typography(13-34)typography(13-34)frontend/context/AuthContext.js (2)
token(18-18)isLoading(20-20)frontend/utils/currency.js (2)
formatCurrency(8-16)formatCurrency(8-16)frontend/navigation/AppNavigator.js (1)
styles(26-32)frontend/utils/balanceCalculator.js (1)
getMemberName(67-72)
frontend/screens/GroupSettingsScreen.js (4)
frontend/screens/LoginScreen.js (1)
useContext(11-11)frontend/screens/SignupScreen.js (1)
useContext(13-13)frontend/screens/AccountScreen.js (1)
useContext(9-9)frontend/context/AuthContext.js (3)
AuthContext(14-14)AuthContext(14-14)user(17-17)
🪛 dotenv-linter (3.3.0)
frontend/.env.example
[warning] 3-3: [UnorderedKey] The EXPO_PUBLIC_GOOGLE_ANDROID_CLIENT_ID key should go before the EXPO_PUBLIC_GOOGLE_EXPO_CLIENT_ID key
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: preview
- GitHub Check: bundle-analysis
🔇 Additional comments (23)
.gitignore (1)
52-52: Good security practice!Adding
frontend/.envto gitignore prevents accidental commits of sensitive OAuth client IDs.frontend/app.json (1)
9-9: OAuth configuration looks good!The scheme and platform identifiers are correctly configured for Google Sign-In OAuth redirects.
Also applies to: 18-18, 24-24
frontend/styles/theme.js (1)
1-42: Excellent theme structure!The centralized theme tokens provide a solid foundation for consistent UI styling across the application. Good use of semantic color names and structured typography scales.
frontend/screens/JoinGroupScreen.js (1)
2-6: Clean theme integration!The screen successfully adopts the centralized theme system with consistent use of colors, spacing, and typography tokens.
Also applies to: 38-101
frontend/screens/AccountScreen.js (2)
19-41: Well-structured menu implementation!The data-driven approach with
menuItemsarray makes the menu easily maintainable and extensible. Good use of conditional styling for the logout item.Also applies to: 72-96
54-66: Appropriate avatar sizing and theming!The 100px avatar size provides good visual hierarchy on the account screen, and the theme integration is consistent.
Also applies to: 117-119
frontend/screens/HomeScreen.js (4)
28-33: Proper Android LayoutAnimation setup!Good platform-specific handling to enable layout animations on Android.
80-95: Solid settlement calculation logic!The function correctly calculates the user's net balance and settlement status with appropriate error handling.
56-71: Efficient group categorization!Good use of
Promise.allfor parallel settlement status fetching and clean separation into active/settled groups.
127-188: Well-implemented responsive grid layout!Good use of
useMemofor row calculation optimization and proper URL validation for avatar images.frontend/screens/GroupDetailsScreen.js (1)
83-99: Verify net-calculation assumptions and data types.Assumptions:
- item.paidBy || item.createdBy is a userId string compatible with user._id.
- item.amount and userShare are numbers (not strings).
If these aren’t guaranteed by the API, coerce to numbers and guard:
-const userShare = userSplit ? userSplit.amount : 0; +const userShare = Number(userSplit?.amount ?? 0); +const amount = Number(item.amount ?? 0); -const net = paidByMe ? item.amount - userShare : -userShare; +const net = paidByMe ? amount - userShare : -userShare;frontend/navigation/GroupsStackNavigator.js (1)
15-18: LGTM: Hiding headers for AddExpense and GroupSettings aligns with in-screen headers.Matches the screens’ internal Appbar headers; avoids double headers.
frontend/api/auth.js (1)
11-13: Google login endpoint hook looks correct.POST /auth/login/google with { id_token } aligns with backend route; leverages shared apiClient.
frontend/screens/SignupScreen.js (1)
38-48: Ensure AuthContext handles the Google response and token exchange.handleGoogleSignIn just calls loginWithGoogle(); confirm the AuthContext’s loginWithGoogle (and response listener) completes the id_token exchange, persists tokens, and updates navigation state.
frontend/screens/FriendsScreen.js (1)
129-131: UIManager LayoutAnimation already enabled on Android
Confirmed thatUIManager.setLayoutAnimationEnabledExperimental(true)is called infrontend/screens/HomeScreen.js, so Android support forLayoutAnimationis already configured beforeFriendsScreenis mounted. No further changes are required here.frontend/screens/EditProfileScreen.js (8)
3-7: LGTM! Good theming integration.The import changes appropriately support the centralized theming approach and remove unused imports.
10-10: LGTM! Simplified context usage.Removing the unused
tokenfrom the context destructuring simplifies the component appropriately.
24-24: LGTM! Simplified MIME type handling.The simplified fallback approach to
"image/jpeg"reduces complexity while maintaining functionality for the common use case.Also applies to: 57-58
64-74: LGTM! Excellent header theming.The header styling with primary color background and white text/icons creates good visual hierarchy and consistency.
76-76: LGTM! Good title implementation.The new Text component with centralized typography and proper styling is an improvement over the previous approach.
79-91: LGTM! Robust avatar source handling.The avatar source logic properly handles multiple scenarios with good fallbacks and URL validation using the regex pattern.
108-108: LGTM! Consistent theming for inputs and buttons.The theme prop usage and labelStyle additions maintain visual consistency across the interface.
Also applies to: 116-116
125-166: Excellent theming implementation!The stylesheet comprehensively uses centralized theme tokens (colors, spacing, typography) throughout, ensuring consistency and maintainability. The organization and naming conventions follow best practices.
| - name: Create .env file | ||
| working-directory: ./frontend | ||
| run: | | ||
| echo "EXPO_PUBLIC_GOOGLE_EXPO_CLIENT_ID=${{ secrets.EXPO_PUBLIC_GOOGLE_EXPO_CLIENT_ID }}" >> .env | ||
| echo "EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID=${{ secrets.EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID }}" >> .env | ||
| echo "EXPO_PUBLIC_GOOGLE_ANDROID_CLIENT_ID=${{ secrets.EXPO_PUBLIC_GOOGLE_ANDROID_CLIENT_ID }}" >> .env | ||
| echo "EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID=${{ secrets.EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID }}" >> .env |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Prevent duplicate entries in .env file
The current implementation uses >> which appends to the file. If the file already exists (from a previous run or cache), this could create duplicate entries. Consider clearing the file first:
- name: Create .env file
working-directory: ./frontend
run: |
+ > .env # Clear the file first
echo "EXPO_PUBLIC_GOOGLE_EXPO_CLIENT_ID=${{ secrets.EXPO_PUBLIC_GOOGLE_EXPO_CLIENT_ID }}" >> .env
echo "EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID=${{ secrets.EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID }}" >> .env
echo "EXPO_PUBLIC_GOOGLE_ANDROID_CLIENT_ID=${{ secrets.EXPO_PUBLIC_GOOGLE_ANDROID_CLIENT_ID }}" >> .env
echo "EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID=${{ secrets.EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID }}" >> .env📝 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.
| - name: Create .env file | |
| working-directory: ./frontend | |
| run: | | |
| echo "EXPO_PUBLIC_GOOGLE_EXPO_CLIENT_ID=${{ secrets.EXPO_PUBLIC_GOOGLE_EXPO_CLIENT_ID }}" >> .env | |
| echo "EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID=${{ secrets.EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID }}" >> .env | |
| echo "EXPO_PUBLIC_GOOGLE_ANDROID_CLIENT_ID=${{ secrets.EXPO_PUBLIC_GOOGLE_ANDROID_CLIENT_ID }}" >> .env | |
| echo "EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID=${{ secrets.EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID }}" >> .env | |
| - name: Create .env file | |
| working-directory: ./frontend | |
| run: | | |
| > .env # Clear the file first | |
| echo "EXPO_PUBLIC_GOOGLE_EXPO_CLIENT_ID=${{ secrets.EXPO_PUBLIC_GOOGLE_EXPO_CLIENT_ID }}" >> .env | |
| echo "EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID=${{ secrets.EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID }}" >> .env | |
| echo "EXPO_PUBLIC_GOOGLE_ANDROID_CLIENT_ID=${{ secrets.EXPO_PUBLIC_GOOGLE_ANDROID_CLIENT_ID }}" >> .env | |
| echo "EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID=${{ secrets.EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID }}" >> .env |
🤖 Prompt for AI Agents
.github/workflows/preview.yml around lines 32 to 38: the workflow currently
appends environment variables to ./frontend/.env using >> which can create
duplicate entries if the file already exists; instead ensure the file is either
truncated or rebuilt before writing (for example, write the first variable with
> to overwrite/truncate then append the rest with >>, or explicitly
remove/truncate the .env file first, or use a here-doc to atomically create the
file), so update the run step to clear or overwrite .env before adding the
variables.
| const [request, response, promptAsync] = useAuthRequest({ | ||
| expoClientId: process.env.EXPO_PUBLIC_GOOGLE_EXPO_CLIENT_ID, | ||
| iosClientId: process.env.EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID, | ||
| androidClientId: process.env.EXPO_PUBLIC_GOOGLE_ANDROID_CLIENT_ID, | ||
| webClientId: process.env.EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID, | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Validate environment variables before use
The Google client IDs are being used directly from environment variables without validation. Consider adding checks to ensure these values exist and provide meaningful error messages during development.
Add validation before the useAuthRequest call:
+const validateGoogleConfig = () => {
+ const requiredVars = [
+ 'EXPO_PUBLIC_GOOGLE_EXPO_CLIENT_ID',
+ 'EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID',
+ 'EXPO_PUBLIC_GOOGLE_ANDROID_CLIENT_ID',
+ 'EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID'
+ ];
+
+ const missing = requiredVars.filter(key => !process.env[key]);
+ if (missing.length > 0) {
+ console.warn(`Missing Google OAuth configuration: ${missing.join(', ')}`);
+ }
+};
+
+validateGoogleConfig();
+
const [request, response, promptAsync] = useAuthRequest({
expoClientId: process.env.EXPO_PUBLIC_GOOGLE_EXPO_CLIENT_ID,
iosClientId: process.env.EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID,
androidClientId: process.env.EXPO_PUBLIC_GOOGLE_ANDROID_CLIENT_ID,
webClientId: process.env.EXPO_PUBLIC_GOOGLE_WEB_CLIENT_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 [request, response, promptAsync] = useAuthRequest({ | |
| expoClientId: process.env.EXPO_PUBLIC_GOOGLE_EXPO_CLIENT_ID, | |
| iosClientId: process.env.EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID, | |
| androidClientId: process.env.EXPO_PUBLIC_GOOGLE_ANDROID_CLIENT_ID, | |
| webClientId: process.env.EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID, | |
| }); | |
| // Validate environment variables before use | |
| const validateGoogleConfig = () => { | |
| const requiredVars = [ | |
| 'EXPO_PUBLIC_GOOGLE_EXPO_CLIENT_ID', | |
| 'EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID', | |
| 'EXPO_PUBLIC_GOOGLE_ANDROID_CLIENT_ID', | |
| 'EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID' | |
| ]; | |
| const missing = requiredVars.filter(key => !process.env[key]); | |
| if (missing.length > 0) { | |
| console.warn(`Missing Google OAuth configuration: ${missing.join(', ')}`); | |
| } | |
| }; | |
| validateGoogleConfig(); | |
| const [request, response, promptAsync] = useAuthRequest({ | |
| expoClientId: process.env.EXPO_PUBLIC_GOOGLE_EXPO_CLIENT_ID, | |
| iosClientId: process.env.EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID, | |
| androidClientId: process.env.EXPO_PUBLIC_GOOGLE_ANDROID_CLIENT_ID, | |
| webClientId: process.env.EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID, | |
| }); |
🤖 Prompt for AI Agents
In frontend/context/AuthContext.js around lines 22 to 27, the Google client IDs
are passed directly from process.env into useAuthRequest without validation; add
a pre-check that verifies each required EXPO_PUBLIC_GOOGLE_* variable is defined
(and non-empty) before calling useAuthRequest, and if any are missing log a
clear error or throw a descriptive error in development (or provide a safe
fallback) so the developer sees which specific env var is absent; then pass the
validated values into useAuthRequest.
| const loginWithGoogle = async () => { | ||
| await promptAsync(); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add error handling to loginWithGoogle
The function should handle potential errors from promptAsync and provide feedback.
const loginWithGoogle = async () => {
- await promptAsync();
+ try {
+ const result = await promptAsync();
+ return result;
+ } catch (error) {
+ console.error("Failed to initiate Google sign-in:", error);
+ throw error;
+ }
};📝 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 loginWithGoogle = async () => { | |
| await promptAsync(); | |
| }; | |
| const loginWithGoogle = async () => { | |
| try { | |
| const result = await promptAsync(); | |
| return result; | |
| } catch (error) { | |
| console.error("Failed to initiate Google sign-in:", error); | |
| throw error; | |
| } | |
| }; |
🤖 Prompt for AI Agents
In frontend/context/AuthContext.js around lines 191 to 193, the loginWithGoogle
function currently awaits promptAsync without handling failures; wrap the
promptAsync call in a try/catch, handle errors by logging/reporting them (e.g.,
process/logging/toast) and optionally update relevant auth/error state or return
a failure indicator, and ensure any resources or loading flags are cleared in a
finally block so the UI gets proper feedback on success or failure.
| "expo-auth-session": "^6.2.1", | ||
| "expo-crypto": "^14.1.5", | ||
| "expo-image-picker": "~16.0.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify SDK compatibility and avoid caret ranges for Expo-managed packages.
expo-auth-session and expo-crypto should match Expo SDK 53’s compatible versions. Using ^ can drift to incompatible minor versions; prefer versions installed via “npx expo install …”.
- Run “npx expo install expo-auth-session expo-crypto” to pin compatible versions.
- If you keep manual versions, confirm compatibility:
🌐 Web query:
What versions of expo-auth-session and expo-crypto are compatible with Expo SDK 53?
Error during web search
🏁 Script executed:
#!/bin/bash
# Retrieve the Expo SDK version from app.json (if present)
rg '"sdkVersion"' -n
# Retrieve the Expo package version from frontend/package.json
rg '"expo"' -n frontend/package.json | sed -n '1,20p'Length of output: 103
Pin Expo-managed package versions with expo install
To ensure compatibility with SDK 53 (as specified by "expo": "~53.0.20" in frontend/package.json), replace the caret (^) ranges for expo-auth-session and expo-crypto with the exact versions that npx expo install selects. This prevents drifting into unsupported minor or patch releases.
• File: frontend/package.json (lines 19–21)
• Run:
npx expo install expo-auth-session expo-crypto• Commit the updated dependency entries (no leading ^).
🤖 Prompt for AI Agents
In frontend/package.json around lines 19 to 21, the expo-auth-session and
expo-crypto dependencies are specified with caret ranges which can drift outside
SDK 53 compatibility; run `npx expo install expo-auth-session expo-crypto` to
let Expo pick the exact compatible versions, replace the current entries to the
exact versions returned by that command (remove the leading ^), save the file
and commit the updated package.json (and package-lock or yarn.lock if
applicable).
| const toggleExpansion = () => { | ||
| LayoutAnimation.configureNext(LayoutAnimation.Presets.easeInEaseOut); | ||
| setSettlementExpanded(!settlementExpanded); | ||
| }; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Enable LayoutAnimation on Android to ensure expansion animates.
On Android, LayoutAnimation requires explicit enabling.
Add this near the top-level (after imports):
import { UIManager, Platform } from "react-native"; // UIManager already available from react-native
if (Platform.OS === "android" && UIManager.setLayoutAnimationEnabledExperimental) {
UIManager.setLayoutAnimationEnabledExperimental(true);
}Verify expansion animates on Android; without this, the toggle may be non-animated.
🏁 Script executed:
#!/bin/bash
# Search for explicit enabling of LayoutAnimation on Android
rg "setLayoutAnimationEnabledExperimental" -n
# Check where UIManager is imported or referenced
rg "UIManager" -n
# Search for Platform.OS checks in the repo
rg "Platform\.OS" -nLength of output: 651
Enable Android LayoutAnimation for GroupDetailsScreen
LayoutAnimation on Android must be enabled explicitly; otherwise your toggleExpansion call will have no visible effect on Android devices. We already do this in HomeScreen.js, but you’ll need to enable it globally (recommended) or at least in GroupDetailsScreen.js.
– Recommended location: App.js (once at app startup)
– Alternative per-screen: frontend/screens/GroupDetailsScreen.js (after imports)
Suggested diff for GroupDetailsScreen.js:
+ import { UIManager, Platform } from 'react-native';
// …other imports
+ // Enable LayoutAnimation on Android
+ if (Platform.OS === 'android' && UIManager.setLayoutAnimationEnabledExperimental) {
+ UIManager.setLayoutAnimationEnabledExperimental(true);
+ }
const toggleExpansion = () => {
LayoutAnimation.configureNext(LayoutAnimation.Presets.easeInEaseOut);
setSettlementExpanded(!settlementExpanded);
};🤖 Prompt for AI Agents
In frontend/screens/GroupDetailsScreen.js around lines 128 to 132,
LayoutAnimation is used in toggleExpansion but on Android it must be explicitly
enabled; add the Android enable call after the module imports (or enable once in
App.js at startup). Specifically, import UIManager from 'react-native' (if not
already) and call UIManager.setLayoutAnimationEnabledExperimental &&
UIManager.setLayoutAnimationEnabledExperimental(true) near the top of the file
(immediately after imports) so LayoutAnimation calls take effect on Android;
alternatively, place that same single-line enable call in App.js to enable
globally.
| {userOwes.map((s, index) => ( | ||
| <View key={`owes-${index}`} style={styles.settlementItem}> | ||
| <View style={styles.personInfo}> | ||
| <Text style={styles.personName}> | ||
| {getMemberName(s.toUserId)} | ||
| </Text> | ||
| <Text style={styles.settlementAmount}> | ||
| {formatCurrency(s.amount)} | ||
| </Text> | ||
| </View> | ||
| <Text style={styles.settlementText}> | ||
| Pay {getMemberName(s.toUserId)} | ||
| </Text> | ||
| <Text style={styles.settlementAmount}> | ||
| {formatCurrency(s.amount)} | ||
| </Text> | ||
| </View> | ||
| ))} | ||
| </View> | ||
| )} | ||
|
|
||
| {/* You receive section - only show if totalToReceive > 0 */} | ||
| {totalToReceive > 0 && ( | ||
| <View style={styles.receiveSection}> | ||
| <Text style={styles.sectionTitle}> | ||
| You will receive:{" "} | ||
| <Text style={styles.amountReceive}> | ||
| {formatCurrency(totalToReceive)} | ||
| </Text> | ||
| </Text> | ||
| {userIsOwed.map((s, index) => ( | ||
| <View key={`is-owed-${index}`} style={styles.settlementItem}> | ||
| <View style={styles.personInfo}> | ||
| <Text style={styles.personName}> | ||
| {getMemberName(s.fromUserId)} | ||
| </Text> | ||
| <Text style={styles.settlementAmount}> | ||
| {formatCurrency(s.amount)} | ||
| </Text> | ||
| </View> | ||
| <Text style={styles.settlementText}> | ||
| Receive from {getMemberName(s.fromUserId)} | ||
| </Text> | ||
| <Text style={styles.settlementAmount}> | ||
| {formatCurrency(s.amount)} | ||
| </Text> | ||
| </View> | ||
| ))} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Use stable keys for settlement items (avoid index).
Index keys can cause incorrect reconciliation during toggling/updates.
-<View key={`owes-${index}`} style={styles.settlementItem}>
+<View key={`owes-${s.fromUserId}-${s.toUserId}`} style={styles.settlementItem}>
@@
-<View key={`is-owed-${index}`} style={styles.settlementItem}>
+<View key={`is-owed-${s.fromUserId}-${s.toUserId}`} style={styles.settlementItem}>📝 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.
| {userOwes.map((s, index) => ( | |
| <View key={`owes-${index}`} style={styles.settlementItem}> | |
| <View style={styles.personInfo}> | |
| <Text style={styles.personName}> | |
| {getMemberName(s.toUserId)} | |
| </Text> | |
| <Text style={styles.settlementAmount}> | |
| {formatCurrency(s.amount)} | |
| </Text> | |
| </View> | |
| <Text style={styles.settlementText}> | |
| Pay {getMemberName(s.toUserId)} | |
| </Text> | |
| <Text style={styles.settlementAmount}> | |
| {formatCurrency(s.amount)} | |
| </Text> | |
| </View> | |
| ))} | |
| </View> | |
| )} | |
| {/* You receive section - only show if totalToReceive > 0 */} | |
| {totalToReceive > 0 && ( | |
| <View style={styles.receiveSection}> | |
| <Text style={styles.sectionTitle}> | |
| You will receive:{" "} | |
| <Text style={styles.amountReceive}> | |
| {formatCurrency(totalToReceive)} | |
| </Text> | |
| </Text> | |
| {userIsOwed.map((s, index) => ( | |
| <View key={`is-owed-${index}`} style={styles.settlementItem}> | |
| <View style={styles.personInfo}> | |
| <Text style={styles.personName}> | |
| {getMemberName(s.fromUserId)} | |
| </Text> | |
| <Text style={styles.settlementAmount}> | |
| {formatCurrency(s.amount)} | |
| </Text> | |
| </View> | |
| <Text style={styles.settlementText}> | |
| Receive from {getMemberName(s.fromUserId)} | |
| </Text> | |
| <Text style={styles.settlementAmount}> | |
| {formatCurrency(s.amount)} | |
| </Text> | |
| </View> | |
| ))} | |
| {userOwes.map((s, index) => ( | |
| <View key={`owes-${s.fromUserId}-${s.toUserId}`} style={styles.settlementItem}> | |
| <Text style={styles.settlementText}> | |
| Pay {getMemberName(s.toUserId)} | |
| </Text> | |
| <Text style={styles.settlementAmount}> | |
| {formatCurrency(s.amount)} | |
| </Text> | |
| </View> | |
| ))} | |
| {userIsOwed.map((s, index) => ( | |
| <View key={`is-owed-${s.fromUserId}-${s.toUserId}`} style={styles.settlementItem}> | |
| <Text style={styles.settlementText}> | |
| Receive from {getMemberName(s.fromUserId)} | |
| </Text> | |
| <Text style={styles.settlementAmount}> | |
| {formatCurrency(s.amount)} | |
| </Text> | |
| </View> | |
| ))} |
🤖 Prompt for AI Agents
In frontend/screens/GroupDetailsScreen.js around lines 167 to 186, the
settlement list items use the array index as React keys which can break
reconciliation; replace the index keys with a stable unique identifier from each
settlement object (preferably s.id). If the settlement objects do not include an
id, construct a deterministic composite key (e.g.,
`${s.fromUserId}-${s.toUserId}-${s.amount}-${s.createdAt}`) that uniquely
identifies the record instead of using the index.
| const onKick = (memberId, memberName) => { | ||
| Alert.alert( | ||
| "Kick Member", | ||
| `Are you sure you want to kick ${memberName} from the group?`, | ||
| [ | ||
| { text: "Cancel", style: "cancel" }, | ||
| { | ||
| text: "Kick", | ||
| style: "destructive", | ||
| onPress: async () => { | ||
| try { | ||
| await apiRemoveMember(groupId, memberId); | ||
| setMembers(members.filter((m) => m.userId !== memberId)); | ||
| Alert.alert("Success", `${memberName} has been kicked.`); | ||
| } catch (error) { | ||
| Alert.alert("Error", "Failed to kick member."); | ||
| } | ||
| await apiRemoveMember(groupId, memberId); | ||
| await load(); | ||
| } catch (e) { | ||
| console.error("Remove failed", e); | ||
| Alert.alert( | ||
| "Error", | ||
| e.response?.data?.detail || "Failed to remove member" | ||
| ); | ||
| } | ||
| }, | ||
| }, | ||
| }, | ||
| ]); | ||
| ] | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider checking for unsettled balances before kicking members
The kick functionality no longer checks for unsettled balances. This could lead to data inconsistency if a member with outstanding balances is removed.
Consider adding a balance check before allowing the kick action:
const onKick = (memberId, memberName) => {
Alert.alert(
"Kick Member",
- `Are you sure you want to kick ${memberName} from the group?`,
+ `Are you sure you want to kick ${memberName} from the group? Make sure they have no unsettled balances.`,
[
{ text: "Cancel", style: "cancel" },
{
text: "Kick",
style: "destructive",
onPress: async () => {
try {
+ // Optional: Check member's balance before removal
+ // const balance = await checkMemberBalance(groupId, memberId);
+ // if (balance !== 0) {
+ // Alert.alert("Warning", "This member has unsettled balances.");
+ // return;
+ // }
await apiRemoveMember(groupId, memberId);
setMembers(members.filter((m) => m.userId !== memberId));
Alert.alert("Success", `${memberName} has been kicked.`);
} catch (error) {
Alert.alert("Error", "Failed to kick member.");
}
},
},
]
);
};📝 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 onKick = (memberId, memberName) => { | |
| Alert.alert( | |
| "Kick Member", | |
| `Are you sure you want to kick ${memberName} from the group?`, | |
| [ | |
| { text: "Cancel", style: "cancel" }, | |
| { | |
| text: "Kick", | |
| style: "destructive", | |
| onPress: async () => { | |
| try { | |
| await apiRemoveMember(groupId, memberId); | |
| setMembers(members.filter((m) => m.userId !== memberId)); | |
| Alert.alert("Success", `${memberName} has been kicked.`); | |
| } catch (error) { | |
| Alert.alert("Error", "Failed to kick member."); | |
| } | |
| await apiRemoveMember(groupId, memberId); | |
| await load(); | |
| } catch (e) { | |
| console.error("Remove failed", e); | |
| Alert.alert( | |
| "Error", | |
| e.response?.data?.detail || "Failed to remove member" | |
| ); | |
| } | |
| }, | |
| }, | |
| }, | |
| ]); | |
| ] | |
| ); | |
| }; | |
| const onKick = (memberId, memberName) => { | |
| Alert.alert( | |
| "Kick Member", | |
| `Are you sure you want to kick ${memberName} from the group? Make sure they have no unsettled balances.`, | |
| [ | |
| { text: "Cancel", style: "cancel" }, | |
| { | |
| text: "Kick", | |
| style: "destructive", | |
| onPress: async () => { | |
| try { | |
| // Optional: Check member's balance before removal | |
| // const balance = await checkMemberBalance(groupId, memberId); | |
| // if (balance !== 0) { | |
| // Alert.alert("Warning", "This member has unsettled balances."); | |
| // return; | |
| // } | |
| await apiRemoveMember(groupId, memberId); | |
| setMembers(members.filter((m) => m.userId !== memberId)); | |
| Alert.alert("Success", `${memberName} has been kicked.`); | |
| } catch (error) { | |
| Alert.alert("Error", "Failed to kick member."); | |
| } | |
| }, | |
| }, | |
| ] | |
| ); | |
| }; |
🤖 Prompt for AI Agents
In frontend/screens/GroupSettingsScreen.js around lines 98 to 119, the onKick
flow currently removes a member without verifying unsettled balances; before
calling apiRemoveMember, fetch or compute the member's outstanding balance (e.g.
call an API like apiGetMemberBalance(groupId, memberId) or derive from current
state) and if the balance is non-zero show an Alert blocking the kick and do not
call apiRemoveMember; if the balance is zero proceed with the existing
confirmation and removal, and ideally surface the balance in the confirmation
prompt or disable the Kick action in the UI to prevent accidental removals; if
possible also rely on the backend to validate and reject removals with unsettled
balances and handle that error gracefully in the catch block.
| const pickImage = async () => { | ||
| if (!isAdmin) return; | ||
| // Only allow delete if no other members present | ||
| const others = members.filter((m) => m.userId !== user?._id); | ||
| if (others.length > 0) { | ||
| Alert.alert( | ||
| "Cannot delete", | ||
| "Remove all members first, or transfer admin." | ||
| ); | ||
| return; | ||
| let result = await ImagePicker.launchImageLibraryAsync({ | ||
| mediaTypes: ImagePicker.MediaTypeOptions.Images, | ||
| allowsEditing: true, | ||
| aspect: [1, 1], | ||
| quality: 1, | ||
| base64: true, | ||
| }); | ||
|
|
||
| if (!result.canceled) { | ||
| setPickedImage(result.assets[0]); | ||
| setIcon(""); // Clear emoji icon when image is picked | ||
| } | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Optimize image quality to reduce memory usage
Using quality: 1 with base64: true can create very large base64 strings, potentially causing memory issues and slow API calls.
const pickImage = async () => {
if (!isAdmin) return;
let result = await ImagePicker.launchImageLibraryAsync({
mediaTypes: ImagePicker.MediaTypeOptions.Images,
allowsEditing: true,
aspect: [1, 1],
- quality: 1,
+ quality: 0.7, // Reduce quality to optimize file size
base64: true,
+ // Consider adding max dimensions
+ // maxWidth: 512,
+ // maxHeight: 512,
});
if (!result.canceled) {
setPickedImage(result.assets[0]);
setIcon(""); // Clear emoji icon when image is picked
}
};📝 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 pickImage = async () => { | |
| if (!isAdmin) return; | |
| // Only allow delete if no other members present | |
| const others = members.filter((m) => m.userId !== user?._id); | |
| if (others.length > 0) { | |
| Alert.alert( | |
| "Cannot delete", | |
| "Remove all members first, or transfer admin." | |
| ); | |
| return; | |
| let result = await ImagePicker.launchImageLibraryAsync({ | |
| mediaTypes: ImagePicker.MediaTypeOptions.Images, | |
| allowsEditing: true, | |
| aspect: [1, 1], | |
| quality: 1, | |
| base64: true, | |
| }); | |
| if (!result.canceled) { | |
| setPickedImage(result.assets[0]); | |
| setIcon(""); // Clear emoji icon when image is picked | |
| } | |
| }; | |
| const pickImage = async () => { | |
| if (!isAdmin) return; | |
| let result = await ImagePicker.launchImageLibraryAsync({ | |
| mediaTypes: ImagePicker.MediaTypeOptions.Images, | |
| allowsEditing: true, | |
| aspect: [1, 1], | |
| quality: 0.7, // Reduce quality to optimize file size | |
| base64: true, | |
| // Consider adding max dimensions | |
| // maxWidth: 512, | |
| // maxHeight: 512, | |
| }); | |
| if (!result.canceled) { | |
| setPickedImage(result.assets[0]); | |
| setIcon(""); // Clear emoji icon when image is picked | |
| } | |
| }; |
🤖 Prompt for AI Agents
In frontend/screens/GroupSettingsScreen.js around lines 153 to 167, the image
picker currently uses quality: 1 with base64: true which can produce huge base64
payloads; change the picker to reduce memory by lowering quality (e.g., quality:
0.7 or 0.8) and avoid base64 (set base64: false) and instead use the returned
URI or run a resize/compress step (ImageManipulator) before uploading; ensure
setPickedImage still receives the asset URI and preserve clearing the emoji
icon.
| const handleGoogleSignIn = async () => { | ||
| setIsLoading(true); | ||
| try { | ||
| await loginWithGoogle(); | ||
| } catch (error) { | ||
| console.error('Google Sign-In error:', error); | ||
| Alert.alert('Google Sign-In Failed', 'An error occurred during Google Sign-In. Please try again.'); | ||
| } finally { | ||
| setIsLoading(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Handle loginWithGoogle properly based on AuthContext implementation
The error handling here assumes loginWithGoogle throws errors, but based on the AuthContext implementation, it doesn't properly propagate errors from the authentication flow.
🏁 Script executed:
#!/bin/bash
# Check how loginWithGoogle is implemented in AuthContext
rg -A 10 "const loginWithGoogle" frontend/context/AuthContext.jsLength of output: 397
🏁 Script executed:
#!/bin/bash
# Inspect AuthContext to see how promptAsync and its result are handled
rg -n "useAuthRequest" -A 5 frontend/context/AuthContext.jsLength of output: 627
🏁 Script executed:
#!/bin/bash
# Display the AuthContext implementation to inspect response handling
sed -n '1,200p' frontend/context/AuthContext.jsLength of output: 6141
Propagate and Handle Google Sign-In Errors Correctly
The current loginWithGoogle in AuthContext simply calls promptAsync() and never rejects or returns a result, so the catch in your screen’s handleGoogleSignIn will never fire. You have two options:
- Make AuthContext throw on failure
Infrontend/context/AuthContext.js, haveloginWithGooglereturn thepromptAsync()result and throw ifresponse.type !== 'success':const loginWithGoogle = async () => { - await promptAsync();
- const result = await promptAsync();
- if (result.type !== 'success') {
-
throw new Error('Google Sign-In cancelled or failed'); - }
- return result;
};
- **Handle the response in the screen**
Return the `promptAsync()` result as above, then in `frontend/screens/LoginScreen.js` unpack it:
```diff
const handleGoogleSignIn = async () => {
setIsLoading(true);
try {
- await loginWithGoogle();
+ const { type } = await loginWithGoogle();
+ if (type !== 'success') throw new Error('User cancelled Google Sign-In');
} catch (error) {
console.error('Google Sign-In error:', error);
Alert.alert(
'Google Sign-In Failed',
error.message || 'An error occurred. Please try again.'
);
} finally {
setIsLoading(false);
}
};
This ensures any cancellation or failure in the Google auth flow surfaces to your UI.
📝 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 handleGoogleSignIn = async () => { | |
| setIsLoading(true); | |
| try { | |
| await loginWithGoogle(); | |
| } catch (error) { | |
| console.error('Google Sign-In error:', error); | |
| Alert.alert('Google Sign-In Failed', 'An error occurred during Google Sign-In. Please try again.'); | |
| } finally { | |
| setIsLoading(false); | |
| } | |
| }; | |
| const handleGoogleSignIn = async () => { | |
| setIsLoading(true); | |
| try { | |
| const { type } = await loginWithGoogle(); | |
| if (type !== 'success') throw new Error('User cancelled Google Sign-In'); | |
| } catch (error) { | |
| console.error('Google Sign-In error:', error); | |
| Alert.alert( | |
| 'Google Sign-In Failed', | |
| error.message || 'An error occurred. Please try again.' | |
| ); | |
| } finally { | |
| setIsLoading(false); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In frontend/screens/LoginScreen.js around lines 26 to 36, the current
handleGoogleSignIn assumes loginWithGoogle will throw on failure but AuthContext
only calls promptAsync() and never returns or throws; update behavior so
failures/cancellations surface: either change frontend/context/AuthContext.js to
return the promptAsync() result and explicitly throw when response.type !==
'success', or leave AuthContext returning the result and modify this screen to
await the result, check response.type (handle 'success' vs 'cancel'/'error'),
and only show success UI on 'success' while showing an Alert or throwing for
other types; ensure setIsLoading(false) still runs in finally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (3)
frontend/context/AuthContext.js (3)
22-30: Environment variables should be validated before use.The Google client IDs from environment variables should be validated to ensure they exist and provide meaningful error messages during development. This prevents silent failures when configuration is missing.
42-71: Handle all response types and improve error feedback.The Google sign-in handler should handle 'cancel' and 'dismiss' response types and provide proper error feedback to the UI instead of only logging to console.
204-206: Add error handling to loginWithGoogle.The function should handle potential errors from
promptAsyncand provide appropriate feedback.
🧹 Nitpick comments (2)
frontend/context/AuthContext.js (2)
32-40: Make debug logging conditional on environment.Debug logging should be disabled in production to avoid exposing sensitive authentication details in logs.
useEffect(() => { - if (request) { + if (request && __DEV__) { console.log("Auth request details:", { url: request.url, params: request.params }); } }, [request]);
116-145: Consider using secure storage for sensitive tokens.While AsyncStorage is acceptable for mobile apps, consider using a more secure storage solution like
expo-secure-storefor storing authentication tokens, especially the refresh token.#!/bin/bash # Check if expo-secure-store is already installed if grep -q "expo-secure-store" package.json; then echo "expo-secure-store is already installed" else echo "Consider installing expo-secure-store for more secure token storage:" echo "npm install expo-secure-store" fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/context/AuthContext.js(5 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
frontend/context/AuthContext.js (5)
frontend/screens/LoginScreen.js (1)
handleGoogleSignIn(26-36)frontend/screens/SignupScreen.js (1)
handleGoogleSignIn(38-48)backend/app/auth/routes.py (1)
refresh_token(172-212)frontend/api/client.js (2)
setAuthTokens(15-18)setAuthTokens(15-18)frontend/api/auth.js (2)
loginWithGoogle(11-13)loginWithGoogle(11-13)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: bundle-analysis
- GitHub Check: preview
🔇 Additional comments (2)
frontend/context/AuthContext.js (2)
2-3: Imports and web browser setup look good.The Google auth imports and WebBrowser session completion are correctly configured for the OAuth flow.
Also applies to: 12-13
25-28: Clarify platform-specific client ID usage.The code forces all platforms to use the web client ID, but the comment suggests this is only for Expo Go. If this is intentional for all environments, update the comment. Otherwise, conditionally apply this based on the environment.
Consider using platform-specific client IDs in production:
+const isExpoGo = Constants.appOwnership === 'expo'; const [request, response, promptAsync] = useAuthRequest({ - expoClientId: process.env.EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID, - iosClientId: process.env.EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID, // Force web client for iOS in Expo Go - androidClientId: process.env.EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID, // Force web client for Android in Expo Go + expoClientId: process.env.EXPO_PUBLIC_GOOGLE_EXPO_CLIENT_ID, + iosClientId: isExpoGo + ? process.env.EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID + : process.env.EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID, + androidClientId: isExpoGo + ? process.env.EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID + : process.env.EXPO_PUBLIC_GOOGLE_ANDROID_CLIENT_ID, webClientId: process.env.EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID,Also import Constants:
import Constants from 'expo-constants';
| iosClientId: process.env.EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID, // Force web client for iOS in Expo Go | ||
| androidClientId: process.env.EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID, // Force web client for Android in Expo Go | ||
| webClientId: process.env.EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID, | ||
| redirectUri: 'https://auth.expo.io/@devasy23/frontend', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid hardcoding the redirect URI.
The redirect URI is hardcoded as 'https://auth.expo.io/@devasy23/frontend' which will break in different environments or for different developers. Consider using an environment variable or dynamic configuration.
- redirectUri: 'https://auth.expo.io/@devasy23/frontend',
+ redirectUri: process.env.EXPO_PUBLIC_REDIRECT_URI || makeRedirectUri({
+ useProxy: true,
+ }),Also import makeRedirectUri from expo-auth-session:
import { makeRedirectUri } from 'expo-auth-session';🤖 Prompt for AI Agents
In frontend/context/AuthContext.js around line 29, the redirectUri is hardcoded
which fails across environments and developers; update the file to import
makeRedirectUri from 'expo-auth-session' and replace the hardcoded URI with a
dynamically generated value (e.g., use makeRedirectUri with appropriate options
or read from an environment/config variable like process.env.EXPO_REDIRECT_URI
or an app config entry), and ensure any environment variable is documented and
used as fallback if makeRedirectUri cannot be used.
…ature/google-signin
Summary by CodeRabbit
New Features
New Features
UI/UX
Documentation
Chores