feat(interview): prevent interviewer double-booking and scheduling co…#326
Merged
Sachinchaurasiya360 merged 1 commit intoMay 21, 2026
Conversation
Contributor
Author
|
Hi @Sachinchaurasiya360 |
Contributor
📝 WalkthroughWalkthroughThis PR adds multi-interviewer interview scheduling with real-time conflict detection across the backend and client. The service layer validates scheduling conflicts by comparing time windows; the controller maps that error to HTTP 409; and the client form accepts comma-separated interviewer IDs with appropriate success and error notifications. ChangesMulti-Interviewer Scheduling Conflict Detection
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
client/src/module/recruiter/hr/InterviewsPage.tsx (1)
700-711: ⚡ Quick winUse a standard text size token for the new field copy.
The new label/help text introduces
text-[10px]. Please switch to the closest standard scale class instead of another arbitrary size.As per coding guidelines,
client/src/**/*.{tsx,ts}:Do not use arbitrary bracket sizes like \text-[17px]`, use standard scale classes instead`.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/module/recruiter/hr/InterviewsPage.tsx` around lines 700 - 711, The label and help text in InterviewsPage.tsx use an arbitrary size class `text-[10px]`; replace those with the nearest standard Tailwind scale (e.g., `text-xs`) to comply with the project rule against arbitrary bracket sizes—update the label class on the element that currently contains `text-[10px]` and the help paragraph that follows (both related to the `interviewerIds` input and the `form.interviewerIds` state) so they use `text-xs` (or another approved scale token) instead of `text-[10px]`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@client/src/module/recruiter/hr/InterviewsPage.tsx`:
- Around line 119-131: The current parsing of form.interviewerIds into parsedIds
silently drops non-numeric tokens (e.g. "12, abc" -> [12]) which can submit
fewer interviewers or an empty array and bypass checks; update the submit flow
that builds parsedIds to validate each comma-separated token from
form.interviewerIds and if any token is not a valid integer or the resulting
parsedIds is empty, reject the submit (return/throw and surface a UI error)
instead of filtering them out; update the code paths around parsedIds and the
api.post("/hr/interviews") call so you only call api.post when all tokens are
valid and include validation error messages for the user explaining which tokens
are invalid.
In `@server/src/module/interview/interview.service.ts`:
- Around line 33-37: The read-then-write TOCTOU in interview.service.ts (the
call to checkSchedulingConflict(...) before saving) must be made atomic: wrap
the conflict check and the subsequent insert/update for the interviewer slots
inside a single DB transaction and lock the affected interviewer rows/slots
(e.g. SELECT ... FOR UPDATE or use SERIALIZABLE/isolation-level locking provided
by your ORM) so concurrent requests cannot both pass the check; apply the same
pattern in the update(...) path as well (acquire locks for data.interviewerIds,
perform the conflict check inside the transaction, then perform the write and
commit).
- Around line 182-190: The current day-bounded conflict query (in
InterviewService where potentialConflicts is built using
prisma.interview.findMany) misses interviews that start before midnight and end
after it; instead compute the proposed slot's start and end
(proposedStart/proposedEnd) and query for any interviews with overlapping
intervals for the same interviewers by replacing the date-range filter with an
overlap filter: interviewerIds: { hasSome: interviewerIds } and
(existingInterviewEnd > proposedStart AND existingInterviewStart < proposedEnd)
— use the actual fields in your model (scheduledAt and scheduledEnd or
scheduledAt + duration) when constructing the prisma.where clause so
cross-midnight overlaps are caught.
- Around line 115-120: In update(), avoid re-running scheduling validation when
the interview's effective new status becomes an inactive state (CANCELLED,
COMPLETED, NO_SHOW): compute the effective status by merging incoming update
fields with the existing interview.status, and only call
this.checkSchedulingConflict(newInterviewerIds, newScheduledAt, newDuration,
interview.id) when that effective status is not one of the inactive statuses;
i.e., add a guard around the checkSchedulingConflict call that skips it for
CANCELLED/COMPLETED/NO_SHOW.
---
Nitpick comments:
In `@client/src/module/recruiter/hr/InterviewsPage.tsx`:
- Around line 700-711: The label and help text in InterviewsPage.tsx use an
arbitrary size class `text-[10px]`; replace those with the nearest standard
Tailwind scale (e.g., `text-xs`) to comply with the project rule against
arbitrary bracket sizes—update the label class on the element that currently
contains `text-[10px]` and the help paragraph that follows (both related to the
`interviewerIds` input and the `form.interviewerIds` state) so they use
`text-xs` (or another approved scale token) instead of `text-[10px]`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dfe67cd3-f11d-4d5f-9967-96e6f42d3ce4
📒 Files selected for processing (3)
client/src/module/recruiter/hr/InterviewsPage.tsxserver/src/module/interview/interview.controller.tsserver/src/module/interview/interview.service.ts
Comment on lines
+119
to
131
| const parsedIds = form.interviewerIds | ||
| .split(",") | ||
| .map((s) => s.trim()) | ||
| .filter((s) => s.length > 0) | ||
| .map(Number) | ||
| .filter((n) => !isNaN(n)); | ||
|
|
||
| await api.post("/hr/interviews", { | ||
| ...form, | ||
| applicationId: Number(form.applicationId), | ||
| durationMinutes: Number(form.durationMinutes), | ||
| interviewerIds: parsedIds, | ||
| }); |
Contributor
There was a problem hiding this comment.
Don't silently drop invalid interviewer IDs.
"12, abc" becomes [12], and "abc" becomes [], so this can submit fewer interviewers than the user entered and even bypass conflict checking entirely when the parsed array ends up empty. Reject invalid tokens instead of filtering them out.
💡 One way to validate before submit
- const parsedIds = form.interviewerIds
- .split(",")
- .map((s) => s.trim())
- .filter((s) => s.length > 0)
- .map(Number)
- .filter((n) => !isNaN(n));
+ const rawIds = form.interviewerIds
+ .split(",")
+ .map((s) => s.trim())
+ .filter((s) => s.length > 0);
+
+ if (rawIds.some((id) => !/^\d+$/.test(id))) {
+ throw new Error("Please enter only numeric interviewer IDs.");
+ }
+
+ const parsedIds = rawIds.map(Number);📝 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 parsedIds = form.interviewerIds | |
| .split(",") | |
| .map((s) => s.trim()) | |
| .filter((s) => s.length > 0) | |
| .map(Number) | |
| .filter((n) => !isNaN(n)); | |
| await api.post("/hr/interviews", { | |
| ...form, | |
| applicationId: Number(form.applicationId), | |
| durationMinutes: Number(form.durationMinutes), | |
| interviewerIds: parsedIds, | |
| }); | |
| const rawIds = form.interviewerIds | |
| .split(",") | |
| .map((s) => s.trim()) | |
| .filter((s) => s.length > 0); | |
| if (rawIds.some((id) => !/^\d+$/.test(id))) { | |
| throw new Error("Please enter only numeric interviewer IDs."); | |
| } | |
| const parsedIds = rawIds.map(Number); | |
| await api.post("/hr/interviews", { | |
| ...form, | |
| applicationId: Number(form.applicationId), | |
| durationMinutes: Number(form.durationMinutes), | |
| interviewerIds: parsedIds, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client/src/module/recruiter/hr/InterviewsPage.tsx` around lines 119 - 131,
The current parsing of form.interviewerIds into parsedIds silently drops
non-numeric tokens (e.g. "12, abc" -> [12]) which can submit fewer interviewers
or an empty array and bypass checks; update the submit flow that builds
parsedIds to validate each comma-separated token from form.interviewerIds and if
any token is not a valid integer or the resulting parsedIds is empty, reject the
submit (return/throw and surface a UI error) instead of filtering them out;
update the code paths around parsedIds and the api.post("/hr/interviews") call
so you only call api.post when all tokens are valid and include validation error
messages for the user explaining which tokens are invalid.
Comment on lines
+33
to
+37
| await this.checkSchedulingConflict( | ||
| data.interviewerIds ?? [], | ||
| new Date(data.scheduledAt), | ||
| data.durationMinutes ?? 60 | ||
| ); |
Contributor
There was a problem hiding this comment.
Make the conflict check and write atomic.
This is still a TOCTOU flow: two concurrent requests can both pass checkSchedulingConflict() and then both insert/update the same interviewer slot. The same read-then-write gap exists in update(), so the double-booking guarantee is not reliable under concurrent traffic. Use a transaction with DB-level serialization/locking for the affected interviewer IDs before the final write.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/src/module/interview/interview.service.ts` around lines 33 - 37, The
read-then-write TOCTOU in interview.service.ts (the call to
checkSchedulingConflict(...) before saving) must be made atomic: wrap the
conflict check and the subsequent insert/update for the interviewer slots inside
a single DB transaction and lock the affected interviewer rows/slots (e.g.
SELECT ... FOR UPDATE or use SERIALIZABLE/isolation-level locking provided by
your ORM) so concurrent requests cannot both pass the check; apply the same
pattern in the update(...) path as well (acquire locks for data.interviewerIds,
perform the conflict check inside the transaction, then perform the write and
commit).
Comment on lines
+115
to
+120
| await this.checkSchedulingConflict( | ||
| newInterviewerIds, | ||
| newScheduledAt, | ||
| newDuration, | ||
| interview.id | ||
| ); |
Contributor
There was a problem hiding this comment.
Skip conflict validation when the effective new status is inactive.
update() re-runs scheduling validation even when this change is turning the interview into CANCELLED, COMPLETED, or NO_SHOW. That can block cancelling or closing legacy/conflicting records even though the resulting state is no longer schedulable.
💡 Narrow fix
+ const newStatus = data.status ?? interview.status;
const newScheduledAt = (updateData["scheduledAt"] as Date) ?? interview.scheduledAt;
const newDuration = data.durationMinutes ?? interview.durationMinutes;
const newInterviewerIds = data.interviewerIds ?? interview.interviewerIds;
- await this.checkSchedulingConflict(
- newInterviewerIds,
- newScheduledAt,
- newDuration,
- interview.id
- );
+ if (!["CANCELLED", "COMPLETED", "NO_SHOW"].includes(newStatus)) {
+ await this.checkSchedulingConflict(
+ newInterviewerIds,
+ newScheduledAt,
+ newDuration,
+ interview.id
+ );
+ }📝 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
| await this.checkSchedulingConflict( | |
| newInterviewerIds, | |
| newScheduledAt, | |
| newDuration, | |
| interview.id | |
| ); | |
| const newStatus = data.status ?? interview.status; | |
| const newScheduledAt = (updateData["scheduledAt"] as Date) ?? interview.scheduledAt; | |
| const newDuration = data.durationMinutes ?? interview.durationMinutes; | |
| const newInterviewerIds = data.interviewerIds ?? interview.interviewerIds; | |
| if (!["CANCELLED", "COMPLETED", "NO_SHOW"].includes(newStatus)) { | |
| await this.checkSchedulingConflict( | |
| newInterviewerIds, | |
| newScheduledAt, | |
| newDuration, | |
| interview.id | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/src/module/interview/interview.service.ts` around lines 115 - 120, In
update(), avoid re-running scheduling validation when the interview's effective
new status becomes an inactive state (CANCELLED, COMPLETED, NO_SHOW): compute
the effective status by merging incoming update fields with the existing
interview.status, and only call this.checkSchedulingConflict(newInterviewerIds,
newScheduledAt, newDuration, interview.id) when that effective status is not one
of the inactive statuses; i.e., add a guard around the checkSchedulingConflict
call that skips it for CANCELLED/COMPLETED/NO_SHOW.
Comment on lines
+182
to
+190
| const startOfDay = new Date(scheduledAt); | ||
| startOfDay.setHours(0, 0, 0, 0); | ||
| const endOfDay = new Date(scheduledAt); | ||
| endOfDay.setHours(23, 59, 59, 999); | ||
|
|
||
| const potentialConflicts = await prisma.interview.findMany({ | ||
| where: { | ||
| interviewerIds: { hasSome: interviewerIds }, | ||
| scheduledAt: { gte: startOfDay, lte: endOfDay }, |
Contributor
There was a problem hiding this comment.
The day-bounded lookup misses overlaps that cross midnight.
This only loads interviews whose scheduledAt starts on the same calendar day as the proposed slot. An interview starting at 11:30 PM and running past midnight will not be seen when scheduling another interview at 12:15 AM the next day, even though the intervals overlap.
Also applies to: 199-205
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/src/module/interview/interview.service.ts` around lines 182 - 190, The
current day-bounded conflict query (in InterviewService where potentialConflicts
is built using prisma.interview.findMany) misses interviews that start before
midnight and end after it; instead compute the proposed slot's start and end
(proposedStart/proposedEnd) and query for any interviews with overlapping
intervals for the same interviewers by replacing the date-range filter with an
overlap filter: interviewerIds: { hasSome: interviewerIds } and
(existingInterviewEnd > proposedStart AND existingInterviewStart < proposedEnd)
— use the actual fields in your model (scheduledAt and scheduledEnd or
scheduledAt + duration) when constructing the prisma.where clause so
cross-midnight overlaps are caught.
Sachinchaurasiya360
added
good first issue
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
2 participants
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.
Description
This PR addresses the issue #314 where recruiters could accidentally double-book an interviewer for two overlapping interviews, resulting in scheduling conflicts that had to be resolved manually.
Changes Made
Backend:
interview.service.ts: Added acheckSchedulingConflicthelper method. Before an interview is created or updated, the system fetches any existing interviews on the same day for the requestedinterviewerIds. It performs an in-memory check to ensure the new[scheduledAt, scheduledAt + durationMinutes]timeframe does not overlap with any active schedules.interview.controller.ts: Updated thecreateandupdateendpoints to catch the specific conflict error message and return a standard409 ConflictHTTP status code instead of a generic500 Internal Server Error.Frontend:
InterviewsPage.tsx:createMutation. If the API responds with a409 Conflict, the UI gracefully alerts the recruiter viareact-hot-toastwith a clear message: "Scheduling conflict: One or more interviewers are already booked at this time."How to Test
Interviewer ID: 1at10:00 AMwith a duration of60 minutes.Interviewer ID: 1at10:30 AM.409 ConflictAPI response and verify that the red Toast notification appears in the UI preventing the action.Checklist
CONTRIBUTING.mdconventionsSummary by CodeRabbit
Release Notes
New Features
Improvements