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

API: add participation viewset #1412

Merged
merged 5 commits into from
Nov 15, 2024
Merged

Conversation

felixrindt
Copy link
Member

@felixrindt felixrindt commented Nov 7, 2024

@jeriox should we keep the users/1/participations endpoint or deprecate it and include user filtering in this one? It's not trivial, as this new ViewSet works on abstract participations and we'd need a few lines to filter for the Local user participations if a user id to filter for is given.

Also, mind that I narrowed the permission check. You now need to be able to view users and the event to see the participations. I'd be fine with only users too, as I find that more important in this case (includes email and stuff). Maybe we should talk about the permission model at some point. For example we could include a reduced view that only shows the VIEW_PUBLIC scoped information like in the web interface (just display name and qualifications, no email).

Todo

  • check the endpoint documentation
  • More tests

@felixrindt felixrindt added the [C] feature New feature or request label Nov 7, 2024
@felixrindt felixrindt marked this pull request as ready for review November 8, 2024 13:20
ephios/api/filters.py Outdated Show resolved Hide resolved
@jeriox
Copy link
Contributor

jeriox commented Nov 8, 2024

@jeriox should we keep the users/1/participations endpoint or deprecate it and include user filtering in this one? It's not trivial, as this new ViewSet works on abstract participations and we'd need a few lines to filter for the Local user participations if a user id to filter for is given.

I think we should keep the other one. The usecase is quite different and it feels weird to include users here as it deals with AbstractParticipations

Also, mind that I narrowed the permission check. You now need to be able to view users and the event to see the participations. I'd be fine with only users too, as I find that more important in this case (includes email and stuff). Maybe we should talk about the permission model at some point. For example we could include a reduced view that only shows the VIEW_PUBLIC scoped information like in the web interface (just display name and qualifications, no email).

Both sounds good.
I liked the narrowed-down version for VIEW_PUBLIC

@felixrindt felixrindt closed this Nov 8, 2024
@felixrindt felixrindt reopened this Nov 8, 2024
@coveralls
Copy link

coveralls commented Nov 10, 2024

Coverage Status

coverage: 85.162% (+0.01%) from 85.149%
when pulling 190e62b on api-filterable-participations
into 3167a63 on main.

@felixrindt felixrindt force-pushed the api-filterable-participations branch 3 times, most recently from 6bfb4a2 to 9037cd3 Compare November 11, 2024 11:13
ephios/api/access/auth.py Show resolved Hide resolved
ephios/api/views/events.py Show resolved Hide resolved
@felixrindt felixrindt merged commit 280df91 into main Nov 15, 2024
15 checks passed
@felixrindt felixrindt deleted the api-filterable-participations branch November 15, 2024 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C] feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants