tidy up the marker changes

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),
+        ),
+    ]

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",
     )

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")
 

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),
+        ),
+    ]

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",
     )

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):

generic/templates/generic/exam_overview_headers.html

@@ -27,8 +27,8 @@
 {% endif %}
 
 
-This exam has {{question_number}} questions. 
+This exam has {{question_number}} questions.
-<span title="Time per question: {% widthratio exam.time_limit question_number 1 %} seconds">Time limit: {{exam.time_limit}} seconds [{{exam.get_time_limit}}] 
+<span title="Time per question: {% widthratio exam.time_limit question_number 1 %} seconds">Time limit: {{exam.time_limit}} seconds [{{exam.get_time_limit}}]
     <i class="bi bi-info-circle"></i>
 </span>.
 Exam mode: {{ exam.exam_mode }}<br />
@@ -37,22 +37,33 @@ Exam mode: {{ exam.exam_mode }}<br />
     Cid candidates: <a href="{{exam.get_cid_edit_url}}">{{candidate_count.0}}</a>, User candidates: <a href="{{exam.get_user_edit_url}}">{{candidate_count.1}}</a><br />
 {% endif %}
 
-<details>
+Markers:
-    <summary>Answer management</summary>
+{% for marker in exam.markers.all %}
+    {{marker}}{% if not forloop.last %},{% endif %}
+{% empty %}
+    <span title="Only exam authors will be able to mark.">No additional markers.</span>
+{% endfor %}
+<br/>
 
-    <div class="alert alert-danger">
-    <p>Manage answers for this collection. </p>
 
-    Please note these are permanant and cannot be undone.
+{% if can_edit %}
-    </div>
+    <details>
+        <summary>Answer management</summary>
 
-    <button title="This will clear all user answers and attempts"
+        <div class="alert alert-danger">
-    hx-post="{% url exam.get_app_name|add:':exam_reset_answers' exam.pk %}"
+            <p>Manage answers for this collection. </p>
-    hx-swap="outerHTML"
+
-    hx-confirm="Are you sure you want to reset all answers? This action cannot be undone."
+            Please note these are permanant and cannot be undone.
-    >Reset all answers</button>
+        </div>
-    
+
-</details>
+        <button title="This will clear all user answers and attempts"
+                hx-post="{% url exam.get_app_name|add:':exam_reset_answers' exam.pk %}"
+                hx-swap="outerHTML"
+                hx-confirm="Are you sure you want to reset all answers? This action cannot be undone."
+        >Reset all answers</button>
+
+    </details>
+{% endif %}
 
 Open access: {{ exam.open_access }}<br />
 
@@ -60,16 +71,24 @@ Open access: {{ exam.open_access }}<br />
     Start date: {{ exam.start_date }}
 {% endif %}
 {% if exam.end_date %}
-  / End date: {{ exam.end_date }}
+    / End date: {{ exam.end_date }}
 {% endif %}
-    
+
 
 <div class="parent-help" title="Click to enable / disable the exam">
-    Exam active: <input type="checkbox" id="exam-active-switch" {% if exam.active %}checked{% endif %} data-posturl="{% url exam.get_app_name|add:':exam_toggle_active' pk=exam.pk %}"> <span class="help-text">[When checked the exam will be available to take in the test system]</span>
+    Exam active: <input type="checkbox"
+                        {% if not can_edit %}
+                            disabled
+                        {% endif %}
+                        id="exam-active-switch" {% if exam.active %}checked{% endif %} data-posturl="{% url exam.get_app_name|add:':exam_toggle_active' pk=exam.pk %}"> <span class="help-text">[When checked the exam will be available to take in the test system]</span>
 </div>
 {% if exam.exam_mode %}
     <div class="parent-help" title="Click to enable / disable the exam results">
-        Publish results: <input type="checkbox" id="exam-publish-results-switch"  data-posturl="{% url exam.get_app_name|add:':exam_toggle_results_published' pk=exam.pk %}"
+        Publish results: <input type="checkbox"
+                                {% if not can_edit %}
+                                    disabled
+                                {% endif %}
+                                id="exam-publish-results-switch"  data-posturl="{% url exam.get_app_name|add:':exam_toggle_results_published' pk=exam.pk %}"
                                 {% if exam.publish_results %}checked{% endif %}>  <span class="help-text">[When checked the exam results will be available to users on this site]</span>
     </div>
     {% if exam.get_app_name == "anatomy" or exam.get_app_name == "rapids" or exam.get_app_name == "longs" %}

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()
+                exam.exam_questions.prefetch_related(
-                .prefetch_related(
+                    #Prefetch(
-                    Prefetch(
+                    #    "series",
-                        "series",
+                    #    queryset=LongSeries.objects.select_related(
-                        queryset=LongSeries.objects.select_related(
+                    #        "modality", "examination", "plane", "contrast"
-                            "modality", "examination", "plane", "contrast"
+                    #    ).prefetch_related(Prefetch("images")),
-                        ).prefetch_related(Prefetch("images")),
+                    #),
-                    ),
+                    "author",
-                    "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")
 

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),
+        ),
+    ]

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'),
+        ),
+    ]

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(

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 %}
                 <tr>

