Skip to content

Commit 54c5590

Browse files
feat: implement instructor API v2 grading POST endpoints (#38299)
1 parent e2cd3df commit 54c5590

4 files changed

Lines changed: 883 additions & 97 deletions

File tree

lms/djangoapps/instructor/tests/views/test_api_v2.py

Lines changed: 293 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,16 @@
11
"""
2-
Tests for Instructor API v2 GET endpoints.
2+
Tests for Instructor API v2 endpoints.
33
"""
44
import json
5+
from textwrap import dedent
6+
from unittest.mock import MagicMock, patch
57
from uuid import uuid4
68

79
from django.urls import reverse
810
from rest_framework import status
911
from rest_framework.test import APIClient
1012

13+
from common.djangoapps.student.models import CourseEnrollment
1114
from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, InstructorFactory, UserFactory
1215
from lms.djangoapps.courseware.models import StudentModule
1316
from lms.djangoapps.instructor_task.models import InstructorTask
@@ -382,41 +385,310 @@ def setUp(self):
382385
self.client.force_authenticate(user=self.instructor)
383386

384387
def test_get_grading_config(self):
385-
"""Test retrieving grading configuration returns graders and grade cutoffs"""
388+
"""Test retrieving grading configuration returns HTML summary from dump_grading_context"""
386389
url = reverse('instructor_api_v2:grading_config', kwargs={
387390
'course_id': str(self.course.id),
388391
})
389392
response = self.client.get(url)
390393

391394
self.assertEqual(response.status_code, status.HTTP_200_OK) # noqa: PT009
392-
data = response.json()
393-
self.assertIn('graders', data) # noqa: PT009
394-
self.assertIn('grade_cutoffs', data) # noqa: PT009
395-
self.assertIsInstance(data['graders'], list) # noqa: PT009
396-
self.assertIsInstance(data['grade_cutoffs'], dict) # noqa: PT009
395+
self.assertEqual(response['Content-Type'], 'text/html') # noqa: PT009
396+
397+
hbar = '-' * 77
398+
expected_html = (
399+
f'<pre>{hbar}\n'
400+
'Course grader:\n'
401+
'&lt;class &#39;xmodule.graders.WeightedSubsectionsGrader&#39;&gt;\n'
402+
'\n'
403+
'Graded sections:\n'
404+
' subgrader=&lt;class &#39;xmodule.graders.AssignmentFormatGrader&#39;&gt;,'
405+
' type=Homework, category=Homework, weight=0.15\n'
406+
' subgrader=&lt;class &#39;xmodule.graders.AssignmentFormatGrader&#39;&gt;,'
407+
' type=Lab, category=Lab, weight=0.15\n'
408+
' subgrader=&lt;class &#39;xmodule.graders.AssignmentFormatGrader&#39;&gt;,'
409+
' type=Midterm Exam, category=Midterm Exam, weight=0.3\n'
410+
' subgrader=&lt;class &#39;xmodule.graders.AssignmentFormatGrader&#39;&gt;,'
411+
' type=Final Exam, category=Final Exam, weight=0.4\n'
412+
f'{hbar}\n'
413+
f'Listing grading context for course {self.course.id}\n'
414+
'graded sections:\n'
415+
'[]\n'
416+
'all graded blocks:\n'
417+
'length=0\n'
418+
'</pre>'
419+
)
420+
self.assertEqual(response.content.decode(), expected_html) # noqa: PT009
421+
422+
def test_get_grading_config_requires_authentication(self):
423+
"""Test that endpoint requires authentication"""
424+
self.client.force_authenticate(user=None)
397425

398-
def test_get_grading_config_grader_fields(self):
399-
"""Test that each grader entry has the expected fields"""
400426
url = reverse('instructor_api_v2:grading_config', kwargs={
401427
'course_id': str(self.course.id),
402428
})
403429
response = self.client.get(url)
404430

405-
self.assertEqual(response.status_code, status.HTTP_200_OK) # noqa: PT009
431+
self.assertIn(response.status_code, [status.HTTP_401_UNAUTHORIZED, status.HTTP_403_FORBIDDEN]) # noqa: PT009
432+
433+
434+
class GradingEndpointTestBase(ModuleStoreTestCase):
435+
"""
436+
Base test class for grading endpoints with real course structures,
437+
real permissions, and real StudentModule records.
438+
"""
439+
440+
PROBLEM_XML = dedent("""\
441+
<problem>
442+
<optionresponse>
443+
<optioninput options="('Option 1','Option 2')" correct="Option 1" />
444+
</optionresponse>
445+
</problem>
446+
""")
447+
448+
def setUp(self):
449+
super().setUp()
450+
self.client = APIClient()
451+
self.course = CourseFactory.create(display_name='Test Course')
452+
self.chapter = BlockFactory.create(
453+
parent=self.course,
454+
category='chapter',
455+
display_name='Week 1'
456+
)
457+
self.sequential = BlockFactory.create(
458+
parent=self.chapter,
459+
category='sequential',
460+
display_name='Homework 1'
461+
)
462+
self.problem = BlockFactory.create(
463+
parent=self.sequential,
464+
category='problem',
465+
display_name='Test Problem',
466+
data=self.PROBLEM_XML,
467+
)
468+
469+
# Real instructor with real course permissions
470+
self.instructor = InstructorFactory(course_key=self.course.id)
471+
self.client.force_authenticate(user=self.instructor)
472+
473+
# Real enrolled student with real module state
474+
self.student = UserFactory(username='test_student', email='[email protected]')
475+
CourseEnrollment.enroll(self.student, self.course.id)
476+
self.student_module = StudentModule.objects.create(
477+
student=self.student,
478+
course_id=self.course.id,
479+
module_state_key=self.problem.location,
480+
state=json.dumps({'attempts': 10}),
481+
)
482+
483+
484+
class ResetAttemptsViewTestCase(GradingEndpointTestBase):
485+
"""
486+
Tests for POST /api/instructor/v2/courses/{course_key}/{problem}/grading/attempts/reset
487+
"""
488+
489+
def _get_url(self, problem=None):
490+
return reverse('instructor_api_v2:reset_attempts', kwargs={
491+
'course_id': str(self.course.id),
492+
'problem': problem or str(self.problem.location),
493+
})
494+
495+
def test_reset_single_learner(self):
496+
"""Single learner reset zeroes attempt count and returns 200."""
497+
response = self.client.post(self._get_url() + '?learner=test_student')
498+
assert response.status_code == status.HTTP_200_OK
499+
data = response.json()
500+
assert data['success'] is True
501+
assert data['learner'] == 'test_student'
502+
assert data['message'] == 'Attempts reset successfully'
503+
504+
# Verify the actual StudentModule was modified
505+
self.student_module.refresh_from_db()
506+
assert json.loads(self.student_module.state)['attempts'] == 0
507+
508+
@patch('lms.djangoapps.instructor_task.tasks.reset_problem_attempts.apply_async')
509+
def test_reset_all_learners(self, mock_apply):
510+
"""Bulk reset queues a background task and returns 202."""
511+
response = self.client.post(self._get_url())
512+
assert response.status_code == status.HTTP_202_ACCEPTED
406513
data = response.json()
407-
for grader in data['graders']:
408-
self.assertIn('type', grader) # noqa: PT009
409-
self.assertIn('min_count', grader) # noqa: PT009
410-
self.assertIn('drop_count', grader) # noqa: PT009
411-
self.assertIn('weight', grader) # noqa: PT009
514+
assert 'task_id' in data
515+
assert 'status_url' in data
516+
assert data['scope']['learners'] == 'all'
517+
mock_apply.assert_called_once()
412518

413-
def test_get_grading_config_requires_authentication(self):
414-
"""Test that endpoint requires authentication"""
415-
self.client.force_authenticate(user=None)
416519

417-
url = reverse('instructor_api_v2:grading_config', kwargs={
520+
class DeleteStateViewTestCase(GradingEndpointTestBase):
521+
"""
522+
Tests for DELETE /api/instructor/v2/courses/{course_key}/{problem}/grading/state
523+
"""
524+
525+
def _get_url(self, problem=None):
526+
return reverse('instructor_api_v2:delete_state', kwargs={
418527
'course_id': str(self.course.id),
528+
'problem': problem or str(self.problem.location),
419529
})
420-
response = self.client.get(url)
421530

422-
self.assertIn(response.status_code, [status.HTTP_401_UNAUTHORIZED, status.HTTP_403_FORBIDDEN]) # noqa: PT009
531+
@patch('lms.djangoapps.grades.signals.handlers.PROBLEM_WEIGHTED_SCORE_CHANGED.send')
532+
def test_delete_state(self, _mock_signal): # noqa: PT019
533+
"""Delete state removes the StudentModule record and returns 200."""
534+
response = self.client.delete(self._get_url() + '?learner=test_student')
535+
assert response.status_code == status.HTTP_200_OK
536+
data = response.json()
537+
assert data['success'] is True
538+
assert data['learner'] == 'test_student'
539+
assert data['message'] == 'State deleted successfully'
540+
541+
# Verify the StudentModule was actually deleted
542+
assert not StudentModule.objects.filter(pk=self.student_module.pk).exists()
543+
544+
def test_delete_state_requires_learner_param(self):
545+
"""DELETE without learner query param returns 400."""
546+
response = self.client.delete(self._get_url())
547+
assert response.status_code == status.HTTP_400_BAD_REQUEST
548+
549+
@patch('lms.djangoapps.grades.signals.handlers.PROBLEM_WEIGHTED_SCORE_CHANGED.send')
550+
def test_delete_state_learner_in_body(self, _mock_signal): # noqa: PT019
551+
"""DELETE with learner in request body (form data) also works."""
552+
response = self.client.delete(self._get_url(), data={'learner': 'test_student'})
553+
assert response.status_code == status.HTTP_200_OK
554+
assert response.json()['success'] is True
555+
556+
557+
class RescoreViewTestCase(GradingEndpointTestBase):
558+
"""
559+
Tests for POST /api/instructor/v2/courses/{course_key}/{problem}/grading/scores/rescore
560+
"""
561+
562+
def _get_url(self, problem=None):
563+
return reverse('instructor_api_v2:rescore', kwargs={
564+
'course_id': str(self.course.id),
565+
'problem': problem or str(self.problem.location),
566+
})
567+
568+
@patch('lms.djangoapps.instructor_task.tasks.rescore_problem.apply_async')
569+
def test_rescore_single_learner(self, mock_apply):
570+
"""Single learner rescore queues a task and returns 202."""
571+
response = self.client.post(self._get_url() + '?learner=test_student')
572+
assert response.status_code == status.HTTP_202_ACCEPTED
573+
data = response.json()
574+
assert 'task_id' in data
575+
assert data['scope']['learners'] == 'test_student'
576+
mock_apply.assert_called_once()
577+
578+
@patch('lms.djangoapps.instructor_task.api.submit_rescore_problem_for_student')
579+
def test_rescore_only_if_higher(self, mock_submit):
580+
"""Rescore with only_if_higher=true passes the flag through."""
581+
mock_task = MagicMock()
582+
mock_task.task_id = str(uuid4())
583+
mock_submit.return_value = mock_task
584+
585+
response = self.client.post(self._get_url() + '?learner=test_student&only_if_higher=true')
586+
assert response.status_code == status.HTTP_202_ACCEPTED
587+
assert mock_submit.call_args[0][3] is True
588+
589+
@patch('lms.djangoapps.instructor_task.tasks.rescore_problem.apply_async')
590+
def test_rescore_all_learners(self, mock_apply):
591+
"""Bulk rescore queues a task and returns 202."""
592+
response = self.client.post(self._get_url())
593+
assert response.status_code == status.HTTP_202_ACCEPTED
594+
data = response.json()
595+
assert data['scope']['learners'] == 'all'
596+
mock_apply.assert_called_once()
597+
598+
599+
class ScoreOverrideViewTestCase(GradingEndpointTestBase):
600+
"""
601+
Tests for PUT /api/instructor/v2/courses/{course_key}/{problem}/grading/scores
602+
"""
603+
604+
def _get_url(self, problem=None):
605+
return reverse('instructor_api_v2:score_override', kwargs={
606+
'course_id': str(self.course.id),
607+
'problem': problem or str(self.problem.location),
608+
})
609+
610+
@patch('lms.djangoapps.instructor_task.tasks.override_problem_score.apply_async')
611+
def test_override_score(self, mock_apply):
612+
"""Score override queues a task and returns 202."""
613+
response = self.client.put(
614+
self._get_url() + '?learner=test_student',
615+
data={'score': 0.5},
616+
format='json',
617+
)
618+
assert response.status_code == status.HTTP_202_ACCEPTED
619+
data = response.json()
620+
assert 'task_id' in data
621+
assert data['scope']['learners'] == 'test_student'
622+
mock_apply.assert_called_once()
623+
624+
task = InstructorTask.objects.get(task_id=data['task_id'])
625+
assert json.loads(task.task_input)['score'] == 0.5
626+
627+
@patch('lms.djangoapps.instructor_task.tasks.override_problem_score.apply_async')
628+
def test_override_score_with_new_score_field(self, mock_apply):
629+
"""Score override also accepts 'new_score' field name (frontend compat)."""
630+
response = self.client.put(
631+
self._get_url(),
632+
data={'new_score': 0.5, 'learner': 'test_student'},
633+
format='json',
634+
)
635+
assert response.status_code == status.HTTP_202_ACCEPTED
636+
mock_apply.assert_called_once()
637+
638+
task = InstructorTask.objects.get(task_id=response.json()['task_id'])
639+
assert json.loads(task.task_input)['score'] == 0.5
640+
641+
@patch('lms.djangoapps.instructor_task.tasks.override_problem_score.apply_async')
642+
def test_override_score_zero(self, mock_apply):
643+
"""Score of 0 via 'score' field is a valid override."""
644+
response = self.client.put(
645+
self._get_url() + '?learner=test_student',
646+
data={'score': 0},
647+
format='json',
648+
)
649+
assert response.status_code == status.HTTP_202_ACCEPTED
650+
mock_apply.assert_called_once()
651+
652+
task = InstructorTask.objects.get(task_id=response.json()['task_id'])
653+
assert json.loads(task.task_input)['score'] == 0
654+
655+
@patch('lms.djangoapps.instructor_task.tasks.override_problem_score.apply_async')
656+
def test_override_new_score_zero(self, mock_apply):
657+
"""Score of 0 via 'new_score' field is a valid override."""
658+
response = self.client.put(
659+
self._get_url() + '?learner=test_student',
660+
data={'new_score': 0},
661+
format='json',
662+
)
663+
assert response.status_code == status.HTTP_202_ACCEPTED
664+
mock_apply.assert_called_once()
665+
666+
task = InstructorTask.objects.get(task_id=response.json()['task_id'])
667+
assert json.loads(task.task_input)['score'] == 0
668+
669+
def test_override_requires_learner_param(self):
670+
"""PUT without learner query param returns 400."""
671+
response = self.client.put(
672+
self._get_url(),
673+
data={'score': 8.5},
674+
format='json',
675+
)
676+
assert response.status_code == status.HTTP_400_BAD_REQUEST
677+
678+
def test_override_requires_score_in_body(self):
679+
"""PUT without score in body returns 400."""
680+
response = self.client.put(
681+
self._get_url() + '?learner=test_student',
682+
data={},
683+
format='json',
684+
)
685+
assert response.status_code == status.HTTP_400_BAD_REQUEST
686+
687+
def test_override_rejects_negative_score(self):
688+
"""PUT with negative score returns 400."""
689+
response = self.client.put(
690+
self._get_url() + '?learner=test_student',
691+
data={'score': -1},
692+
format='json',
693+
)
694+
assert response.status_code == status.HTTP_400_BAD_REQUEST

lms/djangoapps/instructor/views/api_urls.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,27 @@
136136
api_v2.CourseTeamView.as_view(),
137137
name='course_team'
138138
),
139+
# Grading endpoints
140+
re_path(
141+
rf'^courses/{COURSE_ID_PATTERN}/(?P<problem>.+)/grading/attempts/reset$',
142+
api_v2.ResetAttemptsView.as_view(),
143+
name='reset_attempts'
144+
),
145+
re_path(
146+
rf'^courses/{COURSE_ID_PATTERN}/(?P<problem>.+)/grading/state$',
147+
api_v2.DeleteStateView.as_view(),
148+
name='delete_state'
149+
),
150+
re_path(
151+
rf'^courses/{COURSE_ID_PATTERN}/(?P<problem>.+)/grading/scores/rescore$',
152+
api_v2.RescoreView.as_view(),
153+
name='rescore'
154+
),
155+
re_path(
156+
rf'^courses/{COURSE_ID_PATTERN}/(?P<problem>.+)/grading/scores$',
157+
api_v2.ScoreOverrideView.as_view(),
158+
name='score_override'
159+
),
139160
]
140161

141162
urlpatterns = [

0 commit comments

Comments
 (0)