WIP fixing views and views tests

autograder/core/models/user_late_days.py

@@ -29,8 +29,6 @@ class Meta:
 
     def clean(self):
         super().clean()
-        if self.user not in self.course.students.all():
-            raise ValidationError('The user is not a student in the course')
         if self.extra_late_days < 0:
             raise ValidationError('extra_late_days must be non-negative')
 
@@ -71,8 +69,6 @@ def to_dict(self):
 
     @staticmethod
     def get(user: User, course: Course) -> "LateDaysForUser":
-        if user not in course.students.all():
-            raise ValueError("user must be a student in the course")
         queryset = User.objects.filter(pk=user.pk)
         return LateDaysForUser.get_many(queryset, course)[0]
 

autograder/core/tests/test_models/test_late_days.py

@@ -129,11 +129,6 @@ def assert_late_days_correct(self,
 
 
 class LateDaysForUserGetTestCase(LateDaysForUserTestBase):
-    def test_not_a_student_error(self):
-        user = obj_build.make_user()
-        with self.assertRaises(ValueError):
-            ag_models.LateDaysForUser.get(user, self.course)
-
     def test_no_submissions_no_extra(self):
         user = self.make_student()
         late_days = ag_models.LateDaysForUser.get(user, self.course)

autograder/rest_api/schema/schema.yml

@@ -105,8 +105,8 @@ paths:
       tags:
       - courses
       - users
-    patch:
-      operationId: putLateDays
+    put:
+      operationId: putExtraLateDays
       description: ''
       parameters:
       - name: id
@@ -150,11 +150,15 @@ paths:
         description: '
 
           `extra_late_days_granted` + the number of late days set for the project
-          must be greater than or equal to `late_days_used`.
+          must be greater than or
+
+          equal to `late_days_used`.
 
 
           **Note:** the `late_days_used` field is a property computed from submission
-          data and can''t be changed.'
+          data and can''t be
+
+          changed.'
         required: true
       responses:
         '200':

autograder/rest_api/tests/test_views/test_user_views.py

@@ -1,3 +1,6 @@
+from datetime import datetime, timedelta
+import pytz
+
 from django.contrib.auth.models import Permission, User
 from django.urls import reverse
 from rest_framework import status
@@ -230,76 +233,102 @@ def setUp(self):
 
     def test_student_view_own_late_day_count(self):
         student = obj_build.make_student_user(self.course)
-        self.do_get_late_days_test(student, student, self.course, self.initial_num_late_days)
+        self.do_get_late_days_test(student, student, self.course, 0, 0)
 
     def test_guest_view_own_late_day_count(self):
         guest = obj_build.make_user()
-        self.do_get_late_days_test(guest, guest, self.course, self.initial_num_late_days)
+        self.do_get_late_days_test(guest, guest, self.course, 0, 0)
 
     def test_staff_view_other_late_day_count(self):
         staff = obj_build.make_staff_user(self.course)
         student = obj_build.make_student_user(self.course)
-        self.do_get_late_days_test(staff, student, self.course, self.initial_num_late_days)
+        self.do_get_late_days_test(staff, student, self.course, 0, 0)
 
-    def test_admin_change_late_day_count_by_pk(self):
+    def test_admin_change_extra_late_days_by_pk(self):
+        self.maxDiff=None
         admin = obj_build.make_admin_user(self.course)
         student = obj_build.make_student_user(self.course)
 
         self.client.force_authenticate(admin)
         response = self.client.put(self.get_pk_url(student, self.course),
-                                   {'late_days_remaining': 42})
+                                   {'extra_late_days': 42})
         self.assertEqual(status.HTTP_200_OK, response.status_code)
