-
-
Notifications
You must be signed in to change notification settings - Fork 19
Add Quiz Taking UI #3103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Quiz Taking UI #3103
Conversation
Warning Rate limit exceeded@vhpx has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 35 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughThis update introduces a comprehensive overhaul of the quiz-taking and results infrastructure. It adds new React components for quiz-taking, result display, and attempt summaries, restructures localization files for quizzes, updates API endpoints for richer quiz metadata and attempt management, refines database types and migrations, and removes obsolete components and logic. Changes
Sequence Diagram(s)Quiz Attempt Submission and Result RetrievalsequenceDiagram
participant User
participant TakingQuizClient
participant API /take
participant API /submit
participant API /attempts/[attemptId]
participant Database
User->>TakingQuizClient: Start quiz
TakingQuizClient->>API /take: GET quiz metadata
API /take->>Database: Fetch quiz set, questions, attempts
Database-->>API /take: Quiz data
API /take-->>TakingQuizClient: Quiz data
User->>TakingQuizClient: Answer questions, submit
TakingQuizClient->>API /submit: POST answers, duration
API /submit->>Database: Insert attempt, answers
Database-->>API /submit: Inserted attempt, scores
API /submit-->>TakingQuizClient: Attempt result metadata
TakingQuizClient->>API /attempts/[attemptId]: GET attempt details
API /attempts/[attemptId]->>Database: Fetch attempt, questions, options, answers
Database-->>API /attempts/[attemptId]: Attempt detail
API /attempts/[attemptId]-->>TakingQuizClient: Attempt detail
TakingQuizClient-->>User: Show result summary and details
Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate Unit Tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @Puppychan, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request delivers the core user-facing functionality for taking quizzes and reviewing past performance. It establishes the necessary database structure, provides API endpoints to serve dynamic quiz and attempt data, and builds the frontend components for the entire quiz-taking journey, from pre-quiz information to post-submission results.
Highlights
- New Quiz Taking UI: Introduced a comprehensive user interface for taking quizzes, including a landing page with instructions and attempt history, the quiz form itself with question navigation, and dedicated pages for viewing attempt results and summaries.
- Flexible Attempt Review: Implemented logic and UI components to display different levels of detail for past attempts based on quiz settings: a simple summary view showing questions and user answers (if allowed), or a full results view including correctness, scores, and explanations (if results are released).
- Database Schema Updates: Added new columns to
workspace_quiz_attempts
(duration, submitted time) andworkspace_quiz_sets
(availability date, explanation mode, instruction, result release status, view old attempts flag) to support the new quiz features and tracking. - API Endpoints: Created a new API endpoint (
/attempts/[attemptId]
) to fetch specific attempt details based on user permissions and quiz settings, and updated existing/take
and/results
endpoints to provide necessary data for the new UI flow. - Multiple Choice Support: Enhanced the quiz taking UI and backend data structure to correctly handle and display both single and multiple-choice questions.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configureGemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a comprehensive UI for taking quizzes, including new database schema elements, API routes for fetching quiz data and results, and several new frontend components for displaying quiz information, questions, and results. The changes cover the pre-quiz information display, the quiz-taking interface with support for single and multiple-choice questions, and various result views (summary, detailed, and pre-release summary).
Overall, the implementation is quite thorough. Key areas for attention include ensuring type definitions match the schema, addressing potential performance issues in one of the older API routes, and clarifying data flow for some summary components. The new UI components are well-structured and make good use of conditional rendering and state management for a dynamic user experience.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 23
🔭 Outside diff range comments (2)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quizzes/client-ai.tsx (1)
50-61
:fetch
responses insidePromise.all
are not validated
fetch
resolves even when the response status is 4xx/5xx, therefore failed insertions won’t be caught and the wholeacceptQuizzes
flow will silently succeed.
Throw on non-OK responses before thePromise.all
settles.-const promises = object.quizzes.map((quiz) => - fetch(`/api/v1/workspaces/${wsId}/quizzes`, { - method: 'POST', - body: JSON.stringify({ - setId: quizSet.setId, - moduleId, - question: quiz?.question, - quiz_options: quiz?.quiz_options, - }), - }) -); +const promises = object.quizzes.map((quiz) => + fetch(`/api/v1/workspaces/${wsId}/quizzes`, { + method: 'POST', + body: JSON.stringify({ + setId: quizSet.setId, + moduleId, + question: quiz?.question, + quiz_options: quiz?.quiz_options, + }), + }).then((res) => { + if (!res.ok) throw new Error('Failed to create quiz'); + return res; + }) +);apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/take/time-elapsed-status.tsx (1)
49-57
: Add an accessible label for the visibility-toggle button.Screen-reader users cannot infer intent from the icon alone. Provide
aria-label
that changes withisVisible
.-<button - onClick={toggleVisibility} - className="ml-4 text-sm text-muted-foreground underline hover:text-foreground" -> +<button + onClick={toggleVisibility} + aria-label={isVisible ? t('hide_time') : t('show_time')} + className="ml-4 text-sm text-muted-foreground underline hover:text-foreground" +>
♻️ Duplicate comments (1)
apps/upskii/src/app/api/v1/workspaces/[wsId]/quiz-sets/[setId]/submit/route.ts (1)
11-15
: IncorrectPromise
typing forparams
(same as take route).See previous comment – declare
params
as a plain object and drop theawait
.
🧹 Nitpick comments (20)
apps/db/supabase/migrations/20250615104201_new_migration.sql (1)
1-1
: Consider default & NOT-NULL to enforce data integrityIf every quiz should ultimately carry an
instruction
, declare the columnNOT NULL
with a sane default ('{}'::jsonb
) now; otherwise you’ll need an additional migration later to back-fill values and add the constraint.-alter table "public"."workspace_quizzes" add column "instruction" jsonb; +alter table "public"."workspace_quizzes" + add column "instruction" jsonb not null default '{}'::jsonb;apps/db/supabase/migrations/20250615190931_new_migration.sql (1)
1-1
: Potential long-running lock – split NOT NULL & defaultAdding a
NOT NULL
column with a default rewrites the whole table in Postgres ≤13.
On large data sets this blocks writes. A safer pattern:alter table "public"."workspace_quiz_sets" add column "results_released" boolean default false; -- NULL-able for now update "public"."workspace_quiz_sets" set results_released = false where results_released is null; alter table "public"."workspace_quiz_sets" alter column results_released set not null;apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quizzes/client-ai.tsx (1)
15-23
:courseId
prop is forwarded but never used in this component
AIQuizzes
now requirescourseId
, yet internally it is only tunnelled toClientQuizzes
.
If the API that creates quiz sets or quizzes is meant to be course-scoped, remember to includecourseId
in the request body / path, otherwise the new prop adds no value and increases coupling.Also applies to: 108-112
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quizzes/client-quizzes.tsx (1)
107-117
: Button label bypasses localisation systemThe rest of the component is fully internationalised; hard-coding
"Take Quiz"
will surface in English only.-<Button …> - Take Quiz -</Button> +<Button …> + {t('ws-quizzes.take_quiz')} +</Button>apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/result/quiz-result-client.tsx (3)
161-165
: Scope translations to the quiz domain to avoid verbose keys.Using
useTranslations('ws-quizzes')
lets you drop the repeated prefix (t('no_attempt_specified')
) everywhere. Keeps code shorter and less error-prone.
207-212
: Hard-coded “Go back” string breaks localisation.Add the string to the message bundle and pull it through
t
.-<Button className="mt-4" onClick={() => router.back()}> - Go back -</Button> +<Button className="mt-4" onClick={() => router.back()}> + {t('ws-quizzes.go_back') /* fallback: 'Go back' */} +</Button>
170-204
: Abort fetch on unmount to avoid setState-after-unmount warnings.Wrap the request in an
AbortController
and checkcontroller.signal.aborted
before callingsetState
.async function load() { - setLoading(true); + const controller = new AbortController(); + setLoading(true); setError(null); try { const res = await fetch(url, { cache: 'no-store', signal: controller.signal }); @@ } finally { - setLoading(false); + if (!controller.signal.aborted) setLoading(false); } } - load(); + load(); + return () => controller.abort();apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/take/taking-quiz-client.tsx (3)
115-120
: Remove commented debug code.Debug code should not be included in production code, even as comments.
- // Only for debugging purposes - // useEffect(() => { - // localStorage.removeItem(ANSWERS_KEY); - // localStorage.removeItem(STORAGE_KEY); - // clearStartTimestamp(); - // }, [setId]);
186-217
: Consider extracting complex timer logic to a custom hook.The timer management logic is complex and could be reused. Extracting it to a custom hook would improve maintainability and testability.
Create a custom hook like
useQuizTimer
:function useQuizTimer(hasStarted: boolean, totalSeconds: number | null, onTimeUp: () => void) { const [timeLeft, setTimeLeft] = useState<number | null>(null); const timerRef = useRef<NodeJS.Timeout | null>(null); // Timer logic here return { timeLeft, setTimeLeft }; }
49-443
: Consider decomposing this large component.At 443 lines, this component handles many responsibilities: timer management, state persistence, API calls, and complex UI rendering. This makes it difficult to test and maintain.
Consider splitting into smaller, focused components:
QuizTimer
- Handle timer logicQuizForm
- Handle the form and question renderingQuizSubmission
- Handle submission logicuseQuizState
- Custom hook for state managementuseQuizPersistence
- Custom hook for localStorage operationsapps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/result/display-summary-attempts/attempt-summary-view.tsx (1)
202-206
: Consider using CSS classes instead of inline conditional styles.Multiple inline style conditionals make the code harder to maintain.
Consider using utility classes or CSS modules:
- className={`transition-all hover:shadow-md ${ - isAnswered - ? 'border-dynamic-light-green/30 bg-dynamic-green/10' - : 'border-dynamic-light-orange bg-dynamic-light-orange/20' - }`} + className={cn( + 'transition-all hover:shadow-md', + isAnswered + ? 'answered-card' + : 'skipped-card' + )}Also applies to: 284-287, 298-298
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/result/display-results/show-attempt-detail-section.tsx (2)
41-49
: Hard-coded locale in date/duration helpers
new Date(...).toLocaleString()
defaults to the runtime locale; fallback strings above are English.
Instead, pass the currentlocale
(already available via route param oruseLocale()
) so Vietnamese users see native formatting.
94-111
: Icon precedence may hide “your answer” feedbackWhen the user picked the correct option, the branch
opt.isCorrect
fires first, so the “chosen” variant is never rendered. Users lose the visual cue that they selected it.
Swap the conditions or addchosen && opt.isCorrect
as first check to show a distinct icon/state.apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/take/before-taking-quiz-whole.tsx (3)
62-70
:formatDate
forcesen-US
regardless of page localeHard-coding the locale breaks i18n. Use the
locale
param (useLocale()
from next-intl) or leave it undefined to let the browser format naturally.
120-126
: Artificial 500 ms delay beforeonStart
setTimeout
delays UX and complicates tests with no benefit shown in code; callonStart()
directly or show actual async progress.
297-307
:attemptsRemaining
can become negativeWhen
attemptsSoFar
exceedsattemptLimit
, the UI shows e.g. “-2 attempts remaining”. Clamp at zero:const attemptsRemaining = quizData.attemptLimit ? Math.max(0, quizData.attemptLimit - quizData.attemptsSoFar) : null;apps/upskii/messages/en.json (2)
3874-3879
: Inconsistent punctuation in error messagesSome entries in the
errors
block end with a period (e.g.,submission-failed
), while others do not. For a uniform user experience, standardize punctuation across all error strings.
3880-3898
: Duplicate message keys in quiz blockWithin
quiz
, bothpast-due
/quiz-past-due
andnot-available
/quiz-not-available
convey similar information. Verify if both are needed; otherwise, consolidate to avoid redundancy.apps/upskii/src/app/api/v1/workspaces/[wsId]/quiz-sets/[setId]/take/route.ts (1)
92-97
: Use a head-only count query for efficiency.You only need the number of attempts, not the rows. Passing
head: false
returns every row and may hurt performance on large data sets.- .select('attempt_number', { count: 'exact', head: false }) + .select('attempt_number', { count: 'exact', head: true })apps/upskii/src/app/api/v1/workspaces/[wsId]/quiz-sets/[setId]/submit/route.ts (1)
43-55
: Same counting-rows inefficiency as take route.
Switch tohead: true
to avoid fetching all attempt rows.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
apps/db/supabase/migrations/20250615100747_new_migration.sql
(1 hunks)apps/db/supabase/migrations/20250615104201_new_migration.sql
(1 hunks)apps/db/supabase/migrations/20250615190931_new_migration.sql
(1 hunks)apps/db/supabase/migrations/20250616085119_new_migration.sql
(1 hunks)apps/upskii/messages/en.json
(1 hunks)apps/upskii/messages/vi.json
(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/page.tsx
(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/result/display-results/show-attempt-detail-section.tsx
(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/result/display-results/show-result-summary-section.tsx
(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/result/display-summary-attempts/attempt-summary-view.tsx
(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/result/page.tsx
(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/result/quiz-result-client.tsx
(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/take/before-taking-quiz-whole.tsx
(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/take/page.tsx
(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/take/quiz-status-sidebar.tsx
(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/take/taking-quiz-client.tsx
(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/take/time-elapsed-status.tsx
(3 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quizzes/client-ai.tsx
(2 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quizzes/client-quizzes.tsx
(3 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quizzes/page.tsx
(3 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/result/page.tsx
(0 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/quiz-status-sidebar.tsx
(0 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/sections/before-take-quiz-section.tsx
(0 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/sections/past-due-section.tsx
(0 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/sections/show-result-summary-section.tsx
(0 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/taking-quiz-client.tsx
(0 hunks)apps/upskii/src/app/api/v1/workspaces/[wsId]/quiz-sets/[setId]/attempts/[attemptId]/route.ts
(1 hunks)apps/upskii/src/app/api/v1/workspaces/[wsId]/quiz-sets/[setId]/results/route.ts
(1 hunks)apps/upskii/src/app/api/v1/workspaces/[wsId]/quiz-sets/[setId]/submit/route.ts
(2 hunks)apps/upskii/src/app/api/v1/workspaces/[wsId]/quiz-sets/[setId]/take/route.ts
(1 hunks)packages/types/src/supabase.ts
(3 hunks)
💤 Files with no reviewable changes (6)
- apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/result/page.tsx
- apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/quiz-status-sidebar.tsx
- apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/sections/before-take-quiz-section.tsx
- apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/taking-quiz-client.tsx
- apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/sections/show-result-summary-section.tsx
- apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/sections/past-due-section.tsx
🧰 Additional context used
🧬 Code Graph Analysis (2)
apps/upskii/src/app/api/v1/workspaces/[wsId]/quiz-sets/[setId]/results/route.ts (2)
apps/upskii/src/app/api/v1/workspaces/[wsId]/quiz-sets/[setId]/attempts/[attemptId]/route.ts (1)
GET
(12-219)apps/upskii/src/app/api/v1/workspaces/[wsId]/quiz-sets/[setId]/take/route.ts (1)
GET
(27-220)
apps/upskii/src/app/api/v1/workspaces/[wsId]/quiz-sets/[setId]/attempts/[attemptId]/route.ts (2)
apps/upskii/src/app/api/v1/workspaces/[wsId]/quiz-sets/[setId]/results/route.ts (1)
GET
(11-121)apps/upskii/src/app/api/v1/workspaces/[wsId]/quiz-sets/[setId]/take/route.ts (1)
GET
(27-220)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Verify generated types
- GitHub Check: Deploy-Preview
🔇 Additional comments (24)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/page.tsx (1)
139-144
: VerifyClientQuizzes
signature now includescourseId
courseId
is newly passed (l 141). Confirm that:
client-quizzes.tsx
declarescourseId
in its props interface.- All other call-sites have been updated, otherwise TS will error.
No issues in this file, just cross-component consistency to check.
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/take/page.tsx (1)
1-1
: Import path update looks correctThe new deeply-nested import reflects the added
courseId
/moduleId
params and compiles under Next.js’ aliasing rules.
No further action needed.apps/upskii/messages/vi.json (5)
3853-3864
: Quiz status messages look correct.
Translations underquiz-status
align with English semantics and follow file conventions.
3877-3882
: Error messages are properly translated.
Theerrors
block entries are accurate and consistent.
3883-3906
: Quiz UI labels are correctly localized.
Thequiz
section has appropriate Vietnamese translations and matches UI context.
3907-3925
: Quiz info section translations are accurate.
Theinfo
block entries correctly reflect quiz metadata in Vietnamese.
3926-3936
: Instructions section translations are correct.
Theinstructions
block aligns with the intended guidance messages.apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/take/time-elapsed-status.tsx (1)
23-24
: Namespace already provided – remove redundant key prefix in calls.Because
useTranslations('ws-quizzes.time')
is already scoped, calls should bet('remaining')
nott('ws-quizzes.time.remaining')
. Current code is correct; double-check other files for accidental double prefixes to keep i18n consistent.packages/types/src/supabase.ts (2)
5974-5977
: Addition ofinstruction
looks good
workspace_quizzes
now exposes aninstruction: Json | null
field consistently acrossRow
,Insert
, andUpdate
. No further action required.
5918-5929
: Confirm downstream usage after renamingrelease_points_immediately
→results_released
The renamed flag is reflected here, but any frontend / API code that still references
release_points_immediately
will now fail to compile.
Run a quick grep to make sure every usage is migrated.rg --context 2 'release_points_immediately'
If nothing shows up you’re safe; otherwise perform the replacement or add a compatibility alias in your service layer.
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/result/display-results/show-result-summary-section.tsx (1)
1-216
: Well-structured result summary component!The component demonstrates good practices:
- Clear prop types with proper TypeScript interfaces
- Excellent use of helper functions for score visualization
- Accessible UI with ARIA labels and semantic HTML
- Comprehensive feedback based on performance
- Consistent use of the design system
apps/upskii/src/app/api/v1/workspaces/[wsId]/quiz-sets/[setId]/results/route.ts (1)
5-9
: Good improvements to type safety and naming conventions.The changes improve code quality:
- Using a dedicated
Params
interface with Promise for Next.js 15 compatibility- Shorter, consistent variable names
- Clear error messages
Also applies to: 11-22
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/result/display-summary-attempts/attempt-summary-view.tsx (1)
1-371
: Excellent implementation of the attempt summary view!The component demonstrates high-quality code:
- Well-defined TypeScript interfaces
- Clear helper functions for date/duration formatting
- Comprehensive UI with progress indicators
- Excellent accessibility with proper ARIA labels
- Responsive grid layouts
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/take/quiz-status-sidebar.tsx (1)
1-108
: Clean and accessible quiz status sidebar!The component is well-implemented with:
- Clear question tracking logic
- Smooth scroll navigation with focus management
- Comprehensive ARIA labels for screen readers
- Responsive grid layout
- Good use of the translation system
apps/upskii/src/app/api/v1/workspaces/[wsId]/quiz-sets/[setId]/attempts/[attemptId]/route.ts (1)
14-20
:createClient()
likely sync – double-checkawait
Most Supabase helpers (including the sibling browser/client variants in this repo) return a ready client synchronously. If that holds here,
await createClient()
adds an unnecessary micro-task and misleads future readers.apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/take/before-taking-quiz-whole.tsx (1)
94-105
:canRetake
ignoresresultsReleased
business ruleA comment hints the rule changed but the condition is still loose. Consolidate business logic in one place to avoid regressions:
const canRetake = isAvailable && !isPastDue && !quizData.resultsReleased && (attemptsRemaining == null || attemptsRemaining > 0);apps/upskii/messages/en.json (6)
3852-3863
: Well-structured quiz-status namespaceThe new
quiz-status
block groups all progress-related localization strings underws-quizzes
, improving organization and readability. The keys follow the existing kebab-case convention.
3899-3922
: Quiz info section addition is clearThe new
info
block cleanly encapsulates quiz metadata labels and placeholders. The namespace and key naming align well with other sections.
3923-3933
: Quiz instructions localized correctlyThe
instructions
block effectively covers pre-quiz guidelines. Placeholder usage ({minutes}
) is consistent and descriptive.
3934-3942
: Past attempts messages organizedThe
past-attempts
namespace neatly groups UI text for reviewing previous quiz attempts. The labels are intuitive.
3943-3966
: Summary section is well-definedThe
summary
object logically groups attempt-summary labels. This structure improves maintainability and translation context.
3967-3995
: Results feedback is comprehensiveThe
results
block provides a full suite of post-quiz feedback messages, including motivational prompts. It’s well-organized under thews-quizzes
namespace.apps/upskii/src/app/api/v1/workspaces/[wsId]/quiz-sets/[setId]/take/route.ts (1)
131-143
: Guard against nullable timestamps when computing duration.
started_at
orcompleted_at
could benull
(e.g., legacy rows).new Date(null)
→Invalid Date
, causingNaN
durations.-const started = new Date(row.started_at).getTime(); +if (!row.started_at) return null; // skip malformed row +const started = new Date(row.started_at).getTime();Consider filtering or defaulting to
0
to avoid runtime errors.apps/upskii/src/app/api/v1/workspaces/[wsId]/quiz-sets/[setId]/submit/route.ts (1)
196-206
: Error from the finalupdate
is ignored.If the duration patch fails, the API still returns 200, leaving inconsistent data. Handle the
error
field or wrap insert/answer/update in a transaction.
...i/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quizzes/page.tsx
Show resolved
Hide resolved
...]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/result/page.tsx
Show resolved
Hide resolved
...[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quizzes/client-quizzes.tsx
Show resolved
Hide resolved
...d]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/take/before-taking-quiz-whole.tsx
Show resolved
Hide resolved
apps/upskii/src/app/api/v1/workspaces/[wsId]/quiz-sets/[setId]/take/route.ts
Show resolved
Hide resolved
apps/upskii/src/app/api/v1/workspaces/[wsId]/quiz-sets/[setId]/submit/route.ts
Show resolved
Hide resolved
apps/upskii/src/app/api/v1/workspaces/[wsId]/quiz-sets/[setId]/submit/route.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please resolve issues found in our recent review. Thank you!
… feat/upskii/taking-quiz-ui
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
♻️ Duplicate comments (5)
apps/upskii/messages/vi.json (2)
3863-3874
: Fix typo in "second" translation.The key
"second": "giấy"
is misspelled. It should read:-"second": "giấy" +"second": "giây"
3850-3852
: Translate quiz action labels to Vietnamese.These strings remain in English and must be localized:
-"points": "Points", -"submitting": "Submitting...", -"submit": "Submit", +"points": "Điểm", +"submitting": "Đang gửi...", +"submit": "Gửi",apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/take/taking-quiz-client.tsx (3)
32-32
: Avoidany
for the instruction field (duplicate)
Same issue was flagged in a previous review; please introduce a proper typed shape instead ofany
.
139-139
: Stop casting API errors toany
(duplicate)
BothsetMetaError((json as any).error …)
andsetSubmitError((json as any).error …)
circumvent type-safety previously raised. Please introduce a typed error response instead.Also applies to: 266-266
98-101
: Emptycatch
blocks still silently swallow errors (duplicate)
These blocks ignore exceptions, making debugging painful. At minimum log or surface the error; consider a centralized handler.Also applies to: 150-152, 241-245, 384-385, 410-411
🧹 Nitpick comments (5)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/take/taking-quiz-client.tsx (1)
367-373
: Possible duplicate option IDs in multiple-choice array
nextArr.push(opt.id)
adds blindly; if the UI glitches, duplicates slip in and the payload length explodes. Guard with aSet
orincludes
check.-if (checked) { - nextArr.push(opt.id); +if (checked && !nextArr.includes(opt.id)) { + nextArr.push(opt.id); }apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/take/quiz-status-sidebar.tsx (2)
46-50
: Nested<aside>
elements hurt semantics
QuizStatusSidebar
itself returns an<aside>
while the parent places it inside another<aside>
for desktop. Consider using<section>
or<div>
here to avoid nested landmarks.
35-40
:el.focus()
may throw for non-focusable nodes
If the quiz card isn’t focusable, callingfocus()
triggers an exception in some browsers. Guard the call or addtabIndex={-1}
to the card container.apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/take/before-taking-quiz-whole.tsx (2)
121-123
: Left-overconsole.log
in production code
Debug prints leak to the console and bloat bundle size. Please remove.
98-100
:attemptsRemaining
can become negative
WhenattemptsSoFar
>attemptLimit
, the UI might show “-1 attempts remaining”. Clamp at 0.-const attemptsRemaining = quizData.attemptLimit - ? quizData.attemptLimit - quizData.attemptsSoFar - : null; +const attemptsRemaining = + quizData.attemptLimit == null + ? null + : Math.max(0, quizData.attemptLimit - quizData.attemptsSoFar);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/upskii/messages/en.json
(1 hunks)apps/upskii/messages/vi.json
(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/take/before-taking-quiz-whole.tsx
(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/take/quiz-status-sidebar.tsx
(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/take/taking-quiz-client.tsx
(1 hunks)apps/upskii/src/app/api/v1/workspaces/[wsId]/quiz-sets/[setId]/submit/route.ts
(2 hunks)apps/upskii/src/app/api/v1/workspaces/[wsId]/quiz-sets/[setId]/take/route.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/upskii/src/app/api/v1/workspaces/[wsId]/quiz-sets/[setId]/submit/route.ts
- apps/upskii/src/app/api/v1/workspaces/[wsId]/quiz-sets/[setId]/take/route.ts
- apps/upskii/messages/en.json
🧰 Additional context used
🪛 GitHub Check: codecov/patch
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/take/before-taking-quiz-whole.tsx
[warning] 2-442: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/take/before-taking-quiz-whole.tsx#L2-L442
Added lines #L2 - L442 were not covered by tests
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/take/quiz-status-sidebar.tsx
[warning] 2-119: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/take/quiz-status-sidebar.tsx#L2-L119
Added lines #L2 - L119 were not covered by tests
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/take/taking-quiz-client.tsx
[warning] 2-472: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/take/taking-quiz-client.tsx#L2-L472
Added lines #L2 - L472 were not covered by tests
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Verify generated types
...)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/take/taking-quiz-client.tsx
Show resolved
Hide resolved
...)/[wsId]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/take/taking-quiz-client.tsx
Show resolved
Hide resolved
...d]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/take/before-taking-quiz-whole.tsx
Show resolved
Hide resolved
...d]/courses/[courseId]/modules/[moduleId]/quiz-sets/[setId]/take/before-taking-quiz-whole.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks @Puppychan.
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Chores