From 0fcfa2672ebad8dd1b5d4816436e4d6e1cdc7c9b Mon Sep 17 00:00:00 2001 From: Matt Mayfield Date: Fri, 23 Aug 2024 11:37:36 -0400 Subject: [PATCH 1/7] Add `soft_extended_due_date` field to `Group` model Includes server-side validation to ensure `extended_due_date < soft_extended_due_date` --- .../0108_group_soft_extended_due_date.py | 18 +++++++++ autograder/core/models/group/group.py | 34 ++++++++++++++++- .../test_models/test_group/test_group.py | 37 +++++++++++++++++-- autograder/rest_api/schema/schema.yml | 17 +++++++++ 4 files changed, 101 insertions(+), 5 deletions(-) create mode 100644 autograder/core/migrations/0108_group_soft_extended_due_date.py diff --git a/autograder/core/migrations/0108_group_soft_extended_due_date.py b/autograder/core/migrations/0108_group_soft_extended_due_date.py new file mode 100644 index 00000000..ff27b150 --- /dev/null +++ b/autograder/core/migrations/0108_group_soft_extended_due_date.py @@ -0,0 +1,18 @@ +# Generated by Django 3.2.2 on 2024-08-22 19:23 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0107_auto_20240806_1626'), + ] + + operations = [ + migrations.AddField( + model_name='group', + name='soft_extended_due_date', + field=models.DateTimeField(blank=True, default=None, help_text='When this field is set, it indicates the extended due date\n that is visible to members of the group. Members of the group will\n still be able to submit after this date but before the\n extended_due_date.\n Default value: None', null=True), + ), + ] diff --git a/autograder/core/models/group/group.py b/autograder/core/models/group/group.py index c8ccb72d..fb0bbf3e 100644 --- a/autograder/core/models/group/group.py +++ b/autograder/core/models/group/group.py @@ -49,9 +49,9 @@ def validate_and_create( # type: ignore member_names = [ user.username for user in sorted(members, key=lambda user: user.username)] group = self.model(_member_names=member_names, **kwargs) + group.full_clean() group.save() group.members.add(*members) - group.full_clean() return group @@ -100,6 +100,14 @@ def member_names(self) -> List[str]: date, overriding the project closing time. Default value: None""") + soft_extended_due_date = models.DateTimeField( + null=True, default=None, blank=True, + help_text="""When this field is set, it indicates the extended due date + that is visible to members of the group. Members of the group will + still be able to submit after this date but before the + extended_due_date. + Default value: None""") + # Remove in version 5.0.0 old_bonus_submissions_remaining = models.IntegerField( blank=True, @@ -179,6 +187,23 @@ def _is_towards_limit(submission: Submission) -> bool: return utils.count_if(self.submissions.all(), _is_towards_limit) + def clean(self) -> None: + super().clean() + + if self.extended_due_date is not None: + self.extended_due_date = self.extended_due_date.replace(second=0, microsecond=0) + if self.soft_extended_due_date is not None: + self.soft_extended_due_date = self.soft_extended_due_date.replace( + second=0, microsecond=0) + + print(f"{self.extended_due_date=}") + + if self.extended_due_date is not None and self.soft_extended_due_date is not None: + if self.extended_due_date < self.soft_extended_due_date: + raise ValidationError( + {'soft_extended_due_date': ( + 'Soft extended due date must be before hard extended due date')}) + def save(self, *args: Any, **kwargs: Any) -> None: super().save(*args, **kwargs) @@ -235,6 +260,7 @@ def validate_and_update( # type: ignore 'pk', 'project', 'extended_due_date', + 'soft_extended_due_date', 'member_names', 'members', @@ -250,7 +276,11 @@ def validate_and_update( # type: ignore ) SERIALIZE_RELATED = ('members',) - EDITABLE_FIELDS = ('extended_due_date', 'bonus_submissions_remaining') + EDITABLE_FIELDS = ( + 'extended_due_date', + 'soft_extended_due_date', + 'bonus_submissions_remaining' + ) def to_dict(self) -> Dict[str, object]: result = super().to_dict() diff --git a/autograder/core/tests/test_models/test_group/test_group.py b/autograder/core/tests/test_models/test_group/test_group.py index d4ed9c7a..48d2ecb6 100644 --- a/autograder/core/tests/test_models/test_group/test_group.py +++ b/autograder/core/tests/test_models/test_group/test_group.py @@ -63,7 +63,9 @@ def test_valid_initialization_no_defaults(self): group.refresh_from_db() - self.assertEqual(group.extended_due_date, extended_due_date) + self.assertEqual( + group.extended_due_date, + extended_due_date.replace(second=0, microsecond=0)) self.assertCountEqual(self.student_users, group.members.all()) self.assertEqual(self.project, group.project) @@ -202,6 +204,7 @@ def test_serializable_fields(self): 'members', 'project', 'extended_due_date', + 'soft_extended_due_date', 'bonus_submissions_remaining', @@ -221,8 +224,9 @@ def test_serializable_fields(self): self.assertIsInstance(serialized['members'][0], dict) def test_editable_fields(self): - self.assertCountEqual(['extended_due_date', 'bonus_submissions_remaining'], - ag_models.Group.get_editable_fields()) + self.assertCountEqual( + ['extended_due_date', 'soft_extended_due_date', 'bonus_submissions_remaining'], + ag_models.Group.get_editable_fields()) class BonusSubmissionTokenCountTestCase(test_ut.UnitTestBase): @@ -632,3 +636,30 @@ def test_exception_group_mix_of_student_and_staff(self): ag_models.Group.objects.validate_and_create( members=self.staff_users + self.student_users, project=self.project) + + +class HardAndSoftExtendedDueDateTestCase(_SetUp): + def test_valid_soft_extended_due_date_None_extended_due_date(self): + extended_due_date = timezone.now() + datetime.timedelta(days=1) + group = ag_models.Group.objects.validate_and_create( + members=self.student_users, + project=self.project, + extended_due_date=extended_due_date, + soft_extended_due_date=None) + + group.refresh_from_db() + self.assertEqual( + extended_due_date.replace(second=0, microsecond=0), + group.extended_due_date) + self.assertIsNone(group.soft_extended_due_date) + + def test_error_soft_extended_due_date_after_extended_due_date(self): + extended_due_date = timezone.now() + timezone.timedelta(minutes=5) + soft_extended_due_date = extended_due_date + timezone.timedelta(minutes=5) + + with self.assertRaises(exceptions.ValidationError): + ag_models.Group.objects.validate_and_create( + members=self.student_users, + project=self.project, + extended_due_date=extended_due_date, + soft_extended_due_date=soft_extended_due_date) diff --git a/autograder/rest_api/schema/schema.yml b/autograder/rest_api/schema/schema.yml index 4dd9c96f..82143226 100644 --- a/autograder/rest_api/schema/schema.yml +++ b/autograder/rest_api/schema/schema.yml @@ -5551,6 +5551,14 @@ components: nullable: true type: string format: date-time + soft_extended_due_date: + description: "When this field is set, it indicates the extended due date\n\ + \ that is visible to members of the group. Members of the group\ + \ will\n still be able to submit after this date but before\ + \ the\n extended_due_date.\n Default value: None" + nullable: true + type: string + format: date-time member_names: description: A list of usernames of the group members, sorted alphabetically. type: array @@ -5601,6 +5609,7 @@ components: - pk - project - extended_due_date + - soft_extended_due_date - member_names - members - bonus_submissions_remaining @@ -5635,6 +5644,14 @@ components: nullable: true type: string format: date-time + soft_extended_due_date: + description: "When this field is set, it indicates the extended due date\n\ + \ that is visible to members of the group. Members of the group\ + \ will\n still be able to submit after this date but before\ + \ the\n extended_due_date.\n Default value: None" + nullable: true + type: string + format: date-time bonus_submissions_remaining: description: The number of unused bonus submission tokens this group has. type: integer From 3e7870fe20776eaf331c5c3e3ca385120edc0856 Mon Sep 17 00:00:00 2001 From: Matt Mayfield Date: Mon, 26 Aug 2024 12:30:16 -0400 Subject: [PATCH 2/7] Add hard_extended_due_date field, make extended_due_date a property Migration: 1. Add soft_extended_due_date column 2. Add hard_extended_due_date column 3. Copy extended_due_date column to both new columns 4. Delete extended_due_date column deprecated extended_due_date behavior: - reads from soft_extended_due_date field - writes through to soft_extended_due_date and hard_extended_due_date --- .../0108_group_soft_extended_due_date.py | 19 +++++ ...0109_alter_group_hard_extended_due_date.py | 18 +++++ autograder/core/models/group/group.py | 49 ++++++++---- autograder/core/models/project/project.py | 28 ++++--- .../test_models/test_group/test_group.py | 54 +++++++++++-- autograder/core/utils.py | 60 ++++++++++++++- autograder/rest_api/schema/schema.yml | 13 +++- .../test_tasks/test_project_downloads.py | 8 +- .../tests/test_views/ag_view_test_base.py | 13 +++- .../test_group_views/test_group_views.py | 75 +++++++++++++++++-- .../test_project_views/test_project_views.py | 11 ++- autograder/rest_api/views/group_views.py | 1 + 12 files changed, 300 insertions(+), 49 deletions(-) create mode 100644 autograder/core/migrations/0109_alter_group_hard_extended_due_date.py diff --git a/autograder/core/migrations/0108_group_soft_extended_due_date.py b/autograder/core/migrations/0108_group_soft_extended_due_date.py index ff27b150..324bf9b8 100644 --- a/autograder/core/migrations/0108_group_soft_extended_due_date.py +++ b/autograder/core/migrations/0108_group_soft_extended_due_date.py @@ -2,6 +2,10 @@ from django.db import migrations, models +def copy_extended_deadline_to_soft_extended_deadline(apps, schema): + model = apps.get_model('core', 'Group') + model.objects.all().update(soft_extended_due_date=models.F('extended_due_date')) + model.objects.all().update(hard_extended_due_date=models.F('extended_due_date')) class Migration(migrations.Migration): @@ -15,4 +19,19 @@ class Migration(migrations.Migration): name='soft_extended_due_date', field=models.DateTimeField(blank=True, default=None, help_text='When this field is set, it indicates the extended due date\n that is visible to members of the group. Members of the group will\n still be able to submit after this date but before the\n extended_due_date.\n Default value: None', null=True), ), + migrations.AddField( + model_name='group', + name='hard_extended_due_date', + field=models.DateTimeField(blank=True, default=None, null=True, + help_text="""When this field is set, it indicates that members + of this submission group can submit until this specified + date, overriding the project closing time. + Default value: None"""), + ), + migrations.RunPython(copy_extended_deadline_to_soft_extended_deadline), + migrations.RemoveField( + model_name='group', + name='extended_due_date', + field=models.DateTimeField(blank=True, default=None, null=True), + ) ] diff --git a/autograder/core/migrations/0109_alter_group_hard_extended_due_date.py b/autograder/core/migrations/0109_alter_group_hard_extended_due_date.py new file mode 100644 index 00000000..4e9fe134 --- /dev/null +++ b/autograder/core/migrations/0109_alter_group_hard_extended_due_date.py @@ -0,0 +1,18 @@ +# Generated by Django 3.2.2 on 2024-08-26 19:42 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0108_group_soft_extended_due_date'), + ] + + operations = [ + migrations.AlterField( + model_name='group', + name='hard_extended_due_date', + field=models.DateTimeField(blank=True, default=None, help_text='When this field is set, it indicates that members\n of this submission group can submit until this specified\n date, overriding the project closing time.\n Default value: None', null=True), + ), + ] diff --git a/autograder/core/models/group/group.py b/autograder/core/models/group/group.py index fb0bbf3e..06f8c786 100644 --- a/autograder/core/models/group/group.py +++ b/autograder/core/models/group/group.py @@ -1,7 +1,8 @@ from __future__ import annotations import os -from typing import Any, Dict, Iterable, List, cast +from typing import Any, Dict, Iterable, List, Optional, cast +from datetime import datetime import zoneinfo import django.contrib.postgres.fields as pg_fields @@ -93,7 +94,16 @@ def member_names(self) -> List[str]: project = models.ForeignKey(Project, related_name="groups", on_delete=models.CASCADE) - extended_due_date = models.DateTimeField( + @property + def extended_due_date(self) -> Optional[datetime]: + return self.soft_extended_due_date + + @extended_due_date.setter + def extended_due_date(self, value: Optional[datetime]) -> None: + self.soft_extended_due_date = value + self.hard_extended_due_date = value + + hard_extended_due_date = models.DateTimeField( null=True, default=None, blank=True, help_text="""When this field is set, it indicates that members of this submission group can submit until this specified @@ -190,19 +200,24 @@ def _is_towards_limit(submission: Submission) -> bool: def clean(self) -> None: super().clean() - if self.extended_due_date is not None: - self.extended_due_date = self.extended_due_date.replace(second=0, microsecond=0) - if self.soft_extended_due_date is not None: - self.soft_extended_due_date = self.soft_extended_due_date.replace( - second=0, microsecond=0) - - print(f"{self.extended_due_date=}") - - if self.extended_due_date is not None and self.soft_extended_due_date is not None: - if self.extended_due_date < self.soft_extended_due_date: - raise ValidationError( - {'soft_extended_due_date': ( - 'Soft extended due date must be before hard extended due date')}) + try: + clean_soft, clean_hard = core_ut.clean_and_validate_soft_and_hard_deadlines( + self.soft_extended_due_date, self.hard_extended_due_date) + except core_ut.InvalidSoftDeadlineError: + raise ValidationError( + {'soft_extended_due_date': ( + 'Soft extended due date must be a valid date')}) + except core_ut.InvalidHardDeadlineError: + raise ValidationError( + {'extended_due_date': ( + 'Hard extended due date must be a valid date')}) + except core_ut.HardDeadlineBeforeSoftDeadlineError: + raise ValidationError( + {'soft_extended_due_date': ( + 'Soft extended due date must not be after hard extended due date')}) + + self.soft_extended_due_date = clean_soft + self.hard_extended_due_date = clean_hard def save(self, *args: Any, **kwargs: Any) -> None: super().save(*args, **kwargs) @@ -259,8 +274,9 @@ def validate_and_update( # type: ignore SERIALIZABLE_FIELDS = ( 'pk', 'project', - 'extended_due_date', + 'hard_extended_due_date', 'soft_extended_due_date', + 'extended_due_date', 'member_names', 'members', @@ -278,6 +294,7 @@ def validate_and_update( # type: ignore EDITABLE_FIELDS = ( 'extended_due_date', + 'hard_extended_due_date', 'soft_extended_due_date', 'bonus_submissions_remaining' ) diff --git a/autograder/core/models/project/project.py b/autograder/core/models/project/project.py index 7f857f3c..4624616d 100644 --- a/autograder/core/models/project/project.py +++ b/autograder/core/models/project/project.py @@ -238,16 +238,24 @@ def clean(self) -> None: {'max_group_size': ('Maximum group size must be greater than ' 'or equal to minimum group size')}) - if self.closing_time is not None: - self.closing_time = self.closing_time.replace(second=0, microsecond=0) - if self.soft_closing_time is not None: - self.soft_closing_time = self.soft_closing_time.replace(second=0, microsecond=0) - - if self.closing_time is not None and self.soft_closing_time is not None: - if self.closing_time < self.soft_closing_time: - raise exceptions.ValidationError( - {'soft_closing_time': ( - 'Soft closing time must be before hard closing time')}) + try: + clean_soft, clean_hard = core_ut.clean_and_validate_soft_and_hard_deadlines( + self.soft_closing_time, self.closing_time) + except core_ut.InvalidSoftDeadlineError: + raise exceptions.ValidationError( + {'soft_closing_time': ( + 'Soft closing time must be a valid date')}) + except core_ut.InvalidHardDeadlineError: + raise exceptions.ValidationError( + {'closing_time': ( + 'Closing time must be a valid date')}) + except core_ut.HardDeadlineBeforeSoftDeadlineError: + raise exceptions.ValidationError( + {'soft_closing_time': ( + 'Soft closing time must not be after hard closing time')}) + + self.soft_closing_time = clean_soft + self.closing_time = clean_hard @property def has_handgrading_rubric(self) -> bool: diff --git a/autograder/core/tests/test_models/test_group/test_group.py b/autograder/core/tests/test_models/test_group/test_group.py index 48d2ecb6..925f7f6a 100644 --- a/autograder/core/tests/test_models/test_group/test_group.py +++ b/autograder/core/tests/test_models/test_group/test_group.py @@ -205,6 +205,7 @@ def test_serializable_fields(self): 'project', 'extended_due_date', 'soft_extended_due_date', + 'hard_extended_due_date', 'bonus_submissions_remaining', @@ -225,7 +226,8 @@ def test_serializable_fields(self): def test_editable_fields(self): self.assertCountEqual( - ['extended_due_date', 'soft_extended_due_date', 'bonus_submissions_remaining'], + ['extended_due_date', 'soft_extended_due_date', 'hard_extended_due_date', + 'bonus_submissions_remaining'], ag_models.Group.get_editable_fields()) @@ -638,28 +640,64 @@ def test_exception_group_mix_of_student_and_staff(self): project=self.project) -class HardAndSoftExtendedDueDateTestCase(_SetUp): - def test_valid_soft_extended_due_date_None_extended_due_date(self): +class ExtendedDueDateBackwardsCompatabilityTestCase(_SetUp): + def test_extended_due_date_sets_hard_and_soft_extended_due_dates(self): extended_due_date = timezone.now() + datetime.timedelta(days=1) group = ag_models.Group.objects.validate_and_create( members=self.student_users, project=self.project, - extended_due_date=extended_due_date, - soft_extended_due_date=None) + extended_due_date=extended_due_date) group.refresh_from_db() self.assertEqual( extended_due_date.replace(second=0, microsecond=0), group.extended_due_date) + self.assertEqual( + extended_due_date.replace(second=0, microsecond=0), + group.soft_extended_due_date) + self.assertEqual( + extended_due_date.replace(second=0, microsecond=0), + group.hard_extended_due_date) + + def test_getting_extended_due_date_reads_soft_extended_due_date(self): + soft_extended_due_date = timezone.now() + datetime.timedelta(days=1) + hard_extended_due_date = soft_extended_due_date + datetime.timedelta(days=1) + group = ag_models.Group.objects.validate_and_create( + members=self.student_users, + project=self.project, + soft_extended_due_date=soft_extended_due_date, + hard_extended_due_date=hard_extended_due_date) + + self.assertEqual( + soft_extended_due_date.replace(second=0, microsecond=0), + group.extended_due_date) + self.assertEqual( + hard_extended_due_date.replace(second=0, microsecond=0), + group.hard_extended_due_date) + + +class HardAndSoftExtendedDueDateTestCase(_SetUp): + def test_valid_soft_extended_due_date_None_hard_extended_due_date_not_None(self): + hard_extended_due_date = timezone.now() + datetime.timedelta(days=1) + group = ag_models.Group.objects.validate_and_create( + members=self.student_users, + project=self.project, + hard_extended_due_date=hard_extended_due_date, + soft_extended_due_date=None) + + group.refresh_from_db() + self.assertEqual( + hard_extended_due_date.replace(second=0, microsecond=0), + group.hard_extended_due_date) self.assertIsNone(group.soft_extended_due_date) def test_error_soft_extended_due_date_after_extended_due_date(self): - extended_due_date = timezone.now() + timezone.timedelta(minutes=5) - soft_extended_due_date = extended_due_date + timezone.timedelta(minutes=5) + hard_extended_due_date = timezone.now() + timezone.timedelta(minutes=5) + soft_extended_due_date = hard_extended_due_date + timezone.timedelta(minutes=5) with self.assertRaises(exceptions.ValidationError): ag_models.Group.objects.validate_and_create( members=self.student_users, project=self.project, - extended_due_date=extended_due_date, + hard_extended_due_date=hard_extended_due_date, soft_extended_due_date=soft_extended_due_date) diff --git a/autograder/core/utils.py b/autograder/core/utils.py index 0da01b02..052d35e9 100644 --- a/autograder/core/utils.py +++ b/autograder/core/utils.py @@ -6,7 +6,7 @@ import re import subprocess import typing -from typing import List, Tuple, Type, TypeVar, cast +from typing import Any, List, Tuple, Type, TypeVar, cast import zoneinfo from django.conf import settings @@ -22,6 +22,64 @@ from .models.submission import Submission +class InvalidSoftDeadlineError(Exception): + """ + Raised when a soft deadline or due date is an invalid value (not None or + a datetime.datetime object). This is a helper exception to make it easier + to abstract deadline validation despite different field names. + """ + pass + + +class InvalidHardDeadlineError(Exception): + """ + Raised when a hard deadline or due date is an invalid value (not None or + a datetime.datetime object). This is a helper exception to make it easier + to abstract deadline validation despite different field names. + """ + pass + + +class HardDeadlineBeforeSoftDeadlineError(Exception): + """ + Raised when a soft deadline or due date is later than a hard deadline or + due date. This is a helper exception to make it easier to abstract deadline + validation despite different field names. + """ + pass + + +def clean_and_validate_soft_and_hard_deadlines( + soft_deadline: Any, + hard_deadline: Any +) -> Tuple[datetime.datetime | None, datetime.datetime | None]: + """ + Validate and remove seconds and microseconds from deadline dates. + + :raises: + InvalidSoftDeadlineError: when soft_deadline is not a datetime.datetime object or None. + :raises: + InvalidHardDeadlineError: when hard_deadline is not a datetime.datetime object or None. + :raises: + SoftDeadlineAfterHardDeadlineError: when hard_deadline < soft_deadline. + """ + if hard_deadline is not None and not isinstance(hard_deadline, datetime.datetime): + raise InvalidHardDeadlineError + if soft_deadline is not None and not isinstance(soft_deadline, datetime.datetime): + raise InvalidSoftDeadlineError + + if hard_deadline is not None: + hard_deadline = hard_deadline.replace(second=0, microsecond=0) + if soft_deadline is not None: + soft_deadline = soft_deadline.replace(second=0, microsecond=0) + + if hard_deadline is not None and soft_deadline is not None: + if hard_deadline < soft_deadline: + raise HardDeadlineBeforeSoftDeadlineError + + return soft_deadline, hard_deadline + + class DiffResult: def __init__(self, diff_pass: bool, diff_content: List[str]): self.diff_pass = diff_pass diff --git a/autograder/rest_api/schema/schema.yml b/autograder/rest_api/schema/schema.yml index 82143226..b1b94092 100644 --- a/autograder/rest_api/schema/schema.yml +++ b/autograder/rest_api/schema/schema.yml @@ -5543,7 +5543,7 @@ components: description: '' nullable: false type: integer - extended_due_date: + hard_extended_due_date: description: "When this field is set, it indicates that members\n \ \ of this submission group can submit until this specified\n \ \ date, overriding the project closing time.\n Default\ @@ -5559,6 +5559,10 @@ components: nullable: true type: string format: date-time + extended_due_date: + description: '' + nullable: true + type: unknown member_names: description: A list of usernames of the group members, sorted alphabetically. type: array @@ -5608,8 +5612,9 @@ components: required: - pk - project - - extended_due_date + - hard_extended_due_date - soft_extended_due_date + - extended_due_date - member_names - members - bonus_submissions_remaining @@ -5637,6 +5642,10 @@ components: items: type: string extended_due_date: + description: '' + nullable: true + type: unknown + hard_extended_due_date: description: "When this field is set, it indicates that members\n \ \ of this submission group can submit until this specified\n \ \ date, overriding the project closing time.\n Default\ diff --git a/autograder/rest_api/tests/test_tasks/test_project_downloads.py b/autograder/rest_api/tests/test_tasks/test_project_downloads.py index f43f24b9..5575c8db 100644 --- a/autograder/rest_api/tests/test_tasks/test_project_downloads.py +++ b/autograder/rest_api/tests/test_tasks/test_project_downloads.py @@ -354,7 +354,7 @@ def test_group_has_past_extension(self): 'Group Members': ( f'{self.student_group.member_names[0]},{self.student_group.member_names[1]}'), 'Timestamp': str(self.student_submission.timestamp), - 'Extension': str(student_extension), + 'Extension': str(student_extension.replace(second=0, microsecond=0)), 'Total Points': str(self.student_result_fdbk.total_points), 'Total Points Possible': str(self.student_result_fdbk.total_points_possible), # These point values are the same as the totals because its the only test case @@ -369,7 +369,7 @@ def test_group_has_past_extension(self): 'Group Members': ( f'{self.student_group.member_names[0]},{self.student_group.member_names[1]}'), 'Timestamp': str(self.student_submission.timestamp), - 'Extension': str(student_extension), + 'Extension': str(student_extension.replace(second=0, microsecond=0)), 'Total Points': str(self.student_result_fdbk.total_points), 'Total Points Possible': str(self.student_result_fdbk.total_points_possible), f'{self.ag_test_suite.name} Total': (str(self.student_result_fdbk.total_points)), @@ -420,7 +420,7 @@ def test_group_has_extension_not_past_include_pending_extensions_true(self): 'Group Members': ( f'{self.student_group.member_names[0]},{self.student_group.member_names[1]}'), 'Timestamp': str(self.student_submission.timestamp), - 'Extension': str(student_extension), + 'Extension': str(student_extension.replace(second=0, microsecond=0)), 'Total Points': str(self.student_result_fdbk.total_points), 'Total Points Possible': str(self.student_result_fdbk.total_points_possible), # These point values are the same as the totals because its the only test case @@ -435,7 +435,7 @@ def test_group_has_extension_not_past_include_pending_extensions_true(self): 'Group Members': ( f'{self.student_group.member_names[0]},{self.student_group.member_names[1]}'), 'Timestamp': str(self.student_submission.timestamp), - 'Extension': str(student_extension), + 'Extension': str(student_extension.replace(second=0, microsecond=0)), 'Total Points': str(self.student_result_fdbk.total_points), 'Total Points Possible': str(self.student_result_fdbk.total_points_possible), f'{self.ag_test_suite.name} Total': (str(self.student_result_fdbk.total_points)), diff --git a/autograder/rest_api/tests/test_views/ag_view_test_base.py b/autograder/rest_api/tests/test_views/ag_view_test_base.py index aab74c3f..e1aa0044 100644 --- a/autograder/rest_api/tests/test_views/ag_view_test_base.py +++ b/autograder/rest_api/tests/test_views/ag_view_test_base.py @@ -87,8 +87,8 @@ def do_create_object_test(self, model_manager, client, return response def do_patch_object_test(self, ag_model_obj, client, user, url, - request_data, format='json', - ignore_fields=[]): + request_data, expected_response_overrides=None, + format='json', ignore_fields=[]): ignore_fields = list(ignore_fields) ignore_fields.append('last_modified') @@ -100,6 +100,12 @@ def do_patch_object_test(self, ag_model_obj, client, user, url, expected_data[key].update(value) else: expected_data[key] = value + if expected_response_overrides is not None: + for key, value in expected_response_overrides.items(): + if isinstance(value, dict): + expected_data[key].update(value) + else: + expected_data[key] = value client.force_authenticate(user) response = client.patch(url, request_data, format=format) @@ -165,6 +171,9 @@ def _do_bad_update_test(self, ag_model_obj, client, user, url, request_data, self.assertEqual(expected_status, response.status_code) ag_model_obj = ag_model_obj._meta.model.objects.get(pk=ag_model_obj.pk) + + print(f"{expected_data=}") + print(f"{ag_model_obj.to_dict()=}") self.assert_dict_contents_equal(expected_data, ag_model_obj.to_dict()) return response diff --git a/autograder/rest_api/tests/test_views/test_group_views/test_group_views.py b/autograder/rest_api/tests/test_views/test_group_views/test_group_views.py index c7cb18b0..42e38c65 100644 --- a/autograder/rest_api/tests/test_views/test_group_views/test_group_views.py +++ b/autograder/rest_api/tests/test_views/test_group_views/test_group_views.py @@ -406,7 +406,7 @@ def group_url(self, group: ag_models.Group) -> str: class UpdateGroupTestCase(AGViewTestBase): def setUp(self): super().setUp() - self.new_due_date = timezone.now().replace(microsecond=0) + self.new_due_date = timezone.now() self.client = APIClient() self.project = obj_build.make_project() @@ -456,15 +456,77 @@ def test_admin_update_guest_group_members(self): self.assertCountEqual( [serialize_user(user) for user in new_members], response.data['members']) - def test_admin_update_group_extension(self): + def test_admin_update_group_deprecated_extended_due_date(self): group = obj_build.make_group(project=self.project) + self.do_patch_object_test( group, self.client, self.admin, self.group_url(group), - {'extended_due_date': self.new_due_date}) + {'extended_due_date': self.new_due_date}, + expected_response_overrides={ + 'extended_due_date': self.new_due_date.replace(second=0, microsecond=0), + 'soft_extended_due_date': self.new_due_date.replace(second=0, microsecond=0), + 'hard_extended_due_date': self.new_due_date.replace(second=0, microsecond=0) + }) self.do_patch_object_test( group, self.client, self.admin, self.group_url(group), {'extended_due_date': None}) + def test_admin_update_soft_extended_due_date(self): + group = obj_build.make_group( + project=self.project, hard_extended_due_date=self.new_due_date) + valid_soft_extended_due_date = self.new_due_date - datetime.timedelta(days=1) + self.do_patch_object_test( + group, self.client, self.admin, self.group_url(group), + {'soft_extended_due_date': valid_soft_extended_due_date}, + expected_response_overrides={ + 'soft_extended_due_date': valid_soft_extended_due_date.replace( + second=0, microsecond=0), + # extended_due_date is deprecated, should reflect changes to soft_extended_deadline + 'extended_due_date': valid_soft_extended_due_date.replace( + second=0, microsecond=0) + }) + + group.refresh_from_db() + + invalid_soft_extended_due_date = "not a date" + self.do_patch_object_invalid_args_test( + group, self.client, self.admin, self.group_url(group), + {'soft_extended_due_date': invalid_soft_extended_due_date}) + + # soft_extended_due_date can't be later than hard_extended_due_date + invalid_soft_extended_due_date = self.new_due_date + datetime.timedelta(days=1) + self.do_patch_object_invalid_args_test( + group, self.client, self.admin, self.group_url(group), + {'soft_extended_due_date': invalid_soft_extended_due_date}) + + def test_admin_update_hard_extended_due_date(self): + group = obj_build.make_group( + project=self.project, soft_extended_due_date=self.new_due_date) + valid_hard_extended_due_date = self.new_due_date + datetime.timedelta(days=1) + self.do_patch_object_test( + group, self.client, self.admin, self.group_url(group), + {'hard_extended_due_date': valid_hard_extended_due_date}, + expected_response_overrides={ + 'hard_extended_due_date': valid_hard_extended_due_date.replace( + second=0, microsecond=0), + # extended_due_date is deprecated but should reflect soft_extended_deadline + 'extended_due_date': self.new_due_date.replace( + second=0, microsecond=0) + }) + + group.refresh_from_db() + + invalid_hard_extended_due_date = "not a date" + self.do_patch_object_invalid_args_test( + group, self.client, self.admin, self.group_url(group), + {'hard_extended_due_date': invalid_hard_extended_due_date}) + + # hard_extended_due_date can't be before soft_extended_due_date + invalid_hard_extended_due_date = self.new_due_date - datetime.timedelta(days=1) + self.do_patch_object_invalid_args_test( + group, self.client, self.admin, self.group_url(group), + {'hard_extended_due_date': invalid_hard_extended_due_date}) + def test_admin_update_group_invalid_members(self): group = obj_build.make_group(project=self.project) new_members = self.get_names(list(group.members.all())[:-1]) + ['stove'] @@ -583,7 +645,9 @@ def test_one_group_with_extension(self): self.group1.validate_and_update(extended_due_date=extension_date) response = self.client.post(self.get_merge_url(self.group1, self.group2)) self.assertEqual(status.HTTP_201_CREATED, response.status_code) - self.assertEqual(response.data['extended_due_date'], extension_date) + self.assertEqual( + response.data['extended_due_date'], + extension_date.replace(second=0, microsecond=0)) def test_both_have_extension(self): earlier_extension_date = timezone.now() @@ -593,7 +657,8 @@ def test_both_have_extension(self): self.client.force_authenticate(self.admin) response = self.client.post(self.get_merge_url(self.group1, self.group2)) self.assertEqual(status.HTTP_201_CREATED, response.status_code) - self.assertEqual(response.data['extended_due_date'], later_extension_date) + self.assertEqual(response.data['extended_due_date'], + later_extension_date.replace(second=0, microsecond=0)) def test_bonus_submission_merging(self) -> None: fewer_bonus_submissions = 1 diff --git a/autograder/rest_api/tests/test_views/test_project_views/test_project_views.py b/autograder/rest_api/tests/test_views/test_project_views/test_project_views.py index 9fc0d46d..249c6e3d 100644 --- a/autograder/rest_api/tests/test_views/test_project_views/test_project_views.py +++ b/autograder/rest_api/tests/test_views/test_project_views/test_project_views.py @@ -233,7 +233,7 @@ def test_admin_edit_project(self): admin = obj_build.make_admin_user(self.course) self.do_patch_object_test(self.project, self.client, admin, self.url, request_data) - def test_edit_project_invalid_settings(self): + def test_edit_project_invalid_group_size(self): request_data = { 'min_group_size': 2, 'max_group_size': 1 @@ -243,6 +243,15 @@ def test_edit_project_invalid_settings(self): self.do_patch_object_invalid_args_test( self.project, self.client, admin, self.url, request_data) + def test_edit_project_invalid_closing_time(self): + request_data = { + 'closing_time': 'not a date :(' + } + + admin = obj_build.make_admin_user(self.course) + self.do_patch_object_invalid_args_test( + self.project, self.client, admin, self.url, request_data) + def test_non_admin_edit_project_permission_denied(self): staff = obj_build.make_staff_user(self.course) self.do_patch_object_permission_denied_test( diff --git a/autograder/rest_api/views/group_views.py b/autograder/rest_api/views/group_views.py index 85991d94..cc5ff965 100644 --- a/autograder/rest_api/views/group_views.py +++ b/autograder/rest_api/views/group_views.py @@ -207,6 +207,7 @@ def patch(self, request, *args, **kwargs): group = self.get_object() update_data = dict(request.data) + print(update_data) if 'member_names' in update_data: users = [ User.objects.get_or_create( From 0cc31c24d2e810262ac0f8ecb327d4a01946780c Mon Sep 17 00:00:00 2001 From: Matt Mayfield Date: Mon, 26 Aug 2024 21:01:46 -0400 Subject: [PATCH 3/7] Remove hard_extended_due_date from views for non-admins --- .../test_group_views/test_group_views.py | 42 +++++++++++++++---- autograder/rest_api/views/group_views.py | 12 +++++- 2 files changed, 43 insertions(+), 11 deletions(-) diff --git a/autograder/rest_api/tests/test_views/test_group_views/test_group_views.py b/autograder/rest_api/tests/test_views/test_group_views/test_group_views.py index 42e38c65..fa520eeb 100644 --- a/autograder/rest_api/tests/test_views/test_group_views/test_group_views.py +++ b/autograder/rest_api/tests/test_views/test_group_views/test_group_views.py @@ -1,8 +1,8 @@ import datetime -from typing import List +from typing import List, Dict from unittest import mock -from django.contrib.auth.models import User +from django.contrib.auth.models import User, Group from django.core.files.uploadedfile import SimpleUploadedFile from django.urls import reverse from django.utils import timezone @@ -66,8 +66,12 @@ def test_admin_list_groups(self): def test_staff_list_groups(self): staff = obj_build.make_staff_user(self.project.course) + groups = self.build_groups(self.project) + for group in groups: + group.pop('hard_extended_due_date', None) + self.do_list_objects_test( - self.client, staff, self.url, self.build_groups(self.project), check_order=True) + self.client, staff, self.url, groups, check_order=True) def test_student_list_groups_permission_denied(self): self.project.validate_and_update(visible_to_students=True) @@ -77,8 +81,12 @@ def test_student_list_groups_permission_denied(self): def test_handgrader_list_groups(self): handgrader = obj_build.make_handgrader_user(self.course) + groups = self.build_groups(self.project) + for group in groups: + group.pop('hard_extended_due_date', None) + self.do_list_objects_test( - self.client, handgrader, self.url, self.build_groups(self.project)) + self.client, handgrader, self.url, groups) def test_guest_list_groups_permission_denied(self): self.project.validate_and_update(visible_to_students=True, guests_can_submit=True) @@ -315,32 +323,48 @@ def test_admin_or_staff_get_group(self): admin = obj_build.make_admin_user(self.course) staff = obj_build.make_staff_user(self.course) + def expected_data(group): + data = group.to_dict() + if user is staff: + data.pop("hard_extended_due_date", None) + return data + for user in admin, staff: student_group = obj_build.make_group(project=self.project) self.do_get_object_test( - self.client, user, self.group_url(student_group), student_group.to_dict()) + self.client, user, self.group_url(student_group), expected_data(student_group)) guest_group = obj_build.make_group( project=self.project, members_role=obj_build.UserRole.guest) self.do_get_object_test( - self.client, user, self.group_url(guest_group), guest_group.to_dict()) + self.client, user, self.group_url(guest_group), expected_data(guest_group)) admin_group = obj_build.make_group( project=self.project, members_role=obj_build.UserRole.admin) self.do_get_object_test( - self.client, user, self.group_url(admin_group), admin_group.to_dict()) + self.client, user, self.group_url(admin_group), expected_data(admin_group)) def test_student_get_group(self): self.project.validate_and_update(visible_to_students=True) group = obj_build.make_group(project=self.project) + + # students shouldn't see hard extended due date + expected_data = group.to_dict() + expected_data.pop("hard_extended_due_date", None) + self.do_get_object_test( - self.client, group.members.first(), self.group_url(group), group.to_dict()) + self.client, group.members.first(), self.group_url(group), expected_data) def test_guest_get_group(self): self.project.validate_and_update(visible_to_students=True, guests_can_submit=True) group = obj_build.make_group(project=self.project, members_role=obj_build.UserRole.guest) + + # guests shouldn't see hard extended due date + expected_data = group.to_dict() + expected_data.pop("hard_extended_due_date", None) + self.do_get_object_test( - self.client, group.members.first(), self.group_url(group), group.to_dict()) + self.client, group.members.first(), self.group_url(group), expected_data) def test_non_member_get_group_permission_denied(self): self.project.validate_and_update(visible_to_students=True) diff --git a/autograder/rest_api/views/group_views.py b/autograder/rest_api/views/group_views.py index cc5ff965..d1b62341 100644 --- a/autograder/rest_api/views/group_views.py +++ b/autograder/rest_api/views/group_views.py @@ -45,11 +45,19 @@ } +class SerializeGroupMixin: + def serialize_object(self, obj: ag_models.Group) -> dict: + result = super().serialize_object(obj) + if not obj.project.course.is_admin(self.request.user): + result.pop('hard_extended_due_date', None) + return result + + class _ListCreateGroupSchema(AGListViewSchemaMixin, CustomViewSchema): pass -class ListCreateGroupsView(NestedModelView): +class ListCreateGroupsView(SerializeGroupMixin, NestedModelView): schema = _ListCreateGroupSchema([APITags.groups], api_class=ag_models.Group, data={ 'POST': { 'operation_id': 'createGroup', @@ -188,7 +196,7 @@ class _GroupDetailSchema(AGRetrieveViewSchemaMixin, AGPatchViewSchemaMixin, Cust pass -class GroupDetailView(AGModelDetailView): +class GroupDetailView(SerializeGroupMixin, AGModelDetailView): schema = _GroupDetailSchema([APITags.groups], { 'DELETE': {'operation_id': 'pseudoDeleteGroup'} }) From d547de650c80bc147c3e5aff63a130cee9941347 Mon Sep 17 00:00:00 2001 From: Matt Mayfield Date: Tue, 27 Aug 2024 10:21:38 -0400 Subject: [PATCH 4/7] Add date validation tests for project model and remove debug print --- .../test_models/test_project/test_project.py | 18 ++++++++++++++++++ autograder/rest_api/views/group_views.py | 1 - 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/autograder/core/tests/test_models/test_project/test_project.py b/autograder/core/tests/test_models/test_project/test_project.py index 57841d1d..c6b68d57 100644 --- a/autograder/core/tests/test_models/test_project/test_project.py +++ b/autograder/core/tests/test_models/test_project/test_project.py @@ -223,6 +223,24 @@ def test_error_soft_closing_time_after_closing_time(self): self.assertIn('soft_closing_time', cm.exception.message_dict) + def test_error_soft_closing_time_not_a_date(self): + soft_closing_time = "foobar" + with self.assertRaises(exceptions.ValidationError) as cm: + ag_models.Project.objects.validate_and_create( + name='stove', course=self.course, + soft_closing_time=soft_closing_time) + + self.assertIn('soft_closing_time', cm.exception.message_dict) + + def test_error_closing_time_not_a_date(self): + closing_time = "foobar" + with self.assertRaises(exceptions.ValidationError) as cm: + ag_models.Project.objects.validate_and_create( + name='stove', course=self.course, + closing_time=closing_time) + + self.assertIn('closing_time', cm.exception.message_dict) + class ProjectMiscErrorTestCase(UnitTestBase): def setUp(self): diff --git a/autograder/rest_api/views/group_views.py b/autograder/rest_api/views/group_views.py index d1b62341..8629c018 100644 --- a/autograder/rest_api/views/group_views.py +++ b/autograder/rest_api/views/group_views.py @@ -215,7 +215,6 @@ def patch(self, request, *args, **kwargs): group = self.get_object() update_data = dict(request.data) - print(update_data) if 'member_names' in update_data: users = [ User.objects.get_or_create( From 3961e970ca4f10a4053d47f9d47263047d5d1f2d Mon Sep 17 00:00:00 2001 From: Matt Mayfield Date: Tue, 27 Aug 2024 18:21:36 -0400 Subject: [PATCH 5/7] Fix field name in ValidationError msg --- autograder/core/models/group/group.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/autograder/core/models/group/group.py b/autograder/core/models/group/group.py index 06f8c786..5a1caebe 100644 --- a/autograder/core/models/group/group.py +++ b/autograder/core/models/group/group.py @@ -209,7 +209,7 @@ def clean(self) -> None: 'Soft extended due date must be a valid date')}) except core_ut.InvalidHardDeadlineError: raise ValidationError( - {'extended_due_date': ( + {'hard_extended_due_date': ( 'Hard extended due date must be a valid date')}) except core_ut.HardDeadlineBeforeSoftDeadlineError: raise ValidationError( From 05abe1d620631b2a5b12b00b8068c16e5cb0b720 Mon Sep 17 00:00:00 2001 From: Matt Mayfield Date: Tue, 27 Aug 2024 19:37:41 -0400 Subject: [PATCH 6/7] Fix broken tests and refactor tests a bit --- autograder/core/models/group/group.py | 4 +- autograder/core/models/project/project.py | 4 +- .../test_models/test_project/test_project.py | 3 +- .../tests/test_views/ag_view_test_base.py | 7 +- .../test_group_views/test_group_views.py | 66 ++++++++++--------- .../test_project_views/test_project_views.py | 3 +- 6 files changed, 42 insertions(+), 45 deletions(-) diff --git a/autograder/core/models/group/group.py b/autograder/core/models/group/group.py index 5a1caebe..77a175d8 100644 --- a/autograder/core/models/group/group.py +++ b/autograder/core/models/group/group.py @@ -213,8 +213,8 @@ def clean(self) -> None: 'Hard extended due date must be a valid date')}) except core_ut.HardDeadlineBeforeSoftDeadlineError: raise ValidationError( - {'soft_extended_due_date': ( - 'Soft extended due date must not be after hard extended due date')}) + {'hard_extended_due_date': ( + 'Hard extended due date must not be before soft extended due date')}) self.soft_extended_due_date = clean_soft self.hard_extended_due_date = clean_hard diff --git a/autograder/core/models/project/project.py b/autograder/core/models/project/project.py index 4624616d..6cc13b98 100644 --- a/autograder/core/models/project/project.py +++ b/autograder/core/models/project/project.py @@ -251,8 +251,8 @@ def clean(self) -> None: 'Closing time must be a valid date')}) except core_ut.HardDeadlineBeforeSoftDeadlineError: raise exceptions.ValidationError( - {'soft_closing_time': ( - 'Soft closing time must not be after hard closing time')}) + {'closing_time': ( + 'Closing time must not be before soft closing time')}) self.soft_closing_time = clean_soft self.closing_time = clean_hard diff --git a/autograder/core/tests/test_models/test_project/test_project.py b/autograder/core/tests/test_models/test_project/test_project.py index c6b68d57..ffccbcaa 100644 --- a/autograder/core/tests/test_models/test_project/test_project.py +++ b/autograder/core/tests/test_models/test_project/test_project.py @@ -220,8 +220,7 @@ def test_error_soft_closing_time_after_closing_time(self): name='stove', course=self.course, closing_time=closing_time, soft_closing_time=soft_closing_time) - - self.assertIn('soft_closing_time', cm.exception.message_dict) + self.assertIn('closing_time', cm.exception.message_dict) def test_error_soft_closing_time_not_a_date(self): soft_closing_time = "foobar" diff --git a/autograder/rest_api/tests/test_views/ag_view_test_base.py b/autograder/rest_api/tests/test_views/ag_view_test_base.py index e1aa0044..f8549afd 100644 --- a/autograder/rest_api/tests/test_views/ag_view_test_base.py +++ b/autograder/rest_api/tests/test_views/ag_view_test_base.py @@ -92,9 +92,7 @@ def do_patch_object_test(self, ag_model_obj, client, user, url, ignore_fields = list(ignore_fields) ignore_fields.append('last_modified') - expected_data = ag_model_obj.to_dict() - for field in ignore_fields: - expected_data.pop(field, None) + expected_data = utils.exclude_dict(ag_model_obj.to_dict(), ignore_fields) for key, value in request_data.items(): if isinstance(value, dict): expected_data[key].update(value) @@ -171,9 +169,6 @@ def _do_bad_update_test(self, ag_model_obj, client, user, url, request_data, self.assertEqual(expected_status, response.status_code) ag_model_obj = ag_model_obj._meta.model.objects.get(pk=ag_model_obj.pk) - - print(f"{expected_data=}") - print(f"{ag_model_obj.to_dict()=}") self.assert_dict_contents_equal(expected_data, ag_model_obj.to_dict()) return response diff --git a/autograder/rest_api/tests/test_views/test_group_views/test_group_views.py b/autograder/rest_api/tests/test_views/test_group_views/test_group_views.py index fa520eeb..f40fb8f2 100644 --- a/autograder/rest_api/tests/test_views/test_group_views/test_group_views.py +++ b/autograder/rest_api/tests/test_views/test_group_views/test_group_views.py @@ -483,6 +483,12 @@ def test_admin_update_guest_group_members(self): def test_admin_update_group_deprecated_extended_due_date(self): group = obj_build.make_group(project=self.project) + invalid_extended_due_date = "not a date" + response = self.do_patch_object_invalid_args_test( + group, self.client, self.admin, self.group_url(group), + {'extended_due_date': invalid_extended_due_date}) + self.assertIn('soft_extended_due_date', response.data) + self.do_patch_object_test( group, self.client, self.admin, self.group_url(group), {'extended_due_date': self.new_due_date}, @@ -498,6 +504,20 @@ def test_admin_update_group_deprecated_extended_due_date(self): def test_admin_update_soft_extended_due_date(self): group = obj_build.make_group( project=self.project, hard_extended_due_date=self.new_due_date) + + invalid_soft_extended_due_date = "not a date" + response = self.do_patch_object_invalid_args_test( + group, self.client, self.admin, self.group_url(group), + {'soft_extended_due_date': invalid_soft_extended_due_date}) + self.assertIn('soft_extended_due_date', response.data) + + # soft_extended_due_date can't be later than hard_extended_due_date + invalid_soft_extended_due_date = self.new_due_date + datetime.timedelta(days=1) + response = self.do_patch_object_invalid_args_test( + group, self.client, self.admin, self.group_url(group), + {'soft_extended_due_date': invalid_soft_extended_due_date}) + self.assertIn('hard_extended_due_date', response.data) + valid_soft_extended_due_date = self.new_due_date - datetime.timedelta(days=1) self.do_patch_object_test( group, self.client, self.admin, self.group_url(group), @@ -505,27 +525,29 @@ def test_admin_update_soft_extended_due_date(self): expected_response_overrides={ 'soft_extended_due_date': valid_soft_extended_due_date.replace( second=0, microsecond=0), - # extended_due_date is deprecated, should reflect changes to soft_extended_deadline + # setting soft_extended_due_date will also set extended_due_date + # for backwards compatibility 'extended_due_date': valid_soft_extended_due_date.replace( second=0, microsecond=0) }) - group.refresh_from_db() + def test_admin_update_hard_extended_due_date(self): + group = obj_build.make_group( + project=self.project, soft_extended_due_date=self.new_due_date) - invalid_soft_extended_due_date = "not a date" - self.do_patch_object_invalid_args_test( + invalid_hard_extended_due_date = "not a date" + response = self.do_patch_object_invalid_args_test( group, self.client, self.admin, self.group_url(group), - {'soft_extended_due_date': invalid_soft_extended_due_date}) + {'hard_extended_due_date': invalid_hard_extended_due_date}) + self.assertIn('hard_extended_due_date', response.data) - # soft_extended_due_date can't be later than hard_extended_due_date - invalid_soft_extended_due_date = self.new_due_date + datetime.timedelta(days=1) - self.do_patch_object_invalid_args_test( + # hard_extended_due_date can't be before soft_extended_due_date + invalid_hard_extended_due_date = self.new_due_date - datetime.timedelta(days=1) + response = self.do_patch_object_invalid_args_test( group, self.client, self.admin, self.group_url(group), - {'soft_extended_due_date': invalid_soft_extended_due_date}) + {'hard_extended_due_date': invalid_hard_extended_due_date}) + self.assertIn('hard_extended_due_date', response.data) - def test_admin_update_hard_extended_due_date(self): - group = obj_build.make_group( - project=self.project, soft_extended_due_date=self.new_due_date) valid_hard_extended_due_date = self.new_due_date + datetime.timedelta(days=1) self.do_patch_object_test( group, self.client, self.admin, self.group_url(group), @@ -538,19 +560,6 @@ def test_admin_update_hard_extended_due_date(self): second=0, microsecond=0) }) - group.refresh_from_db() - - invalid_hard_extended_due_date = "not a date" - self.do_patch_object_invalid_args_test( - group, self.client, self.admin, self.group_url(group), - {'hard_extended_due_date': invalid_hard_extended_due_date}) - - # hard_extended_due_date can't be before soft_extended_due_date - invalid_hard_extended_due_date = self.new_due_date - datetime.timedelta(days=1) - self.do_patch_object_invalid_args_test( - group, self.client, self.admin, self.group_url(group), - {'hard_extended_due_date': invalid_hard_extended_due_date}) - def test_admin_update_group_invalid_members(self): group = obj_build.make_group(project=self.project) new_members = self.get_names(list(group.members.all())[:-1]) + ['stove'] @@ -575,13 +584,6 @@ def test_admin_update_group_error_non_allowed_domain_guest(self): {'member_names': [allowed_guest.username, non_allowed_guest.username]}) self.assertIn('members', response.data) - def test_admin_update_group_bad_date(self): - group = obj_build.make_group(project=self.project) - response = self.do_patch_object_invalid_args_test( - group, self.client, self.admin, self.group_url(group), - {'extended_due_date': 'not a date'}) - self.assertIn('extended_due_date', response.data) - def test_non_admin_update_group_permission_denied(self): group = obj_build.make_group(project=self.project) staff = obj_build.make_staff_user(self.course) diff --git a/autograder/rest_api/tests/test_views/test_project_views/test_project_views.py b/autograder/rest_api/tests/test_views/test_project_views/test_project_views.py index 249c6e3d..cf23c574 100644 --- a/autograder/rest_api/tests/test_views/test_project_views/test_project_views.py +++ b/autograder/rest_api/tests/test_views/test_project_views/test_project_views.py @@ -249,8 +249,9 @@ def test_edit_project_invalid_closing_time(self): } admin = obj_build.make_admin_user(self.course) - self.do_patch_object_invalid_args_test( + response = self.do_patch_object_invalid_args_test( self.project, self.client, admin, self.url, request_data) + self.assertIn('closing_time', response.data) def test_non_admin_edit_project_permission_denied(self): staff = obj_build.make_staff_user(self.course) From d551ed64ea34989dfe7cf76917b2469671c3fb2c Mon Sep 17 00:00:00 2001 From: Matt Mayfield Date: Tue, 3 Sep 2024 09:18:19 -0400 Subject: [PATCH 7/7] Add requested changes - add docstring to extended_deadline - add exception message check to test_error_soft_extended_due_date_after_extended_due_date - rename test for consistency --- autograder/core/models/group/group.py | 10 ++++++++++ .../tests/test_models/test_group/test_group.py | 5 +++-- autograder/rest_api/schema/schema.yml | 14 ++++++++++++-- 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/autograder/core/models/group/group.py b/autograder/core/models/group/group.py index 77a175d8..a7d4d39d 100644 --- a/autograder/core/models/group/group.py +++ b/autograder/core/models/group/group.py @@ -96,6 +96,16 @@ def member_names(self) -> List[str]: @property def extended_due_date(self) -> Optional[datetime]: + """ + DEPRECATED. For API legacy use, you can continue to use this field as usual. + Do NOT mix usage of this field and the two new "hard_extended_due_date" and + "soft_extended_due_date" fields. + + When this field is set, it indicates that members of this submission group can + submit until this specified date, overriding the project closing time. + + Default value: None + """ return self.soft_extended_due_date @extended_due_date.setter diff --git a/autograder/core/tests/test_models/test_group/test_group.py b/autograder/core/tests/test_models/test_group/test_group.py index 925f7f6a..ca440e31 100644 --- a/autograder/core/tests/test_models/test_group/test_group.py +++ b/autograder/core/tests/test_models/test_group/test_group.py @@ -691,13 +691,14 @@ def test_valid_soft_extended_due_date_None_hard_extended_due_date_not_None(self) group.hard_extended_due_date) self.assertIsNone(group.soft_extended_due_date) - def test_error_soft_extended_due_date_after_extended_due_date(self): + def test_error_soft_extended_due_date_after_hard_extended_due_date(self): hard_extended_due_date = timezone.now() + timezone.timedelta(minutes=5) soft_extended_due_date = hard_extended_due_date + timezone.timedelta(minutes=5) - with self.assertRaises(exceptions.ValidationError): + with self.assertRaises(exceptions.ValidationError) as cm: ag_models.Group.objects.validate_and_create( members=self.student_users, project=self.project, hard_extended_due_date=hard_extended_due_date, soft_extended_due_date=soft_extended_due_date) + self.assertIn('hard_extended_due_date', cm.exception.message_dict) diff --git a/autograder/rest_api/schema/schema.yml b/autograder/rest_api/schema/schema.yml index b1b94092..2b15f869 100644 --- a/autograder/rest_api/schema/schema.yml +++ b/autograder/rest_api/schema/schema.yml @@ -5560,7 +5560,12 @@ components: type: string format: date-time extended_due_date: - description: '' + description: "DEPRECATED. For API legacy use, you can continue to use this\ + \ field as usual.\n Do NOT mix usage of this field and the two\ + \ new \"hard_extended_due_date\" and\n \"soft_extended_due_date\"\ + \ fields.\n\n When this field is set, it indicates that members\ + \ of this submission group can\n submit until this specified date,\ + \ overriding the project closing time.\n\n Default value: None" nullable: true type: unknown member_names: @@ -5642,7 +5647,12 @@ components: items: type: string extended_due_date: - description: '' + description: "DEPRECATED. For API legacy use, you can continue to use this\ + \ field as usual.\n Do NOT mix usage of this field and the two\ + \ new \"hard_extended_due_date\" and\n \"soft_extended_due_date\"\ + \ fields.\n\n When this field is set, it indicates that members\ + \ of this submission group can\n submit until this specified date,\ + \ overriding the project closing time.\n\n Default value: None" nullable: true type: unknown hard_extended_due_date: