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

[PAY-2761] Add managed users endpoints #8238

Merged
merged 10 commits into from
Apr 29, 2024
Merged

Conversation

schottra
Copy link
Contributor

@schottra schottra commented Apr 25, 2024

Description

This adds an endpoint for fetching users managed by the current account as well as the opposite endpoint (users managing a specific user id). The results are returned as a list of objects with a grant and user/manager property Access to the grant allows us to determine grants still waiting for approval as well as already approved active grants.

Summary of changes:

  • Added /users/<id>/managed_users endpoint to full user namespace, with API models and response marshaller
  • Added /users/<id>/managers endpoint to full user namespace, with API models and response marshaller
  • Added queries for fetching managed users based on current user, with options for filtering on revoked/approved status.
  • Updated auth_middleware() with an additional configuration parameter include_wallet. When this is True, the decorated function should expect a kv arg authed_user_wallet which will be populated with the wallet recovered from decoding the signature. This is needed to lookup the managed users as the manager side of the relationship is indicated by wallet address.
  • Added tests
  • Updated SDK to add new endpoints and models

fixes PAY-2761

How Has This Been Tested?

Tested with client against local dev stack.

Copy link

changeset-bot bot commented Apr 25, 2024

🦋 Changeset detected

Latest commit: ca5ab6e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@audius/sdk Minor

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

@schottra schottra changed the title [PAY-2761] Add /grants endpoint [PAY-2761] Add managed users endpoint Apr 26, 2024
@audius-infra
Copy link
Collaborator

Preview this change https://demo.audius.co/pay-2761-grants-endpoint

