Skip to content

904 notifications refresh#927

Open
ekraffmiller wants to merge 7 commits intodevelopfrom
904-notifications-refresh
Open

904 notifications refresh#927
ekraffmiller wants to merge 7 commits intodevelopfrom
904-notifications-refresh

Conversation

@ekraffmiller
Copy link
Contributor

@ekraffmiller ekraffmiller commented Mar 2, 2026

What this PR does / why we need it:

refreshes the unread notifications list when these actions happen in the UI:

  • View unread notifications
  • Create a dataset
  • Publish a dataset
  • Create a Collection

Which issue(s) this PR closes:

Special notes for your reviewer:

There is an issue with the create dataset API, it isn't generating a notification: IQSS/dataverse#12189

Suggestions on how to test this:

  1. Perform the actions in the SPA that will create new unread notifications.
  2. Test that the unread notifications icon in the header is correct.
  3. Go to the notifications tab, and view the unread notifications.
  4. Test that the unread notifications icon in the header is cleared when the notifications are marked as read.

Does this PR introduce a user interface change? If mockups are available, please link/include them here:

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

Additional documentation:

@coveralls
Copy link

coveralls commented Mar 2, 2026

Coverage Status

coverage: 97.763% (+0.4%) from 97.347%
when pulling 7e87722 on 904-notifications-refresh
into 94aecb3 on develop.

@ekraffmiller ekraffmiller marked this pull request as ready for review March 2, 2026 22:40
@ekraffmiller ekraffmiller moved this to Ready for Review ⏩ in IQSS Dataverse Project Mar 3, 2026
@ekraffmiller ekraffmiller added GREI Re-arch GREI re-architecture-related SPA.Q1.2026.4 High Priority Bug Fixes Size: 3 A percentage of a sprint. 2.1 hours. labels Mar 3, 2026
@ChengShi-1 ChengShi-1 self-requested a review March 4, 2026 16:01
@ChengShi-1 ChengShi-1 self-assigned this Mar 4, 2026
@ChengShi-1 ChengShi-1 moved this from Ready for Review ⏩ to In Review 🔎 in IQSS Dataverse Project Mar 4, 2026
Copy link
Contributor

@ChengShi-1 ChengShi-1 left a comment

Choose a reason for hiding this comment

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

Looks good! The notification works well (except for the create dataset you mentioned). The test coverage is failing now, but I guess it should pass after removing "only" in one test case.

cy.get('@markAsRead').should('have.been.calledOnceWith', Cypress.sinon.match.number)
})

it.only('does not render notifications display count when there are no notifications', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you remove the "only" here?

@ChengShi-1 ChengShi-1 removed their assignment Mar 6, 2026
@ChengShi-1 ChengShi-1 moved this from In Review 🔎 to Ready for QA ⏩ in IQSS Dataverse Project Mar 6, 2026
Copy link
Contributor

@ChengShi-1 ChengShi-1 left a comment

Choose a reason for hiding this comment

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

Thanks for the change, approve

@cmbz cmbz added the FY26 Sprint 18 FY26 Sprint 18 (2026-02-25 - 2026-03-11) label Mar 11, 2026
@cmbz cmbz added this to the 6.11 milestone Mar 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

FY26 Sprint 18 FY26 Sprint 18 (2026-02-25 - 2026-03-11) GREI Re-arch GREI re-architecture-related Size: 3 A percentage of a sprint. 2.1 hours. SPA.Q1.2026.4 High Priority Bug Fixes

Projects

Status: Ready for QA ⏩

Development

Successfully merging this pull request may close these issues.

Feature Request/Idea:Notification refresh on operations that may generate a notification.

4 participants