-        self.assertEqual({'late_days_remaining': 42}, response.data)
-
-        remaining = ag_models.LateDaysRemaining.objects.get(user=student, course=self.course)
-        self.assertEqual(42, remaining.late_days_remaining)
-
-    def test_admin_change_late_day_count_by_username(self):
+        self.assertEqual({
+            'extra_late_days': 42,
+            'late_days_used': 0,
+            'late_days_remaining': self.course.num_late_days + 42,
+            'user': serialize_user(student),
+            'course': self.course.to_dict()
+        }, response.data)
+
+        extra = ag_models.ExtraLateDays.objects.get(user=student, course=self.course)
+        self.assertEqual(42, extra.extra_late_days)
+
+    def test_admin_change_extra_late_days_by_username(self):
         admin = obj_build.make_admin_user(self.course)
         student = obj_build.make_student_user(self.course)
 
         self.client.force_authenticate(admin)
         response = self.client.put(self.get_username_url(student, self.course),
-                                   {'late_days_remaining': 42})
+                                   {'extra_late_days': 42})
         self.assertEqual(status.HTTP_200_OK, response.status_code)
-        self.assertEqual({'late_days_remaining': 42}, response.data)
-
-        remaining = ag_models.LateDaysRemaining.objects.get(user=student, course=self.course)
-        self.assertEqual(42, remaining.late_days_remaining)
-
-    def test_admin_change_late_day_count_by_pk_object_exists(self):
+        self.assertEqual({
+            'extra_late_days': 42,
+            'late_days_used': 0,
+            'late_days_remainig': self.course.num_late_days + 42,
+            'user': serialize_user(student),
+            'course': self.course.to_dict()
+        }, response.data)
+
+        extra = ag_models.ExtraLateDays.objects.get(user=student, course=self.course)
+        self.assertEqual(42, extra.extra_late_days)
+
+    def test_admin_change_extra_late_days_by_pk_object_exists(self):
         admin = obj_build.make_admin_user(self.course)
         student = obj_build.make_student_user(self.course)
 
-        ag_models.LateDaysRemaining.objects.validate_and_create(user=student, course=self.course)
-        self.do_get_late_days_test(admin, student, self.course, self.initial_num_late_days)
+        ag_models.ExtraLateDays.objects.validate_and_create(user=student, course=self.course)
+        self.do_get_late_days_test(admin, student, self.course, 0, 0)
 
         self.client.force_authenticate(admin)
         response = self.client.put(self.get_pk_url(student, self.course),
-                                   {'late_days_remaining': 27})
+                                   {'extra_late_days': 27})
         self.assertEqual(status.HTTP_200_OK, response.status_code)
-        self.assertEqual({'late_days_remaining': 27}, response.data)
+        self.assertEqual({
+            'late_days_used': 0,
+            'late_days_remaining': 27 + self.course.num_late_days,
+            'extra_late_days': 27,
+            'user': serialize_user(student),
+            'course': self.course.to_dict()
+        }, response.data)
 
-        remaining = ag_models.LateDaysRemaining.objects.get(user=student, course=self.course)
-        self.assertEqual(27, remaining.late_days_remaining)
+        extra = ag_models.ExtraLateDays.objects.get(user=student, course=self.course)
+        self.assertEqual(27, extra.extra_late_days)
 
     def test_admin_change_late_day_count_by_username_object_exists(self):
         admin = obj_build.make_admin_user(self.course)
         student = obj_build.make_student_user(self.course)
 
-        ag_models.LateDaysRemaining.objects.validate_and_create(user=student, course=self.course)
-        self.do_get_late_days_test(admin, student, self.course, self.initial_num_late_days)
+        ag_models.ExtraLateDays.objects.validate_and_create(user=student, course=self.course)
+        self.do_get_late_days_test(admin, student, self.course, 0, 0)
 
         self.client.force_authenticate(admin)
         response = self.client.put(self.get_username_url(student, self.course),
-                                   {'late_days_remaining': 27})
+                                   {'extra_late_days': 27})
         self.assertEqual(status.HTTP_200_OK, response.status_code)
-        self.assertEqual({'late_days_remaining': 27}, response.data)
+        self.assertEqual({
+            'late_days_used': 0,
+            'late_days_remaining': 27 + self.course.num_late_days,
+            'extra_late_days': 27,
+            'user': serialize_user(student),
+            'course': self.course.to_dict()
+        }, response.data)
 
-        remaining = ag_models.LateDaysRemaining.objects.get(user=student, course=self.course)
-        self.assertEqual(27, remaining.late_days_remaining)
 
