feat: migration notification API from v2 to v3 #7102
Merged
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
this is a large notification architecture API change but offers dynamic platform notifications
Contributor
Author
|
@metamaskbot publish-preview |
| mockGetOnChainNotificationsConfig, | ||
| mockUpdateOnChainNotifications, | ||
| mockGetOnChainNotifications, | ||
| mockGetAPINotifications, |
Contributor
Author
There was a problem hiding this comment.
renamed mock util
| env: { | ||
| featureAnnouncements: FeatureAnnouncementEnv; | ||
| isPushIntegrated?: boolean; | ||
| locale?: () => string; |
Contributor
Author
There was a problem hiding this comment.
We now pass in locale to handle server side in-app platform notifications
...ion-services-controller/src/NotificationServicesController/NotificationServicesController.ts
Outdated
Show resolved
Hide resolved
Contributor
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
| } | ||
|
|
||
| export const TRIGGER_TYPES_WALLET_SET: Set<string> = new Set([ | ||
| export const NOTIFICATION_API_TRIGGER_TYPES_SET: Set<string> = new Set([ |
Contributor
Author
There was a problem hiding this comment.
Update set and add new types supported by our API
| TRIGGER_TYPES.ERC1155_RECEIVED, | ||
| ]) satisfies Set<Exclude<TRIGGER_TYPES, TRIGGER_TYPES.FEATURES_ANNOUNCEMENT>>; | ||
|
|
||
| export enum TRIGGER_TYPES_GROUPS { |
Contributor
Author
There was a problem hiding this comment.
never used, removed
Contributor
Author
There was a problem hiding this comment.
API returns new data shapes, so mocks needed updating
Contributor
Author
There was a problem hiding this comment.
Added mock Platform Notification shape.
| * @param notification - API Notification (On-Chain or Platform Notification) | ||
| * @returns Normalized Notification | ||
| */ | ||
| export function processAPINotifications( |
Contributor
Author
There was a problem hiding this comment.
New process to handle our API notifications (wallet notifs and platform notifs).
Replaces the old processOnChainNotifications.
Also added an auto-expire to expire old notifications (30 days).
| /** | ||
| * This file was auto-generated by openapi-typescript. | ||
| * Do not make direct changes to the file. | ||
| * Script: `npx openapi-typescript <PATH TO NOTIFICATION API SPEC> -o ./schema.d.ts` |
Contributor
Author
There was a problem hiding this comment.
new schema generated from our open-api spec
| const tokenDecimals = n?.data?.token?.decimals; | ||
| const symbol = n?.payload?.data?.token?.symbol; | ||
| const tokenAmount = n?.payload?.data?.token?.amount; | ||
| const tokenDecimals = n?.payload?.data?.token?.decimals; |
Contributor
Author
There was a problem hiding this comment.
Shape has changed, we now have a payload object that contains these properties.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
...ces-controller/src/NotificationServicesController/types/notification-api/notification-api.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Bug: Platform Notifications Silently Dropped
The createOnChainPushNotificationMessage function doesn't handle platform notifications. When a platform notification (with type: 'platform') is passed to this function, it returns null because TRIGGER_TYPES.PLATFORM isn't included in the createOnChainPushNotificationMessages dictionary. Platform notifications have a template field with localized content instead of payload.data, but the function doesn't check for or use this field. This means platform notifications won't generate push notification messages, potentially causing them to be silently dropped in the push notification flow.
packages/notification-services-controller/src/NotificationServicesPushController/utils/get-notification-message.ts#L255-L276
...ces-controller/src/NotificationServicesController/types/notification-api/notification-api.ts
Show resolved
Hide resolved
allows us to filter server side different notifications for each platform
Contributor
Author
|
@metamaskbot publish-preview |
Contributor
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
packages/notification-services-controller/src/shared/to-raw-notification.ts
Show resolved
Hide resolved
Prithpal-Sooriya
changed the title
Prithpal-Sooriya
changed the title
sahar-fehri
approved these changes
Nov 12, 2025
Prithpal-Sooriya
deleted the
ASSETS-1301/refactor-update-notifications-to-v3-api
branch
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.
Explanation
This is a large notification architecture API change but offers dynamic platform notifications
Details
Devlog: https://www.loom.com/share/ea5fc1457c224b2988fadacbfc21ca2a
Extension Preview: MetaMask/metamask-extension#37709
Mobile Preview: MetaMask/metamask-mobile#22539
References
https://consensyssoftware.atlassian.net/browse/ASSETS-1301
Checklist
Note
Migrates notifications to the v3 API with unified on-chain/platform notifications, locale-aware fetching, and updated controller/types/push logic.
/api/v3/notificationsand/api/v3/notifications/mark-as-read.{ addresses: string[], locale: string, platform: 'extension'|'mobile' }; config query unchanged; mark-as-read body{ ids }.NormalisedAPINotification(union of on-chain + platform) andUnprocessedRawNotification.TRIGGER_TYPES.PLATFORM; update trigger type set and constants.toRawOnChainNotificationwithtoRawAPINotification; removeprocess-onchain-notificationsin favor ofprocessAPINotifications.onchain-notificationstoapi-notifications(getAPINotifications,getNotificationsApiConfigCached,markNotificationsAsRead).payload.Written by Cursor Bugbot for commit 3562e62. This will update automatically on new commits. Configure here.