From 9cf7ad6da7b0ca3c0f6b83d03decfda9534daadf Mon Sep 17 00:00:00 2001
From: Ross <ross@xkjq.uk>
Date: Mon, 10 Nov 2025 21:18:58 +0000
Subject: [PATCH] Refactor exam_take function to use a canonical list of
 questions for consistent indexing and improved error handling for question
 navigation

---
 physics/templates/physics/exam_take.html | 385 +++++++++++++----------
 physics/views.py                         |  27 +-
 2 files changed, 244 insertions(+), 168 deletions(-)

diff --git a/physics/templates/physics/exam_take.html b/physics/templates/physics/exam_take.html
index 93e2a42c..e1ef7f25 100755
--- a/physics/templates/physics/exam_take.html
+++ b/physics/templates/physics/exam_take.html
@@ -3,17 +3,24 @@
 {% block content %}
     <span id="user-id">
         {% if request.user.is_authenticated %}
-            User: {{request.user}}
+            User: {{ request.user }}
         {% else %}
-            CID: {{cid}}
+            CID: {{ cid }}
         {% endif %}
     </span>
+
     {% include "exam_clock.html" %}
+
     <div class="no-select">
-        <h2>{{exam}}: Question [<span id="question-number">{{pos|add:1}}</span>/<span id="exam-length">{{exam_length}}</span>]</h2>
+        <h2>
+            {{ exam }}: Question
+            [<span id="question-number">{{ pos|add:1 }}</span>/<span id="exam-length">{{ exam_length }}</span>]
+        </h2>
+
         {% if exam.publish_results %}
             <div class="alert alert-primary review-mode-alert" role="alert">
-                Exam is in review mode. Add question feedback <a href="#" onclick="return window.create_popup_window('{% url 'feedback_create' question_type='physics' pk=question.pk %}')">here</a>.
+                Exam is in review mode. Add question feedback
+                <a href="#" onclick="return window.create_popup_window('{% url "feedback_create" question_type="physics" pk=question.pk %}')">here</a>.
             </div>
         {% elif cid_user_exam.completed %}
             <div class="alert alert-primary review-mode-alert" role="alert">
@@ -22,63 +29,68 @@
         {% endif %}
 
         <div>
-            <p><span id="question-stem">{{question.stem|safe}}</span></p>
+            <p><span id="question-stem">{{ question.stem|safe }}</span></p>
         </div>
+        
 
+        <form method="POST" class="post-form">
+            {% csrf_token %}
+            {{ form.non_field_errors }}
 
-        <div>
+            <div class="form-contents">
+                <ol class="physics-answer-list">
+                    <li data-ans="a">
+                        <div class="fieldWrapper flex-container">
+                            {{ form.a.errors }}
+                            <label for="{{ form.a.id_for_label }}" class="flex-8 question-text">{{ question.a|safe }}</label>
+                            <span class="flex-1">{{ form.a }}<span class="postinput"></span></span>
+                        </div>
+                        {% if exam.publish_results and question.a_feedback %}
+                            <span class="feedback">Feedback: {{ question.a_feedback }}</span>
+                        {% endif %}
+                    </li>
 
-            <form method="POST" class="post-form">{% csrf_token %}
-                {{ form.non_field_errors }}
-                <div class="form-contents">
-                    <ol class="physics-answer-list">
-                        <li data-ans="a">
-                            <div class="fieldWrapper flex-container">
-                                {{ form.a.errors }}
-                                <label for="{{ form.a.id_for_label }}" class="flex-8 question-text">{{question.a|safe}}</label>
-                                <span class="flex-1">{{ form.a }}<span class="postinput"></span></span>
+                    <li data-ans="b">
+                        <div class="fieldWrapper flex-container">
+                            {{ form.b.errors }}
+                            <label for="{{ form.b.id_for_label }}" class="flex-8 question-text">{{ question.b|safe }}</label>
+                            <span class="flex-1">{{ form.b }}<span class="postinput"></span></span>
+                        </div>
+                    </li>
 
-                            </div>
-                            {% if exam.publish_results and question.a_feedback %}
-                                <span class="feedback">Feedback: {{question.a_feedback}}</span>
-                            {% endif %}
-                        </li>
-                        <li data-ans="b">
-                            <div class="fieldWrapper flex-container">
-                                {{ form.b.errors }}
-                                <label for="{{ form.b.id_for_label }}" class="flex-8 question-text">{{question.b|safe}}</label>
-                                <span class="flex-1">{{ form.b }}<span class="postinput"></span></span>
-                            </div>
-                        </li>
-                        <li data-ans="c">
-                            <div class="fieldWrapper flex-container">
-                                {{ form.c.errors }}
-                                <label for="{{ form.c.id_for_label }}" class="flex-8 question-text">{{question.c|safe}}</label>
-                                <span class="flex-1">{{ form.c }}<span class="postinput"></span></span>
-                            </div>
-                        </li>
-                        <li data-ans="d">
-                            <div class="fieldWrapper flex-container">
-                                {{ form.d.errors }}
-                                <label for="{{ form.d.id_for_label }}" class="flex-8 question-text">{{question.d|safe}}</label>
-                                <span class="flex-1">{{ form.d }}<span class="postinput"></span></span>
-                            </div>
-                        </li>
-                        <li data-ans="e">
-                            <div class="fieldWrapper flex-container">
-                                {{ form.e.errors }}
-                                <label for="{{ form.e.id_for_label }}" class="flex-8 question-text">{{question.e|safe}}</label>
-                                <span class="flex-1">{{ form.e }}<span class="postinput"></span></span>
-                            </div>
-                        </li>
-                    </ol>
-                </div>
+                    <li data-ans="c">
+                        <div class="fieldWrapper flex-container">
+                            {{ form.c.errors }}
+                            <label for="{{ form.c.id_for_label }}" class="flex-8 question-text">{{ question.c|safe }}</label>
+                            <span class="flex-1">{{ form.c }}<span class="postinput"></span></span>
+                        </div>
+                    </li>
 
+                    <li data-ans="d">
+                        <div class="fieldWrapper flex-container">
+                            {{ form.d.errors }}
+                            <label for="{{ form.d.id_for_label }}" class="flex-8 question-text">{{ question.d|safe }}</label>
+                            <span class="flex-1">{{ form.d }}<span class="postinput"></span></span>
+                        </div>
+                    </li>
+
+                    <li data-ans="e">
+                        <div class="fieldWrapper flex-container">
+                            {{ form.e.errors }}
+                            <label for="{{ form.e.id_for_label }}" class="flex-8 question-text">{{ question.e|safe }}</label>
+                            <span class="flex-1">{{ form.e }}<span class="postinput"></span></span>
+                        </div>
+                    </li>
+                </ol>
+            </div>
+
+            <div class="mt-2">
                 {% if previous > -1 %}
                     <button type="submit" name="previous" class="save btn btn-default" title="Click to save your answer(s) and go to the previous question">Previous</button>
                 {% endif %}
+
                 {% if next %}
-                    <button type="submit" name="next" class="save btn btn-default" title="Click to save your answer(s) and go to the next quesiton">Next</button>
+                    <button type="submit" name="next" class="save btn btn-default" title="Click to save your answer(s) and go to the next question">Next</button>
                 {% else %}
                     {% if not exam.publish_results %}
                         <button type="submit" name="save" class="save btn btn-default" title="Click to save your current answer(s)">Save</button>
@@ -88,135 +100,192 @@
                 <br />
                 <button type="submit" name="finish" class="save btn btn-default" title="Click to go to the overview page (your answers will be saved).">Overview</button>
                 <button type="submit" id="goto-button" value="test" name="goto" class="hide">goto</button>
-            </form>
-        </div>
+            </div>
+        </form>
     </div>
 
     <h4>Questions</h4>
-    <div id="menu-list">
+    <div id="menu-list"></div>
 
-    </div>
     {% include "physics/exam_take_help.html" %}
 
-    {% endblock %}
+{% endblock %}
 
