Implement Notification Service and Refactor Calendar Screen #186
Implement Notification Service and Refactor Calendar Screen #186Naren456 wants to merge 9 commits intoAOSSIE-Org:mainfrom
Conversation
📝 WalkthroughWalkthroughModularizes the calendar into new UI components and a useCalendar hook with full CRUD and notification scheduling (Notifee). Adds NotificationService, time utilities, multiple appointment modals/timeline, Android notification permissions and gradle Jetifier, dependency updates, and returns created appointment id from backend insert. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant App as React App
participant Hook as useCalendar
participant API as Backend API
participant Notif as NotificationService
participant OS as Android OS
User->>App: Open AddAppointmentModal and submit form
App->>Hook: addAppointment(appointmentData)
Hook->>API: POST /add_appointment (appointmentData)
API-->>Hook: { id, appointment_date, appointment_time, ... }
Hook->>Notif: scheduleAppointmentReminder(title, content, date, time, id)
Notif->>Notif: compute reminder timestamp (5 min prior)
Notif->>OS: schedule notification trigger
OS-->>Notif: confirmation
Hook->>App: refresh appointments list
App->>User: render new appointment
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Fix all issues with AI agents
In `@Frontend/android/app/src/main/AndroidManifest.xml`:
- Around line 3-7: The manifest currently declares RECEIVE_BOOT_COMPLETED and
SCHEDULE_EXACT_ALARM but there is no boot receiver or runtime exact-alarm
handling; either remove the RECEIVE_BOOT_COMPLETED permission or add a
BroadcastReceiver (e.g., BootReceiver) registered for the BOOT_COMPLETED and
optionally REBOOT actions in AndroidManifest.xml that invokes your reminder
rescheduling logic (call the same reschedule method used by
NotificationService.js); additionally, in NotificationService.js add an Android
12+ exact-alarm check (use Platform APIs / canScheduleExactAlarms()) and
implement a fallback path to schedule inexact alarms or surface permission
request flow when exact alarms are not allowed so reminders still fire reliably.
In `@Frontend/App.jsx`:
- Around line 11-14: The useEffect currently calls NotificationService.init()
without awaiting or handling errors; wrap the initialization in an async
function (e.g., an async IIFE) inside useEffect, call await
NotificationService.init(), and surround it with try/catch to surface failures
from createDefaultChannel() or platform issues; in the catch, log the error and
optionally fall back or surface a user-facing error so silent failures are
avoided (refer to useEffect and NotificationService.init/createDefaultChannel in
your changes).
In `@Frontend/src/Components/Calendar/AppointmentDetailsModal.jsx`:
- Around line 21-25: changeDateFormat currently assumes a 'YYYY-MM-DD' string
and will break on ISO datetimes or Date objects; update the changeDateFormat
function to accept either a Date or string, convert strings to a Date via new
Date(date), check validity with isFinite(dateObj.getTime()) (return '' for
invalid input), then use dateObj.getDate(), dateObj.getMonth(), and
dateObj.getFullYear() to build the output using the existing months array
(months) so month indexing is correct and ISO datetimes/Date objects are handled
safely.
In `@Frontend/src/Components/Calendar/AppointmentFormModal.jsx`:
- Around line 19-159: The component AppointmentFormModal uses controlled
TextInput values directly from appointment (e.g., appointment.title,
appointment.content, appointment.appointment_date, appointment.appointment_time,
appointment.appointment_location) which will crash if appointment is
null/undefined; fix by guarding these accesses or providing a safe default
appointment (e.g., default parameter or local fallback like const safe =
appointment || {}) and use safe fields (or optional chaining with fallback:
safe.title ?? '' and formatTime handling null/undefined) and ensure
setAppointment calls still spread from safe to avoid uncontrolled inputs; update
references in AppointmentFormModal, the onChangeText handlers, and the
formatTime usage accordingly.
In `@Frontend/src/Components/Calendar/ScheduleTimeline.jsx`:
- Around line 21-24: The calculateTopOffset function can return negative values
for appointments before START_MINUTES; update calculateTopOffset (which uses
to_min, START_MINUTES and timeSlotHeight) to clamp the computed offset to a
minimum of 0 before returning (e.g., compute ((minutes -
START_MINUTES)/60)*timeSlotHeight then return Math.max(0, offset)) so cards for
pre-6:00 appointments do not render outside the visible area.
In `@Frontend/src/Components/Calendar/WeekDateSelector.jsx`:
- Around line 15-16: The isSelected check currently uses selectedDate.getDate()
=== date.getDate(), which only compares day-of-month and misidentifies dates
across months/years; update the comparison in the weekDates.map callback (the
isSelected logic) to compare full calendar dates — for example by comparing
date.getFullYear(), date.getMonth(), and date.getDate() between selectedDate and
date, or by comparing selectedDate.toDateString() === date.toDateString() (or
normalizing both to midnight and comparing getTime()) so only the exact same
date is marked selected.
In `@Frontend/src/hooks/useCalendar.js`:
- Around line 144-151: The notification cancel/reschedule is happening before
confirming the backend update and before fetchAppointments completes; move the
cancel and schedule calls to after fetchAppointments has successfully completed
(i.e., after awaiting fetchAppointments and verifying its success) so
notifications reflect the confirmed server state. Specifically, in the block
where you check response.ok, await fetchAppointments() (or check its returned
result) and only then call
NotificationService.cancelNotification(editAppointment.id) and
NotificationService.scheduleAppointmentReminder(...) using the editAppointment
fields; ensure any errors from fetchAppointments prevent rescheduling.
- Around line 68-79: fetchAppointments doesn't validate HTTP status or guard
against invalid JSON, so add response.ok checking after the fetch (using the
existing response variable) and throw a descriptive error including
response.status/text when it's not OK, then safely parse JSON inside the
try-catch (catch JSON.parse errors and log them), and on error ensure
setAppointments is not set to invalid data (e.g., leave previous state or set to
an empty array) while still calling setRefreshing(false) in the finally block;
reference the fetchAppointments function and state setters setRefreshing and
setAppointments and the BASE_URL constant when making these changes.
In `@Frontend/src/hooks/useTimeFormat.js`:
- Around line 11-13: In the useTimeFormat hook remove the stray debug logging:
delete the console.log(date) call so the hook no longer logs the Date object on
each mount; ensure you only keep the date creation (const date = new Date()) and
the formatted value (const formatted = date.toLocaleTimeString(...)) inside the
useTimeFormat function.
In `@Frontend/src/services/NotificationService.js`:
- Around line 49-79: The scheduleAppointmentReminder function currently splits
dateStr and timeStr without validation, which will throw if inputs are missing
or malformed; before splitting in scheduleAppointmentReminder, validate that
dateStr and timeStr are non-empty strings and match expected patterns (e.g.,
/^\d{4}-\d{2}-\d{2}$/ for date and /^\d{1,2}:\d{2}$/ for time) or that
Date.parse(date/time) yields a valid date, and if validation fails either
return/throw a clear error or fall back to a safe default (e.g., schedule
immediate notification), then proceed to parse and compute reminderDate and call
scheduleNotification as before; reference the scheduleAppointmentReminder and
scheduleNotification identifiers when applying the checks and handling invalid
inputs.
In `@Frontend/src/utils/timeUtils.js`:
- Around line 1-7: The formatTime function currently assumes timeString contains
"HH:MM" and parses hours/minutes into h and m, which can produce NaN; update
formatTime to validate the input by checking that timeString.split(':') yields
at least two parts and that parsed hours/minutes are valid numbers (use
Number.isFinite or !isNaN on the parsed values), and if validation fails either
return an empty string or default invalid parts to 0 before formatting; adjust
handling of variables hours, minutes, h, and m accordingly to avoid producing
`NaN` in the returned string.
🧹 Nitpick comments (7)
Frontend/src/services/NotificationService.js (1)
24-33: Add platform guard for clarity; iOS no-ops gracefully.
createChannelis Android-only and will no-op on iOS rather than throw, so the code is functionally safe. However, adding a platform guard improves code clarity and avoids unnecessary no-op calls. The try-catch is unnecessary since Notifee handles this gracefully.🔧 Suggested improvement
async createDefaultChannel() { + if (Platform.OS !== 'android') return; await notifee.createChannel({ id: this.CHANNEL_ID, name: 'BabyNest Notifications', description: "Reminders for your baby's journey", sound: 'default', importance: AndroidImportance.HIGH, vibration: true, }); }Frontend/src/hooks/useCalendar.js (3)
14-22:generateWeekDatesmutatesstartDateparameter.The function modifies
startDateindirectly becausenew Date(startDate)creates a date based on the input, but on each iteration,date.setDate(startDate.getDate() + i)uses the originalstartDatewhich could lead to unexpected behavior ifstartDatewere reused. More critically, this function is called on every render since it's defined inside the hook.♻️ Move function outside hook or memoize
+const generateWeekDates = startDate => { + const dates = []; + const start = new Date(startDate); + start.setHours(0, 0, 0, 0); + for (let i = 0; i < 7; i++) { + const date = new Date(start); + date.setDate(start.getDate() + i); + dates.push(date); + } + return dates; +}; + export const useCalendar = () => { const fadeAnim = useRef(new Animated.Value(0)).current; const [selectedDate, setSelectedDate] = useState(new Date()); - - const generateWeekDates = startDate => { - let dates = []; - for (let i = 0; i < 7; i++) { - let date = new Date(startDate); - date.setDate(startDate.getDate() + i); - dates.push(date); - } - return dates; - }; const [weekDates, setWeekDates] = useState(() => generateWeekDates(new Date()));
187-192: Usingalert()instead of Toast for consistency.The validation error on line 189 uses
alert()while other user feedback throughout the hook usesToast.show(). This creates an inconsistent UX.♻️ Use Toast for consistency
const handleDateConfirm = date => { if (date < new Date()) { - alert('Please select a future date and time.'); + Toast.show({ type: 'error', text1: 'Invalid Date', text2: 'Please select a future date and time.' }); setModals(m => ({ ...m, calendar: false, dateSelector: false })); return; }
59-61:fadeAnimin dependency array is unnecessary.The
fadeAnimref is stable (created viauseRef), so including it in the dependency array is superfluous. Consider using an empty dependency array since this animation should only run once on mount.♻️ Remove unnecessary dependency
useEffect(() => { Animated.timing(fadeAnim, { toValue: 1, duration: 800, useNativeDriver: true }).start(); -}, [fadeAnim]); +}, []);Frontend/src/Screens/HomeScreen.jsx (2)
90-101: Filtering logic is correct but creates redundant Date objects.The filtering approach is sound for showing future appointments. However,
new Date()is called inside the filter callback, meaning a new Date object is created for each appointment comparison. For a small number of appointments this is fine, but consider hoisting it.♻️ Hoist current date for efficiency
// ? Corrected Filtering Logic for Appointments (using real-time date) +const now = new Date(); const filteredAppointments = allAppointments .map(appt => { if (!appt.appointment_date || !appt.appointment_time) return null; const [year, month, day] = appt.appointment_date.split('-').map(Number); const [hours, minutes] = appt.appointment_time.split(':').map(Number); const apptDate = new Date(year, month - 1, day, hours, minutes); return {...appt, apptDate}; }) - .filter(appt => appt && appt.apptDate >= new Date()) // Future only + .filter(appt => appt && appt.apptDate >= now) // Future only .sort((a, b) => a.apptDate - b.apptDate) // Soonest first .slice(0, 2);
188-192: Redundant Date object creation in render.
new Date(appt.apptDate)is called twice on lines 190 and 191. Sinceappt.apptDateis already a Date object (created in the filtering logic), you can use it directly.♻️ Use apptDate directly
<View style={styles.dateBox}> - <Text style={styles.dateDay}>{new Date(appt.apptDate).getDate()}</Text> - <Text style={styles.dateMonth}>{new Date(appt.apptDate).toLocaleString('default', { month: 'short' })}</Text> + <Text style={styles.dateDay}>{appt.apptDate.getDate()}</Text> + <Text style={styles.dateMonth}>{appt.apptDate.toLocaleString('default', { month: 'short' })}</Text> </View>Frontend/src/Screens/CalendarScreen.jsx (1)
74-83: Inline style with zIndex wrapper may be unnecessary.The
Viewwrapper withzIndex: 9999around the firstDateTimePickerModalbut not the second suggests inconsistent handling. React Native modals typically manage their own z-ordering. If this was added to fix a specific layering issue, consider documenting why.
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@Frontend/src/Components/Calendar/ScheduleTimeline.jsx`:
- Around line 7-11: The to_min function can yield NaN for malformed strings;
update to_min to robustly parse and validate hours/minutes (use parseInt with
radix 10 and trimming), check Number.isFinite for both parts, and if either part
is invalid return 0 so downstream callers like calculateTopOffset and the sort
comparator never receive NaN; mirror the NaN-guarding approach used in
formatTime and ensure to_min always returns a safe integer (0 when parsing
fails).
In `@Frontend/src/Components/Calendar/WeekDateSelector.jsx`:
- Around line 1-8: The component calls useTheme inside WeekDateSelector but
never imports it; add the missing import (e.g., import { useTheme } from
'react-native-paper'; or from your project's theme provider) at the top of
WeekDateSelector.jsx so useTheme is defined when WeekDateSelector runs; ensure
the import matches the app's theme library.
In `@Frontend/src/hooks/useCalendar.js`:
- Around line 77-84: The parsed JSON assigned via setAppointments may not be an
array and will break dailyAppointments (which calls appointments.filter); update
the try block in useCalendar.js to validate the parsed data before calling
setAppointments: check Array.isArray(data) (and treat null/undefined as not
array) and only set the array when valid, otherwise setAppointments([]) or
optionally extract an array field if your API wraps results; reference the
setAppointments call and the dailyAppointments consumer to ensure appointments
is always an array.
- Around line 101-109: In addAppointment and updateAppointment, avoid calling
response.json() before verifying response.ok: first check if response.ok; if
not, read the raw response (e.g., await response.text()) or build an Error that
includes response.status and the response body, throw it so the catch can show a
meaningful message, and only call await response.json() when response.ok is true
to parse the successful JSON payload.
In `@Frontend/src/services/NotificationService.js`:
- Around line 88-98: The message construction currently uses diffMins and always
plugs it into "in X minutes", causing "in 1 minutes"; update the logic in the
reminder scheduling block (where variables diffMins, message, scheduleDate,
reminderDate, now, date and constant REMINDER_OFFSET_MINUTES are used) to choose
singular/plural: if diffMins === 1 set message = "in 1 minute" (or "in a
minute"), otherwise set message = `in ${diffMins} minutes`; keep the existing
"now" branch unchanged.
- Around line 109-110: The console.log in scheduleNotification calls
date.toISOString() which can throw for invalid Dates; guard against that by
validating the date before logging (e.g., ensure date is a Date and not NaN via
date instanceof Date && !isNaN(date.getTime())) or compute the ISO string inside
a try/catch and fall back to a safe representation like String(date) or 'invalid
date'; alternatively move the log inside the existing try block started later so
any exception is caught. Update scheduleNotification (and consider callers like
scheduleAppointmentReminder) to use the safe/guarded logging approach so
malformed Dates won't crash the function.
🧹 Nitpick comments (10)
Frontend/src/Components/Calendar/AppointmentDetailsModal.jsx (3)
82-98: Inconsistent callback ordering between Edit and Delete handlers.Edit calls
onClose()before the action callback, while Delete calls the action callback beforeonClose(). The Delete ordering is arguably safer — performing the action while theappointmentreference is still guaranteed to be live. Consider aligning both to call the action first, then close.Also, both action buttons are missing
accessibilityLabelprops (the close button has one — good — but these should too).Suggested fix
<TouchableOpacity style={[styles.button, { backgroundColor: theme.primary }]} onPress={() => { - onClose(); onEdit(appointment); - }}> - <Text style={styles.buttonText}>Edit</Text> + onClose(); + }} + accessibilityLabel="Edit appointment"> + <Text style={styles.buttonText}>Edit</Text> </TouchableOpacity> <TouchableOpacity style={[styles.button, styles.deleteButton]} onPress={() => { onDelete(appointment.id); onClose(); - }}> + }} + accessibilityLabel="Delete appointment"> <Text style={styles.deleteButtonText}>Delete</Text> </TouchableOpacity>
106-236: Hardcoded colors bypass the theming system — dark mode will look broken.The component retrieves
themefromuseTheme()but only applies it to a handful of elements (icon color, badge, edit button). Everything else — modal background (#fff), text colors (#333,#444,#666), overlay, divider, close button background, delete button border — is hardcoded to light-theme values.If the app supports dark mode (or plans to), this modal will render as a bright white card regardless of the active theme. Consider replacing the hardcoded color literals with values from
theme, consistent with how other components in this PR (e.g.,AppointmentFormModal,WeekDateSelector) consume the theme.
55-55: Add parentheses around the fallback expression for clarity.
theme.appointment || theme.primary + '20'relies on+binding tighter than||, which is correct but easy to misread. Also, appending'20'only produces a valid color iftheme.primaryis a 6-digit hex string — it would break withrgb()/hsl()formats.-<View style={[styles.topBadge, { backgroundColor: theme.appointment || theme.primary + '20' }]} /> +<View style={[styles.topBadge, { backgroundColor: theme.appointment || (theme.primary + '20') }]} />Frontend/src/Components/Calendar/WeekDateSelector.jsx (1)
7-45: Consider adding basic prop validation or defaults for resilience.If
weekDatesorselectedDateisundefined(e.g., during initial render before the hook state is ready), the component will crash on.map()or.toDateString(). A simple default or guard would make this more robust.♻️ Example defensive defaults
-const WeekDateSelector = ({ weekDates, selectedDate, onDateSelect }) => { +const WeekDateSelector = ({ weekDates = [], selectedDate, onDateSelect }) => { const { theme } = useTheme(); return ( <ScrollView horizontal showsHorizontalScrollIndicator={false} style={styles.weekDatesScroll}> - {weekDates.map((date, index) => { - const isSelected = selectedDate.toDateString() === date.toDateString(); + {weekDates.map((date, index) => { + const isSelected = selectedDate?.toDateString() === date.toDateString();Frontend/src/services/NotificationService.js (1)
146-161: Add error handling togetScheduledNotifications,cancelNotification, andcancelAllfor consistency.Other public methods (
configure,scheduleNotification) wrap notifee calls in try/catch blocks, but these three do not. If a notifee call fails, an unhandled rejection will propagate to the caller. For consistency and resilience, wrap these in try/catch as well.Additionally,
getScheduledNotificationsincludes an unusedcallbackparameter that mixes callback and Promise patterns. Drop the callback parameter since callers can simplyawaitthe result.♻️ Suggested fix (error handling + remove callback)
- async getScheduledNotifications(callback) { - const notifications = await notifee.getTriggerNotifications(); - console.log('[NotificationService] Retrieved scheduled:', notifications); - if (callback) { - callback(notifications); - } - return notifications; + async getScheduledNotifications() { + try { + const notifications = await notifee.getTriggerNotifications(); + console.log('[NotificationService] Retrieved scheduled:', notifications); + return notifications; + } catch (e) { + console.error('[NotificationService] Error retrieving notifications:', e); + return []; + } } - async cancelNotification(id) { - await notifee.cancelNotification(String(id)); + async cancelNotification(id) { + try { + await notifee.cancelNotification(String(id)); + } catch (e) { + console.error('[NotificationService] Error cancelling notification:', e); + } } - async cancelAll() { - await notifee.cancelAllNotifications(); + async cancelAll() { + try { + await notifee.cancelAllNotifications(); + } catch (e) { + console.error('[NotificationService] Error cancelling all notifications:', e); + } }Frontend/src/utils/timeUtils.js (1)
6-7: Use explicit radix and preferconstfor non-reassigned variables.
parseIntwithout a radix can behave unexpectedly with certain inputs (e.g.,"09"on legacy engines).his never reassigned, soconstis appropriate.♻️ Proposed fix
- let h = parseInt(parts[0]); - const m = parseInt(parts[1]); + const h = parseInt(parts[0], 10); + const m = parseInt(parts[1], 10);Frontend/src/Components/Calendar/ScheduleTimeline.jsx (2)
91-91: Unused variablecolWidth.
colWidthis computed on every render iteration but never referenced. Remove it to avoid confusion.🧹 Proposed fix
{processedAppointments.events.map((appt, index) => { - const colWidth = (100 - 15) / processedAppointments.numColumns; - return (
94-105: Percentage-basedleftoffset may misalign with the fixed-width time label column.The time label column is a fixed
50px(line 134), but appointmentleftstarts at18%(line 102). On narrow screens 18% ≈ 65px, on wide tablets 18% could be much larger, causing cards to float away from the guide lines. Consider computingleftfrom the same fixed pixel width or usingDimensions/onLayoutfor consistency.Frontend/src/hooks/useCalendar.js (2)
114-130: Notification errors after a successful add will show a misleading toast.If
showLocalNotificationorscheduleAppointmentReminderthrows (e.g., missing Android channel, permission denied), the outercatchon line 135 displays "Error adding appointment!" — even though the appointment was successfully created on the server. Wrap notification calls in their own try-catch:🛡️ Proposed fix
// Notifications + try { NotificationService.showLocalNotification( "Appointment Created", `"${newAppointment.title}" scheduled for ${newAppointment.appointment_date} at ${newAppointment.appointment_time}.` ); if (data && data.id) { NotificationService.scheduleAppointmentReminder( newAppointment.title, newAppointment.content, newAppointment.appointment_date, newAppointment.appointment_time, data.id ); } else { console.warn('[useCalendar] No ID returned from backend, skipping reminder schedule.'); } + } catch (notifError) { + console.warn('[useCalendar] Notification failed:', notifError); + }The same pattern applies to
updateAppointment(lines 153-164) anddeleteAppointment(lines 183-187).
200-205: PreferAlert.alert()over the globalalert()in React Native.The global
alert()works in RN butAlert.alert()fromreact-nativeis the standard API, providing consistent behavior and customization (e.g., buttons, callbacks).Alertis already imported fromreact-nativeon line 2 viaAnimated— just add it to the import.♻️ Proposed fix
-import { Animated, Platform } from 'react-native'; +import { Alert, Animated, Platform } from 'react-native';- alert('Please select a future date and time.'); + Alert.alert('Invalid Date', 'Please select a future date and time.');
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@Frontend/src/hooks/useCalendar.js`:
- Around line 189-209: In deleteAppointment, ensure fetchAppointments() is
awaited so the appointment list refreshes before the modal is closed: after a
successful response in deleteAppointment, call await fetchAppointments()
(instead of a fire-and-forget call) before showing/cancelling notifications or
letting the finally block run; this guarantees setModals in finally closes the
details modal only after fetchAppointments completes. Reference:
deleteAppointment, fetchAppointments, setModals, selectedAppointment,
NotificationService.
🧹 Nitpick comments (4)
Frontend/src/services/NotificationService.js (2)
161-167: Missing error handling incancelNotificationandcancelAll.Both methods call notifee APIs without try/catch. If a notification ID doesn't exist or notifee throws, the error will propagate unhandled to the caller (e.g.,
deleteAppointmentinuseCalendar), potentially disrupting the user flow even though the primary action (backend delete) succeeded.🛡️ Proposed fix
async cancelNotification(id) { - await notifee.cancelNotification(String(id)); + try { + await notifee.cancelNotification(String(id)); + } catch (e) { + console.warn('[NotificationService] Error cancelling notification:', e); + } } async cancelAll() { - await notifee.cancelAllNotifications(); + try { + await notifee.cancelAllNotifications(); + } catch (e) { + console.warn('[NotificationService] Error cancelling all notifications:', e); + } }
152-159: Mixed return + callback pattern ingetScheduledNotifications.The method both returns the notifications array and optionally invokes a callback. This dual pattern is unusual — callers can simply
awaitthe return value. Consider deprecating the callback parameter unless there's a specific consumer that needs it.Frontend/src/hooks/useCalendar.js (2)
213-218: UseAlert.alert()instead of globalalert()for cross-platform consistency.Line 215 uses the global
alert()function. In React Native, the globalalertworks but is not the recommended API —Alert.alert()fromreact-nativeprovides consistent behavior across platforms and supports customization.Alertis already imported inNotificationService.jsbut not here.♻️ Proposed fix
-import { Animated, Platform } from 'react-native'; +import { Animated, Platform, Alert } from 'react-native';const handleDateConfirm = date => { if (date < new Date()) { - alert('Please select a future date and time.'); + Alert.alert('Invalid Date', 'Please select a future date and time.'); setModals(m => ({ ...m, calendar: false, dateSelector: false })); return; }
14-22:generateWeekDatesis redefined on every render.Since this utility function doesn't depend on any hook state or props, it could be extracted outside the hook to avoid re-creation on each render. It's also used in
useStateinitialization (line 23) and inhandleDateConfirm(line 239).
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@Frontend/src/hooks/useCalendar.js`:
- Around line 155-183: The code has a redundant response.ok check after an early
throw for non-OK responses: once you throw on !response.ok (in useCalendar's
edit/update flow), the later if (response.ok) / else is dead code; remove the
if/else and keep only the success path (Toast.show, await fetchAppointments(),
NotificationService.showLocalNotification/cancelNotification/scheduleAppointmentReminder,
setModals, setEditAppointment) after const data = await response.json(); also
ensure any error messaging uses the catch block (or the thrown Error) instead of
the removed else branch so failures still surface; refer to the edit flow
handlers and functions like fetchAppointments,
NotificationService.scheduleAppointmentReminder, setModals, and
setEditAppointment to locate and update the code.
- Around line 108-141: In useCalendar.js, remove the redundant response.ok
branch after you already throw on !response.ok: delete the unreachable else
block that shows the error Toast and instead handle the successful path only
(keep closeModals(), await fetchAppointments(), success Toast,
NotificationService.showLocalNotification(...) and scheduleAppointmentReminder
when data.id exists, otherwise log the warning); apply the same cleanup in
updateAppointment; also make fetchAppointments() awaited so the appointment list
is refreshed before UI transitions.
🧹 Nitpick comments (1)
Frontend/src/hooks/useCalendar.js (1)
213-218: UseAlert.alert()instead of the globalalert()for cross-platform consistency.The global
alert()is not the idiomatic React Native API and behaves inconsistently across platforms. TheTimelineScreen(line 174) usesAlert.alert()for the same validation. You'll need to addAlertto thereact-nativeimport on line 2.♻️ Proposed fix
-import { Animated, Platform } from 'react-native'; +import { Alert, Animated, Platform } from 'react-native';const handleDateConfirm = date => { if (date < new Date()) { - alert('Please select a future date and time.'); + Alert.alert('Invalid Date', 'Please select a future date and time.'); setModals(m => ({ ...m, calendar: false, dateSelector: false })); return; }
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@Frontend/src/hooks/useCalendar.js`:
- Around line 216-244: The handler handleDateConfirm is overwriting
appointment_time with an empty string when modals.dateSelector is true; change
the setNewAppointment and setEditAppointment updates so they preserve the
existing appointment_time when no time was selected: compute appointmentTime
only when !modals.dateSelector (as now), but when updating call
setNewAppointment(prev => ({ ...prev, appointment_date: appointmentDate,
...(appointmentTime !== '' ? { appointment_time: appointmentTime } : {}) })) and
similarly for setEditAppointment so that appointment_time is only set when
appointmentTime is non-empty (or fallback to prev.appointment_time), ensuring
the date-only selector does not clear previously chosen times.
- Around line 150-190: The updateAppointment function lacks the same input
validation as addAppointment and unnecessarily calls response.json() even when
the server may return 204; add pre-flight validation for editAppointment.title,
editAppointment.appointment_date, and editAppointment.appointment_time and bail
out with Toast.show if missing (mirroring addAppointment), then after the PUT
request only call response.json() when response.ok and the response has a JSON
content-type (or response.status !== 204) to avoid throwing on empty responses;
update references to updateAppointment, editAppointment, fetchAppointments,
Toast.show, and setEditAppointment accordingly so the flow validates inputs,
performs the PUT, conditionally parses the body, and proceeds to notifications
on success.
🧹 Nitpick comments (4)
Frontend/src/hooks/useCalendar.js (4)
1-5: UnusedPlatformimport.
Platformis imported fromreact-nativebut never referenced in this file. This is minor but worth cleaning up.♻️ Proposed fix
-import { Animated, Platform } from 'react-native'; +import { Animated } from 'react-native';
217-221: Inconsistent error feedback:alert()vsToast.All other user-facing messages in this hook use
Toast.show(...), but Line 218 usesalert(). This creates an inconsistent UX (native dialog vs. toast overlay). Consider using Toast here as well for consistency.♻️ Proposed fix
if (date < new Date()) { - alert('Please select a future date and time.'); + Toast.show({ type: 'error', text1: 'Please select a future date and time.' }); setModals(m => ({ ...m, calendar: false, dateSelector: false })); return; }
145-147: Catch blocks discard specific error information.In both
addAppointment(Line 146) andupdateAppointment(Line 188), thethrow new Error(...)on the!response.okpath includes the HTTP status and server error text, but the catch block shows a hardcoded generic toast and swallowserror.message. This makes debugging harder for both developers and users.♻️ Proposed fix (apply similarly to updateAppointment)
} catch (error) { - Toast.show({ type: 'error', text1: 'Error adding appointment!' }); + console.error('[useCalendar] addAppointment error:', error); + Toast.show({ type: 'error', text1: 'Error adding appointment!', text2: error.message }); }
192-212: Error-handling pattern indeleteAppointmentdiffers fromaddAppointment/updateAppointment.
addAppointmentandupdateAppointmentreadresponse.text()on!response.okand throw, whiledeleteAppointmentchecksresponse.okwith anif/elseand shows a generic toast on failure. This inconsistency is minor but worth noting — adopting the same early-throw pattern would keep error handling uniform.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@Frontend/src/hooks/useCalendar.js`:
- Around line 228-233: The date-only Android picker returns midnight so
handleDateConfirm's timestamp comparison rejects "today"; update
handleDateConfirm to normalize to date-only granularity when modals.dateSelector
is true (e.g., compare dates after zeroing time or using startOfDay on both
selected date and now) before deciding it's in the past, then close modals with
setModals(m => ({ ...m, calendar: false, dateSelector: false })) as before; also
replace the alert(...) call with the project's Toast.show pattern (use
Toast.show with the same message and type) so UX is consistent.
🧹 Nitpick comments (2)
Frontend/src/hooks/useCalendar.js (2)
145-147: Error details silently discarded in catch block.The thrown error on line 119 includes the HTTP status and server error text, but the catch block here only shows a generic toast and doesn't log
error. The same pattern exists inupdateAppointment(line 199-201). Consider logging the error for debuggability.🔧 Proposed fix
} catch (error) { + console.error('[useCalendar] addAppointment error:', error); - Toast.show({ type: 'error', text1: 'Error adding appointment!' }); + Toast.show({ type: 'error', text1: 'Error adding appointment!', text2: error.message }); }
204-224: Minor: inconsistent error-handling pattern compared to add/update.
addAppointmentandupdateAppointmentuse early-throw on!response.okand rely on the catch block, whiledeleteAppointmentuses an if/else branch. This works but makes the CRUD error contract inconsistent. Consider aligning for maintainability.
|
@bhavik-mangla |
1 participant
#56
📝 Description
This PR implements a robust notification system using the
notifeelibrary to ensure reliable appointment reminders. It also includes a major refactor of the Calendar screen, extracting complex logic into a custom [useCalendar] hook for better maintainability and separation of concerns. Additionally, visual improvements were made to the Schedule Timeline.🔨 Changes Made
Notification Service
@notifee/react-nativefor better Android support.to trigger notifications 5 minutes before the appointment time.
Refactoring
UI/UX Improvements
📸 Screenshots or Visual Changes (if applicable)
Demo Link : https://drive.google.com/file/d/1zs52TRWwqfACyWuBz90v21tGHp5BGO3K/view?usp=sharing
✅ Checklist
Summary by CodeRabbit
New Features
Dependencies
Chores
Platform