Implemented the V1 notifications system with the corresponding unit t…

[xu-ian]

Description

This PR is for the implementation of Epic 1.5, the V1 Notification System

Related Issue

If this PR addresses an issue, link to it here.
#201

Type of Change

Please select the type(s) of change that apply and delete those that do not.

• New feature: A non-breaking change that adds functionality.
• Test enhancement: Adding or updating tests; no production code change.

Proposed Solution

New dependencies:

• @babel/preset-env: Babel preset for test cases
• @babel/preset-react: Babel preset for react test cases.
• @firebase/testing: Testing library for the backend trigger unit tests.
• @testing-library/dom: Required dependency of @testing-library/react used to unit test dom components.
• @testing-library/react: Used to test react components.
• babel-jest: Allows babel configuration for jest test cases of react components
• cross-fetch: Enables fetch for backend trigger test cases
• firestore-jest-mock: Allows mocking of firestore in jest.
• jest: Testing library used for test cases
• jest-environment-jsdom: Allows the jsdom test environment for react unit tests.
• Reactfire: Used in place of websockets to retrieve notifications in real time from firestore.

Updated Dependency:

• [email protected] -> [email protected]: Fixes a bug where Filter was not added to firebase-admin/firestore. Used in backend trigger unit tests.

Implementation reasonings

• Real-time notifications was implemented with reactfire. This is a library that can retrieve updates to firestore in real-time. This library was used because it is much easier to set up and reuse than websockets.
• The backend has two implemented collections 'notifications' and 'personal-notifications'. 'notifications' are meant for global notifications passed to every user like tool updates. 'personal-notifications' are meant to be notifications that are unique to users like personal alerts.

NotificationsButton.jsx (New)

This component displays a bell icon. On click this toggles the visibility of a list of Notifications.

NotificationList.jsx (New)

This component displays: A togglable all/unread button. A list of notifications relevant to the user. A button to read all notifications. This component is designed to be reused to display a list of notifications anywhere.

Notification.jsx (New)

This component displays: A single notification.

Firebase.jsx and FirebaseProviders.jsx (New)

These components are wrappers that configure reactfire to retrieve data from the configured firebase destination.

setReadStatus.js (New)

This function takes a message and updates the existing message in the database.

SideMenu.jsx (Updated)

Added the notification button to this Layout.

MainAppLayout/styles.js (Updated)

Updated the zIndex in the navigation bar, so the notifications appear above the contents of the page.

notificationController.js (New)

• getNotifications: Takes a user id and an optional notification id and queries the personal-notifications collection. Returns a list of notifications that matches the query.

• setNotificationStats: Takes a user id, an optional notification id, and a status. Queries the personal-notifications collection using user id and notification id and updates the is_read status of the query results with status.

• onNewNotification: Trigger that occurs when a global notification is created. Creates a copy of the notification as a personal notification for every user.

• onRemovedNotification: trigger that occurs when a global notification is removed. Removes every personal notification that was a copy of the global notification.

• onNewTool: Trigger that occurs when a document is added to 'tools'. Creates a global notification that a new tool has been created.

• onUpdatedTool: Trigger that occurs when a document is updated in 'tools'. Creates a global notification that a tool has been updated.

• onNewAssistant: Trigger that occurs when a document is added to 'assistants'. Creates a global notification that a new assistant has been created.

• onUpdatedAssistant: Trigger that occurs when a document is updated in 'assistants'. Creates a global notification that a assistant has been updated.

cloud_db_seed.js

• Added a global notification and two personal notifications to the seed database.

How to Test

Notification System Demo Link on Loom

• Click the notification button to make sure the button toggles the visibility of the notifications menu when clicked.
• Click a 'Mark as Read' button to make sure it changes to 'Mark as Unread' and vice versa
• Navigate to the Unread tab and mark a notification as read to make sure it disappears from the tab
• Click the 'Mark all as Read' button to make sure all notifications change to read.
• Go to firebase emulators to add a new notification. The website should update with the notification without needing to refresh.

Unit Tests

Unit Tests are stored in __tests__. Emulator must be running in order for backend trigger tests to work. The backend trigger tests may not work sometimes due to delays due to unresponsiveness from the server.

__setup__/env-setup.js

• Sets up jest environment for each test by passing .env file and activating cross-fetch for backend triggers.

jest.config.js

• Configures the jest environment.

babel.config.js

• Configures jest environment with babel presets.

NotificationsButton-test.js

• Tests whether the NotificationsButton component starts with the notification list hidden.
• Tests whether clicking the NotificationsButton component toggles the visibility of the notification list.

NotificationList-test.js

• Tests whether the NotificationsList component displays notifications correctly based on passed information.
• Tests whether the correct information is passed to setReadStatus when 'Mark As Read' is clicked.
• Tests whether the correct information is passed to setReadStatus when 'Mark all as read' is clicked.