-    def test_admin_change_late_day_count_for_other_course_permission_denied(self):
+        extra = ag_models.ExtraLateDays.objects.get(user=student, course=self.course)
+        self.assertEqual(27, extra.extra_late_days)
+
+    def test_admin_change_extra_late_days_for_other_course_permission_denied(self):
         admin = obj_build.make_admin_user(self.course)
 
         # Student for other course
@@ -309,22 +338,22 @@ def test_admin_change_late_day_count_for_other_course_permission_denied(self):
 
         self.client.force_authenticate(admin)
         response = self.client.put(self.get_pk_url(other_course_student, other_course),
-                                   {'late_days_remaining': 10})
+                                   {'extra_late_days': 10})
         self.assertEqual(status.HTTP_403_FORBIDDEN, response.status_code)
 
         response = self.client.put(self.get_username_url(other_course_student, other_course),
-                                   {'late_days_remaining': 10})
+                                   {'extra_late_days': 10})
         self.assertEqual(status.HTTP_403_FORBIDDEN, response.status_code)
 
         # Guest for other course
         other_guest = obj_build.make_user()
         response = self.client.put(self.get_pk_url(other_guest, other_course),
-                                   {'late_days_remaining': 7})
+                                   {'extra_late_days': 7})
         self.assertEqual(status.HTTP_403_FORBIDDEN, response.status_code)
 
         other_guest = obj_build.make_user()
         response = self.client.put(self.get_username_url(other_guest, other_course),
-                                   {'late_days_remaining': 7})
+                                   {'extra_late_days': 7})
         self.assertEqual(status.HTTP_403_FORBIDDEN, response.status_code)
 
     def test_staff_view_late_day_count_for_other_course_permission_denied(self):
@@ -373,26 +402,26 @@ def test_guest_view_other_late_day_count_permission_denied(self):
         self.assertEqual(status.HTTP_403_FORBIDDEN, response.status_code)
 
     def test_get_late_day_count_object_exists(self):
-        student = obj_build.make_student_user(self.course)
-
-        remaining: ag_models.LateDaysRemaining = (
-            ag_models.LateDaysRemaining.objects.validate_and_create(
-                user=student, course=self.course))
-        remaining.late_days_remaining -= 1
-        remaining.save()
-
-        self.do_get_late_days_test(student, student, self.course, remaining.late_days_remaining)
-
-    def test_missing_course_pk_query_param(self):
-        student = obj_build.make_student_user(self.course)
-        self.client.force_authenticate(student)
-        url = reverse('user-late-days', kwargs={'username_or_pk': student.pk})
-
-        response = self.client.get(url)
-        self.assertEqual(status.HTTP_400_BAD_REQUEST, response.status_code)
-
-        response = self.client.put(url, {})
-        self.assertEqual(status.HTTP_400_BAD_REQUEST, response.status_code)
+        self.maxDiff = None
+        base_time = pytz.utc.localize(datetime.now()),
+        group = obj_build.build_group(
+            project_kwargs={
+                'course': self.course,
+                'closing_time': base_time,
+            }
+        )
+        student = group.members.first()
+        sub = obj_build.make_finished_submission(group=group, submission_kwargs={
+            'timestamp': group.project.closing_time + timedelta(days=1)
+        })
+
+        self.do_get_late_days_test(
+            student,
+            student,
+            self.course,
+            1,
+            0
+        )
 
     def test_put_missing_body_param(self):
         admin = obj_build.make_admin_user(self.course)
@@ -407,22 +436,29 @@ def test_put_missing_body_param(self):
     def test_get_late_days_course_has_no_late_days(self):
         self.course.validate_and_update(num_late_days=0)
         student = obj_build.make_student_user(self.course)
-        self.do_get_late_days_test(student, student, self.course, 0)
+        self.do_get_late_days_test(student, student, self.course, 0, 0)
 
     def do_get_late_days_test(self, requestor: User, requestee: User, course: ag_models.Course,
-                              expected_num_late_days: int):
+                              expected_late_days_used: int, expected_extra_late_days: int):
         self.client.force_authenticate(requestor)
 
         for url in self.get_pk_url(requestee, course), self.get_username_url(requestee, course):
             response = self.client.get(url)
 
             self.assertEqual(status.HTTP_200_OK, response.status_code)
