Skip to content

Conversation

@ekraffmiller
Copy link
Contributor

@ekraffmiller ekraffmiller commented Oct 6, 2025

What this PR does / why we need it:

Updates the GetAllNotificationsByUser use case to allow results to return only unread messages. Also adds parameters for pagination (limit and offset).

Which issue(s) this PR closes:

Related Dataverse PRs:

Special notes for your reviewer:

The tests are currently failing because it is depending on a dataverse PR that hasn't been merged yet, which is slightly behind develop. When that PR is merged, the .env can be updated to use 'unstable'.

Suggestions on how to test this:

review code and tests. Note that this has a waiting tag. Should not be merged until the related Dataverse PR is merged.
NOTE: before merging, revert the .env file to use the 'unstable' dataverse image

Is there a release notes or changelog update needed for this change?:

Yes, changelog updated.

Additional documentation:

@ekraffmiller ekraffmiller changed the title 386 update notifications use case Update GetAllNotificationsByUser use case Oct 8, 2025
@ekraffmiller ekraffmiller moved this to Ready for Review ⏩ in IQSS Dataverse Project Oct 8, 2025
@ekraffmiller ekraffmiller added GREI Re-arch GREI re-architecture-related SPA.Q3.2025.6 Account Page: Notifications labels Oct 8, 2025
@ekraffmiller ekraffmiller marked this pull request as ready for review October 8, 2025 18:44
@ekraffmiller ekraffmiller added Size: 3 A percentage of a sprint. 2.1 hours. Original size: 3 labels Oct 8, 2025
@cmbz cmbz added the FY26 Sprint 8 FY26 Sprint 8 (2025-10-08 - 2025-10-22) label Oct 8, 2025
@g-saracca g-saracca self-assigned this Oct 9, 2025
@g-saracca g-saracca moved this from Ready for Review ⏩ to In Review 🔎 in IQSS Dataverse Project Oct 9, 2025
Copy link
Contributor

@g-saracca g-saracca left a comment

Choose a reason for hiding this comment

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

I'm leaving a comment about how to handle the query params but approving 👍🏼

@github-project-automation github-project-automation bot moved this from In Review 🔎 to Ready for QA ⏩ in IQSS Dataverse Project Oct 9, 2025
@g-saracca g-saracca removed their assignment Oct 9, 2025
@ekraffmiller ekraffmiller moved this from Ready for QA ⏩ to In Progress 💻 in IQSS Dataverse Project Oct 21, 2025
@ekraffmiller ekraffmiller self-assigned this Oct 22, 2025
@cmbz cmbz added the FY26 Sprint 9 FY26 Sprint 9 (2025-10-22 - 2025-11-05) label Oct 23, 2025
@ekraffmiller ekraffmiller removed their assignment Oct 23, 2025
@ekraffmiller ekraffmiller moved this from In Progress 💻 to Ready for Review ⏩ in IQSS Dataverse Project Oct 23, 2025
Copilot AI review requested due to automatic review settings October 23, 2025 13:47
Copy link
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 enhances the GetAllNotificationsByUser use case to support filtering unread notifications and pagination parameters (limit and offset). The return type has been changed from a simple array to a NotificationSubset object that includes both the notifications array and a total count.

Key changes:

  • Modified GetAllNotificationsByUser to accept onlyUnread, limit, and offset parameters
  • Changed return type from Notification[] to NotificationSubset across the codebase
  • Fixed additionalInfo field type from string to Record<string, unknown>

Reviewed Changes

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

Show a summary per file
File Description
src/notifications/domain/models/NotificationSubset.ts New model interface containing notifications array and totalNotificationCount
src/notifications/domain/useCases/GetAllNotificationsByUser.ts Added pagination and filtering parameters to the use case
src/notifications/domain/repositories/INotificationsRepository.ts Updated interface signature to match new parameters and return type
src/notifications/infra/repositories/NotificationsRepository.ts Implemented query parameter building and response transformation for new features
src/notifications/domain/models/Notification.ts Changed additionalInfo type from string to Record<string, unknown>
src/notifications/infra/transformers/NotificationPayload.ts Updated payload interface to match Notification model change
test/integration/notifications/NotificationsRepository.test.ts Updated tests to handle NotificationSubset return type and added tests for new filtering/pagination features
test/functional/notifications/GetAllNotificationsByUser.test.ts Updated functional tests for new return type and parameters
test/functional/notifications/DeleteNotification.test.ts Updated to work with NotificationSubset structure
test/integration/collections/CollectionsRepository.test.ts Changed test file ID to avoid conflicts
test/environment/.env Updated Docker image configuration for testing
CHANGELOG.md Documented new features and fixes

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@cmbz cmbz added the FY26 Sprint 10 FY26 Sprint 10 (2025-11-05 - 2025-11-19) label Nov 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

FY26 Sprint 8 FY26 Sprint 8 (2025-10-08 - 2025-10-22) FY26 Sprint 9 FY26 Sprint 9 (2025-10-22 - 2025-11-05) FY26 Sprint 10 FY26 Sprint 10 (2025-11-05 - 2025-11-19) GREI Re-arch GREI re-architecture-related Original size: 3 Size: 3 A percentage of a sprint. 2.1 hours. SPA.Q3.2025.6 Account Page: Notifications Waiting

Projects

Status: Ready for Review ⏩

Development

Successfully merging this pull request may close these issues.

Update use case for Notifications: additionalInfo in response, paging, and filtering

4 participants