Convenor Routes updated Dev Routes corrected and tested #16
Conversation
📝 WalkthroughWalkthroughThis PR refactors the convenor management API from bulk replace operations to granular add/edit workflows. It removes the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as /api/convenors/co-convenors
participant DB as Database
participant Formatter as formatSocietyConvenors
rect rgba(200, 100, 150, 0.5)
Note over Client,Formatter: ADD/EDIT Single Co-Convenor via PATCH
Client->>API: PATCH { societyName, coConvenor: {...} }
activate API
API->>DB: Find society by name
activate DB
DB-->>API: society + currentConvenor
deactivate DB
alt Has coConvenorId
API->>DB: Update existing co-convenor in array
DB-->>API: Updated society
else No coConvenorId
API->>API: Append new co-convenor to array
API->>DB: Save updated society
DB-->>API: Saved society
end
API->>DB: Derive tech from society.currentConvenor
activate DB
DB-->>API: currentConvenor.tech
deactivate DB
API->>Formatter: formatSocietyConvenors(society, user, includeHistory)
activate Formatter
Formatter->>Formatter: Format convenor, co-convenors, histories by tech
Formatter-->>API: SocietyConvenorsResponse
deactivate Formatter
API-->>Client: { success: true, message, society, tech, coConvenors: [...] }
deactivate API
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@app/api/convenors/`[id]/route.ts:
- Around line 166-186: The deletion code calls User.findByIdAndDelete(id)
without verifying the convenor isn't referenced by any
Society.currentConvenor.userId; before deleting (in the handler that uses
User.findById, User.findByIdAndDelete), query the Society model for any
documents where currentConvenor.userId equals the convenor id and either return
a 400/409 error preventing deletion or update those Society records to
clear/replace currentConvenor (e.g., set currentConvenor=null or assign a new
convenor) and save before calling User.findByIdAndDelete; reference the
User.findById, User.findByIdAndDelete calls and the
Society.currentConvenor.userId field when making the changes.
In `@app/api/developers/`[id]/route.ts:
- Around line 44-57: Update the GET, DELETE, and PATCH route handlers to accept
params as a Promise and await it before use: change the signature type from {
params }: { params: { id: string } } to { params }: { params: Promise<{ id:
string }> } in the GET, DELETE, and PATCH functions, then do const { id } =
await params at the top of each handler and replace all usages of params.id with
id; ensure this change is applied inside the GET, DELETE, and PATCH functions in
route.ts (e.g., the exported GET function and its DELETE/PATCH counterparts).
In `@lib/formatters/convenor.ts`:
- Around line 22-33: The formatConvenor function currently ignores its tech
parameter and reads user.tech instead; update formatConvenor to use the
passed-in tech value (falling back to user.tech and then 0) for the returned
ConvenorDTO's tech field so callers' society.currentConvenor.tech is
respected—change the tech assignment in formatConvenor to prefer the function
parameter (e.g., tech ?? user.tech ?? 0) while keeping the same function
signature and other fields unchanged.
- Around line 10-20: The function groupByTechSorted currently only groups items
but doesn't perform the promised descending sort; update it to produce groups
ordered by tech descending: after the reduce that builds acc, get the tech keys
(Object.keys(acc).map(Number)), sort them descending (sort((a,b) => b - a)),
then build and return a new Record by iterating the sorted keys and assigning
newAcc[tech] = acc[tech]; ensure you reference the existing function name
groupByTechSorted and variables acc/items so the change is applied in that
function.
🧹 Nitpick comments (9)
app/api/convenors/route.ts (2)
54-59: Use proper type assertion instead ofas any.The
as anycast bypasses type safety. SinceSOCIETY_NAMESis an enum, cast to the enum type directly.♻️ Suggested fix
- if (!Object.values(SOCIETY_NAMES).includes(societyName as any)) { + if (!Object.values(SOCIETY_NAMES).includes(societyName as SOCIETY_NAMES)) {You'll also need to import or reference
SOCIETY_NAMESas a type in the check, or use a type guard:function isValidSocietyName(name: string): name is SOCIETY_NAMES { return Object.values(SOCIETY_NAMES).includes(name as SOCIETY_NAMES); }
45-51: Inconsistent indentation.The return block has extra indentation compared to the similar block at lines 84-90. Consider aligning for consistency.
♻️ Suggested fix
- return NextResponse.json( - { - success: true, - data, - }, - { status: 200 } - ); + return NextResponse.json( + { + success: true, + data, + }, + { status: 200 } + );app/api/developers/[id]/route.ts (2)
20-41: Consider typing thedocparameter more specifically.Using
anydefeats TypeScript's type safety. The document could be typed using Mongoose's document type.♻️ Suggested fix
-function formatDeveloper(doc: any): DeveloperResponse { +import type { IDeveloper } from '@/lib/models/Developer'; +import type { Document, Types } from 'mongoose'; + +type DeveloperDocument = Document<Types.ObjectId, {}, IDeveloper> & IDeveloper; + +function formatDeveloper(doc: DeveloperDocument | Record<string, unknown>): DeveloperResponse {Alternatively, a simpler approach if you need flexibility:
function formatDeveloper(doc: { toObject?: () => Record<string, unknown> } & Record<string, unknown>): DeveloperResponse {
146-150: Truthy checks prevent clearing optional fields.Using
if (developer.name)ignores empty strings. If a user wants to clear a social link (e.g., setgithubto""), this logic would skip the update.Consider checking for
undefinedexplicitly if clearing is allowed:♻️ Suggested fix (if clearing should be allowed)
- if (developer.name) updates.name = developer.name; - if (developer.imgURL) updates.imgURL = developer.imgURL; - if (developer.github) updates.github = developer.github; - if (developer.insta) updates.insta = developer.insta; - if (developer.linkedin) updates.linkedin = developer.linkedin; + if (developer.name !== undefined) updates.name = developer.name; + if (developer.imgURL !== undefined) updates.imgURL = developer.imgURL; + if (developer.github !== undefined) updates.github = developer.github; + if (developer.insta !== undefined) updates.insta = developer.insta; + if (developer.linkedin !== undefined) updates.linkedin = developer.linkedin;If empty strings should be rejected, consider explicit validation instead of silent skipping.
app/api/developers/route.ts (2)
16-17: UnusedVALID_YEARSandVALID_ROLESSets.These Sets are defined at lines 16-17 but the POST validation at lines 107-119 uses
Object.values().includes()instead. Either use the Sets for consistency (as done in[id]/route.tsPATCH) or remove the unused declarations.♻️ Suggested fix (use the Sets)
- if (!Object.values(YEAR_LEVELS).includes(year)) { + if (!VALID_YEARS.has(year)) { return NextResponse.json( { message: 'Invalid year value' }, { status: 400 } ); } - if (!Object.values(DEVELOPER_ROLES).includes(role)) { + if (!VALID_ROLES.has(role)) { return NextResponse.json( { message: 'Invalid role value' }, { status: 400 } ); }Also applies to: 107-119
19-40: DuplicateformatDeveloperfunction.This function is duplicated in
app/api/developers/[id]/route.ts. Consider extracting it to a shared formatter module (e.g.,lib/formatters/developer.ts) similar to theformatSocietyConvenorspattern used for convenors.♻️ Suggested extraction
Create
lib/formatters/developer.ts:import type { DeveloperResponse } from '@/types/dto/developer'; export function formatDeveloper(doc: any): DeveloperResponse { const obj = doc?.toObject ? doc.toObject() : doc; return { id: obj._id?.toString?.() ?? obj._id, name: obj.name, imgURL: obj.imgURL, year: obj.year, role: obj.role, github: obj.github, insta: obj.insta, linkedin: obj.linkedin, createdAt: obj.createdAt instanceof Date ? obj.createdAt.toISOString() : obj.createdAt, updatedAt: obj.updatedAt instanceof Date ? obj.updatedAt.toISOString() : obj.updatedAt, }; }Then import in both route files:
import { formatDeveloper } from '@/lib/formatters/developer';app/api/convenors/co-convenors/route.ts (2)
46-62: Replaceanytypes with a proper interface.The static analysis correctly flags the
anytypes. Define an interface matching the subdocument shape.Proposed fix
+interface CoConvenorSubdoc { + name: string; + imgurl: string; + tech: number; +} - society.currentCoConvenors.forEach((cc: { name: any; imgurl: any; tech: any; }) => { + society.currentCoConvenors.forEach((cc: CoConvenorSubdoc) => {
96-108: Consider validatingcoConvenorobject exists before destructuring.If
coConvenorisundefined, destructuring{ name, imgurl }yieldsundefinedvalues which then fail validation. This works but could provide a clearer error message.Proposed improvement
const { societyName, coConvenorId, coConvenor } = await req.json(); + + if (!coConvenor || typeof coConvenor !== 'object') { + return NextResponse.json( + { message: 'coConvenor object is required' }, + { status: 400 } + ); + } + const { name, imgurl } = coConvenor || {};app/api/convenors/[id]/route.ts (1)
7-14: Unused importConvenorActionResponse.
ConvenorActionResponseis imported but never used in this file.Proposed fix
import type { GetSingleConvenorResponse, - ConvenorActionResponse, ConvenorErrorResponse, } from '@/types/dto/convenor';
| try { | ||
| const { id } = await context.params; // ✅ FIX | ||
|
|
||
| await connectDB(); | ||
|
|
||
| const convenor = await User.findById(id); | ||
|
|
||
| if (!convenor || convenor.role !== 'CONVENOR') { | ||
| return NextResponse.json( | ||
| { message: 'Convenor not found' }, | ||
| { status: 404 } | ||
| ); | ||
| } | ||
|
|
||
| // ❗ Preserve history – only remove user | ||
| await User.findByIdAndDelete(id); | ||
|
|
||
| return NextResponse.json( | ||
| { message: 'Convenor deleted successfully' }, | ||
| { status: 200 } | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "currentConvenor" --type ts -C 3Repository: Debatreya/AdminPanel
Length of output: 6984
🏁 Script executed:
# Let's also search for the Society model definition
rg -n "interface Society|type Society|class Society" --type ts -C 5Repository: Debatreya/AdminPanel
Length of output: 549
🏁 Script executed:
# Check for any cleanup/cascading logic when users are deleted
rg -n "findByIdAndDelete|deleteOne|deleteMany" --type ts -C 3Repository: Debatreya/AdminPanel
Length of output: 1276
🏁 Script executed:
# Look for any validation when deleting convenors
fd -e ts -e js | xargs grep -l "Society" | head -20Repository: Debatreya/AdminPanel
Length of output: 434
🏁 Script executed:
# Get the full DELETE endpoint context around line 166-186
sed -n '150,200p' app/api/convenors/[id]/route.tsRepository: Debatreya/AdminPanel
Length of output: 1031
🏁 Script executed:
# Check User model for any delete hooks or validation
cat lib/models/index.tsRepository: Debatreya/AdminPanel
Length of output: 934
🏁 Script executed:
# Look at the complete User model
fd -name "User.ts" --type fRepository: Debatreya/AdminPanel
Length of output: 294
🏁 Script executed:
# Find User model file
fd User.tsRepository: Debatreya/AdminPanel
Length of output: 82
🏁 Script executed:
# Check if there are any pre-delete hooks or validation in User model
rg -n "pre\(|post\(|findByIdAndDelete" lib/models/User.ts -C 5Repository: Debatreya/AdminPanel
Length of output: 46
🏁 Script executed:
# Read the User model to see its complete definition
cat lib/models/User.tsRepository: Debatreya/AdminPanel
Length of output: 1148
Validate convenor is not the active convenor of any society before deletion.
Deleting the user will leave Society.currentConvenor.userId as an orphaned reference if this convenor is currently assigned to any society. This breaks:
- Authorization middleware that calls
.equals()oncurrentConvenor.userId(line 67 ofconvenorAuth.ts) - Populate queries that will return null for the user
- Data consistency
Either check that the convenor is not assigned as currentConvenor in any society before deletion, or update all societies where they are the current convenor before removing the user.
🤖 Prompt for AI Agents
In `@app/api/convenors/`[id]/route.ts around lines 166 - 186, The deletion code
calls User.findByIdAndDelete(id) without verifying the convenor isn't referenced
by any Society.currentConvenor.userId; before deleting (in the handler that uses
User.findById, User.findByIdAndDelete), query the Society model for any
documents where currentConvenor.userId equals the convenor id and either return
a 400/409 error preventing deletion or update those Society records to
clear/replace currentConvenor (e.g., set currentConvenor=null or assign a new
convenor) and save before calling User.findByIdAndDelete; reference the
User.findById, User.findByIdAndDelete calls and the
Society.currentConvenor.userId field when making the changes.
| export async function GET( | ||
| _request: Request, | ||
| { params }: { params: { id: string } } | ||
| ) { | ||
| try { | ||
| await connectDB() | ||
| await connectDB(); | ||
|
|
||
| const developer = await findDeveloper(params.id) | ||
| const developer = await Developer.findById(params.id); | ||
|
|
||
| if (!developer) { | ||
| return NextResponse.json<DeveloperErrorResponse>( | ||
| { message: 'Developer not found' }, | ||
| { status: 404 } | ||
| ) | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Next.js 15 route handler params async change
💡 Result:
In Next.js 15, Route Handler params became async: the route handler context now provides params as a Promise, so you must await it before reading properties. This matches the broader Next.js 15 change where route params are now Promises. [1][2][3]
Before (Next.js 14 and earlier)
type Params = { slug: string }
export async function GET(_req: Request, ctx: { params: Params }) {
const slug = ctx.params.slug
}After (Next.js 15)
type Params = Promise<{ slug: string }>
export async function GET(_req: Request, ctx: { params: Params }) {
const { slug } = await ctx.params
}[1][2]
Typed helper (optional)
Next.js 15 also documents using RouteContext for strongly typed params (still awaited):
export async function GET(_req: Request, ctx: RouteContext<'/users/[id]'>) {
const { id } = await ctx.params
return Response.json({ id })
}[2]
If you’re upgrading, the Next.js 15 upgrade guide includes this exact migration pattern for Route Handlers. [1]
Sources: Next.js v15 upgrade guide (Route Handlers) [1], Route Handlers docs [2], Next.js error explanation for async params [3].
🏁 Script executed:
# Check Next.js version in the project
if [ -f "package.json" ]; then
echo "=== package.json Next.js dependency ==="
grep -A 2 "\"next\"" package.json || echo "Next.js not found in dependencies"
fi
# Check the actual file to understand the params usage
echo -e "\n=== File: app/api/developers/[id]/route.ts ==="
if [ -f "app/api/developers/[id]/route.ts" ]; then
wc -l "app/api/developers/[id]/route.ts"
fiRepository: Debatreya/AdminPanel
Length of output: 260
🏁 Script executed:
# Read the full file to see all params usages and function signatures
if [ -f "app/api/developers/[id]/route.ts" ]; then
cat -n "app/api/developers/[id]/route.ts"
fiRepository: Debatreya/AdminPanel
Length of output: 6842
Update route handlers to await params for Next.js 16 compatibility.
Next.js 16 requires params to be awaited in route handlers. Update the type annotations and all three route handlers (GET, DELETE, PATCH) to await params before accessing properties:
export async function GET(
_request: Request,
{ params }: { params: Promise<{ id: string }> }
) {
const { id } = await params;
// ... use id instead of params.id
}This affects lines 46 (GET), 79 (DELETE), and 116 (PATCH) - change { params }: { params: { id: string } } to { params }: { params: Promise<{ id: string }> } and update all uses of params.id to id after destructuring from the awaited params object.
🤖 Prompt for AI Agents
In `@app/api/developers/`[id]/route.ts around lines 44 - 57, Update the GET,
DELETE, and PATCH route handlers to accept params as a Promise and await it
before use: change the signature type from { params }: { params: { id: string }
} to { params }: { params: Promise<{ id: string }> } in the GET, DELETE, and
PATCH functions, then do const { id } = await params at the top of each handler
and replace all usages of params.id with id; ensure this change is applied
inside the GET, DELETE, and PATCH functions in route.ts (e.g., the exported GET
function and its DELETE/PATCH counterparts).
| // Groups entries by tech year and sorts descending | ||
|
|
||
| export function groupByTechSorted<T extends { tech: number }>( | ||
| items: T[] | ||
| ): Record<number, T[]> { | ||
| return items.reduce<Record<number, T[]>>((acc, item) => { | ||
| if (!acc[item.tech]) acc[item.tech] = []; | ||
| acc[item.tech].push(item); | ||
| return acc; | ||
| }, {}); | ||
| } |
There was a problem hiding this comment.
Comment claims sorting but no sorting is implemented.
The comment says "sorts descending" but groupByTechSorted only groups by tech without any sorting. Either implement sorting or fix the comment.
Option 1: Fix the comment
-// Groups entries by tech year and sorts descending
+// Groups entries by tech yearOption 2: Implement sorting
export function groupByTechSorted<T extends { tech: number }>(
items: T[]
): Record<number, T[]> {
- return items.reduce<Record<number, T[]>>((acc, item) => {
+ const grouped = items.reduce<Record<number, T[]>>((acc, item) => {
if (!acc[item.tech]) acc[item.tech] = [];
acc[item.tech].push(item);
return acc;
}, {});
+ // Sort keys descending and reconstruct object
+ return Object.keys(grouped)
+ .map(Number)
+ .sort((a, b) => b - a)
+ .reduce<Record<number, T[]>>((acc, key) => {
+ acc[key] = grouped[key];
+ return acc;
+ }, {});
}📝 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.
| // Groups entries by tech year and sorts descending | |
| export function groupByTechSorted<T extends { tech: number }>( | |
| items: T[] | |
| ): Record<number, T[]> { | |
| return items.reduce<Record<number, T[]>>((acc, item) => { | |
| if (!acc[item.tech]) acc[item.tech] = []; | |
| acc[item.tech].push(item); | |
| return acc; | |
| }, {}); | |
| } | |
| // Groups entries by tech year | |
| export function groupByTechSorted<T extends { tech: number }>( | |
| items: T[] | |
| ): Record<number, T[]> { | |
| return items.reduce<Record<number, T[]>>((acc, item) => { | |
| if (!acc[item.tech]) acc[item.tech] = []; | |
| acc[item.tech].push(item); | |
| return acc; | |
| }, {}); | |
| } |
| // Groups entries by tech year and sorts descending | |
| export function groupByTechSorted<T extends { tech: number }>( | |
| items: T[] | |
| ): Record<number, T[]> { | |
| return items.reduce<Record<number, T[]>>((acc, item) => { | |
| if (!acc[item.tech]) acc[item.tech] = []; | |
| acc[item.tech].push(item); | |
| return acc; | |
| }, {}); | |
| } | |
| // Groups entries by tech year and sorts descending | |
| export function groupByTechSorted<T extends { tech: number }>( | |
| items: T[] | |
| ): Record<number, T[]> { | |
| const grouped = items.reduce<Record<number, T[]>>((acc, item) => { | |
| if (!acc[item.tech]) acc[item.tech] = []; | |
| acc[item.tech].push(item); | |
| return acc; | |
| }, {}); | |
| // Sort keys descending and reconstruct object | |
| return Object.keys(grouped) | |
| .map(Number) | |
| .sort((a, b) => b - a) | |
| .reduce<Record<number, T[]>>((acc, key) => { | |
| acc[key] = grouped[key]; | |
| return acc; | |
| }, {}); | |
| } |
🤖 Prompt for AI Agents
In `@lib/formatters/convenor.ts` around lines 10 - 20, The function
groupByTechSorted currently only groups items but doesn't perform the promised
descending sort; update it to produce groups ordered by tech descending: after
the reduce that builds acc, get the tech keys (Object.keys(acc).map(Number)),
sort them descending (sort((a,b) => b - a)), then build and return a new Record
by iterating the sorted keys and assigning newAcc[tech] = acc[tech]; ensure you
reference the existing function name groupByTechSorted and variables acc/items
so the change is applied in that function.
| // Formats active convenor (User + tech) | ||
| export function formatConvenor( | ||
| user: any, | ||
| tech: number | ||
| ): ConvenorDTO { | ||
| return { | ||
| id: user._id.toString(), | ||
| name: user.name, | ||
| imgurl: user.imgurl, | ||
| tech: user.tech??0, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Unused tech parameter - function uses user.tech instead.
The tech parameter is never used; the return value uses user.tech ?? 0. This is likely a bug since callers pass society.currentConvenor.tech expecting it to be used.
Proposed fix
export function formatConvenor(
user: any,
tech: number
): ConvenorDTO {
return {
id: user._id.toString(),
name: user.name,
imgurl: user.imgurl,
- tech: user.tech??0,
+ tech,
};
}📝 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.
| // Formats active convenor (User + tech) | |
| export function formatConvenor( | |
| user: any, | |
| tech: number | |
| ): ConvenorDTO { | |
| return { | |
| id: user._id.toString(), | |
| name: user.name, | |
| imgurl: user.imgurl, | |
| tech: user.tech??0, | |
| }; | |
| } | |
| // Formats active convenor (User + tech) | |
| export function formatConvenor( | |
| user: any, | |
| tech: number | |
| ): ConvenorDTO { | |
| return { | |
| id: user._id.toString(), | |
| name: user.name, | |
| imgurl: user.imgurl, | |
| tech, | |
| }; | |
| } |
🧰 Tools
🪛 ESLint
[error] 24-24: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🤖 Prompt for AI Agents
In `@lib/formatters/convenor.ts` around lines 22 - 33, The formatConvenor function
currently ignores its tech parameter and reads user.tech instead; update
formatConvenor to use the passed-in tech value (falling back to user.tech and
then 0) for the returned ConvenorDTO's tech field so callers'
society.currentConvenor.tech is respected—change the tech assignment in
formatConvenor to prefer the function parameter (e.g., tech ?? user.tech ?? 0)
while keeping the same function signature and other fields unchanged.
1 participant
Summary by CodeRabbit
Release Notes
New Features
Changes
✏️ Tip: You can customize this high-level summary in your review settings.