-
Notifications
You must be signed in to change notification settings - Fork 5
Add Interview Filters #52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds a Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as interview.controller
participant Service as interview.service
participant DB as Prisma/Database
Client->>Controller: GET /interviews?page=1&limit=10&verdict=Selected
Controller->>Controller: Parse & validate `verdict` (default "All")
Controller->>Service: getInterviews(page, limit, verdict)
alt verdict != "All"
Service->>Service: Build where = { verdict: verdict }
else verdict == "All"
Service->>Service: where = {}
end
Service->>DB: findMany(where, pagination, include)
DB-->>Service: Interview records
Service-->>Controller: { interviews, verdict, meta }
Controller-->>Client: JSON response
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)src/services/interview.service.ts (3)
🪛 ESLintsrc/services/interview.service.ts[error] 6-6: Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any) 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/services/interview.service.ts (1)
30-30: Critical: Total count ignores verdict filter.The count query doesn't apply the
wherefilter, so when filtering by verdict, the total count and totalPages will be incorrect. For example, filtering by "Selected" will return the count of all interviews, not just selected ones.Apply this diff to fix the count:
- prisma.interviewExperience.count(), + prisma.interviewExperience.count({ where }),
🧹 Nitpick comments (1)
src/services/interview.service.ts (1)
3-3: LGTM with minor style suggestion.The signature correctly adds the verdict parameter with an appropriate default value.
Consider fixing the spacing for consistency with TypeScript conventions:
-export const getInterviews = async (page: number = 1, limit: number = 10, verdict : string = "All") => { +export const getInterviews = async (page: number = 1, limit: number = 10, verdict: string = "All") => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/controllers/interview.controller.ts(2 hunks)src/services/interview.service.ts(1 hunks)tests/Interview.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/services/interview.service.ts (2)
src/controllers/interview.controller.ts (1)
getInterviews(5-28)src/services/member.service.ts (1)
getInterviews(142-146)
🪛 ESLint
src/services/interview.service.ts
[error] 6-6: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🔇 Additional comments (5)
tests/Interview.test.ts (2)
52-52: Verify service call includes verdict parameter.The test expectation correctly reflects the new response structure, but the mock verification doesn't confirm that
interviewService.getInterviewsis called with theverdictparameter.Consider adding an assertion to verify the service is called with all three parameters:
await getInterviews(req, res); +expect(interviewService.getInterviews).toHaveBeenCalledWith(1, 10, "All"); expect(res.status).toHaveBeenCalledWith(200);
59-59: LGTM!Good formatting improvement for test separation.
src/controllers/interview.controller.ts (2)
17-17: LGTM!Correctly passes the verdict parameter to the service layer.
24-24: LGTM!Including verdict in the response payload provides useful context to API consumers about the applied filter.
src/services/interview.service.ts (1)
13-13: LGTM!Correctly applies the dynamic where filter to the query.
| const where : any = {} | ||
| if(verdict !== "All"){ | ||
| where.verdict = verdict | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace any type with proper Prisma typing.
Using any defeats TypeScript's type safety and is flagged by static analysis. The where clause should use Prisma's generated types.
Apply this diff to use proper typing:
-const where : any = {}
+const where: Prisma.InterviewExperienceWhereInput = {}
if(verdict !== "All"){
where.verdict = verdict
}You'll need to import the Prisma namespace at the top of the file:
-import { prisma } from "../db/client"
+import { prisma, Prisma } from "../db/client"📝 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.
| const where : any = {} | |
| if(verdict !== "All"){ | |
| where.verdict = verdict | |
| } | |
| const where: Prisma.InterviewExperienceWhereInput = {} | |
| if(verdict !== "All"){ | |
| where.verdict = verdict | |
| } |
🧰 Tools
🪛 ESLint
[error] 6-6: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🤖 Prompt for AI Agents
In src/services/interview.service.ts around lines 6 to 9, the variable "where"
is currently typed as "any" which bypasses TypeScript/Prisma typings; change it
to the appropriate Prisma generated type (e.g., Prisma.InterviewWhereInput) and
update usage accordingly so "where.verdict = verdict" remains valid. Add an
import for the Prisma namespace at the top of the file (import { Prisma } from
'@prisma/client') and declare "const where: Prisma.InterviewWhereInput = {}" (or
the exact model WhereInput name if different), preserving the conditional
assignment when verdict !== "All". Ensure no other "any" remains for the where
clause.
Summary by CodeRabbit
New Features
Improvements
Tests