[STU-134] Interview management page for staff#31
Merged
Conversation
- Add certificate_title, certificate_issuer, certificate_url fields to Prisma schema - Add Zod-validated addCandidateCertificate and removeCandidateCertificate server actions - Add certificate form section to CandidateEditForm with type select, title, issuer, dates, and URL - Pass certificate data from edit page to form component - Update data.ts to select and display new certificate fields Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…lity Bumps min-height on three login/shell elements from 40px/42px to 44px to meet WCAG 2.5.5 target size recommendations: nav links, theme toggle, and landing brand. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add tests for suggestion creation, application shortlist, interview completion, invitation creation, and cross-role visibility. Co-Authored-By: Paperclip <noreply@paperclip.ing>
- Replace floating pill bar with fixed full-width bottom tab bar - Add icon+label stacked layout with flex-direction: column - Add backdrop-filter blur, safe-area-inset-bottom support - Add active indicator via border-top highlight - Separate dark mode mobile tab bar styles - Update workspace stage bottom padding from 88px to 76px Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Clear note.suggestion_uuid and story.suggestion_uuid before deleting suggestions to avoid circular FK errors during test data cleanup. Co-Authored-By: Paperclip <noreply@paperclip.ing>
Adds /staff/interviews list and detail pages with status management, data fetching functions, G I keyboard shortcut, and nav integration. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
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:
WalkthroughThis PR adds staff interview list/detail views and a status-update server action; extends navigation with icons and an interviews shortcut; refactors the workflow-test harness and adds E2E pipeline/regression tests; adjusts mobile tab-bar styling and middleware public paths; and adds three certificate fields to the Prisma schema. ChangesStaff Interview Management
Navigation & Workspace UI
Test Harness Refactor & Pipeline Workflows
Mobile Styling & Middleware
Certificates & Candidate Action Tweaks
Sequence DiagramsequenceDiagram
participant StaffUser as Staff User
participant DetailPage as Interview Detail Page
participant StatusAction as updateInterviewStatusAction
participant Database as Prisma/Database
participant RevalidatePath as Path Revalidation
StaffUser->>DetailPage: View interview detail
DetailPage->>Database: getStaffInterviewDetail(interviewUuid, staffId)
Database-->>DetailPage: Interview data + status
StaffUser->>DetailPage: Submit status update form (interview_uuid + status)
DetailPage->>StatusAction: POST form data
StatusAction->>Database: Query request_interview by uuid + staffId
alt Interview owned by staff
StatusAction->>Database: Update status + updated_at
StatusAction->>RevalidatePath: Revalidate /staff/interviews and /staff/interviews/{id}
StatusAction-->>StaffUser: Redirect with notice=interview-updated
else Interview not owned
StatusAction-->>StaffUser: Redirect with notice=not-found
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 |
- Add LucideIcon field to NavItem type and map icons for all roles - Render icons in desktop WorkspaceNavigation and mobile tab bar - Add staff Interviews nav item with Calendar icon - Remove duplicate WorkspaceMobileNavigation when embedded in WorkspaceOS - Redesign mobile tab bar: full-width fixed bottom bar with icon+label layout - Add backdrop-filter blur, safe-area-inset-bottom, and active border-top indicator - Update workspace stage bottom padding from 88px to 76px Co-Authored-By: Paperclip <noreply@paperclip.ing>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/workflow-test.mjs (1)
273-323:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAlways restore mutated fixtures in
finally.Lines 318-321, 361-364, 405-408, 448-451, 492, and 533-536 only run on the happy path. If a POST succeeds and a later assertion/read fails, this script leaves persisted data behind and can cascade into unrelated failures on the next run.
Suggested pattern
async function candidateProfileUpdateTest() { const candidate = await getCandidate(); const originalName = candidate.candidate_name; const testName = `WFTEST_${Date.now()}`; const candidateCookie = signSession({ role: "candidate", id: String(candidate.candidate_id), name: candidate.candidate_name ?? "Candidate", email: candidate.candidate_email ?? "candidate@test.local", }); - const { status, text: pageHtml } = await getPage("/candidate/edit", candidateCookie); - assert(status === 200, `Candidate edit page returned ${status}`); - - const actionFields = extractActionRefs(pageHtml, "candidateEditForm"); - assert(actionFields, "Could not extract server action refs from candidate edit form"); - - const formData = new Map(actionFields); - formData.set("name", testName); - // ... - - const { status: postStatus } = await postFormAction("/candidate/edit", formData, candidateCookie); - assert(postStatus === 303 || postStatus === 200, - `Profile update POST returned ${postStatus} (expected 303 or 200)`); - - const updated = await prisma.candidate.findUniqueOrThrow({ - where: { candidate_id: candidate.candidate_id }, - select: { candidate_name: true }, - }); - assert(updated.candidate_name === testName, - `Expected candidate_name "${testName}" but got "${updated.candidate_name}"`); - - await prisma.candidate.update({ - where: { candidate_id: candidate.candidate_id }, - data: { candidate_name: originalName }, - }); + try { + const { status, text: pageHtml } = await getPage("/candidate/edit", candidateCookie); + assert(status === 200, `Candidate edit page returned ${status}`); + + const actionFields = extractActionRefs(pageHtml, "candidateEditForm"); + assert(actionFields, "Could not extract server action refs from candidate edit form"); + + const formData = new Map(actionFields); + formData.set("name", testName); + // ... + + const { status: postStatus } = await postFormAction("/candidate/edit", formData, candidateCookie); + assert(postStatus === 303 || postStatus === 200, + `Profile update POST returned ${postStatus} (expected 303 or 200)`); + + const updated = await prisma.candidate.findUniqueOrThrow({ + where: { candidate_id: candidate.candidate_id }, + select: { candidate_name: true }, + }); + assert(updated.candidate_name === testName, + `Expected candidate_name "${testName}" but got "${updated.candidate_name}"`); + } finally { + await prisma.candidate.update({ + where: { candidate_id: candidate.candidate_id }, + data: { candidate_name: originalName }, + }); + } }Also applies to: 329-366, 372-410, 416-453, 459-494, 500-539
🤖 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 `@scripts/workflow-test.mjs` around lines 273 - 323, The test function candidateProfileUpdateTest mutates DB state (sets candidate_name to testName) but only restores originalName on the happy path; wrap the entire test body that performs the POST and subsequent assertions in a try/finally so the prisma.candidate.update restoring originalName always runs, regardless of assertion failures or thrown errors. Specifically, after calling postFormAction and before any awaits that read from prisma (e.g., prisma.candidate.findUniqueOrThrow), capture originalName/testName and ensure the prisma.candidate.update(...) call is moved into a finally block tied to candidateProfileUpdateTest (or common helper used by similar tests), so restoration always executes; apply the same try/finally pattern to the other test functions that use postFormAction and prisma.candidate.update.
🧹 Nitpick comments (1)
scripts/workflow-test.mjs (1)
273-323: 🏗️ Heavy liftAdd an actual certificate CRUD workflow here.
The only candidate-facing workflow in this file updates
namethrough/candidate/edit. It never exercises the new add/remove certificate actions or verifies certificate persistence, so the feature added in this cohort is still unguarded by this harness.Also applies to: 603-614
🤖 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 `@scripts/workflow-test.mjs` around lines 273 - 323, The candidateProfileUpdateTest currently only updates the name; add a certificate CRUD flow that (1) uses getPage and extractActionRefs to find the server action refs for the certificate add form (e.g., "addCertificateForm" or the action name used on /candidate/edit) and the remove action(s), (2) builds a Map like formData and calls postFormAction to submit an add (fields like title, issuer, year), assert postStatus is 200/303, (3) query via prisma (e.g., prisma.candidate_certificate.findFirst or findMany) to assert the new certificate exists for candidate.candidate_id, (4) submit the corresponding remove action (using the remove form/action ref or certificate id) via postFormAction, assert success, and (5) verify via prisma that the certificate was deleted; integrate this sequence inside candidateProfileUpdateTest and ensure cleanup/restoration so the test leaves DB state unchanged.
🤖 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 `@scripts/workflow-test.mjs`:
- Around line 522-537: The test currently allows any page containing the word
"suggestion" and never asserts a DB row was created; tighten it by asserting the
page includes the exact suggestion text (use reason and/or the created
suggestion_uuid) instead of the generic "suggestion", then assert that
prisma.suggestion.findFirst(...) returns a non-null record (e.g. throw/assert if
record is null) before running cleanup; reference getPage(detailPath, cookie),
reason, prisma.suggestion.findFirst, and record.suggestion_uuid to locate and
verify the created suggestion.
In `@src/app/styles.css`:
- Around line 10452-10453: The hover rule for .candidateMissingFields li a:hover
uses an undefined CSS variable var(--fg); update that declaration to use a
defined token such as var(--foreground) or var(--ink) so the hover color
resolves consistently—locate the .candidateMissingFields li a:hover selector and
replace var(--fg) with the chosen existing variable.
In `@src/modules/candidates/actions.ts`:
- Around line 498-500: certificateSchema is currently coercing any non-"true"
value into false via certificate_type: z.string().transform(...), which silently
accepts invalid inputs; update certificate_type to only accept the literal
strings "true" or "false" (e.g., use z.enum(["true","false"]) or
z.union/z.literal variants) and then transform that strict enum to a boolean,
and provide a validation error message for invalid values so malformed/tampered
inputs are rejected rather than coerced; locate certificateSchema and the
certificate_type entry to implement this change.
In `@src/modules/requests/interview-actions.ts`:
- Around line 139-141: The current guard uses Number.isInteger(status) which
permits unsupported numeric codes (e.g., 7); replace this with a whitelist check
against the allowed interview status values (e.g., an InterviewStatus enum or a
supportedStatuses array) so that you validate that status is one of those
permitted values before persisting; update both occurrences (the block checking
interviewUuid/status around the redirect and the similar check at the later
location) to use this inclusion check and keep the same
redirect(`${basePath}?notice=missing-fields` as Route) when validation fails.
- Around line 143-149: Add an existence check for the interview before calling
prisma.request_interview.update when session.role !== "staff": use
prisma.request_interview.findFirst (or findUnique) to verify the record for
request_interview_uuid (same UUID used in the update) and if not found perform
redirect(`${basePath}?notice=not-found`) like the staff branch does; apply the
same check/redirect logic around the update path referenced in the block that
includes prisma.request_interview.update (also update the duplicate logic noted
at the second occurrence around lines 152-155) so no update is attempted for an
invalid UUID.
In `@src/modules/workspace/data.ts`:
- Around line 229-230: The query using prisma.request_interview.findMany (the
const rows retrieval) filters only on request_interview.staff_id and misses
interviews where ownership is recorded on the related request
(request.staff_id); update the where clause to include both ownership
possibilities—either add an OR that checks request_interview.staff_id ===
staffId OR the related request.staff_id === staffId (use Prisma relation
filtering on request), and apply the same change to the other occurrence around
the prisma.findMany at the noted spot so list/detail access checks match the
request.staff_id usage seen at request.staff_id (Line ~498).
- Line 1199: The subtitle computation uses a truthy check that treats any truthy
certificate_type as "Experience certificate" and mislabels other types or
numeric falsy values; update the subtitle assignment (where subtitle is set
using item.certificate_issuer and item.certificate_type) to perform an explicit
check or mapping for known certificate types (e.g., certificate_type ===
'experience' or a type-to-label map) and otherwise fall back to
item.certificate_issuer || "Certificate" so non-experience types and falsy
numeric values are labeled correctly.
---
Outside diff comments:
In `@scripts/workflow-test.mjs`:
- Around line 273-323: The test function candidateProfileUpdateTest mutates DB
state (sets candidate_name to testName) but only restores originalName on the
happy path; wrap the entire test body that performs the POST and subsequent
assertions in a try/finally so the prisma.candidate.update restoring
originalName always runs, regardless of assertion failures or thrown errors.
Specifically, after calling postFormAction and before any awaits that read from
prisma (e.g., prisma.candidate.findUniqueOrThrow), capture originalName/testName
and ensure the prisma.candidate.update(...) call is moved into a finally block
tied to candidateProfileUpdateTest (or common helper used by similar tests), so
restoration always executes; apply the same try/finally pattern to the other
test functions that use postFormAction and prisma.candidate.update.
---
Nitpick comments:
In `@scripts/workflow-test.mjs`:
- Around line 273-323: The candidateProfileUpdateTest currently only updates the
name; add a certificate CRUD flow that (1) uses getPage and extractActionRefs to
find the server action refs for the certificate add form (e.g.,
"addCertificateForm" or the action name used on /candidate/edit) and the remove
action(s), (2) builds a Map like formData and calls postFormAction to submit an
add (fields like title, issuer, year), assert postStatus is 200/303, (3) query
via prisma (e.g., prisma.candidate_certificate.findFirst or findMany) to assert
the new certificate exists for candidate.candidate_id, (4) submit the
corresponding remove action (using the remove form/action ref or certificate id)
via postFormAction, assert success, and (5) verify via prisma that the
certificate was deleted; integrate this sequence inside
candidateProfileUpdateTest and ensure cleanup/restoration so the test leaves DB
state unchanged.
🪄 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: 0b520556-ed2d-43a9-88b9-644ae11da4d8
📒 Files selected for processing (15)
prisma/schema.prismascripts/workflow-test.mjssrc/app/candidate/edit/page.tsxsrc/app/staff/interviews/[id]/page.tsxsrc/app/staff/interviews/page.tsxsrc/app/styles.csssrc/middleware.tssrc/modules/auth/capabilities.tssrc/modules/auth/types.tssrc/modules/candidates/CandidateEditForm.tsxsrc/modules/candidates/actions.tssrc/modules/requests/interview-actions.tssrc/modules/workspace/WorkspaceOS.tsxsrc/modules/workspace/data.tssrc/modules/workspace/navigation.ts
Comment on lines
+522
to
+537
| const { status: afterStatus, text: afterHtml } = await getPage(detailPath, cookie); | ||
| assert(afterStatus === 200, `Request detail page returned ${afterStatus} (after suggestion)`); | ||
| assert(afterHtml.includes(reason) || afterHtml.includes("suggestion"), | ||
| "Suggestion text not visible on reloaded request detail page"); | ||
|
|
||
| const record = await prisma.suggestion.findFirst({ | ||
| where: { request_uuid: request.request_uuid, candidate_id: candidate.candidate_id, suggestion_status: 1 }, | ||
| orderBy: { suggestion_datetime: "desc" }, | ||
| select: { suggestion_uuid: true, note_uuid: true }, | ||
| }); | ||
| if (record) { | ||
| await prisma.note.updateMany({ where: { suggestion_uuid: record.suggestion_uuid }, data: { suggestion_uuid: null } }); | ||
| await prisma.story.updateMany({ where: { suggestion_uuid: record.suggestion_uuid }, data: { suggestion_uuid: null } }); | ||
| await prisma.suggestion.delete({ where: { suggestion_uuid: record.suggestion_uuid } }); | ||
| await prisma.note.deleteMany({ where: { note_uuid: record.note_uuid } }); | ||
| } |
There was a problem hiding this comment.
This check can pass even when no new suggestion was created or rendered.
Line 524 accepts any page containing the word "suggestion", and Lines 527-537 never assert that a new row exists. That makes this test largely vacuous for the workflow it claims to cover.
Tighten the assertion
const { status: afterStatus, text: afterHtml } = await getPage(detailPath, cookie);
assert(afterStatus === 200, `Request detail page returned ${afterStatus} (after suggestion)`);
- assert(afterHtml.includes(reason) || afterHtml.includes("suggestion"),
- "Suggestion text not visible on reloaded request detail page");
+ assert(afterHtml.includes(reason),
+ "New suggestion reason not visible on reloaded request detail page");
const record = await prisma.suggestion.findFirst({
where: { request_uuid: request.request_uuid, candidate_id: candidate.candidate_id, suggestion_status: 1 },
orderBy: { suggestion_datetime: "desc" },
select: { suggestion_uuid: true, note_uuid: true },
});
- if (record) {
+ assert(record, "Suggestion was not created in the database");
+ if (record) {🤖 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 `@scripts/workflow-test.mjs` around lines 522 - 537, The test currently allows
any page containing the word "suggestion" and never asserts a DB row was
created; tighten it by asserting the page includes the exact suggestion text
(use reason and/or the created suggestion_uuid) instead of the generic
"suggestion", then assert that prisma.suggestion.findFirst(...) returns a
non-null record (e.g. throw/assert if record is null) before running cleanup;
reference getPage(detailPath, cookie), reason, prisma.suggestion.findFirst, and
record.suggestion_uuid to locate and verify the created suggestion.
Comment on lines
+139
to
+141
| if (!interviewUuid || !Number.isInteger(status)) { | ||
| redirect(`${basePath}?notice=missing-fields` as Route); | ||
| } |
There was a problem hiding this comment.
Restrict status to supported values before persisting.
Number.isInteger(status) still allows unsupported states (e.g., 7), which can corrupt interview status semantics.
Suggested fix
- if (!interviewUuid || !Number.isInteger(status)) {
+ const allowedStatuses = new Set([0, 1, 2]);
+ if (!interviewUuid || !allowedStatuses.has(status)) {
redirect(`${basePath}?notice=missing-fields` as Route);
}Also applies to: 154-155
🤖 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/requests/interview-actions.ts` around lines 139 - 141, The
current guard uses Number.isInteger(status) which permits unsupported numeric
codes (e.g., 7); replace this with a whitelist check against the allowed
interview status values (e.g., an InterviewStatus enum or a supportedStatuses
array) so that you validate that status is one of those permitted values before
persisting; update both occurrences (the block checking interviewUuid/status
around the redirect and the similar check at the later location) to use this
inclusion check and keep the same redirect(`${basePath}?notice=missing-fields`
as Route) when validation fails.
Comment on lines
+143
to
+149
| if (session.role === "staff") { | ||
| const owned = await prisma.request_interview.findFirst({ | ||
| where: { request_interview_uuid: interviewUuid, staff_id: Number(session.id) }, | ||
| select: { request_interview_uuid: true } | ||
| }); | ||
| if (!owned) redirect(`${basePath}?notice=not-found` as Route); | ||
| } |
There was a problem hiding this comment.
Handle missing interview for admin path to avoid unhandled update failure.
For non-staff users, no existence check is performed before update, so an invalid UUID can raise a runtime DB error instead of redirecting with not-found.
Suggested fix
- if (session.role === "staff") {
- const owned = await prisma.request_interview.findFirst({
- where: { request_interview_uuid: interviewUuid, staff_id: Number(session.id) },
- select: { request_interview_uuid: true }
- });
- if (!owned) redirect(`${basePath}?notice=not-found` as Route);
- }
+ const existing = await prisma.request_interview.findFirst({
+ where:
+ session.role === "staff"
+ ? { request_interview_uuid: interviewUuid, staff_id: Number(session.id) }
+ : { request_interview_uuid: interviewUuid },
+ select: { request_interview_uuid: true }
+ });
+ if (!existing) redirect(`${basePath}?notice=not-found` as Route);Also applies to: 152-155
🤖 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/requests/interview-actions.ts` around lines 143 - 149, Add an
existence check for the interview before calling prisma.request_interview.update
when session.role !== "staff": use prisma.request_interview.findFirst (or
findUnique) to verify the record for request_interview_uuid (same UUID used in
the update) and if not found perform redirect(`${basePath}?notice=not-found`)
like the staff branch does; apply the same check/redirect logic around the
update path referenced in the block that includes
prisma.request_interview.update (also update the duplicate logic noted at the
second occurrence around lines 152-155) so no update is attempted for an invalid
UUID.
Comment on lines
+229
to
+230
| const rows = await prisma.request_interview.findMany({ | ||
| where: { staff_id: staffId }, |
There was a problem hiding this comment.
Interview ownership filter is too narrow and can hide valid staff interviews.
Line 230 and Line 258 only scope by request_interview.staff_id. In this same module, interview ownership is also treated as request.staff_id (Line 498), so list/detail can miss valid records and deny access inconsistently.
Suggested fix
export async function getStaffInterviewRows(staffId: number) {
const rows = await prisma.request_interview.findMany({
- where: { staff_id: staffId },
+ where: {
+ OR: [{ staff_id: staffId }, { request: { staff_id: staffId } }]
+ },
orderBy: { interview_at: "desc" },
take: 60,
@@
export async function getStaffInterviewDetail(interviewUuid: string, staffId: number) {
const interview = await prisma.request_interview.findFirst({
- where: { request_interview_uuid: interviewUuid, staff_id: staffId },
+ where: {
+ request_interview_uuid: interviewUuid,
+ OR: [{ staff_id: staffId }, { request: { staff_id: staffId } }]
+ },
select: {Also applies to: 258-258
🤖 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/workspace/data.ts` around lines 229 - 230, The query using
prisma.request_interview.findMany (the const rows retrieval) filters only on
request_interview.staff_id and misses interviews where ownership is recorded on
the related request (request.staff_id); update the where clause to include both
ownership possibilities—either add an OR that checks request_interview.staff_id
=== staffId OR the related request.staff_id === staffId (use Prisma relation
filtering on request), and apply the same change to the other occurrence around
the prisma.findMany at the noted spot so list/detail access checks match the
request.staff_id usage seen at request.staff_id (Line ~498).
| title: item.company_candidate_certificate_company_idTocompany?.company_name ?? item.store?.store_name ?? "Certificate", | ||
| subtitle: item.certificate_type ? "Experience certificate" : "Certificate", | ||
| title: item.certificate_title ?? item.company_candidate_certificate_company_idTocompany?.company_name ?? item.store?.store_name ?? "Certificate", | ||
| subtitle: item.certificate_issuer ?? (item.certificate_type ? "Experience certificate" : "Certificate"), |
There was a problem hiding this comment.
Certificate subtitle fallback can mislabel certificate type.
Line 1199 uses a truthy check and hardcodes "Experience certificate", which can show the wrong label for non-experience types (or falsy numeric type values).
Suggested fix
- subtitle: item.certificate_issuer ?? (item.certificate_type ? "Experience certificate" : "Certificate"),
+ subtitle:
+ item.certificate_issuer ??
+ (item.certificate_type != null ? String(item.certificate_type) : "Certificate"),📝 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
| subtitle: item.certificate_issuer ?? (item.certificate_type ? "Experience certificate" : "Certificate"), | |
| subtitle: | |
| item.certificate_issuer ?? | |
| (item.certificate_type != null ? String(item.certificate_type) : "Certificate"), |
🤖 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/workspace/data.ts` at line 1199, The subtitle computation uses a
truthy check that treats any truthy certificate_type as "Experience certificate"
and mislabels other types or numeric falsy values; update the subtitle
assignment (where subtitle is set using item.certificate_issuer and
item.certificate_type) to perform an explicit check or mapping for known
certificate types (e.g., certificate_type === 'experience' or a type-to-label
map) and otherwise fall back to item.certificate_issuer || "Certificate" so
non-experience types and falsy numeric values are labeled correctly.
Owner
Author
DevRel ReviewStatus: Comment — scope and process issues to address Title/branch mismatchThe PR title says Broad scope — 15 files changedThis PR touches certificates, interviews, mobile navigation, middleware, CSS, workflow tests, capabilities, and workspace navigation all at once. This makes review harder and increases risk. Consider whether some of these changes could be split into separate, focused PRs:
CodeRabbit findingsCodeRabbit posted 7 actionable comments — please review and address the ones that are still valid. VercelPreview deploy status is unknown — verify the build before merge. Next step: Clarify the STU ticket number, consider splitting if practical, and address CodeRabbit findings. |
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 `@src/app/styles.css`:
- Line 5964: Replace the fixed bottom padding declaration "padding: 10px 10px
76px;" with a safe-area aware bottom padding that adds the device inset (use
env(safe-area-inset-bottom) with a constant(...) fallback) so the content bottom
uses calculated value instead of a hard 76px; update the same pattern at the
other occurrences noted (the other "padding: ... 76px" instances) and ensure you
keep the existing top/side paddings unchanged while only changing the bottom
padding expression.
🪄 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: 9bc3e76d-9d7c-479f-a286-12e606cccd42
📒 Files selected for processing (4)
src/app/styles.csssrc/modules/workspace/WorkspaceNavigation.tsxsrc/modules/workspace/WorkspaceShell.tsxsrc/modules/workspace/navigation.ts
✅ Files skipped from review due to trivial changes (1)
- src/modules/workspace/WorkspaceShell.tsx
|
|
||
| .workspaceStage { | ||
| width: auto; | ||
| padding: 10px 10px 76px; |
There was a problem hiding this comment.
Make content bottom padding safe-area aware to prevent clipped last items.
The tab bar accounts for safe-area-inset-bottom, but these content containers use fixed bottom padding. On iOS devices with larger insets, the final rows can sit under the bar.
Proposed fix
`@media` (max-width: 768px) {
.workspaceStage {
width: auto;
- padding: 10px 10px 76px;
+ padding: 10px 10px calc(76px + env(safe-area-inset-bottom, 0px));
}
}
`@media` (max-width: 680px) {
.hubMain {
gap: 12px;
- padding: 10px 10px 76px;
+ padding: 10px 10px calc(76px + env(safe-area-inset-bottom, 0px));
}
.journeyHome {
- padding: 10px 10px 76px;
+ padding: 10px 10px calc(76px + env(safe-area-inset-bottom, 0px));
}
.studentOSMain {
- padding: 10px 10px 76px;
+ padding: 10px 10px calc(76px + env(safe-area-inset-bottom, 0px));
}
}
`@media` (max-width: 480px) {
.workspaceStage {
- padding: 8px 8px 72px;
+ padding: 8px 8px calc(72px + env(safe-area-inset-bottom, 0px));
}
}Also applies to: 6022-6022, 6076-6076, 6117-6117, 9309-9309
🤖 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` at line 5964, Replace the fixed bottom padding
declaration "padding: 10px 10px 76px;" with a safe-area aware bottom padding
that adds the device inset (use env(safe-area-inset-bottom) with a constant(...)
fallback) so the content bottom uses calculated value instead of a hard 76px;
update the same pattern at the other occurrences noted (the other "padding: ...
76px" instances) and ensure you keep the existing top/side paddings unchanged
while only changing the bottom padding expression.
- Wrap candidateProfileUpdateTest DB mutations in try/finally so cleanup always runs even on assertion failure - Add certificate CRUD workflow test with DB row verification - Tighten suggestion test assertion with explicit note_uuid check - Replace undefined --fg CSS variable with --foreground in candidateMissingFields hover rule - Use strict z.enum(["true", "false"]) for certificate_type coercion - Replace Number.isInteger(status) with explicit whitelist (2, 3) in updateInterviewAction - Interview existence check before update already present (no change needed) Tests: added certificate CRUD workflow test, tightened suggestion assertion 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 `@scripts/workflow-test.mjs`:
- Around line 165-190: The extractActionRefsByField function only detects keys
starting with "$ACTION_REF_" and its inputRegex only matches self-closing
<input/> tags, causing forms that use "$ACTION_ID_" or non-self-closing <input>
to be ignored; update the function to accept both marker prefixes (check
key.startsWith("$ACTION_REF_") || key.startsWith("$ACTION_ID_")) and broaden the
inputRegex to match <input ...> with or without a trailing slash (e.g.
/<input\b[^>]*>/g) so the existing hidden-field parsing loop captures all hidden
inputs and reuses decodeHtml/value logic to populate and return the fields Map.
🪄 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: 03e5e556-ba8a-467d-bbd4-5c2a14f350b9
📒 Files selected for processing (4)
scripts/workflow-test.mjssrc/app/styles.csssrc/modules/candidates/actions.tssrc/modules/requests/interview-actions.ts
Comment on lines
165
to
190
| @@ -117,18 +184,12 @@ function extractActionRefs(html, className) { | |||
| } | |||
| } | |||
|
|
|||
| // Must have at least the action reference marker | |||
| for (const key of fields.keys()) { | |||
| if (key.startsWith("$ACTION_REF_")) return fields; | |||
| } | |||
|
|
|||
| return null; | |||
There was a problem hiding this comment.
Accept both action marker variants and reuse the existing hidden-field parser.
This helper only returns when it sees $ACTION_REF_, and its regex ignores non-self-closing <input> tags. A valid form that renders $ACTION_ID_ or serializes inputs as <input ...> will be treated as missing, so the new certificate workflow can fail even when the form is present.
Suggested fix
function extractActionRefsByField(html, fieldName) {
const fieldPos = html.indexOf(`name="${fieldName}"`);
if (fieldPos === -1) return null;
const searchStart = html.lastIndexOf("<form", fieldPos);
if (searchStart === -1) return null;
const formEnd = html.indexOf("</form>", fieldPos);
if (formEnd === -1) return null;
const rawForm = html.substring(searchStart, formEnd);
-
- const fields = new Map();
- const inputRegex = /<input[^>]*\/>/g;
- let m;
- while ((m = inputRegex.exec(rawForm)) !== null) {
- const tag = m[0];
- if (!tag.includes('type="hidden"')) continue;
- const nameMatch = tag.match(/name="([^"]+)"/);
- const valueMatch = tag.match(/value="([^"]*)"/);
- if (nameMatch) {
- fields.set(nameMatch[1], valueMatch ? decodeHtml(valueMatch[1]) : "");
- }
- }
-
- for (const key of fields.keys()) {
- if (key.startsWith("$ACTION_REF_")) return fields;
- }
- return null;
+ const fields = extractHiddenFields(rawForm);
+ return hasActionMarker(fields) ? fields : null;
}🤖 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 `@scripts/workflow-test.mjs` around lines 165 - 190, The
extractActionRefsByField function only detects keys starting with "$ACTION_REF_"
and its inputRegex only matches self-closing <input/> tags, causing forms that
use "$ACTION_ID_" or non-self-closing <input> to be ignored; update the function
to accept both marker prefixes (check key.startsWith("$ACTION_REF_") ||
key.startsWith("$ACTION_ID_")) and broaden the inputRegex to match <input ...>
with or without a trailing slash (e.g. /<input\b[^>]*>/g) so the existing
hidden-field parsing loop captures all hidden inputs and reuses decodeHtml/value
logic to populate and return the fields Map.
Resolved conflicts in styles.css (mobile tab bar), actions.ts (strict certificate_type enum), and navigation.ts (icons in nav entries), keeping the STU-135/STU-170 changes in all cases. Co-Authored-By: Paperclip <noreply@paperclip.ing>
Taking main's actions.ts (which includes approveIdRequest/rejectIdRequest) and re-applying the STU-170 strict certificate_type enum fix. Also pulling in main's WorkLogStaffActions and ID request page for consistency. Co-Authored-By: Paperclip <noreply@paperclip.ing>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… [STU-135] Co-Authored-By: Paperclip <noreply@paperclip.ing>
…ds on PR #36 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Keep STU-101 branch's stricter z.enum validation for certificate_type and preserve certificate_title/certificate_issuer fields. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
/staff/interviewslist page with DataTable showing scheduled interviews/staff/interviews/[id]detail page with status management actions (Complete/Cancel/Reset)getStaffInterviewRows()andgetStaffInterviewDetail()data functionsupdateInterviewStatusAction()server actionG Ikeyboard shortcutTest plan
/staff/interviewsand verify the interview list loadsG Ikeyboard chord navigates to the interviews pagenpx tsc --noEmitpasses with zero errorsCo-Authored-By: Claude Opus 4.7 noreply@anthropic.com
Summary by CodeRabbit
New Features
Improvements
Tests