docs: Define api contract for User-Grouped role assignments - #437
docs: Define api contract for User-Grouped role assignments#437rodmgwgu wants to merge 8 commits into
Conversation
|
Thanks for the pull request, @rodmgwgu! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
|
|
||
| Query Parameters: | ||
| """"""""""""""""" | ||
|
|
There was a problem hiding this comment.
it is missing the roles filter
| `roles`` (optional): Comma-separated list of roles to filter by (e.g. ``course_auditor,library_admin``). |
There was a problem hiding this comment.
Good catch, I've added it, thanks!
| "is_superadmin": false, | ||
| "role": "library_admin", | ||
| "org": "Org1", | ||
| "scope": "lib:Org1:LIB1", |
There was a problem hiding this comment.
the table shows the name of the scope instead of the id
There was a problem hiding this comment.
Thanks for pointing this out. I added a new "scope_display_name" key and documented edge cases and implementation details.
| - ``scopes`` (optional): Comma-separated list of scopes to filter by (e.g. | ||
| ``lib:Org1:LIB1``). | ||
| - ``orgs`` (optional): Comma-separated list of orgs to filter by (e.g. ``Org1,Org2``). | ||
| - ``search`` (optional): Search term to filter users by username, full name, or email. |
There was a problem hiding this comment.
What is the full name field here? Is this something visible in the UI?
edit: I see it is just the user's full name, wondering if we should allow to search by this given is not visible in the UI, it can return results where the search string isn't visible, I think it might be confusing but maybe no big deal
There was a problem hiding this comment.
It's not being displayed in the design, but in past discussions with Guillermo we decided to include it in the search fields.
mariajgrimaldi
left a comment
There was a problem hiding this comment.
I agree with the current contract! I only have a few clarifying questions. Thanks a lot!
| email: string | ||
| assignment_count: number | ||
| assignments: Array<{ | ||
| is_superadmin: boolean |
There was a problem hiding this comment.
Oh I wasn't aware of this, so we are no longer showing superadmins at all?
There was a problem hiding this comment.
Until we figure out a more sustainable way to support them, yes
| "full_name": "Jane Doe", | ||
| "email": "jane_doe@example.com", | ||
| "assignment_count": 3, | ||
| "assignments": [ |
There was a problem hiding this comment.
I'm guessing this will return all types of assignments, right? Will the UI ever have a scope type filter?
There was a problem hiding this comment.
Yes, all type of assignments, the UI as I understand it only filters by specific scopes, not by type, but let me confirm this with María de los Ángeles.
There was a problem hiding this comment.
Confirmed by María de los Ángeles: it only filters by specific scopes.
BryanttV
left a comment
There was a problem hiding this comment.
Thanks for working on this! I only have a couple of suggestions.
| ``lib:Org1:LIB1``). | ||
| - ``orgs`` (optional): Comma-separated list of orgs to filter by (e.g. ``Org1,Org2``). | ||
| - ``search`` (optional): Search term to filter users by username, full name, or email. | ||
| - ``assignments_limit`` (optional): Maximum number of assignments to populate in |
There was a problem hiding this comment.
Maybe we should set an upper limit for assignment_limit? That way, we prevent someone from requesting, for example, 1000 assignments with page_size=50, which could be quite expensive.
There was a problem hiding this comment.
what would be a good number? maybe 10?
| is_superadmin: boolean | ||
| role: string | ||
| org: string | ||
| scope: string | ||
| scope_display_name: string | ||
| permission_count: number |
There was a problem hiding this comment.
I'd like to confirm if these fields in assignments are sufficient to display the information in the table correctly, especially when the scope is a Glob type (org and platform). I was thinking maybe it would be good to have a scope_type field?
cc @dcoa
There was a problem hiding this comment.
Currently we identify glob scopes by the '*' in scope as you can see in PLATFORM_AGGREGATE_SCOPE_KEYS and ORG_AGGREGATE_SCOPE_BUILDERS here.
If we can guarantee that this convention is consistent across all scopes, then the current fields should be sufficient to display the information correctly.
Having a scope_type field isn't a bad idea if we want to be more explicit about the information encoded in scope itself. However, I think in that case, it would need to be a composite value, e.g. course-org-glob, (or adding another field) since simply having org or platform wouldn't tell us what type of resource the Glob applies to and we still need to relay on scope.
There was a problem hiding this comment.
Thanks for confirming that, @dcoa. In that case, if the value in the scope field is sufficient, we can go ahead with the fields defined in the ADR for now.
…e-assignments.rst Co-authored-by: Bryann Valderrama <bryann.valderrama@edunext.co>
…e-assignments.rst Co-authored-by: Bryann Valderrama <bryann.valderrama@edunext.co>
…e-assignments.rst Co-authored-by: Bryann Valderrama <bryann.valderrama@edunext.co>
…e-assignments.rst Co-authored-by: Bryann Valderrama <bryann.valderrama@edunext.co>
BryanttV
left a comment
There was a problem hiding this comment.
For my part, I would just need to add a limit to the assignments_limit field (possibly 10). Thank you for your work!
Closes: #405
The new Team Members tab design for the Admin Console requires fields that are not currently available in the API.
This ADR proposes the contract for updating the existing /api/authz/v1/users/ to support the new Team Members tab use cases.
Merge checklist:
Check off if complete or not applicable:
AI Usage
Kiro was used to refine the prose and clarity of the text, help with rst formatting, as well as to assist on research.
The core ADR contents and direction were drafted by hand.