Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 66 additions & 0 deletions lms/djangoapps/instructor/tests/views/test_api_v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -587,6 +587,72 @@ def test_rescore_only_if_higher(self, mock_submit):
assert response.status_code == status.HTTP_202_ACCEPTED
assert mock_submit.call_args[0][3] is True

@patch('lms.djangoapps.instructor_task.api.submit_rescore_problem_for_student')
def test_rescore_only_if_higher_form_encoded_body(self, mock_submit):
"""
only_if_higher sent form-encoded in the POST body (as the instructor
dashboard MFE does) must be honored, not silently dropped.

Regression test: dropping the flag caused "Rescore Only if Score
Improves" to run an unconditional rescore and lower learner scores.
"""
mock_task = MagicMock()
mock_task.task_id = str(uuid4())
mock_submit.return_value = mock_task

response = self.client.post(
self._get_url(),
data='learner=test_student&only_if_higher=true',
content_type='application/x-www-form-urlencoded',
)
assert response.status_code == status.HTTP_202_ACCEPTED
assert mock_submit.call_args[0][3] is True

@patch('lms.djangoapps.instructor_task.api.submit_rescore_problem_for_student')
def test_rescore_only_if_higher_json_body(self, mock_submit):
"""only_if_higher as a JSON boolean in the body is honored."""
mock_task = MagicMock()
mock_task.task_id = str(uuid4())
mock_submit.return_value = mock_task

response = self.client.post(
self._get_url(),
data={'learner': 'test_student', 'only_if_higher': True},
content_type='application/json',
)
assert response.status_code == status.HTTP_202_ACCEPTED
assert mock_submit.call_args[0][3] is True

@patch('lms.djangoapps.instructor_task.api.submit_rescore_problem_for_student')
def test_rescore_only_if_higher_false_in_body(self, mock_submit):
"""An explicit 'false' string in the body resolves to False."""
mock_task = MagicMock()
mock_task.task_id = str(uuid4())
mock_submit.return_value = mock_task

response = self.client.post(
self._get_url(),
data='learner=test_student&only_if_higher=false',
content_type='application/x-www-form-urlencoded',
)
assert response.status_code == status.HTTP_202_ACCEPTED
assert mock_submit.call_args[0][3] is False

@patch('lms.djangoapps.instructor_task.api.submit_rescore_problem_for_student')
def test_rescore_only_if_higher_query_param_wins_over_body(self, mock_submit):
"""The documented query-param contract takes precedence over a conflicting body value."""
mock_task = MagicMock()
mock_task.task_id = str(uuid4())
mock_submit.return_value = mock_task

response = self.client.post(
self._get_url() + '?only_if_higher=true',
data='learner=test_student&only_if_higher=false',
content_type='application/x-www-form-urlencoded',
)
assert response.status_code == status.HTTP_202_ACCEPTED
assert mock_submit.call_args[0][3] is True

@patch('lms.djangoapps.instructor_task.tasks.rescore_problem.apply_async')
def test_rescore_all_learners(self, mock_apply):
"""Bulk rescore queues a task and returns 202."""
Expand Down
25 changes: 22 additions & 3 deletions lms/djangoapps/instructor/views/api_v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -3605,6 +3605,22 @@ def _get_learner_identifier(request):
return request.query_params.get('learner') or request.data.get('learner')


def _get_only_if_higher(request):
"""
Extract the only_if_higher flag from query params or request body.

Accepts a 'true' string (case-insensitive) or a JSON boolean. Reading the
body as well as query params matches ``_get_learner_identifier``, since the
instructor dashboard MFE sends these fields form-encoded in the POST body.
"""
value = request.query_params.get('only_if_higher')
if value is None:
value = request.data.get('only_if_higher')
if isinstance(value, bool):
return value
return str(value).lower() == 'true'


def _parse_course_and_problem(course_id, problem):
"""
Parse and validate course_id and problem location strings.
Expand Down Expand Up @@ -3871,7 +3887,8 @@ class RescoreView(DeveloperErrorViewMixin, APIView):
**POST** with `learner` query param: rescores a single learner (asynchronous task).
**POST** without `learner`: rescores all learners (asynchronous task).

Optionally accepts `only_if_higher=true` query param to only update if new score is higher.
Optionally accepts `only_if_higher=true` (query param or request body) to only
update if the new score is higher.
"""
permission_classes = (IsAuthenticated, permissions.InstructorPermission)
permission_name = permissions.OVERRIDE_GRADES
Expand All @@ -3896,7 +3913,9 @@ class RescoreView(DeveloperErrorViewMixin, APIView):
apidocs.string_parameter(
'only_if_higher',
apidocs.ParameterLocation.QUERY,
description="Optional: If 'true', only update scores that are higher than current.",
description="Optional: If 'true', only update scores that are higher than current. "
"May be provided as a query parameter or in the request body "
"(JSON boolean or string).",
),
],
responses={
Expand All @@ -3914,7 +3933,7 @@ def post(self, request, course_id, problem):
return error_response
course_key, usage_key = parsed

only_if_higher = request.query_params.get('only_if_higher', 'false').lower() == 'true'
only_if_higher = _get_only_if_higher(request)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be great if a unit test were added for this change

learner_identifier = _get_learner_identifier(request)

if learner_identifier:
Expand Down
Loading