longs/templates/longs/mark_question_double_overview.html

@@ -1,7 +1,7 @@
 {% extends 'longs/exams.html' %}
 
 {% block content %}
-    <a href="{% url 'longs:exam_question_detail' exam.id question_details.current|add:'-1' %}"
+ Question -> <a href="{% url 'longs:exam_question_detail' exam.id question_details.current|add:'-1' %}"
         title="View the Question">View</a>
     <a href="{% url 'longs:long_update' question.id %}" title="Edit the Question">Edit</a> <a
         href="{% url 'admin:longs_long_change' question.id %}" title="Edit the Question using the admin interface">Admin

longs/templates/longs/mark_question_single_overview.html

@@ -1,7 +1,7 @@
 {% extends 'longs/exams.html' %}
 
 {% block content %}
-<a href="{% url 'longs:exam_question_detail' exam.id question_details.current|add:'-1' %}"
+Question -> <a href="{% url 'longs:exam_question_detail' exam.id question_details.current|add:'-1' %}"
     title="View the Question">View</a>
 <a href="{% url 'longs:long_update' question.id %}" title="Edit the Question">Edit</a> <a
     href="{% url 'admin:longs_long_change' question.id %}" title="Edit the Question using the admin interface">Admin
@@ -19,7 +19,7 @@
     <ul>
         {% for answer in user_answers %}
         <li>
-            <a href="{% url 'longs:mark_answer' exam.id question_details.current|add:'-1' answer.id %}"
+            <a class="mark-answer-link" href="{% url 'longs:mark_answer' exam.id question_details.current|add:'-1' answer.id %}"
                 title="Click to mark"> {{answer.get_candidate_masked}}</a>:
             {% if answer.is_marked %}
             Score {{answer.get_answer_score}}

longs/tests/test_longs_exams.py

@@ -30,6 +30,7 @@ from longs.models import (
 from generic.models import CidUser, CidUserGroup, UserUserGroup
 
 from django.contrib.auth.models import User
+from django.core.exceptions import PermissionDenied
 
 from rad.tests.test_helpers import AssertNotFound, create_cid_user_and_groups
 
@@ -150,7 +151,7 @@ def test_exams(db, client):
         == 404
     )
 
-    users_tested = 1
+    users_tested = 0
     for cid_user in [cid_user_1001, cid_user_1000, user1]:
         if isinstance(cid_user, CidUser):
             testing_cid_user = True
@@ -550,16 +551,6 @@ def test_exams(db, client):
         assert str(answer_lis[-1]).count("Not answered") == 5
 
 
-        ## Load the mark overview page
-        #mark_overview_res = client.get(reverse(
-        #        f"{exam.app_name}:mark_overview",
-        #        kwargs={"pk": exam.pk},
-        #    ))
-        #print(mark_overview_res.content)
-        #mark_overview_soup = BeautifulSoup(mark_overview_res.content, "html.parser")
-
-        #to_mark =  mark_overview_soup.find("ul", {"id": "question-mark-list"}).find_all("li")
-        #assert len(to_mark) == 5
 
 
         # Publish the results!
@@ -642,3 +633,55 @@ def test_exams(db, client):
 
         generated_questions.pop(-1).delete()
         users_tested = users_tested + 1
