Skip to content

Commit

Permalink
Taniwha/bb 2497/ironwood fix root block (#74)
Browse files Browse the repository at this point in the history
* Handle invalid key errors for root_block

Catch InvalidKeyError as per opaque_keys docs. This is for BB-2497.

* Handle invalid key errors for course_key

While implementing the fix for root_block, it was found that course_key
handling could run into the same problems. This is a preventative fix.

* Correctly order imports

* Be more verbose for invalid keys

* Bump version and update CHANGELOG

* Correct invalid course key error message

* Be more informative when student is not enrolled

As per request by @Agrendalath, also, it just makes sense to do so.

* Fix line length issue
  • Loading branch information
taniwha authored Jun 9, 2020
1 parent 76dd89c commit 041e23d
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 5 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ Change Log
Unreleased
~~~~~~~~~~

[2.2.1] - 2020-06-05
~~~~~~~~~~~~~~~~~~~~

* Fix handling of invalid keys.

[2.1.3] - 2020-05-08
~~~~~~~~~~~~~~~~~~~~

Expand Down
2 changes: 1 addition & 1 deletion completion_aggregator/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@

from __future__ import absolute_import, unicode_literals

__version__ = '2.2.0'
__version__ = '2.2.1'

default_app_config = 'completion_aggregator.apps.CompletionAggregatorAppConfig' # pylint: disable=invalid-name
21 changes: 17 additions & 4 deletions completion_aggregator/api/v1/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from collections import defaultdict

import waffle
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import CourseKey, UsageKey
from rest_framework.exceptions import NotFound, ParseError
from rest_framework.views import APIView
Expand Down Expand Up @@ -361,7 +362,10 @@ def _parse_aggregator(self, course_key, params=None):
"""
Handles fetching and return aggregator data, regardless of the method used
"""
course_key = CourseKey.from_string(course_key)
try:
course_key = CourseKey.from_string(course_key)
except InvalidKeyError:
raise NotFound("Invalid course key: '{}'.".format(course_key))
paginator = self.pagination_class() # pylint: disable=not-callable
requested_fields = self.get_requested_fields()

Expand All @@ -377,7 +381,10 @@ def _parse_aggregator(self, course_key, params=None):
else:
if not UserEnrollments(self.user).is_enrolled(course_key):
# Return 404 if effective user does not have an active enrollment in the requested course
raise NotFound()
raise NotFound(
"User '{user}' does not have an active enrollment in course '{course_key}'."
.format(user=self.user, course_key=course_key)
)
is_stale = StaleCompletion.objects.filter(
username=self.user.username,
course_key=course_key,
Expand All @@ -395,7 +402,10 @@ def _parse_aggregator(self, course_key, params=None):

root_block = params.get('root_block')
if root_block:
root_block = UsageKey.from_string(root_block).map_into_course(course_key)
try:
root_block = UsageKey.from_string(root_block).map_into_course(course_key)
except InvalidKeyError:
raise NotFound("Invalid block key: '{}'.".format(root_block))

if is_stale and waffle.flag_is_active(self.request, WAFFLE_AGGREGATE_STALE_FROM_SCRATCH):
aggregator_queryset = []
Expand Down Expand Up @@ -541,7 +551,10 @@ def get(self, request, course_key):
"""
Handler for GET requests
"""
course_key = CourseKey.from_string(course_key)
try:
course_key = CourseKey.from_string(course_key)
except InvalidKeyError:
raise NotFound("Invalid course key: '{}'.".format(course_key))
requested_fields = self.get_requested_fields()
roles_to_exclude = self.request.query_params.get('exclude_roles', '').split(',')
cohort_filter = self._parse_cohort_filter(
Expand Down

0 comments on commit 041e23d

Please sign in to comment.