feat: require national_id query param in programs#298
Conversation
johanseto
force-pushed
the
jlc/national-id-constraint-programs
branch
from
4aaf464 to
79005a3
Compare
eox_nelp/programs/api/v1/views.py
Outdated
| List of enrolled courses for the user qs | ||
| """ | ||
| if getattr(self.request, 'user_by_national_id', None): | ||
| if self.request.query_params.get("is_enrolled", "true") == "true": |
There was a problem hiding this comment.
@andrey-canon Do you think the default should be truthy? or falsy, so that all defaults should return visible courses for the user? And then filter the enrolled?
There was a problem hiding this comment.
I don’t think this filter is necessary. What I mean is that the endpoint has basically changed to one focused on returning user information plus the programs associated with that user.
So, having an option that just returns the programs a user can see — without including any relevant user data — doesn’t really make sense to me.
In fact, I believe the next block, which filters by enrollments, should now be the default behavior returned by the get_queryset method.
johanseto
force-pushed
the
jlc/national-id-constraint-programs
branch
from
79005a3 to
1618676
Compare
| def filter_queryset(self, queryset): | ||
| """ | ||
| Filter the queryset by course enrolled if the national_id query param is present. | ||
| Filter the queryset... |
There was a problem hiding this comment.
I think you can improve this comment, feel free to do that in this pr or in the next one where you are implementing the logic
2 participants
Description
Testing instructions
feat: require national_id query param in programs
Before
After
Screencast.from.29-10-25.11.02.30.webm
Additional information
Jira story: https://edunext.atlassian.net/browse/FUTUREX-1544