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: migrates notifications category to support in app messaging channel notifications #11170

Merged
merged 41 commits into from
Oct 15, 2022
Merged

feat: migrates notifications category to support in app messaging channel notifications #11170

merged 41 commits into from
Oct 15, 2022

Conversation

lazpavel
Copy link
Contributor

Description of changes

  • migrates notifications category to support the new in-app messaging channel
  • ensures the migration gets executed when the customer executes any of the [add, remove, update] commands on the analytics or notifications category

Issue #11087

Description of how you validated changes

  • unit and e2e tests pass
  • new e2e tests added to test migration scenarios
  • manual testing

Checklist

  • PR description included
  • yarn test passes
  • Tests are changed or added
  • Relevant documentation is changed or added (and PR referenced)
  • New AWS SDK calls or CloudFormation actions have been added to relevant test and service IAM policies
  • Pull request labels are added

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

lazpavel and others added 30 commits September 26, 2022 15:44
* feat: in app messaging initial commit
@lgtm-com
Copy link

lgtm-com bot commented Oct 13, 2022

This pull request introduces 1 alert when merging 4f7f82b into 9ecc3f4 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Oct 13, 2022

This pull request introduces 1 alert when merging 45bd7bb into 9ecc3f4 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@lazpavel lazpavel marked this pull request as ready for review October 14, 2022 21:18
akshbhu
akshbhu previously approved these changes Oct 14, 2022
Copy link
Contributor

@akshbhu akshbhu left a comment

Choose a reason for hiding this comment

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

LGTM

sobolk
sobolk previously approved these changes Oct 14, 2022
Comment on lines 10 to 13
/**
*
*/
export type PinpointApp = {
Copy link
Member

Choose a reason for hiding this comment

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

For future discussions.

we should tweak eslint to enforce docs only on types that we export outside of single package.

enforcing docs for internals isn't very productive.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually missed it, it was added on save, but will leave it for now to not dismiss the approval, unless the recent changes broke something and will need tweaked

jhockett
jhockett previously approved these changes Oct 14, 2022
sobolk
sobolk previously approved these changes Oct 15, 2022
@lazpavel lazpavel dismissed stale reviews from sobolk and jhockett via 07abaa8 October 15, 2022 10:45
@lazpavel lazpavel merged commit 52f5787 into aws-amplify:dev Oct 15, 2022
awsluja pushed a commit that referenced this pull request Oct 17, 2022
…nnel notifications (#11170)

* feat: migrates notifications category to support in app messaging channel notifications
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.

5 participants