diff --git a/AGENTS.md b/AGENTS.md index 5e544f28..9caf90ba 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -78,6 +78,10 @@ Instead - Do not use `TestCase` unless necessary — prefer plain `pytest` functions with `@pytest.mark.django_db`. - Run tests: `pytest` or `pytest {app}/tests/`. +## Exceptions +- Avoid unnecessary try: except blocks. +- We should not generate empty excepty blocks, if there is an error or bug this should be surfaced (even if just to the logs). + ## Celery / Tasks - Tasks autodiscovered from `{app}/tasks.py`. diff --git a/atlas/decorators.py b/atlas/decorators.py index 62efd247..cf015afd 100755 --- a/atlas/decorators.py +++ b/atlas/decorators.py @@ -108,8 +108,29 @@ def user_is_collection_author_or_atlas_editor(function): or request.user.is_superuser ): return function(request, *args, **kwargs) - else: - raise PermissionDenied + + # Allow access if request.user is the supervisor of the target user and sharing is enabled + user_pk = kwargs.get("user_pk") or kwargs.get("user_id") + if user_pk: + from django.contrib.auth import get_user_model + from django.contrib.contenttypes.models import ContentType + from generic.models import CidUserExam + try: + target_user = get_user_model().objects.get(pk=user_pk) + sup = target_user.userprofile.supervisor + if sup and sup.user_id == request.user.id: + ct = ContentType.objects.get_for_model(CaseCollection) + cid_user_exam = CidUserExam.objects.filter( + content_type=ct, + object_id=atlas.pk, + user_user=target_user + ).first() + if (cid_user_exam and cid_user_exam.share_with_supervisor) or atlas.results_supervisor_visible: + return function(request, *args, **kwargs) + except Exception: + pass + + raise PermissionDenied wrap.__doc__ = function.__doc__ wrap.__name__ = function.__name__ diff --git a/atlas/templates/atlas/collection_case_view_take.html b/atlas/templates/atlas/collection_case_view_take.html index 1db533b4..f33aa06d 100644 --- a/atlas/templates/atlas/collection_case_view_take.html +++ b/atlas/templates/atlas/collection_case_view_take.html @@ -1,9 +1,18 @@ {% extends 'atlas/base.html' %} {% load crispy_forms_tags %} +{% load django_htmx %} {% block content %}
Collection: {{collection}}
+ {% if viewing_as_user %} + + {% endif %} +

