[STU-101] Language CRUD for candidate profile#35
Merged
Conversation
The game files were reverted in 56f25f4 but the middleware still allowed unauthenticated access to the /games route. Co-Authored-By: Paperclip <noreply@paperclip.ing>
Add server actions (addCandidateLanguage, removeCandidateLanguage) with Zod validation, candidate_language Prisma model, and language form section in CandidateEditForm with proficiency badges and toast notifications. Co-Authored-By: Paperclip <noreply@paperclip.ing>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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:
WalkthroughAdds persistent candidate language management (DB model, server actions, data fetching, and edit-form UI), introduces a Civil ID display component plus styles, and restricts middleware public paths to only "/login" and "/". ChangesCandidate Language Management
Civil ID UI
Public Route Access Control & Type tweaks
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 1
🤖 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 `@prisma/schema.prisma`:
- Around line 425-434: The candidate_language_created_at field is nullable and
left unset by addCandidateLanguage, causing NULLs that break ordering in
getCandidateDetail; update the Prisma model (candidate_language ->
candidate_language_created_at) to be non-nullable and set a default timestamp
(e.g., `@default`(now()) with `@db.DateTime`(0)), then regenerate/prisma migrate to
apply the change, and ensure the addCandidateLanguage code path sets or relies
on the DB default for created_at so new rows always have a timestamp for
ordering/auditing.
🪄 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 Plus
Run ID: c718ccb0-a388-4d81-b567-eaeb65f3aed4
📒 Files selected for processing (6)
prisma/schema.prismasrc/app/candidate/edit/page.tsxsrc/middleware.tssrc/modules/candidates/CandidateEditForm.tsxsrc/modules/candidates/actions.tssrc/modules/workspace/data.ts
Show civil ID number, expiry date with warning/expired state, photo front/back thumbnails, and needs-verification badge below the profile facts grid. Adds a dedicated CivilIdPanel component with expiry date threshold alerts (90 days) and responsive photo layout. Co-Authored-By: Paperclip <noreply@paperclip.ing>
2 tasks
Owner
Author
DevRel ReviewStatus: Comment — one actionable finding to address CodeRabbit finding: nullable
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/app/styles.css`:
- Around line 8700-8703: The CSS rule for .civilIdPanelFields span uses a
hardcoded fallback token var(--ink-muted, `#6b7280`); replace that with the theme
token var(--muted) to ensure consistent contrast and theming (including dark
mode). Update the declaration in the .civilIdPanelFields span rule and also the
identical rule elsewhere in the file (the second occurrence mentioned in the
review) so both use color: var(--muted) instead of var(--ink-muted, `#6b7280`).
In `@src/modules/candidates/CandidateProfile.tsx`:
- Around line 112-113: CandidateProfile is rendering sensitive Civil ID fields
and photos (e.g., the Fact at label "Civil ID" and the photo render block in the
component) without any access checks; use server-side RBAC/capability guards so
only authorized users see PII. Wrap the Civil ID and photo rendering behind
requireRole(...) or requireCapability(...) checks (depending on whether broad
role or fine-grained capability is desired) and only render the full value/photo
when the check passes; otherwise render a safe placeholder like "Hidden" or a
masked value. Update any places where viewerRole is currently unused to use the
proper requireRole/requireCapability calls, and ensure the checks are applied to
all occurrences referenced (the Fact at "Civil ID" and the photo rendering block
in CandidateProfile). Ensure you do this in the server component context so
authorization happens before sending data to the client.
🪄 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 Plus
Run ID: 30b82cf7-7533-471c-bce1-fd6720c9695e
📒 Files selected for processing (2)
src/app/styles.csssrc/modules/candidates/CandidateProfile.tsx
Comment on lines
+8700
to
+8703
| .civilIdPanelFields span { | ||
| font-size: 12px; | ||
| color: var(--ink-muted, #6b7280); | ||
| } |
There was a problem hiding this comment.
Use existing theme token for secondary text color in Civil ID labels.
var(--ink-muted, #6b7280) bypasses your defined theme tokens and can produce inconsistent contrast (especially in dark mode). Use var(--muted) here.
Suggested fix
.civilIdPanelFields span {
font-size: 12px;
- color: var(--ink-muted, `#6b7280`);
+ color: var(--muted);
}
@@
.civilIdPhotos span {
font-size: 12px;
- color: var(--ink-muted, `#6b7280`);
+ color: var(--muted);
}Also applies to: 8731-8734
🤖 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 `@src/app/styles.css` around lines 8700 - 8703, The CSS rule for
.civilIdPanelFields span uses a hardcoded fallback token var(--ink-muted,
`#6b7280`); replace that with the theme token var(--muted) to ensure consistent
contrast and theming (including dark mode). Update the declaration in the
.civilIdPanelFields span rule and also the identical rule elsewhere in the file
(the second occurrence mentioned in the review) so both use color: var(--muted)
instead of var(--ink-muted, `#6b7280`).
Comment on lines
+112
to
113
| <Fact label="Civil ID" value={candidate.candidate_civil_id ?? (candidate.candidate_civil_need_verification ? "Needs verification" : "Not set")} /> | ||
| <Fact label="Updated" value={formatDate(candidate.candidate_updated_at)} /> |
There was a problem hiding this comment.
Gate Civil ID PII rendering by role/capability before display.
Line 112 and Lines 172-226 render Civil ID number/photos without an access check, and viewerRole is currently unused. This can expose sensitive data to unauthorized viewers.
Suggested fix
export function CandidateProfile({
detail,
actions,
backHref,
compact = false,
viewerRole
}: {
@@
}) {
@@
+ const canViewCivilId = viewerRole === "staff";
@@
- <Fact label="Civil ID" value={candidate.candidate_civil_id ?? (candidate.candidate_civil_need_verification ? "Needs verification" : "Not set")} />
+ <Fact
+ label="Civil ID"
+ value={
+ canViewCivilId
+ ? candidate.candidate_civil_id ?? (candidate.candidate_civil_need_verification ? "Needs verification" : "Not set")
+ : "Restricted"
+ }
+ />
@@
- <CivilIdPanel candidate={candidate} viewerRole={viewerRole} />
+ <CivilIdPanel candidate={candidate} viewerRole={viewerRole} />
@@
function CivilIdPanel({ candidate, viewerRole }: { candidate: NonNullable<CandidateDetailData["candidate"]>; viewerRole?: string }) {
+ if (viewerRole !== "staff") {
+ return null;
+ }As per coding guidelines, "Use requireRole() in server components for role-based access control" and "Use requireCapability() for granular capability-based access control".
Also applies to: 116-116, 172-226
🤖 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 `@src/modules/candidates/CandidateProfile.tsx` around lines 112 - 113,
CandidateProfile is rendering sensitive Civil ID fields and photos (e.g., the
Fact at label "Civil ID" and the photo render block in the component) without
any access checks; use server-side RBAC/capability guards so only authorized
users see PII. Wrap the Civil ID and photo rendering behind requireRole(...) or
requireCapability(...) checks (depending on whether broad role or fine-grained
capability is desired) and only render the full value/photo when the check
passes; otherwise render a safe placeholder like "Hidden" or a masked value.
Update any places where viewerRole is currently unused to use the proper
requireRole/requireCapability calls, and ensure the checks are applied to all
occurrences referenced (the Fact at "Civil ID" and the photo rendering block in
CandidateProfile). Ensure you do this in the server component context so
authorization happens before sending data to the client.
Add missing certificate fields to candidate_certificate model, rejection_reason to ID request detail query, and fix WorkLogStaffActions form action type cast. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Resolves CodeRabbit finding: nullable created_at was left unset by addCandidateLanguage, causing NULLs that break orderBy in getCandidateDetail. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Add vitest unit tests for language schema validation (9 tests) - Add Playwright e2e tests for full language CRUD flow on edit profile page (5 tests × 2 browser projects = 10 test cases) - Tests cover: schema validation, add language, remove language, empty state, access control Tests: added unit and integration tests for language CRUD Co-Authored-By: Paperclip <noreply@paperclip.ing>
- Updated validate.mjs model count from 128 to 129 (candidate_language added) - Fixed corrupted portalContent.ts with stray merge artifacts Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/modules/candidates/actions-language.test.ts (1)
12-19: ⚡ Quick winAvoid duplicating the validation schema in tests.
This test can pass while production behavior changes if the action schema diverges. Prefer importing a shared
languageSchemafrom the source module (or a dedicated shared schema file) and test that directly.🤖 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 `@src/modules/candidates/actions-language.test.ts` around lines 12 - 19, The test duplicates the validation schema (PROFICIENCY_LEVELS and languageSchema) which risks divergence from production; instead, remove the local definitions and import the shared languageSchema used by the action (or a dedicated shared schema export) into the test, then use that imported languageSchema in assertions; update any test references to PROFICIENCY_LEVELS to use the exported enum/const from the source module (or derive values from the imported schema) so the test always validates the same schema as the runtime code.
🤖 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 `@e2e/smoke/candidate-language-crud.spec.ts`:
- Line 2: Replace the relative import of getFixtures with the project path
alias: change the import statement that currently imports getFixtures from
"../fixtures/auth" to use "`@/fixtures/auth`" so the code uses the repository's `@/`
alias for internal imports (look for the getFixtures import at the top of the
spec file).
In `@vitest.config.ts`:
- Line 1: Add vitest as a devDependency so TypeScript can resolve imports from
"vitest/config" referenced in vitest.config.ts; update package.json's
devDependencies to include an appropriate vitest version (or run the package
manager command to add it) and then reinstall so module resolution errors
(Cannot find module 'vitest/config') are resolved by tsc when running
test:types; ensure package-lock or yarn.lock is updated accordingly.
---
Nitpick comments:
In `@src/modules/candidates/actions-language.test.ts`:
- Around line 12-19: The test duplicates the validation schema
(PROFICIENCY_LEVELS and languageSchema) which risks divergence from production;
instead, remove the local definitions and import the shared languageSchema used
by the action (or a dedicated shared schema export) into the test, then use that
imported languageSchema in assertions; update any test references to
PROFICIENCY_LEVELS to use the exported enum/const from the source module (or
derive values from the imported schema) so the test always validates the same
schema as the runtime code.
🪄 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 Plus
Run ID: 7763cb90-8004-4982-905d-e282f19af3f3
📒 Files selected for processing (5)
e2e/smoke/candidate-language-crud.spec.tsscripts/validate.mjssrc/modules/auth/portalContent.tssrc/modules/candidates/actions-language.test.tsvitest.config.ts
| @@ -0,0 +1,207 @@ | |||
| import { test, expect } from "@playwright/test"; | |||
| import { getFixtures } from "../fixtures/auth"; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use the @/ alias for internal imports.
Please replace the relative import with the project alias to match repository standards.
Suggested change
-import { getFixtures } from "../fixtures/auth";
+import { getFixtures } from "`@/e2e/fixtures/auth`";As per coding guidelines, "**/*.{ts,tsx,js,jsx}: Use @/ path alias for all internal imports".
📝 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.
Suggested change
| import { getFixtures } from "../fixtures/auth"; | |
| import { getFixtures } from "`@/e2e/fixtures/auth`"; |
🤖 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 `@e2e/smoke/candidate-language-crud.spec.ts` at line 2, Replace the relative
import of getFixtures with the project path alias: change the import statement
that currently imports getFixtures from "../fixtures/auth" to use
"`@/fixtures/auth`" so the code uses the repository's `@/` alias for internal
imports (look for the getFixtures import at the top of the spec file).
PR #35 added vitest test files (actions-language.test.ts, vitest.config.ts) but vitest was not installed, causing TypeScript CI failures: "Cannot find module 'vitest/config'" and "Cannot find module 'vitest'" Co-Authored-By: Paperclip <noreply@paperclip.ing>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@package.json`:
- Line 67: package.json was updated to add the "vitest" dependency but the
lockfile wasn't updated, causing CI to fail with `npm ci`; regenerate and commit
the updated package-lock.json by running `npm install` (or `npm install
--package-lock-only`) locally so the lockfile reflects the new "vitest" entry,
then add and commit the updated package-lock.json alongside the package.json
change so CI `npm ci` succeeds.
🪄 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 Plus
Run ID: 439ec31e-150f-4643-b2cf-0e84977be028
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
package.json
npm install on macOS omits optional WASM emnapi packages needed on Linux CI runners (npm ci fails with "Missing: @emnapi/runtime@1.10.0 from lock file"). Restored entries manually from main's lock file. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This was referenced May 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
candidate_languagePrisma model with soft-delete supportaddCandidateLanguageandremoveCandidateLanguageserver actions with Zod validationTest plan
Files changed
prisma/schema.prisma— newcandidate_languagemodelsrc/modules/candidates/actions.ts— language server actions + Zod schemasrc/modules/workspace/data.ts— language query ingetCandidateDetailsrc/modules/candidates/CandidateEditForm.tsx— language form sectionsrc/app/candidate/edit/page.tsx— passlanguagesprop🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
UI / Styles
Security
Tests / Tooling