Conversation
WalkthroughThis update introduces comprehensive member management features, including new controllers, services, and routes for handling members, achievements, projects, and profile photo uploads. It also refines Supabase integration by switching to service role keys, updates environment and Prisma configurations, and manages database schema changes related to the member password field. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ExpressRouter
participant MemberController
participant MemberService
participant PrismaClient
participant SupabaseClient
participant UploadService
Client->>ExpressRouter: POST /members (with profile photo)
ExpressRouter->>MemberController: createAMember / uploadProfilePicture
MemberController->>MemberService: createMember(...)
MemberService->>PrismaClient: Create Member & Account
MemberService-->>MemberController: Member created
MemberController->>UploadService: uploadImage(...)
UploadService->>SupabaseClient: Upload image to storage
SupabaseClient-->>UploadService: Public image URL
UploadService-->>MemberController: Image URL
MemberController->>PrismaClient: Update Member profilePhoto
MemberController-->>ExpressRouter: JSON response
ExpressRouter-->>Client: Success/Error JSON
Suggested reviewers
Poem
🪧 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.
Actionable comments posted: 17
🔭 Outside diff range comments (2)
prisma/migrations/20250715212622_added_password_field/migration.sql (1)
1-9: Redundant migration — adds a column only to be removed in the very next migrationThis migration adds
passwordasNOT NULL, immediately followed by20250716075628_which drops it.
Running both sequentially in production will cause unnecessary downtime and, worse, fail if the table already contains rows because theNOT NULLaddition can’t succeed.Consider squashing or deleting both migrations (or the second one) before shipping to keep the history clean and avoid deploy failures.
src/app.ts (1)
22-22: Validate ALLOWED_ORIGINS split operation.The code assumes
ALLOWED_ORIGINSis a comma-separated string, but there's no validation if the environment variable is undefined or malformed.- origin: config.ALLOWED_ORIGINS.split(','), + origin: config.ALLOWED_ORIGINS ? config.ALLOWED_ORIGINS.split(',') : ['*'],This prevents runtime errors if the environment variable is not set.
♻️ Duplicate comments (2)
prisma/migrations/20250716075628_/migration.sql (1)
1-9: Dropping freshly-addedpasswordcolumn — verify intentThis script irreversibly deletes any
passworddata created by the prior migration.
Unless you’ve already reverted that rollout, merge-time execution will:
- Apply
ADD COLUMN "password" TEXT NOT NULL- Immediately
DROP COLUMN "password"Result: service interruption plus wasted migration time.
Ensure the project truly no longer relies on the column and eliminate the flip-flop sequence as noted in the previous comment.
src/app.ts (1)
15-15: Consistent with previous service role key concerns.The service role key usage here aligns with the configuration change, but the same security concerns apply.
Refer to the security considerations raised in
src/config/index.tsregarding service role key usage.
🧹 Nitpick comments (18)
src/utils/apiError.ts (1)
41-44: Prefer shorthand property assignment for conciseness
message: messagecan be reduced to the object-literal shorthandmessage, matching the surrounding style and keeping the payload terse.const responseBody: ErrorResponse = { error: true, - message: message + message }src/db/client.ts (1)
1-2: Guard against multiple PrismaClient instances in serverless/hot-reload scenariosNode’s module cache usually protects you, but in serverless environments (Vercel, Netlify functions, etc.) or under
ts-nodewatch mode multiple instances can exhaust the Postgres connection limit.Typical pattern:
-import { PrismaClient } from "../generated/prisma"; -export const prisma = new PrismaClient(); +import { PrismaClient } from "../generated/prisma"; + +declare global { + // eslint-disable-next-line no-var + var prisma: PrismaClient | undefined; +} + +export const prisma = + global.prisma ?? + new PrismaClient({ + log: ['query', 'info', 'warn', 'error'], + }); + +if (process.env.NODE_ENV !== 'production') global.prisma = prisma;Adopt this guard if you deploy to environments that spin up on every request.
.env.example (3)
8-8: Remove extra blank line.Static analysis detected an unnecessary blank line that should be removed for consistency.
- - +
9-9: Fix formatting issues with PORT variable.The PORT variable has spacing issues and trailing whitespace that should be corrected.
-PORT= +PORT=
11-12: Fix variable ordering and NODE_ENV format.Multiple formatting issues need to be addressed:
- Variables should be in alphabetical order
- NODE_ENV is missing an equals sign and value
- Missing blank line at end of file
-ALLOWED_ORIGINS= -NODE_ENV +ALLOWED_ORIGINS= +NODE_ENV= +Consider reordering variables alphabetically:
ALLOWED_ORIGINS,NODE_ENV,PORT.src/routes/index.ts (2)
13-13: Remove commented code or add explanation.The projects router is commented out without explanation. Either remove this line entirely or add a comment explaining why it's disabled.
- //router.use('/projects', projectsRouter(upload, supabase))If projects functionality is being developed, consider adding a TODO comment with context.
16-18: Remove unnecessary blank lines.Multiple consecutive blank lines should be reduced to maintain consistent formatting.
- - - +src/services/upload.service.ts (2)
36-36: Review the use of upsert: true for potential unintended overwrites.The
upsert: trueoption will overwrite existing files at the same path. Since you're using UUID filenames, this shouldn't happen in practice, but consider if this behavior is intended.If overwrites are not intended, consider removing the upsert option:
- upsert: true,
15-54: Consider adding file size validation for better error handling.The function doesn't validate file size, which could lead to large uploads failing at the storage level. Consider adding size validation early in the process.
export async function uploadImage( supabase: SupabaseClient, file: MulterFile, folder: string ): Promise<string> { + // Validate file size (e.g., 5MB limit) + const maxSize = 5 * 1024 * 1024; // 5MB + if (file.size > maxSize) { + throw new ApiError('File size exceeds 5MB limit', 400) + } + // 1) Determine file extensionsrc/controllers/supabase.controller.ts (1)
11-30: Remove commented code or move to documentation.The commented example code should be removed from the source file or moved to proper documentation to keep the codebase clean.
Consider removing the commented code blocks and creating proper documentation instead.
src/routes/members.ts (1)
3-3: Remove unused import - supabase is passed as parameter.The
supabaseimport from '../app' is unused since the supabase client is passed as a parameter to themembersRouterfunction.-import { supabase } from '../app';src/controllers/member.controller.ts (4)
5-5: Remove unused import - supabase is passed as parameter.The
supabaseimport from '../app' is unused in theuploadProfilePicturefunction since it's passed as a parameter.-import { supabase } from '../app';
48-48: Fix typo in success message.There's a typo in "succesfully" - should be "successfully".
- res.status(200).json({success: true, message:"Updated member details succesfully"}); + res.status(200).json({success: true, message:"Updated member details successfully"});
108-108: Fix typo in success message.There's a typo in "succesfully" - should be "successfully".
- res.status(200).json({success: true, message: "Image uploaded succesfully"}); + res.status(200).json({success: true, message: "Image uploaded successfully"});
65-65: Remove debug console.log statement.Console.log statements should not be present in production code.
const update = await memberService.approveRequest(isApproved, adminId, memberId); - console.log(update); res.status(200).json({success :true, update, message: "Approve Request Checked"});src/services/member.service.ts (3)
77-77: Remove unused destructuring of name field.The
namefield is destructured but never used in the function.- const { name, ...rest } = payload + const { ...rest } = payloadOr if name needs special handling:
- const { name, ...rest } = payload + const { name, ...rest } = payload + + if (name !== undefined) { + // Add name validation/processing logic here + }
107-107: Remove debug console.log statement.Console.log statements should not be present in production code.
const admin = await checkAdmin(adminId); - console.log(admin);
88-90: Improve undefined filtering logic.The current filtering logic could be more explicit and type-safe.
- const dataToUpdate = Object.fromEntries( - Object.entries(rest).filter(([_, v]) => v !== undefined) - ); + const dataToUpdate = Object.fromEntries( + Object.entries(rest).filter(([_, value]) => value !== undefined && value !== null) + );
📜 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 (15)
.env.example(1 hunks)package.json(2 hunks)prisma/migrations/20250715212622_added_password_field/migration.sql(1 hunks)prisma/migrations/20250716075628_/migration.sql(1 hunks)prisma/schema.prisma(1 hunks)src/app.ts(2 hunks)src/config/index.ts(1 hunks)src/controllers/member.controller.ts(1 hunks)src/controllers/supabase.controller.ts(1 hunks)src/db/client.ts(1 hunks)src/routes/index.ts(1 hunks)src/routes/members.ts(1 hunks)src/services/member.service.ts(1 hunks)src/services/upload.service.ts(1 hunks)src/utils/apiError.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
src/services/upload.service.ts (2)
src/app.ts (1)
supabase(13-16)src/utils/apiError.ts (1)
ApiError(7-18)
src/routes/members.ts (2)
src/app.ts (1)
supabase(13-16)src/controllers/supabase.controller.ts (1)
injectSupabase(5-9)
src/controllers/supabase.controller.ts (2)
src/app.ts (1)
supabase(13-16)src/controllers/member.controller.ts (1)
uploadProfilePicture(99-109)
src/services/member.service.ts (2)
src/db/client.ts (1)
prisma(2-2)src/utils/apiError.ts (1)
ApiError(7-18)
src/controllers/member.controller.ts (4)
src/utils/apiError.ts (1)
ApiError(7-18)src/services/member.service.ts (1)
unapprovedMembers(98-102)src/app.ts (1)
supabase(13-16)src/services/upload.service.ts (1)
uploadImage(15-54)
🪛 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: [EndingBlankLine] No blank line at the end of the file
[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 (3)
prisma/schema.prisma (1)
12-16: Double-check environment variable alignment
urlstill reads fromDATABASE_URLwhiledirectUrlnow referencesSUPABASE_URL.
If the intention is to deprecateDATABASE_URL, update both fields (or keep them in sync) to avoid the ORM using two different connection strings.package.json (1)
45-46: UUID dependencies look appropriate for file upload functionality.The addition of
uuidand@types/uuidis well-suited for generating unique filenames in the upload functionality.Also applies to: 52-53
src/app.ts (1)
32-35: File upload configuration is well-implemented.The multer configuration with memory storage and 5MB limit is appropriate for profile photo uploads. The size limit prevents abuse while accommodating reasonable image sizes.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
package.json (1)
10-14: Scripts section still misses core workflowsPrevious review already requested
dev,build,start,lint,format, etc. You’ve addedapidoc, which is great, but the essential run/build scripts are still absent, forcing contributors to guess commands. Re-add the suggested baseline scripts or explain the chosen alternative.
🧹 Nitpick comments (1)
src/routes/index.ts (1)
5-6: Consider removing excessive empty lines.Multiple consecutive empty lines can impact code readability. Consider reducing them to single empty lines where appropriate.
- - +Also applies to: 16-18
📜 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 (4)
package.json(3 hunks)src/app.ts(2 hunks)src/db/client.ts(1 hunks)src/routes/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/app.ts
- src/db/client.ts
🧰 Additional context used
🪛 Biome (1.9.4)
package.json
[error] 60-60: The key ts-node was already declared.
This where a duplicated key was declared again.
If a key is defined multiple times, only the last definition takes effect. Previous definitions are ignored.
(lint/suspicious/noDuplicateObjectKeys)
🪛 GitHub Actions: Run Tests
package.json
[error] 62-62: ParserError parsing package.json: Syntax errors at line 62 including unexpected token and unexpected comma near "ts-jest": "^29.4.0".
🔇 Additional comments (1)
src/routes/index.ts (1)
12-12: LGTM! Members router activation aligns with PR objectives.The uncommenting of the members router correctly enables the member management functionality as intended by the PR objectives.
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
package.json (1)
58-70: 🚨 Invalid JSON + duplicated keys break the build– Missing comma after the newly-added
"uuid"entry
–ts-jestis now insidedependenciesbut should stay indevDependencies
–ts-nodeappears twice (line 67&line 70), violating JSON object key uniquenessThe pipeline is already failing to parse the file – fix immediately:
@@ - "@types/uuid": "^10.0.0", @@ - "multer": "^2.0.1", - "ts-node": "^10.9.2", - "uuid": "^11.1.0" - "ts-jest": "^29.4.0", - "ts-node": "^10.9.2" + "multer": "^2.0.1", + "uuid": "^11.1.0", + + // --- ⬇ leave **runtime** deps only below this line --- + // (no ts-node / ts-jest / @types/* here) } + +// move to devDependencies (below): +// "ts-jest": "^29.4.0", +// "ts-node": "^10.9.2", +// "@types/uuid": "^10.0.0",Then append the three moved items into the existing
"devDependencies"block (sorted alphabetically if you wish).
This restores valid JSON and aligns packages with their correct scopes.
♻️ Duplicate comments (2)
package.json (1)
67-67:ts-nodeduplicate was already flagged earlierSame issue re-introduced here; please remove it from
"dependencies"and keep a single entry under"devDependencies"only..env.example (1)
9-9: (repeat) Document service-role key security implications.
The service-role key grants superuser access; the file still lacks the required warning and handling guidance flagged in the previous review.
🧹 Nitpick comments (1)
.env.example (1)
8-8: Strip the trailing space and provide an example default forPORT.
The extra space breaks some dotenv loaders and may lead to an empty value at runtime.-PORT= +PORT=3000
📜 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 (3)
.env.example(1 hunks)package.json(3 hunks)prisma/schema.prisma(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- prisma/schema.prisma
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
.env.example
[warning] 8-8: [SpaceCharacter] The line has spaces around equal sign
[warning] 8-8: [TrailingWhitespace] Trailing whitespace detected
[warning] 10-10: [UnorderedKey] The ALLOWED_ORIGINS key should go before the PORT key
[warning] 11-11: [KeyWithoutValue] The NODE_ENV key should be with a value or have an equal sign
[warning] 11-11: [UnorderedKey] The NODE_ENV key should go before the PORT key
🪛 Biome (1.9.4)
package.json
[error] 67-67: The key ts-node was already declared.
This where a duplicated key was declared again.
If a key is defined multiple times, only the last definition takes effect. Previous definitions are ignored.
(lint/suspicious/noDuplicateObjectKeys)
🪛 GitHub Actions: Run Tests
package.json
[error] 69-69: ParserError parsing package.json: Expected "," but found "ts-jest"; Expected string but found ":"; Expected " =" but found "^29.4.0"; Unexpected comma at line 69.
| "@types/jest": "^30.0.0", | ||
| "@types/multer": "^2.0.0", | ||
| "@types/node": "^24.0.13", | ||
| "@types/uuid": "^10.0.0", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Move @types/uuid to devDependencies
Type declaration packages are not needed at runtime; shift them alongside your other @types/* devDeps to reduce production image size.
🤖 Prompt for AI Agents
In package.json at line 58, move the "@types/uuid" dependency from
"dependencies" to "devDependencies" because type declaration packages are only
needed during development and not at runtime. This change will help reduce the
production image size by excluding unnecessary packages.
| # Environment variables for the application | ||
| NODE_ENV=development | ||
| ALLOWED_ORIGINS= | ||
| NODE_ENV |
There was a problem hiding this comment.
NODE_ENV key is malformed (missing =).
Without an equals sign the variable is ignored, which can force the app into production defaults unexpectedly.
-NODE_ENV
+NODE_ENV=development📝 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.
| NODE_ENV | |
| # .env.example (around line 11) | |
| -NODE_ENV | |
| +NODE_ENV=development |
🧰 Tools
🪛 dotenv-linter (3.3.0)
[warning] 11-11: [KeyWithoutValue] The NODE_ENV key should be with a value or have an equal sign
[warning] 11-11: [UnorderedKey] The NODE_ENV key should go before the PORT key
🤖 Prompt for AI Agents
In the .env.example file at line 11, the NODE_ENV environment variable is
missing an equals sign, making it malformed and ignored by the app. Add an
equals sign after NODE_ENV, for example, NODE_ENV=development, to properly
define the variable and avoid unintended production defaults.
|
|
||
| # Connect to Supabase via connection pooling | ||
| DATABASE_URL= | ||
| SUPABASE_URL= |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm downstream consumers are updated to use SUPABASE_URL.
DATABASE_URL appears to have been replaced with SUPABASE_URL. Double-check every reference in the codebase, CI workflow files, and deployment manifests to avoid broken connections.
Run:
🏁 Script executed:
#!/bin/bash
# Ensure no stale `DATABASE_URL` references remain.
rg -n $'DATABASE_URL' || echo "✅ No occurrences"Length of output: 211
Update all remaining DATABASE_URL references to SUPABASE_URL
We ran rg -n DATABASE_URL and found these occurrences that need updating:
-
README.md (line 11)
Update the.envdescription to mentionSUPABASE_URLinstead ofDATABASE_URL. -
prisma/schema.prisma (line 11)
Change- url = env("DATABASE_URL") + url = env("SUPABASE_URL")
-
src/config/index.ts (line 3)
Change- DATABASE_URL: process.env.DATABASE_URL!, + SUPABASE_URL: process.env.SUPABASE_URL!,
After making these updates, please also verify any CI workflows and deployment manifests for leftover DATABASE_URL references.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In the .env.example file at line 3 and related files, update all references from
DATABASE_URL to SUPABASE_URL. Specifically, modify README.md at line 11 to
describe SUPABASE_URL instead of DATABASE_URL, change the datasource URL in
prisma/schema.prisma at line 11 to use SUPABASE_URL, and update the import or
usage in src/config/index.ts at line 3 accordingly. Also, review CI workflows
and deployment manifests to replace any remaining DATABASE_URL references with
SUPABASE_URL to ensure consistency.
Summary by CodeRabbit
New Features
Improvements
Chores