-
-
Notifications
You must be signed in to change notification settings - Fork 19
Add new quiz set attributes #3047
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
Conversation
…eck if same quiz-set name, then auto increase
…thod to quiz-sets if generate quizzes by AI
… to any Supabase type definitions didn't include the workspace_quiz_attempts table, so used 'as any' to unblock the build. Needs proper type update later.
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. |
""" WalkthroughThis update introduces comprehensive quiz set functionality, including quiz grouping, attempt tracking, statistics, and localization. It adds new database tables and fields for quiz attempts and answers, implements API endpoints for quiz-taking and result submission, and provides React components for quiz-taking, status display, and statistics. Localization is expanded for new quiz features. Changes
Sequence Diagram(s)Quiz Set Creation and Quiz AdditionsequenceDiagram
participant User
participant Client (AIQuizzes)
participant API /quiz-sets
participant API /quizzes
User->>Client (AIQuizzes): Submit quizzes for module
Client->>API /quiz-sets: POST create quiz set (with module name)
API /quiz-sets-->>Client: Return quiz set ID
loop For each quiz
Client->>API /quizzes: POST create quiz (with set ID)
API /quizzes-->>Client: Quiz created
end
Quiz Taking and SubmissionsequenceDiagram
participant User
participant TakingQuizClient
participant API /quiz-sets/[setId]/take
participant API /quiz-sets/[setId]/submit
User->>TakingQuizClient: Start quiz
TakingQuizClient->>API /quiz-sets/[setId]/take: GET quiz set data
API /quiz-sets/[setId]/take-->>TakingQuizClient: Return quiz meta & questions
User->>TakingQuizClient: Select answers, submit
TakingQuizClient->>API /quiz-sets/[setId]/submit: POST answers
API /quiz-sets/[setId]/submit-->>TakingQuizClient: Return attempt result
TakingQuizClient-->>User: Show result summary
Fetching Quiz Set StatisticssequenceDiagram
participant User
participant QuizSetStatisticsPage
participant Supabase DB
User->>QuizSetStatisticsPage: Visit statistics page
QuizSetStatisticsPage->>Supabase DB: Query quiz set attempts & scores
Supabase DB-->>QuizSetStatisticsPage: Return statistics data
QuizSetStatisticsPage-->>User: Render summary and per-quiz stats
Possibly related PRs
Poem
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
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/taking-quiz-client.tsxOops! Something went wrong! :( ESLint: 9.28.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@typescript-eslint/parser' imported from /eslint.config.mjs apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/page.tsxOops! Something went wrong! :( ESLint: 9.28.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@typescript-eslint/parser' imported from /eslint.config.mjs apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/statistics/page.tsxOops! Something went wrong! :( ESLint: 9.28.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@typescript-eslint/parser' imported from /eslint.config.mjs
✨ Finishing Touches
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 (
|
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: 33
🔭 Outside diff range comments (2)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quizzes/client-ai.tsx (1)
48-60
: 🛠️ Refactor suggestionAdd Content-Type header and consider handling partial failures.
The quiz creation requests are missing the Content-Type header, and using
Promise.all
means that if one quiz creation fails, all are rejected.Apply this diff to add headers and consider using
Promise.allSettled
for better error handling:-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, - }), - }) -); - -await Promise.all(promises); +const promises = object.quizzes.map((quiz) => + fetch(`/api/v1/workspaces/${wsId}/quizzes`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + }, + body: JSON.stringify({ + setId: quizSet.setId, + moduleId, + question: quiz?.question, + quiz_options: quiz?.quiz_options, + }), + }) +); + +const results = await Promise.allSettled(promises); +const failedCount = results.filter(r => r.status === 'rejected').length; +if (failedCount > 0) { + console.warn(`${failedCount} quiz(es) failed to create`); +}apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quizzes/client-quizzes.tsx (1)
226-226
: 🛠️ Refactor suggestionDisable buttons when quiz ID is missing.
Buttons with undefined onClick handlers provide poor user experience. Disable them when the quiz ID is unavailable.
For line 226:
-onClick={quiz?.id ? () => setEditingQuizId(quiz.id!) : undefined} +onClick={() => setEditingQuizId(quiz.id!)} +disabled={!quiz?.id}For line 254:
-onClick={quiz?.id ? () => onDelete(quiz.id!) : undefined} +onClick={() => onDelete(quiz.id!)} +disabled={!quiz?.id}Also applies to: 254-254
🧹 Nitpick comments (20)
apps/db/supabase/migrations/20250606073849_new_migration.sql (1)
1-4
: Migration syntax is correct, consider migration planning strategy.The migration syntax is correct and the default values are reasonable. However, note that
results_released
is added here but removed in a later migration (20250608051026), which suggests evolving requirements. Consider planning migrations more comprehensively to avoid adding then removing columns.apps/db/supabase/migrations/20250605064241_new_migration.sql (1)
1-6
: Column additions are syntactically correct.The column additions are properly structured with appropriate defaults. However, note that
release_at
is added here but removed in a later migration, suggesting evolving requirements.apps/upskii/src/app/api/v1/workspaces/[wsId]/quiz-sets/route.ts (1)
44-70
: Consider performance optimization for large datasets.The current approach loads all matching names into memory. For better performance with large datasets, consider using a database-level solution to find the next available suffix.
Example alternative approach:
+ // Alternative: Use a more efficient query to find the next available suffix + const { data: maxSuffix } = await supabase + .from('workspace_quiz_sets') + .select('name') + .eq('ws_id', id) + .like('name', `${formattedName}%`) + .order('name', { ascending: false }) + .limit(1); + + // Extract suffix and increment logic hereapps/upskii/src/lib/release-quiz-sets.ts (3)
1-1
: Fix the file path comment to match actual location.The comment indicates
lib/releaseQuizSets.ts
but the actual file path isapps/upskii/src/lib/release-quiz-sets.ts
.-// File: lib/releaseQuizSets.ts +// File: apps/upskii/src/lib/release-quiz-sets.ts
5-9
: Fix typographical inconsistency in documentation.The documentation uses a non-standard dash character in "quiz‐sets" instead of a regular hyphen.
- * Finds all quiz‐sets where: + * Finds all quiz-sets where:
23-23
: Consider structured logging instead of console.error.Using
console.error
may not be appropriate for production environments. Consider using a structured logging library.- console.error('Error querying quiz‐sets to release:', findErr); + // Use your application's logging service instead + // logger.error('Error querying quiz-sets to release', { error: findErr }); + console.error('Error querying quiz-sets to release:', findErr);apps/upskii/messages/vi.json (1)
3783-3794
: New quiz navigation and status labels look accurate.
The added keys (question_status_title
,answered_status_short
,quiz_progress_label
,question_navigation_label
,jump_to_question_aria
,answered_state
,unanswered_state
,answered_icon
,unanswered_icon
,time_elapsed
,hidden_time_elapsed
,hidden_time_remaining
) follow the established snake_case convention and the Vietnamese translations are clear.apps/upskii/messages/en.json (1)
3797-3816
: Verify and propagate new quiz control strings
These additions cover submission, scoring, and navigation states. Ensure they align with the frontend implementation, are included in all locale files, and that the placeholders injump_to_question_aria
are correctly populated at runtime.apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/time-elapsed-status.tsx (1)
41-47
: Simplify nested ternary expressions for better readability.The nested ternary expressions make the code difficult to read and maintain.
Extract the logic into a helper function:
const getTimerText = () => { if (!isVisible) { return timeLeft !== null ? t('ws-quizzes.hidden_time_remaining') || 'Time Hidden' : t('ws-quizzes.hidden_time_elapsed') || 'Time Hidden'; } const formattedTime = timeLeft !== null ? formatSeconds(timeLeft) : '--:--'; return `${timerLabel}: ${formattedTime}`; }; // Then in the JSX: <p className={`text-sm md:text-base lg:text-lg ${timerColorClass}`}> {getTimerText()} </p>apps/upskii/src/app/api/v1/workspaces/[wsId]/quiz-sets/[setId]/submit/route.ts (1)
134-137
: Log or track quizzes not found in the map.Silently skipping quizzes that aren't in the map could hide data integrity issues.
if (!qInfo) { - // If the quizId isn't in our map, ignore it + // If the quizId isn't in our map, log and skip + console.warn(`Quiz ${quizId} not found in set ${setId}`); continue; }apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/before-take-quiz-section.tsx (2)
24-54
: Centralize translation fallback strings.Multiple hardcoded fallback strings are scattered throughout the component. Consider centralizing them for better maintainability.
const TRANSLATION_FALLBACKS = { 'ws-quizzes.due_on': 'Due on', 'ws-quizzes.attempts': 'Attempts', 'ws-quizzes.unlimited': 'unlimited', 'ws-quizzes.time_limit': 'Time Limit', 'ws-quizzes.minutes': 'minutes', 'ws-quizzes.no_time_limit': 'No time limit', 'ws-quizzes.take_quiz': 'Take Quiz', }; // Then use: t('ws-quizzes.due_on') || TRANSLATION_FALLBACKS['ws-quizzes.due_on']
51-51
: Consider using consistent styling approach.The Button component uses inline Tailwind classes. Ensure this aligns with your project's styling conventions.
If your project uses a consistent theming system or CSS-in-JS solution, consider extracting these styles to maintain consistency across components.
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/quiz-status-sidebar.tsx (2)
27-27
: Fix unused parameters to resolve linting warning.The pipeline failure indicates that
options
parameter in the translation function signature is not used. Remove it to fix the linting warning.- t: (key: string, options?: Record<string, any>) => string; + t: (key: string) => string;Also update the
translate
function call sites that don't actually use options:- const translation = t(key, options); + const translation = t(key);🧰 Tools
🪛 GitHub Actions: Vercel Upskii Preview Deployment
[warning] 27-27: ESLint warnings: 'key' and 'options' are defined but never used. (no-unused-vars)
98-102
: Simplify complex aria-label construction.The nested translation calls in the aria-label make the code hard to read and maintain. Consider simplifying this logic.
- aria-label={translate( - 'ws-quizzes.jump_to_question_aria', - `Question ${questionNumber}, ${isAnswered ? translate('ws-quizzes.answered_state', 'Answered') : translate('ws-quizzes.unanswered_state', 'Unanswered')}`, - { number: questionNumber } - )} + aria-label={`Question ${questionNumber}, ${isAnswered ? translate('ws-quizzes.answered_state', 'Answered') : translate('ws-quizzes.unanswered_state', 'Unanswered')}`}apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/page.tsx (2)
1-1
: Remove duplicate file path commentsThese repeated file path comments add no value and should be removed to clean up the code.
-// File: app/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quizzes/[setId]/take/page.tsx 'use client'; import BeforeTakeQuizSection from '@/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/before-take-quiz-section'; import PastDueSection from '@/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/past-due-section'; import QuizStatusSidebar, { Question, } from '@/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/quiz-status-sidebar'; import ShowResultSummarySection from '@/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/show-result-summary-section'; import TimeElapsedStatus from '@/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/time-elapsed-status'; import { Button } from '@tuturuuu/ui/button'; import { ListCheck } from '@tuturuuu/ui/icons'; import { useTranslations } from 'next-intl'; import { useRouter } from 'next/navigation'; import { useEffect, useRef, useState } from 'react'; -// File: app/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quizzes/[setId]/take/page.tsx - -// File: app/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quizzes/[setId]/take/page.tsx - -// File: app/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quizzes/[setId]/take/page.tsx - -// File: app/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quizzes/[setId]/take/page.tsx - -// File: app/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quizzes/[setId]/take/page.tsx - type TakeResponse = {Also applies to: 17-26
45-480
: Consider breaking down this large componentThis 480-line component handles many responsibilities. Consider extracting:
- Timer logic into a custom hook (e.g.,
useQuizTimer
)- Quiz form into a separate component
- Submission logic into a custom hook
This would improve maintainability and testability.
🧰 Tools
🪛 GitHub Actions: Vercel Upskii Preview Deployment
[warning] 71-198: ESLint warnings: 'NodeJS' is not defined (no-undef), empty block statements (no-empty), and missing definition for rule 'react-hooks/exhaustive-deps'.
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quizzes/page.tsx (3)
46-46
: Remove debug console.logDebug logging should not be left in production code.
const quizSets = await getQuizzes(moduleId); - console.log('Quiz Sets:', quizSets); const moduleName = await getModuleName(moduleId);
72-87
: Remove commented code blockLarge blocks of commented code should be removed. Version control preserves the history if this code is needed later.
</div> - {/* <div className="grid gap-4 md:grid-cols-2"> - {quizSets && quizSets.length > 0 && ( - <> - <ClientQuizzes - wsId={wsId} - moduleId={moduleId} - quizSets={quizSets} - /> - <Separator className="col-span-full my-2" /> - </> - )} - - <div className="col-span-full"> - <AIQuizzes wsId={wsId} moduleId={moduleId} moduleName={moduleName} /> - </div> - </div> */} </div>
93-97
: Move data structure comments to documentationThese comments showing the data structure would be better placed in documentation or as JSDoc comments rather than inline.
+/** + * Fetches and groups quizzes by their quiz set + * @param moduleId - The module ID to fetch quizzes for + * @returns Array of quiz sets with their quizzes + */ const getQuizzes = async (moduleId: string) => { - // created_at: "2025-05-29T08:12:16.653395+00:00" - // id: "426d031f-2dc4-4370-972d-756af04288fb" - // question: "What are the main building blocks of a NestJS application?" - // quiz_options: (4) [{…}, {…}, {…}, {…}] - // ws_id: "00000000-0000-0000-0000-000000000000" const supabase = await createClient();apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quizzes/client-quizzes.tsx (1)
113-113
: Move Separator outside the grid container.The Separator is placed inside a grid but uses
col-span-full
, which may not render correctly depending on the grid structure.Move the Separator outside the grid to ensure proper rendering:
- <Separator className="col-span-full my-2" /> </div> + <Separator className="my-2" /> </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
apps/db/supabase/migrations/20250604125645_new_migration.sql
(1 hunks)apps/db/supabase/migrations/20250605064241_new_migration.sql
(1 hunks)apps/db/supabase/migrations/20250606073849_new_migration.sql
(1 hunks)apps/db/supabase/migrations/20250608051026_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
(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
(4 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quizzes/page.tsx
(2 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/result/page.tsx
(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/before-take-quiz-section.tsx
(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/page.tsx
(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/past-due-section.tsx
(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/quiz-status-sidebar.tsx
(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/show-result-summary-section.tsx
(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/time-elapsed-status.tsx
(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quizzes/mock/quizzes-mock-data.ts
(0 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
(1 hunks)apps/upskii/src/app/api/v1/workspaces/[wsId]/quiz-sets/[setId]/take/route.ts
(1 hunks)apps/upskii/src/app/api/v1/workspaces/[wsId]/quiz-sets/route.ts
(2 hunks)apps/upskii/src/lib/release-quiz-sets.ts
(1 hunks)packages/types/src/supabase.ts
(3 hunks)
💤 Files with no reviewable changes (1)
- apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quizzes/mock/quizzes-mock-data.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/page.tsx (5)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/quiz-status-sidebar.tsx (1)
Question
(17-22)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/past-due-section.tsx (1)
PastDueSection
(4-49)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/show-result-summary-section.tsx (1)
ShowResultSummarySection
(3-57)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/before-take-quiz-section.tsx (1)
BeforeTakeQuizSection
(4-58)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/time-elapsed-status.tsx (1)
TimeElapsedStatus
(19-62)
🪛 GitHub Actions: Vercel Upskii Preview Deployment
apps/upskii/src/app/api/v1/workspaces/[wsId]/quiz-sets/[setId]/submit/route.ts
[warning] 44-44: ESLint warning: 'e' is defined but never used. (no-unused-vars)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/time-elapsed-status.tsx
[warning] 14-14: ESLint warnings: 'key' and 'options' are defined but never used. (no-unused-vars)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/before-take-quiz-section.tsx
[warning] 10-10: ESLint warnings: 'key' and 'options' are defined but never used. (no-unused-vars)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/result/page.tsx
[error] 1-1: Type error: Type '{ params: { wsId: string; courseId: string; moduleId: string; setId: string; }; }' does not satisfy the constraint 'PageProps'. Missing properties from type 'Promise': then, catch, finally, [Symbol.toStringTag].
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/show-result-summary-section.tsx
[warning] 12-28: ESLint warnings: 'key' and 'url' are defined but never used. (no-unused-vars)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/quiz-status-sidebar.tsx
[warning] 27-27: ESLint warnings: 'key' and 'options' are defined but never used. (no-unused-vars)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/past-due-section.tsx
[warning] 14-25: ESLint warnings: 'key' and 'url' are defined but never used. (no-unused-vars)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quizzes/client-quizzes.tsx
[warning] 29-32: ESLint warnings: 'id' is defined but never used. (no-unused-vars)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/page.tsx
[warning] 71-198: ESLint warnings: 'NodeJS' is not defined (no-undef), empty block statements (no-empty), and missing definition for rule 'react-hooks/exhaustive-deps'.
🔇 Additional comments (19)
apps/db/supabase/migrations/20250605064241_new_migration.sql (1)
9-18
: SQL function implementation is correct.The
sum_quiz_scores
function is well-implemented with proper error handling usingCOALESCE
to handle null values. The join logic and aggregation are appropriate for calculating total quiz scores within a set.apps/upskii/src/app/api/v1/workspaces/[wsId]/quiz-sets/route.ts (4)
35-42
: Name validation logic is correct.The name validation properly checks for required fields and trims whitespace appropriately.
57-70
: Name uniqueness logic is correct once query is fixed.The logic for generating unique names with numeric suffixes is well-implemented. It properly handles the case where no conflicts exist and incrementally tries new suffixes until a unique name is found.
72-88
: Quiz set insertion logic is correct.The insertion logic properly uses the generated unique name and handles errors appropriately.
111-111
: Response includes appropriate data.The response correctly includes the success message, set ID, and the final resolved name for the client.
apps/upskii/messages/vi.json (1)
3797-3816
: Quiz feature translations added correctly.
The entries (generation_accepted
,please_answer_all
,loading
,results
,attempt
,of
,unlimited
,score
,done
,attempts
,time_limit
,no_time_limit
,minutes
,take_quiz
,time_remaining
,points
,submitting
,submit
,due_on
,quiz_past_due
) are consistent with the rest of the localization file and capture the intended meaning. No duplicate keys detected in this section.apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/page.tsx (1)
43-43
: LGTM! Consistent variable naming.The renaming from
quizzes
toquizSets
properly reflects the new grouped data structure.Also applies to: 137-142
apps/upskii/src/app/api/v1/workspaces/[wsId]/quiz-sets/[setId]/submit/route.ts (1)
151-208
: LGTM! Well-structured quiz submission logic.The implementation correctly:
- Validates attempt limits
- Calculates scores based on correct answers
- Records individual answer details
- Handles database transactions appropriately
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/result/page.tsx (1)
30-39
:⚠️ Potential issueFix TypeScript constraint error for Next.js page props.
The pipeline failure indicates that the
params
prop doesn't satisfy thePageProps
constraint. In Next.js 13+ app directory, page props should be promises.export default function ViewResults({ params, }: { - params: { + params: Promise<{ wsId: string; courseId: string; moduleId: string; setId: string; - }; + }>; }) { - const { wsId, courseId, moduleId, setId } = params; + const { wsId, courseId, moduleId, setId } = await params;Note: This will require marking the component as async.
Likely an incorrect or invalid review comment.
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/show-result-summary-section.tsx (2)
12-29
: ESLint warning appears to be a false positiveThe pipeline warning about unused 'key' and 'url' parameters on lines 12-28 seems incorrect. These lines define the component's prop types, not actual parameters. The warning might be due to ESLint configuration issues or a parsing error.
🧰 Tools
🪛 GitHub Actions: Vercel Upskii Preview Deployment
[warning] 12-28: ESLint warnings: 'key' and 'url' are defined but never used. (no-unused-vars)
31-57
: Well-implemented result summary componentThe component properly displays quiz attempt results with good null handling for
attemptLimit
and appropriate translation fallbacks.apps/db/supabase/migrations/20250604125645_new_migration.sql (1)
1-145
: Well-structured database migrationThe migration properly implements quiz attempt tracking with:
- Appropriate table structure and relationships
- Proper constraints including unique constraint for attempt numbers
- Cascading deletes for data integrity
- Comprehensive permissions for all roles
- Efficient indexes
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quizzes/page.tsx (1)
92-177
: Well-implemented quiz grouping logicThe refactored
getQuizzes
function properly groups quizzes by their sets, and the newgetModuleName
function provides necessary context. Good error handling with appropriate fallbacks.apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quizzes/client-quizzes.tsx (1)
90-119
: Quiz set rendering implementation looks good!The conditional rendering logic cleanly handles both quiz sets and flat quiz lists, maintaining backward compatibility while adding the new grouped functionality.
packages/types/src/supabase.ts (5)
5459-5507
: LGTM! Well-structured type definition for quiz answer tracking.The
workspace_quiz_attempt_answers
type properly defines the schema for tracking individual quiz answers with appropriate foreign key relationships to attempts, questions, and selected options.
5508-5566
: LGTM! Comprehensive type definition for quiz attempt tracking.The
workspace_quiz_attempts
type correctly models quiz attempts with appropriate nullable fields for in-progress attempts and proper relationships to quiz sets and users.
5567-5610
: LGTM! Good extension of quiz set configuration options.The new fields provide comprehensive control over quiz behavior with sensible defaults and appropriate nullable fields for optional constraints.
5611-5642
: LGTM! Appropriate addition of score field to quizzes.The
score
field with a default value of 1 enables flexible point allocation for individual quiz questions.
7370-7375
: Verify the intended return type of sum_quiz_scores function.The function returns an array of objects with just a
sum
property. This seems unusual - typically you'd expect either a single sum value or an array with more context (e.g., sum per user, sum per attempt).Could you confirm if this return type matches the intended use case? The function might be aggregating scores across multiple dimensions that aren't reflected in the type definition.
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/page.tsx
Outdated
Show resolved
Hide resolved
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/page.tsx
Outdated
Show resolved
Hide resolved
...[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quizzes/client-quizzes.tsx
Show resolved
Hide resolved
...[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quizzes/client-quizzes.tsx
Outdated
Show resolved
Hide resolved
...[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/quizzes/client-quizzes.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.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/quiz-status-sidebar.tsx (1)
3-9
: Move function inside component and add error handling for DOM operations.The
onQuestionJump
function is defined at module level but only used within the component. It should be moved inside the component and include error handling for missing elements.
🧹 Nitpick comments (1)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/quiz-status-sidebar.tsx (1)
40-50
: Simplify the translation function implementation.The current translate function implementation is overly complex for a simple fallback scenario. The logic can be simplified while maintaining the same functionality.
Apply this diff to simplify the translation function:
- // Fallback for t function if not provided or key is missing - const translate = useCallback( - (key: string, defaultText: string, options: Record<string, any> = {}) => { - if (typeof t === 'function') { - const translation = t(key, options); - // i18next might return the key if not found, so check against that too - return translation === key ? defaultText : translation || defaultText; - } - return defaultText; - }, - [t] - ); + const translate = useCallback( + (key: string, defaultText: string, options: Record<string, any> = {}) => { + try { + const translation = t?.(key, options); + return translation && translation !== key ? translation : defaultText; + } catch { + return defaultText; + } + }, + [t] + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/before-take-quiz-section.tsx
(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/page.tsx
(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/quiz-status-sidebar.tsx
(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/time-elapsed-status.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/time-elapsed-status.tsx
- apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/before-take-quiz-section.tsx
- apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/page.tsx
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
🔇 Additional comments (2)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/quiz-status-sidebar.tsx (2)
98-102
: Excellent accessibility implementation.The ARIA labels and navigation structure provide comprehensive accessibility support for screen readers and keyboard navigation. The dynamic labels that include question numbers and answered states are particularly well implemented.
64-79
: Well-implemented progress bar with proper ARIA attributes.The progress bar correctly uses ARIA attributes for accessibility and handles the edge case of zero questions. The visual styling with smooth transitions provides good user experience.
…at/upskii/taking-quiz
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 (3)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/page.tsx (3)
132-133
: Add error handling to empty catch block.Empty catch blocks hide errors and make debugging difficult. This was flagged in previous reviews but remains unresolved.
-} catch { +} catch (error) { + console.error('Failed to fetch quiz metadata:', error); setMetaError('Network error'); }
147-174
: Fix missing dependencies in timer useEffect.The effect calls
handleSubmit
but doesn't include it in the dependency array, which can lead to stale closures.Wrap
handleSubmit
inuseCallback
to create a stable reference:+const handleSubmit = useCallback(async () => { + // ... existing handleSubmit logic +}, [quizMeta, selectedAnswers, wsId, setId]);Then update the dependency array:
-}, [hasStarted, quizMeta]); +}, [hasStarted, quizMeta, totalSeconds, handleSubmit]);
176-180
: Fix missing handleSubmit dependency.The
handleSubmit
function is called but not included in the dependency array.-}, [timeLeft, hasStarted, totalSeconds]); +}, [timeLeft, hasStarted, totalSeconds, handleSubmit]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/page.tsx
(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
(1 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]/results/route.ts
- apps/upskii/src/app/api/v1/workspaces/[wsId]/quiz-sets/[setId]/take/route.ts
- apps/upskii/src/app/api/v1/workspaces/[wsId]/quiz-sets/[setId]/submit/route.ts
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Deploy-Preview
- GitHub Check: Verify generated types
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/page.tsx
Outdated
Show resolved
Hide resolved
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/page.tsx
Outdated
Show resolved
Hide resolved
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/page.tsx
Show resolved
Hide resolved
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/page.tsx
Outdated
Show resolved
Hide resolved
…kii/quiz-set-statistics' into feat/upskii/taking-quiz
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: 14
🧹 Nitpick comments (3)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/statistics/page.tsx (1)
299-304
: Extract pass threshold as a configurable valueThe pass threshold of 70 is hardcoded. Consider making this configurable per quiz set or extracting it as a constant.
Add a constant at the top of the file:
const PASS_THRESHOLD = 70;Then use it in the calculation:
- ? (attempts.filter((a) => (a.score || 0) >= 70).length / + ? (attempts.filter((a) => (a.score || 0) >= PASS_THRESHOLD).length /apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/sections/before-take-quiz-section.tsx (1)
34-39
: Add fallback for unlimited translation.For consistency with other translations, add a fallback value.
-{t('ws-quizzes.unlimited')} +{t('ws-quizzes.unlimited') || 'unlimited'}apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/sections/past-due-section.tsx (1)
39-41
: Consider extracting route path construction.Manual path construction is error-prone and duplicated across components.
Consider creating a route helper:
// In a separate utils file export const getQuizResultsPath = (wsId: string, courseId: string, moduleId: string, setId: string) => `/dashboard/${wsId}/courses/${courseId}/modules/${moduleId}/quizzes/${setId}/results`; // Then use it here router.push(getQuizResultsPath(wsId, courseId, moduleId, setId))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/upskii/messages/en.json
(3 hunks)apps/upskii/messages/vi.json
(3 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/layout.tsx
(2 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/page.tsx
(2 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/statistics/page.tsx
(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/page.tsx
(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/sections/before-take-quiz-section.tsx
(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/sections/past-due-section.tsx
(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/sections/show-result-summary-section.tsx
(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/taking-quiz-client.tsx
(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/page.tsx
- apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/layout.tsx
- apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/sections/show-result-summary-section.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/page.tsx
- apps/upskii/messages/vi.json
- apps/upskii/messages/en.json
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
🔇 Additional comments (2)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/statistics/page.tsx (1)
272-275
:❌ Incorrect review comment
Verify Supabase query syntax
The spread operator syntax in the select query may not be standard Supabase/PostgREST syntax.
Consider using standard join syntax:
- const { data: quizzes } = await supabase - .from('quiz_set_quizzes') - .select('...workspace_quizzes(id, question)') - .eq('set_id', setId); + const { data: quizzes } = await supabase + .from('quiz_set_quizzes') + .select('*, workspace_quizzes!inner(id, question)') + .eq('set_id', setId);Verify this works with your Supabase schema:
🏁 Script executed:
#!/bin/bash # Search for similar Supabase query patterns in the codebase rg -A 3 "\.from\(['\"]quiz_set_quizzes" --type tsx --type tsLength of output: 92
🏁 Script executed:
#!/bin/bash # Search for Supabase queries on quiz_set_quizzes without file type filter rg -A 5 "from\\('quiz_set_quizzes'" -g "*.ts*"Length of output: 6014
Supabase spread-operator syntax is used consistently across the codebase
I found the same
…workspace_quizzes(...)
pattern in multiple files—including bothapps/web
andapps/upskii
—which indicates this is an intentional, supported convention in your setup. You can safely ignore the suggestion to switch to*, workspace_quizzes!inner(...)
syntax here.Likely an incorrect or invalid review comment.
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/taking-quiz-client.tsx (1)
71-73
:⚠️ Potential issueVariable accessed before initialization.
totalSeconds
referencesquizMeta?.timeLimitMinutes
butquizMeta
is null initially and only populated after the API call. This could cause issues whentotalSeconds
is used in the firstuseEffect
.Move this calculation inside components/functions where
quizMeta
is guaranteed to be available, or use a useMemo:-const totalSeconds = quizMeta?.timeLimitMinutes - ? quizMeta.timeLimitMinutes * 60 - : null; +const totalSeconds = useMemo(() => + quizMeta?.timeLimitMinutes ? quizMeta.timeLimitMinutes * 60 : null, + [quizMeta?.timeLimitMinutes] +);Likely an incorrect or invalid review comment.
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/statistics/page.tsx
Show resolved
Hide resolved
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/statistics/page.tsx
Show resolved
Hide resolved
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/statistics/page.tsx
Show resolved
Hide resolved
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/statistics/page.tsx
Show resolved
Hide resolved
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/statistics/page.tsx
Outdated
Show resolved
Hide resolved
...kii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/sections/past-due-section.tsx
Show resolved
Hide resolved
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/taking-quiz-client.tsx
Show resolved
Hide resolved
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/taking-quiz-client.tsx
Show resolved
Hide resolved
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/taking-quiz-client.tsx
Outdated
Show resolved
Hide resolved
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/taking-quiz-client.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.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/types/src/supabase.ts (1)
5907-5950
: 🛠️ Refactor suggestionReview due_date field nullability.
The
due_date
field inworkspace_quiz_sets
is marked as non-nullable in the Insert type but should likely be nullable to support quiz sets without deadlines. Consider changing the Insert type todue_date?: string | null
for consistency with typical quiz management patterns.Otherwise, the new quiz management fields appropriately extend the table's capabilities.
Apply this change to make due_date optional in Insert:
Insert: { allow_view_results?: boolean; attempt_limit?: number | null; created_at?: string; - due_date?: string; + due_date?: string | null; id?: string; name?: string; release_points_immediately?: boolean; time_limit_minutes?: number | null; ws_id?: string | null; };
♻️ Duplicate comments (7)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/taking-quiz-client.tsx (7)
78-80
: Empty catch blocks hide errors.This issue persists from previous reviews.
92-143
: Add missingwsId
touseEffect
dependencies.This issue persists from previous reviews - the effect uses both
wsId
andsetId
but only includessetId
in dependencies.
129-129
: Empty catch blocks hide errors.This issue persists from previous reviews.
145-173
: Missing dependencies in useEffect hooks.This issue persists from previous reviews - the timer useEffect is missing dependencies.
175-179
: Missing dependencies in useEffect hooks.This issue persists from previous reviews - the timer useEffect is missing
handleSubmit
dependency.
186-187
: Empty catch blocks hide errors.This issue persists from previous reviews.
33-468
: Component is too large and complex.This issue persists from previous reviews - the component handles too many responsibilities and should be broken down into smaller, focused components and custom hooks.
🧹 Nitpick comments (5)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/taking-quiz-client.tsx (5)
102-103
: Improve type safety in error handling.The type assertion
(json as any).error
bypasses TypeScript's type checking and could lead to runtime errors.- if (!res.ok) { - setMetaError((json as any).error || 'Unknown error'); + if (!res.ok) { + const errorResponse = json as { error: string }; + setMetaError(errorResponse.error || 'Unknown error');
116-129
: Extract localStorage logic into a helper function.The localStorage access logic is repeated and mixed with business logic, making it harder to maintain and test.
const getStoredStartTime = (): number | null => { try { const stored = localStorage.getItem(STORAGE_KEY); if (!stored) return null; const startTs = parseInt(stored, 10); return isNaN(startTs) ? null : startTs; } catch (error) { console.error('Failed to access localStorage:', error); return null; } };
208-210
: Improve type safety in error handling.Similar to the earlier issue, the type assertion bypasses TypeScript checking.
- if (!res.ok) { - setSubmitError((json as any).error || 'Submission failed.'); + if (!res.ok) { + const errorResponse = json as { error: string }; + setSubmitError(errorResponse.error || 'Submission failed.');
270-296
: Extract inline conditional rendering into separate component.The immediate-release results section contains substantial logic and JSX that could be extracted into a dedicated component for better maintainability.
// Extract to: ImmediateResultsSection.tsx const ImmediateResultsSection = ({ quizMeta, wsId, courseId, moduleId, dueDateStr, t, router }) => { // Current JSX content from lines 272-294 };
299-335
: Extract no attempts left section into separate component.This section also contains substantial logic and could benefit from extraction into a dedicated component.
// Extract to: NoAttemptsLeftSection.tsx const NoAttemptsLeftSection = ({ quizMeta, wsId, courseId, moduleId, dueDateStr, t, router }) => { // Current JSX content from lines 305-333 };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/statistics/page.tsx
(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/page.tsx
(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/taking-quiz-client.tsx
(1 hunks)packages/types/src/supabase.ts
(20 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/page.tsx
- apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/statistics/page.tsx
🔇 Additional comments (4)
packages/types/src/supabase.ts (4)
5799-5847
: LGTM: Well-designed quiz answer tracking table.The
workspace_quiz_attempt_answers
table is properly structured with appropriate fields and relationships for tracking individual quiz responses within attempts.
5848-5906
: LGTM: Comprehensive quiz attempt tracking.The
workspace_quiz_attempts
table effectively models quiz attempts with proper relationships, timing tracking, and scoring capabilities.
5956-5956
: LGTM: Quiz scoring field addition.The addition of the
score
field toworkspace_quizzes
appropriately supports individual quiz scoring within the new quiz attempt system.Also applies to: 5963-5963, 5970-5970
7760-7765
: LGTM: Useful utility function for quiz set scoring.The
sum_quiz_scores
function provides a clean way to calculate total scores for a quiz set, which will be valuable for quiz statistics and result calculations.
async function handleSubmit() { | ||
if (!quizMeta) return; | ||
setSubmitting(true); | ||
setSubmitError(null); | ||
|
||
try { | ||
const res = await fetch( | ||
`/api/v1/workspaces/${wsId}/quiz-sets/${setId}/submit`, | ||
{ | ||
method: 'POST', | ||
headers: { 'Content-Type': 'application/json' }, | ||
body: JSON.stringify(buildSubmissionPayload()), | ||
} | ||
); | ||
const json: SubmitResult | { error: string } = await res.json(); | ||
|
||
if (!res.ok) { | ||
setSubmitError((json as any).error || 'Submission failed.'); | ||
return setSubmitting(false); | ||
} | ||
|
||
clearStartTimestamp(); | ||
setSubmitResult(json as SubmitResult); | ||
} catch { | ||
setSubmitError('Network error submitting.'); | ||
} finally { | ||
setSubmitting(false); | ||
} | ||
} |
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.
🛠️ Refactor suggestion
Wrap handleSubmit
in useCallback
to fix useEffect dependencies.
The handleSubmit
function is called from useEffect hooks but isn't memoized, which causes the missing dependency warnings. Wrap it in useCallback
with proper dependencies.
const handleSubmit = useCallback(async () => {
if (!quizMeta) return;
setSubmitting(true);
setSubmitError(null);
// ... rest of the function
}, [quizMeta, wsId, setId, selectedAnswers]);
🤖 Prompt for AI Agents
In
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/quiz-sets/[setId]/take/taking-quiz-client.tsx
around lines 192 to 220, the handleSubmit function is not memoized, causing
missing dependency warnings in useEffect hooks that call it. Wrap handleSubmit
in a useCallback hook, including all relevant dependencies such as quizMeta,
wsId, setId, and selectedAnswers, to ensure it is properly memoized and stable
for useEffect dependencies.
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
Bug Fixes
Refactor
Chores