Ref: Improve course exam results accessibility#294
Conversation
The reason for this change is that this way the `/courses` route directory is less cluttered is less cluttered but also viewing said results does not require share-token. Hence, users can view the results of a given check even after the respective share-token has been deleted.
This commits moves the exam results component into the same hierarchical structure as the exam-results page (`results/[courseId]/examination`) are so that they are easier to locate.
This way users can now view examination results by clicking on the newly introduced menu-item within the `CourseActionMenu`.
This way custom breadcrumbs are shown for both the examination results overview page and the individual attempt results page. The reason for this is that the `GenericBreadcrumbs` show dynamic route parameters like `courseId` in its Breadcrumbs, which is not necessarily good from a user perspective.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 0 minutes and 56 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
WalkthroughServer pages and components were updated to use Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/courses/[share_token]/ExaminationAttemptTable.tsx (1)
160-165:⚠️ Potential issue | 🔴 CriticalIncorrect relative URL path causes navigation to wrong route.
The link
exam/${row.original.id}will resolve incorrectly when the table is rendered on/results/{courseId}/exam. The resulting URL would be/results/{courseId}/exam/exam/{attemptId}instead of the intended/results/{courseId}/exam/{attemptId}.Since this table is rendered on the exam results page at
/results/{courseId}/exam, the relative link should simply be the attempt ID.🐛 Proposed fix
- <Link href={`exam/${row.original.id}`}> + <Link href={`${row.original.id}`}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/courses/`[share_token]/ExaminationAttemptTable.tsx around lines 160 - 165, The Link currently uses a relative path "exam/${row.original.id}" which appends "exam" again and produces the wrong URL; update the Link href to just "${row.original.id}" so it navigates to the attempt ID relative to the current "/results/{courseId}/exam" route (modify the Link wrapping DropdownMenuItem in ExaminationAttemptTable.tsx where Link, row.original.id, DropdownMenuItem and ChartColumnIcon are used).
🧹 Nitpick comments (3)
src/components/results/examination/PreviewQuestionResultDrawer.tsx (1)
6-6: Switch this toimport typefor the table item type.This symbol is type-only and currently creates an unnecessary module edge back to
ExamQuestionResultTable.Proposed fix
-import { PreviewQuestionResult_QuestionItem } from '@/src/components/results/examination/ExamQuestionResultTable' +import type { PreviewQuestionResult_QuestionItem } from '@/src/components/results/examination/ExamQuestionResultTable'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/results/examination/PreviewQuestionResultDrawer.tsx` at line 6, The import of PreviewQuestionResult_QuestionItem in PreviewQuestionResultDrawer.tsx is type-only and should be converted to a TypeScript type-only import to avoid a runtime module dependency; replace the current import statement that brings in PreviewQuestionResult_QuestionItem from ExamQuestionResultTable with an "import type { PreviewQuestionResult_QuestionItem } ..." so the compiler treats it as a type-only import and removes the module edge.src/i18n/locales/de.ts (1)
1-1: Note: Auto-generated file header may be outdated.The file header states it is auto-generated and changes will be replaced. If this is still true, these manual changes could be lost. Consider either removing this comment if the file is now manually maintained, or updating the generation process to include these new keys.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/i18n/locales/de.ts` at line 1, The file header comment "//! Auto generated file, changes to this file will get replaced on next update" is misleading; either remove or update it if this locale file is now maintained manually, or modify the generation process to emit an accurate header that includes the new keys. Locate the header string in src/i18n/locales/de.ts and either (a) replace the auto-generated notice with a current maintenance note (or remove it) when making manual edits, or (b) update the i18n generation script so it regenerates this file with the correct header and includes the added keys.src/i18n/locales/en.ts (1)
1-1: Note: Same auto-generated file concern as the German locale.Same as
de.ts- if this file is still auto-generated, manual changes may be overwritten.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/i18n/locales/en.ts` at line 1, This en.ts file is auto-generated and manual edits will be lost; stop editing it directly and instead either (a) add your manual translations/overrides into the generator's source data (the canonical translations source used to produce en.ts), or (b) create a separate override module (e.g., en.overrides.ts) that imports the generated en.ts and applies your changes, or (c) update the generation config to preserve custom sections; locate the generated header line ("//! Auto generated file, changes to this file will get replaced on next update") in en.ts to identify the file and implement one of these approaches so manual changes aren’t overwritten.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/courses/`(hamburger-menu)/CourseActionMenu.tsx:
- Around line 145-151: The tooltip for the "Show Examination Results" menu item
is using t('edit_course.tooltip') which is misleading; add a new locale key
(e.g. Components.CourseActionMenu.show_exam_results.tooltip) with copy like "You
are not allowed to view exam results for this course!" and update the
DropdownMenuItem's tooltipOptions to use that new key instead of
t('edit_course.tooltip') while keeping the label using
t('show_exam_results.label'); change the call inside the component (around
DropdownMenuItem and tooltipOptions) to reference the new translation key so the
disabled tooltip matches the action.
In `@src/components/results/examination/ExamQuestionTable_ActionMenu.tsx`:
- Line 5: The import of PreviewQuestionResult_QuestionItem in
ExamQuestionTable_ActionMenu.tsx is creating a circular runtime dependency even
though it's only used as a type in function signatures; change the import to a
type-only import (use "import type { PreviewQuestionResult_QuestionItem }") so
the symbol remains available for typing but is erased at runtime, preventing the
circular module dependency with ExamQuestionResultTable which imports
ExamQuestionTable_ActionMenu at runtime.
In `@src/components/results/examination/ExamResultsBreadcrumbs.tsx`:
- Line 46: The breadcrumb component calls getCourseById causing a duplicate DB
fetch; either wrap the database helper getCourseById (in
database/course/select.ts) with React's cache() to dedupe within the same render
pass, i.e., export a cached version of getCourseById, or remove the call from
ExamResultsBreadcrumbs.tsx and accept the already-fetched course as a prop from
the page component (update the ExamResultsBreadcrumbs props and callers to pass
course) so no redundant query runs.
- Line 74: In ExamResultsBreadcrumbs.tsx the JSX currently contains the literal
HTML entity "'" which will render as text; replace {userName}'s
{t('results')} with a real apostrophe (e.g. {userName}'s {t('results')}) or the
Unicode escape (\u2019) to produce a proper apostrophe so the breadcrumb reads
correctly; update the JSX where userName and t('results') are combined.
- Line 1: Remove the top-level "'use server'" directive from
ExamResultsBreadcrumbs.tsx — this file exports React components (e.g., the
ExamResultsBreadcrumbs component) and is a Server Component by default; the
"'use server'" directive is only valid for Server Actions (functions invoked
from the client). Open the file, delete the "'use server'" line, and ensure no
server-action-specific syntax remains; if you intended to define a server
action, move the "'use server'" directive to the specific function instead.
---
Outside diff comments:
In `@src/components/courses/`[share_token]/ExaminationAttemptTable.tsx:
- Around line 160-165: The Link currently uses a relative path
"exam/${row.original.id}" which appends "exam" again and produces the wrong URL;
update the Link href to just "${row.original.id}" so it navigates to the attempt
ID relative to the current "/results/{courseId}/exam" route (modify the Link
wrapping DropdownMenuItem in ExaminationAttemptTable.tsx where Link,
row.original.id, DropdownMenuItem and ChartColumnIcon are used).
---
Nitpick comments:
In `@src/components/results/examination/PreviewQuestionResultDrawer.tsx`:
- Line 6: The import of PreviewQuestionResult_QuestionItem in
PreviewQuestionResultDrawer.tsx is type-only and should be converted to a
TypeScript type-only import to avoid a runtime module dependency; replace the
current import statement that brings in PreviewQuestionResult_QuestionItem from
ExamQuestionResultTable with an "import type {
PreviewQuestionResult_QuestionItem } ..." so the compiler treats it as a
type-only import and removes the module edge.
In `@src/i18n/locales/de.ts`:
- Line 1: The file header comment "//! Auto generated file, changes to this file
will get replaced on next update" is misleading; either remove or update it if
this locale file is now maintained manually, or modify the generation process to
emit an accurate header that includes the new keys. Locate the header string in
src/i18n/locales/de.ts and either (a) replace the auto-generated notice with a
current maintenance note (or remove it) when making manual edits, or (b) update
the i18n generation script so it regenerates this file with the correct header
and includes the added keys.
In `@src/i18n/locales/en.ts`:
- Line 1: This en.ts file is auto-generated and manual edits will be lost; stop
editing it directly and instead either (a) add your manual
translations/overrides into the generator's source data (the canonical
translations source used to produce en.ts), or (b) create a separate override
module (e.g., en.overrides.ts) that imports the generated en.ts and applies your
changes, or (c) update the generation config to preserve custom sections; locate
the generated header line ("//! Auto generated file, changes to this file will
get replaced on next update") in en.ts to identify the file and implement one of
these approaches so manual changes aren’t overwritten.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6cb9c373-6bf3-4331-9eb7-7256a9a06d92
📒 Files selected for processing (12)
src/app/[locale]/results/[courseId]/exam/[attemptId]/page.tsxsrc/app/[locale]/results/[courseId]/exam/page.tsxsrc/components/courses/(hamburger-menu)/CourseActionMenu.tsxsrc/components/courses/[share_token]/ExaminationAttemptTable.tsxsrc/components/results/examination/ExamQuestionResultTable.tsxsrc/components/results/examination/ExamQuestionTable_ActionMenu.tsxsrc/components/results/examination/ExamResultsBreadcrumbs.tsxsrc/components/results/examination/PreviewQuestionResultDrawer.tsxsrc/i18n/locales/de.jsonsrc/i18n/locales/de.tssrc/i18n/locales/en.jsonsrc/i18n/locales/en.ts
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
github-project-automation
Bot
moved this from In review
to Done
in KnowledgeCheckr Project
|
🎉 This PR is included in version 4.0.0-canary.3 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
1 participant
Summary
New Features
Refactors
app/results/[courseId]/examinationdirectoryThis way users are able to view said results even when a course no longer has a share-key.