-
Notifications
You must be signed in to change notification settings - Fork 6
Members logic #12
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
Members logic #12
Conversation
WalkthroughThis update introduces a new member management feature with controllers, routes, services, and types, accompanied by unit tests. It refactors the Prisma schema for consistency, updates project-related controllers and services, modifies environment and ignore files, and includes minor formatting and migration script changes. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Router
participant Controller
participant Service
participant Supabase
participant Prisma
Client->>Router: POST /members (with optional file)
Router->>Controller: createAMember(req, res)
Controller->>Supabase: uploadProfilePhoto (if file present)
Controller->>Service: createMember(email, name, password, passoutYear, imageUrl)
Service->>Prisma: Create Member and Account records
Prisma-->>Service: Member object
Service-->>Controller: Member object
Controller-->>Client: JSON response (success, user data)
sequenceDiagram
participant Client
participant Router
participant Controller
participant Service
participant Supabase
participant Prisma
Client->>Router: PATCH /members/:memberId (with optional file)
Router->>Controller: updateAMember(req, res)
Controller->>Supabase: uploadProfilePhoto (if file present)
Controller->>Service: updateMember(id, payload)
Service->>Prisma: Update Member record
Prisma-->>Service: Updated Member object
Service-->>Controller: Updated Member object
Controller-->>Client: JSON response (success, message)
Estimated code review effort3 (~40–60 minutes) Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 20
🧹 Nitpick comments (14)
src/utils/apiError.ts (1)
32-37:nextis never used – drop it or prefix with underscore
errorHandleracceptsnextbut never calls it or references it. This will trigger eslint/tslint “unused parameter” rules and mis-communicates intent.-export function errorHandler( - err: unknown, - req: Request, - res: Response, - next: NextFunction, -): void { +export function errorHandler( + err: unknown, + req: Request, + res: Response, + _next: NextFunction, // underscore silences unused var checks +): void {src/routes/projects.ts (2)
16-19:uploadandsupabaseparameters are unusedBoth dependencies are injected but never referenced (the only usage is in a commented-out block). This is dead code and confuses readers.
Options:
- Remove the parameters for now.
- Uncomment / implement the photo-upload route.
-export default function projectsRouter( - upload: Multer, - supabase: SupabaseClient, -) { +export default function projectsRouter() {
21-23: Comment typo & misleading wording
// Getting all Usershould be “Get all projects (paginated?)” or similar. Minor but repeated inconsistencies hurt readability.src/types/project.d.ts (1)
1-2: Remove uselessexport {}
import "express";already marks this file as a module. The empty export is redundant and flagged by Biome.-import "express"; -export {}; +import "express";tests/members.test.ts (2)
60-103: Extract duplicate mock object to improve maintainability.The test logic is correct, but the large mock member object (lines 68-89) is duplicated in the next test case. Consider extracting it to a shared constant to follow DRY principles.
+const mockMemberResponse = { + id: "123", + name: "Test User", + email: "test@example.com", + phone: null, + bio: null, + profilePhoto: null, + github: "testgithub", + linkedin: null, + twitter: null, + leetcode: null, + codeforces: null, + codechef: null, + gfg: null, + geeksforgeeks: null, + passoutYear: new Date("2025-05-31"), + isManager: false, + isApproved: false, + approvedById: null, + createdAt: new Date(), + updatedAt: new Date(), +}; it('should update member and respond with 200', async () => { // ... existing setup ... - const mockMember = { - // ... large object ... - }; - const spy = jest.spyOn(memberService, 'updateMember').mockResolvedValue(mockMember); + const spy = jest.spyOn(memberService, 'updateMember').mockResolvedValue(mockMemberResponse);
18-153: Consider expanding test coverage for edge cases.The current test suite provides good coverage for the main scenarios. Consider adding tests for:
createAMemberwith file upload scenario- Error handling in
updateAMember(e.g., service failures)- File upload failures in both controllers
src/utils/imageUtils.ts (1)
4-4: Remove unused import.The imported
supabaseinstance from "../app" is not used anywhere in this file. The functions still acceptSupabaseClientas a parameter. Consider removing this unused import to avoid confusion.-import supabase from "../app";src/routes/members.ts (2)
4-4: Remove unused import.The
supabaseimport from"../app"is unnecessary since thesupabaseclient is passed as a parameter to themembersRouterfunction.-import { supabase } from "../app";
51-51: Fix inconsistent API path in documentation.The API path in the documentation shows
/api/v1/members/:memberIdbut should be/api/members/:memberIdto match the actual route structure.- * @api {patch} /api/v1/members/:memberId Update a member + * @api {patch} /api/members/:memberId Update a membersrc/controllers/project.controller.ts (1)
69-69: Fix inconsistent parameter name.The parameter name
ProjectId(capitalized) is inconsistent with the typical camelCase convention used elsewhere (projectId).- const projectId = parseInt(req.params.ProjectId); + const projectId = parseInt(req.params.projectId);doc/index.html (2)
8-16: Consider using relative paths for assets.The versioned query parameters (
?v=1753052856457) suggest cache busting, but hardcoded timestamps may cause issues during deployment. Consider using relative paths or environment-based versioning.- <link href="assets/bootstrap.min.css?v=1753052856457" rel="stylesheet" media="screen"> + <link href="assets/bootstrap.min.css" rel="stylesheet" media="screen">
1-1048: Consider accessibility improvements.The documentation interface should include better accessibility features such as:
- ARIA labels for interactive elements
- Keyboard navigation support
- Screen reader compatibility
- Focus management for dynamic content
src/services/member.service.ts (2)
62-62: Consider why name field is destructured separately.The
namefield is destructured separately but not used differently from other fields. This seems unnecessary unless there's specific processing planned.- const { name, ...rest } = payload; + const payload = payload;Or if there's a specific reason for handling name separately, add a comment explaining why.
74-79: Improve empty payload validation.The current validation converts object to JSON string which is inefficient. Use Object.keys() for better performance.
const dataToUpdate = Object.fromEntries( Object.entries(rest).filter(([_, v]) => v !== undefined), ); - if (JSON.stringify(dataToUpdate) === "{}") + if (Object.keys(dataToUpdate).length === 0) throw new ApiError("No fields passed", 400);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (13)
bun.lockis excluded by!**/*.lockdoc/assets/android-chrome-192x192.pngis excluded by!**/*.pngdoc/assets/android-chrome-512x512.pngis excluded by!**/*.pngdoc/assets/apple-touch-icon.pngis excluded by!**/*.pngdoc/assets/bootstrap.min.css.mapis excluded by!**/*.mapdoc/assets/favicon-16x16.pngis excluded by!**/*.pngdoc/assets/favicon-32x32.pngis excluded by!**/*.pngdoc/assets/favicon.icois excluded by!**/*.icodoc/assets/glyphicons-halflings-regular.eotis excluded by!**/*.eotdoc/assets/glyphicons-halflings-regular.svgis excluded by!**/*.svgdoc/assets/glyphicons-halflings-regular.ttfis excluded by!**/*.ttfdoc/assets/glyphicons-halflings-regular.woffis excluded by!**/*.woffdoc/assets/glyphicons-halflings-regular.woff2is excluded by!**/*.woff2
📒 Files selected for processing (26)
.env.example(1 hunks)doc/assets/main.css(1 hunks)doc/assets/prism-diff-highlight.css(1 hunks)doc/assets/prism-toolbar.css(1 hunks)doc/assets/prism.css(1 hunks)doc/index.html(1 hunks)prisma/migrations/20250718201903_init/migration.sql(0 hunks)prisma/schema.prisma(2 hunks)src/app.ts(1 hunks)src/config/index.ts(1 hunks)src/controllers/member.controller.ts(1 hunks)src/controllers/project.controller.ts(1 hunks)src/db/client.ts(1 hunks)src/routes/index.ts(2 hunks)src/routes/members.ts(1 hunks)src/routes/progress.ts(1 hunks)src/routes/projects.ts(1 hunks)src/server.ts(1 hunks)src/services/member.service.ts(1 hunks)src/services/project.service.ts(1 hunks)src/types/express.d.ts(2 hunks)src/types/members.d.ts(1 hunks)src/types/project.d.ts(1 hunks)src/utils/apiError.ts(2 hunks)src/utils/imageUtils.ts(1 hunks)tests/members.test.ts(1 hunks)
💤 Files with no reviewable changes (1)
- prisma/migrations/20250718201903_init/migration.sql
🧰 Additional context used
🧬 Code Graph Analysis (7)
src/services/project.service.ts (1)
src/controllers/project.controller.ts (7)
getProjectById(14-22)createProject(24-39)updateProjects(41-52)deleteProjects(54-65)getMembersByProjectId(67-78)addMembers(80-93)removeMembers(95-107)
src/routes/index.ts (1)
src/routes/members.ts (1)
membersRouter(7-151)
src/utils/imageUtils.ts (2)
src/utils/apiError.ts (1)
ApiError(7-18)src/app.ts (1)
supabase(13-16)
src/routes/members.ts (1)
src/app.ts (1)
supabase(13-16)
tests/members.test.ts (3)
src/controllers/member.controller.ts (2)
createAMember(28-52)updateAMember(55-70)src/utils/apiError.ts (1)
ApiError(7-18)src/utils/imageUtils.ts (1)
uploadImage(24-71)
src/controllers/member.controller.ts (4)
src/utils/apiError.ts (1)
ApiError(7-18)src/app.ts (1)
supabase(13-16)src/utils/imageUtils.ts (1)
uploadImage(24-71)src/services/member.service.ts (1)
unapprovedMembers(87-91)
src/services/member.service.ts (4)
src/db/client.ts (1)
prisma(5-5)src/types/members.d.ts (1)
UpdateMemberPayload(1-15)src/utils/apiError.ts (1)
ApiError(7-18)src/controllers/project.controller.ts (1)
getProjects(5-12)
🪛 Biome (1.9.4)
src/types/project.d.ts
[error] 1-2: This empty export is useless because there's another export or import.
This import makes useless the empty export.
Safe fix: Remove this useless empty export.
(lint/complexity/noUselessEmptyExport)
🪛 dotenv-linter (3.3.0)
.env.example
[warning] 8-8: [ExtraBlankLine] Extra blank line detected
[warning] 9-9: [SpaceCharacter] The line has spaces around equal sign
[warning] 9-9: [TrailingWhitespace] Trailing whitespace detected
[warning] 11-11: [UnorderedKey] The ALLOWED_ORIGINS key should go before the PORT key
[warning] 12-12: [KeyWithoutValue] The NODE_ENV key should be with a value or have an equal sign
[warning] 12-12: [UnorderedKey] The NODE_ENV key should go before the PORT key
🔇 Additional comments (27)
src/server.ts (1)
2-3: LGTM! Formatting improvements enhance consistency.The stylistic changes (double quotes, semicolons) improve code consistency across the codebase without affecting functionality.
Also applies to: 5-5, 8-9
src/db/client.ts (1)
5-6: LGTM! Added trailing newline follows best practices.Adding a newline at the end of the file follows standard formatting conventions.
src/routes/progress.ts (1)
39-40: LGTM! Formatting improvements are good.The proper indentation and semicolon addition improve code consistency.
src/config/index.ts (1)
7-8: LGTM! Stylistic improvements enhance consistency.The changes to double quotes, trailing comma, and proper statement termination with semicolon follow JavaScript/TypeScript best practices.
src/types/express.d.ts (1)
20-20:projectIdtype inconsistent with other declarations
projectIdis declared asstring | undefinedhere, whereas insrc/types/project.d.tsit’s anumber. Mixing types will force callers to cast or duplicate logic.Either:
- Change this field to
number | undefined, or- Use a union (
string | number) if both forms truly exist and document the rationale.doc/assets/prism-diff-highlight.css (1)
1-13: LGTM – sensible diff-highlight coloursThe selectors and colour choices match common Prism conventions. No issues spotted.
src/types/project.d.ts (1)
12-20:projectIdtyped asnumber– conflicts with Express request typeSee the mismatch noted in
src/types/express.d.ts. Align both sides to avoid widespreadas anycasts.src/routes/index.ts (2)
4-4: LGTM - Import statement looks correct.The import statement properly imports the default export from the members module.
15-15: LGTM - Router mounting is consistent.The router mounting follows the established pattern and correctly passes the required
uploadandsupabaseparameters as shown in themembersRouterfunction signature fromsrc/routes/members.ts.src/types/members.d.ts (1)
1-15: LGTM - Well-defined interface with comprehensive fields.The
UpdateMemberPayloadinterface is well-structured with appropriate optional fields for member profile updates. The interface covers personal details, social platform links, and approval-related fields, supporting flexible partial updates.doc/assets/prism-toolbar.css (1)
1-66: LGTM - Well-structured CSS with good accessibility considerations.The CSS provides clean styling for the code toolbar component with:
- Smooth opacity transitions for better UX
- Proper focus-within support with fallback comment for older browsers
- Consistent styling across toolbar items
- Good accessibility with focus states and user-select controls
doc/assets/prism.css (1)
1-123: LGTM - Standard Prism.js theme with comprehensive token styling.This is a well-implemented Prism.js theme providing:
- Proper base styling for code elements with good typography
- Comprehensive token color assignments for syntax highlighting
- Support for multiple languages (JavaScript, CoffeeScript, CSS, HTML)
- Appropriate styling for both inline and block code
- Good accessibility with readable color contrasts
tests/members.test.ts (4)
1-16: LGTM! Well-structured test setup.The imports, mocking strategy, and helper functions are properly configured for unit testing the member controller functions.
23-44: Excellent test coverage for successful member creation.The test properly validates the happy path with realistic mock data and comprehensive assertions.
46-52: Good error handling test coverage.The test correctly validates that missing required fields trigger the appropriate ApiError with status 402, matching the controller implementation.
105-152: Good coverage for file upload scenario.The test properly validates the image upload flow, including mocking the uploadImage function and verifying that the profilePhoto field is correctly added to the update body. The duplicate mock object should be addressed as mentioned in the previous comment.
src/utils/imageUtils.ts (1)
1-86: Excellent formatting improvements.The consistent use of double quotes, semicolons, and improved spacing enhances code readability and maintainability. These stylistic changes align well with modern TypeScript conventions.
src/app.ts (2)
34-37: Good addition of file size limits.The 2MB file size limit is a sensible security measure for image uploads, preventing abuse while allowing reasonable profile photo sizes.
2-31: Excellent formatting improvements.The consistent use of double quotes, semicolons, and improved spacing enhances code readability and follows modern TypeScript conventions.
Also applies to: 43-58
prisma/schema.prisma (3)
12-47: Excellent schema organization improvements.The field grouping and logical ordering (basic info → social links → audit fields → relations) significantly improves readability and maintainability while preserving all functionality.
32-46: Good practice with explicit relation annotations.The explicit relation naming (e.g.,
@relation("AchievementCreatedBy")) and consistentonDelete: Cascadeconfiguration improve schema clarity and prevent potential naming conflicts.Also applies to: 75-77, 83-84, 100-101, 107-108, 122-123, 137-139, 150-150, 156-157
162-172: Clean enum organization.Moving the enums to the end of the file with consistent formatting improves overall schema structure and readability.
doc/assets/main.css (4)
8-20: Excellent use of CSS custom properties.The well-organized color system with semantic naming and HTTP method-specific colors follows modern CSS best practices and common API documentation conventions.
276-339: Well-implemented responsive design.The off-canvas navigation pattern with smooth animations and proper mobile breakpoints provides excellent user experience across devices. The hamburger menu animation is a nice touch.
170-196: Appropriate HTTP method styling.The method badges with industry-standard colors (GET=green, POST=blue, DELETE=red) provide clear visual distinction and follow common API documentation conventions.
586-623: Thoughtful print stylesheet implementation.The print styles appropriately hide navigation elements, display link URLs, and optimize colors and spacing for printed documentation. This enhances the documentation's accessibility and offline usability.
doc/index.html (1)
1045-1045: Main JavaScript bundle integrity confirmed
- Verified that
doc/assets/main.bundle.jsexists and matches the versioned reference indoc/index.html.No further action required.
| if (!email || !name || !password || !passoutYear) { | ||
| throw new ApiError("Required fields absent", 402); | ||
| } |
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.
🛠️ Refactor suggestion
Add input validation and sanitization.
The controller should validate and sanitize input fields to prevent injection attacks and ensure data integrity.
const { email, name, password, passoutYear } = req.body;
+ // Add input validation
+ if (!email?.trim() || !name?.trim() || !password?.trim()) {
+ throw new ApiError("Required fields cannot be empty", 400);
+ }
+
+ // Validate email format
+ const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
+ if (!emailRegex.test(email)) {
+ throw new ApiError("Invalid email format", 400);
+ }
if (!email || !name || !password || !passoutYear) {
throw new ApiError("Required fields absent", 402);
}🤖 Prompt for AI Agents
In src/controllers/member.controller.ts around lines 32 to 34, the input fields
email, name, password, and passoutYear are checked only for presence but lack
validation and sanitization. Add validation to ensure each field meets expected
formats (e.g., valid email format, password strength, numeric passoutYear) and
sanitize inputs to remove or escape harmful characters to prevent injection
attacks and maintain data integrity before proceeding with further processing.
src/services/project.service.ts
Outdated
| export const createProject = async (projectContent: projectContent) => { | ||
| return await prisma.project.create({ | ||
| data: { | ||
| name: projectContent.name, | ||
| imageUrl: projectContent.imageUrl, | ||
| githubUrl: projectContent.githubUrl, | ||
| deployUrl: projectContent.deployUrl, | ||
| }, | ||
| }); | ||
| }; |
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.
Add missing type definitions and adminId field.
The function is missing the projectContent type definition and doesn't handle the adminId field that's passed from the controller.
Looking at the controller in src/controllers/project.controller.ts (lines 20-28), it passes an adminId field that isn't being used in the service:
-export const createProject = async (projectContent: projectContent) => {
+export const createProject = async (projectContent: {
+ name: string;
+ imageUrl?: string;
+ githubUrl?: string;
+ deployUrl?: string;
+ adminId?: number;
+}) => {
return await prisma.project.create({
data: {
name: projectContent.name,
imageUrl: projectContent.imageUrl,
githubUrl: projectContent.githubUrl,
deployUrl: projectContent.deployUrl,
+ adminId: projectContent.adminId,
},
});
};📝 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.
| export const createProject = async (projectContent: projectContent) => { | |
| return await prisma.project.create({ | |
| data: { | |
| name: projectContent.name, | |
| imageUrl: projectContent.imageUrl, | |
| githubUrl: projectContent.githubUrl, | |
| deployUrl: projectContent.deployUrl, | |
| }, | |
| }); | |
| }; | |
| export const createProject = async (projectContent: { | |
| name: string; | |
| imageUrl?: string; | |
| githubUrl?: string; | |
| deployUrl?: string; | |
| adminId?: number; | |
| }) => { | |
| return await prisma.project.create({ | |
| data: { | |
| name: projectContent.name, | |
| imageUrl: projectContent.imageUrl, | |
| githubUrl: projectContent.githubUrl, | |
| deployUrl: projectContent.deployUrl, | |
| adminId: projectContent.adminId, | |
| }, | |
| }); | |
| }; |
🤖 Prompt for AI Agents
In src/services/project.service.ts around lines 15 to 24, the createProject
function lacks the type definition for the projectContent parameter and does not
include the adminId field that is passed from the controller. To fix this,
define the projectContent type properly including all expected fields, and
update the prisma.project.create call to include adminId in the data object so
it is saved correctly.
src/services/project.service.ts
Outdated
| export const updateProjects = async ( | ||
| payload: updateContent, | ||
| projectId: number, | ||
| ) => { | ||
| const data = payload; | ||
| return await prisma.project.update({ | ||
| where: { | ||
| id: projectId, | ||
| }, | ||
| data, | ||
| }); | ||
| }; |
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.
Add missing type definition.
The updateContent type is not defined.
-export const updateProjects = async (
- payload: updateContent,
- projectId: number,
-) => {
+export const updateProjects = async (
+ payload: {
+ name?: string;
+ imageUrl?: string;
+ githubUrl?: string;
+ deployUrl?: string;
+ adminId?: number;
+ },
+ projectId: 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.
| export const updateProjects = async ( | |
| payload: updateContent, | |
| projectId: number, | |
| ) => { | |
| const data = payload; | |
| return await prisma.project.update({ | |
| where: { | |
| id: projectId, | |
| }, | |
| data, | |
| }); | |
| }; | |
| export const updateProjects = async ( | |
| payload: { | |
| name?: string; | |
| imageUrl?: string; | |
| githubUrl?: string; | |
| deployUrl?: string; | |
| adminId?: number; | |
| }, | |
| projectId: number, | |
| ) => { | |
| const data = payload; | |
| return await prisma.project.update({ | |
| where: { | |
| id: projectId, | |
| }, | |
| data, | |
| }); | |
| }; |
🤖 Prompt for AI Agents
In src/services/project.service.ts around lines 26 to 37, the type
`updateContent` used for the `payload` parameter is not defined. Define the
`updateContent` type or interface to specify the expected structure of the
update payload, then import or declare it in this file. This will ensure proper
type checking and clarity of the data being passed to the updateProjects
function.
src/services/project.service.ts
Outdated
| export const addMembers = async (addMembersData: addMembersData) => { | ||
| return await prisma.memberProject.create({ | ||
| data: { | ||
| memberId: addMembersData.memberId, | ||
| projectId: addMembersData.projectId, | ||
| }, | ||
| }); | ||
| }; |
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.
Add missing type definition.
The addMembersData type is not defined.
-export const addMembers = async (addMembersData: addMembersData) => {
+export const addMembers = async (addMembersData: {
+ memberId: number;
+ projectId: 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.
| export const addMembers = async (addMembersData: addMembersData) => { | |
| return await prisma.memberProject.create({ | |
| data: { | |
| memberId: addMembersData.memberId, | |
| projectId: addMembersData.projectId, | |
| }, | |
| }); | |
| }; | |
| export const addMembers = async (addMembersData: { | |
| memberId: number; | |
| projectId: number; | |
| }) => { | |
| return await prisma.memberProject.create({ | |
| data: { | |
| memberId: addMembersData.memberId, | |
| projectId: addMembersData.projectId, | |
| }, | |
| }); | |
| }; |
🤖 Prompt for AI Agents
In src/services/project.service.ts around lines 55 to 62, the parameter
addMembersData is used without a defined type. Define the addMembersData type or
interface specifying the expected properties (memberId and projectId) and their
types, then use it to type the addMembersData parameter in the addMembers
function to ensure proper type checking.
src/services/project.service.ts
Outdated
| export const removeMembers = async (removedMemberData: removedMemberData) => { | ||
| return await prisma.memberProject.delete({ | ||
| where: { | ||
| memberId_projectId: { | ||
| projectId: removedMemberData.projectId, | ||
| memberId: removedMemberData.memberId, | ||
| }, | ||
| }, | ||
| }); | ||
| }; |
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.
Add missing type definition.
The removedMemberData type is not defined.
-export const removeMembers = async (removedMemberData: removedMemberData) => {
+export const removeMembers = async (removedMemberData: {
+ memberId: number;
+ projectId: 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.
| export const removeMembers = async (removedMemberData: removedMemberData) => { | |
| return await prisma.memberProject.delete({ | |
| where: { | |
| memberId_projectId: { | |
| projectId: removedMemberData.projectId, | |
| memberId: removedMemberData.memberId, | |
| }, | |
| }, | |
| }); | |
| }; | |
| export const removeMembers = async (removedMemberData: { | |
| memberId: number; | |
| projectId: number; | |
| }) => { | |
| return await prisma.memberProject.delete({ | |
| where: { | |
| memberId_projectId: { | |
| projectId: removedMemberData.projectId, | |
| memberId: removedMemberData.memberId, | |
| }, | |
| }, | |
| }); | |
| }; |
🤖 Prompt for AI Agents
In src/services/project.service.ts around lines 64 to 73, the parameter
removedMemberData lacks a type definition. Define an appropriate TypeScript
interface or type for removedMemberData that includes the properties projectId
and memberId with their correct types, then use this type to annotate the
removedMemberData parameter in the removeMembers function.
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: 0
🧹 Nitpick comments (1)
.gitignore (1)
143-144: Tiny wording / consistency nitThe comment reads “documentation file” (singular) while the rule ignores an entire directory. Consider rephrasing for clarity:
-# documentation file +# generated API documentation
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.gitignore(1 hunks)
🔇 Additional comments (1)
.gitignore (1)
142-144: Add is good, but ensure the already-trackeddoc/files are purged
.gitignorestops future additions, it does not untrack thedoc/assets that were committed earlier in this PR.
Run a cached remove before merging, otherwise those bulky files remain in history:git rm -r --cached doc/ git commit -m "chore: remove generated docs now ignored by .gitignore"
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: 3
♻️ Duplicate comments (3)
src/controllers/member.controller.ts (3)
30-34: Add input validation and sanitization.The controller should validate and sanitize input fields to prevent injection attacks and ensure data integrity.
const { email, name, password, passoutYear } = req.body; + // Add input validation + if (!email?.trim() || !name?.trim() || !password?.trim()) { + throw new ApiError("Required fields cannot be empty", 400); + } + + // Validate email format + const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; + if (!emailRegex.test(email)) { + throw new ApiError("Invalid email format", 400); + } if (!email || !name || !password || !passoutYear) { throw new ApiError("Required fields absent", 402); }
36-39: Add error handling for image upload.The image upload operation should be wrapped in try-catch to handle potential upload failures gracefully.
let imageUrl: string | undefined; if (req.file) { - imageUrl = await uploadImage(supabase, req.file, "members"); + try { + imageUrl = await uploadImage(supabase, req.file, "members"); + } catch (error) { + throw new ApiError("Image upload failed", 500); + } }
17-17: Add input validation for memberId parameter.Several functions accept
memberIdfrom route parameters but don't validate the format. Consider adding a validation helper function.// Add at the top of the file after imports const validateMemberId = (memberId: string) => { if (!memberId?.trim()) { throw new ApiError("No memberId provided", 400); } // Add format validation if using UUIDs or specific format // Example for UUID validation: // const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; // if (!uuidRegex.test(memberId)) { // throw new ApiError("Invalid memberId format", 400); // } }; // Then use in each function: // validateMemberId(memberId);Also applies to: 57-57, 88-88, 110-110, 120-120, 130-130
🧹 Nitpick comments (1)
src/controllers/member.controller.ts (1)
1-6: Add TypeScript interfaces for request bodies.Consider adding interfaces to ensure type safety for request bodies across all controller functions.
// Add these interfaces after imports interface CreateMemberBody { email: string; name: string; password: string; passoutYear: number; } interface UpdateRequestBody { isApproved: boolean; adminId: string; } // Then use in functions: // const body = req.body as CreateMemberBody;
| const { email, name, password, passoutYear } = req.body; | ||
|
|
||
| if (!email || !name || !password || !passoutYear) { | ||
| throw new ApiError("Required fields absent", 402); |
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.
Use correct HTTP status code.
HTTP status code 402 is reserved for "Payment Required". Use 400 (Bad Request) for missing required fields.
- throw new ApiError("Required fields absent", 402);
+ throw new ApiError("Required fields absent", 400);📝 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.
| throw new ApiError("Required fields absent", 402); | |
| if (!email || !name || !password || !passoutYear) { | |
| throw new ApiError("Required fields absent", 400); | |
| } |
🤖 Prompt for AI Agents
In src/controllers/member.controller.ts at line 33, the HTTP status code 402 is
incorrectly used for missing required fields. Change the status code in the
ApiError constructor from 402 to 400 to correctly represent a Bad Request error.
| if (req.file) { | ||
| const oldData = await memberService.getDetails(memberId); | ||
| const oldImage = oldData?.profilePhoto; | ||
|
|
||
| if(oldImage) await deleteImage(supabase, oldImage); | ||
|
|
||
| const imageUrl = await uploadImage(supabase, req.file, "members"); | ||
| body.profilePhoto = imageUrl; | ||
| } |
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.
Add error handling and ensure atomicity for image operations.
The current implementation deletes the old image before uploading the new one. If the upload fails, the user loses their profile photo. Consider uploading the new image first, then deleting the old one only after successful upload.
if (req.file) {
const oldData = await memberService.getDetails(memberId);
const oldImage = oldData?.profilePhoto;
- if(oldImage) await deleteImage(supabase, oldImage);
-
- const imageUrl = await uploadImage(supabase, req.file, "members");
- body.profilePhoto = imageUrl;
+ try {
+ const imageUrl = await uploadImage(supabase, req.file, "members");
+ body.profilePhoto = imageUrl;
+
+ // Delete old image only after successful upload
+ if(oldImage) {
+ try {
+ await deleteImage(supabase, oldImage);
+ } catch (deleteError) {
+ // Log error but don't fail the request
+ console.error("Failed to delete old image:", deleteError);
+ }
+ }
+ } catch (uploadError) {
+ throw new ApiError("Image upload failed", 500);
+ }
}📝 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.
| if (req.file) { | |
| const oldData = await memberService.getDetails(memberId); | |
| const oldImage = oldData?.profilePhoto; | |
| if(oldImage) await deleteImage(supabase, oldImage); | |
| const imageUrl = await uploadImage(supabase, req.file, "members"); | |
| body.profilePhoto = imageUrl; | |
| } | |
| if (req.file) { | |
| const oldData = await memberService.getDetails(memberId); | |
| const oldImage = oldData?.profilePhoto; | |
| try { | |
| // Upload new image first | |
| const imageUrl = await uploadImage(supabase, req.file, "members"); | |
| body.profilePhoto = imageUrl; | |
| // Delete old image only after successful upload | |
| if (oldImage) { | |
| try { | |
| await deleteImage(supabase, oldImage); | |
| } catch (deleteError) { | |
| // Log error but don't fail the request | |
| console.error("Failed to delete old image:", deleteError); | |
| } | |
| } | |
| } catch (uploadError) { | |
| throw new ApiError("Image upload failed", 500); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/controllers/member.controller.ts around lines 63 to 71, the code deletes
the old profile image before uploading the new one, risking loss of the photo if
the upload fails. Modify the logic to first upload the new image and only upon
successful upload, delete the old image. Add error handling to manage failures
during upload or deletion to ensure the operation is atomic and the user's
profile photo is not lost.
| throw new ApiError("No essential creds provided", 400); | ||
| } | ||
|
|
||
| if(!isApproved) throw new ApiError("Someone interrupting the backend flow", 400); |
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.
Function only allows approval, not rejection.
The current logic throws an error when isApproved is false, which means members can only be approved, not rejected. If this is intentional, consider renaming the function to approveMember. Otherwise, remove this check to allow both approval and rejection.
- if(!isApproved) throw new ApiError("Someone interrupting the backend flow", 400);
+ // Remove this line to allow both approval and rejection📝 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.
| if(!isApproved) throw new ApiError("Someone interrupting the backend flow", 400); | |
| - if(!isApproved) throw new ApiError("Someone interrupting the backend flow", 400); | |
| + // Remove this line to allow both approval and rejection |
🤖 Prompt for AI Agents
In src/controllers/member.controller.ts at line 95, the code throws an error if
isApproved is false, preventing rejection of members. To fix this, either rename
the function to approveMember if only approval is intended, or remove the error
check to allow both approval and rejection flows.
src/routes/members.ts
Outdated
| * @apiSuccess {Object} user Member object. | ||
| * @apiError (Error 400) BadRequest No memberId provided. | ||
| */ | ||
| router.get("/details/:memberId", memberCtrl.getUserDetails); |
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.
@shrutiiiyet change name of this endpoint same as api docs
src/routes/members.ts
Outdated
| const router = express.Router(); | ||
|
|
||
| /** | ||
| * @api {get} /api/members/details/:memberId Get a member's details |
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.
@shrutiiiyet remove api from annotations
src/routes/members.ts
Outdated
| router.get("/details/:memberId", memberCtrl.getUserDetails); | ||
|
|
||
| /** | ||
| * @api {get} /api/members List all approved members |
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.
@shrutiiiyet remove api from annotations
src/routes/members.ts
Outdated
| router.get("/", memberCtrl.listAllApprovedMembers); | ||
|
|
||
| /** | ||
| * @api {post} /api/members Create a new member |
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.
@shrutiiiyet remove api from annotations
src/routes/members.ts
Outdated
| router.post("/", upload.single("file"), memberCtrl.createAMember(supabase)); | ||
|
|
||
| /** | ||
| * @api {patch} /api/v1/members/:memberId Update a member |
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.
@shrutiiiyet remove api from annotations
src/routes/members.ts
Outdated
| ); | ||
|
|
||
| /** | ||
| * @api {get} /api/members/unapproved Get unapproved members |
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.
@shrutiiiyet remove api from annotations
src/routes/members.ts
Outdated
| router.get("/unapproved", memberCtrl.getUnapprovedMembers); | ||
|
|
||
| /** | ||
| * @api {patch} /api/members/approve/:memberId Approve/reject a member |
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.
@shrutiiiyet remove api from annotations
src/routes/members.ts
Outdated
| router.patch("/approve/:memberId", memberCtrl.updateRequest); | ||
|
|
||
| /** | ||
| * @api {get} /api/members/achievements/:memberId Get member's achievements |
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.
@shrutiiiyet remove api from annotations
| router.get("/achievements/:memberId", memberCtrl.getUserAchievements); | ||
|
|
||
| /** | ||
| * @api {get} /api/members/projects/:memberId Get member's projects |
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.
@shrutiiiyet remove api from annotations
src/routes/members.ts
Outdated
| router.get("/projects/:memberId", memberCtrl.getUserProjects); | ||
|
|
||
| /** | ||
| * @api {get} /api/members/interviews/:memberId Get member's interviews |
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.
@shrutiiiyet remove api from annotations
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: 0
🔭 Outside diff range comments (2)
src/types/project.d.ts (2)
5-18: Fix duplicate type declaration that will cause compilation errors.There are two
projectContenttype declarations in the global namespace, which will cause TypeScript compilation errors. The types also have incompatible structures:
- Lines 5-10: All required properties, no
AdminId- Lines 12-18: Optional
deployUrl, includesAdminIdThis suggests incomplete refactoring where the old type wasn't removed when the new one was added.
Either consolidate these into a single type or rename one of them:
Option 1: Remove the duplicate and standardize the remaining type
- type projectContent = { - name: string; - imageUrl: string; - githubUrl: string; - deployUrl: string; - }; type projectContent = { - name: string, - imageUrl: string , - githubUrl: string, - deployUrl: string?, - AdminId : string + name: string; + imageUrl: string; + githubUrl: string; + deployUrl?: string; + createdById: string; }Option 2: Rename the types to reflect their specific purposes
- type projectContent = { + type createProjectContent = { name: string; imageUrl: string; githubUrl: string; deployUrl: string; }; - type projectContent = { + type updateProjectContent = { - name: string, - imageUrl: string , - githubUrl: string, - deployUrl: string?, - AdminId : string + name: string; + imageUrl: string; + githubUrl: string; + deployUrl?: string; + createdById: string; }
5-39: Fix inconsistentprojectContenttype declarations and usageYour
projectContentalias is declared twice insrc/types/project.d.tswith mismatched properties (AdminIdvs. code usingcreatedById), which is why the service mapping is commented out and could lead to runtime errors. Please address the following:
- src/types/project.d.ts
- Remove the duplicate
projectContentaliases and replace them with a single interface:interface ProjectContent { name: string; imageUrl: string; githubUrl: string; deployUrl?: string; createdById: string; }- src/services/project.service.ts
- Uncomment and update the Prisma create call to use the new property:
return await prisma.project.create({ data: { name: projectContent.name, imageUrl: projectContent.imageUrl, githubUrl: projectContent.githubUrl, deployUrl: projectContent.deployUrl, createdById: projectContent.createdById, }, });- src/controllers/project.controller.ts
- Ensure you build the payload with
createdById, e.g.:const projectContent: ProjectContent = { name: req.body.projectData.name, imageUrl, githubUrl: req.body.projectData.githubUrl, deployUrl: req.body.projectData.deployUrl, createdById: req.body.projectData.adminId, };- src/routes/projects.ts & tests/Project.test.ts
- Update any OpenAPI docs and tests that reference
AdminIdto usecreatedByIdfor consistency.
🧹 Nitpick comments (1)
src/types/project.d.ts (1)
12-18: Fix formatting inconsistencies and property naming.The type definition has inconsistent formatting and property naming issues:
- Mixed use of commas and semicolons as property separators
- Inconsistent spacing around colons
- Property name
AdminIdshould follow camelCase convention (adminIdor usecreatedByIdfor clarity)Apply consistent formatting:
type projectContent = { - name: string, - imageUrl: string , - githubUrl: string, - deployUrl: string?, - AdminId : string + name: string; + imageUrl: string; + githubUrl: string; + deployUrl?: string; + createdById: string; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
prisma/migrations/20250719123800_image_url_githuburk_make_complusory/migration.sql(2 hunks)prisma/schema.prisma(3 hunks)src/app.ts(1 hunks)src/controllers/project.controller.ts(3 hunks)src/routes/index.ts(3 hunks)src/routes/members.ts(1 hunks)src/routes/projects.ts(1 hunks)src/services/project.service.ts(1 hunks)src/types/project.d.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/routes/projects.ts
- src/routes/members.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- src/app.ts
- prisma/migrations/20250719123800_image_url_githuburk_make_complusory/migration.sql
- src/routes/index.ts
- prisma/schema.prisma
- src/controllers/project.controller.ts
- src/services/project.service.ts
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: 0
🔭 Outside diff range comments (1)
src/types/project.d.ts (1)
10-10: Syntax error: use?after the property name, not the type
deployUrl: string?is invalid TypeScript and causes the parser error reported by Biome.
Replace it with the correct optional-property syntax:- deployUrl: string?, + deployUrl?: string,
🧹 Nitpick comments (1)
src/types/project.d.ts (1)
33-33: Re-add the trailing newline (style / tooling consistency)The sole change in this file was removal of the final newline. Many editors and CI linters (e.g. EditorConfig, ESLint’s
eol-last) expect a newline at EOF. Consider restoring it to avoid needless diffs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/controllers/project.controller.ts(0 hunks)src/routes/projects.ts(1 hunks)src/services/project.service.ts(1 hunks)src/types/project.d.ts(1 hunks)tests/Project.test.ts(0 hunks)
🪛 Biome (1.9.4)
src/types/project.d.ts
[error] 32-33: Expected a statement but instead found '}'.
Expected a statement here.
(parse)
💤 Files with no reviewable changes (2)
- tests/Project.test.ts
- src/controllers/project.controller.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/services/project.service.ts
- src/routes/projects.ts
🧰 Additional context used
🪛 Biome (1.9.4)
src/types/project.d.ts
[error] 32-33: Expected a statement but instead found '}'.
Expected a statement here.
(parse)
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: 1
🧹 Nitpick comments (1)
tests/members.test.ts (1)
79-100: Eliminate duplicate mock objects.The same mock member object is defined twice. Consider extracting it to a shared constant to improve maintainability.
+const mockMemberData = { + id: "123", + name: "Test User", + email: "test@example.com", + phone: null, + bio: null, + profilePhoto: null, + github: "testgithub", + linkedin: null, + twitter: null, + leetcode: null, + codeforces: null, + codechef: null, + gfg: null, + geeksforgeeks: null, + passoutYear: new Date("2025-05-31"), + isManager: false, + isApproved: false, + approvedById: null, + createdAt: new Date(), + updatedAt: new Date(), +}; - const mockMember = { - id: "123", - name: "Test User", - // ... rest of properties - }; + const mockMember = mockMemberData; -const spy = jest.spyOn(memberService, 'updateMember').mockResolvedValue(mockMember); +const spy = jest.spyOn(memberService, 'updateMember').mockResolvedValue(mockMemberData);Also applies to: 127-148
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/routes/members.ts(1 hunks)tests/members.test.ts(1 hunks)
🧬 Code Graph Analysis (1)
tests/members.test.ts (3)
src/controllers/member.controller.ts (2)
createAMember(28-52)updateAMember(55-78)src/utils/apiError.ts (1)
ApiError(7-18)src/utils/imageUtils.ts (1)
uploadImage(23-70)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/routes/members.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/members.test.ts (3)
src/controllers/member.controller.ts (2)
createAMember(28-52)updateAMember(55-78)src/utils/apiError.ts (1)
ApiError(7-18)src/utils/imageUtils.ts (1)
uploadImage(23-70)
🔇 Additional comments (3)
tests/members.test.ts (3)
1-21: Well-structured test imports and mocking setup.The imports cover all necessary dependencies, and the mock configurations for Supabase storage and imageUtils are properly set up to isolate the controller logic during testing.
22-27: Good implementation of response mock helper.The
mockResponsefunction correctly simulates Express Response behavior with proper method chaining support.
29-64: Comprehensive test coverage for createAMember controller.The test suite properly covers both the success path with valid input and the error case for missing required fields. The mock setup and assertions correctly verify the controller's behavior.
| it('should upload image and update profilePhoto', async () => { | ||
| const req = { | ||
| params: { memberId: 'abc-123' }, | ||
| body: {}, | ||
| file: { buffer: Buffer.from('data') }, | ||
| } as unknown as Request; | ||
|
|
||
| const res = mockResponse(); | ||
|
|
||
| (uploadImage as jest.Mock).mockResolvedValue('http://image.url'); | ||
|
|
||
| const mockMember = { | ||
| id: "123", | ||
| name: "Test User", | ||
| email: "test@example.com", | ||
| phone: null, | ||
| bio: null, | ||
| profilePhoto: null, | ||
| github: "testgithub", | ||
| linkedin: null, | ||
| twitter: null, | ||
| leetcode: null, | ||
| codeforces: null, | ||
| codechef: null, | ||
| gfg: null, | ||
| geeksforgeeks: null, | ||
| passoutYear: new Date("2025-05-31"), | ||
| isManager: false, | ||
| isApproved: false, | ||
| approvedById: null, | ||
| createdAt: new Date(), | ||
| updatedAt: new Date(), | ||
| }; | ||
|
|
||
| const spy = jest.spyOn(memberService, 'updateMember').mockResolvedValue(mockMember); | ||
|
|
||
|
|
||
| const handler = updateAMember(mockSupabase); | ||
| await handler(req, res); | ||
|
|
||
| expect(uploadImage).toHaveBeenCalled(); | ||
| expect(spy).toHaveBeenCalledWith('abc-123', { profilePhoto: 'http://image.url' }); | ||
| expect(res.status).toHaveBeenCalledWith(200); | ||
| expect(res.json).toHaveBeenCalledWith({ | ||
| success: true, | ||
| message: 'Updated member details successfully', | ||
| }); | ||
| }); |
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.
Missing mocks for image update workflow.
The test for image upload scenario is missing required mocks. According to the controller code, when a file is provided, it calls memberService.getDetails() and potentially deleteImage(), but these aren't mocked in the test.
Add the missing mocks:
it('should upload image and update profilePhoto', async () => {
+ jest.spyOn(memberService, 'getDetails').mockResolvedValue({ profilePhoto: 'old-image-url' });
+ // Mock deleteImage if it's imported, or mock the module that exports it
const req = {
params: { memberId: 'abc-123' },
body: {},
file: { buffer: Buffer.from('data') },
} as unknown as Request;📝 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.
| it('should upload image and update profilePhoto', async () => { | |
| const req = { | |
| params: { memberId: 'abc-123' }, | |
| body: {}, | |
| file: { buffer: Buffer.from('data') }, | |
| } as unknown as Request; | |
| const res = mockResponse(); | |
| (uploadImage as jest.Mock).mockResolvedValue('http://image.url'); | |
| const mockMember = { | |
| id: "123", | |
| name: "Test User", | |
| email: "test@example.com", | |
| phone: null, | |
| bio: null, | |
| profilePhoto: null, | |
| github: "testgithub", | |
| linkedin: null, | |
| twitter: null, | |
| leetcode: null, | |
| codeforces: null, | |
| codechef: null, | |
| gfg: null, | |
| geeksforgeeks: null, | |
| passoutYear: new Date("2025-05-31"), | |
| isManager: false, | |
| isApproved: false, | |
| approvedById: null, | |
| createdAt: new Date(), | |
| updatedAt: new Date(), | |
| }; | |
| const spy = jest.spyOn(memberService, 'updateMember').mockResolvedValue(mockMember); | |
| const handler = updateAMember(mockSupabase); | |
| await handler(req, res); | |
| expect(uploadImage).toHaveBeenCalled(); | |
| expect(spy).toHaveBeenCalledWith('abc-123', { profilePhoto: 'http://image.url' }); | |
| expect(res.status).toHaveBeenCalledWith(200); | |
| expect(res.json).toHaveBeenCalledWith({ | |
| success: true, | |
| message: 'Updated member details successfully', | |
| }); | |
| }); | |
| it('should upload image and update profilePhoto', async () => { | |
| jest.spyOn(memberService, 'getDetails').mockResolvedValue({ profilePhoto: 'old-image-url' }); | |
| // Mock deleteImage if it's imported | |
| jest.spyOn(supabase, 'deleteImage').mockResolvedValue(undefined); | |
| const req = { | |
| params: { memberId: 'abc-123' }, | |
| body: {}, | |
| file: { buffer: Buffer.from('data') }, | |
| } as unknown as Request; | |
| const res = mockResponse(); | |
| (uploadImage as jest.Mock).mockResolvedValue('http://image.url'); | |
| const mockMember = { | |
| id: "123", | |
| name: "Test User", | |
| email: "test@example.com", | |
| phone: null, | |
| bio: null, | |
| profilePhoto: null, | |
| github: "testgithub", | |
| linkedin: null, | |
| twitter: null, | |
| leetcode: null, | |
| codeforces: null, | |
| codechef: null, | |
| gfg: null, | |
| geeksforgeeks: null, | |
| passoutYear: new Date("2025-05-31"), | |
| isManager: false, | |
| isApproved: false, | |
| approvedById: null, | |
| createdAt: new Date(), | |
| updatedAt: new Date(), | |
| }; | |
| const spy = jest.spyOn(memberService, 'updateMember').mockResolvedValue(mockMember); | |
| const handler = updateAMember(mockSupabase); | |
| await handler(req, res); | |
| expect(uploadImage).toHaveBeenCalled(); | |
| expect(spy).toHaveBeenCalledWith('abc-123', { profilePhoto: 'http://image.url' }); | |
| expect(res.status).toHaveBeenCalledWith(200); | |
| expect(res.json).toHaveBeenCalledWith({ | |
| success: true, | |
| message: 'Updated member details successfully', | |
| }); | |
| }); |
🤖 Prompt for AI Agents
In tests/members.test.ts between lines 116 and 163, the test for image upload
lacks mocks for memberService.getDetails and deleteImage, which are called in
the controller when a file is provided. Add mocks for memberService.getDetails
to return the current member details and for deleteImage to simulate image
deletion. This will ensure the test covers the full image update workflow and
prevents errors from missing mocks.
Summary by CodeRabbit
New Features
Refactor
Chores
Documentation
Style
Tests