Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: ✨ add support for different link types in feature announcements #4620

Conversation

matteoscurati
Copy link
Contributor

Explanation

This PR enhances the notification-services-controller package by adding support for various link types in feature announcements. The following changes have been made:

  • Added new link types: TypeExternalLinkFields, TypePortfolioLinkFields, and TypeMobileLinkFields to handle different types of links in feature announcements.
  • Updated FeatureAnnouncementRawNotificationData: Included fields for external, portfolio, and mobile links.
  • Updated TypeFeatureAnnouncementFields: Included fields for external, portfolio, and mobile links.
  • Modified fetchFeatureAnnouncementNotifications: Updated the function to handle the new link types and include them in the notification data.

These changes improve the flexibility and functionality of feature announcements by allowing them to include various types of links.

References

  • No specific issues are tied to this PR.

Changelog

@metamask/notification-services-controller

  • ADDED: TypeExternalLinkFields, TypePortfolioLinkFields, and TypeMobileLinkFields types to handle different types of links in feature announcements.
  • UPDATED: FeatureAnnouncementRawNotificationData to include fields for external, portfolio, and mobile links.
  • UPDATED: TypeFeatureAnnouncementFields to include fields for external, portfolio, and mobile links.
  • UPDATED: fetchFeatureAnnouncementNotifications to handle the new link types and include them in the notification data.

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

@matteoscurati matteoscurati added the team-notifications Notification Team changes. https://github.com/orgs/MetaMask/teams/notifications label Aug 19, 2024
@matteoscurati matteoscurati requested a review from a team as a code owner August 19, 2024 12:48
@matteoscurati matteoscurati marked this pull request as draft August 19, 2024 12:48
Copy link
Contributor

@Prithpal-Sooriya Prithpal-Sooriya 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. Fix ESLint issues and also can you double check any // TODO comments related to mobile (just a quick search in this /notification-services-controller folder

@matteoscurati matteoscurati marked this pull request as ready for review August 21, 2024 12:32
…ents-links' of github.com:MetaMask/core into feat/notification-services-controller-feature-announcements-links
@matteoscurati matteoscurati merged commit 60fb9b4 into main Sep 3, 2024
116 checks passed
@matteoscurati matteoscurati deleted the feat/notification-services-controller-feature-announcements-links branch September 3, 2024 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-notifications Notification Team changes. https://github.com/orgs/MetaMask/teams/notifications
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants