feat: add interview validation schemas for creation, updating, and querying#103
Merged
feat: add interview validation schemas for creation, updating, and querying#103
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an interviews domain (DB schema and migrations), server APIs (CRUD + send-invitation), validation and email utilities, client composables and UI (list, detail, scheduler, templates), permission updates, and removes the legacy AppSidebar component. Changes
Sequence DiagramsequenceDiagram
autonumber
actor User
participant UI as Browser (Client)
participant API as Server API
participant DB as Database
participant Mail as Email Service
User->>UI: Open scheduler & submit interview form
UI->>API: POST /api/interviews (create)
activate API
API->>DB: Verify application & insert interview
DB-->>API: Interview created
API->>DB: Insert activity_log
DB-->>API: Activity recorded
API-->>UI: 201 Created (interview)
deactivate API
User->>UI: Request send-invitation
UI->>API: POST /api/interviews/{id}/send-invitation
activate API
API->>DB: Fetch interview + application + candidate + job + org
DB-->>API: Interview & related data
API->>Mail: sendInterviewInvitationEmail(rendered subject/body)
Mail-->>API: Sent result
API->>DB: Update interview.invitation_sent_at & log activity
DB-->>API: Updated & logged
API-->>UI: 200 OK (invitation sent)
deactivate API
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (3 warnings)
✏️ 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 |
|
🚅 Deployed to the reqcore-pr-103 environment in applirank
|
…ting functionalities
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (2)
server/utils/schemas/interview.ts (2)
51-56: Consider stronger typing for status transitions.
INTERVIEW_STATUS_TRANSITIONSusesRecord<string, string[]>, which allows any string key. A stricter type would catch typos at compile time and ensure all statuses are covered:♻️ Suggested type-safe improvement
+type InterviewStatus = typeof interviewStatuses[number] + /** Allowed status transitions for interviews */ -export const INTERVIEW_STATUS_TRANSITIONS: Record<string, string[]> = { +export const INTERVIEW_STATUS_TRANSITIONS: Record<InterviewStatus, InterviewStatus[]> = { scheduled: ['completed', 'cancelled', 'no_show'], completed: [], cancelled: ['scheduled'], no_show: ['scheduled'], -} +} satisfies Record<InterviewStatus, InterviewStatus[]>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/utils/schemas/interview.ts` around lines 51 - 56, INTERVIEW_STATUS_TRANSITIONS is typed as Record<string, string[]> which permits arbitrary keys; define a string-union type (e.g., InterviewStatus = 'scheduled' | 'completed' | 'cancelled' | 'no_show') and change the map to a stricter type such as Record<InterviewStatus, InterviewStatus[]> or a mapped type to enforce only valid statuses as keys and ensure all statuses are present; also consider using a readonly/as const approach for the value object to preserve literal types.
41-42: Minor: Add error messages for datetime filters for consistency.Lines 15 and 27 provide custom error messages for datetime validation, but
fromandtofilters do not. Consider adding messages for consistency.♻️ Suggested fix
- from: z.string().datetime().optional(), - to: z.string().datetime().optional(), + from: z.string().datetime({ message: 'Valid ISO 8601 datetime required' }).optional(), + to: z.string().datetime({ message: 'Valid ISO 8601 datetime required' }).optional(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/utils/schemas/interview.ts` around lines 41 - 42, The datetime filters "from" and "to" use z.string().datetime().optional() without custom error messages; update those two validators (the "from" and "to" schema entries) to include a custom message by calling .datetime({ message: '...' }) so they match the existing datetime validators on lines 15 and 27 and maintain consistent validation error wording.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/components/InterviewScheduleSidebar.vue`:
- Around line 36-38: The helper toDateString uses toISOString which converts to
UTC and will shift days for non-UTC users; update the toDateString(Date)
implementation to build a local YYYY-MM-DD string using d.getFullYear(),
(d.getMonth() + 1) and d.getDate(), zero-padding month and day to two digits so
the function returns the local calendar date (e.g., YYYY-MM-DD) consistently for
use in default "tomorrow" and calendar isToday/isPast logic.
In `@app/composables/useInterviews.ts`:
- Around line 34-40: The current useInterviews() always uses the static
'interviews' useFetch key causing different filter combinations to clobber each
other's cache; change the useFetch key inside useInterviews to include a
deterministic fingerprint of the active filters (e.g., serialize applicationId,
jobId, status, from, to, limit) so each unique filter set gets its own cache
key, and ensure cache keys are stable across refs/getters; update any cache
invalidation in useInterview() (the refreshNuxtData('interviews') calls) to
either call refreshNuxtData with the new dynamic key or implement a shared
prefix strategy (e.g., 'interviews:' + filterHash) and use refreshNuxtData on
that prefix or emit an event to refresh the specific useInterviews instance.
In `@app/pages/dashboard/interviews.vue`:
- Around line 40-43: The statusCounts logic is using the already-filtered
interviews (from useInterviews called with activeStatus) so other buckets show 0
when a filter is applied; change it to compute counts from an unfiltered source
instead: either call useInterviews without the status filter (e.g., another hook
instance like allInterviews) or request counts from the backend (add
includeCounts flag or a getInterviewCounts API) and use that result for
statusCounts, leaving interviews/total/fetchStatus/error/refresh from the
filtered useInterviews intact; update references to statusCounts to use the new
unfiltered counts source.
- Around line 178-185: openEdit assigns editForm.date using
toISOString().slice(0,10) which yields a UTC date and can mismatch the local
editForm.time built from local hours; replace that line by computing a local
YYYY-MM-DD string using Date's local getters (getFullYear(), getMonth() + 1,
getDate()) and pad month/day to 2 digits so editForm.date and editForm.time are
both in the same local timezone; update the function openEdit (and ensure it
still sets editingInterview.value, editForm.title, editForm.type,
editForm.status) using those local getters to avoid day shifts when saving.
- Around line 285-299: menuRef is used as a single HTMLElement but in this
component it’s populated by a v-for so menuRef.value is an array; update
handleClickOutsideMenu to iterate over menuRef.value (or normalize it to an
array) and call .contains on each element (if none contain the event target, set
openMenuId.value = null), and ensure the same change is applied for the calendar
view instance; adjust any null checks accordingly and keep toggleMenu, onMounted
and onUnmounted behavior unchanged.
In `@app/pages/dashboard/jobs/`[id]/index.vue:
- Around line 335-345: In handleInterviewScheduled, don’t base the status
transition on currentSummary; use the focused card stored in
interviewTargetApplication instead: check interviewTargetApplication.value
exists and read its status (instead of currentSummary.value.status), compute
allowed = APPLICATION_STATUS_TRANSITIONS[thatStatus] ?? [], and call
changeStatus('interview') only when allowed.includes('interview'); also clear
showInterviewSidebar and interviewTargetApplication as you already do and guard
against null/undefined interviewTargetApplication.value before accessing .status
to avoid runtime errors.
In `@server/api/interviews/`[id]/index.patch.ts:
- Around line 43-47: The update call that does const [updated] = await
db.update(interview).set(updateData).where(and(eq(interview.id, id),
eq(interview.organizationId, orgId))).returning() can yield an empty array if
the row was deleted concurrently, causing updated to be undefined and returning
null with 200; add a defensive check immediately after that assignment to detect
when updated is falsy and respond with an appropriate error (e.g.,
throw/notFound or return 404) so the client receives a proper failure; apply the
same defensive check to the other update-returning block referenced around the
later lines (the other .returning() that assigns to [updated] on lines 62-63) to
handle concurrent deletes consistently.
In `@server/api/interviews/index.get.ts`:
- Around line 16-17: The jobId condition eq(application.jobId, query.jobId) is
being appended to whereClause used by db.$count(interview, whereClause) which
counts from interview only and fails because application isn't joined; fix by
splitting the logic: keep the current whereClause (used by the data query that
explicitly joins application) but create a separate countWhere (or change the
count query) so the count either (A) performs the same join to application
before applying eq(application.jobId, query.jobId) or (B) omits
application-specific predicates and uses only interview fields for db.$count;
apply the same pattern to both places where total is computed so both data and
count use compatible filters/joining.
In `@server/database/migrations/0010_glamorous_mattie_franklin.sql`:
- Line 10: The column definition for scheduled_at currently uses plain
timestamp; change it to timestamptz by updating the migration line to define
scheduled_at as timestamptz NOT NULL (e.g., "scheduled_at timestamptz NOT
NULL"), and ensure any accompanying down/rollback SQL or subsequent migrations
are updated to match; also scan for and update any test fixtures or seed INSERTs
that assume a timezone-less value so they supply timezone-aware ISO timestamps
to preserve the instant.
---
Nitpick comments:
In `@server/utils/schemas/interview.ts`:
- Around line 51-56: INTERVIEW_STATUS_TRANSITIONS is typed as Record<string,
string[]> which permits arbitrary keys; define a string-union type (e.g.,
InterviewStatus = 'scheduled' | 'completed' | 'cancelled' | 'no_show') and
change the map to a stricter type such as Record<InterviewStatus,
InterviewStatus[]> or a mapped type to enforce only valid statuses as keys and
ensure all statuses are present; also consider using a readonly/as const
approach for the value object to preserve literal types.
- Around line 41-42: The datetime filters "from" and "to" use
z.string().datetime().optional() without custom error messages; update those two
validators (the "from" and "to" schema entries) to include a custom message by
calling .datetime({ message: '...' }) so they match the existing datetime
validators on lines 15 and 27 and maintain consistent validation error wording.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6c1c6dbe-fabe-496e-8d23-049920e6d337
📒 Files selected for processing (17)
app/components/AppSidebar.vueapp/components/AppTopBar.vueapp/components/InterviewScheduleSidebar.vueapp/composables/useInterviews.tsapp/pages/dashboard/interviews.vueapp/pages/dashboard/jobs/[id]/index.vueserver/api/interviews/[id]/index.delete.tsserver/api/interviews/[id]/index.get.tsserver/api/interviews/[id]/index.patch.tsserver/api/interviews/index.get.tsserver/api/interviews/index.post.tsserver/database/migrations/0010_glamorous_mattie_franklin.sqlserver/database/migrations/meta/0010_snapshot.jsonserver/database/migrations/meta/_journal.jsonserver/database/schema/app.tsserver/utils/schemas/interview.tsshared/permissions.ts
💤 Files with no reviewable changes (1)
- app/components/AppSidebar.vue
Comment on lines
+40
to
+43
| const { interviews, total, status: fetchStatus, error, refresh } = useInterviews({ | ||
| status: activeStatus, | ||
| limit: 100, | ||
| }) |
There was a problem hiding this comment.
Status cards stop being truthful after a filter is selected.
Once activeStatus is set, the fetch only returns that status, but statusCounts is computed from those already-filtered rows. That turns every other bucket into 0 even when interviews still exist.
Also applies to: 302-308
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/pages/dashboard/interviews.vue` around lines 40 - 43, The statusCounts
logic is using the already-filtered interviews (from useInterviews called with
activeStatus) so other buckets show 0 when a filter is applied; change it to
compute counts from an unfiltered source instead: either call useInterviews
without the status filter (e.g., another hook instance like allInterviews) or
request counts from the backend (add includeCounts flag or a getInterviewCounts
API) and use that result for statusCounts, leaving
interviews/total/fetchStatus/error/refresh from the filtered useInterviews
intact; update references to statusCounts to use the new unfiltered counts
source.
Comment on lines
+178
to
+185
| function openEdit(interviewItem: typeof interviews.value[number]) { | ||
| editingInterview.value = interviewItem | ||
| const d = new Date(interviewItem.scheduledAt) | ||
| editForm.title = interviewItem.title | ||
| editForm.type = interviewItem.type | ||
| editForm.status = interviewItem.status | ||
| editForm.date = d.toISOString().slice(0, 10) | ||
| editForm.time = `${String(d.getHours()).padStart(2, '0')}:${String(d.getMinutes()).padStart(2, '0')}` |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does new Date(value).toISOString().slice(0, 10)preserve the user's local calendar date when filling an HTML?
💡 Result:
No—not reliably.
Date.prototype.toISOString()always outputs UTC (ends withZ). So slicing the first 10 chars gives the UTC calendar day, not the user’s local day. If yourDaterepresents local midnight (or any local time near midnight), converting to UTC can cross a day boundary and you can end up setting the<input type="date">to yesterday/tomorrow for some time zones. [1]- An
<input type="date">value must be a date-only string inyyyy-mm-dd, and the value string itself carries no timezone (it’s just a calendar date). [2][3] - Extra gotcha:
new Date("YYYY-MM-DD")(a date-only ISO string like what you get from<input type="date">) is commonly specified/implemented as UTC midnight, not local midnight—so it can already be “shifted” when you later interpret it in local time. [4]
Safer approaches
- If you need a
yyyy-mm-ddfor the user’s local date, format from local getters (getFullYear(),getMonth()+1,getDate()) instead oftoISOString(). - If you’re interacting with
<input type="date">, consider usinginput.valueas the canonical date string, orinput.valueAsDate(noting it uses a UTC-based instant for these controls). [5]
Sources:
[1] MDN: Date.prototype.toISOString() is always UTC (Z) https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/toISOString
[2] MDN: <input type="date"> value format yyyy-mm-dd https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/input/date
[3] MDN Guide: date/time strings include no timezone; presumed local https://developer.mozilla.org/en-US/docs/Web/HTML/Guides/Date_and_time_formats
[4] Discussion of YYYY-MM-DD parsing as UTC in ECMAScript practice https://stackoverflow.com/questions/2587345/why-does-date-parse-give-incorrect-results
[5] MDN: HTMLInputElement.valueAsDate for date-like inputs returns UTC midnight instant https://developer.mozilla.org/en-US/docs/Web/API/HTMLInputElement/valueAsDate
Editing can shift the interview onto the wrong day.
toISOString().slice(0, 10) converts the stored instant to UTC, but the time field is filled with local hours/minutes. This causes a day mismatch, particularly for interviews near midnight in the local timezone. Saving without changes can move the schedule to the wrong date.
To fix, use local date getters (getFullYear(), getMonth() + 1, getDate()) instead of converting to ISO string.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/pages/dashboard/interviews.vue` around lines 178 - 185, openEdit assigns
editForm.date using toISOString().slice(0,10) which yields a UTC date and can
mismatch the local editForm.time built from local hours; replace that line by
computing a local YYYY-MM-DD string using Date's local getters (getFullYear(),
getMonth() + 1, getDate()) and pad month/day to 2 digits so editForm.date and
editForm.time are both in the same local timezone; update the function openEdit
(and ensure it still sets editingInterview.value, editForm.title, editForm.type,
editForm.status) using those local getters to avoid day shifts when saving.
Comment on lines
+285
to
+299
| const openMenuId = ref<string | null>(null) | ||
| const menuRef = ref<HTMLElement | null>(null) | ||
|
|
||
| function toggleMenu(id: string) { | ||
| openMenuId.value = openMenuId.value === id ? null : id | ||
| } | ||
|
|
||
| function handleClickOutsideMenu(e: MouseEvent) { | ||
| if (menuRef.value && !menuRef.value.contains(e.target as Node)) { | ||
| openMenuId.value = null | ||
| } | ||
| } | ||
|
|
||
| onMounted(() => document.addEventListener('click', handleClickOutsideMenu)) | ||
| onUnmounted(() => document.removeEventListener('click', handleClickOutsideMenu)) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Vue 3, what does a template ref contain when the same ref name is used inside v-for?
💡 Result:
When you use the same template ref name inside a v-for in Vue 3 (e.g. ref="itemRefs"), that ref resolves to an array containing the corresponding DOM elements or component instances created by the loop (populated after render/mount and updated on re-renders). The array’s order is not guaranteed to match the source list order. [1]
Source(s)
[1] Vue docs — Template Refs → Refs inside v-for (vuejs.org)
Citations:
🏁 Script executed:
cat -n app/pages/dashboard/interviews.vue | head -350 | tail -100Repository: reqcore-inc/reqcore
Length of output: 129
🏁 Script executed:
fd -name "interviews.vue" -type fRepository: reqcore-inc/reqcore
Length of output: 293
🏁 Script executed:
fd "interviews.vue" -type fRepository: reqcore-inc/reqcore
Length of output: 233
🏁 Script executed:
fd "interviews.vue"Repository: reqcore-inc/reqcore
Length of output: 45
🏁 Script executed:
git ls-files | head -50Repository: reqcore-inc/reqcore
Length of output: 1412
🏁 Script executed:
find app/pages -name "*.vue" | head -20Repository: reqcore-inc/reqcore
Length of output: 792
🏁 Script executed:
find app -name "*interview*" -type fRepository: reqcore-inc/reqcore
Length of output: 45
🏁 Script executed:
rg "toggleMenu" -A 5 app/Repository: reqcore-inc/reqcore
Length of output: 954
🏁 Script executed:
wc -l app/pages/dashboard/interviews/index.vueRepository: reqcore-inc/reqcore
Length of output: 107
🏁 Script executed:
sed -n '285,299p' app/pages/dashboard/interviews/index.vueRepository: reqcore-inc/reqcore
Length of output: 556
🏁 Script executed:
rg "ref=\"menuRef\"" app/pages/dashboard/interviews/index.vue -B 5 -A 5Repository: reqcore-inc/reqcore
Length of output: 654
🏁 Script executed:
sed -n '200,350p' app/pages/dashboard/interviews/index.vue | cat -nRepository: reqcore-inc/reqcore
Length of output: 6524
🏁 Script executed:
rg "v-for.*interviewItem" app/pages/dashboard/interviews/index.vue -B 2 -A 30Repository: reqcore-inc/reqcore
Length of output: 5790
🏁 Script executed:
sed -n '400,550p' app/pages/dashboard/interviews/index.vue | cat -nRepository: reqcore-inc/reqcore
Length of output: 9897
🏁 Script executed:
sed -n '500,530p' app/pages/dashboard/interviews/index.vue | cat -nRepository: reqcore-inc/reqcore
Length of output: 1835
🏁 Script executed:
sed -n '505,520p' app/pages/dashboard/interviews/index.vue | cat -nRepository: reqcore-inc/reqcore
Length of output: 1147
menuRef is incorrectly used as a single element when the ref is attached inside v-for.
In Vue 3, when the same template ref name is used inside a v-for loop, Vue populates the ref as an array of DOM elements, not a single element. The code calls .contains() on menuRef.value, which will fail because .contains() is not a method on arrays. This occurs in the list view (line 518) and also in the calendar view (line 512).
Suggested fix
-const menuRef = ref<HTMLElement | null>(null)
-
function handleClickOutsideMenu(e: MouseEvent) {
- if (menuRef.value && !menuRef.value.contains(e.target as Node)) {
+ const target = e.target as HTMLElement | null
+ if (!target?.closest('[data-interview-menu]')) {
openMenuId.value = null
}
}- <div ref="menuRef" class="relative">
+ <div data-interview-menu class="relative">Update both occurrences in the list view and calendar view.
📝 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
| const openMenuId = ref<string | null>(null) | |
| const menuRef = ref<HTMLElement | null>(null) | |
| function toggleMenu(id: string) { | |
| openMenuId.value = openMenuId.value === id ? null : id | |
| } | |
| function handleClickOutsideMenu(e: MouseEvent) { | |
| if (menuRef.value && !menuRef.value.contains(e.target as Node)) { | |
| openMenuId.value = null | |
| } | |
| } | |
| onMounted(() => document.addEventListener('click', handleClickOutsideMenu)) | |
| onUnmounted(() => document.removeEventListener('click', handleClickOutsideMenu)) | |
| const openMenuId = ref<string | null>(null) | |
| function toggleMenu(id: string) { | |
| openMenuId.value = openMenuId.value === id ? null : id | |
| } | |
| function handleClickOutsideMenu(e: MouseEvent) { | |
| const target = e.target as HTMLElement | null | |
| if (!target?.closest('[data-interview-menu]')) { | |
| openMenuId.value = null | |
| } | |
| } | |
| onMounted(() => document.addEventListener('click', handleClickOutsideMenu)) | |
| onUnmounted(() => document.removeEventListener('click', handleClickOutsideMenu)) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/pages/dashboard/interviews.vue` around lines 285 - 299, menuRef is used
as a single HTMLElement but in this component it’s populated by a v-for so
menuRef.value is an array; update handleClickOutsideMenu to iterate over
menuRef.value (or normalize it to an array) and call .contains on each element
(if none contain the event target, set openMenuId.value = null), and ensure the
same change is applied for the calendar view instance; adjust any null checks
accordingly and keep toggleMenu, onMounted and onUnmounted behavior unchanged.
Comment on lines
+43
to
+47
| const [updated] = await db | ||
| .update(interview) | ||
| .set(updateData) | ||
| .where(and(eq(interview.id, id), eq(interview.organizationId, orgId))) | ||
| .returning() |
There was a problem hiding this comment.
Consider handling the case where the update returns no rows.
If another request deletes the interview between the existence check (line 13) and the update (line 43), the .returning() call returns an empty array, and updated will be undefined. The endpoint would then return undefined (serialized as null) with a 200 status instead of a proper error.
🛡️ Suggested defensive check
const [updated] = await db
.update(interview)
.set(updateData)
.where(and(eq(interview.id, id), eq(interview.organizationId, orgId)))
.returning()
+ if (!updated) {
+ throw createError({ statusCode: 404, statusMessage: 'Interview not found' })
+ }
+
recordActivity({Also applies to: 62-63
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/api/interviews/`[id]/index.patch.ts around lines 43 - 47, The update
call that does const [updated] = await
db.update(interview).set(updateData).where(and(eq(interview.id, id),
eq(interview.organizationId, orgId))).returning() can yield an empty array if
the row was deleted concurrently, causing updated to be undefined and returning
null with 200; add a defensive check immediately after that assignment to detect
when updated is falsy and respond with an appropriate error (e.g.,
throw/notFound or return 404) so the client receives a proper failure; apply the
same defensive check to the other update-returning block referenced around the
later lines (the other .returning() that assigns to [updated] on lines 62-63) to
handle concurrent deletes consistently.
| "title" text NOT NULL, | ||
| "type" "interview_type" DEFAULT 'video' NOT NULL, | ||
| "status" "interview_status" DEFAULT 'scheduled' NOT NULL, | ||
| "scheduled_at" timestamp NOT NULL, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the difference between PostgreSQL timestampandtimestamptz when storing ISO 8601 datetimes with timezone offsets?
💡 Result:
-
timestamp(timestamp without time zone) stores only a calendar date + wall-clock time. If you insert an ISO 8601 value that includes an offset (e.g.,2026-03-10T12:00:00-05:00), PostgreSQL silently ignores the offset and stores2026-03-10 12:00:00as-is. The value has no time-zone/offset meaning unless your application supplies one. [1] -
timestamptz(timestamp with time zone) stores a point in time (an instant). If you insert an ISO 8601 value with an offset, PostgreSQL uses that offset to convert the value to UTC for storage. On retrieval/display, it renders the instant in the sessionTimeZone, so the shown offset/time can change depending on the session setting. Importantly, it does not preserve the original textual offset you inserted—only the instant. [1]
Practical rule: use timestamptz for real-world events (“the meeting happened at this instant”); use timestamp for “floating” local times (“store opens at 09:00 every day regardless of time zone”). [2]
Sources:
[1] PostgreSQL docs: Date/Time Types (timestamp vs timestamptz behavior on input/output) — https://www.postgresql.org/docs/current/datatype-datetime.html
[2] PostgreSQL Wiki “Don’t Do This” (date/time storage guidance) — https://wiki.postgresql.org/wiki/Don't_Do_This
Use timestamptz for scheduled_at.
Interview scheduling is a real-world event (a specific point in time), not a floating local time. With timestamp (without timezone), PostgreSQL silently ignores any timezone offset in ISO 8601 input from your API, storing only the bare date and time. This breaks cross-timezone scheduling: the application loses the original instant and cannot reliably determine when the interview actually occurs. Use timestamptz instead, which preserves the timestamp as a UTC instant.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/database/migrations/0010_glamorous_mattie_franklin.sql` at line 10,
The column definition for scheduled_at currently uses plain timestamp; change it
to timestamptz by updating the migration line to define scheduled_at as
timestamptz NOT NULL (e.g., "scheduled_at timestamptz NOT NULL"), and ensure any
accompanying down/rollback SQL or subsequent migrations are updated to match;
also scan for and update any test fixtures or seed INSERTs that assume a
timezone-less value so they supply timezone-aware ISO timestamps to preserve the
instant.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
app/pages/dashboard/interviews/[id].vue (2)
77-90: Consider extracting shared interview configuration to a composable.
INTERVIEW_STATUS_TRANSITIONS,statusConfig,typeIcons,typeLabels, and date formatting helpers are duplicated between this file andindex.vue. Extract these to a shared composable (e.g.,useInterviewConfig) or utility module to reduce maintenance burden and ensure consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/pages/dashboard/interviews/`[id].vue around lines 77 - 90, The duplicated interview configuration (INTERVIEW_STATUS_TRANSITIONS, transitionClasses, statusConfig, typeIcons, typeLabels and date formatting helpers) should be moved into a shared composable or utility (e.g., create useInterviewConfig) and imported into both [id].vue and index.vue; implement a new composable that exports these constants and helper functions (and any TypeScript types if needed), replace local definitions in the components with imports from useInterviewConfig, update usages (e.g., references to INTERVIEW_STATUS_TRANSITIONS, transitionClasses, statusConfig, typeIcons, typeLabels and the date format helpers) to use the shared exports, and run tests/build to ensure no breaking references remain.
536-600: Modals lack focus management and keyboard accessibility.The Teleport-based modals (reschedule, edit, delete) don't implement:
- Focus trapping (users can tab out of the modal)
- Escape key to close
aria-modal="true"andaria-labelledbyattributes- Initial focus on first interactive element
Consider using a shared modal component or a library like HeadlessUI's
Dialogto handle these accessibility requirements consistently across all three modals.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/pages/dashboard/interviews/`[id].vue around lines 536 - 600, The Teleport-based reschedule modal (controlled by showReschedule, with form handler handleReschedule and form state rescheduleForm/rescheduleError) lacks focus management and keyboard accessibility; update it (and the edit/delete modals) to use an accessible modal pattern — either replace the Teleport wrapper with a shared Modal/Dialog component (or HeadlessUI Dialog) that implements focus trapping, sets aria-modal="true" and aria-labelledby, moves initial focus to the first interactive element (the date input with id "reschedule-date"), and closes on Escape and backdrop click while restoring focus to the triggering element; ensure the modal root includes aria-labelledby pointing to the h3 and that the submit button remains disabled via isRescheduling while accessibility behaviors are delegated to the shared component so all three modals gain consistent keyboard/focus support.app/pages/dashboard/interviews/index.vue (2)
271-282: Consider adding loading state to prevent duplicate status change requests.
quickStatusChangedoesn't disable buttons during the API call, allowing rapid clicks to trigger multiple requests. Consider tracking loading state per interview or disabling the button while the request is in flight.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/pages/dashboard/interviews/index.vue` around lines 271 - 282, quickStatusChange allows multiple rapid PATCH requests because it doesn't track in-flight state; add a per-interview loading tracker (e.g., a Set or Map named statusChangingIds) and update quickStatusChange to immediately return if interviewItem.id is already tracked, add the id to statusChangingIds before the $fetch call and remove it in a finally block (so it always clears on success/error), and update the template buttons that invoke quickStatusChange to be disabled when the id is present in statusChangingIds; reference the quickStatusChange function, interviews value, refresh, and handlePreviewReadOnlyError when making these changes.
301-308: Status counts reflect filtered data, not totals.When
activeStatusis set,useInterviewsreturns only matching interviews, sostatusCountswill show zeros for non-selected statuses. This may confuse users expecting the summary cards to show overall distribution.Consider fetching total counts separately or fetching all interviews regardless of filter for the summary (using search-only filtering on the client).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/pages/dashboard/interviews/index.vue` around lines 301 - 308, statusCounts is computed from interviews (returned by useInterviews) which are already filtered by activeStatus, so the summary cards show only filtered counts; modify statusCounts to use an unfiltered source instead: either call useInterviews (or the underlying fetch) without the status filter to obtain all interviews and compute totals there, or add a new API/getAllInterviewCounts method to fetch aggregate counts and use that for the summary; update references to interviews.value in the statusCounts computed (or replace statusCounts with a call to the new counts data) so that statusCounts reflects overall distribution while search-only filtering remains client-side.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/pages/dashboard/interviews/`[id].vue:
- Line 256: The variable router is unused—remove the unused declaration "const
router = useRouter()" and, if present, also remove the unused import of the
useRouter composable; keep using navigateTo where navigation is already
implemented (references: router, useRouter, navigateTo).
- Around line 177-179: The pre-fill uses toISOString().slice(0,10) which yields
the UTC date while the time uses local getHours()/getMinutes(), causing a
potential off-by-one-day; change the date assignment for rescheduleForm (where
interview.value.scheduledAt is parsed into d) to build a local date string from
d.getFullYear(), d.getMonth()+1 and d.getDate() with zero-padding (so date and
time are both local), e.g., replace the toISOString() usage with a local
YYYY-MM-DD formatter that pads month/day before assigning rescheduleForm.date
while keeping rescheduleForm.time from d.getHours()/getMinutes().
In `@app/pages/dashboard/interviews/index.vue`:
- Around line 697-706: The status select currently lists all STATUS_OPTIONS
which allows invalid transitions against INTERVIEW_STATUS_TRANSITIONS; change it
to only show allowed next statuses by deriving a new computed/derived array
(e.g., allowedStatusOptions) that uses the interview's current status (or
editForm.originalStatus/currentStatus) to lookup
INTERVIEW_STATUS_TRANSITIONS[currentStatus] and filter STATUS_OPTIONS
accordingly, then replace v-for="s in STATUS_OPTIONS" with v-for="s in
allowedStatusOptions"; if admin override is intended instead, add a short
comment/docs and an explicit override flag (e.g., allowStatusOverride) so the
template can show all options only when that flag is true.
- Around line 517-523: The template uses a single ref menuRef inside a v-for so
it only points to the last menu wrapper, breaking handleClickOutsideMenu and
toggleMenu for other items; change the implementation to track all menu elements
(e.g., use a template ref array or collect wrappers via
document.querySelectorAll) and update handleClickOutsideMenu to test if the
click target is inside any open menu element (or inside the element matching the
current interview id), and update toggleMenu/interview open state logic to use
the correct ref by interviewItem.id or the ref array index so click-outside
detection works for every menu.
- Around line 26-32: The debounce timer started in the watch on searchInput
(debounceTimer assigned via setTimeout) isn't cleared when the component
unmounts; add an onUnmounted hook that calls clearTimeout(debounceTimer) (ensure
debounceTimer is initialized to a compatible type such as ReturnType<typeof
setTimeout> | undefined) so the pending callback that updates debouncedSearch
doesn't run after unmounting—alternatively replace this manual watch logic with
VueUse's refDebounced(searchInput, 250) and remove debounceTimer/watch
accordingly.
---
Nitpick comments:
In `@app/pages/dashboard/interviews/`[id].vue:
- Around line 77-90: The duplicated interview configuration
(INTERVIEW_STATUS_TRANSITIONS, transitionClasses, statusConfig, typeIcons,
typeLabels and date formatting helpers) should be moved into a shared composable
or utility (e.g., create useInterviewConfig) and imported into both [id].vue and
index.vue; implement a new composable that exports these constants and helper
functions (and any TypeScript types if needed), replace local definitions in the
components with imports from useInterviewConfig, update usages (e.g., references
to INTERVIEW_STATUS_TRANSITIONS, transitionClasses, statusConfig, typeIcons,
typeLabels and the date format helpers) to use the shared exports, and run
tests/build to ensure no breaking references remain.
- Around line 536-600: The Teleport-based reschedule modal (controlled by
showReschedule, with form handler handleReschedule and form state
rescheduleForm/rescheduleError) lacks focus management and keyboard
accessibility; update it (and the edit/delete modals) to use an accessible modal
pattern — either replace the Teleport wrapper with a shared Modal/Dialog
component (or HeadlessUI Dialog) that implements focus trapping, sets
aria-modal="true" and aria-labelledby, moves initial focus to the first
interactive element (the date input with id "reschedule-date"), and closes on
Escape and backdrop click while restoring focus to the triggering element;
ensure the modal root includes aria-labelledby pointing to the h3 and that the
submit button remains disabled via isRescheduling while accessibility behaviors
are delegated to the shared component so all three modals gain consistent
keyboard/focus support.
In `@app/pages/dashboard/interviews/index.vue`:
- Around line 271-282: quickStatusChange allows multiple rapid PATCH requests
because it doesn't track in-flight state; add a per-interview loading tracker
(e.g., a Set or Map named statusChangingIds) and update quickStatusChange to
immediately return if interviewItem.id is already tracked, add the id to
statusChangingIds before the $fetch call and remove it in a finally block (so it
always clears on success/error), and update the template buttons that invoke
quickStatusChange to be disabled when the id is present in statusChangingIds;
reference the quickStatusChange function, interviews value, refresh, and
handlePreviewReadOnlyError when making these changes.
- Around line 301-308: statusCounts is computed from interviews (returned by
useInterviews) which are already filtered by activeStatus, so the summary cards
show only filtered counts; modify statusCounts to use an unfiltered source
instead: either call useInterviews (or the underlying fetch) without the status
filter to obtain all interviews and compute totals there, or add a new
API/getAllInterviewCounts method to fetch aggregate counts and use that for the
summary; update references to interviews.value in the statusCounts computed (or
replace statusCounts with a call to the new counts data) so that statusCounts
reflects overall distribution while search-only filtering remains client-side.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: facdedf3-24df-42d2-818a-7e8303beb790
📒 Files selected for processing (2)
app/pages/dashboard/interviews/[id].vueapp/pages/dashboard/interviews/index.vue
Comment on lines
+177
to
+179
| const d = new Date(interview.value.scheduledAt) | ||
| rescheduleForm.date = d.toISOString().slice(0, 10) | ||
| rescheduleForm.time = `${String(d.getHours()).padStart(2, '0')}:${String(d.getMinutes()).padStart(2, '0')}` |
There was a problem hiding this comment.
Timezone mismatch when pre-filling reschedule form.
toISOString().slice(0, 10) returns the UTC date while getHours()/getMinutes() return local time. For users whose local date differs from UTC (e.g., late evening in US timezones), the pre-filled date will be off by one day.
🛠️ Proposed fix using local date components
function openReschedule() {
if (!interview.value) return
const d = new Date(interview.value.scheduledAt)
- rescheduleForm.date = d.toISOString().slice(0, 10)
+ const year = d.getFullYear()
+ const month = String(d.getMonth() + 1).padStart(2, '0')
+ const day = String(d.getDate()).padStart(2, '0')
+ rescheduleForm.date = `${year}-${month}-${day}`
rescheduleForm.time = `${String(d.getHours()).padStart(2, '0')}:${String(d.getMinutes()).padStart(2, '0')}`📝 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
| const d = new Date(interview.value.scheduledAt) | |
| rescheduleForm.date = d.toISOString().slice(0, 10) | |
| rescheduleForm.time = `${String(d.getHours()).padStart(2, '0')}:${String(d.getMinutes()).padStart(2, '0')}` | |
| const d = new Date(interview.value.scheduledAt) | |
| const year = d.getFullYear() | |
| const month = String(d.getMonth() + 1).padStart(2, '0') | |
| const day = String(d.getDate()).padStart(2, '0') | |
| rescheduleForm.date = `${year}-${month}-${day}` | |
| rescheduleForm.time = `${String(d.getHours()).padStart(2, '0')}:${String(d.getMinutes()).padStart(2, '0')}` |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/pages/dashboard/interviews/`[id].vue around lines 177 - 179, The pre-fill
uses toISOString().slice(0,10) which yields the UTC date while the time uses
local getHours()/getMinutes(), causing a potential off-by-one-day; change the
date assignment for rescheduleForm (where interview.value.scheduledAt is parsed
into d) to build a local date string from d.getFullYear(), d.getMonth()+1 and
d.getDate() with zero-padding (so date and time are both local), e.g., replace
the toISOString() usage with a local YYYY-MM-DD formatter that pads month/day
before assigning rescheduleForm.date while keeping rescheduleForm.time from
d.getHours()/getMinutes().
Comment on lines
+26
to
+32
| let debounceTimer: ReturnType<typeof setTimeout> | ||
| watch(searchInput, (val) => { | ||
| clearTimeout(debounceTimer) | ||
| debounceTimer = setTimeout(() => { | ||
| debouncedSearch.value = val.trim().toLowerCase() | ||
| }, 250) | ||
| }) |
There was a problem hiding this comment.
Debounce timer not cleared on component unmount.
If the component unmounts before the 250ms timer fires, the callback will still execute. Clear the timer in onUnmounted to prevent potential issues.
🛠️ Proposed fix
let debounceTimer: ReturnType<typeof setTimeout>
watch(searchInput, (val) => {
clearTimeout(debounceTimer)
debounceTimer = setTimeout(() => {
debouncedSearch.value = val.trim().toLowerCase()
}, 250)
})
+
+onUnmounted(() => {
+ clearTimeout(debounceTimer)
+})Alternatively, consider using VueUse's refDebounced for cleaner implementation:
import { refDebounced } from '@vueuse/core'
const searchInput = ref('')
const debouncedSearch = refDebounced(searchInput, 250)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/pages/dashboard/interviews/index.vue` around lines 26 - 32, The debounce
timer started in the watch on searchInput (debounceTimer assigned via
setTimeout) isn't cleared when the component unmounts; add an onUnmounted hook
that calls clearTimeout(debounceTimer) (ensure debounceTimer is initialized to a
compatible type such as ReturnType<typeof setTimeout> | undefined) so the
pending callback that updates debouncedSearch doesn't run after
unmounting—alternatively replace this manual watch logic with VueUse's
refDebounced(searchInput, 250) and remove debounceTimer/watch accordingly.
Comment on lines
+517
to
+523
| <div ref="menuRef" class="relative"> | ||
| <button | ||
| class="flex items-center justify-center rounded-lg border border-surface-200 dark:border-surface-700/80 p-1.5 text-surface-400 hover:text-surface-600 hover:bg-surface-50 dark:hover:text-surface-300 dark:hover:bg-surface-800 transition-all cursor-pointer" | ||
| @click.stop="toggleMenu(interviewItem.id)" | ||
| > | ||
| <MoreHorizontal class="size-4" /> | ||
| </button> |
There was a problem hiding this comment.
Single ref used inside v-for only captures the last element.
menuRef is bound inside a v-for loop, so it will only reference the last rendered menu wrapper. The click-outside detection in handleClickOutsideMenu may not work correctly for menus other than the last one in the list.
🛠️ Proposed fix using template refs array or simplified logic
Since only one menu can be open at a time, you can simplify by checking if the click target is within any open menu:
-const menuRef = ref<HTMLElement | null>(null)
+const menuRefs = ref<Map<string, HTMLElement>>(new Map())
+
+function setMenuRef(id: string, el: HTMLElement | null) {
+ if (el) menuRefs.value.set(id, el)
+ else menuRefs.value.delete(id)
+}
function handleClickOutsideMenu(e: MouseEvent) {
- if (menuRef.value && !menuRef.value.contains(e.target as Node)) {
+ if (openMenuId.value) {
+ const menuEl = menuRefs.value.get(openMenuId.value)
+ if (menuEl && !menuEl.contains(e.target as Node)) {
+ openMenuId.value = null
+ }
+ }
- openMenuId.value = null
- }
}Then in template:
-<div ref="menuRef" class="relative">
+<div :ref="(el) => setMenuRef(interviewItem.id, el)" class="relative">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/pages/dashboard/interviews/index.vue` around lines 517 - 523, The
template uses a single ref menuRef inside a v-for so it only points to the last
menu wrapper, breaking handleClickOutsideMenu and toggleMenu for other items;
change the implementation to track all menu elements (e.g., use a template ref
array or collect wrappers via document.querySelectorAll) and update
handleClickOutsideMenu to test if the click target is inside any open menu
element (or inside the element matching the current interview id), and update
toggleMenu/interview open state logic to use the correct ref by interviewItem.id
or the ref array index so click-outside detection works for every menu.
- Introduced `createEmailTemplateSchema` and `updateEmailTemplateSchema` for validating email template creation and updates. - Added `sendInterviewInvitationSchema` to ensure either a template ID or both custom subject and body are provided. - Defined a list of allowed placeholder variables for email templates. - Created a set of pre-made system templates for interview invitations with various styles.
- Implemented a new dashboard for managing email templates, including listing, creating, editing, and deleting templates. - Added a dedicated page for viewing and editing individual email templates. - Introduced a new page for creating new email templates with a live preview feature. - Created a utility for system templates and available variables for dynamic content in emails. - Enhanced user experience with loading states, error handling, and confirmation dialogs for deletions.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (4)
app/pages/dashboard/interviews/index.vue (3)
287-297:⚠️ Potential issue | 🔴 CriticalDon't treat a
v-forref as one menu element.Line 525 binds
menuRefonce per row, but Line 294 calls.contains()as if there is a singleHTMLElement. In Vue this is not stable per-item, so click-outside handling becomes unreliable and can throw when the ref resolves to an array. Use a data attribute /closest()check or track refs by interview ID.Suggested fix
-const menuRef = ref<HTMLElement | null>(null) - function handleClickOutsideMenu(e: MouseEvent) { - if (menuRef.value && !menuRef.value.contains(e.target as Node)) { + const target = e.target as HTMLElement | null + if (target && !target.closest('[data-interview-menu]')) { openMenuId.value = null } }- <div ref="menuRef" class="relative"> + <div data-interview-menu class="relative">Also applies to: 525-525
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/pages/dashboard/interviews/index.vue` around lines 287 - 297, The click-outside logic assumes menuRef is a single HTMLElement but menuRef is bound inside a v-for; update handleClickOutsideMenu and the menu ref handling to track per-item elements (e.g., use a Map keyed by interview id or store refs by id like menuRefs[id]) or use event.target.closest with a data attribute on the menu element to determine if the click was inside the specific menu; adjust toggleMenu/openMenuId usage to reference the correct per-item ref (menuRefs.get(id) or querying by [data-menu-id="${openMenuId}"]) so contains() is called on a single HTMLElement and not an array/NodeList.
179-186:⚠️ Potential issue | 🟠 MajorPopulate the edit form date with local components.
Line 185 uses the UTC date from
toISOString(), but Line 186 uses local time. Editing an interview near midnight and saving it unchanged can shift the scheduled day.Suggested fix
function openEdit(interviewItem: typeof interviews.value[number]) { editingInterview.value = interviewItem const d = new Date(interviewItem.scheduledAt) editForm.title = interviewItem.title editForm.type = interviewItem.type editForm.status = interviewItem.status - editForm.date = d.toISOString().slice(0, 10) + const year = d.getFullYear() + const month = String(d.getMonth() + 1).padStart(2, '0') + const day = String(d.getDate()).padStart(2, '0') + editForm.date = `${year}-${month}-${day}` editForm.time = `${String(d.getHours()).padStart(2, '0')}:${String(d.getMinutes()).padStart(2, '0')}` editForm.duration = interviewItem.duration🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/pages/dashboard/interviews/index.vue` around lines 179 - 186, The edit form date is being set using UTC via toISOString().slice(0, 10) in openEdit, causing day shifts when scheduledAt is near midnight; replace that with local date components: read scheduledAt into a Date (d already), then set editForm.date by formatting d.getFullYear(), d.getMonth()+1, and d.getDate() with zero-padding to produce YYYY-MM-DD so the date matches the local calendar day (keep the existing time construction using d.getHours() and d.getMinutes()); update the openEdit function and ensure editForm.date and editForm.time consistently use the same local Date instance derived from interviewItem.scheduledAt.
41-44:⚠️ Potential issue | 🟠 MajorKeep the summary cards independent from the active status filter.
Lines 41-44 already ask
useInterviews()for only the selected status, then Lines 303-308 compute counts from that filtered subset. Once a filter is active, every other card drops to0even when interviews still exist. Use an unfiltered source or backend counts for the summary cards.Also applies to: 302-308
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/pages/dashboard/interviews/index.vue` around lines 41 - 44, The summary cards are being computed from the filtered interviews returned by useInterviews({ status: activeStatus, ... }) so when a filter is active counts go to zero; change the summary card data source to an unfiltered source (e.g., call useInterviews without the status to get allInterviews or call a dedicated backend counts endpoint) and compute the card counts from that unfiltered result instead of the filtered interviews variable. Specifically, add or use a separate hook/result (e.g., allInterviews or summaryCounts) and update the summary card logic (currently using interviews/total for counts around the summary cards) to use that unfiltered data while keeping the existing useInterviews(...) result for the main list and filtering behavior.app/pages/dashboard/interviews/[id].vue (1)
176-180:⚠️ Potential issue | 🟠 MajorKeep the reschedule date in local time.
Line 179 still uses
toISOString().slice(0, 10), while Line 180 uses local hours/minutes. Opening this dialog and saving without edits can move late-evening interviews onto the wrong calendar day.Suggested fix
function openReschedule() { if (!interview.value) return const d = new Date(interview.value.scheduledAt) - rescheduleForm.date = d.toISOString().slice(0, 10) + const year = d.getFullYear() + const month = String(d.getMonth() + 1).padStart(2, '0') + const day = String(d.getDate()).padStart(2, '0') + rescheduleForm.date = `${year}-${month}-${day}` rescheduleForm.time = `${String(d.getHours()).padStart(2, '0')}:${String(d.getMinutes()).padStart(2, '0')}` rescheduleForm.duration = interview.value.duration🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/pages/dashboard/interviews/`[id].vue around lines 176 - 180, openReschedule currently sets rescheduleForm.date using d.toISOString().slice(0, 10) which converts to UTC while rescheduleForm.time uses local hours/minutes; change date construction to use the Date's local components so the date and time stay in local time. In function openReschedule, replace the toISOString-based assignment for rescheduleForm.date with a local YYYY-MM-DD string built from d.getFullYear(), d.getMonth()+1, and d.getDate() (with zero-padding), keep rescheduleForm.time as is (using d.getHours() and d.getMinutes()), and ensure you update only openReschedule/rescheduleForm references so late-evening interviews retain the correct local calendar day.
🧹 Nitpick comments (7)
app/components/InterviewScheduleSidebar.vue (1)
192-195: Consider validating that the scheduled date is in the future.The form only checks for valid date/time format but doesn't prevent scheduling interviews in the past. While past dates are disabled in the calendar picker (line 372:
:disabled="d.isPast"), users could still manually enter a past date if the input allowed it, or select "today" with a time that has already passed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/InterviewScheduleSidebar.vue` around lines 192 - 195, The code validates date/time format but not that it's in the future: after constructing scheduledDate (const scheduledDate = new Date(`${form.date}T${form.time}`)) add a check comparing scheduledDate.getTime() to Date.now() and set errors.value.date = 'Date/time must be in the future' (or similar) when scheduledDate.getTime() <= Date.now(); ensure this logic lives alongside the existing isNaN check (in the InterviewScheduleSidebar.vue form validation) so past or current-past times are rejected before submission.app/utils/system-templates.ts (1)
9-86: Duplication with server-sideSYSTEM_TEMPLATES.These system templates are duplicated in
server/utils/schemas/emailTemplate.ts. If the two copies drift apart, template IDs or content will mismatch, causing the send-invitation endpoint to fail lookups. Consider extracting to a shared location (e.g.,shared/directory) that both client and server can import.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/utils/system-templates.ts` around lines 9 - 86, The client and server each define a duplicated constant named SYSTEM_TEMPLATES (the one in system-templates.ts and the one in the server email template schema), which risks drift and lookup failures; fix this by extracting the canonical SYSTEM_TEMPLATES into a single shared module that both sides import, move the array (IDs, subject/body strings) into that shared export, update the client code that currently references SYSTEM_TEMPLATES and the server emailTemplate code to import from the new shared module, and remove the old duplicate definitions so only the shared constant remains.server/api/email-templates/[id]/index.patch.ts (1)
25-31: Consider validating the update result.Unlike the POST endpoint which checks
if (!created), this PATCH endpoint doesn't verify that the update succeeded. While unlikely in practice (the existence check on lines 12-19 mitigates this), a concurrent delete between the check and update could result inupdatedbeingundefined.Suggested defensive check
const [updated] = await db.update(emailTemplate) .set({ ...body, updatedAt: new Date() }) .where(and( eq(emailTemplate.id, id), eq(emailTemplate.organizationId, orgId), )) .returning() + if (!updated) { + throw createError({ statusCode: 404, statusMessage: 'Email template not found' }) + } + recordActivity({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/api/email-templates/`[id]/index.patch.ts around lines 25 - 31, The PATCH handler currently assigns const [updated] = await db.update(emailTemplate)... but never checks if updated is undefined; add a defensive validation after the update to handle the rare race where the record was deleted between the existence check and the update (similar to the POST path's if (!created) check). Specifically, after the db.update/returning call that assigns updated, verify updated is truthy and return an appropriate 404 or error response if not; reference the db.update call, the emailTemplate model, and the updated variable when making the change.server/api/interviews/[id]/send-invitation.post.ts (1)
104-114: Hardcodeden-USlocale for date/time formatting.The date and time formatting uses hardcoded
en-USlocale. If the application needs to support i18n for email content based on organization or candidate preferences, this would need to be configurable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/api/interviews/`[id]/send-invitation.post.ts around lines 104 - 114, The code hardcodes 'en-US' in scheduledAt.toLocaleDateString and toLocaleTimeString (producing interviewDate and interviewTime); make the locale configurable by resolving the user/organization locale (e.g., prefer organization.locale or candidate.locale, then fall back to a service default or request header) and pass that locale into the formatting calls (or use Intl.DateTimeFormat with the resolved locale) so the email content respects i18n preferences.app/pages/dashboard/interviews/templates/new.vue (1)
73-73: Type assertion(created as any).idbypasses type safety.The
createTemplatefunction returns the created record from the server, but the return type isn't properly inferred. Consider typing the return value ofcreateTemplateinuseEmailTemplates.tsto avoid theas anycast.Type the return value in useEmailTemplates.ts
async function createTemplate(payload: { name: string subject: string body: string - }) { + }): Promise<EmailTemplate> { try { - const created = await $fetch('/api/email-templates', { + const created = await $fetch<EmailTemplate>('/api/email-templates', { method: 'POST', body: payload, })Then in
new.vue:- await navigateTo(localePath(`/dashboard/interviews/templates/${(created as any).id}`)) + await navigateTo(localePath(`/dashboard/interviews/templates/${created.id}`))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/pages/dashboard/interviews/templates/new.vue` at line 73, The code is using a type assertion (created as any).id which bypasses TypeScript safety; update the createTemplate return type in useEmailTemplates.ts to a properly typed interface (e.g., EmailTemplate or CreateTemplateResponse) so callers get a typed object, then change the usage in new.vue to use the typed created.id directly (no any cast). Locate the createTemplate function in useEmailTemplates.ts, export or import the corresponding model/interface and annotate its Promise return type, and update references such as created, navigateTo and localePath to rely on the new typed created.id.app/composables/useEmailTemplates.ts (1)
24-40: Consider typing$fetchcalls for better type inference.The mutation methods return implicitly typed results. Adding explicit types would improve DX and eliminate the need for
as anycasts in consumers.Add explicit types to $fetch calls
async function createTemplate(payload: { name: string subject: string body: string - }) { + }): Promise<EmailTemplate> { try { - const created = await $fetch('/api/email-templates', { + const created = await $fetch<EmailTemplate>('/api/email-templates', { method: 'POST', body: payload, }) await refresh() return created } catch (error) { handlePreviewReadOnlyError(error) throw error } } async function updateTemplate(id: string, payload: Partial<{ name: string subject: string body: string - }>) { + }>): Promise<EmailTemplate> { try { - const updated = await $fetch(`/api/email-templates/${id}`, { + const updated = await $fetch<EmailTemplate>(`/api/email-templates/${id}`, { method: 'PATCH', body: payload, })Also applies to: 42-58
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/composables/useEmailTemplates.ts` around lines 24 - 40, The $fetch call in createTemplate is untyped which yields implicit any results; give the API call an explicit generic return type and add proper TypeScript types to the function signature: specify the payload interface (already present) and annotate createTemplate's return type as the expected response type (e.g., ApiEmailTemplate or similar) and call $fetch as await $fetch<ApiEmailTemplate>('/api/email-templates', ...). Apply the same pattern to the other mutation methods (e.g., updateTemplate, deleteTemplate) so consumers don't need to cast to any and get full type inference.app/components/InterviewEmailModal.vue (1)
20-95: Read built-in templates and variables from the shared registry.This component hard-codes the same built-ins that
app/utils/system-templates.ts:1-86already owns. The next edit to a system template or variable list will make this modal disagree with the template management pages. PullSYSTEM_TEMPLATESandAVAILABLE_VARIABLESfrom the shared module instead of maintaining a second copy.Also applies to: 230-241
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/InterviewEmailModal.vue` around lines 20 - 95, The modal currently defines a local constant systemTemplates duplicating built-in templates/variables; instead import the shared SYSTEM_TEMPLATES and AVAILABLE_VARIABLES from the central module and use those instead of the local systemTemplates/variable list, remove the local hard-coded definition(s) (including the duplicate block later that mirrors lines 230-241), and update any references in this component to use SYSTEM_TEMPLATES and AVAILABLE_VARIABLES so templates stay in sync with the shared registry.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/components/InterviewEmailModal.vue`:
- Around line 151-155: Remove the local renderPreview function from
InterviewEmailModal.vue and replace its usages with the shared
renderTemplatePreview utility from the system templates utils; import
renderTemplatePreview and call it with the same arguments (pass the template
string and the previewVariables object currently referenced by renderPreview),
then delete the local renderPreview definition so template rendering is
delegated to renderTemplatePreview for consistency.
In `@app/components/InterviewScheduleSidebar.vue`:
- Around line 218-228: The catch block swallowing errors after the $fetch to
/api/interviews/${created.id}/send-invitation causes silent failures when
sendInvitationAfter.value is true; update the catch to surface a user-facing
warning (e.g., call the app's toast/notification helper) that the invitation
email failed and include the error message/details and a pointer to the
interview detail page so the user can resend; reference the sendInvitationAfter
value, the created?.id context, and the $fetch POST to
/api/interviews/${created.id}/send-invitation when implementing the
toast/notification in the catch block.
In `@app/pages/dashboard/interviews/templates/`[id].vue:
- Around line 12-18: templateId is captured once during setup causing stale
state after handleDuplicate navigates to a new id; change templateId from a
static const to a computed that returns route.params.id (useRoute()) so
isSystemTemplate, systemTemplate and customTemplate derive from a reactive id,
then add a watcher on that computed templateId to reset hasLoaded (and any other
per-template flags) when it changes so the form reloads for the new template.
In `@app/pages/dashboard/interviews/templates/index.vue`:
- Around line 32-43: The delete-sheet is being closed in finally so failures
still clear state; change handleDelete to only clear templateToDelete.value and
hide showDeleteConfirm.value when deleteTemplate(...) succeeds: keep
deletingId.value set before the call and ensure deletingId.value is cleared
after the attempt (both success and failure), but move the
templateToDelete.value = null and showDeleteConfirm.value = false into the
success path (inside the try after await deleteTemplate(...)); in the catch call
handlePreviewReadOnlyError(err) and do not reset templateToDelete or close the
sheet so retries and error display work as expected.
---
Duplicate comments:
In `@app/pages/dashboard/interviews/`[id].vue:
- Around line 176-180: openReschedule currently sets rescheduleForm.date using
d.toISOString().slice(0, 10) which converts to UTC while rescheduleForm.time
uses local hours/minutes; change date construction to use the Date's local
components so the date and time stay in local time. In function openReschedule,
replace the toISOString-based assignment for rescheduleForm.date with a local
YYYY-MM-DD string built from d.getFullYear(), d.getMonth()+1, and d.getDate()
(with zero-padding), keep rescheduleForm.time as is (using d.getHours() and
d.getMinutes()), and ensure you update only openReschedule/rescheduleForm
references so late-evening interviews retain the correct local calendar day.
In `@app/pages/dashboard/interviews/index.vue`:
- Around line 287-297: The click-outside logic assumes menuRef is a single
HTMLElement but menuRef is bound inside a v-for; update handleClickOutsideMenu
and the menu ref handling to track per-item elements (e.g., use a Map keyed by
interview id or store refs by id like menuRefs[id]) or use event.target.closest
with a data attribute on the menu element to determine if the click was inside
the specific menu; adjust toggleMenu/openMenuId usage to reference the correct
per-item ref (menuRefs.get(id) or querying by [data-menu-id="${openMenuId}"]) so
contains() is called on a single HTMLElement and not an array/NodeList.
- Around line 179-186: The edit form date is being set using UTC via
toISOString().slice(0, 10) in openEdit, causing day shifts when scheduledAt is
near midnight; replace that with local date components: read scheduledAt into a
Date (d already), then set editForm.date by formatting d.getFullYear(),
d.getMonth()+1, and d.getDate() with zero-padding to produce YYYY-MM-DD so the
date matches the local calendar day (keep the existing time construction using
d.getHours() and d.getMinutes()); update the openEdit function and ensure
editForm.date and editForm.time consistently use the same local Date instance
derived from interviewItem.scheduledAt.
- Around line 41-44: The summary cards are being computed from the filtered
interviews returned by useInterviews({ status: activeStatus, ... }) so when a
filter is active counts go to zero; change the summary card data source to an
unfiltered source (e.g., call useInterviews without the status to get
allInterviews or call a dedicated backend counts endpoint) and compute the card
counts from that unfiltered result instead of the filtered interviews variable.
Specifically, add or use a separate hook/result (e.g., allInterviews or
summaryCounts) and update the summary card logic (currently using
interviews/total for counts around the summary cards) to use that unfiltered
data while keeping the existing useInterviews(...) result for the main list and
filtering behavior.
---
Nitpick comments:
In `@app/components/InterviewEmailModal.vue`:
- Around line 20-95: The modal currently defines a local constant
systemTemplates duplicating built-in templates/variables; instead import the
shared SYSTEM_TEMPLATES and AVAILABLE_VARIABLES from the central module and use
those instead of the local systemTemplates/variable list, remove the local
hard-coded definition(s) (including the duplicate block later that mirrors lines
230-241), and update any references in this component to use SYSTEM_TEMPLATES
and AVAILABLE_VARIABLES so templates stay in sync with the shared registry.
In `@app/components/InterviewScheduleSidebar.vue`:
- Around line 192-195: The code validates date/time format but not that it's in
the future: after constructing scheduledDate (const scheduledDate = new
Date(`${form.date}T${form.time}`)) add a check comparing scheduledDate.getTime()
to Date.now() and set errors.value.date = 'Date/time must be in the future' (or
similar) when scheduledDate.getTime() <= Date.now(); ensure this logic lives
alongside the existing isNaN check (in the InterviewScheduleSidebar.vue form
validation) so past or current-past times are rejected before submission.
In `@app/composables/useEmailTemplates.ts`:
- Around line 24-40: The $fetch call in createTemplate is untyped which yields
implicit any results; give the API call an explicit generic return type and add
proper TypeScript types to the function signature: specify the payload interface
(already present) and annotate createTemplate's return type as the expected
response type (e.g., ApiEmailTemplate or similar) and call $fetch as await
$fetch<ApiEmailTemplate>('/api/email-templates', ...). Apply the same pattern to
the other mutation methods (e.g., updateTemplate, deleteTemplate) so consumers
don't need to cast to any and get full type inference.
In `@app/pages/dashboard/interviews/templates/new.vue`:
- Line 73: The code is using a type assertion (created as any).id which bypasses
TypeScript safety; update the createTemplate return type in useEmailTemplates.ts
to a properly typed interface (e.g., EmailTemplate or CreateTemplateResponse) so
callers get a typed object, then change the usage in new.vue to use the typed
created.id directly (no any cast). Locate the createTemplate function in
useEmailTemplates.ts, export or import the corresponding model/interface and
annotate its Promise return type, and update references such as created,
navigateTo and localePath to rely on the new typed created.id.
In `@app/utils/system-templates.ts`:
- Around line 9-86: The client and server each define a duplicated constant
named SYSTEM_TEMPLATES (the one in system-templates.ts and the one in the server
email template schema), which risks drift and lookup failures; fix this by
extracting the canonical SYSTEM_TEMPLATES into a single shared module that both
sides import, move the array (IDs, subject/body strings) into that shared
export, update the client code that currently references SYSTEM_TEMPLATES and
the server emailTemplate code to import from the new shared module, and remove
the old duplicate definitions so only the shared constant remains.
In `@server/api/email-templates/`[id]/index.patch.ts:
- Around line 25-31: The PATCH handler currently assigns const [updated] = await
db.update(emailTemplate)... but never checks if updated is undefined; add a
defensive validation after the update to handle the rare race where the record
was deleted between the existence check and the update (similar to the POST
path's if (!created) check). Specifically, after the db.update/returning call
that assigns updated, verify updated is truthy and return an appropriate 404 or
error response if not; reference the db.update call, the emailTemplate model,
and the updated variable when making the change.
In `@server/api/interviews/`[id]/send-invitation.post.ts:
- Around line 104-114: The code hardcodes 'en-US' in
scheduledAt.toLocaleDateString and toLocaleTimeString (producing interviewDate
and interviewTime); make the locale configurable by resolving the
user/organization locale (e.g., prefer organization.locale or candidate.locale,
then fall back to a service default or request header) and pass that locale into
the formatting calls (or use Intl.DateTimeFormat with the resolved locale) so
the email content respects i18n preferences.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f23f51fe-d56a-4e45-aeb6-3d817583acbc
📒 Files selected for processing (24)
app/components/InterviewEmailModal.vueapp/components/InterviewScheduleSidebar.vueapp/composables/useEmailTemplates.tsapp/composables/useInterviews.tsapp/pages/dashboard/interviews/[id].vueapp/pages/dashboard/interviews/index.vueapp/pages/dashboard/interviews/templates/[id].vueapp/pages/dashboard/interviews/templates/index.vueapp/pages/dashboard/interviews/templates/new.vueapp/utils/system-templates.tsserver/api/email-templates/[id]/index.delete.tsserver/api/email-templates/[id]/index.patch.tsserver/api/email-templates/index.get.tsserver/api/email-templates/index.post.tsserver/api/interviews/[id]/index.get.tsserver/api/interviews/[id]/send-invitation.post.tsserver/api/interviews/index.get.tsserver/database/migrations/0011_oval_enchantress.sqlserver/database/migrations/meta/0011_snapshot.jsonserver/database/migrations/meta/_journal.jsonserver/database/schema/app.tsserver/utils/email.tsserver/utils/schemas/emailTemplate.tsshared/permissions.ts
✅ Files skipped from review due to trivial changes (1)
- server/database/migrations/meta/0011_snapshot.json
🚧 Files skipped from review as they are similar to previous changes (2)
- server/api/interviews/index.get.ts
- server/database/migrations/meta/_journal.json
Comment on lines
+218
to
+228
| if (sendInvitationAfter.value && created?.id) { | ||
| try { | ||
| await $fetch(`/api/interviews/${created.id}/send-invitation`, { | ||
| method: 'POST', | ||
| body: { templateId: 'system-standard' }, | ||
| }) | ||
| } catch { | ||
| // Interview was created successfully — don't block on email failure. | ||
| // The user can always resend from the interview detail page. | ||
| } | ||
| } |
There was a problem hiding this comment.
Silent failure when sending invitation email may confuse users.
When the user checks "Send invitation email" but the invitation fails (lines 224-227), the interview is created successfully but the user receives no feedback about the email failure. Consider showing a warning toast or indicator so users know to manually resend.
💡 Suggested approach
if (sendInvitationAfter.value && created?.id) {
try {
await $fetch(`/api/interviews/${created.id}/send-invitation`, {
method: 'POST',
body: { templateId: 'system-standard' },
})
- } catch {
+ } catch (emailErr) {
// Interview was created successfully — don't block on email failure.
// The user can always resend from the interview detail page.
+ console.warn('Failed to send invitation email:', emailErr)
+ // Optionally show a toast notification here
}
}📝 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
| if (sendInvitationAfter.value && created?.id) { | |
| try { | |
| await $fetch(`/api/interviews/${created.id}/send-invitation`, { | |
| method: 'POST', | |
| body: { templateId: 'system-standard' }, | |
| }) | |
| } catch { | |
| // Interview was created successfully — don't block on email failure. | |
| // The user can always resend from the interview detail page. | |
| } | |
| } | |
| if (sendInvitationAfter.value && created?.id) { | |
| try { | |
| await $fetch(`/api/interviews/${created.id}/send-invitation`, { | |
| method: 'POST', | |
| body: { templateId: 'system-standard' }, | |
| }) | |
| } catch (emailErr) { | |
| // Interview was created successfully — don't block on email failure. | |
| // The user can always resend from the interview detail page. | |
| console.warn('Failed to send invitation email:', emailErr) | |
| // Optionally show a toast notification here | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/components/InterviewScheduleSidebar.vue` around lines 218 - 228, The
catch block swallowing errors after the $fetch to
/api/interviews/${created.id}/send-invitation causes silent failures when
sendInvitationAfter.value is true; update the catch to surface a user-facing
warning (e.g., call the app's toast/notification helper) that the invitation
email failed and include the error message/details and a pointer to the
interview detail page so the user can resend; reference the sendInvitationAfter
value, the created?.id context, and the $fetch POST to
/api/interviews/${created.id}/send-invitation when implementing the
toast/notification in the catch block.
Comment on lines
+12
to
+18
| const route = useRoute() | ||
| const localePath = useLocalePath() | ||
| const templateId = route.params.id as string | ||
| const { handlePreviewReadOnlyError } = usePreviewReadOnly() | ||
|
|
||
| const isSystemTemplate = computed(() => templateId.startsWith('system-')) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f '\[id\]\.vue' app/pagesRepository: reqcore-inc/reqcore
Length of output: 276
🏁 Script executed:
head -n 40 app/pages/dashboard/interviews/templates/\[id\].vueRepository: reqcore-inc/reqcore
Length of output: 1230
🏁 Script executed:
wc -l app/pages/dashboard/interviews/templates/\[id\].vueRepository: reqcore-inc/reqcore
Length of output: 116
🏁 Script executed:
sed -n '50,110p' app/pages/dashboard/interviews/templates/\[id\].vueRepository: reqcore-inc/reqcore
Length of output: 1849
🏁 Script executed:
sed -n '110,180p' app/pages/dashboard/interviews/templates/\[id\].vueRepository: reqcore-inc/reqcore
Length of output: 2777
🏁 Script executed:
sed -n '1,50p' app/pages/dashboard/interviews/templates/\[id\].vue | cat -nRepository: reqcore-inc/reqcore
Length of output: 1900
🏁 Script executed:
sed -n '36,65p' app/pages/dashboard/interviews/templates/\[id\].vue | cat -nRepository: reqcore-inc/reqcore
Length of output: 1183
🏁 Script executed:
grep -n "handleDuplicate\|navigateTo" app/pages/dashboard/interviews/templates/\[id\].vueRepository: reqcore-inc/reqcore
Length of output: 308
🏁 Script executed:
grep -n "hasLoaded" app/pages/dashboard/interviews/templates/\[id\].vueRepository: reqcore-inc/reqcore
Length of output: 286
🏁 Script executed:
grep -A 5 "watch\|onBeforeRouteUpdate\|onRouteChange" app/pages/dashboard/interviews/templates/\[id\].vueRepository: reqcore-inc/reqcore
Length of output: 294
🏁 Script executed:
sed -n '50,70p' app/pages/dashboard/interviews/templates/\[id\].vue | cat -nRepository: reqcore-inc/reqcore
Length of output: 832
Make the route ID reactive to fix stale template state after duplication.
Line 14 captures route.params.id once during setup, binding isSystemTemplate, systemTemplate, and customTemplate to that snapshot. When handleDuplicate() navigates to a new [id] route at line 104, if Nuxt reuses the page instance, templateId and all dependent computeds remain bound to the original ID, leaving the form displaying stale data and hasLoaded unable to reset because it depends on template lookups that never change.
Convert templateId to a computed property reading from route.params and reset hasLoaded when it changes so the form reloads for the new template.
Suggested fix
const route = useRoute()
const localePath = useLocalePath()
-const templateId = route.params.id as string
+const templateId = computed(() => route.params.id as string)
const { handlePreviewReadOnlyError } = usePreviewReadOnly()
-const isSystemTemplate = computed(() => templateId.startsWith('system-'))
+const isSystemTemplate = computed(() => templateId.value.startsWith('system-'))
@@
const systemTemplate = computed(() =>
- SYSTEM_TEMPLATES.find(t => t.id === templateId),
+ SYSTEM_TEMPLATES.find(t => t.id === templateId.value),
)
@@
const customTemplate = computed(() =>
- templates.value?.find(t => t.id === templateId),
+ templates.value?.find(t => t.id === templateId.value),
)
@@
+watch(templateId, () => {
+ hasLoaded.value = false
+})
@@
- await updateTemplate(templateId, {
+ await updateTemplate(templateId.value, {
@@
- await deleteTemplate(templateId)
+ await deleteTemplate(templateId.value)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/pages/dashboard/interviews/templates/`[id].vue around lines 12 - 18,
templateId is captured once during setup causing stale state after
handleDuplicate navigates to a new id; change templateId from a static const to
a computed that returns route.params.id (useRoute()) so isSystemTemplate,
systemTemplate and customTemplate derive from a reactive id, then add a watcher
on that computed templateId to reset hasLoaded (and any other per-template
flags) when it changes so the form reloads for the new template.
Comment on lines
+32
to
+43
| async function handleDelete() { | ||
| if (!templateToDelete.value) return | ||
| deletingId.value = templateToDelete.value.id | ||
| try { | ||
| await deleteTemplate(templateToDelete.value.id) | ||
| } catch (err: any) { | ||
| handlePreviewReadOnlyError(err) | ||
| } finally { | ||
| deletingId.value = null | ||
| showDeleteConfirm.value = false | ||
| templateToDelete.value = null | ||
| } |
There was a problem hiding this comment.
Only close the delete sheet after a successful delete.
Because the cleanup lives in finally, any failed deleteTemplate() call still clears templateToDelete and dismisses the confirmation. That makes retries awkward and can read like success. Keep the sheet open on non-preview errors and surface the failure there instead of resetting unconditionally.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/pages/dashboard/interviews/templates/index.vue` around lines 32 - 43, The
delete-sheet is being closed in finally so failures still clear state; change
handleDelete to only clear templateToDelete.value and hide
showDeleteConfirm.value when deleteTemplate(...) succeeds: keep deletingId.value
set before the call and ensure deletingId.value is cleared after the attempt
(both success and failure), but move the templateToDelete.value = null and
showDeleteConfirm.value = false into the success path (inside the try after
await deleteTemplate(...)); in the catch call handlePreviewReadOnlyError(err)
and do not reset templateToDelete or close the sheet so retries and error
display work as expected.
There was a problem hiding this comment.
🧹 Nitpick comments (3)
app/components/InterviewScheduleSidebar.vue (3)
296-301: Consider adding an accessible label to the close button.The close button uses only an icon without an accessible name. Screen reader users may not understand its purpose.
Add aria-label
<button + aria-label="Close sidebar" class="flex items-center justify-center rounded-lg p-2 -mr-1.5 -mt-0.5 text-surface-400 hover:text-surface-600 hover:bg-surface-100 dark:text-surface-500 dark:hover:text-surface-300 dark:hover:bg-surface-800 transition-colors cursor-pointer" `@click`="emit('close')" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/InterviewScheduleSidebar.vue` around lines 296 - 301, The close button with the icon (the <button> that calls emit('close') and renders <X class="size-4" />) lacks an accessible name; add an accessible label by including aria-label="Close" (or aria-label bound to a localized string) on that button, or add visually hidden text inside the button for screen readers, and ensure the label matches any existing i18n/localization tokens used in the component.
199-212: Consider validating that the scheduled date is in the future.The UI disables past dates in the calendar, but there's no server-side validation here before submission. If the user opens the sidebar, selects a valid date, then waits past midnight before submitting, the date could become past. This is a minor edge case but worth considering.
Optional validation addition
if (isNaN(scheduledDate.getTime())) { errors.value.date = 'Invalid date/time' } + else if (scheduledDate < new Date()) { + errors.value.date = 'Cannot schedule interviews in the past' + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/InterviewScheduleSidebar.vue` around lines 199 - 212, handleSubmit currently validates presence and format but does not ensure the scheduledDate is in the future; add a check after creating scheduledDate to compare scheduledDate.getTime() against Date.now() and, if scheduledDate is <= now, set errors.value.date (or errors.value.time) to something like 'Date/time must be in the future' and early-return. Update references in the same function (handleSubmit, scheduledDate, errors.value, form.date, form.time) so the form cannot be submitted with a past datetime.
498-566: Template dropdown doesn't close on outside click.The template picker dropdown opens/closes only via button click. Clicking elsewhere on the page leaves it open, which is inconsistent with typical dropdown UX. Consider adding a click-outside handler or using a headless UI library's Listbox component.
Example approach using a click-outside directive or watcher
// Add a ref for the dropdown container const templateDropdownRef = ref<HTMLElement | null>(null) // Close on outside click function handleClickOutside(event: MouseEvent) { if (templateDropdownRef.value && !templateDropdownRef.value.contains(event.target as Node)) { showTemplateDropdown.value = false } } // Add/remove listener when dropdown opens/closes watch(showTemplateDropdown, (isOpen) => { if (isOpen) { document.addEventListener('click', handleClickOutside, true) } else { document.removeEventListener('click', handleClickOutside, true) } }) onUnmounted(() => { document.removeEventListener('click', handleClickOutside, true) })Then add
ref="templateDropdownRef"to the dropdown container element.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/InterviewScheduleSidebar.vue` around lines 498 - 566, The dropdown controlled by showTemplateDropdown doesn't close on outside clicks; add a templateDropdownRef (ref<HTMLElement|null>) for the dropdown container, implement handleClickOutside(event) that closes the dropdown when event.target is outside templateDropdownRef.value, and wire a watcher on showTemplateDropdown to add/remove the document click listener (use capture true) when open/closed; also remove the listener in onUnmounted to avoid leaks and attach ref="templateDropdownRef" to the outer dropdown div. Ensure you import/use ref, watch and onUnmounted in the component's setup and keep existing toggling behavior via the button.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/components/InterviewScheduleSidebar.vue`:
- Around line 296-301: The close button with the icon (the <button> that calls
emit('close') and renders <X class="size-4" />) lacks an accessible name; add an
accessible label by including aria-label="Close" (or aria-label bound to a
localized string) on that button, or add visually hidden text inside the button
for screen readers, and ensure the label matches any existing i18n/localization
tokens used in the component.
- Around line 199-212: handleSubmit currently validates presence and format but
does not ensure the scheduledDate is in the future; add a check after creating
scheduledDate to compare scheduledDate.getTime() against Date.now() and, if
scheduledDate is <= now, set errors.value.date (or errors.value.time) to
something like 'Date/time must be in the future' and early-return. Update
references in the same function (handleSubmit, scheduledDate, errors.value,
form.date, form.time) so the form cannot be submitted with a past datetime.
- Around line 498-566: The dropdown controlled by showTemplateDropdown doesn't
close on outside clicks; add a templateDropdownRef (ref<HTMLElement|null>) for
the dropdown container, implement handleClickOutside(event) that closes the
dropdown when event.target is outside templateDropdownRef.value, and wire a
watcher on showTemplateDropdown to add/remove the document click listener (use
capture true) when open/closed; also remove the listener in onUnmounted to avoid
leaks and attach ref="templateDropdownRef" to the outer dropdown div. Ensure you
import/use ref, watch and onUnmounted in the component's setup and keep existing
toggling behavior via the button.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0d3aac9f-395c-4426-bf62-96f20594cb81
📒 Files selected for processing (1)
app/components/InterviewScheduleSidebar.vue
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (2)
app/components/InterviewEmailModal.vue (1)
151-155: 🛠️ Refactor suggestion | 🟠 MajorRemove duplicate
renderPreviewfunction in favor of the shared utility.This duplicates the
renderTemplatePreviewfunction fromapp/utils/system-templates.ts. Using the shared utility ensures consistent template rendering across the application.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/InterviewEmailModal.vue` around lines 151 - 155, Remove the duplicate renderPreview function and replace its usage with the shared renderTemplatePreview utility: delete the local function renderPreview, import renderTemplatePreview, and ensure calls that previously passed template/previewVariables now call renderTemplatePreview(template, previewVariables) (or adapt the argument shape if needed), so the component uses the centralized template renderer instead of the local previewVariables-based implementation.app/pages/dashboard/jobs/[id]/index.vue (1)
327-352:⚠️ Potential issue | 🔴 CriticalStill transition the scheduled application, not
currentSummary.
interviewTargetApplicationonly keepsid/name, thenhandleInterviewScheduled()clears it and falls back tocurrentSummary. If the focused card changes while the scheduler is open, Line 347-350 can move the wrong application tointerviewor skip the transition entirely. Persist the target status too, and patch by the stored target id instead of the currently focused card.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/pages/dashboard/jobs/`[id]/index.vue around lines 327 - 352, The handler clears interviewTargetApplication (which only stores id/name) and then uses currentSummary to change status, causing wrong/ missed transitions if focus changed; update interviewTargetApplication to also persist the target status (or full id-based target object) in openInterviewScheduler, and in handleInterviewScheduled use that stored id to call changeStatus for that specific application (lookup its current status and validate via APPLICATION_STATUS_TRANSITIONS before calling changeStatus), then clear interviewTargetApplication only after successful refreshJobInterviews and status change; update references to interviewTargetApplication, openInterviewScheduler, handleInterviewScheduled, changeStatus, APPLICATION_STATUS_TRANSITIONS and refreshJobInterviews accordingly.
🧹 Nitpick comments (1)
app/components/InterviewEmailModal.vue (1)
230-241:availableVariablesis missing some supported placeholders.The
previewVariablesobject supports{{candidateLastName}},{{candidateEmail}}, and{{interviewTitle}}, but these aren't listed inavailableVariables. Users won't know they can use these variables.📝 Proposed addition
const availableVariables = [ { key: '{{candidateName}}', desc: 'Full name' }, { key: '{{candidateFirstName}}', desc: 'First name' }, + { key: '{{candidateLastName}}', desc: 'Last name' }, + { key: '{{candidateEmail}}', desc: 'Email address' }, { key: '{{jobTitle}}', desc: 'Job title' }, + { key: '{{interviewTitle}}', desc: 'Interview title' }, { key: '{{interviewDate}}', desc: 'Interview date' },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/InterviewEmailModal.vue` around lines 230 - 241, The availableVariables array is missing placeholders that previewVariables supports; update the availableVariables constant to include entries for '{{candidateLastName}}' (e.g., "Last name"), '{{candidateEmail}}' (e.g., "Candidate email"), and '{{interviewTitle}}' (e.g., "Interview title") so the UI lists all supported variables; locate the availableVariables definition and add these objects with appropriate desc values to match previewVariables usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/components/CandidateDetailSidebar.vue`:
- Line 308: The nested InterviewScheduleSidebar (controlled by
showScheduleSidebar) can remain mounted or reopen when the parent aside closes
and doesn't reset on applicationId changes; update the parent component so that
whenever the parent is closed (the same path that flips open/close or the header
close action) it also sets showScheduleSidebar = false and/or emits a dismiss to
InterviewScheduleSidebar, and add a watcher on applicationId to reset
showScheduleSidebar (or re-key InterviewScheduleSidebar) so the nested form is
unmounted and reseeded on app change; locate and modify the places using
showScheduleSidebar, the parent close handler, and any open toggle to ensure the
nested sidebar is dismissed before or as part of closing the parent.
In `@app/components/InterviewEmailModal.vue`:
- Around line 244-252: The modal lacks keyboard accessibility: add an Escape key
handler and focus trap in InterviewEmailModal.vue by defining a handleKeydown
function that calls emit('close') when e.key === 'Escape' and sendSuccess.value
is false, register it in onMounted with document.addEventListener('keydown',
handleKeydown) and remove it in onUnmounted via
document.removeEventListener('keydown', handleKeydown); additionally implement a
focus trap (either via `@vueuse/integrations/useFocusTrap` or manual focus
management) to keep focus inside the modal while open, and add appropriate ARIA
attributes (role="dialog" aria-modal="true" and aria-labelledby pointing to the
modal title id) and give the title element an id so aria-labelledby can
reference it.
- Around line 168-185: The setTimeout created in handleSend can fire after
unmount causing a stale emit; modify handleSend to store the timeout ID (e.g.,
in a ref like sendTimeoutId) instead of calling setTimeout inline, clear any
existing timeout before assigning a new one, and clear the timeout on component
teardown using the lifecycle hook (e.g., onBeforeUnmount) to call
clearTimeout(sendTimeoutId.value); also ensure you clear the timeout when
cleaning up in error paths or before emitting to avoid duplicate timers (refer
to handleSend and the setTimeout invocation).
- Around line 128-149: The previewVariables object is built once and not
reactive; change it into a computed property so it updates when props.interview
changes: import computed from 'vue' (if not already), replace the plain const
previewVariables: Record<string,string> = { ... } with const previewVariables =
computed(() => ({ ...same mapping using props.interview... })), and then update
all usages in the template/script to read previewVariables.value (or destructure
from previewVariables.value) so the preview reflects prop updates; keep the same
keys (candidateName, interviewDate, interviewTime, interviewType,
interviewLocation, interviewers, etc.) and preserve the date/time formatting
logic inside the computed.
- Around line 212-221: handleDeleteTemplate currently swallows all errors from
deleteTemplate; update it to pass caught errors to the existing handler
handlePreviewReadOnlyError and, if that handler returns false (meaning it didn't
handle the error), surface a user-facing notification (e.g., alert or existing
toast) so users know the delete failed. Specifically, in handleDeleteTemplate
wrap deleteTemplate in try/catch, call await handlePreviewReadOnlyError(err) in
the catch and only show an alert/toast when that call returns false; ensure you
still reset selectedTemplateId.value to 'system-standard' only after a
successful delete.
In `@app/pages/dashboard/jobs/`[id]/index.vue:
- Around line 371-373: applicationsWithInterviews currently includes all
interview records so cards with cancelled/completed/no-show interviews still
show the "Interview scheduled" marker; change the computed set to only include
interviews with status === 'scheduled' (e.g. const applicationsWithInterviews =
computed(() => new Set(jobInterviews.value.filter(i => i.status ===
'scheduled').map(i => i.applicationId)))) and update any other places in this
file that use the same logic (the duplicate logic later in the file) to use the
filtered set or rename the indicator if you intend to show any interview history
instead.
---
Duplicate comments:
In `@app/components/InterviewEmailModal.vue`:
- Around line 151-155: Remove the duplicate renderPreview function and replace
its usage with the shared renderTemplatePreview utility: delete the local
function renderPreview, import renderTemplatePreview, and ensure calls that
previously passed template/previewVariables now call
renderTemplatePreview(template, previewVariables) (or adapt the argument shape
if needed), so the component uses the centralized template renderer instead of
the local previewVariables-based implementation.
In `@app/pages/dashboard/jobs/`[id]/index.vue:
- Around line 327-352: The handler clears interviewTargetApplication (which only
stores id/name) and then uses currentSummary to change status, causing wrong/
missed transitions if focus changed; update interviewTargetApplication to also
persist the target status (or full id-based target object) in
openInterviewScheduler, and in handleInterviewScheduled use that stored id to
call changeStatus for that specific application (lookup its current status and
validate via APPLICATION_STATUS_TRANSITIONS before calling changeStatus), then
clear interviewTargetApplication only after successful refreshJobInterviews and
status change; update references to interviewTargetApplication,
openInterviewScheduler, handleInterviewScheduled, changeStatus,
APPLICATION_STATUS_TRANSITIONS and refreshJobInterviews accordingly.
---
Nitpick comments:
In `@app/components/InterviewEmailModal.vue`:
- Around line 230-241: The availableVariables array is missing placeholders that
previewVariables supports; update the availableVariables constant to include
entries for '{{candidateLastName}}' (e.g., "Last name"), '{{candidateEmail}}'
(e.g., "Candidate email"), and '{{interviewTitle}}' (e.g., "Interview title") so
the UI lists all supported variables; locate the availableVariables definition
and add these objects with appropriate desc values to match previewVariables
usage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: df22e3e9-c352-4cd0-88a4-54746e2a1ada
📒 Files selected for processing (5)
app/components/CandidateDetailSidebar.vueapp/components/InterviewEmailModal.vueapp/pages/dashboard/applications/[id].vueapp/pages/dashboard/candidates/[id].vueapp/pages/dashboard/jobs/[id]/index.vue
| // Interview scheduling | ||
| // ───────────────────────────────────────────── | ||
|
|
||
| const showScheduleSidebar = ref(false) |
There was a problem hiding this comment.
Dismiss the nested scheduler before closing the parent sidebar.
This sidebar is rendered outside the <aside> and is not tied to open, so the header close button and the existing Escape path can close the parent while InterviewScheduleSidebar stays mounted or reopens on the next open. It also is not reset on applicationId changes, which can preserve stale initial form state because app/components/InterviewScheduleSidebar.vue only seeds its form on mount (Line 56-62).
⚙️ Suggested fix
function onKeydown(e: KeyboardEvent) {
if (e.key === 'Escape') {
if (showPreview.value) {
closePreview()
} else if (showDocDeleteConfirm.value) {
showDocDeleteConfirm.value = null
+ } else if (showScheduleSidebar.value) {
+ showScheduleSidebar.value = false
} else {
emit('close')
}
}
}
+watch(() => props.open, (isOpen) => {
+ if (!isOpen) showScheduleSidebar.value = false
+})
+
// Reset state when switching to a different application
watch(() => props.applicationId, () => {
isEditingNotes.value = false
activeTab.value = 'overview'
uploadError.value = null
showDocDeleteConfirm.value = null
+ showScheduleSidebar.value = false
closePreview()
})
<InterviewScheduleSidebar
- v-if="showScheduleSidebar && application"
+ v-if="open && showScheduleSidebar && application"
:application-id="props.applicationId"
:candidate-name="`${application.candidate.firstName} ${application.candidate.lastName}`"
:job-title="application.job?.title ?? ''"Also applies to: 345-362, 784-792
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/components/CandidateDetailSidebar.vue` at line 308, The nested
InterviewScheduleSidebar (controlled by showScheduleSidebar) can remain mounted
or reopen when the parent aside closes and doesn't reset on applicationId
changes; update the parent component so that whenever the parent is closed (the
same path that flips open/close or the header close action) it also sets
showScheduleSidebar = false and/or emits a dismiss to InterviewScheduleSidebar,
and add a watcher on applicationId to reset showScheduleSidebar (or re-key
InterviewScheduleSidebar) so the nested form is unmounted and reseeded on app
change; locate and modify the places using showScheduleSidebar, the parent close
handler, and any open toggle to ensure the nested sidebar is dismissed before or
as part of closing the parent.
Comment on lines
+128
to
+149
| const previewVariables: Record<string, string> = { | ||
| candidateName: `${props.interview.candidateFirstName} ${props.interview.candidateLastName}`, | ||
| candidateFirstName: props.interview.candidateFirstName, | ||
| candidateLastName: props.interview.candidateLastName, | ||
| candidateEmail: props.interview.candidateEmail, | ||
| jobTitle: props.interview.jobTitle, | ||
| interviewTitle: props.interview.title, | ||
| interviewDate: new Date(props.interview.scheduledAt).toLocaleDateString('en-US', { | ||
| weekday: 'long', month: 'long', day: 'numeric', year: 'numeric', | ||
| }), | ||
| interviewTime: new Date(props.interview.scheduledAt).toLocaleTimeString('en-US', { | ||
| hour: 'numeric', minute: '2-digit', hour12: true, | ||
| }), | ||
| interviewDuration: String(props.interview.duration), | ||
| interviewType: { | ||
| video: 'Video Call', phone: 'Phone Call', in_person: 'In Person', | ||
| technical: 'Technical Interview', panel: 'Panel Interview', take_home: 'Take-Home Assignment', | ||
| }[props.interview.type] ?? props.interview.type, | ||
| interviewLocation: props.interview.location ?? 'To be confirmed', | ||
| interviewers: props.interview.interviewers?.join(', ') ?? 'To be confirmed', | ||
| organizationName: 'Your Organization', | ||
| } |
There was a problem hiding this comment.
previewVariables is not reactive and won't update if the interview prop changes.
This plain object is evaluated once during component setup. If the parent component passes a different interview (e.g., user navigates to another interview while the modal is open), the preview will display stale data.
Convert to a computed property to maintain reactivity:
🔧 Proposed fix
-const previewVariables: Record<string, string> = {
- candidateName: `${props.interview.candidateFirstName} ${props.interview.candidateLastName}`,
- candidateFirstName: props.interview.candidateFirstName,
- candidateLastName: props.interview.candidateLastName,
- candidateEmail: props.interview.candidateEmail,
- jobTitle: props.interview.jobTitle,
- interviewTitle: props.interview.title,
- interviewDate: new Date(props.interview.scheduledAt).toLocaleDateString('en-US', {
- weekday: 'long', month: 'long', day: 'numeric', year: 'numeric',
- }),
- interviewTime: new Date(props.interview.scheduledAt).toLocaleTimeString('en-US', {
- hour: 'numeric', minute: '2-digit', hour12: true,
- }),
- interviewDuration: String(props.interview.duration),
- interviewType: {
- video: 'Video Call', phone: 'Phone Call', in_person: 'In Person',
- technical: 'Technical Interview', panel: 'Panel Interview', take_home: 'Take-Home Assignment',
- }[props.interview.type] ?? props.interview.type,
- interviewLocation: props.interview.location ?? 'To be confirmed',
- interviewers: props.interview.interviewers?.join(', ') ?? 'To be confirmed',
- organizationName: 'Your Organization',
-}
+const previewVariables = computed<Record<string, string>>(() => ({
+ candidateName: `${props.interview.candidateFirstName} ${props.interview.candidateLastName}`,
+ candidateFirstName: props.interview.candidateFirstName,
+ candidateLastName: props.interview.candidateLastName,
+ candidateEmail: props.interview.candidateEmail,
+ jobTitle: props.interview.jobTitle,
+ interviewTitle: props.interview.title,
+ interviewDate: new Date(props.interview.scheduledAt).toLocaleDateString('en-US', {
+ weekday: 'long', month: 'long', day: 'numeric', year: 'numeric',
+ }),
+ interviewTime: new Date(props.interview.scheduledAt).toLocaleTimeString('en-US', {
+ hour: 'numeric', minute: '2-digit', hour12: true,
+ }),
+ interviewDuration: String(props.interview.duration),
+ interviewType: {
+ video: 'Video Call', phone: 'Phone Call', in_person: 'In Person',
+ technical: 'Technical Interview', panel: 'Panel Interview', take_home: 'Take-Home Assignment',
+ }[props.interview.type] ?? props.interview.type,
+ interviewLocation: props.interview.location ?? 'To be confirmed',
+ interviewers: props.interview.interviewers?.join(', ') ?? 'To be confirmed',
+ organizationName: 'Your Organization',
+}))Then update usages to access .value:
function renderPreview(template: string): string {
+ const vars = previewVariables.value
return template.replace(/\{\{(\w+)\}\}/g, (match, key: string) => {
- return key in previewVariables ? (previewVariables[key] ?? match) : match
+ return key in vars ? (vars[key] ?? match) : match
})
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/components/InterviewEmailModal.vue` around lines 128 - 149, The
previewVariables object is built once and not reactive; change it into a
computed property so it updates when props.interview changes: import computed
from 'vue' (if not already), replace the plain const previewVariables:
Record<string,string> = { ... } with const previewVariables = computed(() => ({
...same mapping using props.interview... })), and then update all usages in the
template/script to read previewVariables.value (or destructure from
previewVariables.value) so the preview reflects prop updates; keep the same keys
(candidateName, interviewDate, interviewTime, interviewType, interviewLocation,
interviewers, etc.) and preserve the date/time formatting logic inside the
computed.
Comment on lines
+168
to
+185
| async function handleSend() { | ||
| sendError.value = '' | ||
| isSending.value = true | ||
|
|
||
| try { | ||
| const payload = activeTab.value === 'custom' | ||
| ? { customSubject: customSubject.value, customBody: customBody.value } | ||
| : { templateId: selectedTemplateId.value } | ||
|
|
||
| await sendInvitation(props.interview.id, payload) | ||
| sendSuccess.value = true | ||
| setTimeout(() => emit('sent'), 1500) | ||
| } catch (err: any) { | ||
| sendError.value = err?.data?.statusMessage ?? err?.message ?? 'Failed to send invitation email' | ||
| } finally { | ||
| isSending.value = false | ||
| } | ||
| } |
There was a problem hiding this comment.
Clear timeout on component unmount to prevent stale emit.
The setTimeout at line 179 could fire after the component unmounts, causing a stale emit or console warning.
🛡️ Proposed fix
+// At the top of script setup, after state definitions:
+let successTimeout: ReturnType<typeof setTimeout> | null = null
+
+onUnmounted(() => {
+ if (successTimeout) clearTimeout(successTimeout)
+})
+
async function handleSend() {
sendError.value = ''
isSending.value = true
try {
const payload = activeTab.value === 'custom'
? { customSubject: customSubject.value, customBody: customBody.value }
: { templateId: selectedTemplateId.value }
await sendInvitation(props.interview.id, payload)
sendSuccess.value = true
- setTimeout(() => emit('sent'), 1500)
+ successTimeout = setTimeout(() => emit('sent'), 1500)
} catch (err: any) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/components/InterviewEmailModal.vue` around lines 168 - 185, The
setTimeout created in handleSend can fire after unmount causing a stale emit;
modify handleSend to store the timeout ID (e.g., in a ref like sendTimeoutId)
instead of calling setTimeout inline, clear any existing timeout before
assigning a new one, and clear the timeout on component teardown using the
lifecycle hook (e.g., onBeforeUnmount) to call
clearTimeout(sendTimeoutId.value); also ensure you clear the timeout when
cleaning up in error paths or before emitting to avoid duplicate timers (refer
to handleSend and the setTimeout invocation).
Comment on lines
+212
to
+221
| async function handleDeleteTemplate(id: string) { | ||
| try { | ||
| await deleteTemplate(id) | ||
| if (selectedTemplateId.value === id) { | ||
| selectedTemplateId.value = 'system-standard' | ||
| } | ||
| } catch { | ||
| // Handled by composable | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if useEmailTemplates composable handles errors with toasts/notifications
rg -n -A 10 "deleteTemplate" app/composables/useEmailTemplates.tsRepository: reqcore-inc/reqcore
Length of output: 471
🏁 Script executed:
# Find handlePreviewReadOnlyError definition
rg -n "handlePreviewReadOnlyError" app/composables/useEmailTemplates.ts -B 5 -A 8Repository: reqcore-inc/reqcore
Length of output: 1880
🏁 Script executed:
# Check the component for any error toast/notification handling
rg -n "toast\|notification\|showError\|error" app/components/InterviewEmailModal.vue | head -30Repository: reqcore-inc/reqcore
Length of output: 45
🏁 Script executed:
# Find usePreviewReadOnly composable
fd "usePreviewReadOnly" app/composables/Repository: reqcore-inc/reqcore
Length of output: 100
🏁 Script executed:
# Search for handlePreviewReadOnlyError definition
rg -n "handlePreviewReadOnlyError" app/ -A 10 | head -50Repository: reqcore-inc/reqcore
Length of output: 3643
🏁 Script executed:
cat -n app/composables/usePreviewReadOnly.tsRepository: reqcore-inc/reqcore
Length of output: 2711
Component silently swallows deletion errors, leaving users without feedback.
The handleDeleteTemplate function catches all errors from deleteTemplate without displaying feedback to the user. While the composable shows a modal for read-only errors, other deletion failures (network errors, server errors, etc.) are silently ignored. Users won't know if the operation failed.
Other components in the codebase (e.g., applications/[id].vue, jobs/[id]/index.vue) follow the correct pattern: check if handlePreviewReadOnlyError handled the error, and if not, display an alert or toast to the user.
🔧 Suggested approach
Follow the pattern used elsewhere in the codebase:
async function handleDeleteTemplate(id: string) {
try {
await deleteTemplate(id)
if (selectedTemplateId.value === id) {
selectedTemplateId.value = 'system-standard'
}
- } catch {
- // Handled by composable
+ } catch (error) {
+ if (!handlePreviewReadOnlyError(error)) {
+ alert(error?.data?.statusMessage ?? 'Failed to delete template')
+ }
}
}Or use a toast notification instead of alert() if available in your UI library.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/components/InterviewEmailModal.vue` around lines 212 - 221,
handleDeleteTemplate currently swallows all errors from deleteTemplate; update
it to pass caught errors to the existing handler handlePreviewReadOnlyError and,
if that handler returns false (meaning it didn't handle the error), surface a
user-facing notification (e.g., alert or existing toast) so users know the
delete failed. Specifically, in handleDeleteTemplate wrap deleteTemplate in
try/catch, call await handlePreviewReadOnlyError(err) in the catch and only show
an alert/toast when that call returns false; ensure you still reset
selectedTemplateId.value to 'system-standard' only after a successful delete.
Comment on lines
+244
to
+252
| <template> | ||
| <Teleport to="body"> | ||
| <div class="fixed inset-0 z-50 flex items-center justify-center"> | ||
| <!-- Backdrop --> | ||
| <div class="absolute inset-0 bg-black/40 backdrop-blur-[2px]" @click="emit('close')" /> | ||
|
|
||
| <!-- Modal --> | ||
| <div class="relative bg-white dark:bg-surface-900 rounded-2xl shadow-2xl shadow-surface-900/10 dark:shadow-black/30 ring-1 ring-surface-200/80 dark:ring-surface-700/60 w-full max-w-2xl mx-4 max-h-[90vh] flex flex-col overflow-hidden"> | ||
| <!-- Header --> |
There was a problem hiding this comment.
Modal lacks keyboard accessibility: no Escape key handler or focus trap.
Modals should be dismissible via the Escape key and should trap focus within the dialog for keyboard users. This is an accessibility requirement for modal dialogs.
♿ Proposed fix
Add keyboard handling in the script section:
// Add after state definitions
onMounted(() => {
document.addEventListener('keydown', handleKeydown)
})
onUnmounted(() => {
document.removeEventListener('keydown', handleKeydown)
})
function handleKeydown(e: KeyboardEvent) {
if (e.key === 'Escape' && !sendSuccess.value) {
emit('close')
}
}For a complete focus trap, consider using a library like @vueuse/integrations/useFocusTrap or implementing manual focus management.
Additionally, add ARIA attributes to the modal:
- <div class="relative bg-white dark:bg-surface-900 rounded-2xl ...">
+ <div
+ role="dialog"
+ aria-modal="true"
+ aria-labelledby="modal-title"
+ class="relative bg-white dark:bg-surface-900 rounded-2xl ..."
+ >And add the id to the title:
- <h2 class="text-base font-semibold ...">
+ <h2 id="modal-title" class="text-base font-semibold ...">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/components/InterviewEmailModal.vue` around lines 244 - 252, The modal
lacks keyboard accessibility: add an Escape key handler and focus trap in
InterviewEmailModal.vue by defining a handleKeydown function that calls
emit('close') when e.key === 'Escape' and sendSuccess.value is false, register
it in onMounted with document.addEventListener('keydown', handleKeydown) and
remove it in onUnmounted via document.removeEventListener('keydown',
handleKeydown); additionally implement a focus trap (either via
`@vueuse/integrations/useFocusTrap` or manual focus management) to keep focus
inside the modal while open, and add appropriate ARIA attributes (role="dialog"
aria-modal="true" and aria-labelledby pointing to the modal title id) and give
the title element an id so aria-labelledby can reference it.
…and email template integration
- Update job detail page to read initial pipeline stage from URL query parameters. - Refactor job listing page to include detailed job status and pipeline stage counts. - Improve loading and error handling states for job listings. - Modify navigation after saving a job draft to redirect to the jobs list instead of the main dashboard. - Extend API to provide per-status breakdown for active jobs in dashboard stats.
…tomization options
…tegration and email validation
…s and enhance error handling in sync process
…e interview ID validation
…e existing content with links
…nd remove unused migration placeholder
- Bump tar version from 7.5.10 to 7.5.11 - Change minimatch and brace-expansion to allow newer versions - Add glob as a dependency with a minimum version of 13.0.6 - Update @posthog/cli reference in @posthog/nuxt to use an empty npm package
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
Type of change
Validation
DCO
Signed-off-by) viagit commit -sSummary by CodeRabbit
New Features
Removed Features