📝 CodeRabbit Chat: Add retry and unsaved notice for question list ordering

assets/controllers/bo/question_list_controller.js

@@ -1,18 +1,19 @@
 import {Controller} from '@hotwired/stimulus';
+import {Modal} from 'bootstrap';
+
+const RETRY_DELAY_MS = 1000;
 
 export default class extends Controller {
-  static targets = ['list', 'item', 'status'];
+  static targets = ['list', 'item', 'status', 'noticeModal'];
   static values = {
     reorderUrl: String,
     csrf: String,
     canModify: Boolean,
     savedLabel: String,
     errorLabel: String,
-    errorHint: String,
   };
 
   connect() {
-    this._locked = false;
     if (this.canModifyValue) {
       this._setupDrag();
     }
@@ -26,6 +27,10 @@ export default class extends Controller {
       handle.setAttribute('draggable', 'true');
 
       handle.addEventListener('dragstart', (e) => {
+        if (this._locked) {
+          e.preventDefault();
+          return;
+        }
         this._dragging = el;
         e.dataTransfer.effectAllowed = 'move';
         setTimeout(() => el.classList.add('opacity-50'), 0);
@@ -68,7 +73,7 @@ export default class extends Controller {
 
     this.listTarget.addEventListener('drop', async (e) => {
       e.preventDefault();
-      if (!this._dragging || !this._placeholder || this._locked) return;
+      if (!this._dragging || !this._placeholder) return;
       this.listTarget.insertBefore(this._dragging, this._placeholder);
       this._removePlaceholder();
       await this._persistOrder();
@@ -109,26 +114,40 @@ export default class extends Controller {
       if (numberEl) numberEl.textContent = String(i + 1);
     });
 
-    for (let attempt = 0; attempt < 2; attempt++) {
+    const attempt = async () => {
+      const res = await fetch(this.reorderUrlValue, {method: 'POST', body: params});
+      if (!res.ok) {
+        throw new Error(`Unexpected response status: ${res.status}`);
+      }
+    };
+
+    try {
+      await attempt();
+    } catch {
+      await new Promise(resolve => setTimeout(resolve, RETRY_DELAY_MS));
       try {
-        const res = await fetch(this.reorderUrlValue, {method: 'POST', body: params});
-        if (res.ok) {
-          this._setStatus('saved');
-          return;
-        }
+        await attempt();
       } catch {
-        // network error — retry on first attempt
+        this._setStatus('error');
+        this._lockReordering();
+        return;
       }
     }
 
-    this._locked = true;
-    this._setStatus('error');
+    this._setStatus('saved');
+  }
 
-    const alert = document.createElement('div');
-    alert.className = 'alert alert-danger alert-dismissible mt-3';
-    alert.setAttribute('role', 'alert');
-    const hint = this.errorHintValue || 'Refresh the page to try again.';
-    alert.innerHTML = `${this.errorLabelValue || 'Error saving order'} &mdash; ${hint} <button type="button" class="btn-close" data-bs-dismiss="alert" aria-label="Close"></button>`;
-    this.listTarget.after(alert);
+  _lockReordering() {
+    this._locked = true;
+    this.itemTargets.forEach(el => {
+      const handle = el.querySelector('[data-drag-handle]');
+      if (handle) {
+        handle.removeAttribute('draggable');
+        handle.classList.add('opacity-25', 'pe-none');
+      }
+    });
+    if (this.hasNoticeModalTarget) {
+      Modal.getOrCreateInstance(this.noticeModalTarget).show();
+    }
   }
 }

src/Controller/Backoffice/QuestionBankController.php

@@ -90,10 +90,7 @@ class QuestionBankController extends AbstractController
 
         $isTurboFrame = $request->headers->has('Turbo-Frame');
 
-        $form = $this->createForm(BankQuestionFormType::class, $bankQuestion, [
-            'season' => $season,
-            'action' => $this->generateUrl('tvdt_backoffice_question_bank_new', ['seasonCode' => $season->seasonCode]),
-        ]);
+        $form = $this->createForm(BankQuestionFormType::class, $bankQuestion, ['season' => $season]);
         $form->handleRequest($request);
 
         if ($form->isSubmitted() && $form->isValid()) {
@@ -141,13 +138,7 @@ class QuestionBankController extends AbstractController
 
         $isTurboFrame = $request->headers->has('Turbo-Frame');
 
-        $form = $this->createForm(BankQuestionFormType::class, $bankQuestion, [
-            'season' => $season,
-            'action' => $this->generateUrl('tvdt_backoffice_question_bank_edit', [
-                'seasonCode' => $season->seasonCode,
-                'bankQuestion' => $bankQuestion->id,
-            ]),
-        ]);
+        $form = $this->createForm(BankQuestionFormType::class, $bankQuestion, ['season' => $season]);
         $form->handleRequest($request);
 
         if ($form->isSubmitted() && $form->isValid()) {

src/Controller/Backoffice/QuizQuestionController.php

@@ -46,13 +46,7 @@ class QuizQuestionController extends AbstractController
 
         $isTurboFrame = $request->headers->has('Turbo-Frame');
 
-        $form = $this->createForm(QuestionFormType::class, $question, [
-            'action' => $this->generateUrl('tvdt_backoffice_quiz_question_edit', [
-                'seasonCode' => $season->seasonCode,
-                'quiz' => $quiz->id,
-                'question' => $question->id,
-            ]),
-        ]);
+        $form = $this->createForm(QuestionFormType::class, $question);
         $form->handleRequest($request);
 
         if ($form->isSubmitted() && $form->isValid()) {
@@ -89,7 +83,6 @@ class QuizQuestionController extends AbstractController
         return $response;
     }
 
-    #[IsGranted(SeasonVoter::EDIT, subject: 'season')]
     #[Route(
         '/backoffice/season/{seasonCode:season}/quiz/{quiz}/question/{question}/view',
         name: 'tvdt_backoffice_quiz_question_view',
@@ -134,10 +127,6 @@ class QuizQuestionController extends AbstractController
             }
         }
 
-        if (\count(array_unique($ordering)) !== \count($questionsById)) {
-            throw new BadRequestHttpException('Ordering must include every question exactly once');
-        }
-
         $position = 1;
         foreach ($ordering as $questionId) {
             $questionsById[$questionId]->ordering = $position++;

templates/backoffice/partials/answer_row.html.twig

@@ -4,7 +4,7 @@
         <span class="text-muted" data-drag-handle style="cursor: grab" title="{{ 'Drag to reorder'|trans }}"><i class="bi bi-grip-vertical"></i></span>
         <div class="flex-grow-1">{{ form_widget(answerForm.text) }}</div>
         <div class="d-none">{{ form_widget(answerForm.isRightAnswer) }}</div>
-        <button type="button"
+        <button type="button" tabindex="-1"
                 class="btn btn-sm {{ answerForm.isRightAnswer.vars.checked ? 'btn-success' : 'btn-danger' }}"
                 title="{{ 'Toggle correct answer'|trans }}"
                 onclick="var cb=this.closest('[data-collection-item]').querySelector('input[type=checkbox]');cb.checked=!cb.checked;this.classList.toggle('btn-success',cb.checked);this.classList.toggle('btn-danger',!cb.checked);this.querySelector('i').className=cb.checked?'bi bi-check-lg':'bi bi-x-lg'">

templates/backoffice/quiz/tab_overview.html.twig

@@ -91,8 +91,7 @@
          data-bo--question-list-csrf-value="{{ csrf_token('question_reorder') }}"
          data-bo--question-list-can-modify-value="{{ is_granted('QUIZ_MODIFY_CONTENT', quiz) ? 'true' : 'false' }}"
          data-bo--question-list-saved-label-value="{{ 'Order saved'|trans }}"
-         data-bo--question-list-error-label-value="{{ 'Error saving order'|trans }}"
-         data-bo--question-list-error-hint-value="{{ 'Refresh the page to try again.'|trans }}">
+         data-bo--question-list-error-label-value="{{ 'Error saving order'|trans }}">
 
         <h4 class="mb-3 d-flex align-items-center gap-2">
             {{ 'Questions'|trans }}
@@ -136,7 +135,7 @@
                     </div>
                 </div>
             {% else %}
-                {{ 'No questions have been added to this quiz yet.'|trans }}
+                {{ 'EMPTY'|trans }}
             {% endfor %}
         </div>
 
@@ -153,6 +152,25 @@
                 </div>
             </div>
         </div>
+
+        <div class="modal fade" tabindex="-1"
+             data-bo--question-list-target="noticeModal"
+             aria-labelledby="questionReorderErrorModalLabel" aria-hidden="true">
+            <div class="modal-dialog">
+                <div class="modal-content">
+                    <div class="modal-header">
+                        <h5 class="modal-title" id="questionReorderErrorModalLabel">{{ 'Could not save order'|trans }}</h5>
+                        <button type="button" class="btn-close" data-bs-dismiss="modal" aria-label="Close"></button>
+                    </div>
+                    <div class="modal-body">
+                        {{ 'The new question order could not be saved. Reordering has been disabled until you refresh the page.'|trans }}
+                    </div>
+                    <div class="modal-footer">
+                        <button type="button" class="btn btn-secondary" data-bs-dismiss="modal">{{ 'Close'|trans }}</button>
+                    </div>
+                </div>
+            </div>
+        </div>
     </div>
 
     {{ _self.confirm_modal(

templates/backoffice/season/tab_question_bank.html.twig

@@ -129,7 +129,6 @@
                             <button type="button" class="btn btn-outline-secondary"
                                     data-action="click->bo--modal#open"
                                     data-src="{{ path('tvdt_backoffice_question_bank_edit', {seasonCode: season.seasonCode, bankQuestion: bankQuestion.id}) }}"
-                                    data-modal-title="{{ 'Edit question'|trans }}"
                                     title="{{ 'Edit'|trans }}"><i class="bi bi-pencil"></i></button>
                             <button type="button" class="btn btn-outline-danger" data-bs-toggle="modal"
                                     data-bs-target="#deleteBankQuestion-{{ bankQuestion.id }}"

translations/messages+intl-icu.nl.xliff

@@ -181,6 +181,10 @@
         <source>Could not find candidate with name {name} in elimination.</source>
         <target>Kon geen kandidaat vinden met de naam {name} in de eliminatie</target>
       </trans-unit>
+      <trans-unit id="Cn0sav3" resname="Could not save order">
+        <source>Could not save order</source>
+        <target>Kon volgorde niet opslaan</target>
+      </trans-unit>
       <trans-unit id="0DvmToq" resname="Create a season">
         <source>Create a season</source>
         <target>Maak een seizoen aan</target>
@@ -259,7 +263,7 @@
       </trans-unit>
       <trans-unit id="NXU7HO." resname="Error saving order">
         <source>Error saving order</source>
-        <target>Fout bij het opslaan van de volgorde</target>
+        <target>Fout bij opslaan volgorde</target>
       </trans-unit>
       <trans-unit id="bgWPQMg" resname="Export to XLSX">
         <source>Export to XLSX</source>
@@ -423,7 +427,7 @@
       </trans-unit>
       <trans-unit id="PywqOf4" resname="Owner(s)">
         <source>Owner(s)</source>
-        <target>Eigenaar/Eigenaren</target>
+        <target>Eigena(a)r(en)</target>
       </trans-unit>
       <trans-unit id="GqmFSHc" resname="Password">
         <source>Password</source>
@@ -649,6 +653,10 @@
         <source>Sync latest changes to this quiz</source>
         <target>Laatste wijzigingen synchroniseren naar deze quiz</target>
       </trans-unit>
+      <trans-unit id="F3q1oXk" resname="The new question order could not be saved. Reordering has been disabled until you refresh the page.">
+        <source>The new question order could not be saved. Reordering has been disabled until you refresh the page.</source>
+        <target>De nieuwe volgorde van de vragen kon niet worden opgeslagen. Herordenen is uitgeschakeld totdat u de pagina vernieuwt.</target>
+      </trans-unit>
       <trans-unit id="_z4el3Z" resname="The password fields must match.">
         <source>The password fields must match.</source>
         <target>De wachtwoorden moeten overeen komen.</target>