enhance(apps/frontend-manage): display basic metadata on evaluation screen if embedded/not active#4920
enhance(apps/frontend-manage): display basic metadata on evaluation screen if embedded/not active#4920rschlaefli wants to merge 7 commits intov3from
Conversation
📝 WalkthroughWalkthroughAdds evaluation status and courseName to the GraphQL evaluation model and service, surfaces them through the frontend evaluation flow, introduces a BlockStatusIndicator, extends Activity/Element evaluation components and EvaluationUnavailableNotification with contextual props, tracks lastRefetchTime, updates i18n, and adds an assessment config flag. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Manager as Manager
participant Page as quizzes/[id]/evaluation page
participant API as GraphQL API
participant CompA as ActivityEvaluation
participant CompE as ElementEvaluation
participant UI1 as EvaluationUnavailableNotification
participant UI2 as BlockStatusIndicator
Manager->>Page: Open evaluation
Page->>API: getLiveQuizEvaluation(id)
API-->>Page: { displayName, status, courseName, stacks, blocks, ... }
Note over Page: useEffect updates lastRefetchTime on data change
alt loading or missing data
Page->>Page: Show Loader
else empty evaluation
Page->>UI1: Render with { activityId, activityName, activityStatus, courseName }
else data available
Page->>CompA: Render with { courseName, activityStatus, lastRefetchTime, ... }
alt no active instance
CompA->>UI1: Render with { courseName, activityName, activityId, activityStatus }
else instance present
CompA->>CompE: Render element with { lastRefetchTime, courseId, courseName, activityName, activityId, activityStatus }
alt LiveQuiz block with status
CompE->>UI2: Show status indicator (Scheduled/Active/Executed)
opt Scheduled
CompE->>UI1: Render unavailable notice with element/activity/course context
end
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
rschlaefli
changed the title
rschlaefli
changed the title
…ity-info-on-eval # Conflicts: # apps/frontend-manage/src/components/evaluation/ActivityEvaluation.tsx # apps/frontend-manage/src/pages/quizzes/[id]/evaluation.tsx # packages/graphql/src/ops.ts # packages/graphql/src/public/client.json # packages/graphql/src/public/server.json # packages/graphql/src/services/liveQuizzes.ts
…ity-info-on-eval
…ity-info-on-eval
dosubot
bot
added
size:L
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (12)
packages/graphql/src/schema/evaluation.ts (1)
16-16: Reusing PublicationStatus enum is fine; watch for duplicate registrationsImporting PublicationStatus from practiceQuiz is OK with a shared builder. If multiple modules later attempt to define the same enum, centralize it in a dedicated module to avoid accidental re‑declarations.
packages/graphql/src/services/liveQuizzes.ts (1)
2221-2221: Selecting course.name is necessary for new field; consider selecting id tooSelecting language and name is correct for courseName. If clients also link to course details (as UI hints suggest), consider selecting course.id here to avoid future query changes.
- course: { select: { language: true, name: true } }, + course: { select: { id: true, language: true, name: true } },packages/i18n/messages/de.ts (2)
671-673: Fix typo: “Aktiväten” → “Aktivitäten”.User‑facing text; low‑risk quick win.
- Grundsätzlich ist eine anonyme Teilnahme an allen Aktiväten in KlickerUZH ausser Gruppenaktivitäten möglich. Für Live‑Quizzes finden Sie die laufenden Quizzes eines Accounts unter + Grundsätzlich ist eine anonyme Teilnahme an allen Aktivitäten in KlickerUZH ausser Gruppenaktivitäten möglich. Für Live‑Quizzes finden Sie die laufenden Quizzes eines Accounts unter
2450-2463: Use i18n placeholders for dates; remove manual concatenation in the UI
- Add a {date} interpolation to instanceLastRefetch and instanceExecutionDate in packages/i18n/messages/en.ts and packages/i18n/messages/de.ts (e.g. "Last data refresh: {date}" / "Letztes Daten-Update: {date}", "Element executed at {date}" / "Element durchgeführt am {date}").
- Update apps/frontend-manage/src/components/evaluation/BlockStatusIndicator.tsx (around lines 41–56) to call t('manage.evaluation.instanceLastRefetch', { date: lastRefetchTime.toLocaleString() }) and t('manage.evaluation.instanceExecutionDate', { date: new Date(expiresAt).toLocaleString() }) and remove the manual ": " + date concatenation.
apps/frontend-manage/src/components/evaluation/ElementEvaluation.tsx (2)
47-53: Unused prop: courseId.The prop is accepted but never used in ElementEvaluation. Either use it (e.g., for links) or drop it from props and callers to avoid drift.
158-167: Overlay indicator layering and hit‑testing.To ensure the status badge isn’t occluded and doesn’t intercept clicks, consider adding z-index and disabling pointer events on the container.
- <div className="absolute bottom-4 left-4"> + <div className="absolute bottom-4 left-4 z-10 pointer-events-none"> <BlockStatusIndicator status={currentStack.status} lastRefetchTime={lastRefetchTime} expiresAt={currentStack.expiresAt} /> </div>apps/frontend-manage/src/components/evaluation/BlockStatusIndicator.tsx (2)
39-43: Locale-aware date/time formatting.Use next-intl’s formatter (or a centralized util) instead of toLocaleString for consistent i18n and timezone handling.
-import { useTranslations } from 'next-intl' +import { useFormatter, useTranslations } from 'next-intl' @@ - const t = useTranslations() + const t = useTranslations() + const f = useFormatter() @@ - {t('manage.evaluation.instanceLastRefetch')}: {lastRefetchTime.toLocaleString()} + {t('manage.evaluation.instanceLastRefetch')}: {f.dateTime(lastRefetchTime, { dateStyle: 'medium', timeStyle: 'short' })} @@ - {t('manage.evaluation.instanceExecutionDate')}: {new Date(expiresAt).toLocaleString()} + {t('manage.evaluation.instanceExecutionDate')}: {f.dateTime(new Date(expiresAt), { dateStyle: 'medium', timeStyle: 'short' })}Also applies to: 55-57
85-96: Improve a11y for the status dot.Expose a text label (aria-label/title) for screen readers and non-hover devices. You can set it on the wrapper using the same tooltip text.
- return ( - <div className={twMerge('flex justify-center', className)}> + return ( + <div + className={twMerge('flex justify-center', className)} + aria-label={t( + status === ElementBlockStatus.Scheduled + ? 'manage.evaluation.instanceScheduled' + : status === ElementBlockStatus.Active + ? 'manage.evaluation.instanceActive' + : 'manage.evaluation.instanceExecuted' + )} + title={t( + status === ElementBlockStatus.Scheduled + ? 'manage.evaluation.instanceScheduled' + : status === ElementBlockStatus.Active + ? 'manage.evaluation.instanceActive' + : 'manage.evaluation.instanceExecuted' + )} + > <Tooltip tooltip={getTooltipContent()}apps/frontend-manage/src/components/evaluation/EvaluationUnavailableNotification.tsx (1)
44-50: Localize element type label.Currently prints the enum. Use existing shared translations for element type.
- : {elementType} + : {t(`shared.${elementType}.typeLabel`, { fallback: elementType })}apps/frontend-manage/src/pages/quizzes/[id]/evaluation.tsx (1)
38-41: Empty-state condition may miss the “no leaderboard” case.If leaderboard is null/undefined,
=== 0is false. Consider treating “absent or empty” as empty.- data.liveQuizEvaluation.results.length === 0 && - data.liveQuizLeaderboard?.length === 0 && - data.liveQuizEvaluation.feedbacks?.length === 0 && - data.liveQuizEvaluation.confusionFeedbacks?.length === 0 + data.liveQuizEvaluation.results.length === 0 && + (!data.liveQuizLeaderboard || data.liveQuizLeaderboard.length === 0) && + ((data.liveQuizEvaluation.feedbacks?.length ?? 0) === 0) && + ((data.liveQuizEvaluation.confusionFeedbacks?.length ?? 0) === 0)apps/frontend-manage/src/components/evaluation/ActivityEvaluation.tsx (2)
89-95: Memoize instanceResults to avoid recomputation.Minor perf tidy-up on re-renders.
-import { useReducer, useState } from 'react' +import { useMemo, useReducer, useState } from 'react' @@ - const instanceResults = stacks.flatMap((stack, stackIx) => - stack.instances.map((instance) => ({ - ...instance, - stackIx, - })) - ) + const instanceResults = useMemo( + () => + stacks.flatMap((stack, stackIx) => + stack.instances.map((instance) => ({ ...instance, stackIx })) + ), + [stacks] + )
189-214: Prop surface expansion LGTM; note pass-through of unused courseId to child.If ElementEvaluation doesn’t use courseId, consider removing the prop here too in a follow-up to keep APIs tight.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
packages/graphql/src/graphql/ops/QGetLiveQuizEvaluation.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 (10)
apps/frontend-manage/src/components/evaluation/ActivityEvaluation.tsx(7 hunks)apps/frontend-manage/src/components/evaluation/BlockStatusIndicator.tsx(1 hunks)apps/frontend-manage/src/components/evaluation/ElementEvaluation.tsx(5 hunks)apps/frontend-manage/src/components/evaluation/EvaluationUnavailableNotification.tsx(2 hunks)apps/frontend-manage/src/pages/quizzes/[id]/evaluation.tsx(3 hunks)deploy/charts/klicker-uzh-v2/templates/cm-frontend-assessment.yaml(1 hunks)packages/graphql/src/schema/evaluation.ts(2 hunks)packages/graphql/src/services/liveQuizzes.ts(3 hunks)packages/i18n/messages/de.ts(3 hunks)packages/i18n/messages/en.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: 2025-03-11T15:07:47.117Z
Learnt from: sjschlapbach
PR: uzh-bf/klicker-uzh#4550
File: apps/frontend-manage/src/components/courses/modals/TemplateConversionModal.tsx:132-138
Timestamp: 2025-03-11T15:07:47.117Z
Learning: The translation keys for template conversion error messages (`noInstances`, `resourcesRequiredMissing`, `noResourceAccessRequired`) are properly defined in the translation files (packages/i18n/messages/en.ts and packages/i18n/messages/de.ts) for the KlickerUZH project.
Applied to files:
packages/i18n/messages/de.ts
📚 Learning: 2025-09-13T17:39:50.281Z
Learnt from: sjschlapbach
PR: uzh-bf/klicker-uzh#4902
File: packages/util/src/blocks.ts:84-107
Timestamp: 2025-09-13T17:39:50.281Z
Learning: FLASHCARD elements are not allowed to be included in live quizzes in the KlickerUZH system, so FLASHCARD handling is not needed in live quiz submission and result processing logic.
Applied to files:
packages/i18n/messages/de.tspackages/i18n/messages/en.ts
📚 Learning: 2025-04-17T12:52:17.002Z
Learnt from: sjschlapbach
PR: uzh-bf/klicker-uzh#4606
File: packages/graphql/src/services/liveQuizzes.ts:1100-1106
Timestamp: 2025-04-17T12:52:17.002Z
Learning: In the application architecture, permission checks for operations like activateLiveQuizBlock, deactivateLiveQuizBlock, endLiveQuiz, cancelLiveQuiz, and deleteLiveQuiz are performed at the GraphQL mutation resolver level using the checkAccess function, rather than in the service functions themselves. This separation of concerns means that ownerId filters within service functions are no longer necessary.
Applied to files:
packages/graphql/src/services/liveQuizzes.ts
📚 Learning: 2025-04-17T12:47:09.182Z
Learnt from: sjschlapbach
PR: uzh-bf/klicker-uzh#4625
File: packages/graphql/src/services/liveQuizzes.ts:771-779
Timestamp: 2025-04-17T12:47:09.182Z
Learning: In the KlickerUZH system, running live quizzes (with status PublicationStatus.PUBLISHED) cannot be deleted as prevented by validation logic in the deleteLiveQuiz function, making the isDeleted: false filter redundant when querying for published quizzes.
Applied to files:
packages/graphql/src/services/liveQuizzes.ts
📚 Learning: 2025-09-08T19:16:04.791Z
Learnt from: sjschlapbach
PR: uzh-bf/klicker-uzh#4884
File: apps/frontend-pwa/src/components/liveQuiz/LiveQuizSubscriber.tsx:21-37
Timestamp: 2025-09-08T19:16:04.791Z
Learning: For RunningLiveQuizUpdatedDocument subscription in apps/frontend-pwa/src/components/liveQuiz/LiveQuizSubscriber.tsx, the backend implementation guarantees non-null values, so frontend null checks would be unnecessary and falsify return types. The subscription schema should be updated to reflect non-nullable return types instead.
Applied to files:
packages/graphql/src/services/liveQuizzes.ts
📚 Learning: 2025-04-17T10:22:49.381Z
Learnt from: sjschlapbach
PR: uzh-bf/klicker-uzh#4624
File: packages/graphql/src/services/feedbacks.ts:298-304
Timestamp: 2025-04-17T10:22:49.381Z
Learning: In the klicker-uzh project, relation filters like `where: { id, feedback: { liveQuizId } }` work with Prisma's `findUnique` method despite documentation suggesting otherwise.
Applied to files:
packages/graphql/src/services/liveQuizzes.ts
📚 Learning: 2024-08-19T20:18:26.837Z
Learnt from: sjschlapbach
PR: uzh-bf/klicker-uzh#4185
File: packages/graphql/src/services/practiceQuizzes.ts:86-91
Timestamp: 2024-08-19T20:18:26.837Z
Learning: In the `getPracticeQuizData` function, only one feedback entry can exist per participant due to a uniqueness constraint, making it unnecessary to fetch more than one feedback.
Applied to files:
packages/graphql/src/services/liveQuizzes.ts
📚 Learning: 2025-06-23T12:32:29.344Z
Learnt from: CR
PR: uzh-bf/klicker-uzh#0
File: apps/frontend-manage/CLAUDE.md:0-0
Timestamp: 2025-06-23T12:32:29.344Z
Learning: For GraphQL integration, always import operation documents from klicker-uzh/graphql and use Apollo Client for data fetching and mutations, handling loading, error, and success states consistently.
Applied to files:
apps/frontend-manage/src/components/evaluation/ElementEvaluation.tsxapps/frontend-manage/src/components/evaluation/ActivityEvaluation.tsx
🧬 Code graph analysis (5)
apps/frontend-manage/src/components/evaluation/EvaluationUnavailableNotification.tsx (1)
packages/graphql/src/schema/practiceQuiz.ts (1)
PublicationStatus(24-26)
apps/frontend-manage/src/components/evaluation/BlockStatusIndicator.tsx (1)
packages/graphql/src/schema/liveQuiz.ts (1)
ElementBlockStatus(11-13)
apps/frontend-manage/src/components/evaluation/ElementEvaluation.tsx (2)
packages/graphql/src/schema/practiceQuiz.ts (1)
PublicationStatus(24-26)packages/graphql/src/schema/liveQuiz.ts (1)
ElementBlockStatus(11-13)
packages/graphql/src/schema/evaluation.ts (1)
packages/graphql/src/schema/practiceQuiz.ts (1)
PublicationStatus(24-26)
apps/frontend-manage/src/components/evaluation/ActivityEvaluation.tsx (2)
packages/graphql/src/schema/practiceQuiz.ts (1)
PublicationStatus(24-26)packages/graphql/src/schema/evaluation.ts (1)
ActivityEvaluation(240-272)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: cypress-run-cloud
- GitHub Check: test
- GitHub Check: build
- GitHub Check: test
- GitHub Check: Analyze (javascript)
- GitHub Check: SonarCloud
- GitHub Check: build
- GitHub Check: check
- GitHub Check: claude-review
🔇 Additional comments (14)
packages/graphql/src/schema/evaluation.ts (2)
244-251: GraphQL exposure of status and courseName aligns with service outputsstatus: PublicationStatus (nullable) and courseName?: string are correctly exposed; displayName now non-nullable matches the interface above. LGTM.
22-27: Confirm DB + migrations guarantee displayName is non-null for LiveQuizPrisma schema and migrations show displayName NOT NULL for Quiz/related activity tables (see packages/prisma/src/prisma/schema/quiz.prisma and migrations such as packages/prisma/src/prisma/schema/migrations/20220925181035_initial_migration/migration.sql and 20240119171242_element_structure_and_unique/migration.sql), but I couldn't find an explicit LiveQuiz model — LiveQuiz is likely represented by Quiz (type='LIVE_QUIZ'). Verify that this GraphQL type maps to that non-null DB column and that all production/legacy rows were migrated/backfilled (no existing nulls remain).
packages/graphql/src/services/liveQuizzes.ts (1)
2278-2286: Expose status and courseName in evaluation payload: LGTMstatus and courseName are correctly sourced from liveQuiz and match the GraphQL schema changes.
packages/i18n/messages/de.ts (2)
599-604: LGTM – clearer LMS path info for Live Quizzes.String reads well and keeps punctuation consistent with surrounding content.
666-668: LGTM – onboarding flow wording is fine.No grammar issues detected in this change.
packages/i18n/messages/en.ts (3)
597-602: Student docs reflow looks good.The content changes read well and keep consistent tone.
664-671: Join-course section text tweaks LGTM.No issues spotted in the link formatting and guidance text.
2413-2424: New evaluation metadata keys — verify parity and usage.
- Ensure corresponding keys exist in all supported locales (e.g., de.ts) and are wired in UI where referenced (BlockStatusIndicator/EvaluationUnavailableNotification).
- Confirm copy consistency: “real-time” vs “real time” used elsewhere.
apps/frontend-manage/src/components/evaluation/ElementEvaluation.tsx (1)
168-177: Scheduled-state message placement LGTM.Rendering the unavailable notice in scheduled state is a good UX touch.
apps/frontend-manage/src/components/evaluation/EvaluationUnavailableNotification.tsx (1)
75-81: Activity status rendering LGTM.Uses shared status translations consistently.
apps/frontend-manage/src/pages/quizzes/[id]/evaluation.tsx (3)
26-31: Refetch timestamp tracking LGTM.Simple and effective for UI hints.
33-35: Stronger loading guard LGTM.
61-72: Prop plumb-through of status/course/refetch time LGTM.apps/frontend-manage/src/components/evaluation/ActivityEvaluation.tsx (1)
131-143: Unavailable-notice enrichment LGTM.Good use of course/activity metadata for context.
| {activityId && ( | ||
| <a | ||
| href={`https://manage.klicker.com/activities?openActivityDetailsId=${activityId}&openActivityDetailsType=LIVE_QUIZ`} | ||
| target="_blank" | ||
| rel="noreferrer" | ||
| > | ||
| <FontAwesomeIcon | ||
| size="sm" | ||
| icon={faExternalLinkAlt} | ||
| className="mr-2" | ||
| /> | ||
| {t('manage.evaluation.linkActivityDetails')} | ||
| </a> | ||
| )} |
There was a problem hiding this comment.
Avoid hardcoded host; make link environment‑agnostic and secure.
Use a relative path (or env base URL) and include noopener.
- <a
- href={`https://manage.klicker.com/activities?openActivityDetailsId=${activityId}&openActivityDetailsType=LIVE_QUIZ`}
- target="_blank"
- rel="noreferrer"
- >
+ <a
+ href={`/activities?openActivityDetailsId=${activityId}&openActivityDetailsType=LIVE_QUIZ`}
+ target="_blank"
+ rel="noopener noreferrer"
+ >📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {activityId && ( | |
| <a | |
| href={`https://manage.klicker.com/activities?openActivityDetailsId=${activityId}&openActivityDetailsType=LIVE_QUIZ`} | |
| target="_blank" | |
| rel="noreferrer" | |
| > | |
| <FontAwesomeIcon | |
| size="sm" | |
| icon={faExternalLinkAlt} | |
| className="mr-2" | |
| /> | |
| {t('manage.evaluation.linkActivityDetails')} | |
| </a> | |
| )} | |
| {activityId && ( | |
| <a | |
| href={`/activities?openActivityDetailsId=${activityId}&openActivityDetailsType=LIVE_QUIZ`} | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| > | |
| <FontAwesomeIcon | |
| size="sm" | |
| icon={faExternalLinkAlt} | |
| className="mr-2" | |
| /> | |
| {t('manage.evaluation.linkActivityDetails')} | |
| </a> | |
| )} |
🤖 Prompt for AI Agents
In
apps/frontend-manage/src/components/evaluation/EvaluationUnavailableNotification.tsx
around lines 51 to 64, the anchor currently uses a hardcoded
https://manage.klicker.com host and only rel="noreferrer"; change it to build
the URL from a relative path or a configured base (e.g.
process.env.MANAGE_BASE_URL or window.location.origin + '/activities') instead
of the fixed host, keep the query string parameter, preserve target="_blank",
and add rel="noopener noreferrer" to avoid reverse tabnabbing and make the link
environment‑agnostic and secure.
| data: | ||
| # SSR-only GraphQL endpoint for Assessment PWA (internal service DNS) | ||
| API_URL_SSR: "http://{{ include "chart.fullname" . }}-backend-assessment:3000/api/graphql" | ||
| NEXT_PUBLIC_IS_ASSESSMENT: "true" |
There was a problem hiding this comment.
🧩 Analysis chain
Public flag looks good; confirm parsing and env split across frontends
Setting NEXT_PUBLIC_IS_ASSESSMENT to "true" is appropriate for Next.js public envs (strings). Please verify:
- All consumers parse it as a boolean (e.g., process.env.NEXT_PUBLIC_IS_ASSESSMENT === 'true').
- Other frontend ConfigMaps explicitly set it to "false" or omit it to avoid accidental leakage of assessment-only UI.
🏁 Script executed:
#!/bin/bash
# Ensure all consumers compare to the string 'true' and not rely on truthy values.
rg -nP --type=ts --type=tsx "NEXT_PUBLIC_IS_ASSESSMENT" -C2Length of output: 27
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) All occurrences of NEXT_PUBLIC_IS_ASSESSMENT"
rg -n --hidden --no-ignore -S "NEXT_PUBLIC_IS_ASSESSMENT" -C3 || true
echo
echo "2) process.env usage"
rg -n --hidden --no-ignore -S -F "process.env.NEXT_PUBLIC_IS_ASSESSMENT" -C3 || true
rg -n --hidden --no-ignore -S -F "process.env['NEXT_PUBLIC_IS_ASSESSMENT']" -C3 || true
rg -n --hidden --no-ignore -S -F 'process.env["NEXT_PUBLIC_IS_ASSESSMENT"]' -C3 || true
echo
echo "3) Programmatic comparisons to 'true' / true (JS/TS)"
rg -n --hidden --no-ignore -S -P "NEXT_PUBLIC_IS_ASSESSMENT\s*(?:===|==)\s*'true'" -C3 || true
rg -n --hidden --no-ignore -S -P 'NEXT_PUBLIC_IS_ASSESSMENT\s*(?:===|==)\s*"true"' -C3 || true
rg -n --hidden --no-ignore -S -P "NEXT_PUBLIC_IS_ASSESSMENT\s*(?:===|==)\s*true\b" -C3 || true
echo
echo "4) YAML/Helm/ConfigMap assignments (look for : true/false or =)"
rg -n --hidden --no-ignore -S -P "NEXT_PUBLIC_IS_ASSESSMENT\s*[:=]\s*\"?(true|false)\"?" -C3 || true
echo
echo "5) .env / env.* files"
rg -n --hidden --no-ignore -S -g "**/.env*" -P "NEXT_PUBLIC_IS_ASSESSMENT\s*=\s*\"?(true|false)\"?" -C3 || true
echo
echo "Done."Length of output: 42493
Fix truthy env checks; ensure non‑assessment frontends unset/false
- Replace truthy checks with explicit string comparison (=== 'true') — e.g. apps/frontend-pwa/src/pages/index.tsx:351 uses
process.env.NEXT_PUBLIC_IS_ASSESSMENT &&→ change toprocess.env.NEXT_PUBLIC_IS_ASSESSMENT === 'true' &&. - Ensure non‑assessment frontends explicitly set NEXT_PUBLIC_IS_ASSESSMENT='false' or omit it. Assessment-only settings found at deploy/charts/klicker-uzh-v2/templates/cm-frontend-assessment.yaml:11, apps/frontend-pwa/.env.assessment:31, apps/frontend-pwa/.env.assessment.qa:29; no 'false' assignments were found.
🤖 Prompt for AI Agents
In deploy/charts/klicker-uzh-v2/templates/cm-frontend-assessment.yaml around
line 11, the NEXT_PUBLIC_IS_ASSESSMENT variable is set truthily; change
deployment/environment values so assessment charts explicitly set
NEXT_PUBLIC_IS_ASSESSMENT: "true" and non‑assessment charts or .env files
explicitly set NEXT_PUBLIC_IS_ASSESSMENT: "false" (or omit it) and audit the
other mentioned files (apps/frontend-pwa/.env.assessment, .env.assessment.qa) to
ensure consistency; then update all runtime checks in the frontend code to use
explicit string comparison (process.env.NEXT_PUBLIC_IS_ASSESSMENT === 'true')
instead of truthy checks.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codex Review: Here are some suggestions.
klicker-uzh/packages/graphql/src/services/liveQuizzes.ts
Lines 2271 to 2274 in 97bbc6d
[P1] Avoid duplicating active block in evaluation results
Since the Prisma query now includes all blocks (no status filter), the subsequent spread [..., liveQuiz.blocks, { ...activeBlockWithResults, active: true }] adds the currently active block a second time. The first entry still carries stale database results while the appended one is live, so the evaluation UI will display the active block twice and may show inconsistent numbers. Consider excluding the active block from liveQuiz.blocks before appending the real-time copy or dropping the extra append when the list already contains the active block.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
klicker-uzh
|
||||||||||||||||||||||||||||||
| Project |
klicker-uzh
|
| Branch Review |
activity-info-on-eval
|
| Run status |
|
| Run duration | 12m 50s |
| Commit |
|
| Committer | Roland Schläfli |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
4
|
Flaky
|
1
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
754
|
Upgrade your plan to view test results. | |
| View all changes introduced in this branch ↗︎ | |
Tests for review
There was a problem hiding this comment.
The evaluation page seems to be broken from a UI point of view and might not even refresh correctly anymore (?) - the reason for the latter is unknown... (the timestamps on the new status point also don't update, etc.)
For some reason, the evaluation does not seem to load the results anymore? Even after having successfully completed hatchet tasks and correct data in the redis cache, the evaluation (both with hmac and without) does not show any results?
When opening an evaluation, the actual element evaluation is no longer hidden, but can be found partially overlapping with the footer when scrolling down:
| message={t('manage.evaluation.evaluationNotYetAvailable')} | ||
| /> | ||
|
|
||
| <div className="mt-4 flex flex-row gap-8 text-xs text-gray-500"> |
There was a problem hiding this comment.
Could we maybe stick to at least small text here? text-xs seems very very small and is barely readable
| <div className="mt-4 flex flex-row gap-8 text-xs text-gray-500"> | |
| <div className="mt-4 flex flex-row gap-8 text-sm text-gray-500"> |
| <span className="font-semibold"> | ||
| {t('manage.evaluation.elementType')} | ||
| </span> | ||
| : {elementType} |
There was a problem hiding this comment.
We should definitely use the properly translated strings here instead of the GraphQL enums
| : {elementType} | |
| : {t(`shared.${elementType}.typeLabel`)} |
|
|
||
| function LiveQuizEvaluation() { | ||
| const router = useRouter() | ||
| const [lastRefetchTime, setLastRefetchTime] = useState<Date>(new Date()) |
There was a problem hiding this comment.
Is it valid to initialize this time with the current timestamp? - probably sufficient as an approximation though
| }, | ||
| evaluation: { | ||
| courseName: 'Course', | ||
| activityName: 'Activity', |
There was a problem hiding this comment.
Most of these translations are already defined under shared.generic.course, shared.generic.activity, shared.generic.element, manage.general.elementType, etc. - we should probably try to re-use these in case we decide to rename them again later?
sjschlapbach
left a comment
sjschlapbach
left a comment
There was a problem hiding this comment.
Additional bug that was observed after closing the block is that the evaluation (observed / checked for the regular one) now seems to show each completed block twice - once with results and once without:
…ity-info-on-eval
|
2 participants
Summary by CodeRabbit