-
Notifications
You must be signed in to change notification settings - Fork 111
Fix 152Added edit profile feature #154
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Fix 152Added edit profile feature #154
Conversation
📝 WalkthroughWalkthroughAdds an EditProfile UI and navigation entry, extends backend profile endpoints and cached profile snapshot with additional fields (lmp, cycle_length, period_length, age, weight), and changes multiple frontend screens to refresh data on screen focus instead of only on mount. Changes
Sequence DiagramsequenceDiagram
participant User
participant EditProfileScreen
participant Backend
participant Database
participant Cache
participant ProfileScreen
User->>EditProfileScreen: Open edit screen
EditProfileScreen->>Backend: GET /get_profile
Backend->>Database: SELECT profile
Database-->>Backend: profile row
Backend-->>EditProfileScreen: profile JSON (includes new fields)
EditProfileScreen->>EditProfileScreen: Populate form
User->>EditProfileScreen: Save changes
EditProfileScreen->>Backend: PUT /update_profile (payload)
Backend->>Backend: calculate_due_date(lmp, cycle_length)
Backend->>Database: UPDATE profile
Database-->>Backend: update result
Backend->>Cache: sync updated profile
Backend-->>EditProfileScreen: 200 OK + updated profile
EditProfileScreen->>ProfileScreen: Navigate back (focus)
ProfileScreen->>Backend: GET /get_profile (on focus)
Backend-->>ProfileScreen: refreshed profile
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Backend/routes/profile.py (1)
10-15: Add error handling for invalid date format.
calculate_due_datewill raise an unhandledValueErroriflmp_stris not in%Y-%m-%dformat. This could crash the endpoint.🔎 Proposed fix
def calculate_due_date(lmp_str, cycle_length): + try: + lmp_date = datetime.strptime(lmp_str, "%Y-%m-%d") + except (ValueError, TypeError): + raise ValueError("Invalid LMP date format. Expected YYYY-MM-DD") - lmp_date = datetime.strptime(lmp_str, "%Y-%m-%d") # Standard: LMP + 280 days for 28-day cycle. Adjust if cycle differs adjustment = int(cycle_length) - 28 if cycle_length else 0 due_date = lmp_date + timedelta(days=280 + adjustment) return due_date.strftime("%Y-%m-%d")
🧹 Nitpick comments (7)
Frontend/src/Screens/HomeScreen.jsx (1)
30-34: Consider adding the fetch function to the dependency array.The empty dependency array in
useCallbackmay trigger ESLint warnings. SincefetchDatais a stable function, you can either:
- Add
fetchDatato the deps:useCallback(() => { fetchData(); }, [fetchData])- Or simplify by removing
useCallback:useFocusEffect(() => { fetchData(); })The current implementation works correctly, but addressing this would align with React Hook best practices and silence linter warnings.
🔎 Suggested simplification
- useFocusEffect( - useCallback(() => { - fetchData(); - }, []) - ); + useFocusEffect( + useCallback(() => { + fetchData(); + }, [fetchData]) + );Frontend/src/Screens/AllTasksScreen.jsx (1)
22-26: Consider including the fetch function in the dependency array.The empty dependency array in
useCallbackmay trigger ESLint exhaustive-deps warnings. Consider addingfetchTasksto the dependencies or simplifying the pattern.🔎 Suggested fix
- useFocusEffect( - useCallback(() => { - fetchTasks(); - }, []) - ); + useFocusEffect( + useCallback(() => { + fetchTasks(); + }, [fetchTasks]) + );Frontend/src/Screens/WeightScreen.jsx (1)
57-61: Consider including the fetch function in the dependency array.The empty dependency array in
useCallbackmay trigger ESLint exhaustive-deps warnings. AddingfetchWeightHistoryto the dependencies would align with React Hook best practices.🔎 Suggested fix
- useFocusEffect( - useCallback(() => { - fetchWeightHistory(); - }, []) - ); + useFocusEffect( + useCallback(() => { + fetchWeightHistory(); + }, [fetchWeightHistory]) + );Frontend/src/Screens/SymptomsScreen.jsx (1)
40-44: Consider including the fetch function in the dependency array.The empty dependency array in
useCallbackmay trigger ESLint exhaustive-deps warnings. AddingfetchSymptomsHistoryto the dependencies would align with React Hook best practices.🔎 Suggested fix
- useFocusEffect( - useCallback(() => { - fetchSymptomsHistory(); - }, []) - ); + useFocusEffect( + useCallback(() => { + fetchSymptomsHistory(); + }, [fetchSymptomsHistory]) + );Frontend/src/Screens/MedicineScreen.jsx (1)
38-42: Consider including the fetch function in the dependency array.The empty dependency array in
useCallbackmay trigger ESLint exhaustive-deps warnings. AddingfetchMedicineHistoryto the dependencies would align with React Hook best practices.🔎 Suggested fix
- useFocusEffect( - useCallback(() => { - fetchMedicineHistory(); - }, []) - ); + useFocusEffect( + useCallback(() => { + fetchMedicineHistory(); + }, [fetchMedicineHistory]) + );Backend/routes/profile.py (1)
62-70: Fragile index-based tuple access—consider using column names or a dictionary cursor.Accessing profile data by index (e.g.,
profile[1],profile[2]) is brittle. If the database schema changes or columns are reordered, these indices will silently return wrong data.🔎 Proposed fix using row_factory
@profile_bp.route('/get_profile', methods=['GET']) def get_profile(): db = open_db() + db.row_factory = sqlite3.Row # Enable dict-like access try: profile = db.execute('SELECT * FROM profile').fetchone() if profile is None: return jsonify({"error": "Profile not found"}), 404 return jsonify({ - "lmp": profile[1], - "cycle_length": profile[2], - "period_length": profile[3], - "age": profile[4], - "weight": profile[5], - "location": profile[6], - "due_date": profile[7] + "lmp": profile["lmp"], + "cycle_length": profile["cycleLength"], + "period_length": profile["periodLength"], + "age": profile["age"], + "weight": profile["weight"], + "location": profile["user_location"], + "due_date": profile["dueDate"] }), 200Frontend/src/Screens/ProfileScreen.jsx (1)
1-1: Remove unuseduseEffectimport.After switching to
useFocusEffect,useEffectis no longer used in this component.🔎 Proposed fix
-import React, { useState, useEffect, useCallback } from 'react'; +import React, { useState, useCallback } from 'react';
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
Backend/cache/context_default.jsonBackend/routes/profile.pyFrontend/src/Screens/AllTasksScreen.jsxFrontend/src/Screens/BloodPressureScreen.jsxFrontend/src/Screens/EditProfileScreen.jsxFrontend/src/Screens/HomeScreen.jsxFrontend/src/Screens/MedicineScreen.jsxFrontend/src/Screens/ProfileScreen.jsxFrontend/src/Screens/SymptomsScreen.jsxFrontend/src/Screens/WeightScreen.jsxFrontend/src/navigation/StackNavigator.jsx
🧰 Additional context used
🧬 Code graph analysis (4)
Frontend/src/navigation/StackNavigator.jsx (1)
Frontend/src/Screens/EditProfileScreen.jsx (1)
EditProfileScreen(24-442)
Frontend/src/Screens/AllTasksScreen.jsx (6)
Frontend/src/Screens/BloodPressureScreen.jsx (1)
refreshing(23-23)Frontend/src/Screens/HomeScreen.jsx (2)
refreshing(23-23)currentWeek(22-22)Frontend/src/Screens/MedicineScreen.jsx (1)
refreshing(23-23)Frontend/src/Screens/ProfileScreen.jsx (1)
refreshing(39-39)Frontend/src/Screens/SymptomsScreen.jsx (1)
refreshing(21-21)Frontend/src/Screens/WeightScreen.jsx (1)
refreshing(21-21)
Backend/routes/profile.py (3)
Frontend/src/Screens/EditProfileScreen.jsx (2)
age(31-31)weight(32-32)Frontend/src/Screens/BasicDetailsScreen.jsx (2)
age(25-25)weight(26-26)Frontend/src/Screens/WeightScreen.jsx (1)
weight(18-18)
Frontend/src/Screens/ProfileScreen.jsx (1)
Frontend/src/Screens/EditProfileScreen.jsx (2)
loading(37-37)fetchProfileData(49-67)
🔇 Additional comments (8)
Backend/routes/profile.py (1)
97-101: Good addition: Pre-checking profile existence before update.The 404 response when profile doesn't exist is a proper improvement that provides clear feedback to clients.
Frontend/src/navigation/StackNavigator.jsx (1)
21-21: LGTM!The EditProfileScreen import and route registration follow the existing navigation patterns correctly.
Also applies to: 41-41
Frontend/src/Screens/BloodPressureScreen.jsx (1)
39-43: LGTM!The switch to
useFocusEffectwithuseCallbackensures blood pressure logs are refreshed whenever the screen gains focus. This is consistent with the data refresh pattern applied across other screens in this PR.Frontend/src/Screens/ProfileScreen.jsx (2)
41-45: LGTM!The
useFocusEffectpattern ensures profile data is refreshed when returning from the EditProfile screen, providing a seamless user experience.
133-143: LGTM!The new Pregnancy Details section and navigation to EditProfile are well-implemented, providing users with a clear view of their profile data and an intuitive edit flow.
Frontend/src/Screens/EditProfileScreen.jsx (2)
101-134: Good implementation of location detection with proper error handling.The geolocation flow correctly handles permissions, timeouts, and API errors. The User-Agent header for Nominatim API is properly included.
136-170: Thorough input validation.The validation logic covers all fields with appropriate range checks (cycle length 20-40, period 1-10, age 12-60, weight 30-200). Error messages are clear and user-friendly.
Backend/cache/context_default.json (1)
1-105: File is appropriately version-controlled as a test fixture.
Backend/cache/context_default.jsonis intentionally included in the repository as a test fixture for cache management testing. The test suite inBackend/tests/test_cache_management.pyexplicitly uses this file to monitor cache behavior. The filename already clarifies its purpose with the "default" designation, and this type of fixture data should remain in version control for reproducible testing.
Backend/routes/profile.py
Outdated
| except sqlite3.OperationalError as error: | ||
| return jsonify({"error": str(error)}), 500 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid exposing raw database error messages to clients.
Returning str(error) exposes internal database details (table names, column names, SQL syntax) which could aid attackers. Return a generic error message instead.
🔎 Proposed fix
except sqlite3.OperationalError as error:
- return jsonify({"error": str(error)}), 500
+ print(f"Database error in update_profile: {error}") # Log internally
+ return jsonify({"error": "Database error occurred"}), 500📝 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.
| except sqlite3.OperationalError as error: | |
| return jsonify({"error": str(error)}), 500 | |
| except sqlite3.OperationalError as error: | |
| print(f"Database error in update_profile: {error}") # Log internally | |
| return jsonify({"error": "Database error occurred"}), 500 |
🤖 Prompt for AI Agents
In Backend/routes/profile.py around lines 128-129, the except block currently
returns the raw sqlite3.OperationalError string to the client; change it to
return a generic error message (e.g., {"error": "Internal server error"}) with a
500 status to avoid leaking DB details, and log the full exception internally
(using app.logger.exception(...) or logging.exception(...)) so developers still
have the stack trace for debugging while clients only receive the generic
message.
|
|
||
| const data = await res.json(); | ||
|
|
||
| if (data.error) { | ||
| Alert.alert('Error', data.error); | ||
| } else { | ||
| Alert.alert('Success', 'Profile updated successfully', [ | ||
| { | ||
| text: 'OK', | ||
| onPress: () => navigation.goBack(), | ||
| }, | ||
| ]); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incomplete HTTP error handling.
The code parses JSON (line 187) and checks data.error (line 189), but doesn't verify res.ok first. If the server returns a 5xx error, res.json() might fail or return unexpected data.
🔎 Proposed fix
const res = await fetch(`${BASE_URL}/update_profile`, {
method: 'PUT',
headers: {'Content-Type': 'application/json'},
body: JSON.stringify({
location: country,
lmp: lmpDate,
cycleLength: cycleLength,
periodLength: periodLength,
age: Number(age),
weight: Number(weight),
}),
});
+ if (!res.ok) {
+ const errorData = await res.json().catch(() => ({}));
+ Alert.alert('Error', errorData.error || `Server error: ${res.status}`);
+ return;
+ }
+
const data = await res.json();
- if (data.error) {
- Alert.alert('Error', data.error);
- } else {
+ Alert.alert('Success', 'Profile updated successfully', [
+ {
+ text: 'OK',
+ onPress: () => navigation.goBack(),
+ },
+ ]);
- Alert.alert('Success', 'Profile updated successfully', [
- {
- text: 'OK',
- onPress: () => navigation.goBack(),
- },
- ]);
- }📝 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 data = await res.json(); | |
| if (data.error) { | |
| Alert.alert('Error', data.error); | |
| } else { | |
| Alert.alert('Success', 'Profile updated successfully', [ | |
| { | |
| text: 'OK', | |
| onPress: () => navigation.goBack(), | |
| }, | |
| ]); | |
| } | |
| const res = await fetch(`${BASE_URL}/update_profile`, { | |
| method: 'PUT', | |
| headers: {'Content-Type': 'application/json'}, | |
| body: JSON.stringify({ | |
| location: country, | |
| lmp: lmpDate, | |
| cycleLength: cycleLength, | |
| periodLength: periodLength, | |
| age: Number(age), | |
| weight: Number(weight), | |
| }), | |
| }); | |
| if (!res.ok) { | |
| const errorData = await res.json().catch(() => ({})); | |
| Alert.alert('Error', errorData.error || `Server error: ${res.status}`); | |
| return; | |
| } | |
| const data = await res.json(); | |
| Alert.alert('Success', 'Profile updated successfully', [ | |
| { | |
| text: 'OK', | |
| onPress: () => navigation.goBack(), | |
| }, | |
| ]); |
🤖 Prompt for AI Agents
In Frontend/src/Screens/EditProfileScreen.jsx around lines 186 to 198, the
response JSON is parsed and data.error is checked without first verifying
res.ok, which can break on 4xx/5xx responses or non-JSON bodies; update the flow
to check res.ok immediately after awaiting the fetch response, attempt to parse
JSON inside a try/catch (or fallback to plain text) only when needed, and if
res.ok is false show an Alert containing the HTTP status and any error message
from the parsed body (or statusText/fallback text), otherwise proceed with the
success Alert and navigation; also wrap the whole network sequence in try/catch
to handle network/parsing exceptions and display an Alert on failure.
| <TouchableOpacity | ||
| style={styles.stepperButton} | ||
| onPress={() => setCycleLength(prev => Math.max(20, prev - 1))}> | ||
| <Text style={styles.stepperButtonText}>-</Text> | ||
| </TouchableOpacity> | ||
| <TextInput | ||
| style={[styles.stepperInput, errors.cycleLength ? styles.errorBorder : null]} | ||
| value={String(cycleLength)} | ||
| onChangeText={text => setCycleLength(text)} | ||
| keyboardType="numeric" | ||
| /> | ||
| <TouchableOpacity | ||
| style={styles.stepperButton} | ||
| onPress={() => setCycleLength(prev => Math.min(40, Number(prev) + 1))}> | ||
| <Text style={styles.stepperButtonText}>+</Text> | ||
| </TouchableOpacity> | ||
| </View> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type inconsistency in stepper handlers can cause unexpected behavior.
The decrement handler on line 287 uses prev - 1 directly, but prev could be a string after text input (line 293 sets it as string). While JavaScript's type coercion may work in many cases, this is fragile. The increment handler correctly uses Number(prev) but decrement doesn't.
🔎 Proposed fix for all steppers
<TouchableOpacity
style={styles.stepperButton}
- onPress={() => setCycleLength(prev => Math.max(20, prev - 1))}>
+ onPress={() => setCycleLength(prev => Math.max(20, Number(prev) - 1))}>
<Text style={styles.stepperButtonText}>-</Text>
</TouchableOpacity>
<TextInput
style={[styles.stepperInput, errors.cycleLength ? styles.errorBorder : null]}
value={String(cycleLength)}
- onChangeText={text => setCycleLength(text)}
+ onChangeText={text => setCycleLength(Number(text) || 28)}
keyboardType="numeric"
/>Apply similar fixes to periodLength, age, and weight steppers.
🤖 Prompt for AI Agents
In Frontend/src/Screens/EditProfileScreen.jsx around lines 285 to 301, the
decrement stepper uses prev - 1 while prev may be a string from TextInput,
causing type inconsistencies; change the decrement handler to coerce prev to
Number (e.g., Number(prev) - 1) and keep the same bounds check (Math.max) as the
increment handler does with Number(prev); also apply the same numeric coercion
fix to the other steppers (periodLength, age, weight) so all increment/decrement
handlers consistently convert prev to Number before arithmetic.
|
Include testing evidence of features implemented |
|
@bhavik-mangla I have attached proof of work. Also when user updates country or cycle period or last menstrual period it automatically updates the information on all pages without doing any manual refresh. Also have resolved the merge issue keeping both changes in main and from my branch Screen.Recording.2026-01-08.at.1.28.20.PM.mov |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
Backend/routes/profile.py (3)
72-73: Standardize error handling across all endpoints.Error handling is inconsistent:
get_profile(lines 72-73) returns a generic"Database Error"messageupdate_profile(lines 128-129) returns the actual error viastr(error)set_profile(line 51) also returnsstr(error)This inconsistency makes debugging harder for API consumers. Additionally, exposing detailed database errors in production could leak sensitive internal information.
🔧 Proposed fix: Consistent error handling with logging
+import logging + +logger = logging.getLogger(__name__) # In get_profile: except sqlite3.OperationalError as error: + logger.error(f"Database error in get_profile: {error}") - return jsonify({"error": "Database Error"}), 500 + return jsonify({"error": "Failed to retrieve profile"}), 500 # In update_profile: except sqlite3.OperationalError as error: + logger.error(f"Database error in update_profile: {error}") - return jsonify({"error": str(error)}), 500 + return jsonify({"error": "Failed to update profile"}), 500This logs detailed errors server-side while returning safe messages to clients.
Also applies to: 128-129
43-46: Extract repeated cache update logic into a helper function.The cache update pattern is duplicated three times across
set_profile,delete_profile, andupdate_profile. This violates the DRY principle and makes maintenance harder.♻️ Proposed refactor: Create a helper function
Add this helper function after the imports:
def update_profile_cache(operation): """Update the agent cache after profile operations.""" db_path = os.path.join(os.path.dirname(os.path.dirname(__file__)), "db", "database.db") agent = get_agent(db_path) agent.update_cache(data_type="profile", operation=operation)Then replace all three occurrences:
- # Update cache after database update - db_path = os.path.join(os.path.dirname(os.path.dirname(__file__)), "db", "database.db") - agent = get_agent(db_path) - agent.update_cache(data_type="profile", operation="create") + update_profile_cache("create")Also applies to: 83-86, 122-125
23-29: Add input validation for numeric and date fields.The endpoints accept
age,weight,cycleLength,periodLength, andlmpwithout validating their types or ranges. Invalid inputs (e.g., negative age, non-numeric weight, malformed dates) could cause issues downstream or duringdue_datecalculation. Currently onlylmpandlocationpresence are checked.Consider adding validation for:
ageandweightare positive numberscycleLengthandperiodLengthare positive integerslmpis a valid date string in YYYY-MM-DD formatThis applies to both
set_profile()(lines 23–29) andupdate_profile()(lines 102–108).
🤖 Fix all issues with AI agents
In @Backend/routes/profile.py:
- Around line 63-69: The get_profile response uses snake_case and fragile tuple
indexing, causing inconsistency with update_profile which expects camelCase;
change DB rows to dict-like access by enabling a row factory in open_db (e.g.,
set sqlite3.Row) and update get_profile to build the response using camelCase
keys (e.g., lmp, cycleLength, periodLength, age, weight, location, dueDate) by
accessing columns by name instead of profile[<index>], ensuring names match
update_profile and avoiding brittle positional indexing.
In @Frontend/src/Screens/ProfileScreen.jsx:
- Around line 42-46: The useFocusEffect callback captures a stale instance of
fetchProfileData because fetchProfileData is recreated each render and not
listed in dependencies; wrap fetchProfileData in useCallback with its own
dependency array (including any props/state it uses) and then reference that
memoized fetchProfileData inside useFocusEffect (or add the memoized function to
the dependency array) so the focus effect always calls the current, stable
function; update both the fetchProfileData definition and the useFocusEffect
invocation (symbols: fetchProfileData, useCallback, useFocusEffect) accordingly.
🧹 Nitpick comments (3)
Frontend/src/Screens/ProfileScreen.jsx (1)
57-61: Consider locale-aware weight units.The weight unit is hardcoded to "kg" (line 59), which may not be appropriate for all locales. Consider making the unit configurable or locale-aware if the app supports multiple regions.
Backend/routes/profile.py (2)
104-105: Consider consistent variable naming for clarity.The code extracts
cycleLengthandperiodLength(camelCase) from the request but assigns them to snake_case variables (cycle_length,period_length). While functionally correct, mixing naming conventions within the same function reduces readability.♻️ Proposed refactor: Use consistent camelCase variable names
data = request.json lmp = data.get('lmp') - cycle_length = data.get('cycleLength') - period_length = data.get('periodLength') + cycleLength = data.get('cycleLength') + periodLength = data.get('periodLength') age = data.get('age') weight = data.get('weight') location = data.get('location') if not lmp or not location: return jsonify({"error": "Last menstrual period and location are required"}), 400 # Recalculate due date based on new LMP and cycle length - due_date = calculate_due_date(lmp, cycle_length) + due_date = calculate_due_date(lmp, cycleLength) db.execute( 'UPDATE profile SET lmp = ?, cycleLength = ?, periodLength = ?, age = ?, weight = ?, user_location = ?, dueDate = ?', - (lmp, cycle_length, period_length, age, weight, location, due_date) + (lmp, cycleLength, periodLength, age, weight, location, due_date) )Also applies to: 113-118
21-22: Consider explicit database connection cleanup.Database connections opened via
open_db()are never explicitly closed. While SQLite connections auto-close when garbage collected, explicit cleanup using context managers is more robust and prevents resource leaks.♻️ Proposed refactor: Use context manager for database connections
Update
db/db.pyto return a context manager, or use try-finally blocks:@profile_bp.route('/get_profile', methods=['GET']) def get_profile(): db = open_db() - try: profile = db.execute('SELECT * FROM profile').fetchone() if profile is None: return jsonify({"error": "Profile not found"}), 404 return jsonify({ "lmp": profile[1], # ... rest of response }), 200 except sqlite3.OperationalError: return jsonify({"error": "Database Error"}), 500 + finally: + db.close()Apply similar changes to all four route handlers.
Also applies to: 55-56, 77-78, 94-95
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Backend/routes/profile.pyFrontend/src/Screens/ProfileScreen.jsx
🧰 Additional context used
🧬 Code graph analysis (2)
Backend/routes/profile.py (4)
Frontend/src/Screens/BasicDetailsScreen.jsx (2)
age(27-27)weight(28-28)Frontend/src/Screens/EditProfileScreen.jsx (2)
age(31-31)weight(32-32)Frontend/src/Screens/WeightScreen.jsx (1)
weight(18-18)Frontend/src/context/AgentContext.jsx (1)
error(29-29)
Frontend/src/Screens/ProfileScreen.jsx (2)
Frontend/src/Screens/EditProfileScreen.jsx (3)
loading(37-37)fetchProfileData(49-67)navigation(25-25)Frontend/src/Components/HeaderWithBack.jsx (1)
navigation(7-7)
🔇 Additional comments (7)
Frontend/src/Screens/ProfileScreen.jsx (6)
1-16: LGTM! Imports are appropriate for the new functionality.The addition of
useCallbackanduseFocusEffectcorrectly supports the screen focus refresh pattern.
29-38: LGTM! State initialization is consistent.The new profile fields follow the established pattern of initializing with 'Not set' default values.
118-123: LGTM! Personal Details section properly extended.The addition of Age and Weight fields follows the established UI pattern and integrates cleanly with existing Location field.
125-131: LGTM! Pregnancy Details section well-structured.The new card provides clear separation between personal and pregnancy-related information, maintaining visual and structural consistency.
194-205: LGTM! Styling improvement adds appropriate spacing.The
marginBottom: 20addition provides proper visual separation between the Personal Details and Pregnancy Details cards.
133-138: Profile editing navigation implemented correctly.The Edit Information button navigates to the EditProfile screen, which is properly registered in the navigation stack. The route name matches the registered component configuration in StackNavigator.jsx.
Backend/routes/profile.py (1)
97-100: Good defensive check.The existence verification prevents attempting to update a non-existent profile and provides a clear 404 response to the client.
| "lmp": profile[1], | ||
| "cycle_length": profile[2], | ||
| "period_length": profile[3], | ||
| "age": profile[4], | ||
| "weight": profile[5], | ||
| "location": profile[6], | ||
| "due_date": profile[7] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix API naming inconsistency between get_profile and update_profile.
The response uses snake_case field names (cycle_length, period_length), but update_profile (lines 104-105) expects camelCase (cycleLength, periodLength). This inconsistency breaks the API contract and forces frontend code to transform keys differently for reads vs. writes.
Additionally, hardcoded tuple indices (profile[1], profile[2], etc.) are fragile and will break if the database schema changes or columns are reordered.
🔧 Proposed fix: Use consistent camelCase naming and dict-based access
First, configure the database connection to return rows as dictionaries. In db/db.py, update open_db():
def open_db():
conn = sqlite3.connect('database.db')
conn.row_factory = sqlite3.Row # Enable dict-like access
return connThen update the response to use consistent camelCase:
return jsonify({
- "lmp": profile[1],
- "cycle_length": profile[2],
- "period_length": profile[3],
- "age": profile[4],
- "weight": profile[5],
- "location": profile[6],
- "due_date": profile[7]
+ "lmp": profile['lmp'],
+ "cycleLength": profile['cycleLength'],
+ "periodLength": profile['periodLength'],
+ "age": profile['age'],
+ "weight": profile['weight'],
+ "location": profile['user_location'],
+ "dueDate": profile['dueDate']
}), 200Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @Backend/routes/profile.py around lines 63 - 69, The get_profile response
uses snake_case and fragile tuple indexing, causing inconsistency with
update_profile which expects camelCase; change DB rows to dict-like access by
enabling a row factory in open_db (e.g., set sqlite3.Row) and update get_profile
to build the response using camelCase keys (e.g., lmp, cycleLength,
periodLength, age, weight, location, dueDate) by accessing columns by name
instead of profile[<index>], ensuring names match update_profile and avoiding
brittle positional indexing.
| useFocusEffect( | ||
| useCallback(() => { | ||
| fetchProfileData(); | ||
| }, []) | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrap fetchProfileData in useCallback to prevent stale closures.
The fetchProfileData function is called inside useFocusEffect but isn't included in the dependency array (line 45). Since fetchProfileData itself isn't wrapped in useCallback, a new instance is created on every render, but the useFocusEffect callback captures only the first instance. This creates a stale closure risk: if fetchProfileData were to depend on props or state in the future, those values would be stale.
♻️ Wrap fetchProfileData in useCallback
+ const fetchProfileData = useCallback(async () => {
- const fetchProfileData = async () => {
try {
const response = await fetch(`${BASE_URL}/get_profile`);
if (response.ok) {
const data = await response.json();
setProfileData({
name: data.name || 'Guest',
due_date: data.due_date || 'Not set',
location: data.location || 'Not set',
lmp: data.lmp || 'Not set',
age: data.age ? String(data.age) : 'Not set',
weight: data.weight ? `${data.weight} kg` : 'Not set',
cycle_length: data.cycle_length ? `${data.cycle_length} days` : 'Not set',
period_length: data.period_length ? `${data.period_length} days` : 'Not set',
});
}
} catch (error) {
console.error('Error fetching profile:', error);
Alert.alert('Error', 'Failed to load profile data');
} finally {
setLoading(false);
setRefreshing(false);
}
- };
+ }, []);
useFocusEffect(
useCallback(() => {
fetchProfileData();
- }, [])
+ }, [fetchProfileData])
);Also applies to: 48-71
🤖 Prompt for AI Agents
In @Frontend/src/Screens/ProfileScreen.jsx around lines 42 - 46, The
useFocusEffect callback captures a stale instance of fetchProfileData because
fetchProfileData is recreated each render and not listed in dependencies; wrap
fetchProfileData in useCallback with its own dependency array (including any
props/state it uses) and then reference that memoized fetchProfileData inside
useFocusEffect (or add the memoized function to the dependency array) so the
focus effect always calls the current, stable function; update both the
fetchProfileData definition and the useFocusEffect invocation (symbols:
fetchProfileData, useCallback, useFocusEffect) accordingly.
Closes #152
Added profile edit feature
Screen.Recording.2026-01-08.at.1.28.20.PM.mov
📝 Description
🔧 Changes Made
📷 Screenshots or Visual Changes (if applicable)
🤝 Collaboration
Collaborated with:
@username(optional)✅ Checklist
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.