feat(escape-room): ship generalized quiz mode - #5143
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughEscape Room mode and QR Scan elements are added across persistence, GraphQL, response processing, management forms, participant applications, lecturer evaluation, scheduled pruning, documentation, and automated tests. The implementation includes timed attempts, sequential gating, hints, lockouts, QR grading, reset controls, and progress reporting. ChangesEscape Room and QR Scan Platform
Estimated code review effort: 5 (Critical) | ~180 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
dosubot
Bot
added
the
size:XXL
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
rschlaefli
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/graphql/src/services/groups.ts (1)
1508-1720: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMake the attempt transition atomic.
Concurrent group members can both pass the
IN_PROGRESScheck, then race to overwritedecisionsand set completion/lockout state. Persist answers, decisions, and a conditional attempt-state transition in one transaction so only one submission wins.🤖 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 `@packages/graphql/src/services/groups.ts` around lines 1508 - 1720, Refactor the escape-room submission flow around the answer persistence block, `updatedActivityInstance` update, and `escapeRoomAttempt` updates to use one Prisma transaction. Within that transaction, persist all answer results and decisions, then conditionally transition the attempt from `IN_PROGRESS` using an updateMany or equivalent guarded update so concurrent submissions cannot both win; derive completion or lockout behavior from the conditional update count and roll back all changes when the submission is rejected.
🟠 Major comments (30)
packages/graphql/src/schema/groupActivity.ts-18-18 (1)
18-18: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUse a path alias for the configuration import.
As per coding guidelines, “Use
@and~path aliases for imports.”🤖 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 `@packages/graphql/src/schema/groupActivity.ts` at line 18, Replace the relative EscapeRoomConfigRef import in groupActivity.ts with the configured @ or ~ path alias, matching the project’s existing alias convention for this module.Source: Coding guidelines
packages/graphql/src/index.ts-13-13 (1)
13-13: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUse configured path aliases for the new imports.
Replace these relative module paths with the corresponding
@or~aliases.As per coding guidelines, “Use
@and~path aliases for imports.”Also applies to: 63-63
🤖 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 `@packages/graphql/src/index.ts` at line 13, Update the new imports in the GraphQL entry module, including the schema/escapeRoomConfig import and the other affected import, to use the project’s configured @ or ~ path aliases instead of relative module paths.Source: Coding guidelines
packages/graphql/src/schema/escapeRoomConfig.ts-2-2 (1)
2-2: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUse a path alias for the builder import.
As per coding guidelines, “Use
@and~path aliases for imports.”🤖 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 `@packages/graphql/src/schema/escapeRoomConfig.ts` at line 2, Replace the relative builder import in the escape room configuration module with the configured @ or ~ path alias, preserving the existing builder symbol and import behavior.Source: Coding guidelines
packages/graphql/src/schema/practiceQuiz.ts-18-18 (1)
18-18: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUse a path alias for the configuration import.
As per coding guidelines, “Use
@and~path aliases for imports.”🤖 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 `@packages/graphql/src/schema/practiceQuiz.ts` at line 18, Update the import in the practice quiz schema to use the configured @ or ~ path alias instead of the relative './escapeRoomConfig.js' path, while preserving the EscapeRoomConfigRef symbol.Source: Coding guidelines
packages/graphql/src/schema/liveQuiz.ts-5-6 (1)
5-6: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUse path aliases for the new schema imports.
As per coding guidelines, “Use
@and~path aliases for imports.”🤖 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 `@packages/graphql/src/schema/liveQuiz.ts` around lines 5 - 6, Update the imports in the live quiz schema to use the project’s configured @ or ~ path aliases instead of relative paths, including EscapeRoomConfigRef, EscapeRoomAttemptRef, and PublicationStatus.Source: Coding guidelines
apps/analytics/prisma/schema/quiz.prisma-489-499 (1)
489-499: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winEnforce one escape-room target per row at
apps/analytics/prisma/schema/quiz.prisma:482-536.EscapeRoomConfigandEscapeRoomAttemptstill allow multiple nullable relation columns to be set together, or none at all, so a row can point at conflicting activities or the wrong participant/group scope. The migration only adds unique indexes and foreign keys; add SQLCHECKconstraints for exactly one config target and one valid attempt target.🤖 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 `@apps/analytics/prisma/schema/quiz.prisma` around lines 489 - 499, The EscapeRoomConfig and EscapeRoomAttempt models allow zero or multiple nullable targets. Add migration SQL CHECK constraints enforcing exactly one non-null target per config row and exactly one valid target per attempt row, including participant/group scope consistency as required by the existing relations. Update the Prisma migration rather than relying on the schema’s unique indexes and foreign keys alone.apps/frontend-manage/src/components/activities/creation/practiceQuiz/PracticeQuizWizard.tsx-134-152 (1)
134-152: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winSame hardcoded validation messages as MicroLearningWizard.
Apply the same i18n fix suggested for
MicroLearningWizard.tsxlines 140-158.🤖 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 `@apps/frontend-manage/src/components/activities/creation/practiceQuiz/PracticeQuizWizard.tsx` around lines 134 - 152, Replace the hardcoded validation messages in the PracticeQuizWizard validation schema for escapeRoomTimeLimit and escapeRoomHintPenalty with the same i18n translation approach used by MicroLearningWizard, including required, integer, positive, and minimum-value messages.apps/frontend-manage/src/components/activities/creation/practiceQuiz/PracticeQuizSettingsStep.tsx-166-206 (1)
166-206: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winSame hardcoded English strings as MicroLearningSettingsStep.
The escape room labels here are also hardcoded instead of using
t(). Apply the same i18n fix as suggested forMicroLearningSettingsStep.tsx.🤖 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 `@apps/frontend-manage/src/components/activities/creation/practiceQuiz/PracticeQuizSettingsStep.tsx` around lines 166 - 206, Replace the hardcoded “Escape Room Mode”, “Time Limit (minutes)”, and “Hint Penalty (seconds)” labels in the PracticeQuizSettingsStep escape-room section with translated strings using the existing t() i18n pattern from MicroLearningSettingsStep. Preserve the current field behavior and required configuration.apps/frontend-manage/src/components/activities/creation/microLearning/MicroLearningSettingsStep.tsx-165-202 (1)
165-202: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUse i18n translations instead of hardcoded English strings.
The escape room labels ("Escape Room Mode", "Time Limit (minutes)", "Hint Penalty (seconds)") are hardcoded while the rest of the component uses
t()fromuseTranslations(). This breaks localization for German users. The same pattern appears inPracticeQuizSettingsStep.tsx.🌐 Proposed fix
- <Checkbox - label="Escape Room Mode" + <Checkbox + label={t('manage.activityWizard.escapeRoomMode')} checked={!!values.isEscapeRoom} - <FormikNumberField - name="escapeRoomTimeLimit" - label="Time Limit (minutes)" + <FormikNumberField + name="escapeRoomTimeLimit" + label={t('manage.activityWizard.escapeRoomTimeLimit')} required - <FormikNumberField - name="escapeRoomHintPenalty" - label="Hint Penalty (seconds)" + <FormikNumberField + name="escapeRoomHintPenalty" + label={t('manage.activityWizard.escapeRoomHintPenalty')} required🤖 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 `@apps/frontend-manage/src/components/activities/creation/microLearning/MicroLearningSettingsStep.tsx` around lines 165 - 202, Replace the hardcoded escape-room labels in the relevant fields within MicroLearningSettingsStep, and apply the same change in PracticeQuizSettingsStep, by using the component’s existing useTranslations() hook and appropriate translation keys for “Escape Room Mode,” “Time Limit (minutes),” and “Hint Penalty (seconds).”apps/frontend-manage/src/components/activities/creation/microLearning/MicroLearningWizard.tsx-140-158 (1)
140-158: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUse i18n translations for validation messages.
The escape room validation messages ("Time limit is required", "Must be an integer", etc.) are hardcoded English strings. The same pattern appears in
PracticeQuizWizard.tsx. Uset()for consistency with the rest of the schema.🌐 Proposed fix
isEscapeRoom: yup.boolean(), escapeRoomTimeLimit: yup.number().when('isEscapeRoom', { is: true, then: (schema) => schema - .required('Time limit is required') - .integer('Must be an integer') - .positive('Must be a positive number of minutes'), + .required(t('manage.activityWizard.escapeRoomTimeLimitRequired')) + .integer(t('manage.activityWizard.mustBeInteger')) + .positive(t('manage.activityWizard.escapeRoomTimeLimitPositive')), otherwise: (schema) => schema.notRequired(), }), escapeRoomHintPenalty: yup.number().when('isEscapeRoom', { is: true, then: (schema) => schema - .required('Hint penalty is required') - .integer('Must be an integer') - .min(0, 'Must be a non-negative number of seconds'), + .required(t('manage.activityWizard.escapeRoomHintPenaltyRequired')) + .integer(t('manage.activityWizard.mustBeInteger')) + .min(0, t('manage.activityWizard.escapeRoomHintPenaltyNonNegative')), otherwise: (schema) => schema.notRequired(), }),🤖 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 `@apps/frontend-manage/src/components/activities/creation/microLearning/MicroLearningWizard.tsx` around lines 140 - 158, Replace the hardcoded escape-room validation messages in the schema fields within MicroLearningWizard with the existing i18n t() helper, using the appropriate translation keys; apply the same change to the corresponding validation schema in PracticeQuizWizard.apps/frontend-manage/src/components/activities/creation/groupActivity/GroupActivityWizard.tsx-247-249 (1)
247-249: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDefault
escapeRoomHintPenaltyof'0'mismatches the backend default of120seconds.The form defaults
escapeRoomHintPenaltyto'0'(no penalty), but the backend fallback inmanipulateGroupActivityis120seconds. Since the frontend always sends a concrete value whenisEscapeRoomis true, the backend's?? 120fallback is never reached. This means users get0seconds penalty by default instead of the intended120. If0is intentional (letting users opt-in to a penalty), consider aligning the backend fallback. If120is the intended default, update the form default to'120'.🔧 Proposed fix (if 120 seconds is the intended default)
isEscapeRoom: false, escapeRoomTimeLimit: '60', - escapeRoomHintPenalty: '0', + escapeRoomHintPenalty: '120', }🤖 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 `@apps/frontend-manage/src/components/activities/creation/groupActivity/GroupActivityWizard.tsx` around lines 247 - 249, Align the escape-room hint penalty defaults: update the initial form state in GroupActivityWizard, specifically escapeRoomHintPenalty, from '0' to '120' so it matches the backend default used by manipulateGroupActivity.apps/frontend-manage/src/components/activities/creation/groupActivity/GroupActivityWizard.tsx-149-167 (1)
149-167: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUse
t()for validation error messages instead of hardcoded English strings.The validation messages
'Time limit is required','Must be an integer','Must be a positive number of minutes','Hint penalty is required', and'Must be a non-negative number of seconds'are hardcoded. The rest of this file usest()for all validation messages. These should be localized for consistency.🌐 Proposed fix for i18n validation messages
escapeRoomTimeLimit: yup.number().when('isEscapeRoom', { is: true, then: (schema) => schema - .required('Time limit is required') - .integer('Must be an integer') - .positive('Must be a positive number of minutes'), + .required(t('manage.activityWizard.escapeRoomTimeLimitRequired')) + .integer(t('manage.activityWizard.mustBeInteger')) + .positive(t('manage.activityWizard.escapeRoomTimeLimitPositive')), otherwise: (schema) => schema.notRequired(), }), escapeRoomHintPenalty: yup.number().when('isEscapeRoom', { is: true, then: (schema) => schema - .required('Hint penalty is required') - .integer('Must be an integer') - .min(0, 'Must be a non-negative number of seconds'), + .required(t('manage.activityWizard.escapeRoomHintPenaltyRequired')) + .integer(t('manage.activityWizard.mustBeInteger')) + .min(0, t('manage.activityWizard.escapeRoomHintPenaltyNonNegative')), otherwise: (schema) => schema.notRequired(), }),🤖 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 `@apps/frontend-manage/src/components/activities/creation/groupActivity/GroupActivityWizard.tsx` around lines 149 - 167, Replace the hardcoded validation messages in the escapeRoomTimeLimit and escapeRoomHintPenalty Yup schemas within the GroupActivityWizard validation schema with localized t() calls, matching the existing translation key and interpolation conventions used elsewhere in the file.apps/frontend-manage/src/components/activities/creation/groupActivity/GroupActivitySettingsStep.tsx-153-190 (1)
153-190: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUse
t()for all user-facing labels instead of hardcoded English strings.The labels
"Escape Room Mode","Time Limit (minutes)", and"Hint Penalty (seconds)"are hardcoded. The rest of this component usest()fromuseTranslations()for all user-facing text. These should be localized to maintain consistency and support non-English locales.🌐 Proposed fix for i18n labels
<Checkbox - label="Escape Room Mode" + label={t('manage.activityWizard.escapeRoomMode')} checked={!!values.isEscapeRoom} onCheck={() => setFieldValue('isEscapeRoom', !values.isEscapeRoom) } className={{ indicator: 'text-xs', root: 'w-4.5 h-4.5', }} data={{ cy: 'toggle-escape-room' }} /> {values.isEscapeRoom && ( <> <FormikNumberField name="escapeRoomTimeLimit" - label="Time Limit (minutes)" + label={t('manage.activityWizard.escapeRoomTimeLimit')} required className={{ root: 'w-full', field: 'w-full', }} data={{ cy: 'escape-room-time-limit' }} /> <FormikNumberField name="escapeRoomHintPenalty" - label="Hint Penalty (seconds)" + label={t('manage.activityWizard.escapeRoomHintPenalty')} required className={{ root: 'w-full', field: 'w-full', }} data={{ cy: 'escape-room-hint-penalty' }} /> </> )}🤖 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 `@apps/frontend-manage/src/components/activities/creation/groupActivity/GroupActivitySettingsStep.tsx` around lines 153 - 190, Replace the hardcoded labels in the Escape Room section with localized strings from the component’s existing useTranslations() hook: update the Checkbox label and both FormikNumberField labels while preserving their current meaning and translation namespace.packages/graphql/src/services/groups.ts-1508-1531 (1)
1508-1531: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winEnforce
lockoutUntilbefore accepting another submission.Line 1699 sets a lockout, but this path only checks
IN_PROGRESS; an incorrect group can immediately submit again. Reject active attempts whoselockoutUntilis still in the future before persisting 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 `@packages/graphql/src/services/groups.ts` around lines 1508 - 1531, Before accepting or persisting another escape-room submission in the active-attempt validation, check `attempt.lockoutUntil` and reject when it is later than the current time. Add this guard alongside the existing `IN_PROGRESS` and expiration checks in the escape-room handling block, before response persistence, using the same GraphQLError pattern.packages/graphql/src/services/groups.ts-1617-1718 (1)
1617-1718: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftValidate the complete expected answer set.
allCorrectstarts astrueand only iterates supplied responses. An empty or partial request can therefore complete the escape room without answering every required question. Derive required instance IDs from the activity stack, validate ownership, and require each required answer before completing.🤖 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 `@packages/graphql/src/services/groups.ts` around lines 1617 - 1718, Update the escape-room validation around allCorrect and the response loop to derive every required element instance ID from the activity stack, verify those instances belong to the current group activity, and require a submitted response for each required instance. Treat empty or partial responses as incorrect, while preserving the existing per-type correctness evaluation before allowing decisionsSubmittedAt or COMPLETED status.apps/response-api/src/escapeRoom.ts-71-99 (1)
71-99: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDo not create attempts before verifying activity access and enrollment.
This response path creates an attempt from any valid participant token without the course-enrollment checks in
startEscapeRoomAttempt. Require the pre-authorized attempt, or perform the same enrollment and activity checks before creation.🤖 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 `@apps/response-api/src/escapeRoom.ts` around lines 71 - 99, The response path creates a new escape-room attempt before verifying course enrollment and activity access. In the attempt lookup/creation flow near getParticipantData, require an existing pre-authorized attempt, or reuse the enrollment and activity validation performed by startEscapeRoomAttempt before calling escapeRoomAttempt.create.apps/response-api/src/escapeRoom.ts-117-126 (1)
117-126: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winApply the configured latency grace consistently.
startEscapeRoomAttemptexpires attempts only aftertotalLimit + 5, but this endpoint rejects responses immediately aftertotalLimit. A submission accepted by the lifecycle/countdown contract can therefore be rejected here.🤖 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 `@apps/response-api/src/escapeRoom.ts` around lines 117 - 126, Update the expiration check in the relevant response-handling function to apply the same 5-second latency grace used by startEscapeRoomAttempt: compare elapsed time against totalLimit + 5 before marking the attempt EXPIRED and returning escape_room_expired. Keep the existing penalty calculation and expiration update behavior unchanged.apps/frontend-pwa/src/components/practiceQuiz/PracticeQuiz.tsx-119-157 (1)
119-157: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReset local navigation state when restarting an attempt.
onResetleavescurrentIxandprogressStateunchanged, while the synchronization effect only promotes entries toCorrect.onStartalso bypasseshandleStartQuiz. Restarting from a later stack can therefore close the overlay onto a missing stack or preserve stale completion state. Clear local progress/index on reset and wire overlay start throughhandleStartQuiz.Also applies to: 188-197
🤖 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 `@apps/frontend-pwa/src/components/practiceQuiz/PracticeQuiz.tsx` around lines 119 - 157, Reset local navigation state in onReset by clearing progressState and setting currentIx to the initial stack index, and ensure overlay onStart invokes handleStartQuiz rather than bypassing it. Update the corresponding restart flow near the additional referenced section so stale completion state and invalid stack selection cannot persist.packages/graphql/src/services/practiceQuizzes.ts-75-107 (1)
75-107: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDo not expose all escape-room stacks to non-participants.
Anonymous and temporary-participant requests bypass this branch and receive the unfiltered published quiz, including future stacks. Return no stacks unless the caller is an authorized preview user or a participant with a valid attempt.
🤖 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 `@packages/graphql/src/services/practiceQuizzes.ts` around lines 75 - 107, Update the escape-room filtering logic in the quiz stack resolver to deny anonymous and temporary-participant callers by default. In the branch around escapeRoomConfig and the attempt lookup, return no stacks unless the user is an authorized preview user or has a valid participant attempt; retain progress-based filtering for valid IN_PROGRESS attempts and prevent unfiltered published stacks from being returned.apps/response-api/src/escapeRoom.ts-128-187 (1)
128-187: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftReject or grade every supported escape-room element type.
Only SC, MC, KPRIM, numerical, and free-text responses are graded. Selection, case-study, QR-scan, content, and any other active-block types retain
pointsPercentage = 0, making those stages impossible to clear.🤖 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 `@apps/response-api/src/escapeRoom.ts` around lines 128 - 187, The grading logic in the response handler only handles SC, MC, KPRIM, NUMERICAL, and FREE_TEXT, leaving other supported active-block types at zero. Extend the type dispatch after parsing solutions to explicitly handle selection, case-study, QR-scan, content, and every other supported escape-room type using the appropriate grading or validation behavior; unsupported or malformed types must be rejected with the existing error response rather than silently graded as zero.packages/graphql/src/services/microLearning.ts-65-107 (1)
65-107: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winGate escape-room content for anonymous and temporary participants.
This branch filters stacks only for
PARTICIPANT. Published escape rooms requested without a user or with a temporary participant bypass it and receive every stack, defeating sequential content gating.🤖 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 `@packages/graphql/src/services/microLearning.ts` around lines 65 - 107, Update the access condition in the microLearning resolver to apply escape-room stack filtering for anonymous users and temporary participants, not only users with role PARTICIPANT. Reuse the existing attempt lookup and status logic in the escape-room branch, while preserving owner-specific behavior and ensuring users without a valid attempt cannot receive stacks.packages/graphql/src/services/practiceQuizzes.ts-897-1060 (1)
897-1060: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftRequire exactly one discriminated attempt target.
The API accepts multiple optional IDs.
startEscapeRoomAttemptvalidates only the first matching branch but persists every supplied relation, while reset uses precedence-based ternaries. Reject zero/multiple targets and resolve one typed target before authorization, lookup, and mutation. Extracting that resolver also addresses the reported cognitive-complexity failures.Also applies to: 1062-1161
🤖 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 `@packages/graphql/src/services/practiceQuizzes.ts` around lines 897 - 1060, The startEscapeRoomAttempt API must require exactly one non-null target ID instead of selecting the first matching branch and persisting all supplied relations. Add a resolver near StartEscapeRoomAttemptArgs that validates one-and-only-one of practiceQuizId, microLearningId, groupActivityId, or elementBlockId and returns a typed discriminated target; use it for authorization, activity lookup, attempt lookup, and creation. Apply the same target resolution to the reset logic around resetEscapeRoomAttempt, replacing precedence-based ternaries and reducing its cognitive complexity.Source: Linters/SAST tools
apps/response-api/src/escapeRoom.ts-57-240 (1)
57-240: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftSplit this validation pipeline into focused helpers.
Extract authentication/attempt resolution, expiry checks, grading, and result persistence. Sonar reports cognitive complexity 34 against the allowed 15, and the current monolith obscures the security-critical branch contracts.
🤖 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 `@apps/response-api/src/escapeRoom.ts` around lines 57 - 240, Refactor handleEscapeRoomValidation into focused helpers for participant authentication and attempt creation/lookup, expiry and lockout validation, response grading, and correct/incorrect result handling. Preserve each existing response status, Redis key behavior, event payload, and Prisma update contract; keep handleEscapeRoomValidation as the coordinator that invokes these helpers and returns immediately when a helper has handled the response. Use the existing symbols handleEscapeRoomValidation, getParticipantData, the grading functions, and the Redis/Prisma operations to make the security-critical branches explicit and reduce cognitive complexity below 15.Source: Linters/SAST tools
packages/graphql/src/services/practiceQuizzes.ts-1163-1202 (1)
1163-1202: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftReset the attempt and progress atomically.
The attempt is deleted before responses, outside a transaction, and deletion errors are suppressed. A partial failure can leave an old attempt with cleared answers—or a new attempt with stale correct answers that immediately unlock later stacks.
🤖 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 `@packages/graphql/src/services/practiceQuizzes.ts` around lines 1163 - 1202, Make the reset operation atomic in the relevant practice-quiz reset function: execute the EscapeRoomAttempt deletion and associated groupActivityInstance or questionResponse cleanup inside a single Prisma transaction, preserving the existing conditional filters. Remove the empty catch handlers so any deletion failure aborts and rolls back the entire transaction, preventing partial reset state.packages/graphql/src/services/microLearning.ts-411-418 (1)
411-418: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not suppress escape-room configuration deletion failures.
Catching every error can report a successful edit while leaving escape-room mode enabled. Use an idempotent deletion or suppress only the expected “not found” case.
Proposed fix
if (!isEscapeRoom && id) { - await prisma.escapeRoomConfig - .delete({ - where: { microLearningId: id }, - }) - .catch(() => {}) + await prisma.escapeRoomConfig.deleteMany({ + where: { microLearningId: id }, + }) }🤖 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 `@packages/graphql/src/services/microLearning.ts` around lines 411 - 418, Update the escape-room configuration cleanup in the micro-learning edit flow to avoid suppressing all deletion errors. Use an idempotent delete operation, or catch only the expected record-not-found error while allowing other failures to propagate; preserve the existing isEscapeRoom and id checks.packages/graphql/src/services/practiceQuizzes.ts-371-378 (1)
371-378: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not ignore configuration deletion failures.
An unexpected deletion failure is swallowed, so the mutation can leave escape-room mode enabled despite saving it as disabled.
Proposed fix
if (!isEscapeRoom && id) { - await prisma.escapeRoomConfig - .delete({ - where: { practiceQuizId: id }, - }) - .catch(() => {}) + await prisma.escapeRoomConfig.deleteMany({ + where: { practiceQuizId: id }, + }) }🤖 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 `@packages/graphql/src/services/practiceQuizzes.ts` around lines 371 - 378, In the isEscapeRoom configuration cleanup within the practice quiz mutation, do not swallow errors from prisma.escapeRoomConfig.delete. Remove the empty catch and allow the deletion failure to propagate so the mutation cannot report a disabled escape room while its configuration remains enabled.apps/response-api/src/index.ts-134-149 (1)
134-149: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winMisleading comment and unnecessary Redis overhead on every response.
Two concerns:
The comment "Synchronous Escape Room Lockout & Correctness Check" is misleading —
handleEscapeRoomValidationis async and the call usesawait.
redis.hgetall(${instanceKey}:info)runs on every response submission. When the key doesn't exist, Redis returns an empty object{}(truthy in JS), soif (instanceInfo)passes andhandleEscapeRoomValidationis called, only to immediately returnfalsewheninstanceInfo.isEscapeRoom !== 'true'. This adds a Redis round-trip to the hot path for all non-escape-room responses. Consider checking a lighter-weight signal first (e.g.,redis.hget(key, 'isEscapeRoom')) or gating the entire block behind a cheaper condition.Additionally, per the implementation review at
project/2026-07-10-pr-5143-escape-room-implementation-review.md(M8), this live-quiz escape path is unreachable dead code — no wizard configures live-quiz escape room mode, soinstanceInfo.isEscapeRoomwill never be'true'in production. Either finish the live-quiz authoring surface or remove this path for v1.🤖 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 `@apps/response-api/src/index.ts` around lines 134 - 149, The escape-room validation block has a misleading synchronous comment, performs an unnecessary Redis hgetall on every response, and is unreachable without live-quiz authoring support. Replace the comment with accurate async wording, then either remove the live-quiz path for v1 or implement the required authoring/configuration support; if retained, gate handleEscapeRoomValidation behind a lightweight isEscapeRoom check using the instance info key and preserve the await behavior.playwright/tests/Z-escape-room.spec.ts-65-102 (1)
65-102: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFragile timer selector and insufficient test coverage for core escape-room mechanics.
The
loginStudentcall at lines 67–70 passes a superfluous second argument, previously flagged by github-code-quality[bot].The timer assertion at lines 88–92 uses
page.locator('span:has-text(":")').first(), which is unscoped — any span containing a colon on the page matches. Scope it to the timer banner, e.g.,page.locator('[data-testid="escape-room-timer"] span')or a more specific selector.The test only covers a single-stack quiz. The core escape-room mechanic — sequential gating (stack N+1 locked until stack N correct) — is untested. Expand to ≥3 stacks and add cases for: wrong-answer lockout, preceding-stack rejection, and expiry overlay. This is also recommended in the implementation review §6
#9.🤖 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 `@playwright/tests/Z-escape-room.spec.ts` around lines 65 - 102, Remove the unnecessary second argument from loginStudent in the Student Solves Escape Room Practice Quiz test. Scope the timer assertion to the escape-room timer element, such as escape-room-timer, instead of an arbitrary colon-containing span. Expand coverage to at least three stacks, verifying wrong-answer lockout, rejection of attempts on later stacks before the preceding stack is solved, and the expiry overlay, while retaining the successful sequential completion flow.apps/frontend-pwa/src/components/practiceQuiz/EscapeRoomOverlay.tsx-42-102 (1)
42-102: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBlocking overlays lack dialog semantics and focus management.
The start, expired, and completed overlays (lines 43–179) use
fixed inset-0 z-[100]to cover the full page but have norole="dialog",aria-modal="true", or focus-trap. Background content remains tab-reachable, making these modals inaccessible to keyboard and screen-reader users.Add
role="dialog"andaria-modal="true"to the outer overlay divs, trap focus within the modal, and associate the<H3>title viaaria-labelledby.Additionally,
bg-slate-750(line 61) is not a standard Tailwind class — Tailwind's slate palette jumps from 700 to 800. Unless the project's Tailwind config extends this, the class has no effect. Verify or usebg-slate-700.This is also documented in the implementation review at
project/2026-07-10-pr-5143-escape-room-implementation-review.md§4 (A11y).🤖 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 `@apps/frontend-pwa/src/components/practiceQuiz/EscapeRoomOverlay.tsx` around lines 42 - 102, Add dialog semantics and keyboard focus trapping to the start, expired, and completed overlay containers in EscapeRoomOverlay: add role="dialog", aria-modal="true", and unique aria-labelledby references connected to each H3 title, using the project’s focus-trap pattern or an equivalent implementation so background content is not tab-reachable. Also replace bg-slate-750 with bg-slate-700 unless the Tailwind configuration explicitly defines the custom class.packages/graphql/src/services/pruneEscapeRooms.ts-85-122 (1)
85-122: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftFailed stats updates silently lose data — attempts are deleted regardless.
The
.catch()blocks at lines 99 and 117 log errors but don't prevent thedeleteManyat line 128 from deleting all attempts. If a statsupdateorcreatefails for any instance, the attempt is still pruned, permanently losing the statistics for that instance. Additionally, the entire operation is non-transactional: if the process crashes after some stats are updated but beforedeleteMany, the next run will re-process those attempts and double-count their statistics.Consider tracking which attempts had all stats successfully updated and only deleting those, or wrapping each attempt's stats-update + deletion in a Prisma transaction.
🔒 Proposed fix: only delete fully-processed attempts
for (const attempt of attempts) { + let allStatsSucceeded = true // Find element instances related to this attempt let instances: { id: number }[] = [] // ... existing instance-finding logic ... for (const instance of instances) { const stats = await globalContext.prisma.instanceStatistics.findUnique({ where: { elementInstanceId: instance.id }, }) if (stats) { // ... existing update logic ... - .catch((err) => { - executionContext.logger.error( - `Failed to update stats for instance ${instance.id}: ${err}` - ) - }) + .catch((err) => { + allStatsSucceeded = false + executionContext.logger.error( + `Failed to update stats for instance ${instance.id}: ${err}` + ) + }) } else { // ... existing create logic ... - .catch((err) => { - executionContext.logger.error( - `Failed to create stats for instance ${instance.id}: ${err}` - ) - }) + .catch((err) => { + allStatsSucceeded = false + executionContext.logger.error( + `Failed to create stats for instance ${instance.id}: ${err}` + ) + }) } } + attempt._statsSucceeded = allStatsSucceeded + } + + // Delete only fully-processed attempts + const successfulAttemptIds = attempts + .filter((a) => a._statsSucceeded) + .map((a) => a.id) + + if (successfulAttemptIds.length > 0) { + await globalContext.prisma.escapeRoomAttempt.deleteMany({ + where: { id: { in: successfulAttemptIds } }, + }) + executionContext.logger.info( + `Successfully pruned ${successfulAttemptIds.length} of ${attempts.length} attempts` + ) + if (successfulAttemptIds.length < attempts.length) { + executionContext.logger.warn( + `${attempts.length - successfulAttemptIds.length} attempts retained due to stats update failures` + ) + } } - // Delete pruned attempts - if (attempts.length > 0) { - await globalContext.prisma.escapeRoomAttempt.deleteMany({ - where: { - id: { in: attempts.map((a) => a.id) }, - }, - }) - executionContext.logger.info( - `Successfully pruned ${attempts.length} attempts` - ) - }🤖 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 `@packages/graphql/src/services/pruneEscapeRooms.ts` around lines 85 - 122, Stats persistence failures are swallowed while deleteMany still removes the related attempts, risking data loss and duplicate processing. Update the pruning logic in the surrounding stats-processing function to track attempts whose update/create operations completed successfully, propagate or record failures, and restrict deleteMany to only those fully processed attempts. Prefer wrapping each attempt’s stats write and deletion in a Prisma transaction to make processing atomic and prevent reprocessing after partial failures.
🟡 Minor comments (6)
packages/graphql/src/schema/practiceQuiz.ts-289-294 (1)
289-294: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winValidate
hintsUsedbefore returning it
packages/graphql/src/schema/practiceQuiz.ts:289-294
hintsUsedis stored as JSON, so a direct cast can pass through non-arrays or mixed values and break the[String!]!contract. Guard withArray.isArray(...)and filter to strings.🤖 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 `@packages/graphql/src/schema/practiceQuiz.ts` around lines 289 - 294, Update the hintsUsed field resolver to validate the JSON value with Array.isArray before returning it, and filter the array to retain only string elements; return an empty array for non-array values to preserve the [String!]! contract.packages/graphql/src/schema/groupActivity.ts-77-88 (1)
77-88: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse a deterministic group lookup here
ParticipantGroupis only unique bycourseId + code, and nothing in the membership flow prevents a participant from being in multiple groups in the same course.findFirstcan therefore pick an arbitrary group and return the wrong attempts. Resolve the group through the activity’s group instance or enforce single-group membership per course.🤖 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 `@packages/graphql/src/schema/groupActivity.ts` around lines 77 - 88, Replace the findFirst lookup in the participant-group attempt resolver with a deterministic lookup tied to the activity’s group instance, using the activity/group relationship and the current participant membership; alternatively enforce single-group membership per course and use that invariant consistently. Update the escapeRoomAttempt query to use the resolved group ID and preserve the empty result when no valid group exists.apps/frontend-pwa/src/pages/group/[groupId]/activity/[activityId].tsx-26-27 (1)
26-27: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse configured aliases for the new local imports.
Replace the relative traversals with the corresponding
@/~aliases. As per coding guidelines, “Use@and~path aliases for imports.”🤖 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 `@apps/frontend-pwa/src/pages/group/`[groupId]/activity/[activityId].tsx around lines 26 - 27, Replace the relative imports for useEscapeRoom and EscapeRoomOverlay with the repository’s configured @ or ~ path aliases, preserving the existing imported symbols.Source: Coding guidelines
apps/frontend-pwa/src/components/liveQuiz/QuestionArea.tsx-240-259 (1)
240-259: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd translations for the new escape-room toasts.
pwa.practiceQuiz.escapeRoomIncorrectToastandescapeRoomLockoutToastare not present inpackages/i18n/messages/de.tslines 801-868, so German users receive the English defaults.🤖 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 `@apps/frontend-pwa/src/components/liveQuiz/QuestionArea.tsx` around lines 240 - 259, Add German translations for the keys pwa.practiceQuiz.escapeRoomIncorrectToast and pwa.practiceQuiz.escapeRoomLockoutToast in the practiceQuiz translations within de.ts, preserving the existing message structure so showStatusCodeToast displays localized text instead of falling back to English.apps/frontend-pwa/src/components/hooks/useEscapeRoom.ts-77-84 (1)
77-84: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick winStop polling after local expiry.
If the server still returns
IN_PROGRESS, this callsrefetch()every second after the timer reaches zero. Refetch once on the transition to zero, then clear the interval or guard with a ref.🤖 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 `@apps/frontend-pwa/src/components/hooks/useEscapeRoom.ts` around lines 77 - 84, In the useEscapeRoom timer logic, prevent repeated refetching after local expiry by tracking whether the countdown has already reached zero. In calculateRemaining, trigger refetch only on the transition to zero, then clear the interval or use a ref guard so subsequent ticks do nothing; ensure the interval cleanup remains intact.apps/frontend-pwa/src/components/practiceQuiz/EscapeRoomOverlay.tsx-181-199 (1)
181-199: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAdd
aria-liveto the countdown timer for screen reader accessibility.The countdown
<span>(line 193) has noaria-liveattribute. Screen readers won't announce time updates, which is critical for a time-limited escape room. Addaria-live="polite"to the timer container so screen readers announce remaining time at appropriate intervals.Also,
remainingSeconds === nullrenders'00:00', which is ambiguous — it could mean "expired" or "not yet initialized." Consider rendering the initialtimeLimitformatted value instead.🤖 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 `@apps/frontend-pwa/src/components/practiceQuiz/EscapeRoomOverlay.tsx` around lines 181 - 199, Add aria-live="polite" to the countdown timer container in EscapeRoomOverlay’s active in-progress render so screen readers announce updates. Replace the null fallback in the remainingSeconds display with the formatted initial timeLimit value, avoiding an ambiguous 00:00 before initialization.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8247ced6-d4c1-464d-b393-6964d063d7be
⛔ Files ignored due to path filters (22)
packages/graphql/src/graphql/ops/FMicroLearningData.graphqlis excluded by!**/**/graphql/ops/**packages/graphql/src/graphql/ops/FMicroLearningDataWithoutSolutions.graphqlis excluded by!**/**/graphql/ops/**packages/graphql/src/graphql/ops/FPracticeQuizData.graphqlis excluded by!**/**/graphql/ops/**packages/graphql/src/graphql/ops/FPracticeQuizDataWithoutSolutions.graphqlis excluded by!**/**/graphql/ops/**packages/graphql/src/graphql/ops/MCreateGroupActivity.graphqlis excluded by!**/**/graphql/ops/**packages/graphql/src/graphql/ops/MCreateMicroLearning.graphqlis excluded by!**/**/graphql/ops/**packages/graphql/src/graphql/ops/MCreatePracticeQuiz.graphqlis excluded by!**/**/graphql/ops/**packages/graphql/src/graphql/ops/MEditGroupActivity.graphqlis excluded by!**/**/graphql/ops/**packages/graphql/src/graphql/ops/MEditMicroLearning.graphqlis excluded by!**/**/graphql/ops/**packages/graphql/src/graphql/ops/MEditPracticeQuiz.graphqlis excluded by!**/**/graphql/ops/**packages/graphql/src/graphql/ops/MResetEscapeRoomAttempt.graphqlis excluded by!**/**/graphql/ops/**packages/graphql/src/graphql/ops/MStartEscapeRoomAttempt.graphqlis excluded by!**/**/graphql/ops/**packages/graphql/src/graphql/ops/QGetActivityTemplate.graphqlis excluded by!**/**/graphql/ops/**packages/graphql/src/graphql/ops/QGetCourseGroupActivities.graphqlis excluded by!**/**/graphql/ops/**packages/graphql/src/graphql/ops/QGetGroupActivity.graphqlis excluded by!**/**/graphql/ops/**packages/graphql/src/graphql/ops/QGroupActivityDetails.graphqlis excluded by!**/**/graphql/ops/**packages/graphql/src/ops.schema.jsonis excluded by!**/**/ops.schema.jsonpackages/graphql/src/ops.tsis excluded by!**/**/ops.tspackages/graphql/src/public/client.jsonis excluded by!**/**/public/**packages/graphql/src/public/schema.graphqlis excluded by!**/**/public/**packages/graphql/src/public/server.jsonis excluded by!**/**/public/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (56)
apps/analytics/prisma/schema/element.prismaapps/analytics/prisma/schema/participant.prismaapps/analytics/prisma/schema/quiz.prismaapps/frontend-manage/src/components/activities/ActivityCreation.tsxapps/frontend-manage/src/components/activities/creation/WizardLayout.tsxapps/frontend-manage/src/components/activities/creation/groupActivity/GroupActivitySettingsStep.tsxapps/frontend-manage/src/components/activities/creation/groupActivity/GroupActivityWizard.tsxapps/frontend-manage/src/components/activities/creation/groupActivity/submitGroupActivityForm.tsapps/frontend-manage/src/components/activities/creation/liveQuiz/LiveQuizWizard.tsxapps/frontend-manage/src/components/activities/creation/microLearning/MicroLearningSettingsStep.tsxapps/frontend-manage/src/components/activities/creation/microLearning/MicroLearningWizard.tsxapps/frontend-manage/src/components/activities/creation/microLearning/submitMicrolearningForm.tsapps/frontend-manage/src/components/activities/creation/practiceQuiz/PracticeQuizSettingsStep.tsxapps/frontend-manage/src/components/activities/creation/practiceQuiz/PracticeQuizWizard.tsxapps/frontend-manage/src/components/activities/creation/practiceQuiz/submitPracticeQuizForm.tsapps/frontend-manage/src/pages/index.tsxapps/frontend-pwa/src/components/hooks/useEscapeRoom.tsapps/frontend-pwa/src/components/liveQuiz/QuestionArea.tsxapps/frontend-pwa/src/components/practiceQuiz/EscapeRoomOverlay.tsxapps/frontend-pwa/src/components/practiceQuiz/PracticeQuiz.tsxapps/frontend-pwa/src/pages/course/[courseId]/microLearnings/[id]/[ix].tsxapps/frontend-pwa/src/pages/course/[courseId]/practiceQuizzes/[id].tsxapps/frontend-pwa/src/pages/group/[groupId]/activity/[activityId].tsxapps/response-api/package.jsonapps/response-api/src/escapeRoom.tsapps/response-api/src/index.tsdocker-compose.ymldocs/domain-model.mddocs/log.mddocs/testing.mdpackages/graphql/src/index.tspackages/graphql/src/schema/escapeRoomConfig.tspackages/graphql/src/schema/groupActivity.tspackages/graphql/src/schema/liveQuiz.tspackages/graphql/src/schema/microLearning.tspackages/graphql/src/schema/mutation.tspackages/graphql/src/schema/practiceQuiz.tspackages/graphql/src/services/courses.tspackages/graphql/src/services/groups.tspackages/graphql/src/services/liveQuizzes.tspackages/graphql/src/services/microLearning.tspackages/graphql/src/services/practiceQuizzes.tspackages/graphql/src/services/pruneEscapeRooms.tspackages/graphql/src/services/stacks.tspackages/graphql/test/helpers.tspackages/hatchet/src/index.tspackages/i18n/messages/de.tspackages/i18n/messages/en.tspackages/prisma/src/prisma/schema/element.prismapackages/prisma/src/prisma/schema/migrations/20260707103049_add_escape_room_mode/migration.sqlpackages/prisma/src/prisma/schema/participant.prismapackages/prisma/src/prisma/schema/quiz.prismapackages/types/src/hatchet.tsplaywright/tests/Z-escape-room.spec.tsproject/2026-07-07-pr-5143-escape-room-quiz-mode-plan.mdproject/2026-07-10-pr-5143-escape-room-implementation-review.md
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/graphql/src/services/pruneEscapeRooms.ts (1)
10-173: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftSplit this workflow to satisfy the complexity gate.
SonarCloud reports cognitive complexity 43 versus the allowed 15. Extract instance resolution, metric calculation, per-attempt aggregation, and housekeeping into focused functions.
🤖 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 `@packages/graphql/src/services/pruneEscapeRooms.ts` around lines 10 - 173, Reduce the cognitive complexity of handlePruneEscapeRooms by extracting focused helpers for resolving instances, calculating attempt metrics, aggregating statistics for one attempt, and pruning stale attempts. Keep handlePruneEscapeRooms as orchestration: fetch pending attempts, invoke the per-attempt aggregation helper, perform housekeeping, log results, and handle top-level errors; preserve existing logging, error handling, and database behavior.Source: Linters/SAST tools
packages/graphql/src/services/practiceQuizzes.ts (1)
1138-1167: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPass the reset target into the lecturer reset path.
packages/graphql/src/services/practiceQuizzes.ts:1145-1167The caller only sends the activity id, soparticipantId/groupIdare still undefined here. That makes the selector fall through to a branch withparticipantId!/elementBlockId!undefined, the delete gets swallowed, and the mutation returnstruewithout clearing anything. Require the target attempt identifier before buildingattemptWhere.🤖 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 `@packages/graphql/src/services/practiceQuizzes.ts` around lines 1138 - 1167, The lecturer reset path builds an invalid selector because reset target identifiers are not passed through from the caller. Update the relevant reset mutation/callers and the lecturer reset logic around attemptWhere to require and forward the target participantId/groupId (and associated activity identifier), then validate the target before constructing attemptWhere so it cannot fall through with undefined values; preserve the existing GraphQLError behavior for missing or invalid targets.
🧹 Nitpick comments (1)
packages/graphql/src/services/practiceQuizzes.ts (1)
20-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a configured path alias for
checkAccess.Replace
./sharing.jswith the repository’s corresponding@or~alias.As per coding guidelines,
**/*.{ts,tsx,js,jsx,mts,mjs,cjs}imports must use@and~path aliases.🤖 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 `@packages/graphql/src/services/practiceQuizzes.ts` at line 20, Update the checkAccess import in practiceQuizzes.ts to use the repository’s configured @ or ~ path alias instead of the relative './sharing.js' path, preserving the existing symbol and extension conventions.Source: Coding guidelines
🤖 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 `@packages/graphql/src/services/pruneEscapeRooms.ts`:
- Around line 151-162: Update the cleanup query in the prune logic to anchor
retention to when the attempt finished or was aggregated, not started. In the
deleteMany filter, replace the startedAt cutoff condition with a cutoff on
statsAggregatedAt (or the explicit completion/expiry timestamp if available),
while retaining the completed/expired statuses and non-null aggregation
requirement.
- Around line 67-125: The statistics update in the escape-room pruning logic
incorrectly applies activity-wide timeSpent, isSuccess, and triesCount to every
instance. Refactor the logic around these calculations and the
instanceStatistics update/create operations to use metrics specific to each
instance, deriving submission or retry counts from per-instance attempt data
rather than hintsUsed and penaltySeconds; do not persist the quiz’s total
duration or final status for every element.
- Around line 22-29: Update the processing logic in the escape-room pruning
service to handle each attempt within a Prisma transaction: perform all
instanceStatistics aggregation writes and set statsAggregatedAt only after every
write succeeds, allowing failures to roll back without marking the attempt
complete. Ensure the find-and-claim or processing flow prevents concurrent jobs
from handling the same unclaimed attempt, using the relevant escapeRoomAttempt
and transaction methods.
---
Outside diff comments:
In `@packages/graphql/src/services/practiceQuizzes.ts`:
- Around line 1138-1167: The lecturer reset path builds an invalid selector
because reset target identifiers are not passed through from the caller. Update
the relevant reset mutation/callers and the lecturer reset logic around
attemptWhere to require and forward the target participantId/groupId (and
associated activity identifier), then validate the target before constructing
attemptWhere so it cannot fall through with undefined values; preserve the
existing GraphQLError behavior for missing or invalid targets.
In `@packages/graphql/src/services/pruneEscapeRooms.ts`:
- Around line 10-173: Reduce the cognitive complexity of handlePruneEscapeRooms
by extracting focused helpers for resolving instances, calculating attempt
metrics, aggregating statistics for one attempt, and pruning stale attempts.
Keep handlePruneEscapeRooms as orchestration: fetch pending attempts, invoke the
per-attempt aggregation helper, perform housekeeping, log results, and handle
top-level errors; preserve existing logging, error handling, and database
behavior.
---
Nitpick comments:
In `@packages/graphql/src/services/practiceQuizzes.ts`:
- Line 20: Update the checkAccess import in practiceQuizzes.ts to use the
repository’s configured @ or ~ path alias instead of the relative './sharing.js'
path, preserving the existing symbol and extension conventions.
🪄 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: CHILL
Plan: Pro
Run ID: 8e7f781b-7600-4d6a-8e65-f2c60ddc695a
📒 Files selected for processing (9)
apps/frontend-manage/src/components/activities/ActivityCreation.tsxpackages/graphql/src/services/groups.tspackages/graphql/src/services/microLearning.tspackages/graphql/src/services/practiceQuizzes.tspackages/graphql/src/services/pruneEscapeRooms.tspackages/graphql/src/services/stacks.tspackages/prisma/src/prisma/schema/migrations/20260707103049_add_escape_room_mode/migration.sqlpackages/prisma/src/prisma/schema/quiz.prismaplaywright/tests/Z-escape-room.spec.ts
💤 Files with no reviewable changes (1)
- apps/frontend-manage/src/components/activities/ActivityCreation.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- playwright/tests/Z-escape-room.spec.ts
- packages/prisma/src/prisma/schema/migrations/20260707103049_add_escape_room_mode/migration.sql
- packages/graphql/src/services/stacks.ts
- packages/prisma/src/prisma/schema/quiz.prisma
- packages/graphql/src/services/microLearning.ts
- packages/graphql/src/services/groups.ts
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
apps/frontend-manage/src/components/activities/creation/EscapeRoomSettingsFields.tsx (1)
4-12: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMark props as read-only (SonarCloud).
Interface fields aren't mutated but aren't marked readonly either.
♻️ Proposed fix
-interface EscapeRoomSettingsFieldsProps { - isEscapeRoom: boolean - onToggle: (nextEnabled: boolean) => void -} +interface EscapeRoomSettingsFieldsProps { + readonly isEscapeRoom: boolean + readonly onToggle: (nextEnabled: boolean) => void +}🤖 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 `@apps/frontend-manage/src/components/activities/creation/EscapeRoomSettingsFields.tsx` around lines 4 - 12, Mark the EscapeRoomSettingsFieldsProps interface properties isEscapeRoom and onToggle as readonly, without changing the component’s behavior or callback signature.Source: Linters/SAST tools
packages/graphql/src/schema/practiceQuiz.ts (2)
195-199: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid
anycasts; the Prisma include already gives a typed shape.♻️ Suggested fix
- if (elements.length === 0) return false - return (elements as any[]).every((elem) => - elem.responses?.some( - (resp: any) => - resp.lastResponseCorrectness === DB.ResponseCorrectness.CORRECT - ) - ) + if (elements.length === 0) return false + return elements.every((elem) => + elem.responses.some( + (resp) => resp.lastResponseCorrectness === DB.ResponseCorrectness.CORRECT + ) + )🤖 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 `@packages/graphql/src/schema/practiceQuiz.ts` around lines 195 - 199, In the response-checking logic, remove the `any` casts from `elements` and `resp`; use the Prisma-inferred type provided by the existing include and access `elem.responses` and `resp.lastResponseCorrectness` directly, preserving the current `every`/`some` behavior.
178-201: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftPotential N+1 if
isCorrectis queried across a list of stacks.Each invocation issues its own
elementInstance.findManyround-trip. If the escape-room UI requestsisCorrectfor everyElementStackin a list (e.g. an overview/map view), this becomes an N+1 query pattern, especially under periodic polling.🤖 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 `@packages/graphql/src/schema/practiceQuiz.ts` around lines 178 - 201, The isCorrect resolver performs a separate database query for every ElementStack, creating an N+1 pattern in list queries. Refactor the practice quiz resolver around isCorrect to batch element-instance lookups per request, using a DataLoader or equivalent ctx-scoped cache keyed by elementStackId and participantId, while preserving the existing empty-stack and correctness behavior.packages/graphql/src/schema/query.ts (1)
628-634: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract nested ternary in permission selector.
SonarCloud flags this nested ternary for readability. Extract into a small named function or if/else chain.
♻️ Suggested refactor
- resolve: withPermission( - (args) => - args.practiceQuizId - ? { practiceQuizId: args.practiceQuizId } - : args.microLearningId - ? { microLearningId: args.microLearningId } - : { groupActivityId: args.groupActivityId }, - DB.PermissionLevel.READ, + resolve: withPermission( + (args) => { + if (args.practiceQuizId) return { practiceQuizId: args.practiceQuizId } + if (args.microLearningId) return { microLearningId: args.microLearningId } + return { groupActivityId: args.groupActivityId } + }, + DB.PermissionLevel.READ,🤖 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 `@packages/graphql/src/schema/query.ts` around lines 628 - 634, Replace the nested ternary in the permission selector callback with a small named helper function or clear if/else chain that selects practiceQuizId, microLearningId, or groupActivityId, then pass the helper’s result to the existing permission lookup.Source: Linters/SAST tools
packages/graphql/src/services/escapeRooms.ts (1)
99-111: 🚀 Performance & Scalability | 🔵 TrivialNo pagination on the lecturer progress query.
escapeRoomAttempt.findManyreturns every attempt for the activity in a single response with notake/cursor. Fine for typical class sizes, but worth keeping in mind if this dashboard is ever used for very large cohorts.🤖 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 `@packages/graphql/src/services/escapeRooms.ts` around lines 99 - 111, Add pagination to the lecturer progress query in the escapeRoomAttempt.findMany call, using an appropriate take/skip or cursor-based scheme and exposing the necessary pagination inputs and metadata through the surrounding resolver/service. Preserve the existing filters, includes, and ordering while ensuring large cohorts are not returned in a single response.
🤖 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
`@apps/frontend-manage/src/components/activities/creation/groupActivity/GroupActivitySettingsStep.tsx`:
- Line 15: Replace the relative EscapeRoomSettingsFields import in
GroupActivitySettingsStep with the configured @ or ~ path alias, matching the
project’s existing alias conventions.
In
`@apps/frontend-manage/src/components/activities/creation/microLearning/MicroLearningSettingsStep.tsx`:
- Line 17: Replace the relative import of EscapeRoomSettingsFields in
MicroLearningSettingsStep.tsx with the configured @ or ~ path alias, matching
the project’s existing alias conventions.
In
`@apps/frontend-manage/src/components/activities/creation/practiceQuiz/PracticeQuizSettingsStep.tsx`:
- Line 16: Replace the relative EscapeRoomSettingsFields import in
PracticeQuizSettingsStep with the configured @ or ~ path alias, preserving the
existing module reference and behavior.
In `@packages/graphql/src/services/escapeRooms.ts`:
- Around line 56-205: Reduce the cognitive complexity of getEscapeRoomProgress
by extracting the cleared-stack calculation (including response loading and
participant/group fallback logic) and the per-attempt progress mapping into
standalone helpers. Move the nested ternary for clearedStacks into explicit
conditional logic within the mapping helper, then have getEscapeRoomProgress
orchestrate these helpers while preserving the existing behavior and returned
fields.
---
Nitpick comments:
In
`@apps/frontend-manage/src/components/activities/creation/EscapeRoomSettingsFields.tsx`:
- Around line 4-12: Mark the EscapeRoomSettingsFieldsProps interface properties
isEscapeRoom and onToggle as readonly, without changing the component’s behavior
or callback signature.
In `@packages/graphql/src/schema/practiceQuiz.ts`:
- Around line 195-199: In the response-checking logic, remove the `any` casts
from `elements` and `resp`; use the Prisma-inferred type provided by the
existing include and access `elem.responses` and `resp.lastResponseCorrectness`
directly, preserving the current `every`/`some` behavior.
- Around line 178-201: The isCorrect resolver performs a separate database query
for every ElementStack, creating an N+1 pattern in list queries. Refactor the
practice quiz resolver around isCorrect to batch element-instance lookups per
request, using a DataLoader or equivalent ctx-scoped cache keyed by
elementStackId and participantId, while preserving the existing empty-stack and
correctness behavior.
In `@packages/graphql/src/schema/query.ts`:
- Around line 628-634: Replace the nested ternary in the permission selector
callback with a small named helper function or clear if/else chain that selects
practiceQuizId, microLearningId, or groupActivityId, then pass the helper’s
result to the existing permission lookup.
In `@packages/graphql/src/services/escapeRooms.ts`:
- Around line 99-111: Add pagination to the lecturer progress query in the
escapeRoomAttempt.findMany call, using an appropriate take/skip or cursor-based
scheme and exposing the necessary pagination inputs and metadata through the
surrounding resolver/service. Preserve the existing filters, includes, and
ordering while ensuring large cohorts are not returned in a single response.
🪄 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: CHILL
Plan: Pro
Run ID: fb39bda4-f85f-42e3-92d1-824e0a97446e
⛔ Files ignored due to path filters (6)
packages/graphql/src/graphql/ops/QGetEscapeRoomProgress.graphqlis excluded by!**/**/graphql/ops/**packages/graphql/src/ops.schema.jsonis excluded by!**/**/ops.schema.jsonpackages/graphql/src/ops.tsis excluded by!**/**/ops.tspackages/graphql/src/public/client.jsonis excluded by!**/**/public/**packages/graphql/src/public/schema.graphqlis excluded by!**/**/public/**packages/graphql/src/public/server.jsonis excluded by!**/**/public/**
📒 Files selected for processing (17)
apps/analytics/prisma/schema/quiz.prismaapps/frontend-manage/src/components/activities/creation/EscapeRoomSettingsFields.tsxapps/frontend-manage/src/components/activities/creation/escapeRoomValidation.tsapps/frontend-manage/src/components/activities/creation/groupActivity/GroupActivitySettingsStep.tsxapps/frontend-manage/src/components/activities/creation/groupActivity/GroupActivityWizard.tsxapps/frontend-manage/src/components/activities/creation/microLearning/MicroLearningSettingsStep.tsxapps/frontend-manage/src/components/activities/creation/microLearning/MicroLearningWizard.tsxapps/frontend-manage/src/components/activities/creation/practiceQuiz/PracticeQuizSettingsStep.tsxapps/frontend-manage/src/components/activities/creation/practiceQuiz/PracticeQuizWizard.tsxpackages/graphql/src/schema/practiceQuiz.tspackages/graphql/src/schema/query.tspackages/graphql/src/services/escapeRooms.tspackages/graphql/test/escapeRoom.test.tspackages/graphql/test/helpers.tspackages/i18n/messages/de.tspackages/i18n/messages/en.tsproject/2026-07-10-pr-5143-escape-room-implementation-review.md
✅ Files skipped from review due to trivial changes (1)
- packages/i18n/messages/en.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/frontend-manage/src/components/activities/creation/groupActivity/GroupActivityWizard.tsx
- apps/frontend-manage/src/components/activities/creation/practiceQuiz/PracticeQuizWizard.tsx
- apps/analytics/prisma/schema/quiz.prisma
- apps/frontend-manage/src/components/activities/creation/microLearning/MicroLearningWizard.tsx
… test login sigs, stray comment)
…f-reset, prune retention, group lockout)
…nd validation bounds - Extract EscapeRoomSettingsFields shared component (Checkbox + time limit + hint penalty) used by practice quiz, microlearning, group activity settings steps (M6) - Extract useEscapeRoomYupFields hook with i18n-backed validation bounds: time limit integer/positive/max 1440min, hint penalty integer/min 0/max 3600s (M4) - Force sequential order and disable the order selector when escape mode is enabled in practice quiz (M5) - Add 10 escape room i18n keys to en + de (labels, tooltips, validation messages) - Sync statsAggregatedAt to apps/analytics prisma schema (prisma:sync drift from B4)
…urer dashboard - New escapeRooms.ts service: getEscapeRoomProgress aggregates all attempts on an escape-room activity (practice quiz / microlearning / group activity) into per-participant progress (cleared/total stacks, status, time spent, penalty, hints used, lockout) for the lecturer view. - Per-stack cleared count reuses the getPracticeQuizData masking logic (leading fully-correct stacks); group attempts fall back to a status-derived estimate. - EscapeRoomProgress / EscapeRoomAttemptProgress Pothos types + query registered with withPermission READ owner authorization; QGetEscapeRoomProgress op + codegen. - elementBlock (live-quiz) path kept in the service for later; not exposed in the query until the live-quiz escape authoring surface (M8) lands.
…ng/lockout/expiry/completion) - New packages/graphql/test/escapeRoom.test.ts (11 tests): B1 anon/non-participant grading guard + owner preview bypass, sequential stack gating, lockout window enforcement, attempt expiry, B2 lecturer-only reset (lecturer/participant/no-write access), completion fires exactly once, B4 prune retention window. - New seedEscapeRoomPracticeQuiz helper (real elementData via processElementData so the SC grading path resolves CORRECT/INCORRECT instead of always-incorrect). - Live-DB integration tests (CI-verified); throw messages traced verbatim from source, Participation + startEscapeRoomAttempt preconditions checked against source. - Update review doc progress log.
…ction # Conflicts: # docs/log.md # packages/graphql/src/ops.ts # packages/graphql/src/public/client.json # packages/graphql/src/public/server.json # packages/graphql/src/services/liveQuizzes.ts
# Conflicts: # apps/frontend-pwa/package.json # apps/hatchet-worker-general/package.json # apps/hatchet-worker-response-processor/package.json # apps/response-api/package.json # docs/log.md # packages/graphql/src/services/groups.ts # packages/graphql/test/helpers.ts # pnpm-lock.yaml
rschlaefli
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Summary
This PR ships Escape Room mode across Practice Quizzes, Microlearnings, Group Activities, and Live Quiz blocks. Participants clear stages in order under a server-controlled timer, can request hints for a time penalty, and receive an enforced lockout after wrong answers. Lecturers can author the mode, monitor progress, and reset attempts.
It also adds the QR Scan element, including owner-only code access, printable decoys, camera scanning, and a validated manual fallback.
The full execution and review record is in the production roadmap.
What changed
v3baseline without changing Escape Room behavior.Security and data integrity
Branch coverage
v3atc8de9c8974be19aa61v3v3delta exactly. Its upstream and merge patches share stable patch-id11faeca2f97673cd2bc0d6f825b12e430b71b802, and it touched no Escape Room behavior path.Review focus
Verification
Current head:
pnpm run check:allin the exact Node 24 DevPod: 24/24 tasks pass, including Prisma parity, formatting, lint, syncpack, and agent-doc checksgit diff --check: pass4be19aa61: 49/49 pass, including all eight Playwright shardsEarlier branch verification, still applicable because the final base sync and plan commits touched no Escape Room behavior path:
v3: 0 findingsFailed or warning:
tsc --noEmitprobe for@klicker-uzh/prisma-datastill reports historical errors undersrc/scripts/*. The package exposes no configured check for that probe, none of the synchronized seed files appear in the errors, and the repository's configured typecheck passes.Not run:
The earlier Playwright run used the exact
codex-escape-room-productionDevPod and namespaced devrouter routes. It covers Practice Quiz, Microlearning, QR fallback, two concurrent Group Activity participants with lecturer monitoring and reset, and Live Quiz participant, cockpit, reset, and reload behavior.Runtime evidence
Known limitation
Camera denial and the complete manual-entry fallback are browser-verified. Camera acquisition, cleanup, validation, and grading have automated coverage, but the final local run did not use a physical camera.
Blocking before merge
Follow-up after merge
None.