Skip to content

issue id : #151#169

Open
vinnnnay wants to merge 2 commits intoAOSSIE-Org:mainfrom
vinnnnay:main
Open

issue id : #151#169
vinnnnay wants to merge 2 commits intoAOSSIE-Org:mainfrom
vinnnnay:main

Conversation

@vinnnnay
Copy link

@vinnnnay vinnnnay commented Jan 13, 2026

Closes #

📝 Description

This PR refactors the frontend to adopt a clean layered architecture inspired by Domain-Driven Design (DDD) principles.
The refactor addresses accumulated technical debt such as scattered API calls, duplicated business logic, and large monolithic services, while preserving existing functionality.

The goal is to improve maintainability, scalability, and clarity of responsibilities across the codebase.


🔧 Changes Made

  • Introduced a centralized API layer (src/api/) as a single source of truth for all backend communication
  • Added a Hooks layer (src/hooks/) to encapsulate state management and side effects
  • Refactored monolithic services into smaller, responsibility-focused domain services
  • Extracted shared helpers into a Utils layer to remove duplicated logic (e.g., date handling)
  • Simplified Screens to UI-only components, improving readability and testability
  • Removed legacy service implementations after successful migration

📷 Screenshots or Visual Changes (if applicable)

No visual changes were introduced.
This PR focuses on internal architecture and code organization only.


🤝 Collaboration

Collaborated with: N/A


✅ Checklist

  • I have read the contributing guidelines.
  • I have tested the changes locally to ensure existing functionality works as expected.
  • I have added or updated documentation where applicable.
  • Any dependent changes have been merged and published in downstream modules.

Summary by CodeRabbit

Release Notes

  • Refactor

    • Simplified home screen to focus on current pregnancy week and due date information for clearer navigation.
    • Streamlined weight tracking interface to display weight history in an optimized format.
    • Restructured analytics service with modular utilities for improved maintainability.
  • Bug Fixes

    • Corrected formatting issue in SOS Alert screen.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 13, 2026

📝 Walkthrough

Walkthrough

This pull request refactors the frontend architecture by replacing large, stateful screen components with lightweight, hook-based alternatives. It introduces a new API client layer with modular endpoint wrappers, adds custom hooks for data management, removes the monolithic AnalyticsService, and implements utility functions for date formatting and pregnancy week calculation.

Changes

Cohort / File(s) Summary
Screen Components
Frontend/src/Screens/HomeScreen.jsx, Frontend/src/Screens/WeightScreen.jsx, Frontend/src/Screens/SOSAlertScreen.jsx
HomeScreen and WeightScreen simplified from stateful, data-fetching implementations to thin components that delegate to custom hooks (useHomeDashboard, useWeight); SOSAlertScreen formatting adjustment only
API Client Layer
Frontend/src/api/client.js, Frontend/src/api/appointment.js, Frontend/src/api/profile.js, Frontend/src/api/tasks.js, Frontend/src/api/weight.js
New modular API wrapper modules; base client.js handles fetch with JSON headers and error parsing; endpoint-specific wrappers expose simple facades (appointmentsApi.list, profileApi.get, tasksApi.list, weightApi CRUD operations)
Custom Hooks
Frontend/src/hooks/useHomeDashboard.js, Frontend/src/hooks/useWeight.js
useHomeDashboard consolidates profile, appointments, tasks fetching and week computation on mount; useWeight manages weight history state with CRUD actions and automatic refresh
Analytics & Services
Frontend/src/services/AnalyticsService.js, Frontend/src/services/analytics/insights.js, Frontend/src/services/analytics/processors.js
Complete removal of AnalyticsService singleton; new modular analytics structure with weightInsights and processWeight for isolated analytics functions
Utility Functions
Frontend/src/utils/dateUtils.js, Frontend/src/utils/pregnancyUtils.js
Added formatLocalDate and formatDateLabel for locale-aware date formatting; added calculateCurrentWeek for pregnancy week computation from due date

Sequence Diagram(s)