+
+    assert users_tested == 3
+
+
+    # Some quick tests to check for user access to the marking page
+    client.login(username="marker", password="test1234")
+
+    
+    # Load the mark overview page
+    mark_overview_res = client.get(reverse(
+            f"{exam.app_name}:mark_overview",
+            kwargs={"pk": exam.pk},
+        ))
+
+    assert mark_overview_res.status_code == 403
+
+    exam.markers.add(User.objects.get(username="marker"))
+
+
+    mark_overview_res = client.get(reverse(
+            f"{exam.app_name}:mark_overview",
+            kwargs={"pk": exam.pk},
+        ))
+    assert mark_overview_res.status_code == 200
+    mark_overview_soup = BeautifulSoup(mark_overview_res.content, "html.parser")
+
+    to_mark =  mark_overview_soup.find("ul", {"id": "question-mark-list"}).find_all("li")
+    assert len(to_mark) == 3
+
+    for n in range(0,3):
+        mark_question_res = client.get(reverse(
+                f"{exam.app_name}:mark",
+                kwargs={"exam_id": exam.pk, "sk": n},
+            ))
+        assert mark_question_res.status_code == 200
+        mark_question_soup = BeautifulSoup(mark_question_res.content, "html.parser")
+        print(mark_question_soup)
+
+        #mark_links = mark_question_soup.find_all("a", {"class": "mark-answer-link"})
+        #assert len(mark_links) == 2
+
+        #for link in mark_links:
+        #    mark_res = client.get(link["href"])
+        #    assert mark_res.status_code == 200
+
+
+    mark_question_res = client.get(reverse(
+            f"{exam.app_name}:mark",
+            kwargs={"exam_id": exam.pk, "sk": 4},
+        ))
+    assert mark_question_res.status_code == 404
+    mark_question_soup = BeautifulSoup(mark_question_res.content, "html.parser")

longs/views.py

@@ -641,17 +641,19 @@ class LongSeriesView(LoginRequiredMixin, SingleTableMixin, FilterView):
     filterset_class = LongSeriesFilter
 
 
-@user_is_long_marker
+#@user_is_long_marker
-@reversion.create_revision()
 def mark_answer_override(request, exam_id, question_number, cid):
     return mark_answer(request, exam_id, question_number, cid, override=True)
 
 
-@user_is_long_marker
+#@user_is_long_marker
 @reversion.create_revision()
 def mark_answer(request, exam_id, question_number, answer_id, override=False):
     exam = get_object_or_404(Exam, pk=exam_id)
 
+    if not GenericExamViews.check_user_marker_access(request.user, exam_id):
+        raise PermissionDenied
+
     questions = exam.get_questions()
 
     n = question_number
@@ -843,10 +845,13 @@ def mark_answer(request, exam_id, question_number, answer_id, override=False):
 
 # @user_passes_test(user_is_admin, login_url="/accounts/login")
 @login_required
-@user_is_long_marker
+#@user_is_long_marker
 def mark(request, exam_id, sk):
     exam = get_object_or_404(Exam, pk=exam_id)
 
+    if not GenericExamViews.check_user_marker_access(request.user, exam_id):
+        raise PermissionDenied
+
     questions = exam.get_questions()
 
     n = sk

physics/migrations/0013_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 = [
+        ('physics', '0012_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='physics_exam_markers', to=settings.AUTH_USER_MODEL),
+        ),
+    ]

physics/models.py

@@ -170,6 +170,7 @@ class Exam(ExamBase):
     markers = models.ManyToManyField(
         settings.AUTH_USER_MODEL,
         blank=True,
+        null=True,
         help_text="Authorised markers for the exam",
         related_name="physics_exam_markers",
     )

rad/tests/test_helpers.py

@@ -47,6 +47,10 @@ def create_cid_user_and_groups(db, marker=None):
     #    marker_group = Group.objects.create(name=f"{marker}_marker")
     #    #marker_group = Group.objects.create(name=f"{marker}_marker")
 
+    marker = User.objects.create_user(
+        username="marker", email="test@marker.com", password="test1234"
+    )
+
 
 def create_exam(db, ExamViews: ExamViews):
     exam: ExamBase = ExamViews.Exam.objects.create(

rapids/migrations/0012_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 = [
+        ('rapids', '0011_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='rapid_exam_markers', to=settings.AUTH_USER_MODEL),
+        ),
+    ]

rapids/models.py

@@ -632,6 +632,7 @@ class Exam(ExamBase):
     markers = models.ManyToManyField(
         settings.AUTH_USER_MODEL,
         blank=True,
+        null=True,
         help_text="Authorised markers for the exam",
         related_name="rapid_exam_markers",
     )

rapids/views.py

@@ -578,6 +578,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")
 

sbas/migrations/0013_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 = [
+        ('sbas', '0012_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='sba_exam_markers', to=settings.AUTH_USER_MODEL),
+        ),
+    ]

sbas/models.py

@@ -188,6 +188,7 @@ class Exam(ExamBase):
     markers = models.ManyToManyField(
         settings.AUTH_USER_MODEL,
         blank=True,
+        null=True,
         help_text="Authorised markers for the exam",
         related_name="sba_exam_markers",
     )