Skip to content

Conversation

@rickyrombo
Copy link
Contributor

@rickyrombo rickyrombo commented Aug 22, 2024

Description

  • Unhides Notifications API from documentation so it can be generated
  • Adds discriminator to OneOfModel to allow for notifications to be strongly typed, and updated it to take a list (or dict if using discriminator) of models instead of fields for ergonomics
  • Adds individual API models for each and every notification type
  • Adds response adapter for notifications
  • Removes feature flag for using discovery notifications
  • Removes remote config var for fallbacks to identity service notifications

How Has This Been Tested?

Tested with a subset of notifications on local stack.

Tried to seed local stack with prod notif data but if users are missing the notifs don't render (and I didn't want to seed the whole prod db). The response adapter worked though, and found no diff - though the real risk here is DN changes.

Should continue testing on staging and maybe side-by-side prod with the changes vs without? There's certainly some risk here and I didn't cover every case.

TODO:

  • Check for DN regressions using prod data seed? QA?
  • Tests for OneOfModel/NestedOneOf?
  • Confirm we don't need Identity fallback

@changeset-bot
Copy link

changeset-bot bot commented Aug 22, 2024

⚠️ No Changeset found

Latest commit: f3c7231

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@audius-infra
Copy link
Collaborator

Preview this change https://demo.audius.co/mjp-sdk-notifications

Copy link
Contributor

@schottra schottra left a comment

Choose a reason for hiding this comment

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

Absolutely huge. Thank you.

@audius-infra
Copy link
Collaborator

Preview this change https://demo.audius.co/mjp-sdk-notifications

@audius-infra
Copy link
Collaborator

Preview this change https://demo.audius.co/mjp-sdk-notifications

Copy link
Contributor

@sliptype sliptype left a comment

Choose a reason for hiding this comment

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

Wow this is awesome, nice work!

})
}

function* fetchDiscoveryNotifications(params: FetchNotificationsParams) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we consolidate fetchDiscoveryNotifications into fetchNotifications? Don't think the wrapper fn is necessary anymore

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah I'll do this in a followup!

@audius-infra
Copy link
Collaborator

Preview this change https://demo.audius.co/mjp-sdk-notifications

@audius-infra
Copy link
Collaborator

Preview this change https://demo.audius.co/mjp-sdk-notifications

@rickyrombo rickyrombo changed the title Migrate Notifications to SDK [PAY-3357] Migrate Notifications to SDK Aug 23, 2024
@rickyrombo rickyrombo merged commit b8c360e into main Aug 23, 2024
@rickyrombo rickyrombo deleted the mjp-sdk-notifications branch August 23, 2024 23:13
rickyrombo added a commit that referenced this pull request Aug 24, 2024
audius-infra pushed a commit that referenced this pull request Aug 24, 2024
[1ba94b8] Reland "[PAY-3357] Migrate Notifications to SDK" (#9539) Marcus Pasell
[c33b4e3] Revert "[PAY-3357] Migrate Notifications to SDK" (#9538) Marcus Pasell
[b8c360e] [PAY-3357] Migrate Notifications to SDK (#9516) Marcus Pasell
[eaef9fd] [QA-1498] Fix long press on DMs (#9533) JD Francis
[4bbfeff] [PAY-3373] Purchasers endpoint (#9525) Reed
[72ff6a3] [C-4909] Improve track metadata list, fix album backlinks (#9531) Dylan Jeffers
[e5248b7] [QA-1515] Remove dismiss on leave from popup (#9527) JD Francis
[46305bb] [PAY-2918] Migrate track endpoints to SDK - Round 3 (#9522) Randy Schott
[b957a52] Fix challenge api tx confirming (#9526) Isaac Solo
[6999b53] [QA-1519] Fix buyer metadata caching (#9512) Isaac Solo
[7f17c95] Hook up remixers count to web blast UI (#9523) Reed
[75ab270] [PAY-3359] Chat blast thread UI on sender side (#9515) Reed
[93e3ac9] Mobile comment fixes (#9507) Sebastian Klingler
[c81ed5d] [QA-1499] Fix long usernames overflowing manager mode button (#9519) JD Francis
[7763deb] Add react-router dependency (#9520) Isaac Solo
[6de56bd] Move trpc to common (#9518) Dylan Jeffers
[76d4095] Active styles for web chat blasts (#9517) Reed
[3464a2a] Add row and column props to flex (#9505) Dylan Jeffers
[e6b1da1] [C-4902] Add harmony checkbox  (#9510) Dylan Jeffers
[0fcf9e4] [PAY-3361] ChatBlastSelectAudienceScreen (#9502) Andrew Mendelsohn
[2566eb8] [PAY-3228] Implement composer unfurl in mobile (#9504) Raymond Jacobson
[b5bf00a] [QA-1210] Fix popup scrolling issues (#9508) JD Francis
[5349315] Update logic for allowing chat blast sending (#9509) Reed
[51391d9] [PAY-3362] Move link resolver to common hook (#9503) Raymond Jacobson
[45dc83d] Add feature flag refresh (#9506) Dylan Jeffers
[3af4e69] [PAY-2918] Migrate track endpoints to SDK, Round 2 (#9494) Randy Schott
[78897c4] [PAY-3355] Address link preview / unfurl QA items  (#9492) Raymond Jacobson
[c2a9387] [PAY-3366] Fix CTA hidden on small screens (#9499) Andrew Mendelsohn
[e6dbb7a] Fix CTA empty states reversed web/mobile (#9498) Andrew Mendelsohn
[f3d7825] Remove listenUserId from chat api (#9479) Reed
[d55dbb1] [PAY-3360] Chat Blast CTA (mobile) (#9491) Andrew Mendelsohn
[adb99e3] Fix challenge claim notification amount (#9485) Isaac Solo
[d4518ce] [C- 4926] Add Menu/MenuItem and update Select (#9427) Dylan Jeffers
[b914ade] [PAY-2918] Migrate tracks endpoints to SDK - round 1 (#9474) Randy Schott
[893ed42] Revamp local solana-test-validator (#9482) Marcus Pasell
[7ff4ab0] Separate saga top level error handling (#9458) Raymond Jacobson
[06a19e6] Remote config for chat blast tier requirement (#9490) Reed
[2b804b2] [PAY-3345] Prevent artist reacting to their own blasts (#9488) Reed
[1cc1915] [C-4976] Refactor comment hooks (#9489) Sebastian Klingler
[b3a4c77] [QA-1511] Fix space & arrow hotkeys not working (#9486) JD Francis
[1342d1f] [C-4823] Mobile comment drawer (#9478) Sebastian Klingler
[075d757] [QA-1514] Fix search filter typing (#9484) Sebastian Klingler
[bfaa536] Enabling blocking users from commenting (#9469) Isaac Solo
[c2d5e85] Bump mobile apps to 111 (#9481) Dylan Jeffers
[5944ced] [PAY-3358] Fire chat message RPCs on blast write (#9476) Reed
[83c97ed] [C-4972] Add comment sort bar (#9477) JD Francis
audius-infra pushed a commit that referenced this pull request Aug 26, 2024
[1ba94b8] Reland "[PAY-3357] Migrate Notifications to SDK" (#9539) Marcus Pasell
[c33b4e3] Revert "[PAY-3357] Migrate Notifications to SDK" (#9538) Marcus Pasell
[b8c360e] [PAY-3357] Migrate Notifications to SDK (#9516) Marcus Pasell
[4bbfeff] [PAY-3373] Purchasers endpoint (#9525) Reed
[7d1ea58] [PAY-3363] Chat blast: support more audiences (#9524) Steve Perkins
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants