From b73b43d1557137f9e9f1db40a4d330e942a2fb25 Mon Sep 17 00:00:00 2001 From: Ross Date: Mon, 5 Aug 2024 13:20:29 +0100 Subject: [PATCH] tidy up the marker changes --- anatomy/migrations/0020_alter_exam_markers.py | 20 ++++++ anatomy/models.py | 1 + anatomy/views.py | 3 + .../0059_alter_casecollection_markers.py | 20 ++++++ atlas/models.py | 1 + generic/forms.py | 2 + .../generic/exam_overview_headers.html | 57 ++++++++++------ generic/views.py | 54 ++++++++++----- longs/migrations/0027_alter_exam_markers.py | 20 ++++++ .../migrations/0028_alter_exam_double_mark.py | 18 +++++ longs/models.py | 3 +- longs/templates/longs/exam_scores.html | 5 -- .../longs/mark_question_double_overview.html | 2 +- .../longs/mark_question_single_overview.html | 4 +- longs/tests/test_longs_exams.py | 65 +++++++++++++++---- longs/views.py | 13 ++-- physics/migrations/0013_alter_exam_markers.py | 20 ++++++ physics/models.py | 1 + rad/tests/test_helpers.py | 4 ++ rapids/migrations/0012_alter_exam_markers.py | 20 ++++++ rapids/models.py | 1 + rapids/views.py | 3 + sbas/migrations/0013_alter_exam_markers.py | 20 ++++++ sbas/models.py | 1 + 24 files changed, 300 insertions(+), 58 deletions(-) create mode 100644 anatomy/migrations/0020_alter_exam_markers.py create mode 100644 atlas/migrations/0059_alter_casecollection_markers.py create mode 100644 longs/migrations/0027_alter_exam_markers.py create mode 100644 longs/migrations/0028_alter_exam_double_mark.py create mode 100644 physics/migrations/0013_alter_exam_markers.py create mode 100644 rapids/migrations/0012_alter_exam_markers.py create mode 100644 sbas/migrations/0013_alter_exam_markers.py diff --git a/anatomy/migrations/0020_alter_exam_markers.py b/anatomy/migrations/0020_alter_exam_markers.py new file mode 100644 index 00000000..57368f6c --- /dev/null +++ b/anatomy/migrations/0020_alter_exam_markers.py @@ -0,0 +1,20 @@ +# Generated by Django 5.0.2 on 2024-08-05 10:09 + +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('anatomy', '0019_exam_markers'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.AlterField( + model_name='exam', + name='markers', + field=models.ManyToManyField(blank=True, help_text='Authorised markers for the exam', null=True, related_name='anatomy_exam_markers', to=settings.AUTH_USER_MODEL), + ), + ] diff --git a/anatomy/models.py b/anatomy/models.py index aff59a3d..b194cff7 100644 --- a/anatomy/models.py +++ b/anatomy/models.py @@ -390,6 +390,7 @@ class Exam(ExamBase): markers = models.ManyToManyField( settings.AUTH_USER_MODEL, blank=True, + null=True, help_text="Authorised markers for the exam", related_name="anatomy_exam_markers", ) diff --git a/anatomy/views.py b/anatomy/views.py index d02330a2..a4171f2d 100644 --- a/anatomy/views.py +++ b/anatomy/views.py @@ -240,6 +240,9 @@ def mark_review(request, exam_pk, sk): def mark(request, exam_pk, sk, unmarked_exam_answers_only=True, review=False): exam = get_object_or_404(Exam, pk=exam_pk) + if not GenericExamViews.check_user_marker_access(request.user, exam_pk): + raise PermissionDenied + if not exam.exam_mode: raise Http404("Packet not in exam mode") diff --git a/atlas/migrations/0059_alter_casecollection_markers.py b/atlas/migrations/0059_alter_casecollection_markers.py new file mode 100644 index 00000000..e21dca6c --- /dev/null +++ b/atlas/migrations/0059_alter_casecollection_markers.py @@ -0,0 +1,20 @@ +# Generated by Django 5.0.2 on 2024-08-05 10:09 + +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('atlas', '0058_casecollection_markers'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.AlterField( + model_name='casecollection', + name='markers', + field=models.ManyToManyField(blank=True, help_text='Authorised markers for the exam', null=True, related_name='casecollection_markers', to=settings.AUTH_USER_MODEL), + ), + ] diff --git a/atlas/models.py b/atlas/models.py index 2163a9b2..042bc416 100644 --- a/atlas/models.py +++ b/atlas/models.py @@ -783,6 +783,7 @@ class CaseCollection(ExamOrCollectionGenericBase): markers = models.ManyToManyField( settings.AUTH_USER_MODEL, blank=True, + null=True, help_text="Authorised markers for the exam", related_name="casecollection_markers", ) diff --git a/generic/forms.py b/generic/forms.py index 5e48aef7..33266a98 100755 --- a/generic/forms.py +++ b/generic/forms.py @@ -195,6 +195,8 @@ class ExamMarkerFormMixin(ModelForm): queryset=User.objects.all(), widget=FilteredSelectMultiple(verbose_name="Markers", is_stacked=False), ) + self.fields["markers"].required = False + class ExaminationForm(ModelForm): diff --git a/generic/templates/generic/exam_overview_headers.html b/generic/templates/generic/exam_overview_headers.html index 296fff83..c3f864a4 100644 --- a/generic/templates/generic/exam_overview_headers.html +++ b/generic/templates/generic/exam_overview_headers.html @@ -27,8 +27,8 @@ {% endif %} -This exam has {{question_number}} questions. -Time limit: {{exam.time_limit}} seconds [{{exam.get_time_limit}}] +This exam has {{question_number}} questions. +Time limit: {{exam.time_limit}} seconds [{{exam.get_time_limit}}] . Exam mode: {{ exam.exam_mode }}
@@ -37,22 +37,33 @@ Exam mode: {{ exam.exam_mode }}
Cid candidates: {{candidate_count.0}}, User candidates: {{candidate_count.1}}
{% endif %} -
- Answer management +Markers: +{% for marker in exam.markers.all %} + {{marker}}{% if not forloop.last %},{% endif %} +{% empty %} + No additional markers. +{% endfor %} +
-
-

Manage answers for this collection.

- Please note these are permanant and cannot be undone. -
+{% if can_edit %} +
+ Answer management - - -
+
+

Manage answers for this collection.

+ + Please note these are permanant and cannot be undone. +
+ + + +
+{% endif %} Open access: {{ exam.open_access }}
@@ -60,16 +71,24 @@ Open access: {{ exam.open_access }}
Start date: {{ exam.start_date }} {% endif %} {% if exam.end_date %} - / End date: {{ exam.end_date }} + / End date: {{ exam.end_date }} {% endif %} - +
- Exam active: [When checked the exam will be available to take in the test system] + Exam active: [When checked the exam will be available to take in the test system]
{% if exam.exam_mode %}
- Publish results: [When checked the exam results will be available to users on this site]
{% if exam.get_app_name == "anatomy" or exam.get_app_name == "rapids" or exam.get_app_name == "longs" %} diff --git a/generic/views.py b/generic/views.py index 979a766b..43240e8c 100644 --- a/generic/views.py +++ b/generic/views.py @@ -427,11 +427,14 @@ class ExamViews(View, LoginRequiredMixin): self.ExtraExamFilter = ExtraExamFilter # TODO: these may be better implemented as decorators - def check_user_access(self, user: User, exam_id: int = None): + def check_user_access(self, user: User, exam_id: int = None, marker=False): """Check if a user should be able to access a view Args: user ([user]): user object + exam_id ([int], optional): exam id. Defaults to None. + + marker ([boolean], optional): Allow access if the user is a marker. Defaults to False. Requires that exam_id is set. Returns: [boolean]: True if the user has access @@ -440,11 +443,18 @@ class ExamViews(View, LoginRequiredMixin): if user.is_superuser: return True + if marker and exam_id is None: + # raise error if exam_id is not provided + raise ValueError("exam_id must be provided if marker checking for a marker") + if exam_id is not None: - exam = get_object_or_404(self.Exam, pk=exam_id) + exam: ExamBase = get_object_or_404(self.Exam, pk=exam_id) if (exam.open_access and exam.active) or user in exam.get_author_objects(): return True + if marker and user in exam.markers.all(): + return True + if exam.authors_only: return False @@ -487,6 +497,9 @@ class ExamViews(View, LoginRequiredMixin): return False return True + def check_user_marker_access(self, user: User, exam_id: int = None): + return self.check_user_access(user, exam_id, marker=True) + def check_user_edit_access(self, user, exam_id=None): """Check if a user should be able to access a view @@ -541,7 +554,7 @@ class ExamViews(View, LoginRequiredMixin): and not user.groups.filter(name="casecollection_checker").exists() ): return False - return True + return False @method_decorator(login_required) def exam_toggle_archived(self, request, pk): @@ -830,7 +843,8 @@ class ExamViews(View, LoginRequiredMixin): exam = get_object_or_404(self.Exam.objects.prefetch_related("author"), pk=pk) if request.user not in exam.author.all(): - if not self.check_user_access(request.user, pk): + # We also let markers view the exam + if not self.check_user_access(request.user, pk, marker=True): raise PermissionDenied can_edit = ( @@ -866,15 +880,22 @@ class ExamViews(View, LoginRequiredMixin): ) elif self.app_name == "longs": questions = ( - exam.exam_questions.select_related() - .prefetch_related( - Prefetch( - "series", - queryset=LongSeries.objects.select_related( - "modality", "examination", "plane", "contrast" - ).prefetch_related(Prefetch("images")), - ), - "author" + exam.exam_questions.prefetch_related( + #Prefetch( + # "series", + # queryset=LongSeries.objects.select_related( + # "modality", "examination", "plane", "contrast" + # ).prefetch_related(Prefetch("images")), + #), + "author", + #"examquestiondetail", + #"longsseries", + "series", + #"seriesdetail", + "series__images", + "series__plane", + "series__examination" + # to_attr="prefetched_primary_answer" # queryset=self.Answer.objects.filter(), # "series", @@ -1311,7 +1332,7 @@ class ExamViews(View, LoginRequiredMixin): raise Http404("Packet not in exam mode") if request.user not in exam.author.all(): - if not self.check_user_access(request.user, pk): + if not self.check_user_marker_access(request.user, pk): raise PermissionDenied if self.app_name == "anatomy": @@ -1443,7 +1464,7 @@ class ExamViews(View, LoginRequiredMixin): exam: ExamBase = get_object_or_404(self.Exam, pk=pk) if request.user not in exam.author.all(): - if not self.check_user_access(request.user, pk): + if not self.check_user_access(request.user, pk, marker=True): raise PermissionDenied question = exam.get_questions()[sk] @@ -2329,6 +2350,9 @@ class ExamViews(View, LoginRequiredMixin): def exam_stats(self, request, exam_id): exam: ExamBase = get_object_or_404(self.Exam, pk=exam_id) + if not self.check_user_edit_access(request.user, exam_id): + raise PermissionDenied() + if not exam.exam_mode: raise Http404("Packet not in exam mode") diff --git a/longs/migrations/0027_alter_exam_markers.py b/longs/migrations/0027_alter_exam_markers.py new file mode 100644 index 00000000..4ce11c40 --- /dev/null +++ b/longs/migrations/0027_alter_exam_markers.py @@ -0,0 +1,20 @@ +# Generated by Django 5.0.2 on 2024-08-05 10:09 + +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('longs', '0026_exam_markers'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.AlterField( + model_name='exam', + name='markers', + field=models.ManyToManyField(blank=True, help_text='Authorised markers for the exam', null=True, related_name='long_exam_markers', to=settings.AUTH_USER_MODEL), + ), + ] diff --git a/longs/migrations/0028_alter_exam_double_mark.py b/longs/migrations/0028_alter_exam_double_mark.py new file mode 100644 index 00000000..354e88f9 --- /dev/null +++ b/longs/migrations/0028_alter_exam_double_mark.py @@ -0,0 +1,18 @@ +# Generated by Django 5.0.2 on 2024-08-05 11:39 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('longs', '0027_alter_exam_markers'), + ] + + operations = [ + migrations.AlterField( + model_name='exam', + name='double_mark', + field=models.BooleanField(default=False, help_text='Defines if an exam is expected to be double marked'), + ), + ] diff --git a/longs/models.py b/longs/models.py index 8aaf6e12..f0963dae 100644 --- a/longs/models.py +++ b/longs/models.py @@ -449,12 +449,13 @@ class Exam(ExamBase): markers = models.ManyToManyField( settings.AUTH_USER_MODEL, blank=True, + null=True, help_text="Authorised markers for the exam", related_name="long_exam_markers", ) double_mark = models.BooleanField( - default=True, help_text="Defines if an exam is expected to be double marked" + default=False, help_text="Defines if an exam is expected to be double marked" ) valid_cid_users = models.ManyToManyField( diff --git a/longs/templates/longs/exam_scores.html b/longs/templates/longs/exam_scores.html index 1372346c..3b6bb180 100644 --- a/longs/templates/longs/exam_scores.html +++ b/longs/templates/longs/exam_scores.html @@ -1,11 +1,6 @@ {% extends 'generic/exam_scores_base.html' %} -{% block navigation %} - {{ block.super }} - {% include 'generic/exam_link_headers.html' %} -{% endblock navigation %} - {% block table_answers %} {% for question in questions %} diff --git a/longs/templates/longs/mark_question_double_overview.html b/longs/templates/longs/mark_question_double_overview.html index 15126c1f..2aa6e2a7 100644 --- a/longs/templates/longs/mark_question_double_overview.html +++ b/longs/templates/longs/mark_question_double_overview.html @@ -1,7 +1,7 @@ {% extends 'longs/exams.html' %} {% block content %} - View Edit Admin diff --git a/longs/templates/longs/mark_question_single_overview.html b/longs/templates/longs/mark_question_single_overview.html index 0113c1e0..95fe6654 100644 --- a/longs/templates/longs/mark_question_single_overview.html +++ b/longs/templates/longs/mark_question_single_overview.html @@ -1,7 +1,7 @@ {% extends 'longs/exams.html' %} {% block content %} - View Edit Admin @@ -19,7 +19,7 @@