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

Fixed ViewSets with course id detail views #309

Merged
merged 1 commit into from
Dec 15, 2020

Conversation

johnbaldwin
Copy link
Contributor

@johnbaldwin johnbaldwin commented Dec 15, 2020

The problem was that course ids with dots in the name would get part of
the url before the course id into the PK for the 'retrieve' call, which
provides a detail view of the single course based content.

The fix was adding lookup_value_regex to the Course based model
viewsets

As part of this:

  • Refactored figures/views.py to DRY up code
  • Added logging when an invalid course id is used as a primary key and
    purposefully raising a NotFound exception instead of an
    InvalidKeyError

For reference, see the DRF DefaultRouter.get_lookup_expr method: https://github.com/encode/django-rest-framework/blob/3.6.3/rest_framework/routers.py#L217-L238

https://appsembler.atlassian.net/browse/RED-1702

The problem was that course ids with dots in the name would get part of
the url before the course id into the PK for the 'retrieve' call, which
provides a detail view of the single course based content.

The fix was adding `lookup_value_regex` to the Course based model
viewsets

As part of this:

* Refactored figures/views.py to DRY up code
* Added logging when an invalid course id is used as a primary key and
purposefully raising a `NotFound` exception instead of an
`InvalidKeyError`

https://appsembler.atlassian.net/browse/RED-1702
@johnbaldwin johnbaldwin merged commit 5c9addd into master Dec 15, 2020
@johnbaldwin johnbaldwin deleted the john/0.4-fix-course-based-api branch December 15, 2020 20:56
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.

2 participants