[PAY-3373] Purchasers endpoint #9525
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
🦋 Changeset detectedLatest commit: 0eb3f4d The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
amendelsohn
approved these changes
Aug 23, 2024
Contributor
amendelsohn
left a comment
amendelsohn
left a comment
There was a problem hiding this comment.
Looks good, but a few things to fix with args
Comment on lines
2597
to
2598
| limit = get_default_max(args.get("limit"), 10, 100) | ||
| offset = get_default_max(args.get("offset"), 0) |
Contributor
There was a problem hiding this comment.
I don't think we want limit and offset for this
| params={"id": "A User ID"}, | ||
| responses={200: "Success", 400: "Bad request", 500: "Server error"}, | ||
| ) | ||
| @full_ns.expect(current_user_parser) |
Contributor
There was a problem hiding this comment.
we need the content_type and content_id args still, right?
Contributor
Author
There was a problem hiding this comment.
dang good point, i think just re-use the purchasers_parser
| return buyers_query | ||
|
|
||
|
|
||
| def get_purchasers_count(args: GetPurchasersArgs): |
Contributor
There was a problem hiding this comment.
probably want a separate args type without pagination limit + offset
Contributor
Author
There was a problem hiding this comment.
i think it's ok to just omit them but use the same type?
| def get_purchasers_count(args: GetPurchasersArgs): | ||
| db = get_db_read_replica() | ||
| with db.scoped_session() as session: | ||
| query = _get_purchasers(session, args) |
Contributor
There was a problem hiding this comment.
should we write a separate query for the count that doesn't join on users table? We don't need that just to get the count of distinct ids
Contributor
Author
There was a problem hiding this comment.
hang on.. do we even need the users join for the normal query?
Contributor
There was a problem hiding this comment.
yeah aren't we returning the users? you could also just return the ids and call a helper to collect them after, but I think the join makes sense?
Contributor
Author
There was a problem hiding this comment.
but the populate_user_metadata happens after
Contributor
There was a problem hiding this comment.
oh, yeah I don't think we need both
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
2 participants
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.
Description
Endpoints + sdk entry points for purchasers: users who have purchased something from the given user ID, also optionally parametrized on a content type and content ID.
How Has This Been Tested?
Untested bc user banks are broken on local stack. Fairly high confidence this will just work on stage though, and we have no consumers yet so probably a safe merge.