sequenceDiagram
    participant Screen as HomeScreen
    participant Hook as useHomeDashboard Hook
    participant ProfileAPI as profileApi
    participant AppointmentsAPI as appointmentsApi
    participant TasksAPI as tasksApi
    participant Utils as pregnancyUtils

    Screen->>Hook: Component mounts
    Hook->>ProfileAPI: profileApi.get()
    ProfileAPI-->>Hook: profile data
    Hook->>Utils: calculateCurrentWeek(profile.due_date)
    Utils-->>Hook: current week number
    Hook->>AppointmentsAPI: appointmentsApi.list()
    AppointmentsAPI-->>Hook: appointments array
    Hook->>TasksAPI: tasksApi.list()
    TasksAPI-->>Hook: tasks array
    Hook-->>Screen: {week, dueDate, appointments, tasks}
    Screen->>Screen: Render dashboard with hook state
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰 A hop through the code, clean and bright,
Hooks now fetch the data just right,
Screens lean and light, no state to bear,
Analytics flow through the air,
The refactor's done, the app's fair! 🌿✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The pull request title 'issue id : #151' is vague and does not convey meaningful information about the changeset, which involves a significant frontend refactoring with layered architecture, API abstraction, and state management. Replace the title with a descriptive summary of the main change, such as 'Refactor frontend architecture with API and hooks layers' or similar, to clearly communicate the intent to reviewers.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

🤖 Fix all issues with AI agents
In @Frontend/src/api/appointment.js:
- Around line 1-5: The module file name mismatches the import: the API module is
defined in appointment.js but useHomeDashboard.js imports '../api/appointments';
fix by renaming Frontend/src/api/appointment.js to
Frontend/src/api/appointments.js so the export appointmentsApi matches the
import, or alternatively update the import in useHomeDashboard.js to reference
'./api/appointment' to match the existing file name; ensure the exported symbol
appointmentsApi and the import path in useHomeDashboard.js are consistent.

In @Frontend/src/api/client.js:
- Line 14: The current return res.json() call will throw when the DELETE
endpoint returns 204 No Content or an empty body; change the client.js response
handling around the return res.json() to first detect empty responses (e.g., if
res.status === 204 or the Content-Length header is "0" or content-type is
missing/not JSON) and return null (or an appropriate empty value) in that case,
otherwise call res.json() to parse and return the JSON.
- Around line 4-7: The fetch call's header merge is wrong: when spreading
...options you overwrite the default headers object if options.headers exists;
update the request construction in client.js to merge headers explicitly by
building headers: { 'Content-Type': 'application/json', ...(options &&
options.headers ? options.headers : {}) } and then spread the rest of options
(ensuring you do not include options.headers twice) so callers can add custom
headers without losing Content-Type; adjust the const res = await fetch(...)
call to create a mergedOptions where headers are the merged object and pass
mergedOptions to fetch.

In @Frontend/src/hooks/useHomeDashboard.js:
- Around line 15-24: The refresh function lacks error handling and runs API
calls sequentially, risking unhandled rejections and state updates after
unmount; wrap its body in try/catch, fetch profile, appointments, and tasks in
parallel via Promise.all (using profileApi.get, appointmentsApi.list,
tasksApi.list), compute week with calculateCurrentWeek(profile.due_date) only
after a successful profile result, call setState({ week, dueDate:
profile.due_date, appointments, tasks }) on success, and in the catch log or
surface the error and avoid calling setState; additionally guard against
updating state on unmounted components by checking an isMounted flag or abort
signal before invoking setState.

In @Frontend/src/hooks/useWeight.js:
- Around line 10-11: The code calls weightApi.list() and then mutates the
returned array with data.reverse(); fix this by avoiding in-place
mutation—replace setHistory(data.reverse()) with a non-mutating reversal such as
setHistory([...data].reverse()) or setHistory(data.slice().reverse()) in the
same block around the const data = await weightApi.list() call in useWeight.js
so the original API response array is not mutated.
- Around line 15-17: The create/update/remove functions currently call
weightApi.create/update/remove(...).then(refresh) but do not return the
resulting promise, so callers cannot await or catch errors; modify create,
update, and remove to return the promise chain (e.g., return
weightApi.create(data).then(refresh)) so callers can handle rejections, or
alternatively capture and surface errors via the hook's state (set an error
state inside the hook when the promise rejects) using the same unique symbols
create, update, remove, weightApi and refresh.
- Around line 8-13: The refresh function can leak loading state and swallow
errors; wrap the weightApi.list() call in try/catch/finally, move
setHistory(data.reverse()) into the try so it only runs on success,
setLoading(false) in finally to guarantee the spinner is cleared, and update or
add an error state (e.g., error / setError via useState) inside the catch so
consumers can display feedback (or rethrow/log the error) instead of silently
swallowing it.

In @Frontend/src/Screens/HomeScreen.jsx:
- Around line 5-11: HomeScreen currently only destructures week and dueDate from
useHomeDashboard and renders them without handling loading or error states;
update HomeScreen to destructure loading and error flags (e.g., isLoading,
error) from useHomeDashboard and render a loading indicator (or placeholder)
while isLoading is true and an error message/UI when error is present, and
either destructure and render appointments and tasks or add a clear comment in
HomeScreen explaining why appointments/tasks are intentionally omitted; refer to
the useHomeDashboard hook name and the HomeScreen component to locate where to
add these checks and the conditional rendering.

In @Frontend/src/services/analytics/insights.js:
- Around line 3-6: The function computes delta = values[values.length - 1] -
values[0] and currently returns 'Weight increasing normally' for delta > 0 and
'Weight stable' otherwise, which mislabels decreases as stable; update the logic
in the insights function (use the existing delta and values variables) to
explicitly handle three cases: positive delta -> return an increasing insight,
negative delta -> return a decreasing insight (e.g., 'Weight decreasing'), and
near-zero delta -> return 'Weight stable' (optionally using a small threshold to
treat tiny changes as stable).
🧹 Nitpick comments (7)
Frontend/src/utils/dateUtils.js (1)

17-27: Consider adding validation for invalid Date objects.

If date is an invalid Date (e.g., new Date('invalid')), calling toLocaleTimeString or toLocaleDateString may throw or return unexpected results depending on the environment. A guard could improve robustness.