-    {% block js %}
-        <script>
-            $(document).ready(() => {
+{% block js %}
+    <script>
+        $(document).ready(() => {
             /* beautify ignore:start */
-                {% if not exam.publish_results and not cid_user_exam.completed %}
-
-                    let time_limit = '{{exam.time_limit}}'
-                    if (time_limit != "None") {
-                        let end_time = new Date({{cid_user_exam.start_time|date:"U"}}*1000+parseInt('{{exam.time_limit}}')*1000);
-                        initializeClock("clockdiv", end_time);
-                    }
-                {% elif exam.publish_results %}
-                    $(`ol.physics-answer-list input`).each((n, el) => { el.disabled = true; });
-                    {{question.get_answers_js}}.forEach((el, n) => {
-                        console.log(el)
-
-                        li_el = $(`ol.physics-answer-list li:eq(${n})`)//.addClass(`answer-${el}`)
-
-                        if (el) {
-                            li_el.addClass("answer-true");
-                        } else {
-                            li_el.addClass("answer-false");
-                        }
-                        if (el == li_el.find("input").get(0).checked) {
-                            li_el.addClass("answer-correct");
-                        } else {
-                            li_el.addClass("answer-incorrect");
-
-                        }
-
-                    });
-                {% elif cid_user_exam.completed %}
-                    $(`ol.physics-answer-list input`).each((n, el) => { el.disabled = true; });
-                //$("ul.physics-answer-list li[data-ans='{{question.best_answer}}']").addClass("correct");
-                {% endif %}
-
-                for (let i = 0; i < {{ exam_length }}; i++) {
-                    qbutton = $(`<button class="question-menu-item" name="goto-${i}" data-qn="${i}">${i+1}</button>`)
-
-                    if (i == {{pos}}) {qbutton.addClass("current-question")}
-                    $("#menu-list").append(qbutton);
-
+            {% if not exam.publish_results and not cid_user_exam.completed %}
+                let time_limit = '{{ exam.time_limit }}';
+                if (time_limit != 'None') {
+                    let end_time = new Date({{ cid_user_exam.start_time|date:"U" }} * 1000 + parseInt('{{ exam.time_limit }}') * 1000);
+                    initializeClock('clockdiv', end_time);
                 }
-                $("button.question-menu-item").on("click", (e) => {
-                    console.log(e.currentTarget.dataset.qn);
-                    document.getElementById("goto-button").value = e.currentTarget.dataset.qn;
-                    $("#goto-button").click();
+
+            {% elif exam.publish_results %}
+                $('ol.physics-answer-list input').each((n, el) => { el.disabled = true; });
+                {{ question.get_answers_js }}.forEach((el, n) => {
+                    const li_el = $(`ol.physics-answer-list li:eq(${n})`);
+
+                    if (el) {
+                        li_el.addClass('answer-true');
+                    } else {
+                        li_el.addClass('answer-false');
+                    }
+
+                    if (el == li_el.find('input').get(0).checked) {
+                        li_el.addClass('answer-correct');
+                    } else {
+                        li_el.addClass('answer-incorrect');
+                    }
                 });
+
+            {% elif cid_user_exam.completed %}
+                $('ol.physics-answer-list input').each((n, el) => { el.disabled = true; });
+            {% endif %}
+
+            for (let i = 0; i < {{ exam_length }}; i++) {
+                const qbutton = $(`<button class="question-menu-item" name="goto-${i}" data-qn="${i}">${i+1}</button>`);
+                if (i == {{ pos }}) { qbutton.addClass('current-question'); }
+                $('#menu-list').append(qbutton);
+            }
+
+            $('button.question-menu-item').on('click', (e) => {
+                document.getElementById('goto-button').value = e.currentTarget.dataset.qn;
+                $('#goto-button').click();
+            });
             /* beautify ignore:end */
+        });
+    </script>
+{% endblock %}
 
-            })
+{% block css %}
+    <style type="text/css">
+        /* Color variables and dark-mode support
+           - Uses CSS variables for easy theming
+           - Honors system preference via prefers-color-scheme
+           - Also supports a manual `.dark-mode` class on body or a parent
+        */
 
+        :root {
+            --bg: #ffffff;
+            --text: #212529;
+            --muted: #6c757d;
+            --card-bg: #ffffff;
+            --card-border: #e9ecef;
+            --primary: #0d6efd;
+            --success: #198754;
+            --danger: #dc3545;
+            --feedback-bg: #f8f9fa;
+            --shadow: 0 1px 2px rgba(0,0,0,0.03);
+        }
 
-
-
-        </script>
-    {% endblock %}
-    {% block css %}
-        <style type="text/css">
-        /*     .form-contents {
-        display: none;
-    } */
-            .flex-container {
-                display: flex;
+        @media (prefers-color-scheme: dark) {
+            :root {
+                --bg: #050607;
+                --text: #e6eef6;
+                --muted: #9aa4ad;
+                --card-bg: #0f1720;
+                --card-border: #1f2a33;
+                --primary: #6ea8ff;
+                --success: #36d07a;
+                --danger: #ff6b6b;
+                --feedback-bg: #071322;
+                --shadow: 0 2px 6px rgba(0,0,0,0.6);
             }
+            body, .exam-take-container { background: var(--bg); color: var(--text); }
+        }
 
-            .flex-1 {
-                flex: 1;
-            }
+        /* Manual override: add `dark-mode` to body or a parent to force dark theme */
+        body.dark-mode, .dark-mode { color: var(--text); background: var(--bg); }
 
-            .flex-8 {
-                flex: 8;
-            }
+        /* Layout and responsiveness */
+        .exam-take-container { max-width: 980px; margin: 0 auto; padding: 1rem; }
+        .no-select { user-select: none; }
 
-            .selected {
-                border: 1px solid purple;
-            }
+        .flex-container { display: flex; align-items: center; gap: 0.75rem; }
+        .flex-1 { flex: 1; }
+        .flex-8 { flex: 8; }
 
-            .answer-true::before {
-                content: "[Correct answer: True]";
-                color: darkgray;
-                float: right;
-                opacity: 50%;
+        /* Answer items styled as cards */
+        .physics-answer-list { list-style: none; padding: 0; margin: 0; }
+        .physics-answer-list li {
+            display: flex;
+            align-items: center;
+            gap: 0.75rem;
+            padding: 0.75rem 1rem;
+            border: 1px solid var(--card-border);
+            border-radius: 0.5rem;
+            background: var(--card-bg);
+            box-shadow: var(--shadow);
+            color: var(--text);
+        }
+        .physics-answer-list li + li { margin-top: 0.75rem; }
 
-            }
-            .answer-false::before {
-                content: "[Correct answer: False]";
-                color: darkgray;
-                float: right;
-                opacity: 50%;
+        .question-text { margin: 0; }
 
-            }
-            .answer-correct {
-                border: 1px dotted darkgreen;
-            }
-            .answer-incorrect {
-                border: 1px dashed darkred;
-            }
+        /* Post-input badge (shows True/False text next to inputs) */
+        .postinput { display: inline-block; min-width: 2.5rem; text-align: left; }
+        .physics-answer-list input:checked + .postinput::after {
+            margin-left: 0.6rem; content: 'True'; color: var(--success); font-weight: 600;
+        }
+        .physics-answer-list input:not(:checked) + .postinput::after {
+            margin-left: 0.6rem; content: 'False'; color: var(--muted); font-weight: 600;
+        }
 
-            .physics-answer-list li {
-                margin-top: 5px;
-                margin-bottom: 5px;
-            }
+        /* Correctness badges and highlights */
+        .answer-true::before,
+        .answer-false::before {
+            display: inline-block;
+            margin-left: auto;
+            padding: 0.18rem 0.5rem;
+            border-radius: 0.25rem;
+            font-size: 0.75rem;
+            font-weight: 600;
+            color: #fff;
+        }
+        .answer-true::before { content: "Correct"; background: var(--success); }
+        .answer-false::before { content: "Not correct"; background: var(--muted); }
 
-            .physics-answer-list input:checked+.postinput::after {
-                margin-left: 20px;
-                content: "True"
-            }
-            .physics-answer-list input:not(:checked)+.postinput::after {
-                margin-left: 20px;
-                content: "False"
-            }
+        .answer-correct { background-color: rgba(25,135,84,0.06); border-color: rgba(25,135,84,0.18); }
+        .answer-incorrect { background-color: rgba(220,53,69,0.04); border-color: rgba(220,53,69,0.12); }
 
-            .feedback {
-                padding: 20px;
-                display: block;
-            }
+        /* Feedback block */
+        .feedback {
+            padding: 0.85rem 1rem;
+            display: block;
+            margin-top: 0.5rem;
+            border-left: 4px solid var(--primary);
+            background: var(--feedback-bg);
+            color: var(--text);
+            border-radius: 0.25rem;
+        }
 
-        </style>
-    {% endblock %}
+        /* Question menu (jump buttons) */
+        #menu-list { margin-top: 1rem; display:flex; flex-wrap:wrap; gap:0.4rem; }
+        button.question-menu-item {
+            padding: 0.35rem 0.6rem;
+            border-radius: 0.35rem;
+            border: 1px solid var(--card-border);
+            background: var(--card-bg);
+            color: var(--text);
+            cursor: pointer;
+            font-size: 0.9rem;
+        }
+        button.question-menu-item.current-question {
+            background: var(--primary); color: #fff; border-color: var(--primary);
+            box-shadow: 0 1px 3px rgba(13,110,253,0.18);
+        }
+
+        /* Control buttons spacing */
+        .mt-2 .save { margin-right: 0.5rem; }
+        .mt-2 .save.btn.btn-default { background: var(--feedback-bg); border: 1px solid var(--card-border); color: var(--text); }
+
+        /* Small helper: make labels wrap nicely on small screens */
+        @media (max-width: 576px) {
+            .flex-container { flex-direction: column; align-items: stretch; }
+            .postinput { margin-top: 0.4rem; }
+            .physics-answer-list li { padding: 0.6rem; }
+        }
+    </style>
+{% endblock %}
diff --git a/physics/views.py b/physics/views.py
index e08e45ac..d8149859 100644
--- a/physics/views.py
+++ b/physics/views.py
@@ -188,7 +188,16 @@ def exam_take_overview(request, pk, cid=None, passcode=None):
     )
 
 
-def exam_take(request, pk: int, sk: int, cid: str| None = None, passcode: str| None = None):
+def exam_take(request, pk: int, sk: int, cid: str | None = None, passcode: str | None = None):
+    """Display and handle answers for a single question in a physics exam.
+
+    Behavior and invariants:
+    - Uses a single canonical list of questions (from `exam.get_questions()`) for
+      selection, navigation and sizing so indexes cannot drift.
+    - Validates the incoming question index and raises 404 for invalid values.
+    - Supports candidate (cid) and logged-in user answers.
+    """
+
     exam = get_object_or_404(Exam, pk=pk)
 
     if not exam.active:
@@ -198,25 +207,23 @@ def exam_take(request, pk: int, sk: int, cid: str| None = None, passcode: str| N
 
     cid_user_exam = exam.get_or_create_cid_user_exam(cid=cid, user_user=request.user)
 
-    # Use a single, canonical list of questions for all indexing/navigation
-    # operations to avoid mismatches between different querysets or orderings.
+    # Canonical questions list for consistent indexing/navigation
     questions = list(exam.get_questions())
 
-    # Validate sk and select the current question
+    # Normalize and validate provided index
     try:
-        sk = int(sk)
+        index = int(sk)
     except Exception:
         raise Http404("Invalid question index")
 
-    if sk < 0 or sk >= len(questions):
+    if index < 0 or index >= len(questions):
         raise Http404("Question not found in exam")
 
-    question = questions[sk]
-
+    question = questions[index]
     exam_length = len(questions)
 
-    # Since we're using the canonical list, the positional index is sk.
-    pos = sk
+    # local numeric position equals the validated index
+    pos = index
 
     if cid is not None:
         answer = question.cid_user_answers.filter(cid=cid, exam=exam).first()