Notifications API extension and new notifications generated by API operations #11696
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…tionsForUser to JsonPrinter
This comment has been minimized.
This comment has been minimized.
…for InApp notifications
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
…cations-api-extension
…ationsJsonPrinter
…CCESS to InAppNotificationsJsonPrinter
… to InAppNotificationsJsonPrinter
This comment has been minimized.
This comment has been minimized.
3 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
3 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
…ationsJsonPrinter
…cations-api-extension
|
📦 Pushed preview images as 🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name. |
pdurbin
approved these changes
Aug 15, 2025
Member
pdurbin
left a comment
pdurbin
left a comment
There was a problem hiding this comment.
Thanks for addresses all my comments! Also, tests are passing: https://jenkins.dataverse.org/job/IQSS-Dataverse-Develop-PR/job/PR-11696/45/testReport/
Approved!
github-project-automation
bot
moved this from In Review 🔎
to Ready for QA ⏩
in IQSS Dataverse Project
Contributor
|
Looks good to me, merging. |
github-project-automation
bot
moved this from QA ✅
to Merged 🚀
in IQSS Dataverse Project
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
Includes the following changes:
getAllNotificationsForUser API extension
/notifications/all), which now supports an optional query parameterinAppNotificationFormatwhich, if sent astrue, retrieves the fields needed to build the in-app notifications for the Notifications section of the Dataverse UI, omitting fields related to email notifications.Notifications triggered by API endpoints
The addDataset and addDataverse API endpoints now trigger user notifications upon successful execution.
Related issue: #1342
Which issue(s) this PR closes:
Special notes for your reviewer:
The new fields returned by the new
inAppNotificationFormatflag are based on the fields required by JSF to compose the notifications on the user page.Suggestions on how to test this:
Creating an account, dataverses, and datasets—these operations should generate notifications that you can then query using the API endpoint.
Test calling the endpoint with the new query parameter and without it, to see the differencies.
Retrieving notifications for In-App:
curl -H "X-Dataverse-key:$API_TOKEN" "$SERVER_URL/api/notifications/all?inAppNotificationFormat=true"Retrieving notifications as before:
curl -H "X-Dataverse-key:$API_TOKEN" "$SERVER_URL/api/notifications/all"❗❗
For dataset creation to trigger a notification, the SendNotificationOnDatasetCreation setting must first be enabled (set to true). This behavior already existed in Dataverse, but previously only curators would receive a notification when the setting was true. Now, the author also receives a notification if the setting is true and the dataset is created through the API.
I am open to removing the setting to avoid requiring an additional step of enabling it in installations that use the SPA. However, I have decided to keep it for backward compatibility.
❗❗
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
No
Is there a release notes update needed for this change?:
Yes, find attached
Additional documentation:
Preview at https://dataverse-guide--11696.org.readthedocs.build/en/11696/api/native-api.html#get-all-notifications-by-user