Conversation
📝 WalkthroughWalkthroughA notification system is introduced with initialization and scheduling capabilities. The App component initializes notifications on startup, while CalendarScreen and MedicineScreen schedule notifications when creating appointments and medicine entries. ChatScreen adds AI agent availability tracking with user-facing alerts when the service is unavailable. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Frontend/src/Screens/MedicineScreen.jsx (1)
48-75: Add error handling for notification scheduling.The
scheduleNotificationcall has no error handling. If notification scheduling fails, users won't be informed and the medicine entry will be saved without a reminder.🔎 Add try-catch wrapper
try { await fetch(`${BASE_URL}/set_medicine`, { method: 'POST', headers: {'Content-Type': 'application/json'}, body: JSON.stringify({week_number: week, name, dose, time, note}), }); + + const medicineName = name; + const medicineDose = dose; + setWeek(''); setName(''); setDose(''); setTime(''); setNote(''); - scheduleNotification( - 'Medicine Reminder', - `${name} - ${dose}`, - new Date(Date.now() + 10000) // test: 10 seconds later -); + + try { + scheduleNotification( + 'Medicine Reminder', + `${medicineName} - ${medicineDose}`, + new Date(Date.now() + 24 * 60 * 60 * 1000) + ); + } catch (notifErr) { + console.error('Failed to schedule notification:', notifErr); + Alert.alert('Warning', 'Medicine saved but notification scheduling failed.'); + } + fetchMedicineHistory(); } catch (err) {Frontend/src/Screens/ChatScreen.jsx (1)
1-934: Clarify the relationship between agent availability changes and notification feature.This PR is titled "Feature/in app notifications" and aims to "implement in-app notifications for upcoming tasks/appointments." However, the changes in ChatScreen.jsx are exclusively about agent availability tracking (checking if AI assistant is available, showing alerts when unavailable, disabling input).
According to the AI summary, the notification system is implemented in App.jsx (initialization) and CalendarScreen/MedicineScreen (scheduling), but those files weren't provided for review. The agent availability changes in ChatScreen appear unrelated to the notification feature.
Please clarify:
- Are the agent availability changes intentionally part of this PR, or should they be in a separate PR?
- How do these changes relate to the notification feature for tasks/appointments?
🧹 Nitpick comments (4)
Frontend/src/services/NotificationService.js (3)
7-29: Remove console.log statements from production code.Lines 17 and 23 contain
console.logstatements that should be removed or replaced with a proper logging solution before production deployment.🔎 Remove console.log statements
PushNotification.createChannel( { channelId: 'default-channel-id', channelName: 'BabyNest Notifications', channelDescription: 'Reminders for appointments and medicines', importance: 4, vibrate: true, }, - (created) => console.log('Notification channel created:', created) + (created) => { + if (!created) { + console.error('Failed to create notification channel'); + } + } ); // ✅ CONFIGURATION PushNotification.configure({ onNotification: function (notification) { - console.log('Notification received:', notification); + // Handle notification tap/interaction here },
22-24: Notification handler is not implemented.The
onNotificationcallback currently only logs the notification. Consider implementing actual notification handling logic (e.g., navigation to relevant screen when notification is tapped).Would you like me to help implement notification tap handling that navigates users to the appropriate screen based on notification type?
1-42: Consider adding notification management functions.The service currently only supports initializing and scheduling notifications. Consider adding functions to:
- Cancel scheduled notifications
- List scheduled notifications
- Cancel all notifications
- Update existing notifications
These would provide better control over the notification lifecycle.
Would you like me to help implement additional notification management functions?
Frontend/src/Screens/ChatScreen.jsx (1)
904-934: Minor improvement: Consistent function declaration style.The refactor to an arrow function for
createMarkdownStylesis consistent with modern React/JavaScript patterns and improves code consistency.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
Backend/schema.sqlFrontend/.expo/README.mdFrontend/.expo/devices.jsonFrontend/App.jsxFrontend/package.jsonFrontend/src/Screens/CalendarScreen.jsxFrontend/src/Screens/ChatScreen.jsxFrontend/src/Screens/MedicineScreen.jsxFrontend/src/services/NotificationService.js
🧰 Additional context used
🧬 Code graph analysis (2)
Frontend/src/Screens/CalendarScreen.jsx (1)
Frontend/src/Screens/TimelineScreen.jsx (1)
newAppointment(33-33)
Frontend/src/Screens/ChatScreen.jsx (4)
Frontend/src/services/RAGService.js (2)
ragService(3664-3664)ragService(3664-3664)Frontend/src/context/AgentContext.jsx (2)
refreshContext(68-94)error(29-29)Frontend/src/services/ConversationContext.js (2)
conversationContext(384-384)conversationContext(384-384)Frontend/src/theme/ThemeContext.js (1)
theme(63-63)
🔇 Additional comments (5)
Backend/schema.sql (1)
103-104: LGTM - formatting adjustment.This is a formatting-only change with no functional impact.
Frontend/.expo/README.md (1)
1-8: LGTM - standard Expo documentation.This documentation accurately describes the
.expofolder's purpose and handling conventions.Frontend/package.json (1)
46-48: Clarify purpose of web platform dependencies.The dependencies
react-dom,react-native-web, and@expo/metro-runtimeare declared inpackage.jsonbut have no usage anywhere in the codebase. They are not referenced in notification services, App.jsx, or screen components. Please clarify why these web platform dependencies were added as part of this notification PR—are they needed for future web support, or should they be removed?Frontend/src/Screens/ChatScreen.jsx (2)
305-308: Good addition: Context refresh after successful commands.Refreshing the agent context after successful command execution ensures the AI has up-to-date information for subsequent interactions. This is a valuable improvement to the agent's reliability.
665-670: Good UX: Clear visual feedback for agent availability.The conditional disabling of the input field and send button, along with the updated placeholder text and opacity changes, provides clear visual feedback to users when the AI assistant is unavailable. This prevents confusion and failed message attempts.
Also applies to: 674-684
| { | ||
| "devices": [] | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check .gitignore for .expo entries
echo "=== Checking .gitignore for .expo ==="
if [ -f .gitignore ]; then
grep -n "\.expo" .gitignore || echo ".expo not found in .gitignore"
else
echo ".gitignore not found"
fi
echo ""
echo "=== Files in Frontend/.expo being tracked ==="
git ls-files Frontend/.expo/ 2>/dev/null || echo "No tracked files found"
echo ""
echo "=== Checking for README.md in Frontend/.expo ==="
if [ -f Frontend/.expo/README.md ]; then
echo "README.md found. First 20 lines:"
head -20 Frontend/.expo/README.md
else
echo "README.md not found in Frontend/.expo"
fi
echo ""
echo "=== Directory structure of Frontend/.expo ==="
find Frontend/.expo -type f 2>/dev/null | head -20Repository: AOSSIE-Org/BabyNest
Length of output: 1164
Add .expo to .gitignore and remove tracked files.
The .expo folder is currently being tracked in version control (Frontend/.expo/README.md and Frontend/.expo/devices.json), but according to the README.md in this folder, it "does not contain any information that is relevant for other developers working on the project, it is specific to your machine." Add /.expo/ to the repository's .gitignore and remove these files from the commit.
🤖 Prompt for AI Agents
In Frontend/.expo/ (lines 1-3) the files Frontend/.expo/README.md and
Frontend/.expo/devices.json are being tracked but are machine-specific; add
"/.expo/" to the repository .gitignore, remove the tracked files from Git with
git rm --cached Frontend/.expo/README.md Frontend/.expo/devices.json (or git rm
-r --cached Frontend/.expo), commit the change with a clear message (e.g.,
"chore: ignore .expo and remove tracked files"), and push the commit so the
.expo folder stays untracked going forward.
| import { AgentProvider } from './src/context/AgentContext'; | ||
| import StackNavigation from './src/navigation/StackNavigator'; | ||
|
|
||
| import { initNotifications } from './src/services/notification-service'; |
There was a problem hiding this comment.
Critical: Import path doesn't match actual file name.
The import uses './src/services/notification-service' but the actual file is named NotificationService.js (PascalCase). This will cause a module resolution error at runtime.
🔎 Fix import path
-import { initNotifications } from './src/services/notification-service';
+import { initNotifications } from './src/services/NotificationService';📝 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.
| import { initNotifications } from './src/services/notification-service'; | |
| import { initNotifications } from './src/services/NotificationService'; |
🤖 Prompt for AI Agents
In Frontend/App.jsx around line 7, the import path uses
'./src/services/notification-service' but the actual module file is named
NotificationService.js (PascalCase); update the import to point to the correct
file name (e.g. './src/services/NotificationService' or
'./src/services/NotificationService.js') and verify that the exported symbol is
named initNotifications (adjust the import name if the module exports a
different identifier), or alternatively rename the file to match the import
(notification-service.js) so the file name and import casing agree.
| useEffect(() => { | ||
| initNotifications(); | ||
| }, []); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add error handling for notification initialization.
If notification initialization fails (e.g., due to missing permissions or unsupported platform), the app will crash or fail silently. Consider adding error handling to gracefully handle initialization failures.
🔎 Add try-catch for initialization
useEffect(() => {
- initNotifications();
+ try {
+ initNotifications();
+ } catch (error) {
+ console.error('Failed to initialize notifications:', error);
+ // App can continue without notifications
+ }
}, []);📝 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.
| useEffect(() => { | |
| initNotifications(); | |
| }, []); | |
| useEffect(() => { | |
| try { | |
| initNotifications(); | |
| } catch (error) { | |
| console.error('Failed to initialize notifications:', error); | |
| // App can continue without notifications | |
| } | |
| }, []); |
🤖 Prompt for AI Agents
In Frontend/App.jsx around lines 11 to 13, the useEffect calls
initNotifications() with no error handling which can crash or fail silently if
initialization throws (permissions/platform issues). Wrap the initNotifications
call in a try-catch inside the useEffect, call initNotifications await if it
returns a promise, and on error log the error (console.error or app logger) and
set a safe fallback state or disable notification features so the app continues
running gracefully; optionally surface a non-blocking user-facing message that
notifications are unavailable.
| "react-dom": "18.3.1", | ||
| "react-native-web": "~0.19.13", | ||
| "@expo/metro-runtime": "~4.0.1" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate NotificationService.js
find . -name "NotificationService.js" -o -name "*Notification*Service*" | head -20Repository: AOSSIE-Org/BabyNest
Length of output: 109
🏁 Script executed:
# Check the Frontend/package.json file to see current dependencies
cat Frontend/package.jsonRepository: AOSSIE-Org/BabyNest
Length of output: 2565
🌐 Web query:
react-native-push-notification latest version compatible React Native 0.77.1
💡 Result:
Latest published release: react-native-push-notification v8.1.1. [GitHub releases][1], [npm/Snyk][2]
Compatibility note: the original zo0r repository was archived (read‑only) in Jan 2025 and the package shows signs of low maintenance; there is no official statement guaranteeing support for React Native 0.77.1. Test v8.1.1 in your project (or consider maintained alternatives/forks) before upgrading to RN 0.77.1. [GitHub archive][1], [Snyk/npm][2]
Sources:
[1] GitHub releases — zo0r/react-native-push-notification (v8.1.1).
[2] npm / security listing for react-native-push-notification (latest v8.1.1).
🏁 Script executed:
# Check what NotificationService.js actually imports
cat Frontend/src/services/NotificationService.jsRepository: AOSSIE-Org/BabyNest
Length of output: 1111
Critical: Missing react-native-push-notification dependency.
NotificationService.js imports react-native-push-notification at the top of the file, but this package is not listed in dependencies. This will cause a runtime import error when the notification service initializes.
Add the following to the dependencies section:
"react-navigation": "^4.4.4",
"react-navigation-bottom-tabs": "^1.0.2",
"react-dom": "18.3.1",
"react-native-web": "~0.19.13",
- "@expo/metro-runtime": "~4.0.1"
+ "@expo/metro-runtime": "~4.0.1",
+ "react-native-push-notification": "^8.1.1"Note: The upstream react-native-push-notification package was archived in January 2025 and shows no official support for React Native 0.77.1. Test this version thoroughly in your environment, or consider maintained alternatives/community forks.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
Frontend/package.json around lines 46-48: package.json is missing the
react-native-push-notification dependency required by NotificationService.js —
add an appropriate version of "react-native-push-notification" to the
dependencies section of package.json, run your package manager install to update
lockfiles, rebuild the app and verify NotificationService initializes without
import/runtime errors; if compatibility issues arise with RN 0.77.1, swap to a
maintained fork or alternative notification library and update imports
accordingly.
| @@ -1,4 +1,5 @@ | |||
| import React, {useEffect, useRef, useState} from 'react'; | |||
| import { scheduleNotification } from '../services/notification-service'; | |||
There was a problem hiding this comment.
Critical: Import path doesn't match actual file name.
The import uses '../services/notification-service' but the actual file is named NotificationService.js (PascalCase). This will cause a module resolution error at runtime.
🔎 Fix import path
-import { scheduleNotification } from '../services/notification-service';
+import { scheduleNotification } from '../services/NotificationService';📝 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.
| import { scheduleNotification } from '../services/notification-service'; | |
| import { scheduleNotification } from '../services/NotificationService'; |
🤖 Prompt for AI Agents
In Frontend/src/Screens/CalendarScreen.jsx around line 2, the import path uses
'../services/notification-service' but the actual module file is named
NotificationService.js; update the import to point to the correct filename (e.g.
'../services/NotificationService' or '../services/NotificationService.js') so
the module resolves at runtime, keeping the named import scheduleNotification
as-is and adjusting the path casing to match the filesystem.
| @@ -1,4 +1,5 @@ | |||
| import React, {useState, useEffect} from 'react'; | |||
| import { scheduleNotification } from '../services/notification-service'; | |||
There was a problem hiding this comment.
Critical: Import path doesn't match actual file name.
The import uses '../services/notification-service' but the actual file is named NotificationService.js (PascalCase). This will cause a module resolution error at runtime.
🔎 Fix import path
-import { scheduleNotification } from '../services/notification-service';
+import { scheduleNotification } from '../services/NotificationService';📝 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.
| import { scheduleNotification } from '../services/notification-service'; | |
| import { scheduleNotification } from '../services/NotificationService'; |
🤖 Prompt for AI Agents
In Frontend/src/Screens/MedicineScreen.jsx around line 2, the import path uses
'../services/notification-service' but the actual module file is named
NotificationService.js; update the import to match the exact file name and
casing (e.g., '../services/NotificationService') and ensure the exported member
name (scheduleNotification) matches the module's export so the module resolves
correctly at runtime.
| scheduleNotification( | ||
| 'Medicine Reminder', | ||
| `${name} - ${dose}`, | ||
| new Date(Date.now() + 10000) // test: 10 seconds later | ||
| ); |
There was a problem hiding this comment.
Critical: Notification uses reset form values and contains test code.
Two issues here:
- The notification is scheduled after state variables are reset to empty strings (lines 60-64), so
nameanddosewill be empty in the notification message. - The 10-second delay with "test:" comment is hardcoded test code that shouldn't be in production.
🔎 Proposed fix
+ // Capture values before reset
+ const medicineName = name;
+ const medicineDose = dose;
+
setWeek('');
setName('');
setDose('');
setTime('');
setNote('');
- scheduleNotification(
- 'Medicine Reminder',
- `${name} - ${dose}`,
- new Date(Date.now() + 10000) // test: 10 seconds later
-);
+
+ // Schedule notification for actual medicine time
+ // TODO: Calculate proper notification time based on week and time input
+ scheduleNotification(
+ 'Medicine Reminder',
+ `${medicineName} - ${medicineDose}`,
+ new Date(Date.now() + 24 * 60 * 60 * 1000) // 24 hours ahead as placeholder
+ );
fetchMedicineHistory();Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In Frontend/src/Screens/MedicineScreen.jsx around lines 65 to 69, the scheduled
notification is using form state after it's been reset and contains a hardcoded
10-second test delay; move the scheduleNotification call to before you clear the
form state so it captures the current name and dose values, and replace the
hardcoded new Date(Date.now() + 10000) with the actual scheduled time (e.g., the
user-selected datetime variable or a validated computed Date); then remove the
"test" hardcode and ensure you validate the scheduled time (not in the past)
before scheduling.
| @@ -0,0 +1,42 @@ | |||
| import PushNotification from 'react-native-push-notification'; | |||
There was a problem hiding this comment.
Critical: Missing package dependency.
This file imports react-native-push-notification, but this package is not listed in package.json. This must be added to the dependencies.
See the comment on Frontend/package.json for the required fix.
🤖 Prompt for AI Agents
In Frontend/src/services/NotificationService.js around line 1, the file imports
'react-native-push-notification' but that package is not declared in
Frontend/package.json; add 'react-native-push-notification' to the
Frontend/package.json dependencies (use the version specified in the comment on
Frontend/package.json or the latest compatible version for the project), run
npm/yarn install to update lockfile, and ensure the app is rebuilt so the native
module is linked.
| export const initNotifications = () => { | ||
| // ✅ ANDROID CHANNEL (MANDATORY) | ||
| PushNotification.createChannel( | ||
| { | ||
| channelId: 'default-channel-id', | ||
| channelName: 'BabyNest Notifications', | ||
| channelDescription: 'Reminders for appointments and medicines', | ||
| importance: 4, | ||
| vibrate: true, | ||
| }, | ||
| (created) => console.log('Notification channel created:', created) | ||
| ); | ||
|
|
||
| // ✅ CONFIGURATION | ||
| PushNotification.configure({ | ||
| onNotification: function (notification) { | ||
| console.log('Notification received:', notification); | ||
| }, | ||
|
|
||
| popInitialNotification: true, | ||
| requestPermissions: true, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
react-native-push-notification iOS Android setup requirements
💡 Result:
Short checklist — setup requirements for react-native-push-notification (iOS & Android)
iOS
- Install/use the community iOS package ( @react-native-community/push-notification-ios ) and run CocoaPods (npx pod-install). [1] (github.com)
- Enable Apple capabilities in Xcode: Push Notifications + Background Modes → Remote notifications. [1] (github.com)
- Configure APNs credentials on Apple Developer portal (APNs Auth Key .p8 or certificates) and upload to your push provider/server. [2] (pushy.me)
- Augment AppDelegate to handle register/notification callbacks per package docs (or follow PushNotificationIOS instructions if using that API). [1][3] (github.com)
Android
- Use Firebase Cloud Messaging (FCM): create Firebase app, download google-services.json → place in android/app. [3] (wix.github.io)
- Add/align firebase-messaging & Google Play services versions in android/build.gradle (ext: firebaseMessagingVersion, googlePlayServicesVersion, compileSdkVersion/targetSdkVersion). [4] (npmjs.com)
- Update AndroidManifest.xml: required permissions (e.g., VIBRATE, RECEIVE_BOOT_COMPLETED) and register any receivers/services the lib requires (see README). [4][5] (npmjs.com)
- Create notification channels for Android 8+ and handle foreground/background message flow per FCM and library docs. [3][4] (wix.github.io)
Notes / references
- The original react-native-push-notification repo has been archived and community alternatives/updated packages exist; prefer current maintained packages (check package README for exact install steps matching your RN version). [5] (github.com)
If you want, I can give the exact code snippets/gradle/Podfile/AppDelegate and AndroidManifest changes for your React Native version — tell me your RN version (and whether you use Expo).
Citations:
- 1: https://github.com/react-native-push-notification/ios?utm_source=openai
- 2: https://github.com/react-native-push-notification/ios?utm_source=openai
- 3: https://pushy.me/docs/additional-platforms/react-native?utm_source=openai
- 4: https://github.com/react-native-push-notification/ios?utm_source=openai
- 5: https://wix.github.io/react-native-notifications/docs/installation-android/?utm_source=openai
- 6: https://www.npmjs.com/package/react-native-push-notification/v/8.0.0?utm_source=openai
- 7: https://www.npmjs.com/package/react-native-push-notification/v/8.0.0?utm_source=openai
- 8: https://wix.github.io/react-native-notifications/docs/installation-android/?utm_source=openai
- 9: https://github.com/zo0r/react-native-push-notification?utm_source=openai
🏁 Script executed:
# Search for iOS push notification imports or configuration
rg -i "push.*notification.*ios|@react-native-community/push-notification-ios|AppDelegate" --type js --type ts --type java
# Check if there's any iOS-specific setup
find . -name "AppDelegate*" -o -name "Podfile" -o -name "ios" -type d | head -20Repository: AOSSIE-Org/BabyNest
Length of output: 142
🏁 Script executed:
cat -n ./Frontend/ios/BabyNest/AppDelegate.swiftRepository: AOSSIE-Org/BabyNest
Length of output: 1220
🏁 Script executed:
cat -n ./Frontend/ios/PodfileRepository: AOSSIE-Org/BabyNest
Length of output: 1343
🏁 Script executed:
cat -n ./Frontend/src/services/NotificationService.jsRepository: AOSSIE-Org/BabyNest
Length of output: 1405
🏁 Script executed:
# Also search for any iOS notification code in the project
rg -i "pushnotification|notification.*ios|ios.*notification" --type js --type ts | head -30Repository: AOSSIE-Org/BabyNest
Length of output: 417
iOS notification support is completely missing and will not function on iOS devices.
The app targets iOS (based on ios/ directory and AppDelegate), but the notification implementation uses Android-only APIs (createChannel, channelId) and lacks all required iOS setup:
- Missing @react-native-community/push-notification-ios dependency in Podfile
- No iOS notification configuration in AppDelegate.swift
- No APNs setup
Add iOS notification handling in AppDelegate.swift and install the iOS push notification package, or restrict this feature to Android only if iOS support is not intended.
🤖 Prompt for AI Agents
Frontend/src/services/NotificationService.js lines 7-29: the current code uses
Android-only APIs (createChannel, channelId) and lacks any iOS setup, so either
add iOS support or gate the feature to Android; to fix, install and link
@react-native-community/push-notification-ios and add its Podfile entry, update
AppDelegate.(m|swift) to register for remote notifications and forward APNs
token/notifications to the RN module, implement APNs provisioning
(entitlements/capabilities and Apple developer certs), and in this file wrap
Android-only calls with a Platform.OS === 'android' check and add iOS handlers
using PushNotificationIOS.configure or the iOS API so iOS devices are properly
registered and notifications handled (or explicitly disable/init only on Android
if iOS is not supported).
| export const scheduleNotification = (title, message, date) => { | ||
| PushNotification.localNotificationSchedule({ | ||
| channelId: 'default-channel-id', | ||
| title, | ||
| message, | ||
| date: new Date(date), | ||
| allowWhileIdle: true, | ||
| }); | ||
| }; No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add error handling and validation to notification scheduling.
The function lacks:
- Error handling if scheduling fails
- Validation that the date is in the future
- Return value to indicate success/failure
🔎 Add validation and error handling
-export const scheduleNotification = (title, message, date) => {
+export const scheduleNotification = (title, message, date) => {
+ try {
+ const notificationDate = new Date(date);
+
+ if (isNaN(notificationDate.getTime())) {
+ throw new Error('Invalid date provided for notification');
+ }
+
+ if (notificationDate <= new Date()) {
+ console.warn('Notification date is in the past, scheduling anyway');
+ }
+
PushNotification.localNotificationSchedule({
channelId: 'default-channel-id',
title,
message,
- date: new Date(date),
+ date: notificationDate,
allowWhileIdle: true,
});
+
+ return true;
+ } catch (error) {
+ console.error('Failed to schedule notification:', error);
+ throw error;
+ }
};📝 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.
| export const scheduleNotification = (title, message, date) => { | |
| PushNotification.localNotificationSchedule({ | |
| channelId: 'default-channel-id', | |
| title, | |
| message, | |
| date: new Date(date), | |
| allowWhileIdle: true, | |
| }); | |
| }; | |
| export const scheduleNotification = (title, message, date) => { | |
| try { | |
| const notificationDate = new Date(date); | |
| if (isNaN(notificationDate.getTime())) { | |
| throw new Error('Invalid date provided for notification'); | |
| } | |
| if (notificationDate <= new Date()) { | |
| console.warn('Notification date is in the past, scheduling anyway'); | |
| } | |
| PushNotification.localNotificationSchedule({ | |
| channelId: 'default-channel-id', | |
| title, | |
| message, | |
| date: notificationDate, | |
| allowWhileIdle: true, | |
| }); | |
| return true; | |
| } catch (error) { | |
| console.error('Failed to schedule notification:', error); | |
| throw error; | |
| } | |
| }; |
🤖 Prompt for AI Agents
In Frontend/src/services/NotificationService.js around lines 34 to 42,
scheduleNotification lacks input validation, error handling, and a return value;
update it to validate the date (ensure it's a valid Date and strictly in the
future), guard against invalid inputs by returning/throwing a clear error, wrap
PushNotification.localNotificationSchedule in a try/catch to handle runtime
errors, and return a boolean or result object indicating success or failure
(with error message) so callers can react accordingly.
|
Include the testing evidences |
|
Hi @charuljain02 |
3 participants
Issue
Closes #56
Description
This pull request adds in-app notification support for upcoming tasks and appointments in the BabyNest application.
The implementation introduces a dedicated notification service and integrates it with relevant screens to ensure users are notified in advance without affecting existing functionality.
Changes Made
Added a centralized NotificationService for handling in-app notifications
Integrated notification logic into the app lifecycle
Updated Home, Calendar, and Medicine screens to trigger notifications
Updated navigation flow where required
Ensured compatibility with the current app architecture
Testing
Application builds successfully
Notifications trigger correctly for upcoming tasks and appointments
No regressions observed in existing features
Notes
This implementation focuses on in-app notifications only
Designed to be extendable for future push notification support
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.