-            self.assertEqual({'late_days_remaining': expected_num_late_days}, response.data)
+            self.assertEqual({
+                'late_days_remaining': course.num_late_days + expected_extra_late_days
+                    - expected_late_days_used,
+                'late_days_used': expected_late_days_used,
+                'extra_late_days': expected_extra_late_days,
+                'course': course.to_dict(),
+                'user': serialize_user(requestee)
+            }, response.data)
 
     def get_pk_url(self, requestee: User, course: ag_models.Course):
-        url = reverse('user-late-days', kwargs={'username_or_pk': requestee.pk})
-        return url + f'?course_pk={course.pk}'
+        url = reverse('user-late-days', kwargs={'pk': course.pk, 'username_or_pk': requestee.pk})
+        return url
 
     def get_username_url(self, requestee: User, course: ag_models.Course):
-        url = reverse('user-late-days', kwargs={'username_or_pk': requestee.username})
-        return url + f'?course_pk={course.pk}'
+        url = reverse('user-late-days', kwargs={'pk': course.pk, 'username_or_pk': requestee.username})
+        return url

autograder/rest_api/views/user_late_days_views.py

@@ -91,8 +91,8 @@ class LateDaysView(AlwaysIsAuthenticatedMixin, APIView):
                 '200': _RESPONSE
             }
         },
-        'PATCH': {
-            'operation_id': 'putLateDays',
+        'PUT': {
+            'operation_id': 'putExtraLateDays',
             'parameters': _PARAMS,
             'request': _REQUEST,
             'responses': {
@@ -109,45 +109,43 @@ def get(self, request: Request, *args, **kwargs):
 
         course = get_object_or_404(ag_models.Course.objects, pk=kwargs['pk'])
 
-        late_days = ag_models.LateDaysForUser.get(user, course.pk)
-
-        self._check_read_permissions(late_days)
+        self._check_read_permissions(user, course)
+        late_days = ag_models.LateDaysForUser.get(user, course)
 
         return response.Response(late_days.to_dict())
 
     @method_decorator(require_body_params('extra_late_days'))
-    def patch(self, request: Request, *args, **kwargs):
+    def put(self, request: Request, *args, **kwargs):
         try:
             user = get_object_or_404(User.objects, pk=int(kwargs['username_or_pk']))
         except ValueError:
             user = get_object_or_404(User.objects, username=kwargs['username_or_pk'])
 
-        course = get_object_or_404(ag_models.Course.objects, pk=request.query_params['course_pk'])
+        course = get_object_or_404(ag_models.Course.objects, pk=int(kwargs['pk']))
+
+        self._check_read_permissions(user, course)
+        self._check_write_permissions(user, course)
 
         with transaction.atomic():
             extra_late_days = ag_models.ExtraLateDays.objects.get_or_create(
                 user=user, course=course
             )[0]
-
-            self._check_read_permissions(extra_late_days)
-            self._check_write_permissions(extra_late_days)
-
             extra_late_days.extra_late_days = request.data['extra_late_days']
             extra_late_days.save()
             late_days = ag_models.LateDaysForUser.get(user, course)
 
             return response.Response(late_days.to_dict())
 
-    def _check_read_permissions(self, late_days: ag_models.LateDaysForUser):
+    def _check_read_permissions(self, requestee: User, course: ag_models.Course):
         user = self.request.user
-        if user == late_days.user:
+        if user == requestee:
             return
 
-        if late_days.course.is_staff(user):
+        if course.is_staff(user):
             return
 
         raise PermissionDenied
 
-    def _check_write_permissions(self, late_days: ag_models.LateDaysForUser):
-        if not late_days.course.is_admin(self.request.user):
+    def _check_write_permissions(self, requestee: User, course: ag_models.Course):
+        if not course.is_admin(self.request.user):
             raise PermissionDenied