NotificationTriggers-test.js

• Tests whether a personal notification is created for a user when a global notification is created.
• Tests whether a personal notification for a user is removed when the corresponding global notification is removed.
• Tests whether a global notification is created when a new tool is created.
• Tests whether a global notification is created when a tool is updated.
• Tests whether a global notification is created when a new assistant is created.
• Tests whether a global notification is created when an assistant is updated.

Documentation Updates

Indicate whether documentation needs to be updated due to this PR.

• Yes

If yes, describe what documentation updates are needed and link to the relevant documentation.

Checklist

• I have performed a self-review of my code.
• I have commented my code, particularly in hard-to-understand areas.
• I have made corresponding changes to the documentation.
• My changes generate no new warnings.
• I have added tests that prove my fix is effective or that my feature works.
• New and existing unit tests pass locally with my changes.
• Any dependent changes have been merged and published in downstream modules.

Additional Information

Add any other information that might be useful for the reviewers.

[poojithreddy28]

Hi @xu-ian,

Great work on implementing the functionality! However, I noticed a couple of issues:
1. Incorrect Date in Notifications: The date field populates “0” instead of “1.” Please check the logic and adjust it accordingly.
2. Notification Styling: The notification system design deviates from the proposed Figma design. Could you style it to align with the Figma design for consistency?

I’ve attached snapshots of the incorrect date issue and the Figma design for your reference:

• Incorrect Date Issue:

• Current notification system UI

Figma Design for Notification System:

[xu-ian]

Hey @poojithreddy28, I have fixed the issues you have brought up. Months now start at 1 instead of 0 and the styling has been adjusted. Here's a screenshot of the new styling:

[poojithreddy28]

Hey @poojithreddy28, I have fixed the issues you have brought up. Months now start at 1 instead of 0 and the styling has been adjusted. Here's a screenshot of the new styling:

Hey @xu-ian ,

Thanks for addressing the issues! The updates look great, and I appreciate the fix for the months starting at 1. The styling adjustments also look much cleaner.

Could you please also develop the notification icon to match the Figma design? Once that’s done, I’ll run everything on my local setup, test it out thoroughly, and will forward this senior devs for additional testing.

Thanks again for your efforts! 😊

[xu-ian]

Hi @poojithreddy28, I have updated the notification icon to match the styling requirements.

[poojithreddy28]

Hi @poojithreddy28, I have updated the notification icon to match the styling requirements.

Great! I’ll conduct thorough testing on the changes and forward them to @yunusj if everything looks good.

[yunusj]

Hey @xu-ian ,

First off, a huge thank you for this contribution! 🎉 Your work on this PR is well-structured, neatly written, and includes comprehensive documentation and test cases, making it a breeze to review. I really appreciate the attention to detail and the effort you’ve put into this!

After reviewing, here are a few areas where we can further optimize for performance, consistency, and scalability:

1. Optimize Notification Reads & Security

• Instead of marking notifications as read via the backend, we can handle this directly on the frontend and enforce access control using Firestore security rules.
• This avoids unnecessary backend execution and reduces compute costs, which is especially important for a free product like ours.

2. Firestore Structure Enhancement

• To improve organization and access control, we should store personal notifications as a subcollection inside the users collection:

  users/{userId}/notifications/{notificationId}
  

• This allows better scalability and efficient querying of user-specific notifications.

3. Database Key Consistency

• Our standard convention follows camelCasing instead of underscores for database keys.
• Example:
  • "is_read" → "isRead"
  • "action_link" → "actionLink"

4. Folder Structure Suggestion

• To maintain clarity and separation of concerns, we propose the following structure for notifications-related files:

  /functions/notificationsController
    - index.js
    - triggers/
        - notifications.triggers.js
        - tools.triggers.js
        - assistants.triggers.js
    - utils/
        - createNotification.js
    - services/ (optional, for notifications CRUD, email service in the future)
  

• This keeps all notification logic organized and modular, making it easier for future contributors.

5. Automatic Cleanup of Old Notifications (Cost Optimization)

• To control Firestore costs, we need a mechanism to automatically remove notifications older than 60 days.
• This can be handled via a scheduled Cloud Function
• Implementation example:
  • Run a daily/weekly job that deletes all notifications exceeding the 60-day limit.

---

Next Steps

If you can incorporate these refinements, we’ll be in a great place to merge this PR. Let me know if you have any questions—I’d be happy to discuss or clarify any of the points above.

Again, fantastic job! 👏 Contributions like these help Marvel AI grow, and we truly appreciate your effort 🚀. Looking forward to your updates!

[xu-ian]

Hi @yunusj, I have made the requested changes in the new commit. The trigger that removes older notifications has a unit test that tests its' functionality, but not whether it runs at the requested interval, as the emulator does not support scheduled functions.