Optional defensive check
 export function formatDateLabel(date, timeframe) {
+  if (!(date instanceof Date) || isNaN(date)) return '';
   switch (timeframe) {
     case 'day':
       return date.toLocaleTimeString([], { hour: '2-digit', minute: '2-digit' });
Frontend/src/services/analytics/processors.js (1)

3-10: Add guard for null/undefined input.

If data is null or undefined, the spread operator on line 4 will throw a TypeError. Consider adding a default or early return.

Suggested fix
 export function processWeight(data, timeframe) {
+  if (!data?.length) return { labels: [], values: [], raw: [] };
   const sorted = [...data].sort((a, b) => new Date(a.date) - new Date(b.date));
   return {
     labels: sorted.map(i => formatDateLabel(new Date(i.date), timeframe)),
     values: sorted.map(i => Number(i.weight)),
     raw: sorted,
   };
 }
Frontend/src/hooks/useHomeDashboard.js (1)

26-28: Consider exposing refresh for manual refresh capability.

The refresh function is defined but not returned, preventing consumers from triggering data refresh (e.g., pull-to-refresh). Consider returning it alongside the state.

♻️ Proposed change
-  return state;
+  return { ...state, refresh };
Frontend/src/Screens/WeightScreen.jsx (2)

7-7: Consider using loading state for better UX.

The useWeight hook provides a loading state that isn't being used here. Users will see an empty screen while data is being fetched.

♻️ Proposed improvement with loading state
-  const { history } = useWeight();
+  const { history, loading } = useWeight();
+
+  if (loading) {
+    return (
+      <View style={{ flex: 1, justifyContent: 'center', alignItems: 'center' }}>
+        <Text>Loading...</Text>
+      </View>
+    );
+  }

10-18: Consider handling empty state.

When history is empty, users see a blank ScrollView with no feedback. Adding an empty state message would improve the experience.

♻️ Proposed empty state handling
   <ScrollView>
+    {history.length === 0 && (
+      <Text style={{ textAlign: 'center', marginTop: 20 }}>No weight entries yet.</Text>
+    )}
     {history.map(item => (
       <View key={item.id}>
         <Text>Week {item.week_number}</Text>
         <Text>{item.weight} kg</Text>
         <Text>{formatLocalDate(item.created_at)}</Text>
       </View>
     ))}
   </ScrollView>
Frontend/src/hooks/useWeight.js (1)

5-6: Consider initializing loading to true.

Since useEffect immediately triggers refresh(), the component briefly renders with loading: false and empty history before the fetch starts. Initializing loading: true provides a more accurate initial state.

-const [loading, setLoading] = useState(false);
+const [loading, setLoading] = useState(true);
Frontend/src/api/client.js (1)

1-1: Consider validating BASE_URL is defined.

If the environment variable is missing or misconfigured, requests will silently fail or hit undefined/endpoint. An early guard would surface configuration issues faster.

 import { BASE_URL } from '@env';
+
+if (!BASE_URL) {
+  throw new Error('BASE_URL environment variable is not configured');
+}
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8893922 and 29f7950.

📒 Files selected for processing (16)
  • Frontend/src/Screens/HomeScreen.jsx
  • Frontend/src/Screens/SOSAlertScreen.jsx
  • Frontend/src/Screens/WeightScreen.jsx
  • Frontend/src/api/appointment.js
  • Frontend/src/api/client.js
  • Frontend/src/api/profile.js
  • Frontend/src/api/tasks.js
  • Frontend/src/api/weight.js
  • Frontend/src/hooks/useHomeDashboard.js
  • Frontend/src/hooks/useWeight.js
  • Frontend/src/services/AnalyticsService.js
  • Frontend/src/services/analytics/AnalyticsService.js
  • Frontend/src/services/analytics/insights.js
  • Frontend/src/services/analytics/processors.js
  • Frontend/src/utils/dateUtils.js
  • Frontend/src/utils/pregnancyUtils.js
💤 Files with no reviewable changes (1)
  • Frontend/src/services/AnalyticsService.js
🧰 Additional context used
🧬 Code graph analysis (6)
Frontend/src/api/profile.js (1)
Frontend/src/api/client.js (1)
  • apiClient (3-15)
Frontend/src/api/tasks.js (1)
Frontend/src/api/client.js (1)
  • apiClient (3-15)
Frontend/src/services/analytics/processors.js (1)
Frontend/src/utils/dateUtils.js (1)
  • formatDateLabel (17-27)
Frontend/src/api/appointment.js (1)
Frontend/src/api/client.js (1)
  • apiClient (3-15)
Frontend/src/api/weight.js (1)
Frontend/src/api/client.js (1)
  • apiClient (3-15)
Frontend/src/Screens/WeightScreen.jsx (2)
Frontend/src/hooks/useWeight.js (2)
  • useWeight (4-22)
  • history (5-5)
Frontend/src/utils/dateUtils.js (1)
  • formatLocalDate (1-15)
🔇 Additional comments (5)
Frontend/src/Screens/SOSAlertScreen.jsx (1)

13-13: LGTM!

Minor formatting adjustment to the else branch. No functional changes.

Frontend/src/utils/dateUtils.js (1)

1-15: LGTM!

Good defensive handling for falsy input and invalid dates. The UTC suffix logic ensures consistent parsing.

Frontend/src/api/tasks.js (1)

1-5: LGTM!

Clean API wrapper following the established pattern. The module correctly delegates to the centralized apiClient for consistent request handling.

Frontend/src/api/profile.js (1)

1-5: LGTM!

Consistent API wrapper pattern for profile data retrieval.

Frontend/src/api/weight.js (1)

3-11: Clean API wrapper implementation.

The module follows a consistent pattern for CRUD operations. The separation of concerns is well-executed.

One minor consideration: if id is undefined or null, the URL becomes /weight/undefined. Consider adding parameter validation or documenting this expectation for callers.

Comment on lines +1 to +5
import { apiClient } from './client';

export const appointmentsApi = {
list: () => apiClient('/get_appointments'),
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Filename mismatch will cause import failure.

The file is named appointment.js (singular), but useHomeDashboard.js at line 3 imports from '../api/appointments' (plural). This path mismatch will cause a module resolution error at runtime.

Either rename this file to appointments.js to match the import, or update the import path in useHomeDashboard.js.

🐛 Recommended fix: Rename file to match import

Rename Frontend/src/api/appointment.jsFrontend/src/api/appointments.js

🤖 Prompt for AI Agents
In @Frontend/src/api/appointment.js around lines 1 - 5, The module file name
mismatches the import: the API module is defined in appointment.js but
useHomeDashboard.js imports '../api/appointments'; fix by renaming
Frontend/src/api/appointment.js to Frontend/src/api/appointments.js so the
export appointmentsApi matches the import, or alternatively update the import in
useHomeDashboard.js to reference './api/appointment' to match the existing file
name; ensure the exported symbol appointmentsApi and the import path in
useHomeDashboard.js are consistent.

Comment on lines +4 to +7
const res = await fetch(`${BASE_URL}${endpoint}`, {
headers: { 'Content-Type': 'application/json' },
...options,
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Header merge logic is incorrect.

The spread ...options overwrites the entire headers object if options.headers is provided, rather than merging individual headers. This means callers cannot add custom headers without losing Content-Type.

Fix: Properly merge headers
 export async function apiClient(endpoint, options = {}) {
+  const { headers: customHeaders, ...restOptions } = options;
   const res = await fetch(`${BASE_URL}${endpoint}`, {
-    headers: { 'Content-Type': 'application/json' },
-    ...options,
+    ...restOptions,
+    headers: {
+      'Content-Type': 'application/json',
+      ...customHeaders,
+    },
   });
📝 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.

Suggested change
const res = await fetch(`${BASE_URL}${endpoint}`, {
headers: { 'Content-Type': 'application/json' },
...options,
});
const { headers: customHeaders, ...restOptions } = options;
const res = await fetch(`${BASE_URL}${endpoint}`, {
...restOptions,
headers: {
'Content-Type': 'application/json',
...customHeaders,
},
});
🤖 Prompt for AI Agents
In @Frontend/src/api/client.js around lines 4 - 7, The fetch call's header merge
is wrong: when spreading ...options you overwrite the default headers object if
options.headers exists; update the request construction in client.js to merge
headers explicitly by building headers: { 'Content-Type': 'application/json',
...(options && options.headers ? options.headers : {}) } and then spread the
rest of options (ensuring you do not include options.headers twice) so callers
can add custom headers without losing Content-Type; adjust the const res = await
fetch(...) call to create a mergedOptions where headers are the merged object
and pass mergedOptions to fetch.

throw new Error(text || 'API Error');
}

return res.json();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

res.json() will throw on empty responses.

DELETE endpoints commonly return 204 No Content with an empty body. Calling res.json() on such responses throws a parse error.

Suggested fix
-  return res.json();
+  const text = await res.text();
+  return text ? JSON.parse(text) : null;
📝 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.

Suggested change
return res.json();
const text = await res.text();
return text ? JSON.parse(text) : null;
🤖 Prompt for AI Agents
In @Frontend/src/api/client.js at line 14, The current return res.json() call
will throw when the DELETE endpoint returns 204 No Content or an empty body;
change the client.js response handling around the return res.json() to first
detect empty responses (e.g., if res.status === 204 or the Content-Length header
is "0" or content-type is missing/not JSON) and return null (or an appropriate
empty value) in that case, otherwise call res.json() to parse and return the
JSON.

Comment on lines +15 to +24
const refresh = async () => {
const profile = await profileApi.get();
const week = calculateCurrentWeek(profile.due_date);
setState({
week,
dueDate: profile.due_date,
appointments: await appointmentsApi.list(),
tasks: await tasksApi.list(),
});
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Missing error handling will cause unhandled promise rejections.

If any API call fails, the error propagates uncaught, potentially leaving the UI in an inconsistent state or causing React warnings about state updates on unmounted components.

🐛 Proposed fix with error handling and parallel fetching
-  const refresh = async () => {
-    const profile = await profileApi.get();
-    const week = calculateCurrentWeek(profile.due_date);
-    setState({
-      week,
-      dueDate: profile.due_date,
-      appointments: await appointmentsApi.list(),
-      tasks: await tasksApi.list(),
-    });
-  };
+  const refresh = async () => {
+    try {
+      const [profile, appointments, tasks] = await Promise.all([
+        profileApi.get(),
+        appointmentsApi.list(),
+        tasksApi.list(),
+      ]);
+      const week = calculateCurrentWeek(profile.due_date);
+      setState({
+        week,
+        dueDate: profile.due_date,
+        appointments,
+        tasks,
+      });
+    } catch (error) {
+      console.error('Failed to load dashboard data:', error);
+      // Consider setting an error state for UI feedback
+    }
+  };
📝 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.

Suggested change
const refresh = async () => {
const profile = await profileApi.get();
const week = calculateCurrentWeek(profile.due_date);
setState({
week,
dueDate: profile.due_date,
appointments: await appointmentsApi.list(),
tasks: await tasksApi.list(),
});
};
const refresh = async () => {
try {
const [profile, appointments, tasks] = await Promise.all([
profileApi.get(),
appointmentsApi.list(),
tasksApi.list(),
]);
const week = calculateCurrentWeek(profile.due_date);
setState({
week,
dueDate: profile.due_date,
appointments,
tasks,
});
} catch (error) {
console.error('Failed to load dashboard data:', error);
// Consider setting an error state for UI feedback
}
};
🤖 Prompt for AI Agents
In @Frontend/src/hooks/useHomeDashboard.js around lines 15 - 24, The refresh
function lacks error handling and runs API calls sequentially, risking unhandled
rejections and state updates after unmount; wrap its body in try/catch, fetch
profile, appointments, and tasks in parallel via Promise.all (using
profileApi.get, appointmentsApi.list, tasksApi.list), compute week with
calculateCurrentWeek(profile.due_date) only after a successful profile result,
call setState({ week, dueDate: profile.due_date, appointments, tasks }) on
success, and in the catch log or surface the error and avoid calling setState;
additionally guard against updating state on unmounted components by checking an
isMounted flag or abort signal before invoking setState.

Comment on lines +8 to +13
const refresh = async () => {
setLoading(true);
const data = await weightApi.list();
setHistory(data.reverse());
setLoading(false);
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Missing error handling leaves UI in broken state.

If weightApi.list() rejects, setLoading(false) never runs, leaving the component stuck in a loading state. The error is also swallowed with no way for consumers to display feedback.

Suggested fix with error handling
+const [error, setError] = useState(null);

 const refresh = async () => {
   setLoading(true);
-  const data = await weightApi.list();
-  setHistory(data.reverse());
-  setLoading(false);
+  setError(null);
+  try {
+    const data = await weightApi.list();
+    setHistory([...data].reverse());
+  } catch (err) {
+    setError(err);
+  } finally {
+    setLoading(false);
+  }
 };
🤖 Prompt for AI Agents
In @Frontend/src/hooks/useWeight.js around lines 8 - 13, The refresh function
can leak loading state and swallow errors; wrap the weightApi.list() call in
try/catch/finally, move setHistory(data.reverse()) into the try so it only runs
on success, setLoading(false) in finally to guarantee the spinner is cleared,
and update or add an error state (e.g., error / setError via useState) inside
the catch so consumers can display feedback (or rethrow/log the error) instead
of silently swallowing it.

Comment on lines +10 to +11
const data = await weightApi.list();
setHistory(data.reverse());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Array.reverse() mutates in place.

data.reverse() mutates the original array returned by the API. If the response is cached or reused elsewhere, this could cause subtle bugs. Use a non-mutating approach.

-  setHistory(data.reverse());
+  setHistory([...data].reverse());
📝 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.

Suggested change
const data = await weightApi.list();
setHistory(data.reverse());
const data = await weightApi.list();
setHistory([...data].reverse());
🤖 Prompt for AI Agents
In @Frontend/src/hooks/useWeight.js around lines 10 - 11, The code calls
weightApi.list() and then mutates the returned array with data.reverse(); fix
this by avoiding in-place mutation—replace setHistory(data.reverse()) with a
non-mutating reversal such as setHistory([...data].reverse()) or
setHistory(data.slice().reverse()) in the same block around the const data =
await weightApi.list() call in useWeight.js so the original API response array
is not mutated.

Comment on lines +15 to +17
const create = data => weightApi.create(data).then(refresh);
const update = (id, data) => weightApi.update(id, data).then(refresh);
const remove = id => weightApi.remove(id).then(refresh);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

CRUD operations swallow errors.

If create, update, or remove fail, the promise rejection is not handled, and the caller has no way to know. Consider returning the promise chain so callers can handle errors, or add error state updates.

Suggested approach
-const create = data => weightApi.create(data).then(refresh);
-const update = (id, data) => weightApi.update(id, data).then(refresh);
-const remove = id => weightApi.remove(id).then(refresh);
+const create = async (data) => {
+  await weightApi.create(data);
+  await refresh();
+};
+const update = async (id, data) => {
+  await weightApi.update(id, data);
+  await refresh();
+};
+const remove = async (id) => {
+  await weightApi.remove(id);
+  await refresh();
+};

This allows callers to wrap in try/catch. Alternatively, set error state internally.

📝 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.

Suggested change
const create = data => weightApi.create(data).then(refresh);
const update = (id, data) => weightApi.update(id, data).then(refresh);
const remove = id => weightApi.remove(id).then(refresh);
const create = async (data) => {
await weightApi.create(data);
await refresh();
};
const update = async (id, data) => {
await weightApi.update(id, data);
await refresh();
};
const remove = async (id) => {
await weightApi.remove(id);
await refresh();
};
🤖 Prompt for AI Agents
In @Frontend/src/hooks/useWeight.js around lines 15 - 17, The
create/update/remove functions currently call
weightApi.create/update/remove(...).then(refresh) but do not return the
resulting promise, so callers cannot await or catch errors; modify create,
update, and remove to return the promise chain (e.g., return
weightApi.create(data).then(refresh)) so callers can handle rejections, or
alternatively capture and surface errors via the hook's state (set an error
state inside the hook when the promise rejects) using the same unique symbols
create, update, remove, weightApi and refresh.

Comment on lines +5 to +11
export default function HomeScreen() {
const { week, dueDate } = useHomeDashboard();
return (
<SafeAreaView style={styles.container}>
<StatusBar barStyle="dark-content" backgroundColor="#FFF5F8" />

<ScrollView
contentContainerStyle={styles.scrollContent}
refreshControl={
<RefreshControl refreshing={refreshing} onRefresh={handleRefresh} />
}>
{/* Header */}
<View style={styles.header}>
<TouchableOpacity onPress={openDrawer}>
<Icon name="menu" size={24} color="#000" />
</TouchableOpacity>
<Text style={styles.appName}>BabyNest</Text>
<TouchableOpacity onPress={() => navigation.navigate('Profile')}>
<Image
source={require('../assets/Avatar.jpeg')}
style={styles.profileImage}
/>
</TouchableOpacity>
</View>

{/* Baby Info */}
<View style={styles.babyInfoContainer}>
<Image
source={require('../assets/Baby.jpeg')}
style={styles.babyImage}
/>
<View style={styles.babyInfo}>
<Text style={styles.weekText}>Week {currentWeek}</Text>
<Text style={styles.babySize}>
{currentWeek >= 4 && 'Size of '}
{currentBabySize}
</Text>
<Text style={styles.dueDate}>
Due:{' '}
{dueDate
? new Date(dueDate).toLocaleDateString('en-GB', {
day: 'numeric',
month: 'long',
year: 'numeric',
})
: 'Not available'}
</Text>
</View>
</View>

{/* Timeline */}
<View style={styles.timelineContainer}>
<Text style={styles.sectionTitle}>Pregnancy Timeline</Text>
<ScrollView
horizontal
ref={weekScrollRef}
showsHorizontalScrollIndicator={false}
contentContainerStyle={styles.timelineScroll}>
{Array.from({length: 40}, (_, i) => i + 1).map(week => (
<TouchableOpacity
key={week}
style={[
styles.weekBubble,
currentWeek === week && styles.activeWeekBubble,
]}
onPress={() => handleWeekSelect(week)}>
<Text
style={[
styles.weekNumber,
currentWeek === week && styles.activeWeekNumber,
]}>
{week}
</Text>
</TouchableOpacity>
))}
</ScrollView>
</View>

{/* Appointments */}
<View style={styles.section}>
<Text style={styles.sectionTitle}>Upcoming Appointments</Text>
{filteredAppointments.map((appt, idx) => (
<View key={idx} style={styles.card}>
<Icon name="calendar" size={20} color="rgb(218,79,122)" />
<View style={styles.cardContent}>
<Text style={styles.cardTitle}>{appt.title}</Text>
<Text>
{appt.appointment_date} at {appt.appointment_time}
</Text>
<Text>{appt.appointment_location}</Text>
</View>
</View>
))}
</View>

{/* Tasks */}
<View style={styles.section}>
<Text style={styles.sectionTitle}>This Week's Tasks</Text>
{filteredTasks.map((task, idx) => (
<View key={idx} style={styles.card}>
<Icon
name="checkmark-circle-outline"
size={20}
color="rgb(218,79,122)"
/>
<View style={styles.cardContent}>
<Text style={styles.cardTitle}>{task.title}</Text>
<Text>
{task.starting_week === task.ending_week
? `Week ${task.starting_week}`
: `Weeks ${task.starting_week}-${task.ending_week}`}
</Text>
</View>
</View>
))}
</View>

{/* Floating Button */}
<TouchableOpacity style={styles.floatingButton}>
<MaterialIcons name="smart-toy" size={24} color="#fff" />
</TouchableOpacity>
</ScrollView>
</SafeAreaView>
<View>
<Text>Week {week}</Text>
<Text>Due: {dueDate}</Text>
</View>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Missing loading and error state handling.

The hook likely performs async data fetching, but the component doesn't handle loading or error states. Users will see empty/undefined values during fetch, which degrades UX.

Additionally, per the AI summary, useHomeDashboard returns { week, dueDate, appointments, tasks }, but only week and dueDate are destructured. If this is intentional (phased rollout), consider adding a comment; otherwise, the unused data suggests incomplete migration.

Suggested improvement
 export default function HomeScreen() {
-  const { week, dueDate } = useHomeDashboard();
+  const { week, dueDate, appointments, tasks, loading, error } = useHomeDashboard();
+
+  if (loading) {
+    return (
+      <View>
+        <Text>Loading...</Text>
+      </View>
+    );
+  }
+
+  if (error) {
+    return (
+      <View>
+        <Text>Error loading dashboard</Text>
+      </View>
+    );
+  }
+
   return (
     <View>
       <Text>Week {week}</Text>
       <Text>Due: {dueDate}</Text>
     </View>
   );
 }
🤖 Prompt for AI Agents
In @Frontend/src/Screens/HomeScreen.jsx around lines 5 - 11, HomeScreen
currently only destructures week and dueDate from useHomeDashboard and renders
them without handling loading or error states; update HomeScreen to destructure
loading and error flags (e.g., isLoading, error) from useHomeDashboard and
render a loading indicator (or placeholder) while isLoading is true and an error
message/UI when error is present, and either destructure and render appointments
and tasks or add a clear comment in HomeScreen explaining why appointments/tasks
are intentionally omitted; refer to the useHomeDashboard hook name and the
HomeScreen component to locate where to add these checks and the conditional
rendering.

Comment on lines +3 to +6
const delta = values[values.length - 1] - values[0];
return delta > 0
? ['Weight increasing normally']
: ['Weight stable'];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Misleading insight for weight decrease.

When delta < 0 (weight decreasing), the function returns 'Weight stable', which is semantically incorrect. A decrease is not the same as stable. Consider distinguishing between stable (no significant change) and decreasing trends.

Suggested improvement
 export function weightInsights(values) {
   if (!values.length) return ['No data yet'];
   const delta = values[values.length - 1] - values[0];
-  return delta > 0
-    ? ['Weight increasing normally']
-    : ['Weight stable'];
+  if (delta > 0) return ['Weight increasing'];
+  if (delta < 0) return ['Weight decreasing'];
+  return ['Weight stable'];
 }
🤖 Prompt for AI Agents
In @Frontend/src/services/analytics/insights.js around lines 3 - 6, The function
computes delta = values[values.length - 1] - values[0] and currently returns
'Weight increasing normally' for delta > 0 and 'Weight stable' otherwise, which
mislabels decreases as stable; update the logic in the insights function (use
the existing delta and values variables) to explicitly handle three cases:
positive delta -> return an increasing insight, negative delta -> return a
decreasing insight (e.g., 'Weight decreasing'), and near-zero delta -> return
'Weight stable' (optionally using a small threshold to treat tiny changes as
stable).

Comment on lines +1 to +7
export function calculateCurrentWeek(dueDate) {
const due = new Date(dueDate);
const conception = new Date(due);
conception.setDate(conception.getDate() - 280);
const diff = Date.now() - conception;
return Math.min(Math.max(Math.floor(diff / (1000 * 60 * 60 * 24 * 7)), 1), 40);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Missing validation for invalid date input.

If dueDate is null, undefined, or an invalid date string, new Date(dueDate) returns an Invalid Date, causing the arithmetic to produce NaN. Since this is consumed by the dashboard, consider adding a guard.

Suggested fix
 export function calculateCurrentWeek(dueDate) {
+  if (!dueDate) return null;
   const due = new Date(dueDate);
+  if (isNaN(due)) return null;
   const conception = new Date(due);
   conception.setDate(conception.getDate() - 280);
   const diff = Date.now() - conception;
   return Math.min(Math.max(Math.floor(diff / (1000 * 60 * 60 * 24 * 7)), 1), 40);
 }

@Naren456
Copy link
Contributor

@vinnnnay
Important Note :
1.Always create a new branch from main branch .Don`t use main branch for pr .
2.Write proper title of pull request. (see the other user pr).

@Naren456
Copy link
Contributor

Please add a demo video.

@Naren456
Copy link
Contributor

Naren456 commented Jan 17, 2026

image

@bhavik-mangla
I have reviewed this pr and found some errors in this pr
Error : file name is appointment.js but it mentions appointments .
Implementation also incomplete .
Appointment.js -> No update operation available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants