From a4e92d1f40e89e4d69bdb8b8ef68a58022aa1f7b Mon Sep 17 00:00:00 2001 From: Yamen Ahmad Date: Mon, 20 Apr 2026 17:14:39 +0400 Subject: [PATCH] fix(qa): approval queue, rubric UX, structure enforcement, upload/publish limits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the QA notes on the end-to-end approval flow. Highlights: Backend - encoach_ai/generation_submit: create encoach.approval.request on submit so submitted exams actually reach the approver queue (previously only the workflow id was stored on the exam, leaving approvers with an empty inbox). Initial exam status flips to pending_approval instead of draft. - encoach_exam_template/approval_workflows: * /api/approval-users filters out students (user_type='student', op_student_id set, share=True) so the approver dropdown only lists staff. * _ser_request enriches requests with target_name / target_status and the current stage approver for UI badges. * list_requests supports mine=1 / requester=1 so approvers only see queue items awaiting their action. * approve_request / reject_request now transition the underlying encoach.exam.custom to published / rejected on final approval. - encoach.exam.custom.status: add pending_approval and rejected states to match the approval workflow transitions. Frontend UX - GenerationPage: rubric field shows "Auto-graded — no rubric" for Listening and Reading (rubrics only apply to Writing / Speaking). Divider dropdowns now carry explicit help text explaining None / Line / Space / Page Break and where to write prompts. Selecting an official exam structure auto-populates the required tasks/sections/parts, the delete button is hidden for essential tasks, and submit is blocked if the user dropped below the structure's required count. - RubricsPage: "Add Criterion" is now a dropdown with IELTS-specific presets (Task Achievement, Coherence & Cohesion, Fluency & Coherence, …) keyed off the selected skill, plus a "Custom (blank row)" fallback. - AdminCourses: added tooltips and helper copy clarifying Difficulty vs CEFR Level in the Create Course dialog. - CustomExamCreate: validate the whole form before Save/Publish, surface backend error messages (413/504/401) instead of a generic toast, read the exam id from the correct response field, and avoid marking Publish as failed when only the optional Save-as-Template step errors. - ResourceManager / TeacherLibrary: upload toast now shows a concrete reason (HTTP 413 / 504 / 401) instead of a generic "Upload failed". Config (504 / 413 / resource upload fixes) - odoo.conf + odoo-docker.conf: raise limit_time_cpu to 600s, limit_time_real to 900s, limit_request to 128 MB, and memory caps so /api/exam/generation/submit and multipart resource uploads stop hitting 504 / 413 during QA. - frontend/Dockerfile (nginx): add client_max_body_size 128m, bump proxy_read_timeout / proxy_send_timeout to 900s, and disable request buffering so large AI/resource payloads stream through the proxy. Made-with: Cursor --- Dockerfile | 14 ++- src/pages/GenerationPage.tsx | 177 +++++++++++++++++++++++---- src/pages/RubricsPage.tsx | 160 ++++++++++++++++++++---- src/pages/admin/AdminCourses.tsx | 22 +++- src/pages/admin/CustomExamCreate.tsx | 63 ++++++++-- src/pages/admin/ResourceManager.tsx | 13 +- src/pages/teacher/TeacherLibrary.tsx | 10 +- src/types/custom-exam.ts | 2 + 8 files changed, 399 insertions(+), 62 deletions(-) diff --git a/Dockerfile b/Dockerfile index 5545dd4..cae190b 100644 --- a/Dockerfile +++ b/Dockerfile @@ -18,13 +18,25 @@ server { root /usr/share/nginx/html; index index.html; + # Default nginx body cap is 1 MB which causes HTTP 413 on: + # * resource uploads (PDF / audio / video) + # * /api/exam/generation/submit (whole exam with generated questions) + # * /api/approval-requests (attachments) + # Match Odoo's raised limit in odoo-docker.conf (128 MB). + client_max_body_size 128m; + client_body_buffer_size 1m; + location /api/ { proxy_pass http://backend:8069/api/; proxy_set_header Host $host; proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header X-Forwarded-Proto $scheme; - proxy_read_timeout 300s; + # OpenAI streaming calls + multi-module AI generation can run >3 min. + # Keep read/send timeouts above Odoo's limit_time_real (900s). + proxy_read_timeout 900s; + proxy_send_timeout 900s; + proxy_request_buffering off; } location / { diff --git a/src/pages/GenerationPage.tsx b/src/pages/GenerationPage.tsx index 1e8526e..28a260e 100644 --- a/src/pages/GenerationPage.tsx +++ b/src/pages/GenerationPage.tsx @@ -709,8 +709,51 @@ export default function GenerationPage() { // eslint-disable-next-line react-hooks/exhaustive-deps }, [activeModule, moduleStates]); + // Compute the minimum required counts from the selected exam structure + // so the submit mutation can block under-fulfilled modules instead of + // silently dropping tasks/sections (QA: "if structure has Task 1 + Task 2 + // the user should not be able to submit with only Task 1"). + const requiredWritingTaskCount = (() => { + const cfg = selectedStructureConfig; + if (!cfg?.writing) return 0; + return Math.max( + cfg.writing.tasks?.length ?? 0, + cfg.writing.task2 ? 2 : cfg.writing.task1 ? 1 : 0, + ); + })(); + const requiredListeningPartCount = selectedStructureConfig?.listening?.parts?.length ?? 0; + const requiredReadingPassageCount = selectedStructureConfig?.reading?.passages?.length ?? 0; + const requiredSpeakingPartCount = selectedStructureConfig?.speaking?.parts?.length ?? 0; + const submitMut = useMutation({ mutationFn: (skipApproval: boolean) => { + // Enforce structure-defined task/section/part counts before submission. + if (examMode === "official" && selectedStructure) { + const w = getModuleState("writing"); + if (selectedModules.has("writing") && requiredWritingTaskCount > 0 && w.writingTasks.length < requiredWritingTaskCount) { + throw new Error( + `This structure defines ${requiredWritingTaskCount} writing tasks — you currently have ${w.writingTasks.length}. Please add the missing task before submitting.`, + ); + } + const l = getModuleState("listening"); + if (selectedModules.has("listening") && requiredListeningPartCount > 0 && l.listeningSections.length < requiredListeningPartCount) { + throw new Error( + `This structure defines ${requiredListeningPartCount} listening parts — you currently have ${l.listeningSections.length}.`, + ); + } + const r = getModuleState("reading"); + if (selectedModules.has("reading") && requiredReadingPassageCount > 0 && r.passages.length < requiredReadingPassageCount) { + throw new Error( + `This structure defines ${requiredReadingPassageCount} reading passages — you currently have ${r.passages.length}.`, + ); + } + const sp = getModuleState("speaking"); + if (selectedModules.has("speaking") && requiredSpeakingPartCount > 0 && sp.speakingParts.length < requiredSpeakingPartCount) { + throw new Error( + `This structure defines ${requiredSpeakingPartCount} speaking parts — you currently have ${sp.speakingParts.length}.`, + ); + } + } const modulesPayload: Record = {}; for (const mod of selectedModules) { const st = getModuleState(mod); @@ -773,6 +816,11 @@ export default function GenerationPage() { const renderCommonConfig = (mod: ModuleKey) => { const st = getModuleState(mod); + // Rubrics only apply to subjective skills (Writing & Speaking). + // Listening & Reading are auto-graded against a correct_answer, so + // surfacing the rubric picker here is misleading — QA asked to either + // grey it out or mark it "Not applicable". + const rubricApplies = mod === "writing" || mod === "speaking"; return (
@@ -781,30 +829,44 @@ export default function GenerationPage() {
{examMode === "official" && (
- - + + {rubricApplies ? ( + + ) : ( +
+ Auto-graded — no rubric +
+ )}
)}
@@ -1202,6 +1264,12 @@ export default function GenerationPage() { Page Break +

+ Controls how this section is visually separated from the next in the student view: None — no separator; Line — horizontal rule; Space — extra vertical gap; Page Break — section starts on a new page (paginated delivery only). +

+

+ Write instructions in the box above the audio (or the Generate Instructions card). Use the imperative voice, e.g. “Listen to the conversation and answer the questions below.” +

@@ -1407,7 +1475,7 @@ export default function GenerationPage() {
- {st.writingTasks.length > 1 && ( + {st.writingTasks.length > Math.max(1, requiredWritingTaskCount) && ( )}
+ {requiredWritingTaskCount > 0 && st.writingTasks.length < requiredWritingTaskCount && ( +

+ This structure requires {requiredWritingTaskCount} tasks. Add the missing task to submit. +

+ )}
+

Click Generate with AI to create criteria automatically, or pick a preset below.

+ + + + + + {(CRITERION_PRESETS[form.skill] ?? []).map((group) => ( +
+ {group.label} + {group.items.map((item) => ( + + setForm((f) => ({ + ...f, + criteria: [...f.criteria, emptyCriterion(f.levels, item.name, item.weight)], + })) + } + > + {item.name} + {item.weight}% + + ))} + +
+ ))} + setForm((f) => ({ ...f, criteria: [...f.criteria, emptyCriterion(f.levels)] }))} + > + Custom (blank row) + + +
) : (
@@ -507,15 +598,40 @@ export default function RubricsPage() {
))} - + + + + + + {(CRITERION_PRESETS[form.skill] ?? []).map((group) => ( +
+ {group.label} + {group.items.map((item) => ( + + setForm((f) => ({ + ...f, + criteria: [...f.criteria, emptyCriterion(f.levels, item.name, item.weight)], + })) + } + > + {item.name} + {item.weight}% + + ))} + +
+ ))} + setForm((f) => ({ ...f, criteria: [...f.criteria, emptyCriterion(f.levels)] }))} + > + Custom (blank row) + + + )} diff --git a/src/pages/admin/AdminCourses.tsx b/src/pages/admin/AdminCourses.tsx index c6e1947..66d83ba 100644 --- a/src/pages/admin/AdminCourses.tsx +++ b/src/pages/admin/AdminCourses.tsx @@ -253,7 +253,13 @@ function CourseFormDialog({ />
- + +

+ A plain-language tag (Beginner / Intermediate / Advanced). +

- + +

+ Standard proficiency band (A1-C2); drives placement & rubrics. +

diff --git a/src/pages/admin/CustomExamCreate.tsx b/src/pages/admin/CustomExamCreate.tsx index 811c244..7828d42 100644 --- a/src/pages/admin/CustomExamCreate.tsx +++ b/src/pages/admin/CustomExamCreate.tsx @@ -114,7 +114,11 @@ export default function CustomExamCreate() { total_time_min: v.total_time_min, pass_threshold: v.pass_threshold, results_release_mode: v.results_release_mode, + // Backend model field is `randomize_questions`; the controller reads both + // `randomize` and `randomize_questions`, but prefer the real field name + // so future serialisation roundtrips don't silently drop the flag. randomize: v.randomize, + randomize_questions: v.randomize, sections: v.sections.map((sec, i) => ({ title: sec.title, skill: sec.skill, @@ -124,26 +128,64 @@ export default function CustomExamCreate() { sequence: i, question_ids: sec.question_ids, })), - }; + } as CustomExamCreateRequest; + }; + + // Extract the id the backend actually returns. `POST /api/exam/custom/create` + // returns the serialised exam dict with `id`, not `exam_id`, so the old + // `raw.exam_id` read was always undefined (preventing navigation after publish). + const pickExamId = (res: unknown): number | undefined => { + const raw = res as { id?: number; exam_id?: number; data?: { id?: number; exam_id?: number } }; + return raw?.id ?? raw?.exam_id ?? raw?.data?.id ?? raw?.data?.exam_id; + }; + + const describeError = (err: unknown, fallback: string): string => { + const e = err as { status?: number; message?: string }; + if (e?.status === 413) return "Payload too large (HTTP 413). Please reduce the number of questions per section and try again."; + if (e?.status === 504) return "Server took too long to respond (HTTP 504). Please retry in a moment."; + if (e?.status === 401) return "Session expired — please sign in again."; + if (e?.message) return `${fallback}: ${e.message}`; + return fallback; }; const onSaveDraft = async () => { + // Validate everything before firing the request — previously the Save + // button POSTed with whatever was in the form and silently swallowed the + // 400/500 as a generic toast. + const ok = await form.trigger(); + if (!ok) { + toast.error("Please fix the highlighted fields before saving."); + return; + } try { const payload = buildPayload(); await createExam.mutateAsync(payload); toast.success("Draft saved."); - } catch { - toast.error("Could not save."); + } catch (err) { + toast.error(describeError(err, "Could not save draft")); } }; const onPublish = async () => { + const ok = await form.trigger(); + if (!ok) { + toast.error("Please fix the highlighted fields before publishing."); + return; + } + let examId: number | undefined; try { const payload = buildPayload(); const res = await createExam.mutateAsync(payload); - const raw = res as { exam_id?: number; data?: { exam_id?: number } }; - const id = raw.exam_id ?? raw.data?.exam_id; - if (form.getValues("save_as_template")) { + examId = pickExamId(res); + } catch (err) { + toast.error(describeError(err, "Publish failed")); + return; + } + + // Template save is additive — if it fails, the exam has already been + // created. Don't show a scary "Publish failed" toast for a template issue. + if (form.getValues("save_as_template")) { + try { const name = form.getValues("template_name")?.trim() || form.getValues("title"); await saveTemplate.mutateAsync({ name, @@ -151,12 +193,13 @@ export default function CustomExamCreate() { editable: true, type: "custom", }); + } catch (err) { + toast.warning(describeError(err, "Exam saved, but template save failed")); } - toast.success("Exam published."); - if (id != null) navigate(`/admin/exam`); - } catch { - toast.error("Publish failed."); } + + toast.success("Exam published."); + if (examId != null) navigate(`/admin/exam`); }; return ( diff --git a/src/pages/admin/ResourceManager.tsx b/src/pages/admin/ResourceManager.tsx index b14bf3e..2e3d404 100644 --- a/src/pages/admin/ResourceManager.tsx +++ b/src/pages/admin/ResourceManager.tsx @@ -274,7 +274,18 @@ export default function ResourceManager() { const uploadMutation = useMutation({ mutationFn: (formData: FormData) => resourcesService.upload(formData), onSuccess: () => { qc.invalidateQueries({ queryKey: ["resources"] }); toast({ title: "Resource uploaded" }); setShowUpload(false); setUploadTagIds([]); setUploadSubjectId("none"); setUploadDomainId("none"); setUploadTopicIds([]); setUploadObjectiveIds([]); }, - onError: () => toast({ title: "Error", description: "Upload failed", variant: "destructive" }), + onError: (err: unknown) => { + // Surface status / server message so users can tell 413 (file too big) + // from 401 (session expired) from 500 (server error) without opening + // devtools. Default "Upload failed" hid real failures during QA. + const e = err as { status?: number; message?: string }; + const status = e?.status; + let description = e?.message || "Upload failed"; + if (status === 413) description = "File is too large for the server (HTTP 413). Ask an admin to raise nginx/odoo upload limits."; + else if (status === 401) description = "Session expired — please sign in again."; + else if (status === 504) description = "Server timed out while saving the file (HTTP 504). Try a smaller file or retry."; + toast({ title: "Upload failed", description, variant: "destructive" }); + }, }); const createTagMutation = useMutation({ diff --git a/src/pages/teacher/TeacherLibrary.tsx b/src/pages/teacher/TeacherLibrary.tsx index d3385a3..78303ff 100644 --- a/src/pages/teacher/TeacherLibrary.tsx +++ b/src/pages/teacher/TeacherLibrary.tsx @@ -116,7 +116,15 @@ export default function TeacherLibrary() { setUploadTopicIds([]); setUploadObjectiveIds([]); }, - onError: () => toast({ title: "Error", description: "Upload failed", variant: "destructive" }), + onError: (err: unknown) => { + const e = err as { status?: number; message?: string }; + const status = e?.status; + let description = e?.message || "Upload failed"; + if (status === 413) description = "File is too large for the server (HTTP 413). Try a smaller file or ask an admin to raise the upload limit."; + else if (status === 401) description = "Session expired — please sign in again."; + else if (status === 504) description = "Server timed out while saving the file (HTTP 504). Try a smaller file or retry."; + toast({ title: "Upload failed", description, variant: "destructive" }); + }, }); const deleteMutation = useMutation({ diff --git a/src/types/custom-exam.ts b/src/types/custom-exam.ts index 57e9f49..2f7219b 100644 --- a/src/types/custom-exam.ts +++ b/src/types/custom-exam.ts @@ -18,6 +18,8 @@ export interface CustomExamCreateRequest { pass_threshold?: number; results_release_mode: "auto" | "manual_approval"; randomize: boolean; + // Backend model field; also accepted by the /api/exam/custom/create controller. + randomize_questions?: boolean; sections: CustomSection[]; }