Case {{case_number|add:1}} @@ -306,6 +315,7 @@ {% endif %} {% endif %} + {% if not viewing_as_user %}
{% if previous %} @@ -323,11 +333,24 @@ {% endif %} -
+ {% else %} + {# Read-only navigation for reviewer mode — uses anchor links to avoid mutating POST #} +
+ {% if previous %} + Previous + {% endif %} + {% if next %} + Next + {% endif %} +
+
+ Overview +
+ {% endif %} {% if question_completed and collection.case_query_messaging_enabled %} diff --git a/atlas/templates/atlas/collection_history_user.html b/atlas/templates/atlas/collection_history_user.html index a744183e..6a2ffbf1 100644 --- a/atlas/templates/atlas/collection_history_user.html +++ b/atlas/templates/atlas/collection_history_user.html @@ -45,7 +45,7 @@
+ href="{% url 'atlas:collection_case_view_take_review_user' pk=collection.id case_number=forloop.counter0 user_pk=user.pk %}"> Open Case {% if request.user.is_superuser and row.user_answer %} diff --git a/atlas/urls.py b/atlas/urls.py index 8238f352..416f3b8e 100755 --- a/atlas/urls.py +++ b/atlas/urls.py @@ -438,6 +438,11 @@ urlpatterns = [ views.redirect_collection_case_view_take_user_answers_by_id, name="collection_case_view_take_user_answers_legacy", ), + path( + "collection///take/review//", + views.collection_case_view_take_review_user, + name="collection_case_view_take_review_user", + ), path( "collection//json_edit", views.GenericExamViews.exam_json_edit, diff --git a/atlas/views.py b/atlas/views.py index e99651eb..520b1b69 100755 --- a/atlas/views.py +++ b/atlas/views.py @@ -6790,14 +6790,28 @@ def collection_case_review_thread(request, exam_id: int, cid_user_exam_id: int, can_feedback = _user_can_moderate_collection(collection, request.user) can_query = False if not can_feedback: - try: - collection.check_user_can_take(cid, passcode, request.user) - if cid_user_exam.user_user_id: - can_query = request.user.is_authenticated and cid_user_exam.user_user_id == request.user.id - elif cid_user_exam.cid_user_id and cid is not None: - can_query = str(cid_user_exam.cid_user.cid) == str(cid) - except Exception: - can_query = False + # Check if request.user is the supervisor of the attempt owner and sharing is enabled + is_supervisor = False + if request.user.is_authenticated and cid_user_exam.user_user_id: + try: + sup = cid_user_exam.user_user.userprofile.supervisor + if sup and sup.user_id == request.user.id: + if cid_user_exam.share_with_supervisor or collection.results_supervisor_visible: + is_supervisor = True + except Exception: + pass + + if is_supervisor: + can_feedback = True + else: + try: + collection.check_user_can_take(cid, passcode, request.user) + if cid_user_exam.user_user_id: + can_query = request.user.is_authenticated and cid_user_exam.user_user_id == request.user.id + elif cid_user_exam.cid_user_id and cid is not None: + can_query = str(cid_user_exam.cid_user.cid) == str(cid) + except Exception: + can_query = False if not (can_feedback or can_query): raise PermissionDenied @@ -8129,6 +8143,149 @@ def collection_case_view_take_user_answers(request, pk: int, case_number: int): return collection_case_view_take_answers(request, pk, case_number) +@login_required +@user_is_collection_author_or_atlas_editor +def collection_case_view_take_review_user(request, pk: int, case_number: int, user_pk: int): + """Read-only case review screen for supervisors and collection authors/editors. + + Renders the same case-taking template as the candidate view but in a purely + read-only capacity — loading the *target user's* saved answers and exam + state without mutating anything (no start_time, no answer creation, no POST + handling). + + Access is gated by ``@user_is_collection_author_or_atlas_editor`` which also + permits an authorised supervisor when the trainee has enabled + ``share_with_supervisor`` or the collection is marked + ``results_supervisor_visible``. + + Args: + pk: CaseCollection primary key. + case_number: Zero-based index of the case within the collection. + user_pk: Primary key of the trainee whose answers are to be viewed. + """ + from django.contrib.auth import get_user_model + + User = get_user_model() + + collection = get_object_or_404(CaseCollection, pk=pk) + target_user = get_object_or_404(User, pk=user_pk) + + case, case_count = collection.get_case_by_index(case_number, case_count=True) + casedetail = CaseDetail.objects.get(case=case, collection=collection) + + # Load the target user's exam record (read-only — no create side effects if + # using get_or_create, but we deliberately avoid creating one here). + from generic.models import CidUserExam + from django.contrib.contenttypes.models import ContentType + ct = ContentType.objects.get_for_model(CaseCollection) + cid_user_exam = CidUserExam.objects.filter( + content_type=ct, + object_id=collection.pk, + user_user=target_user, + ).first() + + # Load existing answer for this case — never create a placeholder. + answer = None + form = None + if collection.collection_type in ("REP", "QUE"): + answer = casedetail.userreportanswer_set.filter(user=target_user).first() + if answer: + if collection.collection_type == "REP": + from atlas.forms import UserReportAnswerForm + form = UserReportAnswerForm(instance=answer, casedetail=casedetail) + else: + from atlas.forms import UserQuestionAnswerForm + form = UserQuestionAnswerForm(instance=answer, casedetail=casedetail) + + # Determine visibility / review state — treat the exam as completed for a + # reviewer so all post-completion sections (report, findings, discussion) + # are shown, matching what the trainee sees when reviewing their own answers. + question_completed = True + + show_title = collection.show_title_post + show_history = collection.show_history_post + show_description = collection.show_description_post + show_discussion = collection.show_discussion_post + show_report = collection.show_report_post + + series_list = casedetail.get_visible_case_series(review_mode=True) + + prior_cases = casedetail.caseprior_set.all() + series_to_load = [] + for series in series_list: + series_to_load.append((series, False, "")) + for prior in prior_cases: + match prior.prior_visibility: + case "AL": + for series in prior.prior_case.get_series(): + series_to_load.append((series, True, prior.relation_text)) + case "RE": + for series in prior.prior_case.get_series(): + series_to_load.append((series, True, prior.relation_text)) + case _: + continue + + has_priors = any(prior_flag for (_, prior_flag, _) in series_to_load) + prior_count = prior_cases.count() + + case_review_stats = {} + self_review = [] + resources = case.caseresource_set.all() + + if cid_user_exam: + case_review_stats_map, _, _ = _get_case_review_stats_for_exam( + cid_user_exam=cid_user_exam, + casedetails=[casedetail], + messaging_enabled=_collection_case_query_messaging_enabled(collection), + ) + case_review_stats = case_review_stats_map.get(casedetail.case_id, {}) + self_review = cid_user_exam.selfreview_set.filter(case=case) + + previous = case_number > 0 + next_case = case_number < (case_count - 1) + + return render( + request, + "atlas/collection_case_view_take.html", + { + "form": form, + "collection": collection, + "case": case, + "casedetail": casedetail, + "series_list": series_list, + "named_stacks_json": casedetail.get_case_named_stacks(review_mode=True), + "series_to_load": series_to_load, + "has_priors": has_priors, + "prior_count": prior_count, + "case_number": case_number, + "previous": previous, + "next": next_case, + "cid": None, + "passcode": None, + "answer": answer, + "show_title": show_title, + "show_history": show_history, + "show_description": show_description, + "show_discussion": show_discussion, + "show_report": show_report, + "resources": resources, + "cid_user_exam": cid_user_exam, + "case_review_stats": case_review_stats, + "question_completed": question_completed, + "self_review": self_review, + "answer_started_at_iso": answer.started_at.isoformat() if answer and getattr(answer, "started_at", None) else None, + "effective_question_time_limit": None, + "effective_answer_entry_grace_period": None, + "case_view_locked": False, + "answer_entry_closed": False, + "start_screen_resources": collection.start_screen_resources.all(), + # Read-only mode flags + "viewing_as_user": True, + "target_user": target_user, + }, + ) + + def collection_case_view_take_answers( request, pk: int, case_number: int, cid=None, passcode=None ): diff --git a/generic/templates/generic/supervisor_collection_feedback.html b/generic/templates/generic/supervisor_collection_feedback.html new file mode 100644 index 00000000..23e760e1 --- /dev/null +++ b/generic/templates/generic/supervisor_collection_feedback.html @@ -0,0 +1,99 @@ +{% extends 'generic/base_supervisor.html' %} + +{% block content %} +
+
+
+ +

Collection: {{ collection.name }}

+
+ Trainee: {{ trainee.first_name }} {{ trainee.last_name }} ({{ trainee.email }}) +
+
+
+
+
Feedback Summary
+
{{ total_outstanding_feedback }} outstanding feedback item{{ total_outstanding_feedback|pluralize }}
+
Across {{ outstanding_case_count }} case{{ outstanding_case_count|pluralize }}
+
+
+
+ + {% if outstanding_rows %} +
+

Outstanding Feedback

+
+ {% for row in outstanding_rows %} +
+
+
+ Case {{ row.casedetail.sort_order|add:1 }} + {% if collection.show_title_post %} + {{ row.casedetail.case.title|default:row.casedetail.case.pk|truncatechars:80 }} + {% endif %} +
+ {{ row.stats.outstanding_feedback_count }} awaiting acknowledgement +
+
+
+
Loading conversation...
+
+
+
+ {% endfor %} +
+
+ {% else %} + + {% endif %} + + {% if history_rows %} +
+

Previous Conversations

+
+ {% for row in history_rows %} +
+

+ +

+
+
+
+
Open to load conversation history...
+
+
+
+
+ {% endfor %} +
+
+ {% elif not outstanding_rows %} +
+
+ + No case conversations have been started yet. +
+
+ {% endif %} +
+{% endblock %} diff --git a/generic/templates/generic/supervisor_detail.html b/generic/templates/generic/supervisor_detail.html index 02aa81f9..7642b576 100755 --- a/generic/templates/generic/supervisor_detail.html +++ b/generic/templates/generic/supervisor_detail.html @@ -25,13 +25,7 @@
{{ object.name|default:"-" }}
Email
-
- {% if object.email %} - {{ object.email }} - {% else %} - - - {% endif %} -
+
{% if object.email %}{{ object.email }}{% else %}-{% endif %}
User Account
{{ object.user|default:"-" }}
diff --git a/generic/templates/generic/supervisor_overview.html b/generic/templates/generic/supervisor_overview.html index cadfc000..c9cffec9 100644 --- a/generic/templates/generic/supervisor_overview.html +++ b/generic/templates/generic/supervisor_overview.html @@ -3,41 +3,201 @@ {% load crispy_forms_tags %} {% block content %} +
+ +
+
+
+
+ +
+
+

Supervisor: {{ supervisor.name }}

+

{{ supervisor.email }} · {{ supervisor.site|default:"No site linked" }}

+
+
+ +
+
+
Trainees
+
{{ trainees.count }}
+
+
+
Total Attempts
+
{{ attempts|length }}
+
+
+
+
-

Supervisor: {{supervisor.name}}

-
- Help -

As a supervisor you have the ability to view results on the platform for your associated trainees.

-

Some exams / collection results will be automatically available for you to review, others may require the trainee to allow access

-

If your trainees are incorrect please contact ross.kruger@nhs.net

+ +
+
+ +
+ Supervisor Dashboard Guide: +

This hub allows you to monitor and review your trainees' progress on exams and case collections.

+

Trainees have option to choose which attempts to share. Shared attempts are fully viewable (score and answers link). Private attempts show the completion date only, keeping results confidential.

+
+
+
-
+ + +
+ +
+
+
+
+ + + + + + + + + + + + + + {% for stat in trainee_stats %} + + + + + + + + + + {% empty %} + + + + {% endfor %} + +
Trainee NameEmailGradeSiteShared / Private AttemptsLast ActivityAction
+ {{ stat.trainee.first_name }} {{ stat.trainee.last_name }} + {{ stat.trainee.email }}{{ stat.trainee.userprofile.grade|default:"N/A" }}{{ stat.trainee.userprofile.site|default:"N/A" }} + {{ stat.shared_count }} + {{ stat.private_count }} + + {% if stat.last_active %} + {{ stat.last_active|date:"d M Y H:i" }} + {% else %} + No activity + {% endif %} + + + View Progress + +
+ + You currently have no associated trainees. +
+
+
+
+
-

Trainees

-You have the following trainee(s): - - - + +
+
+
+
+ + + + + + + + + + + + + + {% for item in attempts %} + + + + + + + + + + {% empty %} + + + + {% endfor %} + +
TraineeAttempt ItemTypeDate CompletedSharingScore / ProgressAction
+ {{ item.trainee.first_name }} {{ item.trainee.last_name }} + {{ item.exam }} + + {{ item.type_display }} + + {{ item.date|date:"d M Y H:i" }} + {% if item.is_shared %} + Shared + {% else %} + Private + {% endif %} + + {% if item.is_shared %} + {{ item.score_str }} + {% else %} + Locked + {% endif %} + + {% if item.is_shared and item.detail_url %} + + View Details + + {% else %} + + {% endif %} +
+ + No trainee attempts recorded. +
+
+
+
+
+
+
{% endblock %} - {% block css %} - {% endblock css %} - - diff --git a/generic/templates/generic/supervisor_trainee.html b/generic/templates/generic/supervisor_trainee.html index 7d04af3a..ee177347 100644 --- a/generic/templates/generic/supervisor_trainee.html +++ b/generic/templates/generic/supervisor_trainee.html @@ -1,38 +1,111 @@ {% extends 'generic/base_supervisor.html' %} -{% load crispy_forms_tags %} -{% block css %} -{% endblock %} - {% block content %} +
+ + -

Trainee: {{trainee.first_name}} {{trainee.last_name}}

-email: {{trainee.email}} + +
+
+
+
+ +
+
+

{{ trainee.first_name }} {{ trainee.last_name }}

+

{{ trainee.email }}

+
+
+ +
+
+
Grade
+ {{ trainee.userprofile.grade|default:"N/A" }} +
+
+
Primary Site
+ {{ trainee.userprofile.site|default:"N/A" }} +
+
+
Attempts
+ {{ attempts|length }} +
+
+
+
- -

Results

-
- {% if all_exams %} - The following exam results for the trainee are available. - {% for exam_type, exams in all_exams %} - {% if exams %} -

{{exam_type|title}}

-
    - {% for exam in exams %} -
  • - - - - {{exam}} {% if exam.active %}[Active]{% endif %} {% if exam.publish_results %}[Results Published]{% endif %}
  • - {% endfor %} -
- {% endif %} - {% endfor %} - {% else %} - No results are available for this trainee. -{% endif %} + +
+
+

Trainee Attempts History

+
+
+
+ + + + + + + + + + + + + {% for item in attempts %} + + + + + + + + + {% empty %} + + + + {% endfor %} + +
Attempt ItemTypeDate AttemptedSharing StatusScore / ProgressAction
{{ item.exam }} + + {{ item.type_display }} + + {{ item.date|date:"d M Y H:i" }} + {% if item.is_shared %} + Shared + {% else %} + Private + {% endif %} + + {% if item.is_shared %} + {{ item.score_str }} + {% else %} + Locked (Private) + {% endif %} + + {% if item.is_shared and item.detail_url %} + + View Details + + {% else %} + + {% endif %} +
+ + This trainee has not started any attempts yet. +
+
+
+
- - {% endblock %} - diff --git a/generic/tests/test_exam_collection.py b/generic/tests/test_exam_collection.py index 5b924ad4..7851ea27 100644 --- a/generic/tests/test_exam_collection.py +++ b/generic/tests/test_exam_collection.py @@ -11,6 +11,9 @@ import datetime class TestExamCollections: def test_list_url(self, db, client): + from django.contrib.auth.models import User + user = User.objects.create_user(username="test_list_user", password="password") + client.force_login(user) # Test with no content response = client.get(reverse("generic:examcollection_list")) assert response.status_code == 200 @@ -22,11 +25,12 @@ class TestExamCollections: assert response.status_code == 404 collection = ExamCollection.objects.create(name="test collection", date=datetime.date.today()) + collection.author.add(user) response = client.get(reverse("generic:examcollection_list")) assert response.status_code == 200 soup = BeautifulSoup(response.content, "html.parser") - assert collection.name in soup.find("li", class_="collection").text + assert collection.name in soup.find("a", class_="stretched-link").text response = client.get(reverse("generic:examcollection_detail", args=( collection.pk, ))) diff --git a/generic/tests/test_supervisor_dashboard.py b/generic/tests/test_supervisor_dashboard.py new file mode 100644 index 00000000..975645bb --- /dev/null +++ b/generic/tests/test_supervisor_dashboard.py @@ -0,0 +1,265 @@ +import pytest +from django.urls import reverse +from django.contrib.auth.models import User +from django.test import Client +from django.contrib.contenttypes.models import ContentType +from generic.models import Supervisor, UserProfile, CidUserExam +from physics.models import Exam as PhysicsExam +from atlas.models import CaseCollection, Case, CaseDetail + +@pytest.fixture +def supervisor_user(db): + return User.objects.create_user(username="supervisor_user", password="password", email="supervisor@test.com") + +@pytest.fixture +def supervisor_model(db, supervisor_user): + return Supervisor.objects.create(name="Test Supervisor", email="supervisor@test.com", user=supervisor_user) + +@pytest.fixture +def other_supervisor_user(db): + return User.objects.create_user(username="other_supervisor", password="password", email="other@test.com") + +@pytest.fixture +def other_supervisor(db, other_supervisor_user): + return Supervisor.objects.create(name="Other Supervisor", email="other@test.com", user=other_supervisor_user) + +@pytest.fixture +def trainee_user(db, supervisor_model): + user = User.objects.create_user(username="trainee", password="password", email="trainee@test.com") + profile = user.userprofile + profile.supervisor = supervisor_model + profile.save() + return user + +@pytest.fixture +def physics_exam(db): + return PhysicsExam.objects.create(name="Test Physics Exam", exam_mode=True) + +@pytest.fixture +def case_collection(db): + collection = CaseCollection.objects.create(name="Test Case Collection") + # Enable messaging + collection.feedback_once_collection_complete = True + collection.save() + return collection + +@pytest.fixture +def case_model(db): + return Case.objects.create(title="Test Case") + +@pytest.fixture +def case_detail(db, case_collection, case_model): + return CaseDetail.objects.create(collection=case_collection, case=case_model, sort_order=0) + +@pytest.fixture +def exam_attempt(db, trainee_user, physics_exam): + ct = ContentType.objects.get_for_model(PhysicsExam) + return CidUserExam.objects.create( + content_type=ct, + object_id=physics_exam.pk, + user_user=trainee_user, + share_with_supervisor=False + ) + +@pytest.fixture +def collection_attempt(db, trainee_user, case_collection): + ct = ContentType.objects.get_for_model(CaseCollection) + return CidUserExam.objects.create( + content_type=ct, + object_id=case_collection.pk, + user_user=trainee_user, + share_with_supervisor=False + ) + +@pytest.mark.django_db +def test_supervisor_overview_access( + supervisor_user, + other_supervisor_user, + supervisor_model, + trainee_user, + physics_exam, + exam_attempt, + case_collection, + collection_attempt, +): + client = Client() + url = reverse("generic:supervisor_overview", kwargs={"pk": supervisor_model.pk}) + + # Anonymous blocked + response = client.get(url) + assert response.status_code == 302 + + # Other supervisor blocked + client.force_login(other_supervisor_user) + response = client.get(url) + assert response.status_code == 403 + + # Authorized supervisor allowed + client.force_login(supervisor_user) + + # First test with private attempts + response = client.get(url) + assert response.status_code == 200 + assert b"Test Physics Exam" in response.content + assert b"Test Case Collection" in response.content + assert b"Locked" in response.content + + # Now test with shared attempts + exam_attempt.share_with_supervisor = True + exam_attempt.save() + collection_attempt.share_with_supervisor = True + collection_attempt.save() + + response = client.get(url) + assert response.status_code == 200 + assert b"Test Physics Exam" in response.content + assert b"Test Case Collection" in response.content + assert b"0 / 0 cases" in response.content # collection progress check + +@pytest.mark.django_db +def test_supervisor_trainee_access(supervisor_user, other_supervisor_user, supervisor_model, trainee_user): + client = Client() + url = reverse("generic:supervisor_trainee", kwargs={"pk": supervisor_model.pk, "trainee_id": trainee_user.pk}) + + # Other supervisor blocked + client.force_login(other_supervisor_user) + response = client.get(url) + assert response.status_code == 403 + + # Authorized supervisor allowed + client.force_login(supervisor_user) + response = client.get(url) + assert response.status_code == 200 + +@pytest.mark.django_db +def test_exam_scores_user_supervisor_sharing(supervisor_user, other_supervisor_user, supervisor_model, trainee_user, physics_exam, exam_attempt): + client = Client() + url = reverse("physics:exam_scores_user_supervisor", kwargs={"pk": physics_exam.pk, "user_id": trainee_user.pk}) + + # Other supervisor blocked regardless + client.force_login(other_supervisor_user) + response = client.get(url) + assert response.status_code == 403 + + # Authorized supervisor blocked when not shared + client.force_login(supervisor_user) + response = client.get(url) + assert response.status_code == 403 + + # Share and check allowed + exam_attempt.share_with_supervisor = True + exam_attempt.save() + response = client.get(url) + assert response.status_code == 200 + + # Unshare, but set results_supervisor_visible = True + exam_attempt.share_with_supervisor = False + exam_attempt.save() + physics_exam.results_supervisor_visible = True + physics_exam.save() + response = client.get(url) + assert response.status_code == 200 + +@pytest.mark.django_db +def test_supervisor_collection_feedback_sharing(supervisor_user, supervisor_model, trainee_user, case_collection, collection_attempt): + client = Client() + url = reverse("atlas:collection_history_user", kwargs={ + "exam_id": case_collection.pk, + "user_pk": trainee_user.pk + }) + + # Supervisor blocked when not shared + client.force_login(supervisor_user) + response = client.get(url) + assert response.status_code == 403 + + # Supervisor allowed when shared + collection_attempt.share_with_supervisor = True + collection_attempt.save() + response = client.get(url) + assert response.status_code == 200 + +@pytest.mark.django_db +def test_collection_case_review_thread_supervisor_access(supervisor_user, supervisor_model, trainee_user, case_collection, collection_attempt, case_model, case_detail): + client = Client() + url = reverse("atlas:collection_case_review_thread", kwargs={ + "exam_id": case_collection.pk, + "cid_user_exam_id": collection_attempt.pk, + "case_id": case_model.pk + }) + + # Blocked when attempt is not shared + client.force_login(supervisor_user) + response = client.get(url) + assert response.status_code == 403 + + # Allowed when shared + collection_attempt.share_with_supervisor = True + collection_attempt.save() + response = client.get(url) + assert response.status_code == 200 + + # Post a feedback reply as supervisor + response = client.post(url, { + "action": "add_message", + "message_type": "FB", + "message": "Nice job trainee!" + }, HTTP_X_REQUESTED_WITH="XMLHttpRequest") + assert response.status_code == 200 + assert b"Nice job trainee!" in response.content + + +@pytest.mark.django_db +def test_collection_case_view_take_review_user( + supervisor_user, + other_supervisor_user, + supervisor_model, + trainee_user, + case_collection, + case_model, + case_detail, + collection_attempt, +): + """Supervisors can view a trainee's case answers in read-only mode. + + - Unshared attempts are blocked (403). + - Shared attempts return 200 with the reviewer banner present. + - Other supervisors with no relationship to the trainee are blocked. + - A collection author can always view the answers. + """ + client = Client() + url = reverse( + "atlas:collection_case_view_take_review_user", + kwargs={"pk": case_collection.pk, "case_number": 0, "user_pk": trainee_user.pk}, + ) + + # Anonymous user is redirected to login + response = client.get(url) + assert response.status_code == 302 + + # Authorized supervisor blocked when attempt not shared + client.force_login(supervisor_user) + response = client.get(url) + assert response.status_code == 403 + + # Authorized supervisor allowed when attempt is shared + collection_attempt.share_with_supervisor = True + collection_attempt.save() + response = client.get(url) + assert response.status_code == 200 + # Reviewer banner should be present + assert b"Reviewer mode" in response.content + assert b"read-only" in response.content + + # Other supervisor (not linked to trainee) is still blocked even when shared + client.force_login(other_supervisor_user) + response = client.get(url) + assert response.status_code == 403 + + # Collection author can always view the answers + author_user = User.objects.create_user(username="author_user", password="password") + case_collection.author.add(author_user) + client.force_login(author_user) + response = client.get(url) + assert response.status_code == 200 + assert b"Reviewer mode" in response.content diff --git a/generic/urls.py b/generic/urls.py index ca1e8b36..088d36e0 100755 --- a/generic/urls.py +++ b/generic/urls.py @@ -239,6 +239,11 @@ urlpatterns = [ views.supervisor_trainee, name="supervisor_trainee", ), + path( + "supervisor//trainee//collection//", + views.supervisor_collection_feedback, + name="supervisor_collection_feedback", + ), path( "supervisor/", views.SupervisorDetail.as_view(), diff --git a/generic/views.py b/generic/views.py index 06194589..ab0ea1e8 100644 --- a/generic/views.py +++ b/generic/views.py @@ -380,6 +380,126 @@ class CidManagerRequiredMixin(UserPassesTestMixin): # raise PermissionDenied() # or Http404 +def get_exam_attempt_score(exam_type, exam, user): + from django.contrib.contenttypes.models import ContentType + from physics.models import UserAnswer as PhysicsUserAnswer + from anatomy.models import UserAnswer as AnatomyUserAnswer + from rapids.models import UserAnswer as RapidsUserAnswer + from shorts.models import UserAnswer as ShortsUserAnswer + from longs.models import UserAnswer as LongsUserAnswer + from sbas.models import UserAnswer as SbasUserAnswer + + MAP = { + "physics": PhysicsUserAnswer, + "anatomy": AnatomyUserAnswer, + "rapids": RapidsUserAnswer, + "shorts": ShortsUserAnswer, + "longs": LongsUserAnswer, + "sbas": SbasUserAnswer, + } + UserAnswer = MAP.get(exam_type) + if not UserAnswer: + return None, None + + questions = list(exam.get_questions()) + if not questions: + return 0, 0 + user_answers = UserAnswer.objects.filter(user=user, exam=exam).select_related("question") + user_answers_map = {ua.question_id: ua for ua in user_answers} + + total_score = 0 + for q in questions: + ua = user_answers_map.get(q.pk) + if ua: + total_score += ua.get_answer_score() + else: + score, _ = q.get_unanswered_mark_and_text() + total_score += score + + # Max score + match exam_type: + case "rapids" | "anatomy": + max_score = len(questions) * 2 + case "longs": + max_score = len(questions) * 8 + case "physics" | "shorts": + max_score = len(questions) * 5 + case _: + max_score = len(questions) + + return total_score, max_score + + +def get_supervisor_attempts(supervisor, trainees=None): + from django.contrib.contenttypes.models import ContentType + from generic.models import CidUserExam + from atlas.models import CaseCollection + from django.urls import reverse + from django.utils import timezone + + if trainees is None: + trainees = User.objects.filter(userprofile__supervisor=supervisor) + + attempts = CidUserExam.objects.filter(user_user__in=trainees).select_related("user_user", "content_type") + + parsed_attempts = [] + for att in attempts: + exam = att.exam + if not exam: + continue + + friendly_type = att.content_type.app_label + if friendly_type == "atlas": + friendly_type = "casecollection" + + exam_supervisor_visible = getattr(exam, "results_supervisor_visible", False) + is_shared = att.share_with_supervisor or exam_supervisor_visible + + score_str = "N/A" + detail_url = None + + if is_shared: + if friendly_type == "casecollection": + total_cases = exam.casedetail_set.count() + reviewed_cases = att.selfreview_set.count() + score_str = f"{reviewed_cases} / {total_cases} cases" + detail_url = reverse( + "atlas:collection_history_user", + kwargs={ + "exam_id": exam.pk, + "user_pk": att.user_user.pk, + }, + ) + else: + total_score, max_score = get_exam_attempt_score(friendly_type, exam, att.user_user) + if total_score is not None: + pct = int((total_score / max_score) * 100) if max_score > 0 else 0 + score_str = f"{total_score} / {max_score} ({pct}%)" + else: + score_str = "No answers" + detail_url = reverse( + f"{friendly_type}:exam_scores_user_supervisor", + kwargs={"pk": exam.pk, "user_id": att.user_user.pk}, + ) + else: + score_str = "Private" + + parsed_attempts.append({ + "attempt": att, + "exam": exam, + "trainee": att.user_user, + "type": friendly_type, + "type_display": friendly_type.title() if friendly_type != "sbas" else "SBAs", + "date": att.end_time or att.start_time, + "is_shared": is_shared, + "score_str": score_str, + "detail_url": detail_url, + "completed": att.completed, + }) + parsed_attempts.sort(key=lambda x: x["date"] or timezone.now(), reverse=True) + return parsed_attempts + + def get_user_exams(user, supervisor_view=False): EXAM_ANSWER_MAP = { "physics": (PhysicsUserAnswer, PhysicsExam, "physics_exams"), @@ -3547,11 +3667,34 @@ class ExamViews(View, LoginRequiredMixin): return self.exam_scores_cid_user(request, pk, user=user) def exam_scores_user_supervisor(self, request, pk, user_id): + from django.contrib.contenttypes.models import ContentType + from generic.models import CidUserExam + user = User.objects.get(id=user_id) if not request.user.is_superuser: - if not request.user.supervisor == user.userprofile.supervisor: + try: + if not request.user.supervisor == user.userprofile.supervisor: + raise PermissionDenied + except Exception: raise PermissionDenied + exam = get_object_or_404(self.Exam, pk=pk) + ct = ContentType.objects.get_for_model(self.Exam) + cid_user_exam = CidUserExam.objects.filter( + content_type=ct, + object_id=exam.pk, + user_user=user + ).first() + + is_visible = False + if exam.results_supervisor_visible: + is_visible = True + elif cid_user_exam and cid_user_exam.share_with_supervisor: + is_visible = True + + if not is_visible: + raise PermissionDenied("This exam attempt has not been shared with the supervisor.") + return self.exam_scores_cid_user( request, pk, user=user, supervisor=user.userprofile.supervisor.user ) @@ -5988,27 +6131,26 @@ def create_user(request, context=None, trainee: bool = False): class SupervisorDetail(DetailView): model = Supervisor - def get_queryset(self): - """Allow supervisors to view their own detail page; managers see all.""" - qs = super().get_queryset() - if not self.request.user.is_authenticated: - return qs.none() - if self.request.user.is_superuser or self.request.user.groups.filter(name="cid_user_manager").exists(): - return qs - if hasattr(self.request.user, "supervisor"): - return qs.filter(user=self.request.user) - return qs.none() - def get_object(self, queryset=None): - """Override to check permissions before returning object.""" + """Return the supervisor if the requesting user is authorised. + + - Superusers and ``cid_user_manager`` group members may view any supervisor. + - A supervisor linked to the requesting user may view their own record. + - All other authenticated users receive PermissionDenied (403). + - Unauthenticated users are redirected to the login page by + ``LoginRequiredMixin`` (enforced via the URLconf decorator or mixin). + """ obj = super().get_object(queryset) - if not ( + if not self.request.user.is_authenticated: + from django.contrib.auth.views import redirect_to_login + return redirect_to_login(self.request.get_full_path()) + if ( self.request.user.is_superuser or self.request.user.groups.filter(name="cid_user_manager").exists() or (hasattr(self.request.user, "supervisor") and obj.user == self.request.user) ): - raise PermissionDenied - return obj + return obj + raise PermissionDenied @user_is_cid_user_manager @@ -6529,12 +6671,38 @@ def supervisor_overview(request, pk): except User.supervisor.RelatedObjectDoesNotExist: raise PermissionDenied() - trainees = User.objects.filter(userprofile__supervisor=supervisor) + trainees = User.objects.filter(userprofile__supervisor=supervisor).select_related("userprofile__grade", "userprofile__site") + attempts = get_supervisor_attempts(supervisor, trainees) + + # Calculate statistics for trainees + trainee_stats = [] + for trainee in trainees: + trainee_attempts = [a for a in attempts if a["trainee"] == trainee] + total_count = len(trainee_attempts) + shared_count = sum(1 for a in trainee_attempts if a["is_shared"]) + private_count = total_count - shared_count + + last_active = None + if trainee_attempts: + last_active = trainee_attempts[0]["date"] + + trainee_stats.append({ + "trainee": trainee, + "total_count": total_count, + "shared_count": shared_count, + "private_count": private_count, + "last_active": last_active, + }) return render( request, "generic/supervisor_overview.html", - {"supervisor": supervisor, "trainees": trainees}, + { + "supervisor": supervisor, + "trainees": trainees, + "trainee_stats": trainee_stats, + "attempts": attempts, + }, ) @@ -6549,15 +6717,72 @@ def supervisor_trainee(request, pk, trainee_id): except User.supervisor.RelatedObjectDoesNotExist: raise PermissionDenied() - # We do this to check that the supervisor can (should) see the trainee results trainee = User.objects.get(userprofile__supervisor=supervisor, pk=trainee_id) - - exams = get_user_exams(trainee, supervisor_view=True) + attempts = get_supervisor_attempts(supervisor, [trainee]) return render( request, "generic/supervisor_trainee.html", - {"supervisor": supervisor, "trainee": trainee, "all_exams": exams}, + { + "supervisor": supervisor, + "trainee": trainee, + "attempts": attempts, + }, + ) + + +@login_required +def supervisor_collection_feedback(request, pk, trainee_id, collection_id): + from atlas.models import CaseCollection, CaseDetail + from atlas.views import _build_collection_feedback_rows + from django.contrib.contenttypes.models import ContentType + + supervisor = get_object_or_404(Supervisor, pk=pk) + + if not request.user.is_superuser: + try: + if not request.user.supervisor == supervisor: + raise PermissionDenied() + except User.supervisor.RelatedObjectDoesNotExist: + raise PermissionDenied() + + trainee = get_object_or_404(User, userprofile__supervisor=supervisor, pk=trainee_id) + collection = get_object_or_404(CaseCollection, pk=collection_id) + + ct = ContentType.objects.get_for_model(CaseCollection) + cid_user_exam = get_object_or_404( + CidUserExam, user_user=trainee, content_type=ct, object_id=collection.pk + ) + + if not (cid_user_exam.share_with_supervisor or collection.results_supervisor_visible): + raise PermissionDenied("This collection attempt has not been shared with the supervisor.") + + casedetails = list( + CaseDetail.objects.filter(collection=collection).select_related("case").order_by("sort_order") + ) + feedback_rows, outstanding_case_count, total_outstanding_feedback = _build_collection_feedback_rows( + collection=collection, + cid_user_exam=cid_user_exam, + casedetails=casedetails, + ) + + outstanding_rows = [row for row in feedback_rows if row["stats"].get("has_outstanding_feedback")] + history_rows = [row for row in feedback_rows if row["stats"].get("has_messages") and not row["stats"].get("has_outstanding_feedback")] + + return render( + request, + "generic/supervisor_collection_feedback.html", + { + "supervisor": supervisor, + "trainee": trainee, + "collection": collection, + "cid_user_exam": cid_user_exam, + "feedback_rows": feedback_rows, + "outstanding_rows": outstanding_rows, + "history_rows": history_rows, + "outstanding_case_count": outstanding_case_count, + "total_outstanding_feedback": total_outstanding_feedback, + }, ) diff --git a/templates/base.html b/templates/base.html index 650371cb..48617f96 100644 --- a/templates/base.html +++ b/templates/base.html @@ -314,6 +314,11 @@ People {% endif %} + {% if request.user.supervisor %} + + {% endif %} diff --git a/templates/index.html b/templates/index.html index e5ffc93c..a35df43a 100644 --- a/templates/index.html +++ b/templates/index.html @@ -43,6 +43,16 @@
{% endif %} + {% if request.user.supervisor %} +
+
+
Supervisor Dashboard
+

Welcome, {{ request.user.supervisor.name }}. You can track, review, and leave feedback on your trainees' attempts here.

+ Go to Dashboard +
+
+ {% endif %} +
Exams / Courses