fix: improve app-wide code health - #1
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR centralizes quiz creation state, introduces database/application question transformations, updates quiz-taking and review components, extracts ChangesQuiz workflow refactor
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CreateQuizCSV
participant useCreateQuiz
participant csvParser
participant Supabase
participant Clipboard
CreateQuizCSV->>useCreateQuiz: submit quiz metadata
useCreateQuiz->>Supabase: insert quiz row
CreateQuizCSV->>useCreateQuiz: select CSV file
useCreateQuiz->>csvParser: parseAndValidateCSV
csvParser-->>useCreateQuiz: validated question rows
useCreateQuiz->>Supabase: insert mapped questions
useCreateQuiz->>Clipboard: copy shareable quiz URL
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Comment |
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/useTakeQuiz.ts (1)
31-77: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPrevent stale quiz requests from overwriting the active quiz
src/hooks/useTakeQuiz.ts:31-77A slower request for a previous
quizIdcan still finish after navigation and callsetQuiz,setQuestions,setError, ortoast.error, leaving the new session with the wrong quiz state. Add cleanup/versioning (or abort the request) and skip updates for stale responses.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/useTakeQuiz.ts` around lines 31 - 77, Update fetchQuizData and its useEffect cleanup so requests for an outdated quizId cannot update quiz state, loading/error state, or show toast errors after navigation. Track request activity with an abort signal or request version, invalidate it during effect cleanup, and guard both success and catch updates while preserving updates for the active request.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/prt.md`:
- Around line 1489-1495: Update the documented successful upload flow to remove
the "quiz_builder_draft" localStorage entry immediately after the quiz insert
succeeds, ensuring subsequent refreshes cannot restore stale builder questions.
- Line 109: Update the lint verification instructions in docs/prt.md to use npm
run lint instead of npx next lint at all referenced occurrences, while
preserving the surrounding verification steps.
- Around line 588-626: Update the test configuration in vitest.config.ts to set
test.passWithNoTests to true, ensuring the existing npm test and coverage
scripts exit successfully when no test files are present.
- Around line 1513-1527: Update createQuiz() so its Supabase insert payload maps
the local timeLimit state to the database time_limit field, preserving null or
undefined handling as appropriate for optional values.
In `@package.json`:
- Line 10: Update the package.json lint script to run ESLint against the app/
routes in addition to src/, preferably by targeting the repository root with
eslint ..
In `@src/components/CreateQuizForm.tsx`:
- Around line 50-53: Update CreateQuizForm’s form submission flow so pressing
Enter in the title field creates the quiz: attach the existing onSubmit handler
to the form’s onSubmit prop and make the submit button type="submit" instead of
type="button", while preserving its disabled condition.
In `@src/hooks/useCreateQuiz.ts`:
- Around line 101-125: Update uploadQuestions in useCreateQuiz so a successfully
published quiz cannot be submitted again: guard against the existing
published/shareable state before inserting questions, and ensure the related
upload control uses that state to remain disabled after success. Preserve an
explicit replace flow only if one already exists.
- Around line 73-77: Remove the exact file.type === "text/csv" rejection from
setQuestionsFromCSV so files with empty or alternate MIME types reach
parseAndValidateCSV. Preserve the parser’s validation flow and avoid adding a
replacement MIME check.
- Around line 122-125: Update the quiz publishing flow around the clipboard
write in useCreateQuiz so navigator.clipboard.writeText is awaited before
showing the success toast. Preserve the shareableLink state update, and add
separate rejection handling so denied clipboard permissions do not produce an
unhandled promise or success notification.
In `@src/lib/types.ts`:
- Around line 53-64: Update the AppQuestion.correctIndex property to use the
literal union type 0 | 1 | 2 | 3, ensuring appToDBQuestion() can only pass valid
option indexes to indexToLetter().
---
Outside diff comments:
In `@src/hooks/useTakeQuiz.ts`:
- Around line 31-77: Update fetchQuizData and its useEffect cleanup so requests
for an outdated quizId cannot update quiz state, loading/error state, or show
toast errors after navigation. Track request activity with an abort signal or
request version, invalidate it during effect cleanup, and guard both success and
catch updates while preserving updates for the active request.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b7dc4787-f2aa-4634-8b7a-e9043b02c219
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (24)
.gitignoredocs/prt.mdeslint.config.mjspackage.jsonsrc/components/CreateQuizForm.tsxsrc/components/DashboardLayout.tsxsrc/components/RequireAuth.tsxsrc/components/UploadQuestionsForm.tsxsrc/components/auth/AuthPageClient.tsxsrc/components/quiz/QuestionCard.tsxsrc/components/quiz/QuizResults.tsxsrc/components/quiz/QuizReview.tsxsrc/context/AuthContext.tsxsrc/hooks/useAuth.tssrc/hooks/useCreateQuiz.tssrc/hooks/useQuizProgress.tssrc/hooks/useTakeQuiz.tssrc/lib/types.tssrc/utils/csvParser.tssrc/utils/handlers.tssrc/utils/helpers.tssrc/utils/transforms.tssrc/views/Auth/AuthForm.tsxsrc/views/CreateQuiz.tsx
💤 Files with no reviewable changes (1)
- src/utils/handlers.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/views/CreateQuiz.tsx (1)
42-48: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winDisable form fields while creation is in flight.
isLoadingdisables only the submit button; the inputs remain editable becausedisabledexcludesisCreatingQuiz. SincecreateQuizcaptures state before awaiting Supabase, edits made during the request are not persisted but remain visible afterward.Proposed fix
- disabled={!!quiz.id} + disabled={!!quiz.id || isCreatingQuiz}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/views/CreateQuiz.tsx` around lines 42 - 48, Update the form props in CreateQuiz so the disabled state includes isCreatingQuiz as well as the existing quiz.id condition, preventing title and description edits while createQuiz is in flight.src/hooks/useCreateQuiz.ts (1)
43-62: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winClear loading flags in
finallyblocks.A rejected Supabase call or thrown transformation bypasses the current cleanup, leaving
isCreatingQuizorisUploadingQuestionspermanently enabled. Wrap both operations withtry/finallywhile preserving the existing error handling.Also applies to: 107-119
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/useCreateQuiz.ts` around lines 43 - 62, Update the quiz-creation flow around the Supabase calls in the hook’s creation method and the corresponding question-upload operation so each sets its loading flag back to false in a finally block. Preserve the existing authentication and operation error handling, while ensuring rejected calls or thrown transformations cannot leave isCreatingQuiz or isUploadingQuestions enabled.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/hooks/useCreateQuiz.ts`:
- Around line 43-62: Update the quiz-creation flow around the Supabase calls in
the hook’s creation method and the corresponding question-upload operation so
each sets its loading flag back to false in a finally block. Preserve the
existing authentication and operation error handling, while ensuring rejected
calls or thrown transformations cannot leave isCreatingQuiz or
isUploadingQuestions enabled.
In `@src/views/CreateQuiz.tsx`:
- Around line 42-48: Update the form props in CreateQuiz so the disabled state
includes isCreatingQuiz as well as the existing quiz.id condition, preventing
title and description edits while createQuiz is in flight.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9937d443-334e-4d42-833d-c683ebc5a728
📒 Files selected for processing (5)
src/components/CreateQuizForm.tsxsrc/hooks/useCreateQuiz.tssrc/lib/types.tssrc/utils/transforms.tssrc/views/CreateQuiz.tsx
Summary
Six code health fixes found during grounded gap analysis: CSV parser correctly handles lowercase and whitespace-padded correct answers;
useAuthsplit into its own file;saveProgressandclearProgresswrapped inuseCallback;fetchQuizDatawrapped inuseCallback;DBQuestion/AppQuestiontype layer introduced withtransforms.ts; andhandlers.tsreplaced byuseCreateQuizhook.Changed files
src/utils/csvParser.tssrc/context/AuthContext.tsxsrc/hooks/useAuth.ts(new)src/hooks/useQuizProgress.tssrc/hooks/useTakeQuiz.tssrc/lib/types.tssrc/utils/transforms.ts(new)src/utils/helpers.tssrc/hooks/useCreateQuiz.ts(new)src/utils/handlers.ts(deleted)src/views/CreateQuiz.tsxsrc/components/CreateQuizForm.tsxsrc/components/UploadQuestionsForm.tsxsrc/components/quiz/QuizResults.tsxChecklist
npx tsc --noEmitpasses with zero errorsnpx next lintpasses with zero errors and warningshandlers.tsis deletedconsole.logstatements left in codemainSummary by CodeRabbit