(SP: 3) [Frontend] Quiz Admin: Create & Publish Quizzes (JSON upload)#353
Merged
ViktorSvertoka merged 6 commits intodevelopfrom Feb 22, 2026
Merged
(SP: 3) [Frontend] Quiz Admin: Create & Publish Quizzes (JSON upload)#353ViktorSvertoka merged 6 commits intodevelopfrom
ViktorSvertoka merged 6 commits intodevelopfrom
Conversation
Add quiz creation via JSON upload, publish/unpublish workflow with validation, metadata editing, difficulty management, and quiz deletion. - Create quiz page with category dropdown, locale tabs, JSON upload - Status controls with confirmation modals (Mark as Ready / Revert to Draft) - Publish validation: all 3 locales complete for questions + answers - Metadata editor: title, description (per locale), time limit - Difficulty dropdown per question (beginner/medium/advanced) - Upload more questions to draft quizzes - Delete draft quizzes and individual questions - Sidebar: New Quiz link - Migration 0015: status column (draft/ready)
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
✅ Deploy Preview for develop-devlovers ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
📝 WalkthroughWalkthroughA comprehensive admin quiz management system is implemented, enabling creation of quizzes with draft/ready statuses, editing metadata and questions, uploading additional questions, publishing validation, and deletion with proper constraints. Backend API routes handle CRUD operations with admin authentication, CSRF protection, and cache invalidation. Database schema extended with quiz status tracking. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client (Browser)
participant Page as New Quiz Page
participant API as /api/admin/quiz
participant DB as Database
Client->>Page: Click "New Quiz"
activate Page
Page->>DB: getAdminCategoryList()
DB-->>Page: categories[]
Page->>Page: Generate CSRF tokens
Page-->>Client: Render CreateQuizForm
deactivate Page
Client->>Client: Fill form (category, title, slug, questions)
Client->>API: POST /api/admin/quiz {categories, translations, questions}
activate API
API->>API: Validate CSRF & auth
API->>API: Validate payload schema
API->>DB: Check slug uniqueness
DB-->>API: ✓ unique
API->>DB: INSERT quiz
DB-->>API: quizId
API->>DB: INSERT quiz_translations (en, uk, pl)
API->>DB: INSERT questions + translations
API->>DB: INSERT answers + translations
API-->>Client: {quizId}
deactivate API
Client->>Client: Navigate to quiz edit page
sequenceDiagram
participant Client as Client (Browser)
participant EditPage as Quiz Edit Page
participant StatusControls as StatusControls Component
participant API as /api/admin/quiz/{id}
participant Validator as validateQuizForPublish
participant DB as Database
Client->>EditPage: View quiz (status=draft)
activate EditPage
EditPage->>EditPage: isDraft = status === 'draft'
EditPage-->>Client: Render QuizStatusControls + metadata
deactivate EditPage
Client->>StatusControls: Click "Mark as Ready"
activate StatusControls
StatusControls-->>Client: Show confirmation modal
Client->>StatusControls: Confirm
StatusControls->>API: PATCH {status: 'ready'}
deactivate StatusControls
activate API
API->>Validator: validateQuizForPublish(quizId)
activate Validator
Validator->>DB: Fetch translations
Validator->>DB: Fetch questions
Validator->>DB: Fetch question content
Validator->>DB: Fetch answers & translations
Validator-->>API: errors[] or empty
deactivate Validator
alt Validation fails
API-->>Client: 422 {errors}
else Validation passes
API->>DB: UPDATE quiz SET status='ready'
API->>DB: invalidateQuizCache()
API-->>Client: 200 {quiz}
Client->>Client: router.refresh()
end
deactivate API
sequenceDiagram
participant Client as Client (Browser)
participant DraftQuiz as Draft Quiz Edit Page
participant UploadComponent as UploadMoreQuestions
participant API as /api/admin/quiz/{id}/questions
participant DB as Database
Client->>DraftQuiz: View draft quiz
activate DraftQuiz
DraftQuiz->>DraftQuiz: isDraft = status === 'draft'
DraftQuiz-->>Client: Render UploadMoreQuestions (if isDraft)
deactivate DraftQuiz
Client->>UploadComponent: Select JSON file with questions
activate UploadComponent
UploadComponent->>UploadComponent: Validate JSON schema
UploadComponent-->>Client: Show preview & upload button
Client->>UploadComponent: Click upload
UploadComponent->>API: POST /api/admin/quiz/{id}/questions {questions}
deactivate UploadComponent
activate API
API->>API: Validate CSRF & auth
API->>DB: Fetch quiz (ensure draft)
DB-->>API: quiz
API->>DB: Get max display_order
API->>DB: INSERT questions + content (en, uk, pl)
API->>DB: INSERT answers + translations
API->>DB: UPDATE quiz SET questionsCount
API->>DB: invalidateQuizCache()
API-->>Client: 200 {addedCount, totalCount}
deactivate API
Client->>Client: router.refresh()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/db/queries/quizzes/quiz.ts (1)
115-160:⚠️ Potential issue | 🟠 Major
getQuizBySlugdoes not filter or expose thestatusfield, allowing non-ready quizzes to be accessed.
getQuizBySlughas no filter onstatus, and theQuizinterface (lines 27–37) does not include it in the SELECT projection. The public quiz page (frontend/app/[locale]/quiz/[slug]/page.tsx:49) checksisActivebut notstatus. Although the schema default isstatus = 'ready', an admin can explicitly set a quiz tostatus = 'draft'(or another value), and it will still be returned and displayed to anyone who knows the slug because:
getQuizBySlugdoes not filter bystatus- Callers cannot check
statusbecause the field is absent from theQuizinterfaceEither filter at the query level or expose
statusthrough the interface so callers can enforce it.Option A — filter in the query (simplest)
.where(eq(quizzes.slug, slug)) + .where(and(eq(quizzes.slug, slug), eq(quizzes.status, 'ready')))Replace the existing
.where(eq(quizzes.slug, slug))clause.Option B — expose status so callers can decide
export interface Quiz { id: string; slug: string; + status: string; ... } // In getQuizBySlug select projection: + status: quizzes.status,Then the page calling
getQuizBySlugcannotFound()whenquiz.status !== 'ready'.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/db/queries/quizzes/quiz.ts` around lines 115 - 160, getQuizBySlug currently returns quizzes regardless of quizzes.status and the Quiz type doesn't include status; update the query to either filter by status or expose it so callers can enforce readiness. Easiest fix: change the .where(eq(quizzes.slug, slug)) clause in getQuizBySlug to also require eq(quizzes.status, 'ready') (or the appropriate enum value) so only ready quizzes are returned; alternatively, add quizzes.status to the SELECT projection and update the Quiz interface to include status so callers (e.g., the page at frontend/app/[locale]/quiz/[slug]/page.tsx) can check quiz.status !== 'ready' and call notFound(). Ensure you reference quizzes.status and the getQuizBySlug function when making the change.
🧹 Nitpick comments (16)
frontend/components/admin/quiz/JsonUploadArea.tsx (2)
30-60: Stalefilesclosure causes data loss on concurrent uploads.
handleFilesreadsfilesfrom the component closure at line 55 (const updated = [...files, ...newFiles]). The input is only reset at line 59, after allawait file.text()calls complete. During the async file-reading loop a user can click the zone again, triggering a second concurrenthandleFilescall that reads the same stalefiles = [], so the first batch's entries are overwritten.♻️ Proposed fix — reset input first, use functional state update
async function handleFiles(fileList: FileList) { + if (inputRef.current) inputRef.current.value = ''; // prevent re-entry const newFiles: UploadedFile[] = []; for (const file of Array.from(fileList)) { // ...existing processing... } - const updated = [...files, ...newFiles]; - setFiles(updated); - updateParent(updated); - - if (inputRef.current) inputRef.current.value = ''; + setFiles(prev => { + const updated = [...prev, ...newFiles]; + onQuestionsChange(updated.flatMap(f => f.questions)); + return updated; + }); }And remove the standalone
updateParentcall (its logic is now inlined into the functional update above);updateParentis no longer needed separately.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/admin/quiz/JsonUploadArea.tsx` around lines 30 - 60, handleFiles currently reads the stale files variable and appends newFiles after awaiting file.text(), causing race conditions on concurrent uploads; fix by clearing the file input (inputRef.current.value = '') before any awaits to prevent re-entry, and use a functional state update like setFiles(prev => { const updated = [...prev, ...newFiles]; /* call updateParent(updated) here inline */ return updated; }) so you read the latest state atomically; inline the updateParent call inside that functional updater and remove the separate setFiles/updateParent calls at the end of handleFiles.
74-92: Drop-and-drag visual affordance has no drag event handlers.The dashed border and
cursor-pointerstyling imply drag-and-drop, butonDropandonDragOverare not implemented; only click-to-upload works. Either add the handlers or update the UI hint text to "Click to upload".♻️ Minimal drag-and-drop addition
<div className="..." onClick={() => inputRef.current?.click()} + onDragOver={e => e.preventDefault()} + onDrop={e => { e.preventDefault(); e.dataTransfer.files && handleFiles(e.dataTransfer.files); }} role="button" tabIndex={0} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/admin/quiz/JsonUploadArea.tsx` around lines 74 - 92, The UI implies drag-and-drop but lacks handlers; add drag support to the upload area by wiring onDragOver and onDrop on the outer div (the same element that currently has onClick) and implement handlers that call e.preventDefault() in onDragOver and extract files from e.dataTransfer?.files in onDrop and pass them to handleFiles; ensure inputRef and the existing input remain for click fallback and keep accept=".json" and multiple behavior. Also add a small visual state change on onDragEnter/onDragLeave if desired (e.g., toggling a CSS class) so users get feedback while dragging.frontend/db/queries/categories/admin-categories.ts (1)
23-26: Non-idiomatic join condition — useand(eq(), eq())instead of rawsql.Using the
sqltemplate literal for the join predicate bypasses Drizzle's type safety (column renames won't be caught at compile time) and is inconsistent with every other join in the codebase.♻️ Proposed refactor
-import { eq, max, sql } from 'drizzle-orm'; +import { and, eq, max } from 'drizzle-orm'; ... .innerJoin( categoryTranslations, - sql`${categoryTranslations.categoryId} = ${categories.id} AND ${categoryTranslations.locale} = ${ADMIN_LOCALE}` + and( + eq(categoryTranslations.categoryId, categories.id), + eq(categoryTranslations.locale, ADMIN_LOCALE) + ) )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/db/queries/categories/admin-categories.ts` around lines 23 - 26, The join predicate currently uses a raw sql template which bypasses Drizzle's type safety; replace the sql`...` in the .innerJoin call with a typed predicate built from Drizzle helpers, e.g. use and(eq(categoryTranslations.categoryId, categories.id), eq(categoryTranslations.locale, ADMIN_LOCALE)) so that column renames and types are checked; ensure eq and and are imported from Drizzle and used in the same .innerJoin invocation that references categoryTranslations, categories, and ADMIN_LOCALE.frontend/lib/validation/quiz-publish-validation.ts (3)
59-77: O(n²)findIndexinside the content-rows loop — precompute a position Map.
questions.findIndex(...)is called once (or twice) for every row incontentRows, making the combined complexity O(questions × contentRows). For quizzes with hundreds of questions this is noticeable. A simple Map eliminates the repeated scans.♻️ Proposed fix
+ const questionIndexMap = new Map(questions.map((q, i) => [q.id, i])); + const contentMap = new Map<string, Set<string>>(); for (const row of contentRows) { if (!contentMap.has(row.quizQuestionId)) contentMap.set(row.quizQuestionId, new Set()); contentMap.get(row.quizQuestionId)!.add(row.locale); - if (!row.questionText) errors.push(`Q${questions.findIndex(q => q.id === row.quizQuestionId) + 1}: questionText empty (${row.locale})`); + const qNum = (questionIndexMap.get(row.quizQuestionId) ?? -1) + 1; + if (!row.questionText) errors.push(`Q${qNum}: questionText empty (${row.locale})`); if (!row.explanation || (Array.isArray(row.explanation) && row.explanation.length === 0)) { - errors.push(`Q${questions.findIndex(q => q.id === row.quizQuestionId) + 1}: explanation empty (${row.locale})`); + errors.push(`Q${qNum}: explanation empty (${row.locale})`); } }Apply the same
questionIndexMaplookup in the answer-translation loop (line 119) for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/lib/validation/quiz-publish-validation.ts` around lines 59 - 77, The code currently calls questions.findIndex(...) inside the contentRows loop (in the block using contentMap, contentRows, and pushing into errors), causing O(n²) behavior; create a questionIndexMap that maps question id -> index (based on questions array) and replace questions.findIndex(...) usages with a lookup (e.g., questionIndexMap.get(row.quizQuestionId) or questionIndexMap.get(questions[i].id) when building messages) to avoid repeated scans; update the analogous answer-translation loop that also uses findIndex (the loop referencing answer translation checks) to use the same questionIndexMap for consistency and performance, and keep existing error message format and references to contentMap, errors, and LOCALES.
12-12:LOCALESduplicatesfrontend/i18n/config.ts— import instead.If a new locale is ever added to the project, this local constant will silently diverge and allow quizzes with incomplete translations to be published.
♻️ Proposed fix
+import { locales } from '@/i18n/config'; + -const LOCALES = ['en', 'uk', 'pl']; +const LOCALES: readonly string[] = locales;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/lib/validation/quiz-publish-validation.ts` at line 12, Replace the duplicated local LOCALES constant in quiz-publish-validation.ts with the canonical export from the shared i18n config; remove the const LOCALES = [...] and add an import for the exported locales constant (the same symbol used in your i18n config module), then use that imported LOCALES wherever referenced in the file so the validation always reflects the single source-of-truth locale list.
108-113: Answer error messages expose raw UUIDs — use a human-readable index instead.Line 112 emits
Answer ${row.quizAnswerId}: answerText empty (${row.locale})with a raw UUID, inconsistent with theQ${i+1}format used for every other error. Admins can't correlate UUIDs to answers without a separate lookup.♻️ Proposed fix
+ const answerQuestionMap = new Map(answers.map(a => [a.id, a.quizQuestionId])); + for (const row of answerTransRows) { if (!answerTransMap.has(row.quizAnswerId)) answerTransMap.set(row.quizAnswerId, new Set()); answerTransMap.get(row.quizAnswerId)!.add(row.locale); - if (!row.answerText) errors.push(`Answer ${row.quizAnswerId}: answerText empty (${row.locale})`); + const qIdx = questions.findIndex(q => q.id === answerQuestionMap.get(row.quizAnswerId)) + 1; + if (!row.answerText) errors.push(`Q${qIdx}: answer ${row.quizAnswerId.slice(0, 8)}: answerText empty (${row.locale})`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/lib/validation/quiz-publish-validation.ts` around lines 108 - 113, The error message for missing answer text exposes raw UUIDs (row.quizAnswerId) instead of the human-readable answer index used elsewhere (e.g., "Q${i+1}"), so update the loop over answerTransRows to resolve the answer's display index before pushing errors: use answer mapping from the parent quiz answers (or derive an index map keyed by quizAnswerId) and replace `${row.quizAnswerId}` with the human-readable index (e.g., `A${index+1}` or match the existing Q-format) in the errors.push call; ensure you update references in the block around answerTransMap/answerTransRows and preserve locale inclusion `( ${row.locale} )`.frontend/components/admin/quiz/UploadMoreQuestions.tsx (1)
65-76: Stale success message shown on re-expansion.After a successful upload the panel collapses with
setSuccess(...). If the user re-expands the panel, the previous success message is displayed alongside a fresh, emptyJsonUploadArea, which is misleading. Consider clearingsuccess(anderror) when togglingexpandedtotrue.♻️ Proposed fix
- onClick={() => setExpanded(!expanded)} + onClick={() => { + if (!expanded) { setSuccess(''); setError(''); } + setExpanded(!expanded); + }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/admin/quiz/UploadMoreQuestions.tsx` around lines 65 - 76, The success message persists when the panel is re-opened; update the toggle logic used by the Upload More Questions button (the onClick that calls setExpanded(!expanded)) to clear previous feedback by setting success and error to null/empty when expanding (i.e., when changing expanded to true), so the JsonUploadArea and any status text start fresh; modify the handler that toggles expanded (or create a small toggleExpanded function) to call setSuccess(null) and setError(null) before/after calling setExpanded(true).frontend/components/admin/quiz/DeleteQuizButton.tsx (1)
16-42:window.confirm/alertare inconsistent with the modal-based UX used elsewhere.The PR description states that status controls use confirmation modals. Using blocking native browser dialogs here produces an inconsistent feel, cannot be styled/translated, and breaks automated tests that stub
window.confirm.Consider replacing with the same React confirmation modal pattern used for status controls (e.g. a small inline
<Dialog>or a sharedConfirmModalcomponent), keeping the button's disabled/loading state logic as-is.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/admin/quiz/DeleteQuizButton.tsx` around lines 16 - 42, Replace the native window.confirm and alert usage in DeleteQuizButton's handleDelete with the app's React confirmation modal pattern: add local state (e.g., isConfirmOpen) to open a shared ConfirmModal or Dialog when the delete button is clicked, move the deletion logic (the fetch to /api/admin/quiz/${quizId} that uses csrfToken, setDeleting, try/catch/finally, and router.refresh) into the modal's confirmed handler (e.g., onConfirmDelete), and surface errors via the app's UI (toast or modal error) instead of alert while preserving the existing disabled/loading behavior controlled by setDeleting. Ensure the modal component is used consistently with other status controls and that cancelling the modal simply closes it without calling the API.frontend/db/queries/quizzes/admin-quiz.ts (1)
83-93: Inconsistent separator in interface definition.Line 88 uses a comma (
,) while other members inAdminQuizFulluse semicolons (;). Both are valid TypeScript, but mixing them reduces readability.- status: string, + status: string;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/db/queries/quizzes/admin-quiz.ts` around lines 83 - 93, The AdminQuizFull interface mixes separators: change the trailing comma after the status property to a semicolon so all members (id, slug, questionsCount, timeLimitSeconds, status, isActive, categoryId, translations, questions) consistently use semicolons; update the status line in the AdminQuizFull definition accordingly.frontend/app/api/admin/quiz/[id]/questions/[questionId]/route.ts (2)
111-117: Prefer an explicitundefinedcheck over a truthy check fordifficulty.
if (parsed.data.difficulty)will skip the update for any falsy value (e.g.,"",null). While current valid values (beginner,medium,advanced) are all truthy, using!== undefinedis more intentional and future-proof — it correctly distinguishes "field absent from payload" from "field explicitly set to a falsy value."- if (parsed.data.difficulty) { + if (parsed.data.difficulty !== undefined) {Also, the indentation on line 111 is inconsistent with the surrounding code (extra leading spaces).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/api/admin/quiz/`[id]/questions/[questionId]/route.ts around lines 111 - 117, The update block should check for explicit undefined rather than truthiness to allow falsy-but-intentional values: change the condition to test parsed.data.difficulty !== undefined before calling db.update(quizQuestions).set({ difficulty: parsed.data.difficulty }).where(eq(quizQuestions.id, questionId)); also fix the extra leading spaces on the if line so its indentation matches the surrounding code.
288-300: Delete + recount + update should be wrapped in a transaction.Lines 289–300 perform three dependent operations (delete question → count remaining → update quiz) without transactional guarantees. If the process fails between the delete and the count update,
questionsCountbecomes stale. Wrapping indb.transaction()ensures atomicity.Proposed fix
- // Delete question (cascades content, answers, answer translations) - await db.delete(quizQuestions).where(eq(quizQuestions.id, questionId)); - - // Update questionsCount - const [countRow] = await db - .select({ total: sql<number>`COUNT(*)::int` }) - .from(quizQuestions) - .where(eq(quizQuestions.quizId, quizId)); - - await db - .update(quizzes) - .set({ questionsCount: countRow.total }) - .where(eq(quizzes.id, quizId)); + // Delete question and update count atomically + const countRow = await db.transaction(async (tx) => { + await tx.delete(quizQuestions).where(eq(quizQuestions.id, questionId)); + + const [row] = await tx + .select({ total: sql<number>`COUNT(*)::int` }) + .from(quizQuestions) + .where(eq(quizQuestions.quizId, quizId)); + + await tx + .update(quizzes) + .set({ questionsCount: row.total }) + .where(eq(quizzes.id, quizId)); + + return row; + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/api/admin/quiz/`[id]/questions/[questionId]/route.ts around lines 288 - 300, Wrap the three dependent DB operations (deleting the question, recounting, and updating the quiz) in a single transaction to guarantee atomicity: use db.transaction(...) and perform the delete on quizQuestions (where id = questionId), then the COUNT select from quizQuestions (where quizId = quizId) and finally the update on quizzes setting questionsCount = countRow.total (where id = quizId) all via the transaction client so failures roll back; ensure you reference the same questionId/quizId variables and propagate/throw errors as needed so the transaction will rollback on failure.frontend/db/schema/quiz.ts (1)
32-32: Consider using apgEnumfor thestatuscolumn instead ofvarchar.A constrained enum (
pgEnum('quiz_status', ['draft', 'ready'])) would enforce valid values at the DB level and prevent typos or invalid status strings. The currentvarchar(20)allows any string up to 20 characters. This is a minor type-safety concern since all writes go through your application code, but the DB-level constraint adds a useful safety net.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/db/schema/quiz.ts` at line 32, Replace the loose varchar status column with a Postgres enum to enforce allowed values: change the column definition for status in the Quiz schema (the current status: varchar('status', { length: 20 }).notNull().default('ready')) to use pgEnum('quiz_status', ['draft', 'ready']) and set the column to that enum with .notNull().default('ready'); also add the pgEnum import from your PG core (e.g., pgEnum from drizzle-orm/pg-core) and ensure any TypeScript types/usages of the Quiz model expect the enum values so code and migrations reference the new quiz_status enum.frontend/app/[locale]/admin/quiz/[id]/page.tsx (1)
54-76: Extract status and active badges into reusable components.The
QuizListTable.tsxcontainsStatusBadgeandActiveBadgefunctions with identical logic to the inline badges here. However, these components are not currently exported fromQuizListTable.tsx, preventing reuse. Extract them to a shared component file (e.g.,components/admin/quiz/badges.tsx) and export them, then import and use them in this file to eliminate duplication and maintain consistent styling.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/`[locale]/admin/quiz/[id]/page.tsx around lines 54 - 76, The inline badge markup duplicates logic already in StatusBadge and ActiveBadge in QuizListTable.tsx; create a shared component file (components/admin/quiz/badges.tsx) that exports both StatusBadge and ActiveBadge, move the conditional class/name logic there (using props like isDraft and isActive or quiz), update QuizListTable.tsx to import/export or remove its local definitions, and replace the inline badge spans in page.tsx with the imported <StatusBadge> and <ActiveBadge> components so the UI uses the single shared implementation.frontend/components/admin/quiz/QuizStatusControls.tsx (2)
34-35: Fragile loading-key derivation fromObject.keys(body)[0].Using the first key of the body object as the loading indicator works for single-field patches (
{ status: 'ready' }or{ isActive: true }) but would break if a patch ever includes multiple keys. Consider passing the loading key explicitly.💡 Slightly more robust approach
- async function patchQuiz(body: Record<string, unknown>) { - setLoading(Object.keys(body)[0]); + async function patchQuiz(loadingKey: string, body: Record<string, unknown>) { + setLoading(loadingKey);Then update call sites:
- action: () => patchQuiz({ status: 'ready' }), + action: () => patchQuiz('status', { status: 'ready' }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/admin/quiz/QuizStatusControls.tsx` around lines 34 - 35, The current patchQuiz function derives the loading key by Object.keys(body)[0], which is fragile for multi-key patches—change patchQuiz signature to accept an explicit loadingKey parameter (e.g., async function patchQuiz(body: Record<string, unknown>, loadingKey: string)) and use setLoading(loadingKey) inside; update all call sites that invoke patchQuiz (references to patchQuiz in this component) to pass the appropriate explicit loading key (like 'status' or 'isActive') instead of relying on the body shape.
169-201: Consider reusing the existingConfirmModalcomponent instead of duplicating modal UI.There's already a
ConfirmModalcomponent atfrontend/components/ui/confirm-modal.tsxthat acceptsisOpen,title,message,variant,onConfirm, andonCancelprops — matching exactly what this inline modal does. Reusing it would reduce duplication and inherit any accessibility improvements (e.g., Escape-to-close) added to that shared component.The inline modal also lacks keyboard dismiss (Escape key) and focus trapping, which the shared component may already handle or could be added in one place.
♻️ Proposed refactor to use existing ConfirmModal
Add the import:
import { ConfirmModal } from '@/components/ui/confirm-modal';Then replace lines 170–201 with:
- {modal && ( - <div className="fixed inset-0 z-50 flex items-center justify-center"> - ... - </div> - )} + <ConfirmModal + isOpen={modal !== null} + title={modal?.title ?? ''} + message={modal?.message ?? ''} + variant={modal?.variant ?? 'default'} + onConfirm={confirmAction} + onCancel={() => setModal(null)} + />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/admin/quiz/QuizStatusControls.tsx` around lines 169 - 201, The inline confirmation modal duplicates shared UI and lacks keyboard accessibility; replace the manual JSX with the existing ConfirmModal component by importing ConfirmModal and rendering it with isOpen={Boolean(modal)}, title={modal?.title}, message={modal?.message}, variant={modal?.variant}, onConfirm={confirmAction}, and onCancel={() => setModal(null)} so confirmAction and setModal remain wired up; ensure the import for ConfirmModal is added at the top and remove the inline modal block that references modal, confirmAction, and setModal.frontend/app/api/admin/quiz/route.ts (1)
29-37:noStoreJsonandtextToExplanationBlocksare duplicated across route files.These helpers appear identically in at least 3 files (
route.ts,[id]/questions/route.ts,[id]/route.ts). Extract them into a shared module (e.g.,@/lib/api/admin-helpers) to reduce duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/api/admin/quiz/route.ts` around lines 29 - 37, The functions noStoreJson and textToExplanationBlocks are duplicated; extract them into a new shared module (e.g., export functions from a new admin-helpers module) and replace the local definitions in each route file with imports from that module. Create and export noStoreJson(body, init?) and textToExplanationBlocks(text) from the new module, update each route file that currently defines those functions to remove the duplicates and import the named exports, and run a quick compile to ensure NextResponse is imported/available in the shared module or passed through as needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/app/api/admin/categories/route.ts`:
- Around line 60-94: The category creation currently does the duplicate-slug
check, category insert (categories), and translation inserts
(categoryTranslations) separately, risking orphaned rows and TOCTOU races; wrap
the logic that checks for existing slug, computes max(categories.displayOrder),
inserts the categories row and inserts LOCALES-based translations for
category.id in a single db transaction (using your DB client's transaction API)
so any failure rolls back the category insert, and consider enforcing a unique
constraint on categories.slug to guard against races; locate references to db,
categories, categoryTranslations, slug, displayOrder, LOCALES and category.id to
modify the flow.
In `@frontend/app/api/admin/quiz/`[id]/questions/[questionId]/route.ts:
- Line 1: Remove the unused import symbol `count` from the import list in the
module (currently importing { and, count, eq, sql } from 'drizzle-orm'); update
the import to only include the actually used symbols (`and`, `eq`, `sql`) so the
unused `count` is eliminated (the DELETE handler already uses a raw `sql`
COUNT(*)::int template).
In `@frontend/app/api/admin/quiz/`[id]/questions/route.ts:
- Around line 111-173: Wrap the entire multi-step DB routine (steps that create
insertedQuestions, insert into quizQuestionContent, build answerValues, insert
into quizAnswers producing insertedAnswers, insert into quizAnswerTranslations,
and update quizzes with countRow) inside a single db.transaction(...) so all
operations run on the transaction context (use the transaction object instead of
top-level db when calling insert/select/update). Use the transaction to produce
insertedQuestions/insertedAnswers and compute countRow inside it, and let the
transaction commit or roll back on error; call invalidateQuizCache(quizId) only
after the transaction resolves successfully so cache invalidation happens only
on commit. Ensure you replace references to quizQuestions, quizQuestionContent,
quizAnswers, quizAnswerTranslations, quizzes, insertedQuestions,
insertedAnswers, answerValues, and countRow to use the transaction's query
methods.
- Around line 149-160: The translation insert assumes 4 answers per question by
using Math.floor(i / 4) and i % 4; instead compute offsets dynamically to match
the actual answers array lengths: build translations by iterating the original
questions and for each question iterating its answers (or compute a running
index/pointer into insertedAnswers) so you pair each insertedAnswers element
(from insertedAnswers) with the corresponding
questions[qIdx].answers[aIdx][locale] without hardcoding 4; update the same
pattern in the other occurrence (frontend/app/api/admin/quiz/route.ts) and keep
references to quizAnswerTranslations, insertedAnswers, questions, LOCALES and
the db.insert call when making the change.
In `@frontend/app/api/admin/quiz/`[id]/route.ts:
- Around line 203-215: There is a TOCTOU between counting quizAttempts and
deleting quizzes; wrap the check+delete in a single DB atomic operation: either
run both the count and delete inside a transaction with serializable/isolation
(use the db.transaction API and perform the .select({ total: count()
}).from(quizAttempts).where(eq(quizAttempts.quizId, quizId)) and then
conditionally db.delete(quizzes).where(eq(quizzes.id, quizId)) inside that
transaction), or replace with a single conditional delete like
db.delete(quizzes).where(eq(quizzes.id,
quizId)).where(notExists(db.select().from(quizAttempts).where(eq(quizAttempts.quizId,
quizId)))) and then check the number of affected rows to return the 409
HAS_ATTEMPTS error if the delete did not occur because attempts exist; reference
quizAttempts and quizzes to locate the code to change.
- Around line 96-123: The update to quizzes (using
db.update(quizzes).set(updateData).where(eq(quizzes.id, quizId))) and the
subsequent translation upserts (in the loop inserting into quizTranslations
using parsed.data.translations and locales) must be executed inside a single
Drizzle transaction so either all changes commit or none do; refactor to call
db.transaction(...) (or the project’s Drizzle transaction helper) and move the
update and the for-loop of upserts into that transaction callback, using the
transactional DB handle for all db.update and db.insert calls to ensure
atomicity.
- Around line 1-3: The project is using an insecure Next.js release; update the
Next.js dependency to "^16.1.6" (or higher stable LTS) in your package manifest,
regenerate the lockfile (npm/yarn/pnpm install) and commit the updated lockfile,
then run the test suite and a local dev build to verify routes that use
NextRequest/NextResponse and server components still work; ensure CI installs
the new lockfile and redeploys with the patched version.
In `@frontend/app/api/admin/quiz/route.ts`:
- Around line 97-172: All six dependent inserts (involving quizzes,
quizTranslations, quizQuestions, quizQuestionContent, quizAnswers,
quizAnswerTranslations) must run inside a single db.transaction so the work is
atomic; wrap the sequence in await db.transaction(async tx => { ... }), replace
calls to db.insert(...) with tx.insert(...) and keep the same .values(...) and
.returning(...) logic (including using the returned ids like quiz.id,
insertedQuestions, insertedAnswers) so any failure rolls back and no partial
rows remain.
- Around line 161-172: The insert for quizAnswerTranslations assumes each
question has exactly 4 answers by using Math.floor(i / 4) and i % 4; update the
logic that builds translation rows so it mirrors how insertedAnswers were
created instead of hardcoding 4: derive quizAnswer->question/answer indexes by
iterating questions and their answers (or by precomputing an answersPerQuestion
array) and flat-mapping into the same order as insertedAnswers, then map each
produced dbA (from insertedAnswers) to the correct locale entries using LOCALES,
quizAnswerId: dbA.id, and the corresponding answer text from
questions[qIdx].answers[aIdx][locale]; ensure you replace the Math.floor(i /
4)/i % 4 math in the quizAnswerTranslations insertion with this dynamic mapping
using the insertedAnswers and questions structures.
In `@frontend/components/admin/quiz/CreateQuizForm.tsx`:
- Around line 113-134: The payload construction in CreateQuizForm sends
timeLimitSeconds as NaN when a non-numeric string is entered; before building
the body (where timeLimitSeconds is used) validate/normalize the input like
QuizMetadataEditor does: parse the value to a number (e.g., Number or
parseInt/parseFloat), check Number.isFinite or isNaN, and set timeLimitSeconds
to null if invalid or empty; update the body creation in CreateQuizForm (the
timeLimitSeconds assignment) to use the validated/normalized value so you never
send NaN to the server.
In `@frontend/components/admin/quiz/JsonUploadArea.tsx`:
- Around line 74-85: The upload drop zone div in JsonUploadArea.tsx is not
keyboard accessible; make the container act like a button by adding
role="button", tabIndex={0}, and an onKeyDown handler that listens for Enter and
Space and calls inputRef.current?.click() (preventDefault for Space), while
keeping the existing onClick and aria-label/aria-describedby as needed for
screen readers; ensure the inputRef and handleFiles remain unchanged and use the
same click invocation so keyboard and mouse trigger identical behavior.
In `@frontend/components/admin/quiz/QuestionEditor.tsx`:
- Line 32: EditorState currently types difficulty as string and code uses a
non-null assertion (question.difficulty!) and a wide handler signature; change
EditorState.difficulty to the union 'beginner' | 'medium' | 'advanced',
initialize state from question.difficulty with a safe default (e.g.,
question.difficulty ?? 'beginner') instead of using the non-null assertion so
undefined never flows into the select, and tighten handleDifficultyChange to
accept the same union type for its parameter; update the select onChange to pass
that narrowed value.
- Around line 123-153: isDirty currently becomes true when
editorState.difficulty !== initialStateRef.current.difficulty but dirtyLocales
remains empty, causing handleSave to send an empty dirtyLocales and server
validation to fail; update the client to mark all locales dirty when difficulty
changes (same approach as handleCorrectChange) so dirtyLocales contains every
locale before save and the confirmation "modified" string is non-empty;
specifically, in the difficulty change handler (where editorState.difficulty is
updated) add all locale keys to the dirtyLocales Set, and ensure isDirty uses
dirtyLocales.size > 0 || editorState.difficulty !==
initialStateRef.current.difficulty so handleSave and the prompt receive the
populated dirtyLocales.
In `@frontend/components/admin/quiz/QuizMetadataEditor.tsx`:
- Around line 129-151: The read-only summary in QuizMetadataEditor currently
renders timeLimitSeconds / 60 directly (variable timeLimitSeconds) which can
produce non-integer values like "1.5 min"; update the display logic to format
the duration cleanly — e.g., compute minutes = Math.floor(timeLimitSeconds/60)
and seconds = timeLimitSeconds % 60 and render as "X min Y sec" when seconds
exist, or alternately format to one decimal place and strip a trailing ".0"
(using (timeLimitSeconds/60).toFixed(1) and trimming) so the summary shows a
clean, user-friendly string instead of raw fractional minutes.
In `@frontend/lib/validation/admin-quiz.ts`:
- Around line 55-78: jsonQuestionSchema currently allows any string for id and
uses plain strings for explanation fields, which mismatches the rest of the
codebase: change id in jsonQuestionSchema from z.string().optional() to
z.string().uuid().optional() to enforce UUIDs, and annotate/adjust the exp
fields (uk.exp, en.exp, pl.exp) to reflect that the persisted model expects
rich-text block arrays (or rename to expPlain and add an expBlocks field) and
ensure the API handlers that use createQuizSchema / addQuestionsSchema convert
these plain-string exp values into the block-array format expected by
patchQuestionSchema before saving; add a short inline comment on
jsonQuestionSchema documenting the required conversion so future reviewers see
the contract.
---
Outside diff comments:
In `@frontend/db/queries/quizzes/quiz.ts`:
- Around line 115-160: getQuizBySlug currently returns quizzes regardless of
quizzes.status and the Quiz type doesn't include status; update the query to
either filter by status or expose it so callers can enforce readiness. Easiest
fix: change the .where(eq(quizzes.slug, slug)) clause in getQuizBySlug to also
require eq(quizzes.status, 'ready') (or the appropriate enum value) so only
ready quizzes are returned; alternatively, add quizzes.status to the SELECT
projection and update the Quiz interface to include status so callers (e.g., the
page at frontend/app/[locale]/quiz/[slug]/page.tsx) can check quiz.status !==
'ready' and call notFound(). Ensure you reference quizzes.status and the
getQuizBySlug function when making the change.
---
Nitpick comments:
In `@frontend/app/`[locale]/admin/quiz/[id]/page.tsx:
- Around line 54-76: The inline badge markup duplicates logic already in
StatusBadge and ActiveBadge in QuizListTable.tsx; create a shared component file
(components/admin/quiz/badges.tsx) that exports both StatusBadge and
ActiveBadge, move the conditional class/name logic there (using props like
isDraft and isActive or quiz), update QuizListTable.tsx to import/export or
remove its local definitions, and replace the inline badge spans in page.tsx
with the imported <StatusBadge> and <ActiveBadge> components so the UI uses the
single shared implementation.
In `@frontend/app/api/admin/quiz/`[id]/questions/[questionId]/route.ts:
- Around line 111-117: The update block should check for explicit undefined
rather than truthiness to allow falsy-but-intentional values: change the
condition to test parsed.data.difficulty !== undefined before calling
db.update(quizQuestions).set({ difficulty: parsed.data.difficulty
}).where(eq(quizQuestions.id, questionId)); also fix the extra leading spaces on
the if line so its indentation matches the surrounding code.
- Around line 288-300: Wrap the three dependent DB operations (deleting the
question, recounting, and updating the quiz) in a single transaction to
guarantee atomicity: use db.transaction(...) and perform the delete on
quizQuestions (where id = questionId), then the COUNT select from quizQuestions
(where quizId = quizId) and finally the update on quizzes setting questionsCount
= countRow.total (where id = quizId) all via the transaction client so failures
roll back; ensure you reference the same questionId/quizId variables and
propagate/throw errors as needed so the transaction will rollback on failure.
In `@frontend/app/api/admin/quiz/route.ts`:
- Around line 29-37: The functions noStoreJson and textToExplanationBlocks are
duplicated; extract them into a new shared module (e.g., export functions from a
new admin-helpers module) and replace the local definitions in each route file
with imports from that module. Create and export noStoreJson(body, init?) and
textToExplanationBlocks(text) from the new module, update each route file that
currently defines those functions to remove the duplicates and import the named
exports, and run a quick compile to ensure NextResponse is imported/available in
the shared module or passed through as needed.
In `@frontend/components/admin/quiz/DeleteQuizButton.tsx`:
- Around line 16-42: Replace the native window.confirm and alert usage in
DeleteQuizButton's handleDelete with the app's React confirmation modal pattern:
add local state (e.g., isConfirmOpen) to open a shared ConfirmModal or Dialog
when the delete button is clicked, move the deletion logic (the fetch to
/api/admin/quiz/${quizId} that uses csrfToken, setDeleting, try/catch/finally,
and router.refresh) into the modal's confirmed handler (e.g., onConfirmDelete),
and surface errors via the app's UI (toast or modal error) instead of alert
while preserving the existing disabled/loading behavior controlled by
setDeleting. Ensure the modal component is used consistently with other status
controls and that cancelling the modal simply closes it without calling the API.
In `@frontend/components/admin/quiz/JsonUploadArea.tsx`:
- Around line 30-60: handleFiles currently reads the stale files variable and
appends newFiles after awaiting file.text(), causing race conditions on
concurrent uploads; fix by clearing the file input (inputRef.current.value = '')
before any awaits to prevent re-entry, and use a functional state update like
setFiles(prev => { const updated = [...prev, ...newFiles]; /* call
updateParent(updated) here inline */ return updated; }) so you read the latest
state atomically; inline the updateParent call inside that functional updater
and remove the separate setFiles/updateParent calls at the end of handleFiles.
- Around line 74-92: The UI implies drag-and-drop but lacks handlers; add drag
support to the upload area by wiring onDragOver and onDrop on the outer div (the
same element that currently has onClick) and implement handlers that call
e.preventDefault() in onDragOver and extract files from e.dataTransfer?.files in
onDrop and pass them to handleFiles; ensure inputRef and the existing input
remain for click fallback and keep accept=".json" and multiple behavior. Also
add a small visual state change on onDragEnter/onDragLeave if desired (e.g.,
toggling a CSS class) so users get feedback while dragging.
In `@frontend/components/admin/quiz/QuizStatusControls.tsx`:
- Around line 34-35: The current patchQuiz function derives the loading key by
Object.keys(body)[0], which is fragile for multi-key patches—change patchQuiz
signature to accept an explicit loadingKey parameter (e.g., async function
patchQuiz(body: Record<string, unknown>, loadingKey: string)) and use
setLoading(loadingKey) inside; update all call sites that invoke patchQuiz
(references to patchQuiz in this component) to pass the appropriate explicit
loading key (like 'status' or 'isActive') instead of relying on the body shape.
- Around line 169-201: The inline confirmation modal duplicates shared UI and
lacks keyboard accessibility; replace the manual JSX with the existing
ConfirmModal component by importing ConfirmModal and rendering it with
isOpen={Boolean(modal)}, title={modal?.title}, message={modal?.message},
variant={modal?.variant}, onConfirm={confirmAction}, and onCancel={() =>
setModal(null)} so confirmAction and setModal remain wired up; ensure the import
for ConfirmModal is added at the top and remove the inline modal block that
references modal, confirmAction, and setModal.
In `@frontend/components/admin/quiz/UploadMoreQuestions.tsx`:
- Around line 65-76: The success message persists when the panel is re-opened;
update the toggle logic used by the Upload More Questions button (the onClick
that calls setExpanded(!expanded)) to clear previous feedback by setting success
and error to null/empty when expanding (i.e., when changing expanded to true),
so the JsonUploadArea and any status text start fresh; modify the handler that
toggles expanded (or create a small toggleExpanded function) to call
setSuccess(null) and setError(null) before/after calling setExpanded(true).
In `@frontend/db/queries/categories/admin-categories.ts`:
- Around line 23-26: The join predicate currently uses a raw sql template which
bypasses Drizzle's type safety; replace the sql`...` in the .innerJoin call with
a typed predicate built from Drizzle helpers, e.g. use
and(eq(categoryTranslations.categoryId, categories.id),
eq(categoryTranslations.locale, ADMIN_LOCALE)) so that column renames and types
are checked; ensure eq and and are imported from Drizzle and used in the same
.innerJoin invocation that references categoryTranslations, categories, and
ADMIN_LOCALE.
In `@frontend/db/queries/quizzes/admin-quiz.ts`:
- Around line 83-93: The AdminQuizFull interface mixes separators: change the
trailing comma after the status property to a semicolon so all members (id,
slug, questionsCount, timeLimitSeconds, status, isActive, categoryId,
translations, questions) consistently use semicolons; update the status line in
the AdminQuizFull definition accordingly.
In `@frontend/db/schema/quiz.ts`:
- Line 32: Replace the loose varchar status column with a Postgres enum to
enforce allowed values: change the column definition for status in the Quiz
schema (the current status: varchar('status', { length: 20
}).notNull().default('ready')) to use pgEnum('quiz_status', ['draft', 'ready'])
and set the column to that enum with .notNull().default('ready'); also add the
pgEnum import from your PG core (e.g., pgEnum from drizzle-orm/pg-core) and
ensure any TypeScript types/usages of the Quiz model expect the enum values so
code and migrations reference the new quiz_status enum.
In `@frontend/lib/validation/quiz-publish-validation.ts`:
- Around line 59-77: The code currently calls questions.findIndex(...) inside
the contentRows loop (in the block using contentMap, contentRows, and pushing
into errors), causing O(n²) behavior; create a questionIndexMap that maps
question id -> index (based on questions array) and replace
questions.findIndex(...) usages with a lookup (e.g.,
questionIndexMap.get(row.quizQuestionId) or
questionIndexMap.get(questions[i].id) when building messages) to avoid repeated
scans; update the analogous answer-translation loop that also uses findIndex
(the loop referencing answer translation checks) to use the same
questionIndexMap for consistency and performance, and keep existing error
message format and references to contentMap, errors, and LOCALES.
- Line 12: Replace the duplicated local LOCALES constant in
quiz-publish-validation.ts with the canonical export from the shared i18n
config; remove the const LOCALES = [...] and add an import for the exported
locales constant (the same symbol used in your i18n config module), then use
that imported LOCALES wherever referenced in the file so the validation always
reflects the single source-of-truth locale list.
- Around line 108-113: The error message for missing answer text exposes raw
UUIDs (row.quizAnswerId) instead of the human-readable answer index used
elsewhere (e.g., "Q${i+1}"), so update the loop over answerTransRows to resolve
the answer's display index before pushing errors: use answer mapping from the
parent quiz answers (or derive an index map keyed by quizAnswerId) and replace
`${row.quizAnswerId}` with the human-readable index (e.g., `A${index+1}` or
match the existing Q-format) in the errors.push call; ensure you update
references in the block around answerTransMap/answerTransRows and preserve
locale inclusion `( ${row.locale} )`.
Comment on lines
+60
to
+94
| // Check duplicate slug | ||
| const [existing] = await db | ||
| .select({ id: categories.id }) | ||
| .from(categories) | ||
| .where(eq(categories.slug, slug)) | ||
| .limit(1); | ||
|
|
||
| if (existing) { | ||
| return noStoreJson( | ||
| { error: 'Category with this slug already exists', code: 'DUPLICATE_SLUG' }, | ||
| { status: 409 } | ||
| ); | ||
| } | ||
|
|
||
| // Auto displayOrder | ||
| const [maxRow] = await db | ||
| .select({ maxOrder: max(categories.displayOrder) }) | ||
| .from(categories); | ||
|
|
||
| const displayOrder = (maxRow?.maxOrder ?? 0) + 1; | ||
|
|
||
| // Insert category | ||
| const [category] = await db | ||
| .insert(categories) | ||
| .values({ slug, displayOrder }) | ||
| .returning({ id: categories.id }); | ||
|
|
||
| // Insert translations | ||
| await db.insert(categoryTranslations).values( | ||
| LOCALES.map(locale => ({ | ||
| categoryId: category.id, | ||
| locale, | ||
| title: translations[locale].title, | ||
| })) | ||
| ); |
Contributor
There was a problem hiding this comment.
Category + translations insert should be wrapped in a transaction.
If the translation insert (line 88) fails after the category was already inserted (line 82), you'll have an orphaned category row with no translations. Same pattern as the quiz creation routes.
Additionally, the duplicate-slug check (line 61) is subject to a TOCTOU race — two concurrent requests could both pass the check. This is low-risk for an admin endpoint but would be cleanly solved by doing the check + insert inside a transaction (or relying on a DB unique constraint on categories.slug).
🔧 Sketch: wrap in transaction
- // Check duplicate slug
- const [existing] = await db
+ const result = await db.transaction(async (tx) => {
+ const [existing] = await tx
.select({ id: categories.id })
...
- // Insert category
- const [category] = await db
+ const [category] = await tx
.insert(categories)
...
- // Insert translations
- await db.insert(categoryTranslations).values(
+ await tx.insert(categoryTranslations).values(
...
+ return category;
+ });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/api/admin/categories/route.ts` around lines 60 - 94, The
category creation currently does the duplicate-slug check, category insert
(categories), and translation inserts (categoryTranslations) separately, risking
orphaned rows and TOCTOU races; wrap the logic that checks for existing slug,
computes max(categories.displayOrder), inserts the categories row and inserts
LOCALES-based translations for category.id in a single db transaction (using
your DB client's transaction API) so any failure rolls back the category insert,
and consider enforcing a unique constraint on categories.slug to guard against
races; locate references to db, categories, categoryTranslations, slug,
displayOrder, LOCALES and category.id to modify the flow.
frontend/app/api/admin/quiz/[id]/questions/[questionId]/route.ts
Outdated
Show resolved
Hide resolved
Comment on lines
+111
to
+173
| // 1. Insert questions | ||
| const insertedQuestions = await db | ||
| .insert(quizQuestions) | ||
| .values( | ||
| questions.map((q, i) => ({ | ||
| quizId, | ||
| displayOrder: orderOffset + i + 1, | ||
| difficulty: q.difficulty, | ||
| })) | ||
| ) | ||
| .returning({ id: quizQuestions.id }); | ||
|
|
||
| // 2. Insert question content | ||
| await db.insert(quizQuestionContent).values( | ||
| insertedQuestions.flatMap((dbQ, i) => | ||
| LOCALES.map(locale => ({ | ||
| quizQuestionId: dbQ.id, | ||
| locale, | ||
| questionText: questions[i][locale].q, | ||
| explanation: textToExplanationBlocks(questions[i][locale].exp), | ||
| })) | ||
| ) | ||
| ); | ||
|
|
||
| // 3. Insert answers | ||
| const answerValues = insertedQuestions.flatMap((dbQ, i) => | ||
| questions[i].answers.map((a, aIdx) => ({ | ||
| quizQuestionId: dbQ.id, | ||
| displayOrder: aIdx + 1, | ||
| isCorrect: a.correct, | ||
| })) | ||
| ); | ||
|
|
||
| const insertedAnswers = await db | ||
| .insert(quizAnswers) | ||
| .values(answerValues) | ||
| .returning({ id: quizAnswers.id }); | ||
|
|
||
| // 4. Insert answer translations | ||
| await db.insert(quizAnswerTranslations).values( | ||
| insertedAnswers.flatMap((dbA, i) => { | ||
| const qIdx = Math.floor(i / 4); | ||
| const aIdx = i % 4; | ||
| return LOCALES.map(locale => ({ | ||
| quizAnswerId: dbA.id, | ||
| locale, | ||
| answerText: questions[qIdx].answers[aIdx][locale], | ||
| })); | ||
| }) | ||
| ); | ||
|
|
||
| // 5. Update questionsCount | ||
| const [countRow] = await db | ||
| .select({ count: sql<number>`COUNT(*)::int` }) | ||
| .from(quizQuestions) | ||
| .where(eq(quizQuestions.quizId, quizId)); | ||
|
|
||
| await db | ||
| .update(quizzes) | ||
| .set({ questionsCount: countRow.count }) | ||
| .where(eq(quizzes.id, quizId)); | ||
|
|
||
| await invalidateQuizCache(quizId); |
Contributor
There was a problem hiding this comment.
Multiple sequential inserts without a transaction — partial failures leave inconsistent data.
The handler performs 5 sequential DB operations (insert questions → insert content → insert answers → insert answer translations → update count). If any step fails (e.g., a constraint violation on answer translations), the preceding inserts are already committed, leaving orphaned rows in the database.
Wrap the entire block in a db.transaction() call to ensure atomicity.
🔧 Sketch: wrap in transaction
- // 1. Insert questions
- const insertedQuestions = await db
- .insert(quizQuestions)
+ const result = await db.transaction(async (tx) => {
+ // 1. Insert questions
+ const insertedQuestions = await tx
+ .insert(quizQuestions)
...
- // 5. Update questionsCount
+ // 5. Update questionsCount
...
+ return { addedCount: questions.length, totalCount: countRow.count };
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // 1. Insert questions | |
| const insertedQuestions = await db | |
| .insert(quizQuestions) | |
| .values( | |
| questions.map((q, i) => ({ | |
| quizId, | |
| displayOrder: orderOffset + i + 1, | |
| difficulty: q.difficulty, | |
| })) | |
| ) | |
| .returning({ id: quizQuestions.id }); | |
| // 2. Insert question content | |
| await db.insert(quizQuestionContent).values( | |
| insertedQuestions.flatMap((dbQ, i) => | |
| LOCALES.map(locale => ({ | |
| quizQuestionId: dbQ.id, | |
| locale, | |
| questionText: questions[i][locale].q, | |
| explanation: textToExplanationBlocks(questions[i][locale].exp), | |
| })) | |
| ) | |
| ); | |
| // 3. Insert answers | |
| const answerValues = insertedQuestions.flatMap((dbQ, i) => | |
| questions[i].answers.map((a, aIdx) => ({ | |
| quizQuestionId: dbQ.id, | |
| displayOrder: aIdx + 1, | |
| isCorrect: a.correct, | |
| })) | |
| ); | |
| const insertedAnswers = await db | |
| .insert(quizAnswers) | |
| .values(answerValues) | |
| .returning({ id: quizAnswers.id }); | |
| // 4. Insert answer translations | |
| await db.insert(quizAnswerTranslations).values( | |
| insertedAnswers.flatMap((dbA, i) => { | |
| const qIdx = Math.floor(i / 4); | |
| const aIdx = i % 4; | |
| return LOCALES.map(locale => ({ | |
| quizAnswerId: dbA.id, | |
| locale, | |
| answerText: questions[qIdx].answers[aIdx][locale], | |
| })); | |
| }) | |
| ); | |
| // 5. Update questionsCount | |
| const [countRow] = await db | |
| .select({ count: sql<number>`COUNT(*)::int` }) | |
| .from(quizQuestions) | |
| .where(eq(quizQuestions.quizId, quizId)); | |
| await db | |
| .update(quizzes) | |
| .set({ questionsCount: countRow.count }) | |
| .where(eq(quizzes.id, quizId)); | |
| await invalidateQuizCache(quizId); | |
| const result = await db.transaction(async (tx) => { | |
| // 1. Insert questions | |
| const insertedQuestions = await tx | |
| .insert(quizQuestions) | |
| .values( | |
| questions.map((q, i) => ({ | |
| quizId, | |
| displayOrder: orderOffset + i + 1, | |
| difficulty: q.difficulty, | |
| })) | |
| ) | |
| .returning({ id: quizQuestions.id }); | |
| // 2. Insert question content | |
| await tx.insert(quizQuestionContent).values( | |
| insertedQuestions.flatMap((dbQ, i) => | |
| LOCALES.map(locale => ({ | |
| quizQuestionId: dbQ.id, | |
| locale, | |
| questionText: questions[i][locale].q, | |
| explanation: textToExplanationBlocks(questions[i][locale].exp), | |
| })) | |
| ) | |
| ); | |
| // 3. Insert answers | |
| const answerValues = insertedQuestions.flatMap((dbQ, i) => | |
| questions[i].answers.map((a, aIdx) => ({ | |
| quizQuestionId: dbQ.id, | |
| displayOrder: aIdx + 1, | |
| isCorrect: a.correct, | |
| })) | |
| ); | |
| const insertedAnswers = await tx | |
| .insert(quizAnswers) | |
| .values(answerValues) | |
| .returning({ id: quizAnswers.id }); | |
| // 4. Insert answer translations | |
| await tx.insert(quizAnswerTranslations).values( | |
| insertedAnswers.flatMap((dbA, i) => { | |
| const qIdx = Math.floor(i / 4); | |
| const aIdx = i % 4; | |
| return LOCALES.map(locale => ({ | |
| quizAnswerId: dbA.id, | |
| locale, | |
| answerText: questions[qIdx].answers[aIdx][locale], | |
| })); | |
| }) | |
| ); | |
| // 5. Update questionsCount | |
| const [countRow] = await tx | |
| .select({ count: sql<number>`COUNT(*)::int` }) | |
| .from(quizQuestions) | |
| .where(eq(quizQuestions.quizId, quizId)); | |
| await tx | |
| .update(quizzes) | |
| .set({ questionsCount: countRow.count }) | |
| .where(eq(quizzes.id, quizId)); | |
| return { addedCount: questions.length, totalCount: countRow.count }; | |
| }); | |
| await invalidateQuizCache(quizId); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/api/admin/quiz/`[id]/questions/route.ts around lines 111 - 173,
Wrap the entire multi-step DB routine (steps that create insertedQuestions,
insert into quizQuestionContent, build answerValues, insert into quizAnswers
producing insertedAnswers, insert into quizAnswerTranslations, and update
quizzes with countRow) inside a single db.transaction(...) so all operations run
on the transaction context (use the transaction object instead of top-level db
when calling insert/select/update). Use the transaction to produce
insertedQuestions/insertedAnswers and compute countRow inside it, and let the
transaction commit or roll back on error; call invalidateQuizCache(quizId) only
after the transaction resolves successfully so cache invalidation happens only
on commit. Ensure you replace references to quizQuestions, quizQuestionContent,
quizAnswers, quizAnswerTranslations, quizzes, insertedQuestions,
insertedAnswers, answerValues, and countRow to use the transaction's query
methods.
Comment on lines
+149
to
+160
| // 4. Insert answer translations | ||
| await db.insert(quizAnswerTranslations).values( | ||
| insertedAnswers.flatMap((dbA, i) => { | ||
| const qIdx = Math.floor(i / 4); | ||
| const aIdx = i % 4; | ||
| return LOCALES.map(locale => ({ | ||
| quizAnswerId: dbA.id, | ||
| locale, | ||
| answerText: questions[qIdx].answers[aIdx][locale], | ||
| })); | ||
| }) | ||
| ); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check the file structure and line count
wc -l frontend/app/api/admin/quiz/[id]/questions/route.ts
wc -l frontend/app/api/admin/quiz/route.tsRepository: DevLoversTeam/devlovers.net
Length of output: 167
🏁 Script executed:
# Read the first problematic file around lines 149-160
sed -n '140,170p' frontend/app/api/admin/quiz/[id]/questions/route.ts | cat -nRepository: DevLoversTeam/devlovers.net
Length of output: 1128
🏁 Script executed:
# Read the second file around lines 162-172
sed -n '155,180p' frontend/app/api/admin/quiz/route.ts | cat -nRepository: DevLoversTeam/devlovers.net
Length of output: 1101
🏁 Script executed:
# Search for validation schemas for quiz answers to check the constraint
fd -t f '.*' frontend/app/api --type ts | xargs rg -l 'answers' | head -20Repository: DevLoversTeam/devlovers.net
Length of output: 1011
🏁 Script executed:
# Look for schema definitions that might define answer structure
rg -A10 -B2 'answers.*schema\|schema.*answers' --type tsRepository: DevLoversTeam/devlovers.net
Length of output: 53
🏁 Script executed:
# Read the admin-quiz validation schema
cat -n frontend/lib/validation/admin-quiz.ts | head -150Repository: DevLoversTeam/devlovers.net
Length of output: 5806
🏁 Script executed:
# Check the types definition for quiz
cat -n frontend/types/quiz.tsRepository: DevLoversTeam/devlovers.net
Length of output: 1803
🏁 Script executed:
# Also check if there's any schema or type that defines the structure of questions with answers
rg -B5 -A10 'answers.*length\|answers.*\[' frontend/lib/validation/admin-quiz.tsRepository: DevLoversTeam/devlovers.net
Length of output: 53
Critical: Hardcoded assumption of exactly 4 answers per question will corrupt data if answer counts vary.
Lines 152–153 use Math.floor(i / 4) and i % 4 to map the flat insertedAnswers array back to questions[qIdx].answers[aIdx]. This only works if every question has exactly 4 answers. While the validation schema currently enforces this constraint (.length(4) in both jsonQuestionSchema and patchQuestionSchema), this hardcoding is fragile—any schema change to allow variable answer counts would silently corrupt translation assignments without raising errors.
This same bug exists in frontend/app/api/admin/quiz/route.ts at lines 160–161.
🐛 Proposed fix: compute offsets dynamically instead of assuming 4
// 4. Insert answer translations
+ // Build a mapping from flat answer index → (questionIndex, answerIndex)
+ const answerMapping: { qIdx: number; aIdx: number }[] = [];
+ for (let qi = 0; qi < questions.length; qi++) {
+ for (let ai = 0; ai < questions[qi].answers.length; ai++) {
+ answerMapping.push({ qIdx: qi, aIdx: ai });
+ }
+ }
+
await db.insert(quizAnswerTranslations).values(
- insertedAnswers.flatMap((dbA, i) => {
- const qIdx = Math.floor(i / 4);
- const aIdx = i % 4;
- return LOCALES.map(locale => ({
+ insertedAnswers.flatMap((dbA, i) =>
+ LOCALES.map(locale => ({
quizAnswerId: dbA.id,
locale,
- answerText: questions[qIdx].answers[aIdx][locale],
- }));
- })
+ answerText: questions[answerMapping[i].qIdx].answers[answerMapping[i].aIdx][locale],
+ }))
+ )
);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/api/admin/quiz/`[id]/questions/route.ts around lines 149 - 160,
The translation insert assumes 4 answers per question by using Math.floor(i / 4)
and i % 4; instead compute offsets dynamically to match the actual answers array
lengths: build translations by iterating the original questions and for each
question iterating its answers (or compute a running index/pointer into
insertedAnswers) so you pair each insertedAnswers element (from insertedAnswers)
with the corresponding questions[qIdx].answers[aIdx][locale] without hardcoding
4; update the same pattern in the other occurrence
(frontend/app/api/admin/quiz/route.ts) and keep references to
quizAnswerTranslations, insertedAnswers, questions, LOCALES and the db.insert
call when making the change.
Comment on lines
+1
to
+3
| import { count, eq } from 'drizzle-orm'; | ||
| import { NextRequest, NextResponse } from 'next/server'; | ||
| import { z } from 'zod'; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
CVE-2025-55182 CVE-2025-66478 Next.js RCE vulnerability
💡 Result:
CVE-2025-55182 and CVE-2025-66478 refer to the same critical, pre-auth remote code execution issue in the React Server Components (RSC) “Flight” protocol—often dubbed React2Shell—that can be exploited via a crafted HTTP request against vulnerable servers. CVE-2025-66478 (Next.js) was later merged/rejected as a duplicate of CVE-2025-55182 (upstream React). [1][3]
Who is affected (Next.js)
Next.js apps are affected only if they use React Server Components with the App Router and run:
- Next.js 15.x
- Next.js 16.x
- Next.js 14.3.0-canary.77+ (canary) [1][3]
Not affected (per Next.js): Next.js 13.x, Next.js 14.x stable, Pages Router apps, and the Edge Runtime. [1]
Fixed Next.js versions
Upgrade to one of these patched releases:
- 15.0.5, 15.1.9, 15.2.6, 15.3.6, 15.4.8, 15.5.7, 16.0.7 [1]
What to do now
- Patch immediately to the fixed Next.js versions above. [1]
- Next.js also provided an updater:
npx fix-react2shell-next. [1] - If your app was exposed and unpatched around initial exploitation, Next.js recommends rotating secrets (starting with the most critical). [1]
- If you maintain affected React deployments directly, AWS notes patched React lines (for React 19.x) include 19.0.1 / 19.1.2 / 19.2.1. [3]
Exploitation status
Public reporting and vendor/security research indicate exploitation began rapidly after disclosure (early December 2025), including widespread scanning/attempts and observed compromises (often dropping coin miners). [2][4]
Sources: Next.js advisory [1], Microsoft security research writeup [2], AWS security bulletin [3], VulnCheck analysis [4].
🌐 Web query:
CVE-2025-55184 CVE-2025-55183 Next.js DoS source exposure
💡 Result:
What these CVEs are (Next.js / React Server Components):
- CVE-2025-55184 (High, CVSS 7.5) – DoS: Specially crafted requests to React Server Components / Server Function endpoints can trigger pathological deserialization behavior (e.g., infinite-loop style hangs / CPU exhaustion), making the server unresponsive. The original fix path later required follow-up fixes. [1]
- CVE-2025-55183 (Medium, CVSS 5.3) – Source code exposure: Specially crafted requests to a vulnerable Server Function / Server Action can cause the server to return compiled source code for that function. This can leak business logic and hardcoded secrets in that function’s code, but runtime secrets (e.g.,
process.env.*) are not affected. [1]
Who is affected (in Next.js terms):
- Impacts apps using React Server Components, which in Next.js typically means App Router-based apps in affected Next.js versions. Pages Router apps are not affected. [2]
What to do (patch versions):
- React RSC packages: React reports these issues in
react-server-dom-webpack,react-server-dom-parcel, andreact-server-dom-turbopack, and recommends upgrading to the fixed backports 19.0.4 / 19.1.5 / 19.2.4. [1] - Next.js: Vercel’s bulletin lists patched Next.js versions (by release line), including:
- 14.x → 14.2.35
- 15.0.x → 15.0.7, 15.1.x → 15.1.11, 15.2.x → 15.2.8, 15.3.x → 15.3.8, 15.4.x → 15.4.10, 15.5.x → 15.5.9
- 16.0.x → 16.0.10 [2]
Operational notes:
- If you may have hardcoded secrets inside Server Actions/Functions, rotate those secrets after patching. [1]
- If you host on Vercel, they added WAF rules as a defense-in-depth measure, but you still must upgrade. [2]
Sources:
[1] React blog advisory (Dec 11, 2025; updated Jan 26, 2026) (react.dev)
[2] Vercel Security Bulletin (last updated Dec 26, 2025) (vercel.com)
Citations:
- 1: https://react.dev/blog/2025/12/11/denial-of-service-and-source-code-exposure-in-react-server-components
- 2: https://vercel.com/kb/bulletin/security-bulletin-cve-2025-55184-and-cve-2025-55183
🌐 Web query:
Next.js 16.1.6 LTS security update February 2026
💡 Result:
- Latest Next.js 16 LTS as of Feb 22, 2026: v16.1.6, released Jan 27, 2026. [1]
- What 16.1.6 is: an LTS backport bugfix release (not “all canary changes”). Notable items called out in the v16.1.6 notes include SWC upgrade and response-cache/LRU fixes (minimal mode response cache changes). [2]
- Security context (Jan–Feb 2026):
- The React Server Components DoS issue CVE-2026-23864 is listed as fixed for Next.js 16.1.x in 16.1.5 (so 16.1.6 includes the fix as it’s newer). [3]
- Next.js also disclosed CVE-2025-59471 (and CVE-2025-59472) as being patched in the same wave of releases around late Jan 2026; versions <16.1.5 are listed as affected for CVE-2025-59471. [4][5]
- Upgrade guidance: for Next.js 16 (Active LTS), upgrade to 16.1.6 (or at minimum 16.1.5 if you’re targeting just the RSC DoS fix). [1][3][6]
Sources: [1] [2] [3] [4] [5] [6]
Upgrade Next.js to 16.1.6 — Current version has unpatched vulnerabilities
Next.js 16.0.7 is patched for the critical Remote Code Execution (CVE-2025-55182 / CVE-2025-66478) in React Server Components, but remains vulnerable to two additional issues:
- CVE-2025-55184 (High, CVSS 7.5): Denial of Service via specially crafted RSC requests
- CVE-2025-55183 (Medium, CVSS 5.3): Source code exposure in Server Functions
Both require upgrading to at least 16.0.10. The latest stable LTS release is Next.js 16.1.6 (January 2026), which includes all security patches. Upgrade next to ^16.1.6 as soon as possible.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/api/admin/quiz/`[id]/route.ts around lines 1 - 3, The project is
using an insecure Next.js release; update the Next.js dependency to "^16.1.6"
(or higher stable LTS) in your package manifest, regenerate the lockfile
(npm/yarn/pnpm install) and commit the updated lockfile, then run the test suite
and a local dev build to verify routes that use NextRequest/NextResponse and
server components still work; ensure CI installs the new lockfile and redeploys
with the patched version.
| type EditorState = { | ||
| locales: Record<AdminLocale, LocaleContent>; | ||
| answers: AnswerState[]; | ||
| difficulty: string; |
Contributor
There was a problem hiding this comment.
difficulty should use the enum union type; non-null assertion is unsafe
Three related typing issues:
- Line 32:
difficulty: stringinEditorStateshould be'beginner' | 'medium' | 'advanced'to match the Zod enum and prevent accidental assignment of arbitrary strings. - Line 63:
question.difficulty!— the non-null assertion silently passesundefinedfor records that pre-date thedifficultycolumn (e.g., before migration 0015). This producesdifficulty: undefinedin state, which leaves the<select>uncontrolled. Use a safe default instead. - Line 233:
handleDifficultyChange(value: string)should accept the narrower union type.
✏️ Proposed fix
+type Difficulty = 'beginner' | 'medium' | 'advanced';
+
type EditorState = {
locales: Record<AdminLocale, LocaleContent>;
answers: AnswerState[];
- difficulty: string;
+ difficulty: Difficulty;
};- difficulty: question.difficulty!,
+ difficulty: (question.difficulty as Difficulty | null | undefined) ?? 'beginner',- function handleDifficultyChange(value: string) {
+ function handleDifficultyChange(value: Difficulty) {
setEditorState(prev => ({ ...prev, difficulty: value }));
}And update the onChange handler accordingly:
- onChange={e => handleDifficultyChange(e.target.value)}
+ onChange={e => handleDifficultyChange(e.target.value as Difficulty)}Also applies to: 63-63, 233-235
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/components/admin/quiz/QuestionEditor.tsx` at line 32, EditorState
currently types difficulty as string and code uses a non-null assertion
(question.difficulty!) and a wide handler signature; change
EditorState.difficulty to the union 'beginner' | 'medium' | 'advanced',
initialize state from question.difficulty with a safe default (e.g.,
question.difficulty ?? 'beginner') instead of using the non-null assertion so
undefined never flows into the select, and tighten handleDifficultyChange to
accept the same union type for its parameter; update the select onChange to pass
that narrowed value.
Comment on lines
+123
to
+153
| const isDirty = dirtyLocales.size > 0 || editorState.difficulty !== initialStateRef.current.difficulty; | ||
|
|
||
| const [deleting, setDeleting] = useState(false); | ||
|
|
||
| async function handleDelete() { | ||
| const confirmed = window.confirm('Delete this question? This cannot be undone.'); | ||
| if (!confirmed) return; | ||
|
|
||
| setDeleting(true); | ||
| try { | ||
| const res = await fetch( | ||
| `/api/admin/quiz/${quizId}/questions/${question.id}`, | ||
| { | ||
| method: 'DELETE', | ||
| headers: { 'x-csrf-token': csrfTokenDelete ?? '' }, | ||
| } | ||
| ); | ||
|
|
||
| if (!res.ok) { | ||
| const data = await res.json().catch(() => ({})); | ||
| alert(data.error ?? 'Failed to delete question'); | ||
| return; | ||
| } | ||
|
|
||
| router.refresh(); | ||
| } catch { | ||
| alert('Network error'); | ||
| } finally { | ||
| setDeleting(false); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Save always fails when only difficulty is changed (empty dirtyLocales fails server validation)
isDirty is true whenever difficulty changes (line 123), so the Save button becomes available. However, dirtyLocales stays empty in that case. handleSave sends dirtyLocales: [] in the request body, and the server's patchQuestionSchema enforces .min(1, 'At least one locale must be dirty') on that field — resulting in a guaranteed 400 every time a user saves a difficulty-only change.
The confirmation dialog also misfires: with dirtyLocales.size === 0, modified renders as an empty string, producing the confusing prompt "You only modified . EN, UK, PL will be saved with existing content unchanged."
The cleanest fix is to mark all locales dirty when difficulty changes, consistent with what handleCorrectChange already does:
🐛 Proposed fix
function handleDifficultyChange(value: string) {
setEditorState(prev => ({ ...prev, difficulty: value }));
+ setDirtyLocales(new Set(ALL_LOCALES));
}Alternatively, update patchQuestionSchema on the server to make dirtyLocales optional when difficulty is present — but the client-side fix is simpler and keeps both sides consistent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/components/admin/quiz/QuestionEditor.tsx` around lines 123 - 153,
isDirty currently becomes true when editorState.difficulty !==
initialStateRef.current.difficulty but dirtyLocales remains empty, causing
handleSave to send an empty dirtyLocales and server validation to fail; update
the client to mark all locales dirty when difficulty changes (same approach as
handleCorrectChange) so dirtyLocales contains every locale before save and the
confirmation "modified" string is non-empty; specifically, in the difficulty
change handler (where editorState.difficulty is updated) add all locale keys to
the dirtyLocales Set, and ensure isDirty uses dirtyLocales.size > 0 ||
editorState.difficulty !== initialStateRef.current.difficulty so handleSave and
the prompt receive the populated dirtyLocales.
Comment on lines
+55
to
+78
| export const jsonQuestionSchema = z.object({ | ||
| id: z.string().optional(), | ||
| order: z.number().int().positive(), | ||
| difficulty: z.enum(['beginner', 'medium', 'advanced']), | ||
| uk: z.object({ | ||
| q: z.string().min(1), | ||
| exp: z.string().min(1), | ||
| }), | ||
| en: z.object({ | ||
| q: z.string().min(1), | ||
| exp: z.string().min(1), | ||
| }), | ||
| pl: z.object({ | ||
| q: z.string().min(1), | ||
| exp: z.string().min(1), | ||
| }), | ||
| answers: z | ||
| .array(jsonQuestionAnswerSchema) | ||
| .length(4) | ||
| .refine( | ||
| answers => answers.filter(a => a.correct).length === 1, | ||
| { message: 'Exactly one correct answer required' } | ||
| ), | ||
| }); |
Contributor
There was a problem hiding this comment.
id field should enforce UUID format; exp plain-string type diverges from the stored block-array format
Two related concerns in jsonQuestionSchema:
-
id: z.string().optional()(line 56) accepts any non-empty string. Since question IDs are UUIDs everywhere else in the codebase (paramsSchema,patchQuestionAnswerSchema), this should bez.string().uuid().optional()to reject malformed values at the schema boundary. -
exp: z.string().min(1)vs block-array (lines 61, 65, 69) —patchQuestionSchemastores explanations asz.array(z.unknown())(rich-text blocks), but the JSON upload schema accepts a plain string forexp. This means the API route that handlescreateQuizSchema/addQuestionsSchemamust convert the string into the block format before persisting. That conversion is currently an undocumented implicit contract; consider adding a comment here (or enforcing anexpBlockstype after conversion) so future contributors know this transform is required.
✏️ Suggested fix for the id field
export const jsonQuestionSchema = z.object({
- id: z.string().optional(),
+ id: z.string().uuid().optional(),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export const jsonQuestionSchema = z.object({ | |
| id: z.string().optional(), | |
| order: z.number().int().positive(), | |
| difficulty: z.enum(['beginner', 'medium', 'advanced']), | |
| uk: z.object({ | |
| q: z.string().min(1), | |
| exp: z.string().min(1), | |
| }), | |
| en: z.object({ | |
| q: z.string().min(1), | |
| exp: z.string().min(1), | |
| }), | |
| pl: z.object({ | |
| q: z.string().min(1), | |
| exp: z.string().min(1), | |
| }), | |
| answers: z | |
| .array(jsonQuestionAnswerSchema) | |
| .length(4) | |
| .refine( | |
| answers => answers.filter(a => a.correct).length === 1, | |
| { message: 'Exactly one correct answer required' } | |
| ), | |
| }); | |
| export const jsonQuestionSchema = z.object({ | |
| id: z.string().uuid().optional(), | |
| order: z.number().int().positive(), | |
| difficulty: z.enum(['beginner', 'medium', 'advanced']), | |
| uk: z.object({ | |
| q: z.string().min(1), | |
| exp: z.string().min(1), | |
| }), | |
| en: z.object({ | |
| q: z.string().min(1), | |
| exp: z.string().min(1), | |
| }), | |
| pl: z.object({ | |
| q: z.string().min(1), | |
| exp: z.string().min(1), | |
| }), | |
| answers: z | |
| .array(jsonQuestionAnswerSchema) | |
| .length(4) | |
| .refine( | |
| answers => answers.filter(a => a.correct).length === 1, | |
| { message: 'Exactly one correct answer required' } | |
| ), | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/lib/validation/admin-quiz.ts` around lines 55 - 78,
jsonQuestionSchema currently allows any string for id and uses plain strings for
explanation fields, which mismatches the rest of the codebase: change id in
jsonQuestionSchema from z.string().optional() to z.string().uuid().optional() to
enforce UUIDs, and annotate/adjust the exp fields (uk.exp, en.exp, pl.exp) to
reflect that the persisted model expects rich-text block arrays (or rename to
expPlain and add an expBlocks field) and ensure the API handlers that use
createQuizSchema / addQuestionsSchema convert these plain-string exp values into
the block-array format expected by patchQuestionSchema before saving; add a
short inline comment on jsonQuestionSchema documenting the required conversion
so future reviewers see the contract.
- categories/route: replace TOCTOU slug check with onConflictDoNothing, add orphan cleanup on translation insert failure - quiz/route: wrap steps 2-6 in try/catch, delete quiz on failure (CASCADE) - quiz/[id]/questions/route: wrap steps 2-4 in try/catch, cleanup on failure - quiz/[id]/questions/[questionId]/route: remove unused `count` import - QuestionEditor: add Difficulty union type, allow difficulty-only saves (skip locale validation and confirmation when dirtyLocales is empty) - admin-quiz.ts: dirtyLocales allows empty when difficulty present (.refine), jsonQuestionSchema id field enforces UUID format - JsonUploadArea: add keyboard accessibility (role, tabIndex, onKeyDown) - QuizMetadataEditor: round time limit display to nearest minute
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (9)
frontend/components/admin/quiz/QuestionEditor.tsx (2)
499-516: Consider disabling the Save button when!isDirty
isDirtyis already computed on line 124 but not used to gate the Save button. A user can click Save with zero changes, firing a request that sendsdirtyLocales: []anddifficulty: undefined— a no-op round-trip.♻️ Proposed change
- disabled={saveStatus === 'saving' || saveStatus === 'saved'} + disabled={saveStatus === 'saving' || saveStatus === 'saved' || !isDirty}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/admin/quiz/QuestionEditor.tsx` around lines 499 - 516, The Save button should be disabled when there are no changes: update the disabled predicate on the button that uses handleSave and saveStatus to also include !isDirty (i.e. disabled when saveStatus === 'saving' || saveStatus === 'saved' || !isDirty) and ensure the visual disabled styling still applies when !isDirty (the cn call around className for saveStatus can remain but confirm disabled:opacity-50 covers the new case); also ensure any early-return or validation in handleSave honors isDirty so no network request is sent when isDirty is false.
124-126:deletingstate declaration is separated from the otheruseStatecalls
const [deleting, setDeleting](line 126) is visually isolated from the rest of the hook declarations (lines 113–122) by theisDirtyderived constant and carries extra indentation. Grouping alluseStatecalls together at the top of the component makes the state shape easier to scan.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/admin/quiz/QuestionEditor.tsx` around lines 124 - 126, Move the deleting state declaration next to the other useState hooks so all state hooks are declared together at the top of the component; specifically relocate "const [deleting, setDeleting] = useState(false)" so it sits with the other useState calls (those around editorState, locale-related hooks) instead of being separated by the derived "isDirty" constant, and remove the extra indentation so the hook grouping and visual structure are consistent.frontend/components/admin/quiz/QuizMetadataEditor.tsx (2)
30-42:buildDraftcan be hoisted to module scopeThe function has no closures over component state — it only uses its arguments and the module-level
LOCALESconstant. Defining it inside the component body creates a new function object every render and obscures its pure-utility nature.♻️ Proposed refactor
+function buildDraft( + trans: Record<string, { title: string; description: string | null }>, + timeLimit: number | null +) { + const t: Record<string, { title: string; description: string }> = {}; + for (const locale of LOCALES) { + t[locale] = { + title: trans[locale]?.title ?? '', + description: trans[locale]?.description ?? '', + }; + } + return { translations: t, timeLimitMinutes: timeLimit ? String(timeLimit / 60) : '' }; +} export function QuizMetadataEditor({ ... }) { ... - function buildDraft( - trans: Record<string, { title: string; description: string | null }>, - timeLimit: number | null - ) { - ... - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/admin/quiz/QuizMetadataEditor.tsx` around lines 30 - 42, Hoist the pure utility function buildDraft out of the component body into module scope (above the component) since it only depends on its arguments and the module-level LOCALES constant; move the function definition (including the typed parameters trans: Record<string,{title:string;description:string|null}> and timeLimit: number|null and the local type t) to the top-level, keep using LOCALES directly, and update any references in the component to call the now-module-level buildDraft — export it only if other modules need it.
119-120: StaleisDirtykeepsbeforeunloadactive after a successful save
draftis never reset aftersetEditing(false), soisDirtystaystrueuntilrouter.refresh()fully propagates the updatedtranslationsprop. During that async window thebeforeunloadguard is still attached — a navigating user would see a spurious "Discard unsaved changes?" dialog even though their data was already saved.♻️ Proposed fix — reset draft to the just-saved values immediately
setEditing(false); + // Collapse isDirty before the async refresh arrives so beforeunload is detached. + setDraft(buildDraft( + Object.fromEntries( + LOCALES.map(l => [l, draft.translations[l]]) + ), + timeLimitSec + )); router.refresh();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/admin/quiz/QuizMetadataEditor.tsx` around lines 119 - 120, After a successful save, reset the local draft state to the just-saved values immediately so isDirty goes false and the beforeunload guard detaches; specifically, in the save completion block where setEditing(false) and router.refresh() are called, set the draft (the state variable named draft) to the saved translations (or the value you just persisted) before calling router.refresh() so the UI and the beforeunload check reflect the saved state without waiting for the refreshed props to propagate.frontend/app/api/admin/quiz/[id]/questions/[questionId]/route.ts (1)
111-117: Inconsistent indentation and minor style nit on difficulty update.Line 111 has extra leading whitespace (8 spaces) compared to the surrounding code (4 spaces). Also, the condition
if (parsed.data.difficulty)works because all enum values are truthy, but!== undefinedwould be more explicit about the intent (checking presence rather than truthiness).Proposed fix
- // Update difficulty if provided - if (parsed.data.difficulty) { + // Update difficulty if provided + if (parsed.data.difficulty !== undefined) { await db .update(quizQuestions) .set({ difficulty: parsed.data.difficulty }) .where(eq(quizQuestions.id, questionId)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/api/admin/quiz/`[id]/questions/[questionId]/route.ts around lines 111 - 117, Fix the indentation and make the presence check explicit for difficulty: align the block updating difficulty to the same 4-space indent as surrounding code and change the condition from if (parsed.data.difficulty) to if (parsed.data.difficulty !== undefined) so you update via db.update(quizQuestions).set({ difficulty: parsed.data.difficulty }).where(eq(quizQuestions.id, questionId)) only when the field is provided.frontend/app/api/admin/quiz/[id]/questions/route.ts (1)
37-39: DuplicatedtextToExplanationBlockshelper across route files.This exact function also exists in
frontend/app/api/admin/quiz/route.ts(line 35-37). Consider extracting it to a shared utility.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/api/admin/quiz/`[id]/questions/route.ts around lines 37 - 39, The helper function textToExplanationBlocks is duplicated; extract it into a shared utility module (e.g., export a function named textToExplanationBlocks from a new util file) and replace the local definitions in both route files with an import of that exported function; ensure the util exports the same function signature (text: string) and update the two route modules to import and use textToExplanationBlocks instead of defining it inline.frontend/components/admin/quiz/JsonUploadArea.tsx (1)
54-60: Potential stale-closure onfilesstate inhandleFiles.Line 55 reads
filesfrom the closure, but sincehandleFilesisasync(it awaitsfile.text()), React state could have been updated by a concurrentremoveFilecall beforesetFilesruns. Using a functional updater avoids this race:Proposed fix
- const updated = [...files, ...newFiles]; - setFiles(updated); - updateParent(updated); + setFiles(prev => { + const updated = [...prev, ...newFiles]; + // Call updateParent with the new list + const merged = updated.flatMap(f => f.questions); + onQuestionsChange(merged); + return updated; + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/admin/quiz/JsonUploadArea.tsx` around lines 54 - 60, The async handleFiles uses the closed-over files variable which can be stale if state changes concurrently; change setFiles to use a functional updater and call updateParent with the new array inside that updater to avoid the race. Specifically, in handleFiles replace setFiles(updated); updateParent(updated); with setFiles(prev => { const merged = [...prev, ...newFiles]; updateParent(merged); return merged; }); then keep the inputRef.current.value = '' reset as before. Use the function name handleFiles and state setters setFiles and updateParent when making this change.frontend/lib/validation/admin-quiz.ts (1)
113-125: Consider adding constraints to prevent conflictingpatchQuizSchemapayloads.The schema allows sending
status: 'draft'andisActive: truesimultaneously, which would be semantically contradictory (a draft quiz shouldn't be active). While the API route handler likely validates this, a schema-level refinement could catch it earlier.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/lib/validation/admin-quiz.ts` around lines 113 - 125, Update the patchQuizSchema refinement to also reject semantically conflicting combinations: add a refine on the existing patchQuizSchema (or extend the current .refine chain) that returns false when data.status === 'draft' && data.isActive === true (and optionally when data.status === 'ready' && data.isActive === false if you want the inverse constraint), and provide a clear message like "Cannot set draft quizzes as active" so the validation fails early; target the const patchQuizSchema and its existing refine block to implement this check.frontend/app/api/admin/quiz/route.ts (1)
35-37: DuplicatedtextToExplanationBlockshelper.This function is identical to the one in
frontend/app/api/admin/quiz/[id]/questions/route.ts(line 37-39). Extract to a shared utility to avoid drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/api/admin/quiz/route.ts` around lines 35 - 37, The helper function textToExplanationBlocks is duplicated; extract it into a shared utility module (e.g., create and export textToExplanationBlocks from a new util file) and replace both local definitions with an import of that exported function; ensure the exported function signature and return shape match the existing implementation and update the two places that currently define textToExplanationBlocks to import and use the shared utility instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/app/api/admin/quiz/`[id]/questions/route.ts:
- Around line 161-168: The rollback delete currently builds the WHERE clause
with a sql template using insertedQuestions -> questionIds which treats the JS
array as a single bound value; replace that with drizzle-orm's inArray helper so
the delete properly expands the array. Update the catch block where insertError
is handled to call db.delete(quizQuestions).where(inArray(quizQuestions.id,
questionIds)) (and add the inArray import if missing) so the CASCADE cleanup
reliably deletes the insertedQuestions by id.
In `@frontend/components/admin/quiz/JsonUploadArea.tsx`:
- Around line 74-80: The onKeyDown handler on the interactive div (the handler
that checks e.key === 'Enter' || e.key === ' ') should call e.preventDefault()
when the Space key is pressed so the page does not scroll; update the div's
onKeyDown to detect the Space key (the current ' ' check) and call
e.preventDefault() before invoking inputRef.current?.click() (leave Enter
behavior unchanged).
In `@frontend/components/admin/quiz/QuizMetadataEditor.tsx`:
- Around line 177-196: The Title/Description labels aren’t linked to their
controls so clicking them won’t focus the inputs and screen readers can’t
associate them; update the JSX in QuizMetadataEditor to add unique id attributes
to the input/textarea (e.g. `id={`title-${activeLocale}`}` and
`id={`description-${activeLocale}`}`) and set the corresponding label htmlFor to
those ids, doing the same for the Time Limit label/control; keep using the
existing symbols (draft.translations[activeLocale].title,
draft.translations[activeLocale].description, updateField, LOCALE_LABELS,
activeLocale) so the handlers and values remain unchanged.
- Around line 47-52: The beforeunload handler in the QuizMetadataEditor
useEffect (the handler created as (e: BeforeUnloadEvent) => e.preventDefault())
should also set e.returnValue (e.g., e.returnValue = '') in addition to calling
e.preventDefault() to ensure cross-browser compatibility; update the handler
inside the useEffect that uses isDirty and the
window.addEventListener('beforeunload', handler) / removeEventListener call so
the handler sets both e.preventDefault() and e.returnValue before returning.
---
Duplicate comments:
In `@frontend/app/api/admin/categories/route.ts`:
- Around line 83-95: Wrap the category creation + translation inserts in a
single database transaction so the translations insert cannot leave an orphaned
category; replace the current try/catch that calls
db.insert(categoryTranslations) and on error calls db.delete(categories) with a
transactional block (e.g. using db.transaction or your ORM's transaction API)
that performs the category insert and the db.insert(categoryTranslations). On
error let the transaction rollback automatically (do not call
db.delete(categories) manually) and rethrow the error; reference the existing
symbols categoryTranslations, categories, db.insert and the current deletion
logic to locate where to change to a transaction.
In `@frontend/app/api/admin/quiz/`[id]/questions/route.ts:
- Around line 111-168: Wrap the entire multi-step insert flow in a single DB
transaction so it’s atomic: replace the try/catch/manual cascade delete with
db.transaction(async (tx) => { ... }) and perform all operations using the
transaction handle (use tx.insert(...) and tx.delete(...) if needed) for the
steps that create quizQuestions, quizQuestionContent, quizAnswers, and
quizAnswerTranslations (referencing quizQuestions, quizQuestionContent,
quizAnswers, quizAnswerTranslations, insertedQuestions, insertedAnswers,
questions, LOCALES, textToExplanationBlocks). Rely on the transaction to
auto-rollback on error (remove the manual delete and rethrow logic) and ensure
you return/consume the inserted IDs coming from tx.insert(...).returning(...)
inside the transaction.
- Around line 149-160: The code assumes exactly 4 answers per question by using
Math.floor(i / 4) and i % 4; change it to not hardcode 4 — either compute
answersPerQuestion dynamically (e.g., const answersPerQuestion =
questions[0].answers.length and use Math.floor(i / answersPerQuestion) / i %
answersPerQuestion) if every question has the same count, or (safer) build the
quizAnswerTranslations by iterating questions and their answers and consuming
insertedAnswers with a pointer (e.g., let idx = 0; for each question q and each
answer a { const dbA = insertedAnswers[idx++]; LOCALES.map(...) }) so you no
longer rely on fixed counts; update the insertion code that references
insertedAnswers, questions, LOCALES, and quizAnswerTranslations accordingly.
In `@frontend/app/api/admin/quiz/route.ts`:
- Around line 162-173: The insertion currently assumes 4 answers per question by
using Math.floor(i / 4) and i % 4; replace that with a dynamic mapping that does
not hardcode 4: build the quizAnswerTranslations values by iterating over
questions and their answers (e.g., for each question index q and for each answer
index a within questions[q].answers) and mapping to the corresponding
insertedAnswers entry (or maintain a running index into insertedAnswers) so
quizAnswerId = insertedAnswers[matchingIndex].id, locale = LOCALES[k], and
answerText = questions[q].answers[a][locale]; update the code that calls
db.insert(quizAnswerTranslations).values(...) to use this dynamic iteration
instead of Math.floor(i / 4) and i % 4.
- Around line 113-178: Wrap the multi-step insert sequence in a single database
transaction so all inserts are atomic: replace the try/catch + manual cascade
delete with db.transaction(async (tx) => { ... }) and perform all operations
using the transaction handle (use tx.insert(...) / tx.delete(...) /
tx.insert(...).returning(...) etc. Move the LOCALES map/insert logic, the
quizTranslations insert, the quizQuestions insert and returning,
quizQuestionContent insert, quizAnswers insert and returning, and
quizAnswerTranslations insert into the transaction block, and remove the catch
block that deletes via db.delete(quizzes) — let the transaction rollback by
re-throwing any error from inside the transaction. Ensure you reference
quizTranslations, quizQuestions, quizQuestionContent, quizAnswers,
quizAnswerTranslations, quizzes, and the insertedQuestions/insertedAnswers
variables inside the tx context.
In `@frontend/components/admin/quiz/QuestionEditor.tsx`:
- Around line 234-236: handleDifficultyChange currently only updates editorState
and does not mark locales dirty, so difficulty-only saves still send
dirtyLocales: [] to the server and trigger validation failures; update
handleDifficultyChange to call setDirtyLocales(new Set(ALL_LOCALES)) (or add the
same lines used elsewhere to mark all locales dirty) whenever difficulty is
changed, so Array.from(dirtyLocales) contains at least one locale during the
PATCH, and/or verify and document that the server-side patchQuestionSchema has
been relaxed to allow an empty dirtyLocales when difficulty is present (if
server change exists, add a comment referencing patchQuestionSchema instead of
changing client code).
- Line 64: The code uses a type assertion "question.difficulty as Difficulty"
which does not guard at runtime and lets editorState.difficulty become
null/undefined (causing the <select value={editorState.difficulty}> to be
uncontrolled); change the assignment in QuestionEditor (where editorState is
initialized) to perform a runtime fallback like question.difficulty ??
<SOME_VALID_DIFFICULTY> (or map invalid values to a valid Difficulty) so
editorState.difficulty is always a valid Difficulty enum/string; update any
initialization/restore logic that sets editorState.difficulty (search for
"question.difficulty as Difficulty" and the editorState initialization in
QuestionEditor.tsx) to use this nullish-coalescing fallback instead.
In `@frontend/lib/validation/admin-quiz.ts`:
- Around line 56-57: The jsonQuestionSchema's id should accept arbitrary source
identifiers (non-UUIDs) — update the schema definition in jsonQuestionSchema to
use id: z.string().optional() (remove any .uuid() or UUID refinements previously
added) so uploaded JSON IDs like "sql-fundamentals-q01" remain valid; ensure
only the jsonQuestionSchema is changed (not DB models) and run/schema tests to
confirm validation accepts non-UUID strings.
---
Nitpick comments:
In `@frontend/app/api/admin/quiz/`[id]/questions/[questionId]/route.ts:
- Around line 111-117: Fix the indentation and make the presence check explicit
for difficulty: align the block updating difficulty to the same 4-space indent
as surrounding code and change the condition from if (parsed.data.difficulty) to
if (parsed.data.difficulty !== undefined) so you update via
db.update(quizQuestions).set({ difficulty: parsed.data.difficulty
}).where(eq(quizQuestions.id, questionId)) only when the field is provided.
In `@frontend/app/api/admin/quiz/`[id]/questions/route.ts:
- Around line 37-39: The helper function textToExplanationBlocks is duplicated;
extract it into a shared utility module (e.g., export a function named
textToExplanationBlocks from a new util file) and replace the local definitions
in both route files with an import of that exported function; ensure the util
exports the same function signature (text: string) and update the two route
modules to import and use textToExplanationBlocks instead of defining it inline.
In `@frontend/app/api/admin/quiz/route.ts`:
- Around line 35-37: The helper function textToExplanationBlocks is duplicated;
extract it into a shared utility module (e.g., create and export
textToExplanationBlocks from a new util file) and replace both local definitions
with an import of that exported function; ensure the exported function signature
and return shape match the existing implementation and update the two places
that currently define textToExplanationBlocks to import and use the shared
utility instead.
In `@frontend/components/admin/quiz/JsonUploadArea.tsx`:
- Around line 54-60: The async handleFiles uses the closed-over files variable
which can be stale if state changes concurrently; change setFiles to use a
functional updater and call updateParent with the new array inside that updater
to avoid the race. Specifically, in handleFiles replace setFiles(updated);
updateParent(updated); with setFiles(prev => { const merged = [...prev,
...newFiles]; updateParent(merged); return merged; }); then keep the
inputRef.current.value = '' reset as before. Use the function name handleFiles
and state setters setFiles and updateParent when making this change.
In `@frontend/components/admin/quiz/QuestionEditor.tsx`:
- Around line 499-516: The Save button should be disabled when there are no
changes: update the disabled predicate on the button that uses handleSave and
saveStatus to also include !isDirty (i.e. disabled when saveStatus === 'saving'
|| saveStatus === 'saved' || !isDirty) and ensure the visual disabled styling
still applies when !isDirty (the cn call around className for saveStatus can
remain but confirm disabled:opacity-50 covers the new case); also ensure any
early-return or validation in handleSave honors isDirty so no network request is
sent when isDirty is false.
- Around line 124-126: Move the deleting state declaration next to the other
useState hooks so all state hooks are declared together at the top of the
component; specifically relocate "const [deleting, setDeleting] =
useState(false)" so it sits with the other useState calls (those around
editorState, locale-related hooks) instead of being separated by the derived
"isDirty" constant, and remove the extra indentation so the hook grouping and
visual structure are consistent.
In `@frontend/components/admin/quiz/QuizMetadataEditor.tsx`:
- Around line 30-42: Hoist the pure utility function buildDraft out of the
component body into module scope (above the component) since it only depends on
its arguments and the module-level LOCALES constant; move the function
definition (including the typed parameters trans:
Record<string,{title:string;description:string|null}> and timeLimit: number|null
and the local type t) to the top-level, keep using LOCALES directly, and update
any references in the component to call the now-module-level buildDraft — export
it only if other modules need it.
- Around line 119-120: After a successful save, reset the local draft state to
the just-saved values immediately so isDirty goes false and the beforeunload
guard detaches; specifically, in the save completion block where
setEditing(false) and router.refresh() are called, set the draft (the state
variable named draft) to the saved translations (or the value you just
persisted) before calling router.refresh() so the UI and the beforeunload check
reflect the saved state without waiting for the refreshed props to propagate.
In `@frontend/lib/validation/admin-quiz.ts`:
- Around line 113-125: Update the patchQuizSchema refinement to also reject
semantically conflicting combinations: add a refine on the existing
patchQuizSchema (or extend the current .refine chain) that returns false when
data.status === 'draft' && data.isActive === true (and optionally when
data.status === 'ready' && data.isActive === false if you want the inverse
constraint), and provide a clear message like "Cannot set draft quizzes as
active" so the validation fails early; target the const patchQuizSchema and its
existing refine block to implement this check.
Comment on lines
+47
to
+52
| useEffect(() => { | ||
| if (!isDirty) return; | ||
| const handler = (e: BeforeUnloadEvent) => e.preventDefault(); | ||
| window.addEventListener('beforeunload', handler); | ||
| return () => window.removeEventListener('beforeunload', handler); | ||
| }, [isDirty]); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
beforeunload preventDefault returnValue browser compatibility 2025
💡 Result:
- Spec status (2025):
BeforeUnloadEvent.returnValueis legacy; the HTML Standard says authors should cancel viaevent.preventDefault()rather than relying onreturnValue. [2] - What works across modern browsers (Chrome/Edge, Firefox, Safari):
- In a
beforeunloadlistener, callevent.preventDefault()to request the confirmation dialog. [1] - Also set
event.returnValueto a truthy value for legacy/compat cases (MDN notes this is needed for older Chromium builds, e.g. Chrome/Edge < 119). [1]
- In a
- Handler return value compatibility nuance: Returning a truthy value only works when using the
window.onbeforeunload = fnproperty, not when usingaddEventListener("beforeunload", fn). [1] - User-gesture requirement (important for “it doesn’t work” reports): Modern browsers only show the dialog if the page has had sticky activation (some prior user interaction). [1]
- Dialog text: Browsers show a generic message; sites can’t customize it anymore. [1]
- Reliability/perf:
beforeunloadis not reliably fired on mobile in several scenarios, and its use is recommended to be limited/conditional; prefervisibilitychange/pagehidefor saving state. [1] (Related platform guidance also discourages unload-style handlers.) [3]
Recommended pattern (most compatible):
window.addEventListener("beforeunload", (e) => {
e.preventDefault(); // standard
e.returnValue = true; // legacy/compat
});Sources: MDN beforeunload event (last modified Jun 23, 2025) [1], WHATWG HTML Standard BeforeUnloadEvent (notes returnValue is legacy) [2], Chrome platform guidance on unload/beforeunload caveats (updated Jan 29, 2026) [3].
Set e.returnValue alongside e.preventDefault() for cross-browser beforeunload compatibility
e.preventDefault() alone doesn't reliably trigger the native confirmation dialog in older browsers. While modern versions of Chrome/Edge, Firefox, and Safari handle preventDefault() correctly, the recommended pattern for maximum compatibility is to set both preventDefault() and returnValue:
- const handler = (e: BeforeUnloadEvent) => e.preventDefault();
+ const handler = (e: BeforeUnloadEvent) => {
+ e.preventDefault();
+ e.returnValue = '';
+ };This ensures the unload guard works consistently across legacy and modern browsers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/components/admin/quiz/QuizMetadataEditor.tsx` around lines 47 - 52,
The beforeunload handler in the QuizMetadataEditor useEffect (the handler
created as (e: BeforeUnloadEvent) => e.preventDefault()) should also set
e.returnValue (e.g., e.returnValue = '') in addition to calling
e.preventDefault() to ensure cross-browser compatibility; update the handler
inside the useEffect that uses isDirty and the
window.addEventListener('beforeunload', handler) / removeEventListener call so
the handler sets both e.preventDefault() and e.returnValue before returning.
Comment on lines
+177
to
+196
| <label className="text-foreground mb-1 block text-xs font-medium"> | ||
| Title ({LOCALE_LABELS[activeLocale]}) | ||
| </label> | ||
| <input | ||
| type="text" | ||
| value={draft.translations[activeLocale].title} | ||
| onChange={e => updateField(activeLocale, 'title', e.target.value)} | ||
| className="border-border bg-background text-foreground w-full rounded-md border px-3 py-2 text-sm" | ||
| /> | ||
| </div> | ||
| <div> | ||
| <label className="text-foreground mb-1 block text-xs font-medium"> | ||
| Description ({LOCALE_LABELS[activeLocale]}) | ||
| </label> | ||
| <textarea | ||
| rows={3} | ||
| value={draft.translations[activeLocale].description} | ||
| onChange={e => updateField(activeLocale, 'description', e.target.value)} | ||
| className="border-border bg-background text-foreground w-full rounded-md border px-3 py-2 text-sm" | ||
| /> |
Contributor
There was a problem hiding this comment.
Labels not wired to their inputs (htmlFor/id missing)
None of the <label> + <input>/<textarea> pairs use htmlFor/id, so clicking a label does not focus its control, and screen readers cannot programmatically associate them. The same applies to the Time Limit label at line 202.
♻️ Proposed fix for title and description (apply same pattern to Time Limit)
- <label className="text-foreground mb-1 block text-xs font-medium">
+ <label
+ htmlFor={`title-${activeLocale}`}
+ className="text-foreground mb-1 block text-xs font-medium"
+ >
Title ({LOCALE_LABELS[activeLocale]})
</label>
<input
+ id={`title-${activeLocale}`}
type="text"
...
/>
</div>
<div>
- <label className="text-foreground mb-1 block text-xs font-medium">
+ <label
+ htmlFor={`description-${activeLocale}`}
+ className="text-foreground mb-1 block text-xs font-medium"
+ >
Description ({LOCALE_LABELS[activeLocale]})
</label>
<textarea
+ id={`description-${activeLocale}`}
rows={3}
...
/>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/components/admin/quiz/QuizMetadataEditor.tsx` around lines 177 -
196, The Title/Description labels aren’t linked to their controls so clicking
them won’t focus the inputs and screen readers can’t associate them; update the
JSX in QuizMetadataEditor to add unique id attributes to the input/textarea
(e.g. `id={`title-${activeLocale}`}` and `id={`description-${activeLocale}`}`)
and set the corresponding label htmlFor to those ids, doing the same for the
Time Limit label/control; keep using the existing symbols
(draft.translations[activeLocale].title,
draft.translations[activeLocale].description, updateField, LOCALE_LABELS,
activeLocale) so the handlers and values remain unchanged.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
frontend/components/admin/quiz/JsonUploadArea.tsx (1)
33-57: Consider guarding against duplicate file uploads.If the user opens the picker twice and selects the same file, both copies are appended and their questions are merged into the parent payload, resulting in duplicate questions in the quiz. A simple guard on
file.namewould prevent silent data duplication:♻️ Proposed guard
for (const file of Array.from(fileList)) { + if (files.some(f => f.name === file.name)) { + newFiles.push({ name: file.name, questions: [], error: 'Already uploaded' }); + continue; + } if (!file.name.endsWith('.json')) {Note: if
setFilesis migrated to the functional update form (per the stale-closure fix above), compare againstprevinside the updater instead of closing overfiles.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/admin/quiz/JsonUploadArea.tsx` around lines 33 - 57, Guard against duplicate uploads by skipping files whose name already exists in the current state or in the current batch: inside the loop over fileList, check if file.name is present in files.map(f=>f.name) or in newFiles.map(f=>f.name) and if so continue (push nothing) instead of adding a duplicate entry; update the state using the functional updater form setFiles(prev => { const existingNames = new Set(prev.map(f => f.name)); for each file, skip if existingNames.has(file.name) or newFiles already contains it, otherwise parse and push and add to existingNames; finally call updateParent with the new merged array returned from the updater.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/components/admin/quiz/JsonUploadArea.tsx`:
- Around line 70-72: The <label> in JsonUploadArea.tsx (component
JsonUploadArea) is orphaned and should be made semantic: either replace the
<label className="text-foreground text-sm font-medium">Questions (JSON
files)</label> with a non-form element like <span> or <p> since it's purely
presentational, or if you prefer keeping a label for accessibility, give the
hidden <input> an id and add a matching htmlFor on the <label>; update the
element reference in the component accordingly so there are no unbound <label>
elements.
- Around line 55-57: handleFiles captures a stale files closure and can
reintroduce removed items; change setFiles([...files, ...newFiles]) in
handleFiles to the functional updater setFiles(prev => [...prev, ...newFiles])
and remove the direct updateParent call from handleFiles; instead add a
useEffect(() => { updateParent(files); }, [files]) to synchronize the parent
whenever files state commits; this ensures removeFile (and other setters) race
safely and the parent is only notified from the latest committed files state.
---
Duplicate comments:
In `@frontend/app/api/admin/quiz/`[id]/questions/route.ts:
- Around line 111-180: The sequential multi-step insert/update logic (inserting
into quizQuestions, quizQuestionContent, quizAnswers, quizAnswerTranslations,
then updating quizzes and calling invalidateQuizCache) must be wrapped in a
single db.transaction so all steps atomically commit or rollback: move the
entire block that creates insertedQuestions, inserts content, inserts answers,
inserts answer translations, and updates quizzes.questionsCount into
db.transaction(async tx => { ... }) and use the transaction handle (tx) for all
DB calls (e.g., replace db.insert(...)/db.update(...) with tx.insert/... and
tx.select/tx.update), keep the try/catch inside the transaction to allow
throwing to trigger rollback, and call invalidateQuizCache(quizId) only after
the transaction resolves successfully (outside the transaction) so cache
invalidation never runs on a partial commit; reference symbols: quizQuestions,
quizQuestionContent, quizAnswers, quizAnswerTranslations, quizzes,
insertedQuestions, insertedAnswers, and invalidateQuizCache.
- Around line 152-153: The current index math uses a hardcoded 4 (qIdx =
Math.floor(i / 4); aIdx = i % 4) which breaks if answer counts change; instead
compute the per-question answer count dynamically (e.g., derive
answersPerQuestion from the questions array such as questions[0].answers.length
or per-question answers length) and then calculate qIdx = Math.floor(i /
answersPerQuestion) and aIdx = i % answersPerQuestion (or use per-question
lengths when mapping to a specific question). Update the logic that computes
qIdx and aIdx in route.ts to use that dynamic answersPerQuestion value (refer to
the qIdx and aIdx calculations and the questions/answers data structure) so
translation assignments stay correct if schema changes.
---
Nitpick comments:
In `@frontend/components/admin/quiz/JsonUploadArea.tsx`:
- Around line 33-57: Guard against duplicate uploads by skipping files whose
name already exists in the current state or in the current batch: inside the
loop over fileList, check if file.name is present in files.map(f=>f.name) or in
newFiles.map(f=>f.name) and if so continue (push nothing) instead of adding a
duplicate entry; update the state using the functional updater form
setFiles(prev => { const existingNames = new Set(prev.map(f => f.name)); for
each file, skip if existingNames.has(file.name) or newFiles already contains it,
otherwise parse and push and add to existingNames; finally call updateParent
with the new merged array returned from the updater.
Comment on lines
+55
to
+57
| const updated = [...files, ...newFiles]; | ||
| setFiles(updated); | ||
| updateParent(updated); |
Contributor
There was a problem hiding this comment.
Stale files closure in async handleFiles can restore removed files.
handleFiles is async and awaits inside a loop (Line 40). By the time execution reaches Line 55, the files value captured in the closure at the last render may be stale — for example, if the user clicks Remove on an already-uploaded file while the new batch is being read, removeFile triggers setFiles([]), a re-render occurs, but handleFiles then resumes and executes [...files, ...newFiles] using the pre-removal snapshot, effectively restoring the deleted entry.
When a callback that returns the new state based on the previous one is supplied to the state update function, React makes sure that the latest state value is supplied as an argument to that callback. The cleanest fix is to switch setFiles to the functional update form and use a useEffect to derive the parent notification from committed state:
🐛 Proposed fix — functional update + `useEffect` for parent sync
-import { useRef, useState } from 'react';
+import { useEffect, useRef, useState } from 'react';
export function JsonUploadArea({ onQuestionsChange }: JsonUploadAreaProps) {
const [files, setFiles] = useState<UploadedFile[]>([]);
const inputRef = useRef<HTMLInputElement>(null);
- function updateParent(updatedFiles: UploadedFile[]) {
- const merged = updatedFiles.flatMap(f => f.questions);
- onQuestionsChange(merged);
- }
+ // Sync aggregated questions to parent whenever committed state changes.
+ // Parent should stabilise onQuestionsChange with useCallback to avoid
+ // unnecessary re-runs.
+ useEffect(() => {
+ onQuestionsChange(files.flatMap(f => f.questions));
+ }, [files, onQuestionsChange]);
async function handleFiles(fileList: FileList) {
const newFiles: UploadedFile[] = [];
for (const file of Array.from(fileList)) {
// ... same validation logic ...
}
- const updated = [...files, ...newFiles];
- setFiles(updated);
- updateParent(updated);
+ // Functional update avoids reading stale `files` from the closure
+ // captured before the awaits above.
+ setFiles(prev => [...prev, ...newFiles]);
if (inputRef.current) inputRef.current.value = '';
}
function removeFile(index: number) {
- const updated = files.filter((_, i) => i !== index);
- setFiles(updated);
- updateParent(updated);
+ setFiles(prev => prev.filter((_, i) => i !== index));
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/components/admin/quiz/JsonUploadArea.tsx` around lines 55 - 57,
handleFiles captures a stale files closure and can reintroduce removed items;
change setFiles([...files, ...newFiles]) in handleFiles to the functional
updater setFiles(prev => [...prev, ...newFiles]) and remove the direct
updateParent call from handleFiles; instead add a useEffect(() => {
updateParent(files); }, [files]) to synchronize the parent whenever files state
commits; this ensures removeFile (and other setters) race safely and the parent
is only notified from the latest committed files state.
Comment on lines
+70
to
+72
| <label className="text-foreground text-sm font-medium"> | ||
| Questions (JSON files) | ||
| </label> |
Contributor
There was a problem hiding this comment.
<label> has no associated form control — use a <p> or <span> instead.
The <label> element at Line 70 has no htmlFor and doesn't wrap any input, so it is semantically unbound. The hidden <input> also lacks an id. An orphaned <label> is invalid HTML and some browsers/AT may attempt to infer an association with a nearby control, producing unpredictable behaviour. Since the element is purely presentational here, replace it with <p> or <span>:
🛡️ Proposed fix
- <label className="text-foreground text-sm font-medium">
- Questions (JSON files)
- </label>
+ <p className="text-foreground text-sm font-medium">
+ Questions (JSON files)
+ </p>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <label className="text-foreground text-sm font-medium"> | |
| Questions (JSON files) | |
| </label> | |
| <p className="text-foreground text-sm font-medium"> | |
| Questions (JSON files) | |
| </p> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/components/admin/quiz/JsonUploadArea.tsx` around lines 70 - 72, The
<label> in JsonUploadArea.tsx (component JsonUploadArea) is orphaned and should
be made semantic: either replace the <label className="text-foreground text-sm
font-medium">Questions (JSON files)</label> with a non-form element like <span>
or <p> since it's purely presentational, or if you prefer keeping a label for
accessibility, give the hidden <input> an id and add a matching htmlFor on the
<label>; update the element reference in the component accordingly so there are
no unbound <label> elements.
ViktorSvertoka
approved these changes
Feb 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Test plan
/admin/quiz/newnpm run buildpassesSummary by CodeRabbit
Release Notes
New Features
Chores