-
Notifications
You must be signed in to change notification settings - Fork 107
feat: add mark as taken functionality for medicine tracker (#44) #115
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughA new feature is added to mark medicines as taken or not taken. The backend introduces a PATCH endpoint at Changes
Estimated code review effortπ― 3 (Moderate) | β±οΈ ~20 minutes
Poem
Pre-merge checks and finishing touchesβ Failed checks (1 warning)
β Passed checks (2 passed)
β¨ Finishing touches
π§ͺ Generate unit tests (beta)
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: 0
π§Ή Nitpick comments (3)
Backend/routes/medicine.py (1)
163-163: Validate thetakenfield type for robustness.The
taken_statusis extracted without type validation. While the frontend sends a boolean, a malformed request could pass a non-boolean value (e.g., string, number, null), which SQLite might coerce unexpectedly.Apply this diff to add type validation:
- taken_status = data.get('taken', True) # Default to True if not specified + taken_status = data.get('taken', True) + if not isinstance(taken_status, bool): + return jsonify({"error": "Taken status must be a boolean"}), 400Frontend/src/Screens/MedicineScreen.jsx (2)
129-141: Add response validation and consider optimistic updates.The function doesn't validate the response status before refreshing. If the backend returns an error (e.g., 404, 500), the history is still refreshed, potentially showing stale data.
Apply this diff to validate the response:
const handleMarkAsTaken = async (id, currentStatus) => { try { - await fetch(`${BASE_URL}/medicine/${id}/taken`, { + const response = await fetch(`${BASE_URL}/medicine/${id}/taken`, { method: 'PATCH', headers: {'Content-Type': 'application/json'}, body: JSON.stringify({taken: !currentStatus}), }); + if (!response.ok) { + throw new Error(`Server responded with ${response.status}`); + } fetchMedicineHistory(); } catch (err) { console.error('Failed to mark medicine:', err); Alert.alert('Error', 'Failed to update medicine status.'); } };Alternatively, consider implementing optimistic updates to improve perceived responsiveness.
216-222: Consider debouncing to prevent race conditions.Rapid clicks on the icon could trigger multiple concurrent PATCH requests, leading to inconsistent state or unexpected toggle behavior.
Add a loading state or debounce the handler:
const [updatingMedicine, setUpdatingMedicine] = useState(null); const handleMarkAsTaken = async (id, currentStatus) => { if (updatingMedicine === id) return; // Prevent concurrent updates setUpdatingMedicine(id); try { const response = await fetch(`${BASE_URL}/medicine/${id}/taken`, { method: 'PATCH', headers: {'Content-Type': 'application/json'}, body: JSON.stringify({taken: !currentStatus}), }); if (!response.ok) { throw new Error(`Server responded with ${response.status}`); } fetchMedicineHistory(); } catch (err) { console.error('Failed to mark medicine:', err); Alert.alert('Error', 'Failed to update medicine status.'); } finally { setUpdatingMedicine(null); } };Conditionally disable the icon while updating:
<TouchableOpacity onPress={() => handleMarkAsTaken(entry.id, entry.taken)} disabled={updatingMedicine === entry.id}> <Icon name={entry.taken ? 'check-circle' : 'radio-button-unchecked'} size={24} color={updatingMedicine === entry.id ? '#bdc3c7' : (entry.taken ? '#27ae60' : '#95a5a6')} style={styles.checkIcon} /> </TouchableOpacity>
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (2)
Backend/routes/medicine.py(1 hunks)Frontend/src/Screens/MedicineScreen.jsx(3 hunks)
π§° Additional context used
𧬠Code graph analysis (1)
Backend/routes/medicine.py (3)
Backend/db/db.py (1)
open_db(8-13)Backend/agent/agent.py (2)
get_agent(97-102)update_cache(63-72)Backend/agent/cache.py (1)
update_cache(270-364)
π Additional comments (3)
Frontend/src/Screens/MedicineScreen.jsx (2)
372-378: LGTM!The styling for taken entries is clear and appropriate. The line-through decoration and muted color effectively communicate the taken state.
216-222: > Likely an incorrect or invalid review comment.Backend/routes/medicine.py (1)
152-174: The database schema correctly includes thetakencolumn.The
weekly_medicinetable inBackend/schema.sqldefinestakenas aBOOLEANcolumn with a default value of 0, confirming the endpoint implementation is properly supported by the schema.
Closes #
π Description
π§ Changes Made
π· Screenshots or Visual Changes (if applicable)
π€ Collaboration
Collaborated with:
@username(optional)β Checklist
Summary by CodeRabbit
Release Notes
βοΈ Tip: You can customize this high-level summary in your review settings.