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

APIv3: return permissions expandable field on projects #10978

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

humitos
Copy link
Member

@humitos humitos commented Jan 2, 2024

Add ?expand=permissions to project list/detail API endpoints.

Related readthedocs/ext-theme#128

Add `?expand=permissions` to project list/detail API endpoints.

Related readthedocs/ext-theme#128
@agjohnson agjohnson force-pushed the humitos/projects-api-permissions branch from ef4b5bc to ec87791 Compare July 1, 2024 22:17
@humitos
Copy link
Member Author

humitos commented Jul 11, 2024

@agjohnson I think we want to move away from expandable fields because it's pretty complex. So, we may not want to use this PR at all -- cc @stsewd

@agjohnson
Copy link
Contributor

What is the complexity here? This PR is very useful in removing the page load time on project/organization/build listings.

If we aren't using expand fields, what other options do we have here for returning this data in a single API request? Does this need to be a second API request from the dashboard?

@stsewd
Copy link
Member

stsewd commented Jul 11, 2024

This field is specific to the current user, so it shouldn't be a problem. But be aware that this will probably add lots of querysets when listing projects...

@agjohnson
Copy link
Contributor

Good point, I am only using this in the detail view. If there is a way to only allow this expansion on detail views, I think that should be enough as well. Or we can monitor this and see if it is ever used/abused. This should never be documented for users, though it is in a public request that users can inspect too.

@agjohnson
Copy link
Contributor

Do we need to do anything additional with this PR? This feels like a fairly small change, as are the changes to use this in

@humitos
Copy link
Member Author

humitos commented Jul 18, 2024

I'm not 100% sure about where we are regarding the expandable fields and this comment from @stsewd. I'm happy to add some test cases to this PR and mark it as ready for review after that if that's the required work here.

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.

None yet

3 participants