Skip to content

fix: add publish event in deleteNotificationsById and export snap types#4836

Merged
hmalik88 merged 3 commits intomainfrom
hm/add-publish-to-delete-notifications
Oct 23, 2024
Merged

fix: add publish event in deleteNotificationsById and export snap types#4836
hmalik88 merged 3 commits intomainfrom
hm/add-publish-to-delete-notifications

Conversation

@hmalik88
Copy link
Contributor

@hmalik88 hmalik88 commented Oct 22, 2024

Explanation

  • What is the current state of things and why does it need to change? The deleteNotificationsById doesn't fire an event with the updated list of notifications and thus, the updateBadge function inside the extension is not fired.
  • What is the solution your changes offer and how does it work? Publish an event after notifications are deleted
  • Are there any changes whose purpose might not obvious to those unfamiliar with the domain? Adding export for snap types, not able to pull the type out directly otherwise.

@metamask/notification-services-controller

  • FIX: The deleteNotificationsById function did not fire an event with the updated notifications list, this was added after the function runs through all the notifications it needs to delete.

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've highlighted breaking changes using the "BREAKING" category above as appropriate
  • I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

@hmalik88 hmalik88 requested a review from a team as a code owner October 22, 2024 21:17
@hmalik88 hmalik88 merged commit 849548e into main Oct 23, 2024
@hmalik88 hmalik88 deleted the hm/add-publish-to-delete-notifications branch October 23, 2024 12:11
@hmalik88 hmalik88 mentioned this pull request Oct 23, 2024
hmalik88 added a commit that referenced this pull request Oct 23, 2024
Releasing `@metamask/notification-services-controller` to include
#4836
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.

2 participants