-
-
Notifications
You must be signed in to change notification settings - Fork 218
fix params and remove refresh params #2287
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
fix params and remove refresh params #2287
Conversation
Summary by CodeRabbit
WalkthroughRemoved Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)frontend/src/components/ModuleCard.tsx (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (3)
frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx (3)
10-10
: Drop GET_PROGRAM_AND_MODULES if you switch to entity-level cache updates.If you adopt the writeFragment refactor below, this import becomes unused.
Apply if adopting the refactor:
-import { GET_PROGRAM_DETAILS, GET_PROGRAM_AND_MODULES } from 'server/queries/programsQueries' +import { GET_PROGRAM_DETAILS } from 'server/queries/programsQueries'Also add
gql
to the Apollo import:// at line 2 import { useQuery, useMutation, gql } from '@apollo/client'
140-140
: Use server-returned key for navigation to avoid client/server slug drift.Prevents mis-routing if backend normalizes or de-dupes slugs differently.
Apply:
- router.push(`/my/mentorship/programs/${slugify(formData.name)}`) + router.push(`/my/mentorship/programs/${result?.updateProgram?.key ?? slugify(formData.name)}`)
117-121
: If you keep the current approach, minimally fix the lints.
- Remove
as any
with a typedreadQuery
.- Make
catch
non-empty.Apply:
- const existing = cache.readQuery({ + const existing = cache.readQuery<{ + getProgram?: Record<string, unknown> + getProgramModules?: unknown + }>({ query: GET_PROGRAM_AND_MODULES, variables: { programKey: input.key }, - }) as any + }) @@ - } catch {} + } catch (e) { + /* Query not in cache; nothing to update */ void e + }Also applies to: 128-128
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
frontend/__tests__/unit/pages/CreateModule.test.tsx
(1 hunks)frontend/__tests__/unit/pages/EditModule.test.tsx
(1 hunks)frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx
(3 hunks)frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx
(3 hunks)frontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx
(3 hunks)frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
(4 hunks)frontend/src/server/mutations/programsMutations.ts
(2 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
Applied to files:
frontend/__tests__/unit/pages/EditModule.test.tsx
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx
frontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx
frontend/src/server/mutations/programsMutations.ts
frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details. These queries cannot be removed or merged as they serve different use cases in the application.
Applied to files:
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx
frontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx
frontend/src/server/mutations/programsMutations.ts
frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details.
Applied to files:
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx
frontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx
frontend/src/server/mutations/programsMutations.ts
frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx
📚 Learning: 2025-07-13T07:31:06.511Z
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: frontend/src/components/ModuleCard.tsx:53-55
Timestamp: 2025-07-13T07:31:06.511Z
Learning: In Next.js 13+ app router, useRouter from 'next/navigation' does not provide asPath or query properties. Use useParams to extract route parameters and usePathname to get the current pathname instead.
Applied to files:
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
📚 Learning: 2025-07-12T17:14:28.536Z
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: frontend/src/app/mentorship/programs/[programKey]/edit/page.tsx:90-128
Timestamp: 2025-07-12T17:14:28.536Z
Learning: Both ProgramForm (programCard.tsx) and ModuleForm (mainmoduleCard.tsx) components already implement HTML validation using the `required` attribute on form fields. The browser's native validation prevents form submission and displays error messages when required fields are empty, eliminating the need for additional JavaScript validation before GraphQL mutations.
Applied to files:
frontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx
🧬 Code graph analysis (4)
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (1)
frontend/src/server/queries/programsQueries.ts (1)
GET_PROGRAM_AND_MODULES
(44-81)
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx (1)
frontend/src/server/queries/programsQueries.ts (1)
GET_PROGRAM_AND_MODULES
(44-81)
frontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx (1)
frontend/src/server/queries/programsQueries.ts (1)
GET_PROGRAM_AND_MODULES
(44-81)
frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx (1)
frontend/src/server/queries/programsQueries.ts (2)
GET_PROGRAM_DETAILS
(21-43)GET_PROGRAM_AND_MODULES
(44-81)
🪛 GitHub Actions: Run CI/CD
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
[warning] 78-78: Command: pnpm run lint:check. ESLint: Unexpected any. Specify a different type. (no-explicit-any)
[error] 89-89: Command: pnpm run lint:check. ESLint: Empty block statement. (no-empty)
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx
[warning] 121-121: Command: pnpm run lint:check. ESLint: Unexpected any. Specify a different type. (no-explicit-any)
[warning] 123-123: Command: pnpm run lint:check. ESLint: Unexpected any. Specify a different type. (no-explicit-any)
[error] 132-132: Command: pnpm run lint:check. ESLint: Empty block statement. (no-empty)
frontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx
[warning] 106-106: Command: pnpm run lint:check. ESLint: Unexpected any. Specify a different type. (no-explicit-any)
[error] 117-117: Command: pnpm run lint:check. ESLint: Empty block statement. (no-empty)
frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx
[warning] 120-120: Command: pnpm run lint:check. ESLint: Unexpected any. Specify a different type. (no-explicit-any)
[error] 128-128: Command: pnpm run lint:check. ESLint: Empty block statement. (no-empty)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CodeQL (python)
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (10)
frontend/__tests__/unit/pages/EditModule.test.tsx (1)
117-117
: Test expectation update aligns with removal of refresh param.The navigation assertion now matches the new route shape. Good.
frontend/__tests__/unit/pages/CreateModule.test.tsx (1)
114-114
: Post-create navigation expectation updated correctly.Matches the new behavior (no refresh query).
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx (1)
11-11
: Using GET_PROGRAM_AND_MODULES for cache sync is the right call.frontend/src/server/mutations/programsMutations.ts (2)
6-6
: Including id in UpdateProgram payload is correct for cache normalization.
47-47
: Including id in UpdateProgramStatus payload is correct for cache updates.frontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx (1)
10-10
: Importing GET_PROGRAM_AND_MODULES enables local cache prepending.frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (3)
6-6
: Good: removed router/search param coupling; rely purely on Apollo cache.
32-36
: Loading state simplification is fine.Also applies to: 38-38
124-133
: Minor: guard against undefined experienceLevels.Already handled with fallback, looks good.
frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx (1)
106-130
: Refactor cache updates to writeFragment — verified safeUPDATE_PROGRAM selection set in frontend/src/server/mutations/programsMutations.ts includes id and key; apply the suggested cache.writeFragment diff (removes try/catch and fixes no-explicit-any/no-empty).
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx
Outdated
Show resolved
Hide resolved
frontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx
Show resolved
Hide resolved
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
♻️ Duplicate comments (1)
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx (1)
112-132
: Strengthen cache update typing and match by id when available.Avoid the cast, type the readQuery result, and prefer id equality with key as fallback. This is more robust if keys change.
- await updateModule({ + await updateModule({ variables: { input }, - update: (cache, { data: mutationData }) => { - const updated = mutationData?.updateModule + update: (cache, { data: mutationData }) => { + const updated = mutationData?.updateModule as Module | undefined if (!updated) return - const existing = cache.readQuery({ - query: GET_PROGRAM_AND_MODULES, - variables: { programKey }, - }) as { getProgramModules: Module[] } + const existing = cache.readQuery<{ getProgram?: Program; getProgramModules?: Module[] }>({ + query: GET_PROGRAM_AND_MODULES, + variables: { programKey }, + }) if (existing?.getProgramModules) { - const nextModules = existing.getProgramModules.map((m: Module) => - m.key === updated.key ? { ...m, ...updated } : m - ) + const nextModules = (existing.getProgramModules ?? []).map((m) => + (updated.id ? m.id === updated.id : m.key === updated.key) + ? { ...m, ...updated } + : m + ) cache.writeQuery({ query: GET_PROGRAM_AND_MODULES, variables: { programKey }, data: { ...existing, getProgramModules: nextModules }, }) } }, })
🧹 Nitpick comments (1)
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx (1)
13-13
: Import types as type‑only to avoid runtime imports (TS 5 “verbatimModuleSyntax”).Mark Module (and Program if used) as type‑only to prevent bundling a non‑value. Also aligns with lint rules.
- import { EXPERIENCE_LEVELS, Module, type ModuleFormData } from 'types/mentorship' + import { EXPERIENCE_LEVELS, type Module, type Program, type ModuleFormData } from 'types/mentorship'
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx
(3 hunks)frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx
(3 hunks)frontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx
(3 hunks)frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
- frontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx
- frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
Applied to files:
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details. These queries cannot be removed or merged as they serve different use cases in the application.
Applied to files:
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details.
Applied to files:
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx
🧬 Code graph analysis (1)
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx (2)
frontend/src/server/queries/programsQueries.ts (1)
GET_PROGRAM_AND_MODULES
(44-81)frontend/src/types/mentorship.ts (1)
Module
(52-64)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run backend tests
- GitHub Check: Run frontend unit tests
🔇 Additional comments (2)
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx (2)
11-11
: Good: warming the program page cache correctly.Using GET_PROGRAM_AND_MODULES here ensures the destination page renders updated modules without a refresh param round‑trip.
141-141
: Navigation without refresh param — correct.Matches the PR goal to remove user‑visible refresh mechanics.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx (1)
112-137
: Don’t surface cache-miss as an error; strengthen typing and match by id when available.
- readQuery throws if the query isn’t in cache; calling handleAppError here will toast a false error. Ignore silently.
- Type updated and readQuery properly; prefer id match, fall back to key. This also addresses prior ESLint feedback.
- await updateModule({ + await updateModule({ variables: { input }, - update: (cache, { data: mutationData }) => { - const updated = mutationData?.updateModule + update: (cache, { data: mutationData }) => { + const updated = mutationData?.updateModule as Module | undefined if (!updated) return try { - const existing = cache.readQuery({ + const existing = cache.readQuery<{ getProgram?: Program; getProgramModules?: Module[] }>({ query: GET_PROGRAM_AND_MODULES, variables: { programKey }, - }) as { getProgramModules: Module[] } - if (existing?.getProgramModules) { - const nextModules = existing.getProgramModules.map((m: Module) => - m.key === updated.key ? { ...m, ...updated } : m - ) + }) + if (existing?.getProgramModules) { + const nextModules = (existing.getProgramModules ?? []).map((m) => + (updated.id ? m.id === updated.id : m.key === updated.key) ? { ...m, ...updated } : m + ) cache.writeQuery({ query: GET_PROGRAM_AND_MODULES, variables: { programKey }, data: { ...existing, getProgramModules: nextModules }, }) } - } catch (_err) { - handleAppError(_err) - return - } + } catch { + // Cache miss: nothing to update + return + } }, })To verify mutation selections include id and key for robust matching:
#!/bin/bash # Verify updateModule selection includes id and key rg -n -C3 -P -g 'frontend/**/server/**' 'mutation\s+\w*UpdateModule|updateModule\s*\('
🧹 Nitpick comments (2)
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx (1)
13-13
: Use type-only imports for types.Import Module (and Program, if used in cache typing) as type-only to avoid generating runtime imports.
-import { EXPERIENCE_LEVELS, Module, type ModuleFormData } from 'types/mentorship' +import { EXPERIENCE_LEVELS, type Module, type Program, type ModuleFormData } from 'types/mentorship'frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx (1)
12-12
: Prefer type-only import for Program.Avoid runtime import for a type-only symbol.
-import { Program } from 'types/mentorship' +import type { Program } from 'types/mentorship'
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx
(3 hunks)frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx
(3 hunks)frontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx
(3 hunks)frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx
- frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
Applied to files:
frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details. These queries cannot be removed or merged as they serve different use cases in the application.
Applied to files:
frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details.
Applied to files:
frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx
🧬 Code graph analysis (2)
frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx (3)
frontend/src/server/queries/programsQueries.ts (2)
GET_PROGRAM_DETAILS
(21-43)GET_PROGRAM_AND_MODULES
(44-81)frontend/src/types/mentorship.ts (1)
Program
(22-37)frontend/src/app/global-error.tsx (1)
handleAppError
(66-86)
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx (3)
frontend/src/server/queries/programsQueries.ts (1)
GET_PROGRAM_AND_MODULES
(44-81)frontend/src/types/mentorship.ts (1)
Module
(52-64)frontend/src/app/global-error.tsx (1)
handleAppError
(66-86)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run backend tests
- GitHub Check: Run frontend unit tests
- GitHub Check: Run frontend e2e tests
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (3)
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx (2)
11-11
: Apollo cache target query import — OK.Importing GET_PROGRAM_AND_MODULES here is appropriate for post-mutation cache sync.
146-146
: Navigation without refresh parameter — OK.Routing back to the program page without ?refresh is aligned with the PR objective.
frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx (1)
10-10
: Importing combined query for cache merge — OK.GET_PROGRAM_AND_MODULES here enables updating both program details and the modules list coherently.
frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx
Outdated
Show resolved
Hide resolved
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (1)
62-70
: Remove extraneous field from UpdateProgramStatusInput.Passing name with updateProgramStatus likely violates the input schema and can 400. Send only key and status.
variables: { inputData: { key: program.key, - name: program.name, status: newStatus, }, },
♻️ Duplicate comments (2)
frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx (2)
107-141
: Tighten cache update types and silence cache‑miss.Same concerns as earlier review: cast updated, write GET_PROGRAM_DETAILS without try/catch, and don’t toast on readQuery miss.
- await updateProgram({ + const res = await updateProgram({ variables: { input }, update: (cache, { data: mutationData }) => { - const updated = mutationData?.updateProgram + const updated = mutationData?.updateProgram as Program | undefined if (!updated) return - try { - cache.writeQuery<{ getProgram?: Program; getProgramModules?: Module[] }>({ - query: GET_PROGRAM_DETAILS, - variables: { programKey: input.key }, - data: { getProgram: updated }, - }) - } catch (_err) { - handleAppError(_err) - } + cache.writeQuery<{ getProgram?: Program; getProgramModules?: Module[] }>({ + query: GET_PROGRAM_DETAILS, + variables: { programKey: input.key }, + data: { getProgram: updated }, + }) try { const existing = cache.readQuery<{ getProgram?: Program getProgramModules?: Module[] }>({ query: GET_PROGRAM_AND_MODULES, variables: { programKey: input.key }, }) if (existing?.getProgram) { cache.writeQuery<{ getProgram?: Program; getProgramModules?: Module[] }>({ query: GET_PROGRAM_AND_MODULES, variables: { programKey: input.key }, data: { ...existing, getProgram: { ...existing.getProgram, ...updated } }, }) } - } catch (_err) { - handleAppError(_err) - return - } + } catch { + // Cache miss: nothing to update + return + } }, })
151-151
: Route by key, not slugified name. Capture returned key.Slugifying name can 404 when key doesn’t change. Use returned key or fallback to current param.
- router.push(`/my/mentorship/programs/${slugify(formData.name)}`) + const newKey = res?.data?.updateProgram?.key ?? programKey + router.push(`/my/mentorship/programs/${newKey}`)
🧹 Nitpick comments (2)
frontend/src/server/mutations/programsMutations.ts (1)
35-45
: Add admins.id to CREATE_PROGRAM selection.CREATE_PROGRAM returns admins without id, but GET_PROGRAM_DETAILS/GET_PROGRAM_AND_MODULES expect it and Apollo normalization benefits from ids. Add id for parity.
Apply:
admins { + id login name avatarUrl }
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (1)
71-96
: Don’t toast on cache miss; keep cache update quiet.readQuery throws on cache miss; that’s benign. Avoid handleAppError here.
- update: (cache, { data: mutationData }) => { + update: (cache, { data: mutationData }) => { const updated = mutationData?.updateProgramStatus if (!updated) return try { const existing = cache.readQuery<{ getProgram?: Program getProgramModules?: Module[] }>({ query: GET_PROGRAM_AND_MODULES, variables: { programKey }, }) if (existing?.getProgram) { cache.writeQuery({ query: GET_PROGRAM_AND_MODULES, variables: { programKey }, data: { ...existing, getProgram: { ...existing.getProgram, status: updated.status }, }, }) } - } catch (_err) { - handleAppError(_err) - return - } + } catch { + // Cache miss: nothing to update + return + } },
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx
(3 hunks)frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
(4 hunks)frontend/src/server/mutations/moduleMutations.ts
(2 hunks)frontend/src/server/mutations/programsMutations.ts
(3 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details. These queries cannot be removed or merged as they serve different use cases in the application.
Applied to files:
frontend/src/server/mutations/programsMutations.ts
frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
Applied to files:
frontend/src/server/mutations/programsMutations.ts
frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details.
Applied to files:
frontend/src/server/mutations/programsMutations.ts
frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
📚 Learning: 2025-07-08T17:24:36.501Z
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: backend/apps/mentorship/graphql/mutations/program.py:41-44
Timestamp: 2025-07-08T17:24:36.501Z
Learning: In the mentorship program GraphQL mutations, date validation is handled at the GraphQL schema/node level in the input types (CreateProgramInput, UpdateProgramInput), preventing null values from reaching the mutation logic where date comparisons are performed.
Applied to files:
frontend/src/server/mutations/programsMutations.ts
📚 Learning: 2025-07-13T07:31:06.511Z
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: frontend/src/components/ModuleCard.tsx:53-55
Timestamp: 2025-07-13T07:31:06.511Z
Learning: In Next.js 13+ app router, useRouter from 'next/navigation' does not provide asPath or query properties. Use useParams to extract route parameters and usePathname to get the current pathname instead.
Applied to files:
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
🧬 Code graph analysis (2)
frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx (3)
frontend/src/types/mentorship.ts (2)
Program
(22-37)Module
(52-64)frontend/src/server/queries/programsQueries.ts (2)
GET_PROGRAM_DETAILS
(21-43)GET_PROGRAM_AND_MODULES
(44-81)frontend/src/app/global-error.tsx (1)
handleAppError
(66-86)
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (3)
frontend/src/server/queries/programsQueries.ts (1)
GET_PROGRAM_AND_MODULES
(44-81)frontend/src/types/mentorship.ts (2)
Program
(22-37)Module
(52-64)frontend/src/app/global-error.tsx (1)
handleAppError
(66-86)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run backend tests
- GitHub Check: Run frontend unit tests
- GitHub Check: Run frontend e2e tests
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (5)
frontend/src/server/mutations/moduleMutations.ts (2)
17-21
: Mentor identity added — good for cache normalization.Including mentors.id will help Apollo normalize Module mentors correctly. Confirm backend always exposes id for mentors to avoid nulls.
40-44
: Replicated mentors.id on create — consistent.Matching the update selection keeps CREATE/UPDATE payloads aligned for cache writes.
frontend/src/server/mutations/programsMutations.ts (2)
6-22
: UPDATE_PROGRAM selection now matches queries — good.Adding id, experienceLevels, and full admins payload prevents partial cache overwrites.
52-55
: Include id in status update — good.Helps downstream cache merges if needed.
frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx (1)
10-13
: Confirm UPDATE_PROGRAM returns full shape used by GET_PROGRAM_DETAILS/GET_PROGRAM_AND_MODULES.Previous verification failed (nl missing). Re-run and paste the extracted gql blocks from these files to confirm UPDATE_PROGRAM includes fields used by the queries (e.g., experienceLevels, admins{id,login,name,avatarUrl}):
#!/bin/bash set -euo pipefail mut="frontend/src/server/mutations/programsMutations.ts" qry="frontend/src/server/queries/programsQueries.ts" echo ">>> UPDATE_PROGRAM (from $mut)" awk '/export\s+const\s+UPDATE_PROGRAM\s*=\s*gql`/{p=1; print; next} p{print} /`/{if(p){print; exit}}' "$mut" || echo "UPDATE_PROGRAM block not found" echo echo ">>> GET_PROGRAM_DETAILS (from $qry)" awk '/export\s+const\s+GET_PROGRAM_DETAILS\s*=\s*gql`/{p=1; print; next} p{print} /`/{if(p){print; exit}}' "$qry" || echo "GET_PROGRAM_DETAILS block not found" echo echo ">>> GET_PROGRAM_AND_MODULES (from $qry)" awk '/export\s+const\s+GET_PROGRAM_AND_MODULES\s*=\s*gql`/{p=1; print; next} p{print} /`/{if(p){print; exit}}' "$qry" || echo "GET_PROGRAM_AND_MODULES block not found"
@Rajgupta36 I believe this should go into your feature branch and not into main? 🤔 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (4)
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (4)
39-39
: Drop the isLoading alias.Use isQueryLoading directly to reduce indirection.
Apply:
- const isLoading = isQueryLoading ... - if (isLoading) return <LoadingSpinner /> + if (isQueryLoading) return <LoadingSpinner /> - if (!program && !isLoading) { + if (!program && !isQueryLoading) {Also applies to: 119-119, 121-121
72-97
: Cache update works; prefer id-targeted cache.modify and don’t Sentry-report cache misses.cache.modify avoids coupling to the full query and prevents noisy error reporting when the entry isn’t in cache.
Apply:
update: (cache, { data: mutationData }) => { const updated = mutationData?.updateProgramStatus if (!updated) return - try { - const existing = cache.readQuery<{ - getProgram?: Program - getProgramModules?: Module[] - }>({ - query: GET_PROGRAM_AND_MODULES, - variables: { programKey }, - }) - if (existing?.getProgram) { - cache.writeQuery({ - query: GET_PROGRAM_AND_MODULES, - variables: { programKey }, - data: { - ...existing, - getProgram: { ...existing.getProgram, status: updated.status }, - }, - }) - } - } catch (_err) { - handleAppError(_err) - return - } + // Try normalized update first + try { + const existing = cache.readQuery<{ getProgram?: Program }>({ + query: GET_PROGRAM_AND_MODULES, + variables: { programKey }, + }) + const programId = existing?.getProgram?.id + if (programId) { + cache.modify({ + id: cache.identify({ __typename: 'Program', id: programId }), + fields: { + status: () => updated.status, + }, + }) + return + } + } catch { + // Ignore cache read misses; they’re benign during initial loads. + } + // Fallback: patch the query result if present + try { + const existing = cache.readQuery<{ getProgram?: Program; getProgramModules?: Module[] }>({ + query: GET_PROGRAM_AND_MODULES, + variables: { programKey }, + }) + if (existing?.getProgram) { + cache.writeQuery({ + query: GET_PROGRAM_AND_MODULES, + variables: { programKey }, + data: { + ...existing, + getProgram: { ...existing.getProgram, status: updated.status }, + }, + }) + } + } catch { + // Still nothing to do if entry isn’t cached + } },
101-106
: Prefer titleCaseWord for status in toast.upperFirst on enum-like values (e.g., IN_REVIEW) isn’t ideal.
Apply:
- title: `Program status updated to ${upperFirst(newStatus)}`, + title: `Program status updated to ${titleCaseWord(newStatus)}`,
135-135
: Guard against undefined menteesLimit.String(undefined) renders “undefined”; show N/A.
Apply:
- { label: 'Mentees Limit', value: String(program.menteesLimit) }, + { + label: 'Mentees Limit', + value: program.menteesLimit != null ? String(program.menteesLimit) : 'N/A', + },
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
(4 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
Applied to files:
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details. These queries cannot be removed or merged as they serve different use cases in the application.
Applied to files:
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details.
Applied to files:
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
📚 Learning: 2025-07-13T07:31:06.511Z
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: frontend/src/components/ModuleCard.tsx:53-55
Timestamp: 2025-07-13T07:31:06.511Z
Learning: In Next.js 13+ app router, useRouter from 'next/navigation' does not provide asPath or query properties. Use useParams to extract route parameters and usePathname to get the current pathname instead.
Applied to files:
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
🧬 Code graph analysis (1)
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (3)
frontend/src/server/queries/programsQueries.ts (1)
GET_PROGRAM_AND_MODULES
(44-81)frontend/src/types/mentorship.ts (2)
Program
(22-37)Module
(52-64)frontend/src/app/global-error.tsx (1)
handleAppError
(66-86)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run frontend unit tests
- GitHub Check: Run backend tests
- GitHub Check: Run frontend e2e tests
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (1)
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (1)
6-6
: Correct use of App Router params.Using useParams here is the right replacement for legacy router/query access in the App Router.
* fix ui bugs * update format * fix test cases * fix test cases * update UI * update UI * update line clamp property * fix test case * Update styling --------- Co-authored-by: Kate Golovanova <[email protected]> Co-authored-by: Arkadii Yakovets <[email protected]>
dcb5321
to
c02929e
Compare
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/package.json (1)
121-123
: Engines mismatch breaks CI: package requires Node 22, CI runs Node 20.19.5.Align one of: (a) update CI to Node 22 (use corepack to enforce pnpm version), or (b) relax engines to support Node 20 if no Node‑22 features are used.
Two alternative diffs (pick one):
Option A (keep Node 22; enforce in CI via corepack):
"engines": { - "node": "22" + "node": "22" },And in CI:
- uses: actions/setup-node@v4 - with: { node-version: 20 } + uses: actions/setup-node@v4 + with: { node-version: 22 } + - run: corepack enable && corepack prepare [email protected] --activateOption B (relax engines to match current CI):
"engines": { - "node": "22" + "node": ">=20 <23" },
🧹 Nitpick comments (3)
frontend/package.json (3)
39-39
: Sentry downgrade (10.12.0 → 10.11.0).Downgrades can drop fixes. If not driven by a regression, prefer staying on 10.12.x. Otherwise, add a short note in the PR about the incompatibility motivating this.
- "@sentry/nextjs": "^10.11.0", + "@sentry/nextjs": "^10.12.0",
50-50
: Framer Motion downgrade (12.23.13 → 12.23.12).Likely harmless, but verify no usage of APIs added in .13. If not necessary, keep the higher patch.
- "framer-motion": "^12.23.12", + "framer-motion": "^12.23.13",
92-93
: typescript‑eslint downgrades: keep trio in lockstep.Parser/plugin/meta are aligned at 8.43.0; confirm compatibility with ESLint 9.35.0 and your config. If the repo previously ran on 8.44.0 without issues, consider keeping that to reduce churn.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
frontend/package.json
(6 hunks)
🧰 Additional context used
🪛 GitHub Actions: Run CodeQL
frontend/package.json
[error] 1-1: Lockfile specifiers do not match package.json dependencies. This mismatch occurred while running 'pnpm install --frozen-lockfile'. Run 'pnpm install' to update the lockfile.
🪛 GitHub Actions: Run CI/CD
frontend/package.json
[warning] 1-1: Unsupported engine: Node 22 required; current: v20.19.5 (pnpm 10.17.0).
🔇 Additional comments (5)
frontend/package.json (5)
3-3
: Version bump to 0.1.0: OK, confirm release notes.Minor version bump aligns with user‑visible behavior changes. Ensure CHANGELOG and release/tag follow-up are updated accordingly.
118-118
: typescrip-eslint meta package pinned: OK.Matches parser/plugin versions; no action needed.
115-115
: No action required — ts-jest 29.4.1 is compatible with TypeScript 5.8.ts-jest 29.4.x declares "typescript": ">=4.3 <6" and was tested with TS ~5.8.3, so TypeScript 5.8 is officially supported.
22-25
: Font Awesome downgraded to v6 — confirm intent or revertfrontend/package.json (lines 22–25) and frontend/pnpm-lock.yaml pin @fortawesome/* at ^6.7.2; many source files import icons from those packages. If this downgrade is accidental, revert to v7; if intentional, run a full build/test and confirm all icon imports resolve and versions are deduped.
Apply this diff to revert if downgrade wasn’t required:
- "@fortawesome/fontawesome-svg-core": "^6.7.2", - "@fortawesome/free-brands-svg-icons": "^6.7.2", - "@fortawesome/free-regular-svg-icons": "^6.7.2", - "@fortawesome/free-solid-svg-icons": "^6.7.2", + "@fortawesome/fontawesome-svg-core": "^7.0.1", + "@fortawesome/free-brands-svg-icons": "^7.0.1", + "@fortawesome/free-regular-svg-icons": "^7.0.1", + "@fortawesome/free-solid-svg-icons": "^7.0.1",
136-136
: Sync pnpm toolchain and add the lockfile
- frontend/package.json pins packageManager = [email protected]+sha512... (normalized 10.10.0).
- pnpm-lock.yaml is missing from the repo root — CI runs using a frozen lockfile will fail / cause CodeQL errors.
- Fix: regenerate pnpm-lock.yaml locally using pnpm 10.10.0 and commit it, or ensure CI activates the pinned pnpm before install. Example CI step:
- run: corepack enable && corepack prepare [email protected] --activate
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.
Hey Raj!
Thanks for working on this. I noticed that you added id
s to GraphQL queries and it was the main change we needed for this to work.
I reverted changes in files for editing programs and modules - this does not need any custom logic now, since query has ID to match with cache on response.
Next time please read docs more closely - it specifically describes this situation here
We still need it for creating items though, so good call there.
I also left a question about changes in package.json
file. And noticed that you didn't run make check
since there were no generated types in your PR.
Please follow our contributing guidelines - you are not a newbie here 👌🏼
}, | ||
"dependencies": { | ||
"@apollo/client": "^3.14.0", | ||
"@fortawesome/fontawesome-svg-core": "^7.0.1", |
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.
Why did you downgraded these? And all package.json changes 🤔
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.
maybe changing base cause problem. i'm solving it
} | ||
|
||
await updateProgram({ variables: { input } }) | ||
await updateProgram({ |
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.
We don't need to do this for edit mutations.
Since you've added id
to GraphQL queries - this now works as is. This falls under this first point in cache update options.
}, | ||
}, | ||
refetchQueries: [{ query: GET_PROGRAM_AND_MODULES, variables: { programKey } }], | ||
update: (cache, { data: mutationData }) => { |
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.
Same here - not needed, as long as GraphQL query has id
await createModule({ variables: { input } }) | ||
await createModule({ | ||
variables: { input }, | ||
update: (cache, { data: mutationData }) => { |
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.
For create logic we still need to have custom update module because as stated here auto-update would not work with any new objects in a list (for example, list of modules in a program)
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
♻️ Duplicate comments (1)
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (1)
33-37
: Handle query errors explicitly and drop unnecessary notifyOnNetworkStatusChangeRight now network/server errors will fall through and likely render the 404 branch. Also
notifyOnNetworkStatusChange
isn’t needed without refetch flows.Apply:
- const { data, loading: isQueryLoading } = useQuery(GET_PROGRAM_AND_MODULES, { + const { data, loading: isQueryLoading, error: queryError } = useQuery(GET_PROGRAM_AND_MODULES, { variables: { programKey }, skip: !programKey, - notifyOnNetworkStatusChange: true, })Add outside the hunk:
useEffect(() => { if (queryError) handleAppError(queryError) }, [queryError]) // Render guards (replace existing) if (isQueryLoading) return <LoadingSpinner /> if (queryError) { return ( <ErrorDisplay statusCode={500} title="Error Loading Program" message="An unexpected error occurred while loading the program." /> ) }
🧹 Nitpick comments (1)
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (1)
86-91
: Avoid stale UI: clear local state on completed 404 and on program switchThe effect only sets state when truthy and never clears, leaving stale data after a 404 or when navigating to a different program.
Apply:
- useEffect(() => { - if (data?.getProgram) { - setProgram(data.getProgram) - setModules(data.getProgramModules || []) - } - }, [data]) + useEffect(() => { + if (data?.getProgram) { + setProgram(data.getProgram) + setModules(data.getProgramModules || []) + } else if (data && !isQueryLoading) { + setProgram(null) + setModules([]) + } + }, [data, isQueryLoading])Add outside the hunk to prevent showing prior program while switching:
useEffect(() => { setProgram(null) setModules([]) }, [programKey])
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
frontend/src/types/__generated__/moduleMutations.generated.ts
is excluded by!**/__generated__/**
frontend/src/types/__generated__/programsMutations.generated.ts
is excluded by!**/__generated__/**
📒 Files selected for processing (4)
frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx
(1 hunks)frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx
(1 hunks)frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
(3 hunks)frontend/src/middleware.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx
- frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
Applied to files:
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details. These queries cannot be removed or merged as they serve different use cases in the application.
Applied to files:
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details.
Applied to files:
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
📚 Learning: 2025-07-13T07:31:06.511Z
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: frontend/src/components/ModuleCard.tsx:53-55
Timestamp: 2025-07-13T07:31:06.511Z
Learning: In Next.js 13+ app router, useRouter from 'next/navigation' does not provide asPath or query properties. Use useParams to extract route parameters and usePathname to get the current pathname instead.
Applied to files:
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
🧬 Code graph analysis (1)
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (1)
frontend/src/server/queries/programsQueries.ts (1)
GET_PROGRAM_AND_MODULES
(44-81)
🔇 Additional comments (2)
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (2)
51-84
: Ensure program status is written to the cache after updateProgramUPDATE_PROGRAM_STATUS_MUTATION likely returns only
id
, so the cache'sgetProgram.status
won't update and the page will keep the old status. Add anupdate
callback that writes the new status into theGET_PROGRAM_AND_MODULES
cache.File: frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (updateProgram call)
Apply:
await updateProgram({ variables: { inputData: { key: program.key, name: program.name, status: newStatus, }, }, + update: (cache, { data: mutationData }) => { + const existing = cache.readQuery<{ getProgram?: Program; getProgramModules?: Module[] }>({ + query: GET_PROGRAM_AND_MODULES, + variables: { programKey }, + }) + if (existing?.getProgram) { + cache.writeQuery({ + query: GET_PROGRAM_AND_MODULES, + variables: { programKey }, + data: { + ...existing, + getProgram: { ...existing.getProgram, status: newStatus }, + }, + }) + } + }, })Run this to confirm the mutation selection set includes
status
; if not, add it server-side:#!/bin/bash fd programsMutations.ts | xargs -I{} rg -nP "UPDATE_PROGRAM_STATUS" -n {}
33-37
: Sanity check: ensure all ?refresh / refreshs query params are removedrg reported "No files were searched" — verification inconclusive. Re-run these searches locally and paste results:
rg -nP -S --hidden --no-ignore-vcs -C2 '\?(refresh|refreshs)($|=|%3D)' -g '!**/node_modules/**' rg -nP -S --hidden --no-ignore-vcs -C2 '(?:[?&])(refresh|refreshs)=(true|1)' -g '!**/node_modules/**'
If those return nothing, resolve this check.
Hi @kasya, |
Update docker-compose/local.yaml UI/ux mentorship program update (OWASP#2244) * fix ui bugs * update format * fix test cases * fix test cases * update UI * update UI * update line clamp property * fix test case * Update styling --------- Co-authored-by: Kate Golovanova <[email protected]> Co-authored-by: Arkadii Yakovets <[email protected]>
28ee26d
to
589f3fc
Compare
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
♻️ Duplicate comments (1)
frontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx (1)
99-103
: Confirm CREATE_MODULE returns all fields used by GET_PROGRAM_AND_MODULES.getProgramModules.If startedAt, endedAt, and mentors { id, login, name, avatarUrl } aren’t in the mutation selection set, the UI may render partially after writeQuery.
#!/bin/bash # Verify CREATE_MODULE selection includes required fields rg -n -C3 -P 'CREATE_MODULE|mutation\\s+\\w*CreateModule' frontend/src/server/mutations/moduleMutations.ts # Expect within createModule selection at least: # id, key, name, description, experienceLevel, startedAt, endedAt, # mentors { id login name avatarUrl }
🧹 Nitpick comments (2)
frontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx (2)
12-12
: Prefer type‑only imports and prep for typed cache reads.Use type modifiers to avoid bundling types and import Program so readQuery can be typed precisely.
- import { EXPERIENCE_LEVELS, Module } from 'types/mentorship' + import { EXPERIENCE_LEVELS, type Module, type Program } from 'types/mentorship'
97-122
: Make cache update safe on cache miss, strongly typed, and de‑dupe by id.Current catch triggers a user toast for a benign cache miss; also the read is loosely typed and can duplicate entries.
- await createModule({ + await createModule({ variables: { input }, - update: (cache, { data: mutationData }) => { - const created = mutationData?.createModule + update: (cache, { data: mutationData }) => { + const created = mutationData?.createModule as Module | undefined if (!created) return try { - const existing = cache.readQuery({ + const existing = cache.readQuery<{ getProgram?: Program; getProgramModules?: Module[] }>({ query: GET_PROGRAM_AND_MODULES, variables: { programKey }, - }) as { getProgramModules: Module[] } - if (existing?.getProgramModules) { + }) + const current = existing?.getProgramModules ?? [] + const alreadyPresent = current.some(m => m.id === created.id) + if (!alreadyPresent) { cache.writeQuery({ query: GET_PROGRAM_AND_MODULES, variables: { programKey }, data: { - ...existing, - getProgramModules: [created, ...existing.getProgramModules], + ...existing, + getProgramModules: [created, ...current], }, }) } - } catch (_err) { - handleAppError(_err) - return - } + } catch (_err) { + // Cache entry not present yet (e.g., page not queried). This is benign. + return + } }, })
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
frontend/src/types/__generated__/moduleMutations.generated.ts
is excluded by!**/__generated__/**
frontend/src/types/__generated__/programsMutations.generated.ts
is excluded by!**/__generated__/**
📒 Files selected for processing (9)
frontend/__tests__/unit/pages/CreateModule.test.tsx
(1 hunks)frontend/__tests__/unit/pages/EditModule.test.tsx
(1 hunks)frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx
(1 hunks)frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx
(1 hunks)frontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx
(3 hunks)frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
(3 hunks)frontend/src/middleware.ts
(1 hunks)frontend/src/server/mutations/moduleMutations.ts
(2 hunks)frontend/src/server/mutations/programsMutations.ts
(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- frontend/tests/unit/pages/EditModule.test.tsx
- frontend/src/middleware.ts
- frontend/src/server/mutations/moduleMutations.ts
- frontend/tests/unit/pages/CreateModule.test.tsx
- frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx
- frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx
- frontend/src/server/mutations/programsMutations.ts
- frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
Applied to files:
frontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx
📚 Learning: 2025-07-12T17:14:28.536Z
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: frontend/src/app/mentorship/programs/[programKey]/edit/page.tsx:90-128
Timestamp: 2025-07-12T17:14:28.536Z
Learning: Both ProgramForm (programCard.tsx) and ModuleForm (mainmoduleCard.tsx) components already implement HTML validation using the `required` attribute on form fields. The browser's native validation prevents form submission and displays error messages when required fields are empty, eliminating the need for additional JavaScript validation before GraphQL mutations.
Applied to files:
frontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx
📚 Learning: 2025-07-13T07:31:06.511Z
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: frontend/src/components/ModuleCard.tsx:53-55
Timestamp: 2025-07-13T07:31:06.511Z
Learning: In Next.js 13+ app router, useRouter from 'next/navigation' does not provide asPath or query properties. Use useParams to extract route parameters and usePathname to get the current pathname instead.
Applied to files:
frontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx
📚 Learning: 2025-07-13T11:29:25.245Z
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: frontend/src/app/mentorship/programs/page.tsx:59-61
Timestamp: 2025-07-13T11:29:25.245Z
Learning: In Next.js 13+ app router, components with the 'use client' directive run entirely on the client side and don't require window object existence checks or SSR hydration considerations. Direct access to window.location and other browser APIs is safe in client components.
Applied to files:
frontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx
📚 Learning: 2025-07-13T11:34:31.823Z
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: frontend/src/app/mentorship/programs/page.tsx:14-14
Timestamp: 2025-07-13T11:34:31.823Z
Learning: In the Next.js frontend mentorship application, there are two distinct types for authentication-related data: ExtendedSession for useSession hook (containing accessToken and user.login properties) and UserRolesData for useUserRoles hook (containing currentUserRoles.roles array). The correct access pattern for GitHub username is `(session as ExtendedSession)?.user?.login`.
Applied to files:
frontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx
🧬 Code graph analysis (1)
frontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx (3)
frontend/src/server/queries/programsQueries.ts (1)
GET_PROGRAM_AND_MODULES
(44-81)frontend/src/types/mentorship.ts (1)
Module
(52-64)frontend/src/app/global-error.tsx (1)
handleAppError
(66-86)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run backend tests
- GitHub Check: Run frontend unit tests
🔇 Additional comments (3)
frontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx (3)
8-8
: Good: centralized error handling introduced.Importing handleAppError keeps error UX consistent across the app.
10-10
: Importing GET_PROGRAM_AND_MODULES for cache updates is appropriate.Ensure the mutation response includes all fields that this query expects so the writeQuery injects complete objects. See separate verification note below.
132-132
: Navigation without refresh param looks right.Removing ?refresh=true aligns with the PR objectives.
b87a4c1
to
ec16079
Compare
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 (2)
frontend/__tests__/unit/components/ProgramCard.test.tsx (2)
27-33
: Improve Tooltip mock: add __esModule, widencontent
type, and forward propsPrevents ESM interop quirks and better matches library API (content is ReactNode). Also forwards extra props to reduce drift.
-jest.mock('@heroui/tooltip', () => ({ - Tooltip: ({ children, content }: { children: React.ReactNode; content: string }) => ( - <div data-testid="tooltip" title={content}> - {children} - </div> - ), -})) +jest.mock('@heroui/tooltip', () => ({ + __esModule: true, + Tooltip: ({ + children, + content, + ...props + }: { + children: React.ReactNode + content?: React.ReactNode + }) => ( + <div + data-testid="tooltip" + // Only set title when content is a string to avoid [object Object]. + title={typeof content === 'string' ? content : undefined} + {...props} + > + {children} + </div> + ), +}))
189-199
: LGTM: test now reflects CSS line‑clamp behaviorAsserting presence plus
line-clamp-6
is appropriate for the switch from manual truncation to CSS clamping.If feasible, consider targeting a stable selector (e.g.,
data-testid="program-description"
) instead of matching a 300‑char string to reduce brittleness.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/__tests__/unit/components/ProgramCard.test.tsx
(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: frontend/src/app/mentorship/programs/[programKey]/edit/page.tsx:90-128
Timestamp: 2025-07-12T17:14:28.536Z
Learning: Both ProgramForm (programCard.tsx) and ModuleForm (mainmoduleCard.tsx) components already implement HTML validation using the `required` attribute on form fields. The browser's native validation prevents form submission and displays error messages when required fields are empty, eliminating the need for additional JavaScript validation before GraphQL mutations.
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
🔇 Additional comments (1)
frontend/__tests__/unit/components/ProgramCard.test.tsx (1)
201-201
: Incorrect — clamp class is applied unconditionallyProgramCard renders the description inside
(frontend/src/components/ProgramCard.tsx, lines ~66–68), so the test asserting the class for both short and long descriptions is valid.
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/src/components/ModuleCard.tsx (1)
72-74
: Consider using relative navigation for better maintainability.The current implementation constructs the path using
window.location.pathname
, which could be brittle if the component is used in different contexts or if the URL structure changes.Consider using a more predictable approach:
-const handleClick = () => { - router.push(`${window.location.pathname}/modules/${details.key}`) -} +const handleClick = () => { + router.push(`./modules/${details.key}`) +}Or if you need more control over the path construction, consider accepting the base path as a prop to make the component more reusable.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/components/ModuleCard.tsx
(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
📚 Learning: 2025-09-17T02:42:41.928Z
Learnt from: Rajgupta36
PR: OWASP/Nest#2288
File: frontend/src/components/ActionButton.tsx:0-0
Timestamp: 2025-09-17T02:42:41.928Z
Learning: In frontend/src/components/ActionButton.tsx, the user Rajgupta36 intentionally changed text-blue-600 to text-[#1D7BD7] to align the text color with the border color (#1D7BD7) for visual consistency, prioritizing design alignment over theme tokens.
Applied to files:
frontend/src/components/ModuleCard.tsx
🧬 Code graph analysis (1)
frontend/src/components/ModuleCard.tsx (1)
frontend/src/components/TruncatedText.tsx (1)
TruncatedText
(3-45)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run frontend unit tests
- GitHub Check: Run backend tests
🔇 Additional comments (1)
frontend/src/components/ModuleCard.tsx (1)
78-84
: LGTM: Clean replacement of ActionButton with native button.The replacement of the ActionButton component with a native
button
element maintains the same functionality while simplifying the component structure. The styling withtext-start font-semibold text-blue-400 hover:underline
provides appropriate visual feedback for the clickable module name.
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.
Left one comment! The rest looks fine and works as expected 👍🏼
<button | ||
type="button" | ||
onClick={handleClick} | ||
className="text-start font-semibold text-blue-400 hover:underline" | ||
> |
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.
Why is this a button? 😮 Somehow missed that before..
This should be just a Link
element. Works the same and the navigation is actually faster.
<Link
href={`${window.location.pathname}/modules/${details.key}`}
className="text-start font-semibold text-blue-400 hover:underline"
>
<TruncatedText text={details?.name} />
</Link>
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)
frontend/src/components/ModuleCard.tsx (1)
73-79
: Removetype="button"
and stop usingwindow.location.pathname
— use usePathname()
- Drop the invalid
type="button"
on the Link in frontend/src/components/ModuleCard.tsx (ModuleItem).- Replace
${window.location.pathname}/…
with usePathname() from next/navigation in:
- frontend/src/components/ModuleCard.tsx (ModuleItem href)
- frontend/src/components/SingleModuleCard.tsx (Link href and router.push in handleEdit/handleCreate)
Add:import { usePathname } from 'next/navigation'
andconst pathname = usePathname()
inside the relevant components, then build routes with${pathname}/modules/${...}
.- types/mentorship.ts declares
Module.key: string
sodetails.key
should be present; the map still falls back tomodule.id
for the React key.- Optional: add
prefetch={false}
on ModuleCard links to avoid over-prefetching in large grids.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/components/ModuleCard.tsx
(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
🧬 Code graph analysis (1)
frontend/src/components/ModuleCard.tsx (1)
frontend/src/components/TruncatedText.tsx (1)
TruncatedText
(3-45)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run backend tests
- GitHub Check: Run frontend unit tests
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (1)
frontend/src/components/ModuleCard.tsx (1)
10-10
: Switching tonext/link
is the right call.This aligns with the earlier feedback to use a Link instead of a button for faster client-side navigation. Looks good.
|
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.
Nice 💪🏼
* Update docker-compose/local.yaml * UI/ux mentorship program update (#2244) * fix ui bugs * update format * fix test cases * fix test cases * update UI * update UI * update line clamp property * fix test case * Update styling --------- Co-authored-by: Kate Golovanova <[email protected]> Co-authored-by: Arkadii Yakovets <[email protected]> * fix params and remove refresh params * update direct cache * fix test cases * better exception caching * update logic Update docker-compose/local.yaml UI/ux mentorship program update (#2244) * fix ui bugs * update format * fix test cases * fix test cases * update UI * update UI * update line clamp property * fix test case * Update styling --------- Co-authored-by: Kate Golovanova <[email protected]> Co-authored-by: Arkadii Yakovets <[email protected]> * Update apollo cache logic on updating programs and modules * restore files * fix test cases * fix test cases * update button to link * fix type --------- Co-authored-by: Arkadii Yakovets <[email protected]> Co-authored-by: Kate Golovanova <[email protected]> Co-authored-by: Arkadii Yakovets <[email protected]>
* Improve UI/UX * update button padding * fix unit test cases * fix suggestions * Update docker-compose/local.yaml * UI/ux mentorship program update (#2244) * fix ui bugs * update format * fix test cases * fix test cases * update UI * update UI * update line clamp property * fix test case * Update styling --------- Co-authored-by: Kate Golovanova <[email protected]> Co-authored-by: Arkadii Yakovets <[email protected]> * fix conflicts * update ui * fix bugs * updated focus ring * format fix * Update docker-compose/local.yaml * UI/ux mentorship program update (#2244) * fix ui bugs * update format * fix test cases * fix test cases * update UI * update UI * update line clamp property * fix test case * Update styling --------- Co-authored-by: Kate Golovanova <[email protected]> Co-authored-by: Arkadii Yakovets <[email protected]> * fix params and remove refresh params (#2287) * Update docker-compose/local.yaml * UI/ux mentorship program update (#2244) * fix ui bugs * update format * fix test cases * fix test cases * update UI * update UI * update line clamp property * fix test case * Update styling --------- Co-authored-by: Kate Golovanova <[email protected]> Co-authored-by: Arkadii Yakovets <[email protected]> * fix params and remove refresh params * update direct cache * fix test cases * better exception caching * update logic Update docker-compose/local.yaml UI/ux mentorship program update (#2244) * fix ui bugs * update format * fix test cases * fix test cases * update UI * update UI * update line clamp property * fix test case * Update styling --------- Co-authored-by: Kate Golovanova <[email protected]> Co-authored-by: Arkadii Yakovets <[email protected]> * Update apollo cache logic on updating programs and modules * restore files * fix test cases * fix test cases * update button to link * fix type --------- Co-authored-by: Arkadii Yakovets <[email protected]> Co-authored-by: Kate Golovanova <[email protected]> Co-authored-by: Arkadii Yakovets <[email protected]> * update button --------- Co-authored-by: Arkadii Yakovets <[email protected]> Co-authored-by: Kate Golovanova <[email protected]> Co-authored-by: Arkadii Yakovets <[email protected]>
* Update docker-compose/local.yaml * UI/ux mentorship program update (#2244) * fix ui bugs * update format * fix test cases * fix test cases * update UI * update UI * update line clamp property * fix test case * Update styling --------- Co-authored-by: Kate Golovanova <[email protected]> Co-authored-by: Arkadii Yakovets <[email protected]> * fix params and remove refresh params * update direct cache * fix test cases * better exception caching * update logic Update docker-compose/local.yaml UI/ux mentorship program update (#2244) * fix ui bugs * update format * fix test cases * fix test cases * update UI * update UI * update line clamp property * fix test case * Update styling --------- Co-authored-by: Kate Golovanova <[email protected]> Co-authored-by: Arkadii Yakovets <[email protected]> * Update apollo cache logic on updating programs and modules * restore files * fix test cases * fix test cases * update button to link * fix type --------- Co-authored-by: Arkadii Yakovets <[email protected]> Co-authored-by: Kate Golovanova <[email protected]> Co-authored-by: Arkadii Yakovets <[email protected]>
* Improve UI/UX * update button padding * fix unit test cases * fix suggestions * Update docker-compose/local.yaml * UI/ux mentorship program update (#2244) * fix ui bugs * update format * fix test cases * fix test cases * update UI * update UI * update line clamp property * fix test case * Update styling --------- Co-authored-by: Kate Golovanova <[email protected]> Co-authored-by: Arkadii Yakovets <[email protected]> * fix conflicts * update ui * fix bugs * updated focus ring * format fix * Update docker-compose/local.yaml * UI/ux mentorship program update (#2244) * fix ui bugs * update format * fix test cases * fix test cases * update UI * update UI * update line clamp property * fix test case * Update styling --------- Co-authored-by: Kate Golovanova <[email protected]> Co-authored-by: Arkadii Yakovets <[email protected]> * fix params and remove refresh params (#2287) * Update docker-compose/local.yaml * UI/ux mentorship program update (#2244) * fix ui bugs * update format * fix test cases * fix test cases * update UI * update UI * update line clamp property * fix test case * Update styling --------- Co-authored-by: Kate Golovanova <[email protected]> Co-authored-by: Arkadii Yakovets <[email protected]> * fix params and remove refresh params * update direct cache * fix test cases * better exception caching * update logic Update docker-compose/local.yaml UI/ux mentorship program update (#2244) * fix ui bugs * update format * fix test cases * fix test cases * update UI * update UI * update line clamp property * fix test case * Update styling --------- Co-authored-by: Kate Golovanova <[email protected]> Co-authored-by: Arkadii Yakovets <[email protected]> * Update apollo cache logic on updating programs and modules * restore files * fix test cases * fix test cases * update button to link * fix type --------- Co-authored-by: Arkadii Yakovets <[email protected]> Co-authored-by: Kate Golovanova <[email protected]> Co-authored-by: Arkadii Yakovets <[email protected]> * update button --------- Co-authored-by: Arkadii Yakovets <[email protected]> Co-authored-by: Kate Golovanova <[email protected]> Co-authored-by: Arkadii Yakovets <[email protected]>
Proposed change
Resolves #2274
Description