Fix/multiple UI issues #131
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.
WalkthroughUpdated many frontend screens to replace react-native-paper UIs with theme-driven components and Ionicons, add a centralized theme and SkeletonLoader (reanimated), refactor split/settlement logic and animations, hide headers for AddExpense and GroupSettings, add a backend debug expense endpoint, and modify CI checkout/preview behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant Theme
participant API
participant Skeleton
User->>App: Navigate to screen (Home / Group / Friends / AddExpense / Settings)
App->>Theme: Load centralized theme (colors, spacing, typography)
App->>Skeleton: Show skeleton loading state
App->>API: Fetch required data (members, expenses, settlements, friends/groups)
API-->>App: Return data
App->>Skeleton: Hide skeleton
App->>App: Run fade-in animation & render themed UI (icons, custom rows, expand/collapse)
User->>App: Interact (toggle, add expense, share, kick, navigate)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
✨ 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/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (15.38%) is below the target coverage (70.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #131 +/- ##
==========================================
- Coverage 72.79% 72.35% -0.45%
==========================================
Files 17 17
Lines 1669 1682 +13
Branches 154 155 +1
==========================================
+ Hits 1215 1217 +2
- Misses 400 411 +11
Partials 54 54
🚀 New features to boost your workflow:
|
|
🚀 Expo preview is ready!
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🔭 Outside diff range comments (2)
frontend/screens/JoinGroupScreen.js (1)
12-13: Guard optional route param to prevent crashIf this screen is navigated without
onGroupJoined, calling it will throw. Use optional chaining.- const { onGroupJoined } = route.params; + const onGroupJoined = route?.params?.onGroupJoined; @@ - Alert.alert("Success", "Successfully joined the group."); - onGroupJoined(); + Alert.alert("Success", "Successfully joined the group."); + onGroupJoined?.();Also applies to: 23-24
frontend/screens/LoginScreen.js (1)
18-23: Add error handling aroundlogin()to guarantee spinner reset.If
login()throws (e.g. network error),setIsLoading(false)is never reached, leaving the screen stuck in a loading state.- setIsLoading(true); - const success = await login(email, password); - setIsLoading(false); - if (!success) { - Alert.alert('Login Failed', 'Invalid email or password. Please try again.'); - } + try { + setIsLoading(true); + const success = await login(email, password); + if (!success) { + Alert.alert( + 'Login Failed', + 'Invalid email or password. Please try again.', + ); + } + } catch (err) { + Alert.alert('Error', 'Unexpected error while logging in.'); + } finally { + setIsLoading(false); + }
🧹 Nitpick comments (11)
frontend/styles/theme.js (2)
1-42: Good centralization; add Paper theme export to avoid per-input themingYou’re passing
themeto individual TextInputs/Buttons across screens. Prefer wiring a single React Native Paper Provider theme once to propagate colors/typography globally.Add a Paper theme export and consume it via
<Provider theme={paperTheme}>at the app root:+import { MD3LightTheme as DefaultTheme } from 'react-native-paper'; + export const colors = { primary: "#1E3A8A", secondary: "#F3F4F6", accent: "#3B82F6", text: "#111827", textSecondary: "#6B7280", success: "#10B981", error: "#EF4444", white: "#FFFFFF", black: "#000000", }; export const typography = { h1: { fontSize: 32, fontWeight: "bold", }, h2: { fontSize: 24, fontWeight: "bold", }, h3: { fontSize: 20, fontWeight: "bold", }, body: { fontSize: 16, fontWeight: "normal", }, caption: { fontSize: 12, fontWeight: "normal", }, }; export const spacing = { xs: 4, sm: 8, md: 16, lg: 24, xl: 32, }; + +// Optional: export a Paper theme to apply app-wide +export const paperTheme = { + ...DefaultTheme, + colors: { + ...DefaultTheme.colors, + primary: colors.primary, + secondary: colors.secondary, + onPrimary: colors.white, + surface: colors.white, + background: colors.secondary, + error: colors.error, + }, +};
1-42: Plan for dark mode and accessibilityConsider adding dark palette tokens and typography line-heights to improve readability and dynamic type support. Also add semantic colors (warning, info, divider, disabled) used across screens.
frontend/screens/HomeScreen.js (2)
127-133: Minor: chunking into rows is fine; consider FlatList numColumns for simplicity
useMemo+ manual row chunking works; RN FlatList’snumColumns={2}can simplify and improve accessibility and virtualization.
275-297: ListEmptyComponent condition is redundantYou’re already in the
!isLoadingbranch; the!isLoading && (...)check is unnecessary.- ListEmptyComponent={ - !isLoading && ( - <View style={styles.emptyContainer}> - <Text style={styles.emptyText}> - No active groups. Create or join one! - </Text> - </View> - ) - } + ListEmptyComponent={ + <View style={styles.emptyContainer}> + <Text style={styles.emptyText}> + No active groups. Create or join one! + </Text> + </View> + }frontend/screens/SignupScreen.js (1)
40-73: Enhance inputs with platform hints for UX and autofillAdd
textContentTypeandautoCompleteprops for better autofill and accessibility.- <TextInput + <TextInput label="Name" value={name} onChangeText={setName} style={styles.input} autoCapitalize="words" + textContentType="name" + autoComplete="name" theme={{ colors: { primary: colors.accent } }} /> - <TextInput + <TextInput label="Email" value={email} onChangeText={setEmail} style={styles.input} keyboardType="email-address" autoCapitalize="none" + textContentType="emailAddress" + autoComplete="email" theme={{ colors: { primary: colors.accent } }} /> - <TextInput + <TextInput label="Password" value={password} onChangeText={setPassword} style={styles.input} secureTextEntry + textContentType="password" + autoComplete="password-new" theme={{ colors: { primary: colors.accent } }} /> - <TextInput + <TextInput label="Confirm Password" value={confirmPassword} onChangeText={setConfirmPassword} style={styles.input} secureTextEntry + textContentType="password" + autoComplete="password-new" theme={{ colors: { primary: colors.accent } }} />frontend/screens/JoinGroupScreen.js (1)
51-58: Input UX nicetiesAdd
autoCorrect={false}andtextContentType="oneTimeCode"for typical join codes to reduce typos and enable OS features.<TextInput label="Join Code" value={joinCode} onChangeText={setJoinCode} style={styles.input} autoCapitalize="characters" + autoCorrect={false} + textContentType="oneTimeCode" theme={{ colors: { primary: colors.accent } }} />frontend/screens/AccountScreen.js (1)
75-93: Consider addingaccessibilityRole="button"to menu items.Wrapping each row in
TouchableOpacityis correct, but adding the accessibility role improves screen-reader experience and keeps the app WCAG-friendly.- <TouchableOpacity style={styles.menuItem} onPress={item.onPress}> + <TouchableOpacity + style={styles.menuItem} + onPress={item.onPress} + accessibilityRole="button" + >frontend/screens/FriendsScreen.js (1)
128-131: EnableLayoutAnimationon Android.
LayoutAnimationrequiresUIManager.setLayoutAnimationEnabledExperimental(true)once on Android; otherwise the animation silently fails. Add this in your app bootstrap or in this component’suseEffect.import { UIManager, Platform, /* … */ } from 'react-native'; useEffect(() => { if (Platform.OS === 'android' && UIManager.setLayoutAnimationEnabledExperimental) { UIManager.setLayoutAnimationEnabledExperimental(true); } }, []);frontend/screens/AddExpenseScreen.js (2)
132-146: Stale validation / type tag.Inside the ‘exact’ branch you tag the split with
type: "unequal". If onlyequalandexactstay, consider renaming to"exact"for clarity and downstream filtering.
48-50: Remove unused state to keep component lean.
percentagesandsharesare no longer referenced. Drop them (plus related initialisations) to cut dead code.frontend/screens/GroupSettingsScreen.js (1)
1-10: Prune unused imports.
expo-image-pickerandShareare no longer referenced after stubbing the handlers; remove them to shrink bundle size and avoid linter warnings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
frontend/navigation/GroupsStackNavigator.js(1 hunks)frontend/screens/AccountScreen.js(3 hunks)frontend/screens/AddExpenseScreen.js(6 hunks)frontend/screens/EditProfileScreen.js(5 hunks)frontend/screens/FriendsScreen.js(2 hunks)frontend/screens/GroupDetailsScreen.js(6 hunks)frontend/screens/GroupSettingsScreen.js(2 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
🧠 Learnings (2)
📚 Learning: 2025-07-26T09:41:01.332Z
Learnt from: CR
PR: Devasy23/splitwiser#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-26T09:41:01.332Z
Learning: Applies to backend/app/expenses/**/*.py : Settlements must track debt resolution between users in the expense tracking logic.
Applied to files:
frontend/screens/HomeScreen.jsfrontend/screens/GroupDetailsScreen.jsfrontend/screens/AddExpenseScreen.js
📚 Learning: 2025-07-26T09:41:01.332Z
Learnt from: CR
PR: Devasy23/splitwiser#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-26T09:41:01.332Z
Learning: Applies to backend/app/expenses/**/*.py : Support for different expense split types (equal, unequal, percentage-based) must be implemented in the expense tracking logic.
Applied to files:
frontend/screens/AddExpenseScreen.js
🧬 Code Graph Analysis (1)
frontend/screens/JoinGroupScreen.js (3)
frontend/screens/EditProfileScreen.js (1)
styles(125-166)frontend/screens/LoginScreen.js (1)
styles(67-100)frontend/screens/SignupScreen.js (1)
styles(96-129)
⏰ 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). (1)
- GitHub Check: bundle-analysis
🔇 Additional comments (5)
frontend/navigation/GroupsStackNavigator.js (1)
15-17: No in-screen header in GroupDetailsScreen—keep the default stack header
Our searches for any Appbar or Header imports and JSX infrontend/screens/GroupDetailsScreen.jsreturned no hits, so there’s no built-in header that would conflict with React Navigation’s. You can ignore the suggestion to addoptions={{ headerShown: false }}on theGroupDetailsroute.Likely an incorrect or invalid review comment.
frontend/screens/SignupScreen.js (1)
96-128: Styling consistency looks goodTheme usage for spacing, colors, and typography is consistent with other screens.
frontend/screens/JoinGroupScreen.js (2)
75-101: Theming and layout LGTMConsistent use of centralized theme for spacing, colors, and typography.
38-48: Confirmed support forAppbar.Contentprops in v5.14.5
Verified that react-native-paper is at version ^5.14.5 in frontend/package.json, and in v5 thecolorandtitleStyleprops onAppbar.Contentare fully supported—no changes needed.frontend/screens/EditProfileScreen.js (1)
24-26: LGTM – simplified MIME‐type handling looks good.The fallback to
"image/jpeg"is sensible and the data-URI construction is correct.
No issues spotted.
| const calculateSettlementStatus = async (groupId, userId) => { | ||
| try { | ||
| const response = await getOptimizedSettlements(groupId); | ||
| const settlements = response.data.optimizedSettlements || []; | ||
| const userOwes = settlements.filter((s) => s.fromUserId === userId); | ||
| const userIsOwed = settlements.filter((s) => s.toUserId === userId); | ||
| return { | ||
| isSettled: userOwes.length === 0 && userIsOwed.length === 0, | ||
| netBalance: | ||
| userIsOwed.reduce((sum, s) => sum + s.amount, 0) - | ||
| userOwes.reduce((sum, s) => sum + s.amount, 0), | ||
| }; | ||
| } catch (error) { | ||
| return { isSettled: true, netBalance: 0 }; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Settlement error handling may hide active debt
On failure, returning { isSettled: true } wrongly classifies the group as settled, potentially hiding actionable balances from users.
Prefer treating failures as “unknown/assume active” and log the error. Also guard for float rounding when determining settled:
const calculateSettlementStatus = async (groupId, userId) => {
try {
const response = await getOptimizedSettlements(groupId);
const settlements = response.data.optimizedSettlements || [];
const userOwes = settlements.filter((s) => s.fromUserId === userId);
const userIsOwed = settlements.filter((s) => s.toUserId === userId);
- return {
- isSettled: userOwes.length === 0 && userIsOwed.length === 0,
- netBalance:
- userIsOwed.reduce((sum, s) => sum + s.amount, 0) -
- userOwes.reduce((sum, s) => sum + s.amount, 0),
- };
+ const net =
+ userIsOwed.reduce((sum, s) => sum + s.amount, 0) -
+ userOwes.reduce((sum, s) => sum + s.amount, 0);
+ const epsilon = 0.01;
+ const isSettled =
+ (userOwes.length === 0 && userIsOwed.length === 0) ||
+ Math.abs(net) < epsilon;
+ return { isSettled, netBalance: net };
} catch (error) {
- return { isSettled: true, netBalance: 0 };
+ console.warn(`Failed to fetch settlements for group ${groupId}:`, error);
+ return { isSettled: false, netBalance: 0 };
}
};This aligns with the need to accurately reflect unresolved debts and avoid false “settled” states.
📝 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 calculateSettlementStatus = async (groupId, userId) => { | |
| try { | |
| const response = await getOptimizedSettlements(groupId); | |
| const settlements = response.data.optimizedSettlements || []; | |
| const userOwes = settlements.filter((s) => s.fromUserId === userId); | |
| const userIsOwed = settlements.filter((s) => s.toUserId === userId); | |
| return { | |
| isSettled: userOwes.length === 0 && userIsOwed.length === 0, | |
| netBalance: | |
| userIsOwed.reduce((sum, s) => sum + s.amount, 0) - | |
| userOwes.reduce((sum, s) => sum + s.amount, 0), | |
| }; | |
| } catch (error) { | |
| return { isSettled: true, netBalance: 0 }; | |
| } | |
| }; | |
| const calculateSettlementStatus = async (groupId, userId) => { | |
| try { | |
| const response = await getOptimizedSettlements(groupId); | |
| const settlements = response.data.optimizedSettlements || []; | |
| const userOwes = settlements.filter((s) => s.fromUserId === userId); | |
| const userIsOwed = settlements.filter((s) => s.toUserId === userId); | |
| const net = | |
| userIsOwed.reduce((sum, s) => sum + s.amount, 0) - | |
| userOwes.reduce((sum, s) => sum + s.amount, 0); | |
| const epsilon = 0.01; | |
| const isSettled = | |
| (userOwes.length === 0 && userIsOwed.length === 0) || | |
| Math.abs(net) < epsilon; | |
| return { isSettled, netBalance: net }; | |
| } catch (error) { | |
| console.warn(`Failed to fetch settlements for group ${groupId}:`, error); | |
| return { isSettled: false, netBalance: 0 }; | |
| } | |
| }; |
🤖 Prompt for AI Agents
In frontend/screens/HomeScreen.js around lines 80 to 95, the error handling in
calculateSettlementStatus returns isSettled: true on failure, which incorrectly
marks the group as settled and hides active debts. Modify the catch block to log
the error and return isSettled: false with netBalance: 0 to indicate an unknown
or active debt state. Additionally, adjust the isSettled calculation to consider
a small epsilon threshold for float rounding errors when checking if netBalance
is effectively zero.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
frontend/screens/GroupSettingsScreen.js (1)
61-62: Restore fallback to legacyiconfield when loading.Dropping the old
iconfield breaks display/edit for existing groups.Apply this diff:
- setIcon(gRes.data.imageUrl || ""); + setIcon(gRes.data.imageUrl ?? gRes.data.icon ?? "");
🧹 Nitpick comments (4)
frontend/screens/GroupSettingsScreen.js (1)
79-81: Use the correct MIME type for base64 data URIs.Hardcoding
image/jpegmay break PNGs or other formats. Prefer the picked asset’s MIME type when available, with a safe fallback.Example:
- updates.imageUrl = `data:image/jpeg;base64,${pickedImage.base64}`; + const mime = pickedImage?.mimeType || "image/jpeg"; + updates.imageUrl = `data:${mime};base64,${pickedImage.base64}`;frontend/screens/AddExpenseScreen.js (3)
25-34: Add basic accessibility props to the custom checkbox.
CustomCheckboxis touch-only; screen-reader users receive no role / state cues.
AddaccessibilityRole="checkbox"andaccessibilityState={{ checked }}(or Paper’s Checkbox) to announce status changes.
316-333: Improve “Paid by” menu anchor hit-box & a11y.Wrapping the text & chevron in a
TouchableOpacitywithout padding reduces tap area on small devices and lacks accessibility labelling.Consider:
-<TouchableOpacity …> +<TouchableOpacity + accessibilityRole="button" + accessibilityLabel={`Paid by, ${selectedPayerName}, double tap to change`} + hitSlop={{ top: 8, bottom: 8, left: 8, right: 8 }} + …>
453-461: Hard-coded input width limits localisation.
width: 100onsplitInputtruncates long currencies / large numbers in some locales.
Switch tominWidthor flex-based sizing to keep the UI resilient.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/screens/AddExpenseScreen.js(7 hunks)frontend/screens/GroupSettingsScreen.js(5 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-07-26T09:41:01.332Z
Learnt from: CR
PR: Devasy23/splitwiser#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-26T09:41:01.332Z
Learning: Applies to backend/app/expenses/**/*.py : Support for different expense split types (equal, unequal, percentage-based) must be implemented in the expense tracking logic.
Applied to files:
frontend/screens/AddExpenseScreen.js
📚 Learning: 2025-07-26T09:41:01.332Z
Learnt from: CR
PR: Devasy23/splitwiser#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-26T09:41:01.332Z
Learning: Applies to backend/app/expenses/**/*.py : Settlements must track debt resolution between users in the expense tracking logic.
Applied to files:
frontend/screens/AddExpenseScreen.js
⏰ 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 (3)
frontend/screens/GroupSettingsScreen.js (2)
230-240: Verify default navigation header is hidden to avoid double headers.Since you render a custom
Appbar.Header, ensure the native header is off for this screen.If not already configured in your navigator:
- In the screen options for GroupSettingsScreen:
options={{ headerShown: false }}; or- Programmatically:
navigation.setOptions({ headerShown: false }).
121-140: Good fix: critical actions wired back up.Leave, Share, and Delete handlers are implemented again. Looks good.
Also applies to: 143-151, 169-188
frontend/screens/AddExpenseScreen.js (1)
171-177:totalPercentagecan becomeNaN, breaking validation and split math.
parseFloat('')returnsNaN; when any input is left blank the reducer propagatesNaN, so
Math.abs(totalPercentage - 100)→NaN, the check silently passes, and subsequent amount calculations yieldNaN.Guard against non-numeric inputs before aggregation, e.g.:
-const totalPercentage = Object.values(percentages).reduce( - (sum, val) => sum + parseFloat(val || "0"), - 0 -); +const totalPercentage = Object.values(percentages).reduce((sum, val) => { + const n = Number(val); + return sum + (isFinite(n) ? n : 0); +}, 0);Likely an incorrect or invalid review comment.
- 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.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
frontend/screens/HomeScreen.js (1)
91-95: Settlement check should use OR with epsilon (prevents false “unsettled” when net ≈ 0)Right now, both lists must be empty AND net ≈ 0. If entries cancel out to ~0, the group stays “active”. Prefer OR with epsilon.
- const isSettled = - userOwes.length === 0 && - userIsOwed.length === 0 && - Math.abs(netBalance) < epsilon; + const isSettled = + (userOwes.length === 0 && userIsOwed.length === 0) || + Math.abs(netBalance) < epsilon;frontend/screens/GroupDetailsScreen.js (1)
245-261: Acknowledged: Platform import fixed for Platform.select usagePrevents runtime ReferenceError flagged previously.
🧹 Nitpick comments (11)
frontend/utils/avatar.js (1)
9-11: Broaden supported schemes and slightly tighten data URI matchingConsider accepting Expo asset URIs and making data URI matching a bit stricter to avoid edge false-positives.
-const VALID_URI_REGEX = /^(https?:|data:image|file:|content:)/ +// Accept http(s), image data URIs, local file/content, and Expo asset URIs +const VALID_URI_REGEX = /^(https?:\/\/|data:image\/[a-zA-Z0-9.+-]+(?:;[^,]*)?,|file:|content:|asset:)/frontend/screens/HomeScreen.js (2)
150-151: Avoid passing ambiguous “groupIcon” (raw URI or single char) as a single propDownstream, this can lead to showing raw paths as text. Pass explicit fields.
- groupIcon: item.imageUrl || item.name?.charAt(0) || "?", + groupImageUri: isValidImageUri(item.imageUrl) ? item.imageUrl : undefined, + groupInitial: getInitial(item.name),Update the target screen to use groupImageUri if present, else fallback to groupInitial.
132-139: Consider FlatList numColumns instead of manual row chunkingUsing numColumns=2 simplifies state and layout code and improves accessibility/focus order.
- const activeGroupRows = useMemo(() => { - const rows = []; - for (let i = 0; i < activeGroups.length; i += 2) { - rows.push(activeGroups.slice(i, i + 2)); - } - return rows; - }, [activeGroups]); + // Not needed if using FlatList with numColumns- data={activeGroupRows} - renderItem={renderGroupRow} - keyExtractor={(item, index) => `row-${index}`} + data={activeGroups} + renderItem={renderGroupCard} // one card renderer + keyExtractor={(item) => item._id} + numColumns={2} + columnWrapperStyle={styles.row}Also applies to: 296-300
frontend/screens/FriendsScreen.js (2)
7-11: Enable LayoutAnimation on Android to actually animate togglesWithout enabling, Android won’t animate. Add UIManager gate.
-import { - Alert, - FlatList, - LayoutAnimation, - StyleSheet, - TouchableOpacity, - View, -} from "react-native"; +import { + Alert, + FlatList, + LayoutAnimation, + Platform, + UIManager, + StyleSheet, + TouchableOpacity, + View, +} from "react-native"; + +// Enable LayoutAnimation on Android +if ( + Platform.OS === "android" && + UIManager.setLayoutAnimationEnabledExperimental +) { + UIManager.setLayoutAnimationEnabledExperimental(true); +}
158-159: Ensure keyExtractor returns a stringGuard against numeric IDs to avoid warnings.
- keyExtractor={(item) => item.id} + keyExtractor={(item) => String(item.id)}frontend/screens/GroupDetailsScreen.js (2)
31-33: Reuse shared currency formatter for consistencyAvoid local currency function; use the app’s currency util.
- const currency = "₹"; - const formatCurrency = (amount) => `${currency}${amount.toFixed(2)}`; + // Use shared formatter for consistency across screens + // import { formatCurrency } from "../utils/currency";Outside this hunk, add:
+import { formatCurrency } from "../utils/currency";
128-131: Optional: Enable LayoutAnimation on Android here as wellTo animate the settlement summary expand/collapse on Android.
Add near imports:
+import { Platform, UIManager } from "react-native"; + +if ( + Platform.OS === "android" && + UIManager.setLayoutAnimationEnabledExperimental +) { + UIManager.setLayoutAnimationEnabledExperimental(true); +}frontend/screens/GroupSettingsScreen.js (4)
60-64: Reduce image size before base64; avoid large payloads and memory spikes.Picking with
base64: trueandquality: 1can create very large strings. Resize and compress first; then request base64.const pickImage = async () => { if (!isAdmin) return; - const 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(''); } + const result = await ImagePicker.launchImageLibraryAsync({ + mediaTypes: ImagePicker.MediaTypeOptions.Images, + allowsEditing: true, + aspect: [1, 1], + quality: 1, + base64: false + }); + if (!result.canceled) { + // Resize to ~512x512, compress to ~70%, and get base64 + const asset = result.assets[0]; + const manip = await ImageManipulator.manipulateAsync( + asset.uri, + [{ resize: { width: 512, height: 512 } }], + { compress: 0.7, format: ImageManipulator.SaveFormat.JPEG, base64: true } + ); + setPickedImage({ ...asset, ...manip }); + setIcon(''); + } };Add this import (outside the selected lines):
+import * as ImageManipulator from 'expo-image-manipulator';
118-121: Use titleStyle for color; Appbar.Content doesn’t supportcolorprop.Move color into
titleStyleto ensure it applies consistently.-<Appbar.Content title='Group Settings' color={colors.white} titleStyle={{ ...typography.h2 }} /> +<Appbar.Content + title='Group Settings' + titleStyle={{ ...typography.h2, color: colors.white }} +/ >
128-131: Improve selected emoji contrast and reuse styles.On selection, emoji sits on a primary background but text remains default (low contrast). Add selected styles and avoid inline style objects.
- {ICON_CHOICES.map(i => ( - <TouchableOpacity key={i} style={[styles.iconBtn, icon === i && { backgroundColor: colors.primary }]} onPress={() => { setIcon(i); if (pickedImage) setPickedImage(null); }} disabled={!isAdmin}> - <Text style={{ fontSize: 24 }}>{i}</Text> - </TouchableOpacity> - ))} + {ICON_CHOICES.map(i => ( + <TouchableOpacity + key={i} + style={[styles.iconBtn, icon === i && styles.iconBtnSelected]} + onPress={() => { setIcon(i); if (pickedImage) setPickedImage(null); }} + disabled={!isAdmin} + > + <Text style={[styles.iconText, icon === i && styles.iconTextSelected]}>{i}</Text> + </TouchableOpacity> + ))} @@ const styles = StyleSheet.create({ @@ iconRow: { flexDirection: 'row', flexWrap: 'wrap', marginBottom: spacing.md }, - iconBtn: { padding: spacing.sm, borderRadius: spacing.sm, borderWidth: 1, borderColor: colors.primary, marginRight: spacing.sm, marginBottom: spacing.sm }, + iconBtn: { padding: spacing.sm, borderRadius: spacing.sm, borderWidth: 1, borderColor: colors.primary, marginRight: spacing.sm, marginBottom: spacing.sm }, + iconBtnSelected: { backgroundColor: colors.primary, borderColor: colors.primary }, + iconText: { fontSize: 24 }, + iconTextSelected: { color: colors.white },Also applies to: 160-178
146-148: Guard share action when join code is unavailable.Avoid sharing an empty message and show clearer state to the user.
-<Text style={styles.joinCode}>Join Code: {group?.joinCode}</Text> -<Button mode='outlined' onPress={onShare} icon='share-variant' labelStyle={{ color: colors.primary }}>Share</Button> +<Text style={styles.joinCode}>Join Code: {group?.joinCode || 'Unavailable'}</Text> +<Button + mode='outlined' + onPress={onShare} + icon='share-variant' + disabled={!group?.joinCode} + labelStyle={{ color: colors.primary }} +> + Share +</Button>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
frontend/screens/AddExpenseScreen.js(4 hunks)frontend/screens/EditProfileScreen.js(5 hunks)frontend/screens/FriendsScreen.js(2 hunks)frontend/screens/GroupDetailsScreen.js(6 hunks)frontend/screens/GroupSettingsScreen.js(1 hunks)frontend/screens/HomeScreen.js(7 hunks)frontend/utils/avatar.js(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- frontend/screens/EditProfileScreen.js
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-07-26T09:41:01.332Z
Learnt from: CR
PR: Devasy23/splitwiser#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-26T09:41:01.332Z
Learning: Applies to backend/app/expenses/**/*.py : Settlements must track debt resolution between users in the expense tracking logic.
Applied to files:
frontend/screens/GroupDetailsScreen.jsfrontend/screens/HomeScreen.jsfrontend/screens/AddExpenseScreen.js
📚 Learning: 2025-07-26T09:41:01.332Z
Learnt from: CR
PR: Devasy23/splitwiser#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-26T09:41:01.332Z
Learning: Applies to backend/app/expenses/**/*.py : Support for different expense split types (equal, unequal, percentage-based) must be implemented in the expense tracking logic.
Applied to files:
frontend/screens/AddExpenseScreen.js
🔇 Additional comments (7)
frontend/utils/avatar.js (1)
2-7: LGTM on initial fallback logicSimple, robust, and safe fallback to '?' for non-strings/blank. Works well across screens.
frontend/screens/HomeScreen.js (1)
235-241: Good fix: keys added to mapped settled groupsAvoids React warnings and improves reconciliation.
frontend/screens/AddExpenseScreen.js (1)
281-291: SegmentedButtons theme and icons are fine; keep consistent across screensNo action needed; noting alignment with the app theme.
frontend/screens/GroupSettingsScreen.js (4)
36-36: Good fix: restored backward-compat icon fallback.Reading both
imageUrland legacyiconprevents blank icons for older groups.
104-105: PII-safe avatar fallback looks good.Using
isValidImageUri+Avatar.Textwith initials removes the external avatar service and avoids name leakage.
114-114: Loading UX is solid and themed.Full-screen loader with themed color is consistent with the app’s style.
71-73: ConfirmedgetOptimizedSettlementsreturns an array
The API’s/groups/{group_id}/settlements/optimizeendpoint is defined to return anOptimizedSettlementsResponsewith anoptimizedSettlements: Array<OptimizedSettlement>. Your use of(settlementsRes.data.optimizedSettlements || [])is therefore valid and will correctly handle the response shape. No changes needed.
frontend/screens/AddExpenseScreen.js
Outdated
| const includedMembers = Object.keys(selectedMembers).filter((id) => selectedMembers[id]); | ||
| if (includedMembers.length === 0) | ||
| throw new Error("Select at least one member for the split."); | ||
| const base = Math.floor((numericAmount * 100) / includedMembers.length); | ||
| const remainder = (numericAmount * 100) - base * includedMembers.length; | ||
| splits = includedMembers.map((userId, idx) => ({ | ||
| userId, | ||
| amount: index === 0 ? splitAmount + remainder : splitAmount, // Add remainder to first member | ||
| amount: (base + (idx === 0 ? remainder : 0)) / 100, | ||
| type: "equal", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use integer cents for equal split to prevent rounding drift
Current math may yield fractional remainders due to float math. Work in integer cents and distribute 1¢ remainders.
- const base = Math.floor((numericAmount * 100) / includedMembers.length);
- const remainder = (numericAmount * 100) - base * includedMembers.length;
- splits = includedMembers.map((userId, idx) => ({
- userId,
- amount: (base + (idx === 0 ? remainder : 0)) / 100,
- type: "equal",
- }));
+ const totalCents = Math.round(numericAmount * 100);
+ const base = Math.floor(totalCents / includedMembers.length);
+ const remainder = totalCents - base * includedMembers.length;
+ splits = includedMembers.map((userId, idx) => ({
+ userId,
+ // Optionally spread the 1¢ remainder across the first N members
+ amount: (base + (idx < remainder ? 1 : 0)) / 100,
+ type: "equal",
+ }));📝 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 includedMembers = Object.keys(selectedMembers).filter((id) => selectedMembers[id]); | |
| if (includedMembers.length === 0) | |
| throw new Error("Select at least one member for the split."); | |
| const base = Math.floor((numericAmount * 100) / includedMembers.length); | |
| const remainder = (numericAmount * 100) - base * includedMembers.length; | |
| splits = includedMembers.map((userId, idx) => ({ | |
| userId, | |
| amount: index === 0 ? splitAmount + remainder : splitAmount, // Add remainder to first member | |
| amount: (base + (idx === 0 ? remainder : 0)) / 100, | |
| type: "equal", | |
| const includedMembers = Object.keys(selectedMembers).filter((id) => selectedMembers[id]); | |
| if (includedMembers.length === 0) | |
| throw new Error("Select at least one member for the split."); | |
| const totalCents = Math.round(numericAmount * 100); | |
| const base = Math.floor(totalCents / includedMembers.length); | |
| const remainder = totalCents - base * includedMembers.length; | |
| splits = includedMembers.map((userId, idx) => ({ | |
| userId, | |
| // Optionally spread the 1¢ remainder across the first N members | |
| amount: (base + (idx < remainder ? 1 : 0)) / 100, | |
| type: "equal", | |
| })); |
🤖 Prompt for AI Agents
In frontend/screens/AddExpenseScreen.js around lines 106 to 114, the current
split calculation uses floating point math which can cause rounding errors. To
fix this, convert the total amount to integer cents before dividing, calculate
the base share and remainder in cents, then distribute the remainder by adding 1
cent to the first few members as needed. Finally, convert the split amounts back
to dollars by dividing by 100.
frontend/screens/FriendsScreen.js
Outdated
| const balanceColor = item.netBalance < 0 ? colors.error : colors.success; | ||
| const balanceText = item.netBalance < 0 | ||
| ? `You owe ${formatCurrency(Math.abs(item.netBalance))}` | ||
| : `Owes you ${formatCurrency(item.netBalance)}`; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Show neutral styling for “Settled up” (avoid green text at zero)
Currently zero picks success color; use a neutral color and copy when balance ≈ 0.
- const balanceColor = item.netBalance < 0 ? colors.error : colors.success;
- const balanceText = item.netBalance < 0
+ const zeroish = Math.abs(item.netBalance) < 0.005;
+ const balanceColor = zeroish
+ ? colors.textSecondary
+ : item.netBalance < 0
+ ? colors.error
+ : colors.success;
+ const balanceText = zeroish
+ ? 'Settled up'
+ : item.netBalance < 0
? `You owe ${formatCurrency(Math.abs(item.netBalance))}`
: `Owes you ${formatCurrency(item.netBalance)}`;Also applies to: 42-44
🤖 Prompt for AI Agents
In frontend/screens/FriendsScreen.js around lines 26 to 30 and 42 to 44, the
code assigns the success color and text when the net balance is zero, which
should instead have a neutral color and message. Update the conditional logic to
check if the net balance is exactly zero and in that case, set balanceColor to a
neutral color and balanceText to a "Settled up" message. Keep the existing logic
for positive and negative balances unchanged.
| const onSave = async () => { | ||
| if (!isAdmin) return; const updates = {}; | ||
| if (name && name !== group?.name) updates.name = name; | ||
| if (pickedImage?.base64) { | ||
| updates.imageUrl = `data:image/jpeg;base64,${pickedImage.base64}`; | ||
| } else { | ||
| const original = group?.imageUrl || group?.icon || ''; | ||
| if (icon !== original) updates.imageUrl = icon || ''; | ||
| } | ||
| if (Object.keys(updates).length === 0) return; | ||
| try { setSaving(true); const res = await apiUpdateGroup(groupId, updates); setGroup(res.data); if (pickedImage) setPickedImage(null); Alert.alert('Success','Group updated successfully.'); } | ||
| catch (e) { Alert.alert('Error', e.response?.data?.detail || 'Failed to update.'); } | ||
| finally { setSaving(false); } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Don’t write emoji into imageUrl; preserve legacy icon field and sync form state after save.
Current logic sets updates.imageUrl = icon when an emoji is selected. This can violate the data model (emoji ends up in imageUrl) and may break consumers expecting imageUrl to be an image URI/DataURI. Also, after saving, local name/icon state is not synced to the backend response, risking stale UI.
Refactor: update icon when an emoji is chosen; update imageUrl only for images; clear the opposite field when switching; trim name; sync local form state from the response.
-const onSave = async () => {
- if (!isAdmin) return; const updates = {};
- if (name && name !== group?.name) updates.name = name;
- if (pickedImage?.base64) {
- updates.imageUrl = `data:image/jpeg;base64,${pickedImage.base64}`;
- } else {
- const original = group?.imageUrl || group?.icon || '';
- if (icon !== original) updates.imageUrl = icon || '';
- }
- if (Object.keys(updates).length === 0) return;
- try { setSaving(true); const res = await apiUpdateGroup(groupId, updates); setGroup(res.data); if (pickedImage) setPickedImage(null); Alert.alert('Success','Group updated successfully.'); }
- catch (e) { Alert.alert('Error', e.response?.data?.detail || 'Failed to update.'); }
- finally { setSaving(false); }
-};
+const onSave = async () => {
+ if (!isAdmin) return;
+ const updates = {};
+ const trimmedName = name?.trim();
+ if (trimmedName && trimmedName !== group?.name) updates.name = trimmedName;
+
+ const originalImage = group?.imageUrl ?? '';
+ const originalEmoji = group?.icon ?? '';
+
+ if (pickedImage?.base64) {
+ // Image chosen: set imageUrl and clear emoji if previously set
+ updates.imageUrl = `data:image/jpeg;base64,${pickedImage.base64}`;
+ if (originalEmoji) updates.icon = '';
+ } else {
+ // Emoji chosen: set icon and clear image if previously set
+ if (ICON_CHOICES.includes(icon)) {
+ if (icon !== originalEmoji) {
+ updates.icon = icon;
+ if (originalImage) updates.imageUrl = '';
+ }
+ } else {
+ // Clearing any previously set image/emoji
+ if ((icon ?? '') === '' && (originalImage || originalEmoji)) {
+ if (originalImage) updates.imageUrl = '';
+ if (originalEmoji) updates.icon = '';
+ }
+ }
+ }
+
+ if (Object.keys(updates).length === 0) return;
+ try {
+ setSaving(true);
+ const res = await apiUpdateGroup(groupId, updates);
+ setGroup(res.data);
+ // keep form in sync with server
+ setName(res.data?.name ?? name);
+ setIcon(res.data?.imageUrl || res.data?.icon || '');
+ if (pickedImage) setPickedImage(null);
+ Alert.alert('Success','Group updated successfully.');
+ } catch (e) {
+ Alert.alert('Error', e.response?.data?.detail || 'Failed to update.');
+ } finally {
+ setSaving(false);
+ }
+};Happy to adjust this if your backend schema differs (e.g., always migrating to imageUrl).
📝 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 onSave = async () => { | |
| if (!isAdmin) return; const updates = {}; | |
| if (name && name !== group?.name) updates.name = name; | |
| if (pickedImage?.base64) { | |
| updates.imageUrl = `data:image/jpeg;base64,${pickedImage.base64}`; | |
| } else { | |
| const original = group?.imageUrl || group?.icon || ''; | |
| if (icon !== original) updates.imageUrl = icon || ''; | |
| } | |
| if (Object.keys(updates).length === 0) return; | |
| try { setSaving(true); const res = await apiUpdateGroup(groupId, updates); setGroup(res.data); if (pickedImage) setPickedImage(null); Alert.alert('Success','Group updated successfully.'); } | |
| catch (e) { Alert.alert('Error', e.response?.data?.detail || 'Failed to update.'); } | |
| finally { setSaving(false); } | |
| }; | |
| const onSave = async () => { | |
| if (!isAdmin) return; | |
| const updates = {}; | |
| const trimmedName = name?.trim(); | |
| if (trimmedName && trimmedName !== group?.name) { | |
| updates.name = trimmedName; | |
| } | |
| const originalImage = group?.imageUrl ?? ''; | |
| const originalEmoji = group?.icon ?? ''; | |
| if (pickedImage?.base64) { | |
| // Image chosen: set imageUrl and clear emoji if previously set | |
| updates.imageUrl = `data:image/jpeg;base64,${pickedImage.base64}`; | |
| if (originalEmoji) { | |
| updates.icon = ''; | |
| } | |
| } else { | |
| // Emoji chosen: set icon and clear image if previously set | |
| if (ICON_CHOICES.includes(icon)) { | |
| if (icon !== originalEmoji) { | |
| updates.icon = icon; | |
| if (originalImage) { | |
| updates.imageUrl = ''; | |
| } | |
| } | |
| } else { | |
| // Clearing any previously set image/emoji | |
| if ((icon ?? '') === '' && (originalImage || originalEmoji)) { | |
| if (originalImage) { | |
| updates.imageUrl = ''; | |
| } | |
| if (originalEmoji) { | |
| updates.icon = ''; | |
| } | |
| } | |
| } | |
| } | |
| if (Object.keys(updates).length === 0) return; | |
| try { | |
| setSaving(true); | |
| const res = await apiUpdateGroup(groupId, updates); | |
| setGroup(res.data); | |
| // keep form in sync with server | |
| setName(res.data?.name ?? name); | |
| setIcon(res.data?.imageUrl || res.data?.icon || ''); | |
| if (pickedImage) { | |
| setPickedImage(null); | |
| } | |
| Alert.alert('Success', 'Group updated successfully.'); | |
| } catch (e) { | |
| Alert.alert('Error', e.response?.data?.detail || 'Failed to update.'); | |
| } finally { | |
| setSaving(false); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In frontend/screens/GroupSettingsScreen.js around lines 45 to 58, the current
onSave function incorrectly assigns emoji icons to imageUrl, which should only
hold image URIs or DataURIs, and does not update local name and icon state after
saving. Refactor the code to assign emoji values to the icon field only, assign
image URIs to imageUrl only, clear the opposite field when switching between
icon and image, trim the name before saving, and after a successful save, update
the local name and icon state from the backend response to keep the UI in sync.
| color: | ||
| item.settlementStatus.netBalance > 0 | ||
| ? colors.success | ||
| : colors.error, | ||
| }, | ||
| ]} | ||
| > | ||
| {item.settlementStatus.netBalance > 0 | ||
| ? `You are owed ${formatCurrency( | ||
| item.settlementStatus.netBalance | ||
| )}` | ||
| : `You owe ${formatCurrency( | ||
| Math.abs(item.settlementStatus.netBalance) | ||
| )}`} | ||
| </Text> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Neutralize color and copy when balance is effectively zero
Avoid red “You owe 0.00”. Treat near-zero as settled/neutral for clarity.
- {
- color:
- item.settlementStatus.netBalance > 0
- ? colors.success
- : colors.error,
- },
+ (() => {
+ const net = item.settlementStatus.netBalance;
+ const zeroish = Math.abs(net) < 0.005;
+ return {
+ color: zeroish
+ ? colors.textSecondary
+ : net > 0
+ ? colors.success
+ : colors.error,
+ };
+ })(),
]}
>
- {item.settlementStatus.netBalance > 0
- ? `You are owed ${formatCurrency(
- item.settlementStatus.netBalance
- )}`
- : `You owe ${formatCurrency(
- Math.abs(item.settlementStatus.netBalance)
- )}`}
+ {(() => {
+ const net = item.settlementStatus.netBalance;
+ const zeroish = Math.abs(net) < 0.005;
+ if (zeroish) return "Settled up";
+ return net > 0
+ ? `You are owed ${formatCurrency(net)}`
+ : `You owe ${formatCurrency(Math.abs(net))}`;
+ })()}📝 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.
| color: | |
| item.settlementStatus.netBalance > 0 | |
| ? colors.success | |
| : colors.error, | |
| }, | |
| ]} | |
| > | |
| {item.settlementStatus.netBalance > 0 | |
| ? `You are owed ${formatCurrency( | |
| item.settlementStatus.netBalance | |
| )}` | |
| : `You owe ${formatCurrency( | |
| Math.abs(item.settlementStatus.netBalance) | |
| )}`} | |
| </Text> | |
| // replace the static color object with a computed one that handles near-zero as neutral | |
| (() => { | |
| const net = item.settlementStatus.netBalance; | |
| const zeroish = Math.abs(net) < 0.005; | |
| return { | |
| color: zeroish | |
| ? colors.textSecondary | |
| : net > 0 | |
| ? colors.success | |
| : colors.error, | |
| }; | |
| })(), | |
| ]} | |
| > | |
| {/* replace the simple ternary with a computed label that shows "Settled up" for near-zero */} | |
| {(() => { | |
| const net = item.settlementStatus.netBalance; | |
| const zeroish = Math.abs(net) < 0.005; | |
| if (zeroish) return "Settled up"; | |
| return net > 0 | |
| ? `You are owed ${formatCurrency(net)}` | |
| : `You owe ${formatCurrency(Math.abs(net))}`; | |
| })()} | |
| </Text> |
🤖 Prompt for AI Agents
In frontend/screens/HomeScreen.js between lines 182 and 196, the code currently
shows red text and a "You owe" message even when the netBalance is zero or very
close to zero. To fix this, add a condition to treat near-zero balances as
neutral: if the absolute value of netBalance is below a small threshold (e.g.,
0.01), display a neutral color and a message indicating the balance is settled
instead of showing "You owe 0.00" in red.
… functions" This reverts commit 6d75613.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
frontend/screens/FriendsScreen.js (1)
25-31: Neutral styling and copy for near‑zero balances (“Settled up”)Avoid showing green when balance ≈ 0; use a neutral color and message. Applies to both color and text.
- const balanceColor = - item.netBalance < 0 ? colors.error : colors.success; - const balanceText = - item.netBalance < 0 - ? `You owe ${formatCurrency(Math.abs(item.netBalance))}` - : `Owes you ${formatCurrency(item.netBalance)}`; + const zeroish = Math.abs(item.netBalance) < 0.005; + const balanceColor = zeroish + ? colors.textSecondary + : item.netBalance < 0 + ? colors.error + : colors.success; + const balanceText = zeroish + ? "Settled up" + : item.netBalance < 0 + ? `You owe ${formatCurrency(Math.abs(item.netBalance))}` + : `Owes you ${formatCurrency(item.netBalance)}`; @@ - {item.netBalance !== 0 ? balanceText : "Settled up"} + {balanceText}Also applies to: 45-47
🧹 Nitpick comments (3)
frontend/screens/FriendsScreen.js (3)
171-171: Normalize FlatList keys to stringsAvoid RN key warnings by ensuring string keys.
- keyExtractor={(item) => item.id} + keyExtractor={(item) => String(item.id)}
95-105: Make fetch resilient to partial failures (don’t fail friends if groups fail)Promise.all rejects if any request fails; use allSettled to show friends even if groups metadata is unavailable.
- const [friendsResponse, groupsResponse] = await Promise.all([ - getFriendsBalance(), - getGroups(), - ]); - const friendsData = friendsResponse.data.friendsBalance || []; - const groupMeta = new Map( - (groupsResponse.data.groups || []).map((g) => [ - g._id, - { name: g.name, imageUrl: g.imageUrl }, - ]) - ); + const [friendsRes, groupsRes] = await Promise.allSettled([ + getFriendsBalance(), + getGroups(), + ]); + if (friendsRes.status !== "fulfilled") { + throw friendsRes.reason; + } + const friendsData = friendsRes.value.data.friendsBalance || []; + const groupsArr = + groupsRes.status === "fulfilled" + ? groupsRes.value.data.groups || [] + : []; + const groupMeta = new Map( + groupsArr.map((g) => [g._id, { name: g.name, imageUrl: g.imageUrl }]) + );
129-130: Android LayoutAnimation requires enabling experimental flagEnsure UIManager.setLayoutAnimationEnabledExperimental(true) is set on Android (e.g., in App.js) so the animation actually runs.
// In App.js (top-level), once: import { Platform, UIManager } from "react-native"; if (Platform.OS === "android" && UIManager.setLayoutAnimationEnabledExperimental) { UIManager.setLayoutAnimationEnabledExperimental(true); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
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)
🚧 Files skipped from review as they are similar to previous changes (5)
- frontend/screens/GroupDetailsScreen.js
- frontend/screens/EditProfileScreen.js
- frontend/screens/HomeScreen.js
- frontend/screens/AddExpenseScreen.js
- frontend/screens/GroupSettingsScreen.js
🧰 Additional context used
🧬 Code Graph Analysis (1)
frontend/screens/FriendsScreen.js (3)
frontend/utils/currency.js (2)
formatCurrency(8-16)formatCurrency(8-16)frontend/context/AuthContext.js (3)
AuthContext(10-10)AuthContext(10-10)token(14-14)frontend/api/groups.js (4)
getFriendsBalance(29-30)getFriendsBalance(29-30)getGroups(3-3)getGroups(3-3)
⏰ 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). (1)
- GitHub Check: bundle-analysis
🔇 Additional comments (2)
frontend/screens/FriendsScreen.js (2)
12-18: Inconsistency with PR summary: react-native-paper components still in useThe AI summary claims react-native-paper components were replaced with custom ones, but this file still uses Appbar, Avatar, Divider, Text, and ActivityIndicator.
If the intent is to migrate away from react-native-paper here as well, I can provide a targeted refactor to native/Ionicons-based components.
106-117: Data transformation LGTMMapping friends and enriching group breakdown via groupMeta is clean and defensive.
| <Avatar.Image | ||
| size={48} | ||
| source={{ | ||
| uri: | ||
| item.imageUrl || | ||
| `https://avatar.iran.liara.run/username?username=${item.name}`, | ||
| }} | ||
| /> | ||
| <View style={styles.friendInfo}> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid leaking PII to third‑party avatar service; use local initials fallback
Using external URLs with user/group names can leak PII and introduces network/privacy risk. Prefer a local fallback (e.g., Avatar.Text with initials).
- <Avatar.Image
- size={48}
- source={{
- uri:
- item.imageUrl ||
- `https://avatar.iran.liara.run/username?username=${item.name}`,
- }}
- />
+ {item.imageUrl ? (
+ <Avatar.Image size={48} source={{ uri: item.imageUrl }} />
+ ) : (
+ <Avatar.Text size={48} label={getInitials(item.name)} />
+ )}
@@
- <Avatar.Image
- size={32}
- source={{
- uri:
- group.imageUrl ||
- `https://avatar.iran.liara.run/username?username=${group.name}`,
- }}
- />
+ {group.imageUrl ? (
+ <Avatar.Image size={32} source={{ uri: group.imageUrl }} />
+ ) : (
+ <Avatar.Text size={32} label={getInitials(group.name)} />
+ )}Add this helper near the top of the file (above FriendItem):
const getInitials = (name) => {
if (!name) return "?";
const parts = String(name).trim().split(/\s+/);
const initials = parts.slice(0, 2).map((p) => p[0]?.toUpperCase()).join("");
return initials || "?";
};Also applies to: 60-67
🤖 Prompt for AI Agents
In frontend/screens/FriendsScreen.js around lines 35 to 43, avoid using external
avatar URLs with user names to prevent leaking PII and privacy risks. Instead,
add a helper function getInitials at the top of the file to generate initials
from the user's name, and use Avatar.Text with these initials as a local
fallback for the avatar display. Apply the same change for lines 60 to 67 where
similar external avatar URLs are used.
| <Text | ||
| style={{ | ||
| color: group.balance < 0 ? colors.error : colors.success, | ||
| }} | ||
| > | ||
| {formatCurrency(group.balance)} | ||
| </Text> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Apply the same neutral treatment at the group level
Show “Settled up” with a neutral color when group balance ≈ 0.
- <Text
- style={{
- color: group.balance < 0 ? colors.error : colors.success,
- }}
- >
- {formatCurrency(group.balance)}
+ <Text
+ style={{
+ color:
+ Math.abs(group.balance) < 0.005
+ ? colors.textSecondary
+ : group.balance < 0
+ ? colors.error
+ : colors.success,
+ }}
+ >
+ {Math.abs(group.balance) < 0.005
+ ? "Settled up"
+ : formatCurrency(group.balance)}📝 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.
| <Text | |
| style={{ | |
| color: group.balance < 0 ? colors.error : colors.success, | |
| }} | |
| > | |
| {formatCurrency(group.balance)} | |
| </Text> | |
| <Text | |
| style={{ | |
| color: | |
| Math.abs(group.balance) < 0.005 | |
| ? colors.textSecondary | |
| : group.balance < 0 | |
| ? colors.error | |
| : colors.success, | |
| }} | |
| > | |
| {Math.abs(group.balance) < 0.005 | |
| ? "Settled up" | |
| : formatCurrency(group.balance)} | |
| </Text> |
🤖 Prompt for AI Agents
In frontend/screens/FriendsScreen.js around lines 69 to 75, the Text component
displaying the group balance currently only changes color for positive or
negative balances. Modify the conditional styling to also handle the case when
the group balance is approximately zero by displaying the text "Settled up" in a
neutral color instead of a currency value. Use a threshold to determine when the
balance is close enough to zero to be considered settled.
| LayoutAnimation.configureNext(LayoutAnimation.Presets.easeInEaseOut); | ||
| setExpandedFriend(expandedFriend === friendId ? null : friendId); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use functional state update to avoid stale closure on rapid toggles
Prevents race conditions when tapping multiple items quickly.
- setExpandedFriend(expandedFriend === friendId ? null : friendId);
+ setExpandedFriend((prev) => (prev === friendId ? null : friendId));📝 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.
| LayoutAnimation.configureNext(LayoutAnimation.Presets.easeInEaseOut); | |
| setExpandedFriend(expandedFriend === friendId ? null : friendId); | |
| }; | |
| LayoutAnimation.configureNext(LayoutAnimation.Presets.easeInEaseOut); | |
| - setExpandedFriend(expandedFriend === friendId ? null : friendId); | |
| + setExpandedFriend((prev) => (prev === friendId ? null : friendId)); | |
| }; |
🤖 Prompt for AI Agents
In frontend/screens/FriendsScreen.js around lines 129 to 131, the state update
for setExpandedFriend uses the current expandedFriend value directly, which can
cause stale closure issues during rapid toggles. Refactor the setExpandedFriend
call to use a functional state update by passing a function that receives the
previous state and returns the new state. This ensures the update always uses
the latest state value and prevents race conditions.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
frontend/screens/FriendsScreen.js (4)
26-31: Use neutral styling and copy for near-zero balances (“Settled up”)Zero currently renders green; treat ~0 as settled with neutral color and text. This was raised previously and still applies.
- const balanceColor = - item.netBalance < 0 ? colors.error : colors.success; - const balanceText = - item.netBalance < 0 - ? `You owe ${formatCurrency(Math.abs(item.netBalance))}` - : `Owes you ${formatCurrency(item.netBalance)}`; + const zeroish = Math.abs(item.netBalance) < 0.005; + const balanceColor = zeroish + ? colors.textSecondary + : item.netBalance < 0 + ? colors.error + : colors.success; + const balanceText = zeroish + ? "Settled up" + : item.netBalance < 0 + ? `You owe ${formatCurrency(Math.abs(item.netBalance))}` + : `Owes you ${formatCurrency(item.netBalance)}`; @@ - <Text style={[styles.friendBalance, { color: balanceColor }]}> - {item.netBalance !== 0 ? balanceText : "Settled up"} - </Text> + <Text style={[styles.friendBalance, { color: balanceColor }]}> + {balanceText} + </Text>Also applies to: 46-48
36-43: Stop leaking PII via external avatar service; use local initials fallbackEmbedding user/group names in remote avatar URLs can leak PII and create privacy risk. Prefer local initials when no imageUrl exists. This was previously flagged and remains unresolved.
- <Avatar.Image - size={48} - source={{ - uri: - item.imageUrl || - `https://avatar.iran.liara.run/username?username=${item.name}`, - }} - /> + {item.imageUrl ? ( + <Avatar.Image size={48} source={{ uri: item.imageUrl }} /> + ) : ( + <Avatar.Text size={48} label={getInitials(item.name)} /> + )} @@ - <Avatar.Image - size={32} - source={{ - uri: - group.imageUrl || - `https://avatar.iran.liara.run/username?username=${group.name}`, - }} - /> + {group.imageUrl ? ( + <Avatar.Image size={32} source={{ uri: group.imageUrl }} /> + ) : ( + <Avatar.Text size={32} label={getInitials(group.name)} /> + )}Add this helper near the top of the file (outside these lines):
const getInitials = (name) => { if (!name) return "?"; const parts = String(name).trim().split(/\s+/); const initials = parts.slice(0, 2).map((p) => p[0]?.toUpperCase()).join(""); return initials || "?"; };Also applies to: 61-68
70-76: Apply the same “Settled up” treatment at group levelShow neutral color and text when group balance ≈ 0. Previously suggested; still pending.
- <Text - style={{ - color: group.balance < 0 ? colors.error : colors.success, - }} - > - {formatCurrency(group.balance)} + <Text + style={{ + color: + Math.abs(group.balance) < 0.005 + ? colors.textSecondary + : group.balance < 0 + ? colors.error + : colors.success, + }} + > + {Math.abs(group.balance) < 0.005 + ? "Settled up" + : formatCurrency(group.balance)}
141-144: Use functional state update to avoid stale closures on rapid togglesPrevents races when toggling quickly with LayoutAnimation in play. Previously suggested.
const handleToggleFriend = (friendId) => { LayoutAnimation.configureNext(LayoutAnimation.Presets.easeInEaseOut); - setExpandedFriend(expandedFriend === friendId ? null : friendId); + setExpandedFriend((prev) => (prev === friendId ? null : friendId)); };
🧹 Nitpick comments (3)
frontend/components/SkeletonLoader.js (1)
30-31: Mark skeleton as non-accessibleAvoids confusing screen readers with decorative placeholders.
- return <Animated.View style={[styles.skeleton, animatedStyle, style]} />; + return ( + <Animated.View + accessible={false} + style={[styles.skeleton, animatedStyle, style]} + /> + );frontend/screens/FriendsScreen.js (2)
7-8: Confirm Android LayoutAnimation is enabledOn Android, LayoutAnimation requires enabling the experimental flag; otherwise animations may no-op.
Add once at app startup (e.g., App.js):
import { Platform, UIManager } from "react-native"; if (Platform.OS === "android" && UIManager.setLayoutAnimationEnabledExperimental) { UIManager.setLayoutAnimationEnabledExperimental(true); }
25-33: Consider memoizing FriendItem to reduce re-rendersLarge lists benefit from React.memo when only isExpanded changes per item.
Outside this diff, wrap FriendItem:
const FriendItem = React.memo(({ item, onToggle, isExpanded }) => { // ...component });And provide a stable keyExtractor (already present) and avoid recreating callbacks by inlining handleToggleFriend as shown (already OK).
Also applies to: 176-183
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
frontend/components/SkeletonLoader.js(1 hunks)frontend/package.json(1 hunks)frontend/screens/FriendsScreen.js(2 hunks)frontend/screens/HomeScreen.js(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/screens/HomeScreen.js
🧰 Additional context used
🧬 Code Graph Analysis (1)
frontend/screens/FriendsScreen.js (5)
frontend/utils/currency.js (2)
formatCurrency(8-16)formatCurrency(8-16)frontend/components/SkeletonLoader.js (2)
styles(33-38)SkeletonLoader(12-31)frontend/screens/JoinGroupScreen.js (1)
styles(74-102)frontend/context/AuthContext.js (2)
AuthContext(10-10)AuthContext(10-10)frontend/api/groups.js (4)
getFriendsBalance(29-30)getFriendsBalance(29-30)getGroups(3-3)getGroups(3-3)
| import Animated, { | ||
| useSharedValue, | ||
| useAnimatedStyle, | ||
| withRepeat, | ||
| withTiming, | ||
| interpolateColor, | ||
| } from "react-native-reanimated"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add animation cleanup and satisfy hooks deps
Cleanup prevents lingering animations after unmount and fixes the missing dependency warning.
Apply:
import Animated, {
useSharedValue,
useAnimatedStyle,
withRepeat,
withTiming,
interpolateColor,
+ cancelAnimation,
} from "react-native-reanimated";
@@
- useEffect(() => {
- progress.value = withRepeat(withTiming(1, { duration: 1000 }), -1, true);
- }, []);
+ useEffect(() => {
+ progress.value = withRepeat(withTiming(1, { duration: 1000 }), -1, true);
+ return () => cancelAnimation(progress);
+ }, [progress]);Also applies to: 15-18
🤖 Prompt for AI Agents
In frontend/components/SkeletonLoader.js around lines 3–9 (and also apply the
same to lines 15–18), the animated shared values and effects are not cleaned up
and some hook dependencies are missing; add a useEffect that returns a cleanup
function which cancels any running animations (use reanimated's cancelAnimation
or stop methods on the relevant shared values) when the component unmounts, and
ensure all hook dependency arrays (useEffect/useAnimatedStyle/useDerivedValue)
include the values/config they reference (or memoize them) to satisfy the
missing-deps warnings.
| "react-native-web": "^0.20.0", | ||
| "react-native-reanimated": "~3.17.4" | ||
| }, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure Reanimated is fully configured (Babel plugin + install checks)
Adding react-native-reanimated requires the Babel plugin and correct plugin ordering to avoid runtime errors and to enable worklets. Verify the project’s babel.config.js includes the plugin last, and that web support is considered if you target web.
Run this script to confirm the plugin is present and last in the plugins array:
If missing, add this to babel.config.js (ensure it’s the last plugin):
module.exports = function (api) {
api.cache(true);
return {
presets: ["babel-preset-expo"],
plugins: [
// other plugins...
"react-native-reanimated/plugin", // must be last
],
};
};🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate babel config(s)
fd -a '^babel\.config\.js$' | while read -r f; do
echo "Checking $f"
# Show plugins line(s)
rg -n "plugins" -A 5 -n "$f"
# Check if reanimated plugin is present
if rg -q "react-native-reanimated/plugin" "$f"; then
echo "✓ Found reanimated plugin in $f"
else
echo "✗ Missing reanimated plugin in $f"
fi
doneLength of output: 61
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Locate babel config files (common names):"
find . -maxdepth 4 -type f \( -iname "babel.config.js" -o -iname "babel.config.cjs" -o -iname "babel.config.mjs" -o -iname ".babelrc" -o -iname ".babelrc.js" -o -iname ".babelrc.json" \) -print || true
echo
echo "2) Check package.json for a 'babel' field:"
if [ -f package.json ]; then
rg -n '"babel"\s*:' package.json || echo "No 'babel' key in package.json"
else
echo "No package.json at repository root"
fi
echo
echo "3) Search entire repo for reanimated plugin string:"
rg -n --hidden --no-ignore 'react-native-reanimated/plugin' --glob '!node_modules' || echo "No occurrences of reanimated plugin string found"
echo
echo "4) Search for any mention of react-native-reanimated:"
rg -n --hidden --no-ignore 'react-native-reanimated' --glob '!node_modules' || echo "No occurrences of react-native-reanimated found"
echo
echo "5) If any babel config files found above, print their contents (best-effort):"
for f in $(find . -maxdepth 4 -type f \( -iname "babel.config.js" -o -iname "babel.config.cjs" -o -iname ".babelrc" -o -iname ".babelrc.js" -o -iname ".babelrc.json" \) -print); do
echo "---- $f ----"
sed -n '1,240p' "$f"
echo
doneLength of output: 1918
Add react-native-reanimated Babel plugin (must be last) and verify setup
react-native-reanimated is present (frontend/package.json) and imported (frontend/components/SkeletonLoader.js), but no Babel config (.babelrc / babel.config.*) was found — the Reanimated Babel plugin appears to be missing.
Files to fix / check:
- frontend/package.json — dependency added
- frontend/components/SkeletonLoader.js — imports reanimated
- Missing: no babel.config.js or .babelrc detected in the repo root or common locations — add one in the frontend package
Suggested babel config (ensure the plugin is the last entry in plugins):
// frontend/babel.config.js
module.exports = function (api) {
api.cache(true);
return {
presets: ["babel-preset-expo"], // or your project's presets
plugins: [
// other plugins...
"react-native-reanimated/plugin" // MUST be last
],
};
};After adding the plugin: clear Metro cache / rebuild the app and verify worklets/runtime errors are gone. If you target web, also verify Reanimated's web setup for your project.
🤖 Prompt for AI Agents
In frontend/package.json around lines 27 to 29, react-native-reanimated is added
but there is no Babel config in the frontend package; create
frontend/babel.config.js exporting a function that returns your project presets
(e.g. babel-preset-expo or app presets) and a plugins array where
"react-native-reanimated/plugin" is the last entry; ensure the file is placed in
the frontend package root, then clear Metro cache and rebuild (and verify
web-specific Reanimated setup if targeting web).
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| }, | ||
| } | ||
| except Exception as e: | ||
| return {"error": str(e), "type": type(e).__name__} |
Check warning
Code scanning / CodeQL
Information exposure through an exception Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 5 months ago
To fix the problem, the code should avoid returning the string representation of the exception and its type to the client. Instead, it should return a generic error message, such as "An internal error has occurred", and optionally log the exception details on the server for debugging purposes. This change should be made in the exception handler of the debug_expense route (lines 510-511 in backend/app/expenses/routes.py). If logging is desired, use the existing logger imported from app.config to log the exception details. No changes to the endpoint's functionality are required, except for the error reporting.
| @@ -508,4 +508,5 @@ | ||
| }, | ||
| } | ||
| except Exception as e: | ||
| return {"error": str(e), "type": type(e).__name__} | ||
| logger.error(f"Error in debug_expense: {e}", exc_info=True) | ||
| return {"error": "An internal error has occurred"} |
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
.github/workflows/run-tests.yml (1)
58-71: Fix Codecov action inputs: replace deprecatedfilewithfilesand remove/adjustworking-directoryCodecov v5 deprecates the single
fileinput — usefiles.working-directoryis a valid input, butworking-directory: ./is redundant here (remove it or setdirectory/working-directoryto the repo subdir if prior steps change dirs). Also consider gating the override_branch/override_commit inputs for pull_request events to avoid warnings.
- File: .github/workflows/run-tests.yml (lines 58-71)
Suggested diff:
- with: - token: ${{ secrets.CODECOV_TOKEN }} - file: backend/coverage.xml - flags: backend,python,api - name: backend-coverage - fail_ci_if_error: false - verbose: true - working-directory: ./ - override_branch: ${{ github.head_ref }} - override_commit: ${{ github.event.pull_request.head.sha }} + with: + token: ${{ secrets.CODECOV_TOKEN }} + files: backend/coverage.xml + flags: backend,python,api + name: backend-coverage + fail_ci_if_error: false + verbose: true + # working-directory is valid but unnecessary here; remove it or set to the correct subdir + # directory: ./backend + override_branch: ${{ github.head_ref }} + override_commit: ${{ github.event.pull_request.head.sha }}
♻️ Duplicate comments (8)
frontend/screens/GroupSettingsScreen.js (5)
74-75: Restore backward-compatibility with legacyiconfield when initializing.Falling back only to
imageUrlregresses older groups that usedicon(emoji). Include the legacy field to avoid blank icons.Apply this diff:
- setIcon(gRes.data.imageUrl || ""); + setIcon(gRes.data.imageUrl ?? gRes.data.icon ?? "");
88-110: Don’t write emoji intoimageUrl; compare against legacy field; sync local form state after save.Current
onSavewrites emojis toimageUrland compares only toimageUrl, which breaks the data model and causes unnecessary updates. Also, localname/iconaren’t synced to server response. Refactor to:
- Trim and update
name.- If an image was picked: set
imageUrland clearicon.- If emoji selected: set
icon, clearimageUrl.- Compare against both
imageUrland legacyicon.- Sync local state from the response.
const onSave = async () => { if (!isAdmin) return; - const updates = {}; - if (name && name !== group?.name) updates.name = name; - if (pickedImage?.base64) { - updates.imageUrl = `data:image/jpeg;base64,${pickedImage.base64}`; - } else if (icon !== group?.imageUrl) { - updates.imageUrl = icon; - } - - if (Object.keys(updates).length === 0) return; - try { - setSaving(true); - const res = await apiUpdateGroup(groupId, updates); - setGroup(res.data); - if (pickedImage) setPickedImage(null); - Alert.alert("Success", "Group updated successfully."); - } catch (e) { - Alert.alert("Error", e.response?.data?.detail || "Failed to update."); - } finally { - setSaving(false); - } + const updates = {}; + const trimmedName = name?.trim(); + if (trimmedName && trimmedName !== group?.name) updates.name = trimmedName; + + const originalImage = group?.imageUrl ?? ""; + const originalEmoji = group?.icon ?? ""; + + if (pickedImage?.base64) { + // Image chosen: set imageUrl and clear emoji if previously set + updates.imageUrl = `data:image/jpeg;base64,${pickedImage.base64}`; + if (originalEmoji) updates.icon = ""; + } else { + // Emoji chosen: set icon and clear image if previously set + if (ICON_CHOICES.includes(icon)) { + if (icon !== originalEmoji) { + updates.icon = icon; + if (originalImage) updates.imageUrl = ""; + } + } else { + // Clearing any previously set image/emoji + if ((icon ?? "") === "" && (originalImage || originalEmoji)) { + if (originalImage) updates.imageUrl = ""; + if (originalEmoji) updates.icon = ""; + } + } + } + + if (Object.keys(updates).length === 0) return; + try { + setSaving(true); + const res = await apiUpdateGroup(groupId, updates); + setGroup(res.data); + // keep form in sync with server + setName(res.data?.name ?? name); + setIcon(res.data?.imageUrl || res.data?.icon || ""); + if (pickedImage) setPickedImage(null); + Alert.alert("Success", "Group updated successfully."); + } catch (e) { + Alert.alert("Error", e.response?.data?.detail || "Failed to update."); + } finally { + setSaving(false); + } };
273-283: Ensure emoji selection overrides any previously picked image.If an image was picked,
onSavewill prefer it. ClearpickedImagewhen choosing an emoji so the emoji actually applies.- onPress={() => setIcon(i)} + onPress={() => { + setIcon(i); + setPickedImage(null); + }}
121-129: Prevent stale state; optionally block kicks when unsettled balances exist.
- Use functional
setMembersto avoid race conditions.- Optionally disallow kicking if there are outstanding settlements (you already import
getOptimizedSettlements).onPress: async () => { try { + // Optional guard: block kicking if unsettled balances exist + const settlements = await getOptimizedSettlements(groupId); + if (Array.isArray(settlements?.data) && settlements.data.length > 0) { + Alert.alert("Cannot kick member", "Settle outstanding balances first."); + return; + } await apiRemoveMember(groupId, memberId); - setMembers(members.filter((m) => m.userId !== memberId)); + setMembers((prev) => prev.filter((m) => m.userId !== memberId)); Alert.alert("Success", `${memberName} has been kicked.`); } catch (error) { Alert.alert("Error", "Failed to kick member."); } },
209-216: Stop leaking PII to a third-party avatar service.Avoid sending user names to external avatar URLs. Use initials fallback locally if no
imageUrl.- <Avatar.Image - size={40} - source={{ - uri: - m.user?.imageUrl || - `https://avatar.iran.liara.run/username?username=${m.user?.name}`, - }} - /> + {m.user?.imageUrl ? ( + <Avatar.Image size={40} source={{ uri: m.user.imageUrl }} /> + ) : ( + <Avatar.Text + size={40} + label={(m.user?.name || "?").slice(0, 1).toUpperCase()} + /> + )}frontend/screens/GroupDetailsScreen.js (1)
3-11: MissingPlatformimport causes runtime crash.
styles.summaryCardusesPlatform.selectbutPlatformisn’t imported.-import { - Alert, - FlatList, - LayoutAnimation, - StyleSheet, - Text, - TouchableOpacity, - View, -} from "react-native"; +import { + Alert, + FlatList, + LayoutAnimation, + Platform, + StyleSheet, + Text, + TouchableOpacity, + View, +} from "react-native";frontend/screens/AddExpenseScreen.js (2)
200-213: Shares split: guard againstNaNand zero; round outputs.If any input is non-numeric,
totalSharesmay beNaNand bypass current checks.- } else if (splitMethod === "shares") { - const totalShares = Object.values(shares).reduce( - (sum, val) => sum + parseFloat(val || "0"), - 0 - ); - if (totalShares === 0) { - throw new Error("Total shares cannot be zero."); - } - splits = Object.entries(shares) - .filter(([, value]) => parseFloat(value || "0") > 0) - .map(([userId, value]) => ({ - userId, - amount: (numericAmount * parseFloat(value)) / totalShares, - type: "shares", - })); + } else if (splitMethod === "shares") { + const totalShares = Object.values(shares).reduce( + (sum, val) => sum + toNum(val), + 0 + ); + if (!Number.isFinite(totalShares) || totalShares <= 0) { + throw new Error("Total shares must be a positive number."); + } + splits = Object.entries(shares) + .map(([userId, value]) => ({ userId, s: toNum(value) })) + .filter(({ s }) => s > 0) + .map(({ userId, s }) => ({ + userId, + amount: Math.round((numericAmount * s / totalShares) * 100) / 100, + type: "shares", + }));
159-169: Equal split: prevent rounding drift; compute in integer cents.Floating-point rounding here can drift totals. Use integer cents and distribute 1¢ remainders.
- const splitAmount = - Math.round((numericAmount / includedMembers.length) * 100) / 100; - const remainder = - Math.round( - (numericAmount - splitAmount * includedMembers.length) * 100 - ) / 100; - splits = includedMembers.map((userId, index) => ({ - userId, - amount: index === 0 ? splitAmount + remainder : splitAmount, - type: "equal", - })); + const totalCents = Math.round(numericAmount * 100); + const base = Math.floor(totalCents / includedMembers.length); + const remainder = totalCents - base * includedMembers.length; + splits = includedMembers.map((userId, idx) => ({ + userId, + amount: (base + (idx < remainder ? 1 : 0)) / 100, + type: "equal", + }));
🧹 Nitpick comments (11)
backend/app/expenses/routes.py (2)
465-465: Limit exposure of the debug endpoint to non-production environmentsEven with include_in_schema=False, please ensure this route is not enabled in production (feature-flag via settings.DEBUG or environment, or guard it behind an admin-only dependency). This reduces the blast radius if accidentally left deployed.
Do you want me to wire this to a DEBUG flag in app.config (e.g., settings.DEBUG), or add an AdminRequired dependency to gate access?
474-481: Keep the service layer boundary consistentDirect DB access here bypasses the expense_service’s access checks and error semantics. Consider delegating to expense_service.get_expense_by_id(...) and building the debug payload from that result to avoid drift.
.github/workflows/run-tests.yml (1)
14-16: Checkout downgraded to v4 — consider pinning and disabling credentials persistenceUsing actions/checkout@v4 is fine. For supply-chain hardening and least-privilege, please pin to a commit SHA and set persist-credentials: false.
- - uses: actions/checkout@v4 + - uses: actions/checkout@v4 + with: + # Pin to the latest known-good commit for v4 (example SHA shown) + # ref: sha-XXXXXXXX # optional: if you maintain a pinned SHA policy + fetch-depth: 0 # Full history for better diff analysis + persist-credentials: false - with: - fetch-depth: 0 # Full history for better diff analysis.github/workflows/rn-bundle-analysis.yml (2)
18-21: Fix YAML indentation under steps to satisfy linter and avoid surprisesYAMLlint flags wrong indentation (expected 6, found 4). While GitHub often tolerates it, aligning indentation prevents brittle parsing.
- - uses: actions/checkout@v4 - with: - fetch-depth: 0 + - uses: actions/checkout@v4 + with: + fetch-depth: 0
18-21: Harden checkout step (pin SHA and disable credentials persistence)Consider pinning to a commit SHA and turning off credential persistence for defense-in-depth.
- - uses: actions/checkout@v4 - with: - fetch-depth: 0 + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + persist-credentials: false + # ref: sha-XXXXXXXX # optional: if you maintain a pinned SHA policy.github/workflows/bundle-analysis.yml (2)
18-21: Normalize indentation under steps (lint warning)Indent list items two spaces under steps: to satisfy the linter.
- - uses: actions/checkout@v4 - with: - fetch-depth: 0 + - uses: actions/checkout@v4 + with: + fetch-depth: 0
18-21: Add minimal hardening to checkoutPinning and disabling credentials persistence are low-effort improvements.
- - uses: actions/checkout@v4 - with: - fetch-depth: 0 + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + persist-credentials: false + # ref: sha-XXXXXXXX.github/workflows/publish.yml (1)
18-19: Checkout v4 looks fine; consider pinning and disabling persisted credentialsNo functional concerns with the downgrade. Add persist-credentials: false and optionally pin to a SHA for supply-chain hardening.
- uses: actions/checkout@v4 + uses: actions/checkout@v4 + with: + persist-credentials: false + # ref: sha-XXXXXXXX.github/workflows/preview.yml (1)
15-16: Checkout v4: add defense-in-depth tweaksPin to a commit SHA and set persist-credentials: false.
- uses: actions/checkout@v4 + uses: actions/checkout@v4 + with: + persist-credentials: false + # ref: sha-XXXXXXXXfrontend/screens/GroupSettingsScreen.js (1)
167-176: Optional: request media permissions before launching the picker.On some platforms, launching without permissions can fail or prompt in an inconsistent way. Consider requesting permission via
ImagePicker.requestMediaLibraryPermissionsAsync()and handling denial with a user-friendly alert.Happy to provide a small helper to request + cache permission and surface a clear error message if denied.
frontend/screens/GroupDetailsScreen.js (1)
43-45: Use centralized currency formatter for consistency.Avoid hardcoding the currency symbol; prefer the shared
utils/currencyformatter to keep formatting consistent across screens and configurable.- const currency = "₹"; - const formatCurrency = (amount) => `${currency}${amount.toFixed(2)}`; + // Prefer shared formatter; adjust if you need to override the symbol per group. + // import { formatCurrency } from "../utils/currency"; // add this import at topAdd at top (imports section):
+import { formatCurrency } from "../utils/currency";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
⛔ Files ignored due to path filters (9)
frontend/assets/adaptive-icon.pngis excluded by!**/*.pngfrontend/assets/adaptive-icon/background.pngis excluded by!**/*.pngfrontend/assets/adaptive-icon/foreground.pngis excluded by!**/*.pngfrontend/assets/favicon.pngis excluded by!**/*.pngfrontend/assets/icon.pngis excluded by!**/*.pngfrontend/assets/pwa/chrome-icon/chrome-icon-144.pngis excluded by!**/*.pngfrontend/assets/pwa/chrome-icon/chrome-icon-192.pngis excluded by!**/*.pngfrontend/assets/pwa/chrome-icon/chrome-icon-512.pngis excluded by!**/*.pngfrontend/assets/splash.pngis excluded by!**/*.png
📒 Files selected for processing (9)
.github/workflows/bundle-analysis.yml(1 hunks).github/workflows/preview.yml(1 hunks).github/workflows/publish.yml(1 hunks).github/workflows/rn-bundle-analysis.yml(1 hunks).github/workflows/run-tests.yml(1 hunks)backend/app/expenses/routes.py(1 hunks)frontend/screens/AddExpenseScreen.js(6 hunks)frontend/screens/GroupDetailsScreen.js(5 hunks)frontend/screens/GroupSettingsScreen.js(5 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
backend/**/*.py
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Backend code must be implemented using FastAPI with Python 3.9+ and organized under the /backend/ directory.
Files:
backend/app/expenses/routes.py
backend/app/{auth,user,groups,expenses}/**/*.py
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
backend/app/{auth,user,groups,expenses}/**/*.py: Backend services should be modular, with authentication and user registration in app/auth/, user profile management in app/user/, group management in app/groups/, and expense tracking in app/expenses/.
When adding a new API endpoint, add the route to the appropriate service router file in the backend.
Files:
backend/app/expenses/routes.py
backend/app/expenses/**/*.py
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
backend/app/expenses/**/*.py: Support for different expense split types (equal, unequal, percentage-based) must be implemented in the expense tracking logic.
Settlements must track debt resolution between users in the expense tracking logic.
Files:
backend/app/expenses/routes.py
🧠 Learnings (1)
📚 Learning: 2025-07-26T09:41:01.332Z
Learnt from: CR
PR: Devasy23/splitwiser#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-26T09:41:01.332Z
Learning: Applies to backend/app/expenses/**/*.py : Support for different expense split types (equal, unequal, percentage-based) must be implemented in the expense tracking logic.
Applied to files:
frontend/screens/AddExpenseScreen.js
🧬 Code Graph Analysis (4)
backend/app/expenses/routes.py (3)
backend/app/auth/security.py (1)
get_current_user(118-137)backend/app/dependencies.py (1)
get_current_user(12-59)backend/app/expenses/service.py (3)
create_expense(41-115)get_expense_by_id(243-290)update_expense(292-451)
frontend/screens/GroupDetailsScreen.js (4)
frontend/screens/GroupSettingsScreen.js (3)
opacity(52-52)animatedStyle(54-58)styles(381-456)frontend/styles/theme.js (6)
spacing(36-42)spacing(36-42)colors(1-11)colors(1-11)typography(13-34)typography(13-34)frontend/utils/currency.js (2)
formatCurrency(8-16)formatCurrency(8-16)frontend/components/SkeletonLoader.js (1)
SkeletonLoader(12-31)
frontend/screens/GroupSettingsScreen.js (3)
frontend/screens/GroupDetailsScreen.js (6)
route(28-28)useContext(29-29)members(30-30)opacity(35-35)animatedStyle(37-41)styles(273-402)frontend/context/AuthContext.js (3)
AuthContext(10-10)AuthContext(10-10)user(13-13)frontend/components/SkeletonLoader.js (1)
SkeletonLoader(12-31)
frontend/screens/AddExpenseScreen.js (4)
frontend/screens/GroupDetailsScreen.js (7)
styles(273-402)route(28-28)useContext(29-29)members(30-30)isLoading(33-33)opacity(35-35)animatedStyle(37-41)frontend/context/AuthContext.js (3)
AuthContext(10-10)AuthContext(10-10)isLoading(16-16)frontend/api/groups.js (2)
getGroupMembers(15-16)getGroupMembers(15-16)frontend/components/SkeletonLoader.js (1)
SkeletonLoader(12-31)
🪛 YAMLlint (1.37.1)
.github/workflows/rn-bundle-analysis.yml
[warning] 18-18: wrong indentation: expected 6 but found 4
(indentation)
.github/workflows/bundle-analysis.yml
[warning] 18-18: wrong indentation: expected 6 but found 4
(indentation)
🪛 Ruff (0.12.2)
backend/app/expenses/routes.py
470-470: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🪛 GitHub Check: CodeQL
backend/app/expenses/routes.py
[warning] 511-511: Information exposure through an exception
Stack trace information flows to this location and may be exposed to an external user.
⏰ 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). (3)
- GitHub Check: preview
- GitHub Check: bundle-analysis
- GitHub Check: preview
🔇 Additional comments (2)
frontend/screens/GroupDetailsScreen.js (1)
96-113: LGTM on per-expense balance copy and color logic.Clear, user-friendly messages with appropriate color cues. Works for payer/non-payer and settled cases.
frontend/screens/AddExpenseScreen.js (1)
367-379: UI parity check: supported split modes now have inputs – good.Inputs for equal (checkboxes), exact, percentage, and shares are restored and consistent with validation paths.
| pull_request: | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Switch to pull_request improves security; gate secrets-dependent steps for forks
Running on pull_request prevents secret exfiltration from forks, but EXPO_TOKEN won’t be available on forked PRs, causing preview creation to fail. Either skip previews for forks or conditionally run steps only when secrets are accessible.
Two common patterns:
- Job-level gating (skip on forks):
jobs:
preview:
if: ${{ github.event.pull_request.head.repo.fork == false }}- Step-level gating (only run EAS steps when token is present):
- name: Setup EAS
if: ${{ secrets.EXPO_TOKEN != '' }}
uses: expo/expo-github-action@v8
with:
eas-version: latest
token: ${{ secrets.EXPO_TOKEN }}
- name: Create preview
if: ${{ secrets.EXPO_TOKEN != '' }}
uses: expo/expo-github-action/preview@v8
with:
command: eas update --auto --branch ${{ github.event.pull_request.head.ref }}
working-directory: ./frontendConfirm your intended behavior for forked PRs (skip vs. run without preview), and I can provide a concrete patch.
Gate preview steps for forked PRs (prevent failing previews / secret exposure)
pull_request workflows don't receive secrets from forked repos — EXPO_TOKEN will be empty on forked PRs, so preview creation will fail. Either skip previews for forks or conditionally run steps that require the token.
Files to update:
- .github/workflows/preview.yml — lines ~4-5 (trigger: pull_request)
Two common fixes:
- Job-level gating (skip the preview job on forks)
jobs:
preview:
if: ${{ github.event.pull_request.head.repo.fork == false }}
# ...- Step-level gating (only run EAS steps when token is present)
- name: Setup EAS
if: ${{ secrets.EXPO_TOKEN != '' }}
uses: expo/expo-github-action@v8
with:
eas-version: latest
token: ${{ secrets.EXPO_TOKEN }}
- name: Create preview
if: ${{ secrets.EXPO_TOKEN != '' }}
uses: expo/expo-github-action/preview@v8
with:
command: eas update --auto --branch ${{ github.event.pull_request.head.ref }}
working-directory: ./frontendConfirm whether you want to skip previews for forked PRs or allow PRs without previews and I will provide a concrete patch.
🤖 Prompt for AI Agents
In .github/workflows/preview.yml around lines 4-5 (the pull_request trigger),
the workflow will run for forked PRs where secrets.EXPO_TOKEN is unavailable
causing preview steps to fail or expose behavior; update the workflow to either
(A) gate the entire preview job by adding an if: ${{
github.event.pull_request.head.repo.fork == false }} on the preview job so it
skips forks, or (B) gate only the token-dependent steps by adding if: ${{
secrets.EXPO_TOKEN != '' }} to the EAS setup and preview/create steps so the job
runs but token-requiring steps are skipped when EXPO_TOKEN is empty.
| @router.get("/expenses/{expense_id}/debug") | ||
| async def debug_expense( | ||
| group_id: str, | ||
| expense_id: str, | ||
| current_user: Dict[str, Any] = Depends(get_current_user), | ||
| ): | ||
| """Debug endpoint to check expense details and user permissions""" | ||
| try: | ||
| from app.database import mongodb | ||
| from bson import ObjectId | ||
|
|
||
| # Check if expense exists | ||
| expense = await mongodb.database.expenses.find_one( | ||
| {"_id": ObjectId(expense_id)} | ||
| ) | ||
| if not expense: | ||
| return {"error": "Expense not found", "expense_id": expense_id} | ||
|
|
||
| # Check group membership | ||
| group = await mongodb.database.groups.find_one( | ||
| {"_id": ObjectId(group_id), "members.userId": current_user["_id"]} | ||
| ) | ||
|
|
||
| # Check if user created the expense | ||
| user_created = expense.get("createdBy") == current_user["_id"] | ||
|
|
||
| return { | ||
| "expense_exists": True, | ||
| "expense_id": expense_id, | ||
| "group_id": group_id, | ||
| "user_id": current_user["_id"], | ||
| "expense_created_by": expense.get("createdBy"), | ||
| "user_created_expense": user_created, | ||
| "user_in_group": group is not None, | ||
| "expense_group_id": expense.get("groupId"), | ||
| "group_id_match": expense.get("groupId") == group_id, | ||
| "expense_data": { | ||
| "description": expense.get("description"), | ||
| "amount": expense.get("amount"), | ||
| "splits_count": len(expense.get("splits", [])), | ||
| "created_at": expense.get("createdAt"), | ||
| "updated_at": expense.get("updatedAt"), | ||
| }, | ||
| } | ||
| except Exception as e: | ||
| return {"error": str(e), "type": type(e).__name__} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden the debug endpoint: prevent info disclosure, validate IDs, enforce group scoping, and hide from schema
- Line 511: Returning raw exception messages and types leaks internal details. Use HTTPException with a generic message and log the error instead.
- Lines 477-483: Expense is fetched before verifying group membership and without scoping to the group, enabling existence probing across groups.
- Lines 474-476: Import missing for bson.errors to handle invalid ObjectIds.
- Line 470: Ruff B008 flags Depends in default arg; for FastAPI, suppress locally.
- Hide the endpoint from the OpenAPI schema to reduce accidental exposure.
Apply the following patch:
-@router.get("/expenses/{expense_id}/debug")
+@router.get("/expenses/{expense_id}/debug", include_in_schema=False)
async def debug_expense(
group_id: str,
expense_id: str,
- current_user: Dict[str, Any] = Depends(get_current_user),
+ current_user: Dict[str, Any] = Depends(get_current_user), # noqa: B008
):
"""Debug endpoint to check expense details and user permissions"""
try:
- from app.database import mongodb
- from bson import ObjectId
-
- # Check if expense exists
- expense = await mongodb.database.expenses.find_one(
- {"_id": ObjectId(expense_id)}
- )
- if not expense:
- return {"error": "Expense not found", "expense_id": expense_id}
-
- # Check group membership
- group = await mongodb.database.groups.find_one(
- {"_id": ObjectId(group_id), "members.userId": current_user["_id"]}
- )
-
- # Check if user created the expense
- user_created = expense.get("createdBy") == current_user["_id"]
-
- return {
- "expense_exists": True,
- "expense_id": expense_id,
- "group_id": group_id,
- "user_id": current_user["_id"],
- "expense_created_by": expense.get("createdBy"),
- "user_created_expense": user_created,
- "user_in_group": group is not None,
- "expense_group_id": expense.get("groupId"),
- "group_id_match": expense.get("groupId") == group_id,
- "expense_data": {
- "description": expense.get("description"),
- "amount": expense.get("amount"),
- "splits_count": len(expense.get("splits", [])),
- "created_at": expense.get("createdAt"),
- "updated_at": expense.get("UpdatedAt"),
- },
- }
- except Exception as e:
- return {"error": str(e), "type": type(e).__name__}
+ from app.database import mongodb
+ from bson import ObjectId, errors
+
+ # Validate IDs
+ try:
+ group_obj_id = ObjectId(group_id)
+ expense_obj_id = ObjectId(expense_id)
+ except errors.InvalidId:
+ raise HTTPException(status_code=400, detail="Invalid group ID or expense ID")
+
+ # Verify group membership first
+ group = await mongodb.database.groups.find_one(
+ {"_id": group_obj_id, "members.userId": current_user["_id"]}
+ )
+ if not group:
+ raise HTTPException(status_code=403, detail="You are not a member of this group")
+
+ # Fetch expense within the same group
+ expense = await mongodb.database.expenses.find_one(
+ {"_id": expense_obj_id, "groupId": group_id}
+ )
+ if not expense:
+ raise HTTPException(status_code=404, detail="Expense not found")
+
+ user_created = expense.get("createdBy") == current_user["_id"]
+
+ return {
+ "expenseExists": True,
+ "expenseId": expense_id,
+ "groupId": group_id,
+ "userId": current_user["_id"],
+ "expenseCreatedBy": expense.get("createdBy"),
+ "userCreatedExpense": user_created,
+ "userInGroup": True,
+ "expenseGroupId": expense.get("groupId"),
+ "groupIdMatch": True,
+ "expenseData": {
+ "description": expense.get("description"),
+ "amount": expense.get("amount"),
+ "splitsCount": len(expense.get("splits", [])),
+ "createdAt": expense.get("createdAt"),
+ "updatedAt": expense.get("updatedAt"),
+ },
+ }
+ except HTTPException:
+ raise
+ except Exception:
+ logger.exception("Unhandled error in debug_expense", exc_info=True)
+ raise HTTPException(status_code=500, detail="Debug inspection failed")📝 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.
| @router.get("/expenses/{expense_id}/debug") | |
| async def debug_expense( | |
| group_id: str, | |
| expense_id: str, | |
| current_user: Dict[str, Any] = Depends(get_current_user), | |
| ): | |
| """Debug endpoint to check expense details and user permissions""" | |
| try: | |
| from app.database import mongodb | |
| from bson import ObjectId | |
| # Check if expense exists | |
| expense = await mongodb.database.expenses.find_one( | |
| {"_id": ObjectId(expense_id)} | |
| ) | |
| if not expense: | |
| return {"error": "Expense not found", "expense_id": expense_id} | |
| # Check group membership | |
| group = await mongodb.database.groups.find_one( | |
| {"_id": ObjectId(group_id), "members.userId": current_user["_id"]} | |
| ) | |
| # Check if user created the expense | |
| user_created = expense.get("createdBy") == current_user["_id"] | |
| return { | |
| "expense_exists": True, | |
| "expense_id": expense_id, | |
| "group_id": group_id, | |
| "user_id": current_user["_id"], | |
| "expense_created_by": expense.get("createdBy"), | |
| "user_created_expense": user_created, | |
| "user_in_group": group is not None, | |
| "expense_group_id": expense.get("groupId"), | |
| "group_id_match": expense.get("groupId") == group_id, | |
| "expense_data": { | |
| "description": expense.get("description"), | |
| "amount": expense.get("amount"), | |
| "splits_count": len(expense.get("splits", [])), | |
| "created_at": expense.get("createdAt"), | |
| "updated_at": expense.get("updatedAt"), | |
| }, | |
| } | |
| except Exception as e: | |
| return {"error": str(e), "type": type(e).__name__} | |
| @router.get("/expenses/{expense_id}/debug", include_in_schema=False) | |
| async def debug_expense( | |
| group_id: str, | |
| expense_id: str, | |
| current_user: Dict[str, Any] = Depends(get_current_user), # noqa: B008 | |
| ): | |
| """Debug endpoint to check expense details and user permissions""" | |
| try: | |
| from app.database import mongodb | |
| from bson import ObjectId, errors | |
| # Validate IDs | |
| try: | |
| group_obj_id = ObjectId(group_id) | |
| expense_obj_id = ObjectId(expense_id) | |
| except errors.InvalidId: | |
| raise HTTPException(status_code=400, detail="Invalid group ID or expense ID") | |
| # Verify group membership first | |
| group = await mongodb.database.groups.find_one( | |
| {"_id": group_obj_id, "members.userId": current_user["_id"]} | |
| ) | |
| if not group: | |
| raise HTTPException(status_code=403, detail="You are not a member of this group") | |
| # Fetch expense within the same group | |
| expense = await mongodb.database.expenses.find_one( | |
| {"_id": expense_obj_id, "groupId": group_id} | |
| ) | |
| if not expense: | |
| raise HTTPException(status_code=404, detail="Expense not found") | |
| user_created = expense.get("createdBy") == current_user["_id"] | |
| return { | |
| "expenseExists": True, | |
| "expenseId": expense_id, | |
| "groupId": group_id, | |
| "userId": current_user["_id"], | |
| "expenseCreatedBy": expense.get("createdBy"), | |
| "userCreatedExpense": user_created, | |
| "userInGroup": True, | |
| "expenseGroupId": expense.get("groupId"), | |
| "groupIdMatch": True, | |
| "expenseData": { | |
| "description": expense.get("description"), | |
| "amount": expense.get("amount"), | |
| "splitsCount": len(expense.get("splits", [])), | |
| "createdAt": expense.get("createdAt"), | |
| "updatedAt": expense.get("updatedAt"), | |
| }, | |
| } | |
| except HTTPException: | |
| raise | |
| except Exception: | |
| logger.exception("Unhandled error in debug_expense", exc_info=True) | |
| raise HTTPException(status_code=500, detail="Debug inspection failed") |
🧰 Tools
🪛 Ruff (0.12.2)
470-470: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🪛 GitHub Check: CodeQL
[warning] 511-511: Information exposure through an exception
Stack trace information flows to this location and may be exposed to an external user.
| const total = Object.values(exactAmounts).reduce( | ||
| (sum, val) => sum + parseFloat(val || "0"), | ||
| 0 | ||
| ); | ||
| if (Math.abs(total - numericAmount) > 0.01) { | ||
| throw new Error( | ||
| `The exact amounts must add up to ${numericAmount.toFixed( | ||
| 2 | ||
| )}. Current total: ${total.toFixed(2)}` | ||
| ); | ||
| } | ||
| if (Math.abs(total - numericAmount) > 0.01) | ||
| throw new Error("Exact amounts must add up to the total."); | ||
| splits = Object.entries(exactAmounts) | ||
| .filter(([userId, value]) => parseFloat(value || "0") > 0) | ||
| .filter(([, value]) => parseFloat(value || "0") > 0) | ||
| .map(([userId, value]) => ({ | ||
| userId, | ||
| amount: Math.round(parseFloat(value) * 100) / 100, | ||
| amount: parseFloat(value), | ||
| type: "unequal", | ||
| })); |
There was a problem hiding this comment.
Exact split: sanitize inputs; robust total check; avoid NaN propagation.
parseFloat on arbitrary strings yields NaN and the current logic can silently pass validation. Sanitize values and compare totals in cents.
- } else if (splitMethod === "exact") {
- const total = Object.values(exactAmounts).reduce(
- (sum, val) => sum + parseFloat(val || "0"),
- 0
- );
- if (Math.abs(total - numericAmount) > 0.01)
- throw new Error("Exact amounts must add up to the total.");
- splits = Object.entries(exactAmounts)
- .filter(([, value]) => parseFloat(value || "0") > 0)
- .map(([userId, value]) => ({
- userId,
- amount: parseFloat(value),
- type: "unequal",
- }));
+ } else if (splitMethod === "exact") {
+ const total = Object.values(exactAmounts).reduce(
+ (sum, val) => sum + toNum(val),
+ 0
+ );
+ if (Math.round(total * 100) !== Math.round(numericAmount * 100)) {
+ throw new Error("Exact amounts must add up to the total.");
+ }
+ splits = Object.entries(exactAmounts)
+ .map(([userId, value]) => ({ userId, amount: toNum(value) }))
+ .filter(({ amount }) => amount > 0)
+ .map(({ userId, amount }) => ({
+ userId,
+ amount: Math.round(amount * 100) / 100,
+ type: "unequal",
+ }));Add this helper near the top of the file (outside the selected range):
// Helper to safely parse numeric input; treats invalid values as 0
const toNum = (v) => {
const n = parseFloat(String(v).trim());
return Number.isFinite(n) ? n : 0;
};🤖 Prompt for AI Agents
In frontend/screens/AddExpenseScreen.js around lines 171 to 183, the exact-split
logic uses parseFloat directly which can produce NaN and allow incorrect
validation; add a safe parser helper near the top of the file (outside this
range) that trims input, parses to float and returns 0 for invalid values, then
update this block to use that helper (e.g. toNum) when computing each value,
compute totals in integer cents (multiply by 100 and round) and compare cents
for exact equality (or within 1 cent if you prefer tolerance), and when building
splits use the safe parsed number and filter for amounts > 0 to avoid NaN
propagation.
| const totalPercentage = Object.values(percentages).reduce( | ||
| (sum, val) => sum + parseFloat(val || "0"), | ||
| 0 | ||
| ); | ||
| if (Math.abs(total - 100) > 0.01) { | ||
| throw new Error( | ||
| `Percentages must add up to 100%. Current total: ${total.toFixed( | ||
| 2 | ||
| )}%` | ||
| ); | ||
| if (Math.abs(totalPercentage - 100) > 0.01) { | ||
| throw new Error("Percentages must add up to 100."); | ||
| } | ||
| splits = Object.entries(percentages) | ||
| .filter(([userId, value]) => parseFloat(value || "0") > 0) | ||
| .filter(([, value]) => parseFloat(value || "0") > 0) | ||
| .map(([userId, value]) => ({ | ||
| userId, | ||
| amount: | ||
| Math.round(numericAmount * (parseFloat(value) / 100) * 100) / 100, | ||
| amount: (numericAmount * parseFloat(value)) / 100, | ||
| type: "percentage", | ||
| })); |
There was a problem hiding this comment.
Percentage split: handle invalid inputs; enforce total = 100; round outputs.
Un-sanitized parseFloat can produce NaN, creating invalid payloads.
- } else if (splitMethod === "percentage") {
- const totalPercentage = Object.values(percentages).reduce(
- (sum, val) => sum + parseFloat(val || "0"),
- 0
- );
- if (Math.abs(totalPercentage - 100) > 0.01) {
- throw new Error("Percentages must add up to 100.");
- }
- splits = Object.entries(percentages)
- .filter(([, value]) => parseFloat(value || "0") > 0)
- .map(([userId, value]) => ({
- userId,
- amount: (numericAmount * parseFloat(value)) / 100,
- type: "percentage",
- }));
+ } else if (splitMethod === "percentage") {
+ const totalPercentage = Object.values(percentages).reduce(
+ (sum, val) => sum + toNum(val),
+ 0
+ );
+ if (!Number.isFinite(totalPercentage) || Math.abs(totalPercentage - 100) > 0.01) {
+ throw new Error("Percentages must add up to 100.");
+ }
+ splits = Object.entries(percentages)
+ .map(([userId, value]) => ({ userId, pct: toNum(value) }))
+ .filter(({ pct }) => pct > 0)
+ .map(({ userId, pct }) => ({
+ userId,
+ amount: Math.round((numericAmount * pct) ) / 100, // pct is percentage (e.g., 25)
+ type: "percentage",
+ }));📝 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 totalPercentage = Object.values(percentages).reduce( | |
| (sum, val) => sum + parseFloat(val || "0"), | |
| 0 | |
| ); | |
| if (Math.abs(total - 100) > 0.01) { | |
| throw new Error( | |
| `Percentages must add up to 100%. Current total: ${total.toFixed( | |
| 2 | |
| )}%` | |
| ); | |
| if (Math.abs(totalPercentage - 100) > 0.01) { | |
| throw new Error("Percentages must add up to 100."); | |
| } | |
| splits = Object.entries(percentages) | |
| .filter(([userId, value]) => parseFloat(value || "0") > 0) | |
| .filter(([, value]) => parseFloat(value || "0") > 0) | |
| .map(([userId, value]) => ({ | |
| userId, | |
| amount: | |
| Math.round(numericAmount * (parseFloat(value) / 100) * 100) / 100, | |
| amount: (numericAmount * parseFloat(value)) / 100, | |
| type: "percentage", | |
| })); | |
| const totalPercentage = Object.values(percentages).reduce( | |
| (sum, val) => sum + toNum(val), | |
| 0 | |
| ); | |
| if (!Number.isFinite(totalPercentage) || Math.abs(totalPercentage - 100) > 0.01) { | |
| throw new Error("Percentages must add up to 100."); | |
| } | |
| splits = Object.entries(percentages) | |
| .map(([userId, value]) => ({ userId, pct: toNum(value) })) | |
| .filter(({ pct }) => pct > 0) | |
| .map(({ userId, pct }) => ({ | |
| userId, | |
| amount: Math.round((numericAmount * pct)) / 100, // pct is percentage (e.g., 25) | |
| type: "percentage", | |
| })); |
| const expenseData = { | ||
| description, | ||
| amount: numericAmount, | ||
| paidBy: payerId, // Use the selected payer | ||
| splitType, | ||
| paidBy: payerId, | ||
| splitType: splitMethod, | ||
| splits, | ||
| tags: [], | ||
| }; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify backend contract for splitType values and “shares” support.
The payload uses splitType: splitMethod with possible values equal, exact, percentage, shares. Ensure the backend expects these exact strings (some backends use “unequal” instead of “exact”) and that “shares” is supported.
Run this script to find accepted values/enums on the backend:
🏁 Script executed:
#!/usr/bin/env bash
# Search the repo for splitType handling and allowed values
rg -n "splitType" -A 4
rg -n "shares" -A 3
rg -n "unequal" -A 3Length of output: 118811
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "==== AddExpenseScreen.js (relevant sections) ===="
sed -n '1,420p' frontend/screens/AddExpenseScreen.js || true
echo
echo "==== backend SplitType enum and schemas.py ===="
sed -n '1,220p' backend/app/expenses/schemas.py || true
echo
echo "==== openapi SplitType enum in openapi.json (snippet) ===="
rg -n '"SplitType"' -n openapi.json || true
rg -n '"enum":\s*\[' openapi.json -n || true
jq '.components.schemas.SplitType' openapi.json | sed -n '1,120p' || trueLength of output: 20327
Action required — frontend must send backend-accepted splitType values ("equal", "unequal", "percentage").
Backend confirmation: backend/app/expenses/schemas.py defines SplitType = "equal" | "unequal" | "percentage". The frontend currently sends "exact" and "shares", which will fail validation.
Files to update:
- frontend/screens/AddExpenseScreen.js
- Map splitMethod "exact" and "shares" → backend "unequal" for payload.splitType.
- Ensure splits[].type uses "unequal" for both exact and shares (the shares branch currently uses "shares").
Suggested minimal changes (replace at handleAddExpense before sending payload):
Current:
const expenseData = {
description,
amount: numericAmount,
paidBy: payerId,
splitType: splitMethod,
splits,
};
Replace with:
const backendSplitType =
splitMethod === "exact" || splitMethod === "shares" ? "unequal" : splitMethod;
const expenseData = {
description,
amount: numericAmount,
paidBy: payerId,
splitType: backendSplitType,
splits,
};
Also change the "shares" branch mapping from type: "shares" to type: "unequal":
.map(([userId, value]) => ({
userId,
amount: (numericAmount * parseFloat(value)) / totalShares,
type: "unequal",
}));
Reason: backend schema and tests validate splitType and ExpenseSplit.type against SplitType enum; sending "exact" or "shares" will trigger validation errors. Adjust frontend mapping or extend backend enum (if you prefer adding explicit "shares"/"exact" semantics) — choose one approach and implement consistently.
🤖 Prompt for AI Agents
In frontend/screens/AddExpenseScreen.js around lines 215 to 221, the frontend
sends splitType values like "exact" and "shares" (and sets splits[].type to
"shares") which the backend rejects; change the payload to map "exact" and
"shares" → "unequal" for expenseData.splitType and ensure all splits created for
those branches use type "unequal" instead of "shares"; update the mapping logic
right before sending (compute backendSplitType = splitMethod === "exact" ||
splitMethod === "shares" ? "unequal" : splitMethod and use that in expenseData)
and change the shares-branch .map(...) to produce type: "unequal" for each
split.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
Design Revamp.html (8)
14-14: Comment contradicts code: SVG is used for the loading spinnerThe “NO SVG” statement conflicts with the inline SVG spinner used later.
If the spinner is intended, clarify the comment:
- <!-- CONFIRMATION: NO SVG graphics used. NO Mermaid JS used. --> + <!-- NOTE: Inline SVG used for the loading spinner only. NO Mermaid JS used. -->Alternatively, remove SVG usage by switching to a pure-CSS spinner (see loading button comment for a CSS-only variant).
7-7: Pin Tailwind CDN version to avoid unexpected breaking changesUnpinned CDN can introduce silent style changes. Pin to a specific version (e.g., 3.4.10) for stability.
- <script src="https://cdn.tailwindcss.com"></script> + <script src="https://cdn.tailwindcss.com/3.4.10"></script>
119-119: Add reduced-motion support and anchor offset for fixed headerImprove accessibility for motion-sensitive users and ensure anchors aren’t hidden behind the fixed header.
- </style> + /* Offset in-page anchors to account for fixed header */ + section { scroll-margin-top: 5rem; } + + /* Respect users' reduced motion preferences */ + @media (prefers-reduced-motion: reduce) { + html { scroll-behavior: auto; } + .animated-element, + .component-showcase button, + .component-showcase input { + transition: none !important; + } + .animate-spin { + animation: none !important; + } + } + </style>
269-277: Improve loading button accessibility and behavior (ARIA + JS toggle)Make the loading state announced to assistive tech and prevent double submissions. Also set explicit button type.
- <button id="loading-btn" class="btn-primary px-6 py-3 rounded-lg font-semibold w-36 flex items-center justify-center"> - <span class="btn-text">Click for loading</span> - <span class="btn-loader hidden"> + <button id="loading-btn" class="btn-primary px-6 py-3 rounded-lg font-semibold w-36 flex items-center justify-center" type="button" aria-busy="false" aria-controls="loading-btn-status"> + <span class="btn-text" aria-hidden="false">Click for loading</span> + <span id="loading-btn-status" class="btn-loader hidden" role="status" aria-live="polite" aria-hidden="true" aria-label="Loading"> <svg class="animate-spin h-5 w-5 text-white" xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24"> <circle class="opacity-25" cx="12" cy="12" r="10" stroke="currentColor" stroke-width="4"></circle> <path class="opacity-75" fill="currentColor" d="M4 12a8 8 0 018-8V0C5.373 0 0 5.373 0 12h4zm2 5.291A7.962 7.962 0 014 12H0c0 3.042 1.135 5.824 3 7.938l3-2.647z"></path> </svg> </span> </button>Add this lightweight script near the end of the body to toggle the loading state:
<script> (function () { const btn = document.getElementById('loading-btn'); if (!btn) return; const text = btn.querySelector('.btn-text'); const loader = btn.querySelector('.btn-loader'); btn.addEventListener('click', () => { const loading = btn.getAttribute('aria-busy') === 'true'; btn.setAttribute('aria-busy', String(!loading)); btn.toggleAttribute('disabled'); if (text) text.setAttribute('aria-hidden', String(!loading)); if (loader) { loader.classList.toggle('hidden', loading); loader.setAttribute('aria-hidden', String(loading)); } }); })(); </script>If you prefer to avoid SVG altogether, replace the inner loader span with a CSS-only spinner:
<span id="loading-btn-status" class="btn-loader hidden" role="status" aria-live="polite" aria-hidden="true" aria-label="Loading"> <span class="inline-block h-5 w-5 border-2 border-white/30 border-t-white rounded-full animate-spin"></span> </span>
293-295: Mark the error input as invalid and link help text for screen readersExpose validation status via ARIA and connect the hint to the field.
- <input type="email" id="error" class="input-field error w-full p-3 rounded-lg" value="invalid-email"> - <p class="text-sm mt-1" style="color: var(--color-semantic-error);">Please enter a valid email address.</p> + <input type="email" id="error" class="input-field error w-full p-3 rounded-lg" value="invalid-email" aria-invalid="true" aria-describedby="error-help"> + <p id="error-help" class="text-sm mt-1" style="color: var(--color-semantic-error);">Please enter a valid email address.</p>
214-217: Make color swatches accessible with labelsColor-only divs are invisible to screen readers. Add roles and labels.
- <div class="w-full h-16 rounded-lg" style="background-color: #8B5CF6;"></div> - <div class="w-full h-16 rounded-lg" style="background-color: #22D3EE;"></div> - <div class="w-full h-16 rounded-lg" style="background-color: #E52A6F;"></div> + <div class="w-full h-16 rounded-lg" role="img" aria-label="Accent Purple #8B5CF6" style="background-color: #8B5CF6;"></div> + <div class="w-full h-16 rounded-lg" role="img" aria-label="Accent Cyan #22D3EE" style="background-color: #22D3EE;"></div> + <div class="w-full h-16 rounded-lg" role="img" aria-label="Accent Pink #E52A6F" style="background-color: #E52A6F;"></div>- <div class="w-full h-16 rounded-lg" style="background-color: #F9FAFB; border: 1px solid #E5E7EB;"></div> - <div class="w-full h-16 rounded-lg" style="background-color: #E5E7EB;"></div> - <div class="w-full h-16 rounded-lg" style="background-color: #6B7280;"></div> - <div class="w-full h-16 rounded-lg" style="background-color: #1F2937;"></div> + <div class="w-full h-16 rounded-lg" role="img" aria-label="Neutral 50 #F9FAFB" style="background-color: #F9FAFB; border: 1px solid #E5E7EB;"></div> + <div class="w-full h-16 rounded-lg" role="img" aria-label="Neutral 200 #E5E7EB" style="background-color: #E5E7EB;"></div> + <div class="w-full h-16 rounded-lg" role="img" aria-label="Neutral 500 #6B7280" style="background-color: #6B7280;"></div> + <div class="w-full h-16 rounded-lg" role="img" aria-label="Neutral 800 #1F2937" style="background-color: #1F2937;"></div>- <div class="w-full h-16 rounded-lg" style="background-color: var(--color-semantic-success);"></div> - <div class="w-full h-16 rounded-lg" style="background-color: #F59E0B;"></div> - <div class="w-full h-16 rounded-lg" style="background-color: var(--color-semantic-error);"></div> + <div class="w-full h-16 rounded-lg" role="img" aria-label="Success Green" style="background-color: var(--color-semantic-success);"></div> + <div class="w-full h-16 rounded-lg" role="img" aria-label="Warning Amber #F59E0B" style="background-color: #F59E0B;"></div> + <div class="w-full h-16 rounded-lg" role="img" aria-label="Error Red" style="background-color: var(--color-semantic-error);"></div>Also applies to: 222-226, 231-233
137-137: Add a “skip to content” target to support keyboard usersProvide a skip link target on main to quickly bypass the fixed header/nav.
- <main class="pt-20"> + <main id="main" class="pt-20">Then place this skip link just after the opening body tag (outside shown lines):
<a href="#main" class="sr-only focus:not-sr-only focus:fixed focus:top-2 focus:left-2 focus:z-50 bg-white text-gray-900 px-3 py-2 rounded shadow"> Skip to content </a>
32-37: Glassmorphism fallback: ensure graceful degradation where backdrop-filter isn’t supportedSome browsers don’t support backdrop-filter. You already have a semi-transparent background; optionally add a subtle opaque fallback class on no-support.
Add this progressive enhancement inside your style block (no changes to existing markup required):
@supports not ((-webkit-backdrop-filter: blur(24px)) or (backdrop-filter: blur(24px))) { .glassmorphic { background: rgba(255, 255, 255, 0.85); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
Design Revamp.html(1 hunks)
⏰ 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). (3)
- GitHub Check: preview
- GitHub Check: bundle-analysis
- GitHub Check: preview
Summary by CodeRabbit
New Features
UI Improvements
Bug Fixes
Chores