@@ -87,7 +91,16 @@ def wrapper(*args, **kwargs):
logger.info(
f"auth_middleware.py | authed_user_id: {authed_user_id}"
)
return func(*args, **kwargs, authed_user_id=authed_user_id)
return (
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 learned today that if you pass a kv argument that isn't consumed, it will often generate an error.
So I had to do this weirdness of conditionally calling the inner function differently. I would have preferred to conditionally add to the arguments list. But I could not wrap my head around arguments in python well enough to do it.

Copy link
Member

Choose a reason for hiding this comment

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

I think you could include in it kwargs, like this

def inner(*args, **kwargs):
  print("fun!", args, kwargs)

def outer(*args, **kwargs):
  kwargs["thing"] = True
  fun(*args, **kwargs)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh interesting. Maybe we should do that for the other variable as well then?
In my reading I couldn't find any suggestion that you can modify kwargs like that. But I'll give it a try.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@audius-infra
Copy link
Collaborator

Preview this change https://demo.audius.co/pay-2761-grants-endpoint

Copy link
Member

@raymondjacobson raymondjacobson left a comment

Choose a reason for hiding this comment

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

nice!

db = get_db()

entities = copy.deepcopy(test_entities)
# Record for a user which won't be found
Copy link
Member

Choose a reason for hiding this comment

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

nice way of setting this up

@@ -87,7 +91,16 @@ def wrapper(*args, **kwargs):
logger.info(
f"auth_middleware.py | authed_user_id: {authed_user_id}"
)
return func(*args, **kwargs, authed_user_id=authed_user_id)
return (
Copy link
Member

Choose a reason for hiding this comment

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

I think you could include in it kwargs, like this

def inner(*args, **kwargs):
  print("fun!", args, kwargs)

def outer(*args, **kwargs):
  kwargs["thing"] = True
  fun(*args, **kwargs)


if is_approved is not None:
query = query.filter(Grant.is_approved == is_approved)
if is_revoked is not None:
Copy link
Member

Choose a reason for hiding this comment

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

idt it's possible for is_revoked to actually be None

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The input is a dict, so it's possible to not set the value at all. I had assumed that results in None. I can add a test to make sure this actually does something.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a test which sets both filters to None and makes sure we get back all records (which are a mix of permutations of the two flags)

@audius-infra
Copy link
Collaborator

Preview this change https://demo.audius.co/pay-2761-grants-endpoint

@audius-infra
Copy link
Collaborator

Preview this change https://demo.audius.co/pay-2761-grants-endpoint

@schottra schottra changed the title [PAY-2761] Add managed users endpoint [PAY-2761] Add managed users endpoints Apr 29, 2024
@schottra
Copy link
Contributor Author

@raymondjacobson @rickyrombo I added the /managers endpoint since @nicoback will need it for upcoming work and I was already here. You can review this commit if you want.

@audius-infra
Copy link
Collaborator

Preview this change https://demo.audius.co/pay-2761-grants-endpoint

@audius-infra
Copy link
Collaborator

Preview this change https://demo.audius.co/pay-2761-grants-endpoint

Copy link
Contributor

@nicoback2 nicoback2 left a comment

Choose a reason for hiding this comment

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

Looks great!

@schottra schottra merged commit df0349a into main Apr 29, 2024
34 of 36 checks passed
@schottra schottra deleted the pay-2761-grants-endpoint branch April 29, 2024 22:27
if authed_user_id is None:
abort_unauthorized(full_ns)

# TODO: If accessing this endpoint as a manager, this check will not
Copy link
Contributor

Choose a reason for hiding this comment

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

is this mainly auth'd because it makes it easier to get the user wallet? or are there reasons beyond that that we'd want to make it auth'd?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Auth does help with getting the user wallet, but also it made sense that you shouldn't be able to fetch the grants and managements relationships of arbitrary users. You should only see relationships you are part of.

audius-infra pushed a commit that referenced this pull request Apr 30, 2024
[bd2fbc3] [⚠️] Clean up discovery node build artifacts and URSM code (#8234) Raymond Jacobson
[df0349a] [PAY-2761] Add managed users endpoints (#8238) Randy Schott
[ca8133d] [PROTO-1789] Fix up minor entrypoint issues and deploy (#8252) Raymond Jacobson
[7e4ec20] Add cloudflare ip check fix (#8253) Raymond Jacobson
[321ebf6] [ONC-83] Add stage-only migration to fix indexing the sepolia chain (#8257) Saliou Diallo
@schottra schottra restored the pay-2761-grants-endpoint branch May 2, 2024 21:44
raymondjacobson added a commit that referenced this pull request May 2, 2024
raymondjacobson added a commit that referenced this pull request May 2, 2024
raymondjacobson added a commit that referenced this pull request May 3, 2024
audius-infra pushed a commit that referenced this pull request May 4, 2024
[9f44712] Reapply "Convert swagger.json to OAS 3.0 before generating SDK types (#8274)" (#8324) (#8344) Marcus Pasell
[2ea74f3] Fix issues with text-link variant (#8343) Dylan Jeffers
[f005b25] [PAY-2860] SDK Services Config - Configure SDK using environment (#8329) Marcus Pasell
[11a0813] Disable add to playlist for prem tracks if not purchased (#8336) Reed
[33b1916] [C-4335] Migrate all user and collection cards (#8338) Dylan Jeffers
[dafaa21] [QA-1241] Fix hidden track add to playlist allowing non hidden items (#8340) JD Francis
[4c95031] [QA-956] Add deleted tile for tracks/albums/playlists (#8333) JD Francis
[b8f5990] Add error message to upload flow (#8334) JD Francis
[22894e8] [QA-957] Fix android overflowing text on access & sale modal (#8313) JD Francis
[86f6447] [PAY-2812] Change spend to earn challenge copy (#8332) Reed
[d4c4264] Close modal on done (#8331) Saliou Diallo
[ded4933] [PAY-2861] CollectionCard fixes (#8330) Dylan Jeffers
[36277cd] Revert "Convert swagger.json to OAS 3.0 before generating SDK types (#8274)" (#8324) Marcus Pasell
[146b2e0] [QA-1212] Fix chromecast (#8323) Raymond Jacobson
[0b33e68] [PAY-2738] Public album with prem track always shows buy button (#8321) Reed
[d897a98] [C-4333] Migrate follow-users to new sign-up flow (#8312) Dylan Jeffers
[3129a3a] [PAY-2799] Show preview button for collections (#8271) Reed
[8c58601] Modify /etc/hosts on CI machines before api test (#8319) Danny
[3586f41] [PAY-2859] Fix bugs in edit app flow (#8320) Raymond Jacobson
[1bf3e97] [PAY-2801] Purchased selectable filter in library albums (#8318) Reed
[6db35c6] [C-4285] Add claim all rewards button functionality (#8292) Saliou Diallo
[a20d85a] [PAY-2728] Implement edit app flow (#8256) Raymond Jacobson
[714a3d1] [C-4288] Migrate to new cards in profile, search, and library (#8300) Dylan Jeffers
[4dceb61] Upgrade dapp-store to 0.8.1 (#8311) Dylan Jeffers
[4d37233] [PAY-2794] Use UserLink for purchases/sales tables (#8306) Reed
[fc2aef3] [PAY-2772] Fix collection description text size (#8309) Reed
[c35bfe1] [PAY-2822] Convert LockedContentDetailsTile to Harmony (#8308) Reed
[f7a921e] [PAY-2781] Poll for track access on album purchase (#8296) Reed
[621c3ee] Convert swagger.json to OAS 3.0 before generating SDK types (#8274) Marcus Pasell
[09789c1] Retrigger logic for android notification permissions post-manifest fix (#8305) JD Francis
[b9c042a] [PAY-2815] Show unlocked icon if dm content is purchased (#8303) Andrew Mendelsohn
[722e933] [PAY-2779] Fix validation for albumTrackPrice (#8302) Andrew Mendelsohn
[963bec4] Fix EditCollection playwright test (#8304) Andrew Mendelsohn
[7e0aa7e] [PAY-2823] Edit Album access control fixes (#8298) Andrew Mendelsohn
[b42b6fd] [QA-971] Fix smart collections (#8299) Raymond Jacobson
[eceb876] [PAY-2791] Show only own albums in artist dashboard (#8297) Reed
[87ce027] [Web] [Harmony] Add accounts managing you modal [C-4295] [C-4292] (#8258) nicoback2
[7a3567c] [C-4238, C-4318] Add ssr small bundle flow for mobile track page (#8293) Kyle Shanks
[c7c3752] [PAY-2800] Fix mobile navigate to album on purchase success (#8295) Reed
[f113a91] [PAY-2802] Auto-favorite tracks in purchased album (#8294) Reed
[d32934f] [C-4284] Fix rewards in mobile web (#8291) Saliou Diallo
[45c4bc6] [PAY-2775] Remove suggested tracks for albums (#8288) Andrew Mendelsohn
[22280c6] [C-4080] Mobile cooldown and claim all (#8263) Isaac Solo
[80cca19] Jail broken test (#8289) Raymond Jacobson
[d7c8e34] [PAY-2766] Analytics events for premium albums (#8262) JD Francis
[c3ee4a8] [PAY-2817] Web collections show reposts, remove artist column (#8272) Reed
[6f8bab6] Fix lint items (#8285) Raymond Jacobson
[e8debc0] [PAY-2703] Redesign mobile collection and track screen tiles (#8255) Andrew Mendelsohn
[a3964a8] Bump android build number for release-client-v1.5.78 (#8286) Raymond Jacobson
[8481f8d] [C-4327] Fix track ordering within playlist/album (#8284) Raymond Jacobson
[5165172] Bump android version for manifest changes (#8283) JD Francis
[4e60f97] [C-4312] Add UserCard (#8276) Dylan Jeffers
[35ed4ec] [ONC-82] Fix android notif permissions (#8279) JD Francis
[82e1c67] [C-4323] Improve Avatar (#8273) Dylan Jeffers
[909db7c] [C-4316, C-4317] SSR work to render webplayer and track page for web (#8268) Kyle Shanks
[7417a40] Generate sdk after ursm code removal (#8275) Raymond Jacobson
[62b5a08] Update Add Funds copy and loading state (#8266) Saliou Diallo
[4503cc7] PROTO-1328: always default to discovery relay (#8247) alecsavvy
[f42c6cd] Fix text-input spacing (#8270) Dylan Jeffers
[75fd847] [C-4270] Fix text-link transition (#8260) Dylan Jeffers
[df0349a] [PAY-2761] Add managed users endpoints (#8238) Randy Schott
[5b4c998] [ONC-84] Add try/catch (#8264) Raymond Jacobson
[2b5f89c] Fix clipped text in TracksTable name column (#8265) Reed
[c3256fa] [C-4275] Add google-inspectiontool to crawler regex (#8261) Sebastian Klingler
[5750045] [C-4033] Prevent useTabs from redirecting on page load (#8259) Sebastian Klingler
[b17ff70] [PAY-2758] Allow gated tracks to be added to playlists (#8227) Reed
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.

None yet

5 participants