Skip to content

Notification fixes#430

Merged
prooflesben merged 18 commits intomainfrom
notification-fixes
Apr 18, 2026
Merged

Notification fixes#430
prooflesben merged 18 commits intomainfrom
notification-fixes

Conversation

@lyannne
Copy link
Copy Markdown
Collaborator

@lyannne lyannne commented Apr 15, 2026

ℹ️ Issue

Closes #426

📝 Description

Write a short summary of what you added. Why is it important? Any member of C4C should be able to read this and understand your contribution -- not just your team members.

Briefly list the changes made to the code:

  1. Used Camila's code for updating grant poc on profile change & also included updating notifications on profile change (if email was changed).
  2. Added syncing for updating and deleting grants as well
    3.Made it so only useful notifications are created (for example, if today is 4/15 and the grant application is due 4/25, I would not need the 'application is due in 14 days' notification, so onlythe 7 day and 3 day one will be created)
  3. Made save button on settings page disabled while loading
  4. changed timestamp display on notification popup to show date on notifications older than 6 days

✔️ Verification

What steps did you take to verify your changes work? These should be clear enough for someone to be able to clone the branch and follow the steps themselves.

Provide screenshots of any new components, styling changes, or pages.

  • Mostly just had console logs to check notifications being created and updated and deleted accordingly (it works)
  • small change to popup, adding vertical line between day and time when full date is displayed
image

Test Changes

If your new feature required some test to be changed or added to fit the new functionality or changes please document these changes here.

  • Updated notification, grant, and user tests

🏕️ (Optional) Future Work / Notes

Did you notice anything ugly during the course of this ticket? Any bugs, design challenges, or unexpected behavior? Write it down so we can clean it up in a future ticket!

  • Syncing everything with grants and notifications made latency kind of an issue...it takes a while for everything to be updated.
  • Question: what if a user is deleted by an admin? what will happen to their grants and notifs?

Copilot AI review requested due to automatic review settings April 15, 2026 21:52
@lyannne lyannne linked an issue Apr 15, 2026 that may be closed by this pull request
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR focuses on fixing notification correctness and syncing across profile and grant lifecycle changes, while also improving a few UX details in the notifications/settings UI.

Changes:

  • Add grantId to notification models and implement backend grant-notification sync on grant update/delete and profile (POC) changes.
  • Add frontend notification fetching utilities and trigger notification refreshes on app data fetch and grant mutations.
  • Improve settings “Save” UX (disable while saving) and adjust notification timestamp display formatting.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 12 comments.

Show a summary per file
File Description
middle-layer/types/Notification.ts Adds grantId to the shared notification type to support grant-linked syncing.
frontend/src/main-page/settings/Settings.tsx Disables Save while saving; refetches grants after profile update.
frontend/src/main-page/notifications/processNotificationData.ts Adds a shared fetchNotifications helper to refresh notifications in store.
frontend/src/main-page/notifications/GrantNotification.tsx Changes notification timestamp formatting for older notifications.
frontend/src/main-page/notifications/Bell.tsx Refactors bell click behavior and imports notification fetch helper.
frontend/src/main-page/grants/edit-grant/processGrantDataEditSave.ts Refetches notifications after grant create/edit/delete.
frontend/src/context/auth/authContext.tsx Fetches notifications as part of initial/periodic authenticated data refresh.
backend/src/notifications/types/notification.types.ts Extends notification DTO to include grantId.
backend/src/notifications/notification.service.ts Adds grantId-based notification lookup and bulk update for email/org changes.
backend/src/notifications/test/notification.service.test.ts Updates notification tests for grantId and adds coverage for new service methods.
backend/src/grant/grant.service.ts Syncs notifications on grant update/delete; adds “relevant notification times” filtering; adds updateGrantsByPOC.
backend/src/grant/grant.module.ts Exports GrantService for cross-module injection.
backend/src/grant/grant.controller.ts Uses ParseIntPipe for grantId in delete endpoint.
backend/src/grant/test/grant.service.spec.ts Updates notification helper expectations and adds tests for updateGrant notification sync behavior.
backend/src/auth/auth.service.ts Injects GrantService and updates grants after profile changes with rollback on failure.
backend/src/auth/auth.module.ts Imports GrantModule (and NotificationsModule) and simplifies provider wiring.
backend/src/auth/test/auth.service.spec.ts Updates auth tests setup (but currently missing a GrantService provider mock).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread backend/src/grant/grant.service.ts Outdated
Comment thread backend/src/auth/__test__/auth.service.spec.ts
Comment thread backend/src/notifications/notification.service.ts Outdated
Comment thread backend/src/notifications/notification.service.ts Outdated
Comment thread backend/src/grant/grant.service.ts
Comment thread backend/src/auth/auth.module.ts
Comment thread frontend/src/main-page/notifications/Bell.tsx Outdated
Comment thread frontend/src/main-page/notifications/Bell.tsx
Comment thread frontend/src/main-page/notifications/processNotificationData.ts Outdated
Comment thread frontend/src/main-page/settings/Settings.tsx
Copy link
Copy Markdown
Contributor

@camila-carrillo camila-carrillo left a comment

Choose a reason for hiding this comment

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

lgtm

@prooflesben prooflesben merged commit a848a22 into main Apr 18, 2026
1 check passed
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.

DEV Notification fixes

5 participants