Added minor UI\UX updates for Mentorship portal#2731
Merged
Conversation
Contributor
Summary by CodeRabbitRelease Notes
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughRenames mentee identifier from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx (1)
4-4: Experience level display normalization is correct; consider aligning other viewsUsing a capitalization helper for
module.experienceLevelgives the desired "Advanced"/"Beginner" style and is safe sinceexperienceLevelis required and non-null (no extra checks needed). Based on learnings, this matches the intended UX for experience level.For consistency, you may want to use the same normalization wherever module experience level is shown (e.g., in
ModuleCard), so users don’t see mixed casing between different module views.Also applies to: 51-59
frontend/src/components/ModuleCard.tsx (1)
73-93: ModuleItem refactor using module + isAdmin is solid; consider matching experience-level casing with other viewsPassing the full
moduleplusisAdminintoModuleItemmakes the component more self-contained and enables the clean admin-only labels rendering (isAdmin && module.labels && module.labels.length > 0), which aligns with the requirement to hide labels on public module cards. Directly accessingmodule.experienceLevelis also fine since that field is required. Based on learnings, this avoids unnecessary null checks.If you want a fully consistent UX, consider applying the same title-style normalization for the experience level here as on the module details page so users see “Advanced” rather than an all-caps variant in different places.
frontend/__tests__/unit/pages/Program.test.tsx (1)
101-105: Consider a more specific selector for the card.Using
getAllByRole('button')[0]assumes the first button is the program card, which could be fragile if the component structure changes or other buttons are added.Consider using a more specific selector:
- const card = screen.getAllByRole('button')[0] // First program card + const card = screen.getByRole('button', { name: /Program 1/i }) fireEvent.click(card)Or add a data-testid to the ProgramCard component for more reliable testing.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
frontend/src/types/__generated__/graphql.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/menteeQueries.generated.tsis excluded by!**/__generated__/**
📒 Files selected for processing (27)
backend/apps/mentorship/api/internal/queries/mentorship.py(3 hunks)frontend/__tests__/unit/components/ProgramActions.test.tsx(0 hunks)frontend/__tests__/unit/components/ProgramCard.test.tsx(10 hunks)frontend/__tests__/unit/pages/CreateModule.test.tsx(1 hunks)frontend/__tests__/unit/pages/CreateProgram.test.tsx(4 hunks)frontend/__tests__/unit/pages/EditModule.test.tsx(1 hunks)frontend/__tests__/unit/pages/EditProgram.test.tsx(1 hunks)frontend/__tests__/unit/pages/Program.test.tsx(2 hunks)frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx(2 hunks)frontend/src/app/mentorship/programs/[programKey]/page.tsx(1 hunks)frontend/src/app/mentorship/programs/page.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/[moduleKey]/issues/[issueId]/page.tsx(2 hunks)frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/issues/page.tsx(1 hunks)frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/mentees/[menteeKey]/page.tsx(2 hunks)frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx(1 hunks)frontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx(1 hunks)frontend/src/app/my/mentorship/programs/[programKey]/page.tsx(1 hunks)frontend/src/components/CardDetailsPage.tsx(3 hunks)frontend/src/components/EntityActions.tsx(4 hunks)frontend/src/components/ModuleCard.tsx(2 hunks)frontend/src/components/ModuleForm.tsx(1 hunks)frontend/src/components/ProgramCard.tsx(4 hunks)frontend/src/components/ProgramForm.tsx(1 hunks)frontend/src/components/SingleModuleCard.tsx(3 hunks)frontend/src/server/queries/menteeQueries.ts(2 hunks)
💤 Files with no reviewable changes (1)
- frontend/tests/unit/components/ProgramActions.test.tsx
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 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-07-12T17:14:28.536Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 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/__tests__/unit/pages/CreateModule.test.tsxfrontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsxfrontend/__tests__/unit/pages/CreateProgram.test.tsxfrontend/src/components/ModuleForm.tsxfrontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsxfrontend/src/components/ProgramForm.tsxfrontend/__tests__/unit/components/ProgramCard.test.tsxfrontend/__tests__/unit/pages/Program.test.tsxfrontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsxfrontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 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.tsxfrontend/__tests__/unit/pages/CreateProgram.test.tsxfrontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsxfrontend/src/components/ProgramForm.tsxfrontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/issues/[issueId]/page.tsxfrontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/issues/page.tsxfrontend/src/app/my/mentorship/programs/[programKey]/page.tsxfrontend/src/app/mentorship/programs/page.tsxfrontend/src/server/queries/menteeQueries.tsfrontend/__tests__/unit/components/ProgramCard.test.tsxfrontend/src/components/ProgramCard.tsxfrontend/__tests__/unit/pages/EditProgram.test.tsxfrontend/__tests__/unit/pages/Program.test.tsxfrontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsxfrontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsxfrontend/src/components/SingleModuleCard.tsxfrontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsxfrontend/src/app/mentorship/programs/[programKey]/page.tsxfrontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/mentees/[menteeKey]/page.tsxfrontend/src/components/CardDetailsPage.tsxfrontend/src/components/EntityActions.tsx
📚 Learning: 2025-09-29T06:02:35.566Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2178
File: frontend/src/components/SingleModuleCard.tsx:54-54
Timestamp: 2025-09-29T06:02:35.566Z
Learning: In the Module type from types/mentorship.ts, the experienceLevel field is required (experienceLevel: ExperienceLevelEnum), not optional, so null/undefined checks are not needed when accessing this field.
Applied to files:
frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsxfrontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsxfrontend/src/components/ModuleCard.tsxfrontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/issues/[issueId]/page.tsxfrontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/issues/page.tsxfrontend/src/app/my/mentorship/programs/[programKey]/page.tsxfrontend/src/app/mentorship/programs/page.tsxfrontend/src/server/queries/menteeQueries.tsfrontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsxfrontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsxfrontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsxfrontend/src/app/mentorship/programs/[programKey]/page.tsxfrontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/mentees/[menteeKey]/page.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 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.tsxfrontend/__tests__/unit/pages/CreateProgram.test.tsxfrontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsxfrontend/src/components/ProgramForm.tsxfrontend/src/app/my/mentorship/programs/[programKey]/page.tsxfrontend/src/app/mentorship/programs/page.tsxfrontend/src/server/queries/menteeQueries.tsfrontend/__tests__/unit/components/ProgramCard.test.tsxfrontend/src/components/ProgramCard.tsxfrontend/__tests__/unit/pages/Program.test.tsxfrontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsxfrontend/src/app/mentorship/programs/[programKey]/page.tsx
📚 Learning: 2025-07-13T07:31:06.511Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 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]/edit/page.tsxfrontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsxfrontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/issues/page.tsxfrontend/src/app/my/mentorship/programs/[programKey]/page.tsxfrontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsxfrontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsxfrontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsxfrontend/src/app/mentorship/programs/[programKey]/page.tsxfrontend/src/components/CardDetailsPage.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 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.tsxfrontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsxfrontend/src/components/ProgramForm.tsxfrontend/src/app/mentorship/programs/page.tsxfrontend/src/server/queries/menteeQueries.tsfrontend/__tests__/unit/components/ProgramCard.test.tsxfrontend/src/components/ProgramCard.tsxfrontend/__tests__/unit/pages/Program.test.tsxfrontend/src/app/mentorship/programs/[programKey]/page.tsx
📚 Learning: 2025-07-12T17:36:57.255Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: frontend/__tests__/unit/pages/createProgram.test.tsx:70-86
Timestamp: 2025-07-12T17:36:57.255Z
Learning: When testing React page components that use mocked form components, validation logic should be tested at the form component level, not the page level. Page-level tests should focus on authentication, role checking, submission handling, and navigation logic.
Applied to files:
frontend/__tests__/unit/pages/CreateProgram.test.tsxfrontend/__tests__/unit/components/ProgramCard.test.tsxfrontend/__tests__/unit/pages/EditProgram.test.tsxfrontend/__tests__/unit/pages/EditModule.test.tsxfrontend/__tests__/unit/pages/Program.test.tsx
📚 Learning: 2025-11-17T16:47:05.578Z
Learnt from: anurag2787
Repo: OWASP/Nest PR: 2671
File: frontend/__tests__/unit/components/MultiSearch.test.tsx:427-427
Timestamp: 2025-11-17T16:47:05.578Z
Learning: In the frontend test files for the OWASP/Nest repository, `expect(true).toBe(true)` no-op assertions may be intentionally added as workarounds when ESLint rule jest/expect-expect doesn't detect expectations inside helper functions or waitFor callbacks. These can be resolved by configuring the ESLint rule's assertFunctionNames option to recognize custom assertion helpers instead of flagging them as redundant.
Applied to files:
frontend/__tests__/unit/pages/CreateProgram.test.tsxfrontend/__tests__/unit/components/ProgramCard.test.tsxfrontend/__tests__/unit/pages/EditProgram.test.tsxfrontend/__tests__/unit/pages/Program.test.tsx
📚 Learning: 2025-07-13T11:34:31.823Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 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]/page.tsxfrontend/src/components/SingleModuleCard.tsxfrontend/src/components/CardDetailsPage.tsx
📚 Learning: 2025-07-08T17:24:36.501Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 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/app/mentorship/programs/page.tsx
📚 Learning: 2025-07-13T11:29:25.245Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 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/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx
📚 Learning: 2025-07-12T17:12:25.807Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: frontend/__tests__/unit/pages/ProgramDetails.test.tsx:35-0
Timestamp: 2025-07-12T17:12:25.807Z
Learning: In React applications, button elements should always have an explicit type attribute (type="button", type="submit", or type="reset") to prevent unintended form submission behavior, even when the button appears to be outside of a form context. The default type is "submit" which can cause unexpected behavior.
Applied to files:
frontend/src/components/EntityActions.tsx
🧬 Code graph analysis (5)
frontend/src/components/ModuleCard.tsx (6)
backend/apps/mentorship/models/module.py (1)
Module(17-99)frontend/src/types/mentorship.ts (1)
Module(37-51)frontend/src/components/TruncatedText.tsx (1)
TruncatedText(3-45)frontend/src/components/InfoItem.tsx (1)
TextInfoItem(37-52)frontend/src/utils/dateFormatter.ts (1)
formatDate(1-20)frontend/src/components/LabelList.tsx (1)
LabelList(40-40)
frontend/src/app/mentorship/programs/page.tsx (1)
backend/apps/mentorship/api/internal/nodes/enum.py (1)
ProgramStatusEnum(22-27)
frontend/src/components/SingleModuleCard.tsx (2)
backend/apps/mentorship/api/internal/nodes/program.py (1)
admins(32-34)frontend/src/types/auth.ts (1)
ExtendedSession(8-20)
frontend/src/components/CardDetailsPage.tsx (2)
backend/apps/mentorship/api/internal/nodes/program.py (1)
admins(32-34)frontend/src/types/auth.ts (1)
ExtendedSession(8-20)
frontend/src/components/EntityActions.tsx (1)
backend/apps/mentorship/api/internal/nodes/enum.py (1)
ProgramStatusEnum(22-27)
🪛 GitHub Check: SonarCloud Code Analysis
frontend/src/components/ProgramCard.tsx
[warning] 56-67: Use , , , , or instead of the "button" role to ensure accessibility across all devices.
⏰ 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). (5)
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run backend tests
- GitHub Check: Run frontend unit tests
- GitHub Check: CodeQL (javascript-typescript)
- GitHub Check: Analyze (python)
🔇 Additional comments (29)
frontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx (1)
21-21: LGTM! TypeScript best practice applied.The refactoring from type assertion to generic type parameter correctly follows TypeScript best practices for
useParams. This provides better type inference and aligns with the PR's objective to replace type assertions with preferred patterns.frontend/src/app/mentorship/programs/page.tsx (1)
57-59: I'll help verify the necessity of the.toUpperCase()call and type alignment. Let me search the codebase for relevant type definitions and usage patterns.
<function_calls>
#!/bin/bashFind ProgramStatusEnum definition in generated types
echo "=== Searching for ProgramStatusEnum definition ==="
fd -e ts -e tsx | xargs rg -n "enum ProgramStatusEnum|ProgramStatusEnum\s*=" | head -20echo -e "\n=== Searching for Program type definition ==="
rg -n "type Program\s*=|interface Program" --type ts --type tsx -A 5 | head -50echo -e "\n=== Searching for direct status comparisons without toUpperCase ==="
rg -n ".status\s*===\s*ProgramStatusEnum" --type ts --type tsx -B 2 -A 2 | head -40echo -e "\n=== Searching for status with toUpperCase ==="
rg -n ".status.toUpperCase|toUpperCase..status" --type ts --type tsx -B 2 -A 2 | head -40
Let me refine the search to locate the GraphQL generated types more effectively: <function_calls> <invoke name="shell"> <parameter name="command">#!/bin/bash # Find generated GraphQL types files echo "=== Finding generated GraphQL types ===" fd -e ts -e tsx -path "*generated*" -o -path "*__generated__*" | head -20 echo -e "\n=== Looking for GraphQL schema or types with Program ===" fd -e ts -e tsx | xargs rg -l "ProgramStatusEnum" | head -10 </parameter> </invoke>Let me check the specific file under review and trace its imports:
<function_calls>
#!/bin/bashRead the file under review to see imports
echo "=== File: frontend/src/app/mentorship/programs/page.tsx ==="
wc -l frontend/src/app/mentorship/programs/page.tsxecho -e "\n=== First 80 lines (imports and context) ==="
head -80 frontend/src/app/mentorship/programs/page.tsxecho -e "\n=== Lines 50-70 (around the code in question) ==="
sed -n '50,70p' frontend/src/app/mentorship/programs/page.tsx
</blockquote></details> <details> <summary>frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (1)</summary><blockquote> `19-36`: **Typed useParams generic is a clean TS-only improvement** Switching to `useParams<{ programKey: string }>()` removes a type assertion, keeps runtime identical, and makes route param usage clearer. </blockquote></details> <details> <summary>frontend/src/app/mentorship/programs/[programKey]/page.tsx (1)</summary><blockquote> `15-28`: **Consistent route-param typing across program details pages** Using `useParams<{ programKey: string }>()` here matches the pattern on the corresponding “my/mentorship” page and improves type safety without changing behavior. Based on learnings, this aligns with our Next.js app router usage. </blockquote></details> <details> <summary>frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx (1)</summary><blockquote> `18-31`: **Route param typing tightened for edit page** `useParams<{ programKey: string }>()` is a straightforward TS improvement and fits the existing use with GraphQL variables and `skip: !programKey`. </blockquote></details> <details> <summary>frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx (1)</summary><blockquote> `18-36`: **Generic useParams for programKey/moduleKey matches usage** Typing `useParams<{ programKey: string; moduleKey: string }>()` aligns with how both params are used (query variables, redirects) and avoids unsafe casting. </blockquote></details> <details> <summary>frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx (1)</summary><blockquote> `14-25`: **Type-safe useParams for module details page** `useParams<{ programKey: string; moduleKey: string }>()` is a nice type-safety upgrade and keeps the existing data-fetch and DetailsCard wiring intact. </blockquote></details> <details> <summary>frontend/src/components/ModuleForm.tsx (1)</summary><blockquote> `62-80`: **Form label rename to “Name *” is consistent and contextual** Renaming the label to “Name *” is fine given the surrounding “Edit/Create Module” context and keeps behavior (required, id, name binding) unchanged. </blockquote></details> <details> <summary>frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/issues/page.tsx (1)</summary><blockquote> `18-35`: **Typed useParams for issues list aligns with GraphQL usage** The generic `useParams<{ programKey: string; moduleKey: string }>()` matches how these params are used for querying and routing, removing the need for casts. </blockquote></details> <details> <summary>frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/issues/[issueId]/page.tsx (2)</summary><blockquote> `26-32`: **Params typing now matches issue view usage** Using `useParams<{ programKey: string; moduleKey: string; issueId: string }>()` lines up with the GraphQL variables and the issue mutations hook; this is a clean TS-only enhancement. --- `263-305`: **Assignee link now targets mentee profile route** Updating the assignee link to `/mentees/${a.login}` matches the PR’s intent to route assignees to mentee pages rather than generic members. Please just confirm that all expected assignee users have corresponding `/mentees/[login]` pages so this never 404s. </blockquote></details> <details> <summary>frontend/src/server/queries/menteeQueries.ts (1)</summary><blockquote> `4-6`: **menteeKey rename in mentee details query is consistent with backend** The variable and argument rename to `menteeKey` is applied consistently within this query and matches the backend resolver signatures, without altering the response shape. This should keep the mentee details and issues fetching behavior intact as long as callers pass `menteeKey` in variables. Also applies to: 15-19 </blockquote></details> <details> <summary>frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/mentees/[menteeKey]/page.tsx (1)</summary><blockquote> `16-20`: **Route params and data-fetch variables now use menteeKey consistently** Typing `useParams` to include `menteeKey` and wiring that through to the query variables (plus the `skip` guard) brings this page in line with the updated mentee key naming and avoids running the query when any required param is absent. The state/update logic below remains compatible with this change. Also applies to: 28-36 </blockquote></details> <details> <summary>backend/apps/mentorship/api/internal/queries/mentorship.py (1)</summary><blockquote> `51-78`: **mentee_key renames in mentorship queries are internally consistent** Both `get_mentee_details` and `get_mentee_module_issues` now take `mentee_key` and use it consistently for GitHub user lookup and error messages, without altering the underlying logic for enrollment checks or issue retrieval. This lines up with the frontend query updates and should keep behavior unchanged aside from the naming. Also applies to: 85-104 </blockquote></details> <details> <summary>frontend/__tests__/unit/pages/EditProgram.test.tsx (1)</summary><blockquote> `89-93`: **EditProgramPage test correctly targets the updated “Name *” label** Updating the expectation to look for the “Name *” label keeps this test aligned with the simplified form label text while still verifying that the program name field renders as expected. </blockquote></details> <details> <summary>frontend/__tests__/unit/pages/CreateModule.test.tsx (1)</summary><blockquote> `68-71`: **CreateModulePage test now matches the unified “Name *” label** Switching the module name field selector to `getByLabelText('Name *')` reflects the updated label text and keeps the form-submission test behavior intact. </blockquote></details> <details> <summary>frontend/__tests__/unit/pages/EditModule.test.tsx (1)</summary><blockquote> `88-91`: **EditModulePage test aligned with the new “Name *” label** The test now locates the module name field via the updated “Name *” label and updates its value accordingly, staying in sync with the simplified labels on the edit module form while preserving the original test intent. </blockquote></details> <details> <summary>frontend/src/components/ModuleCard.tsx (1)</summary><blockquote> `40-48`: **No action needed—the href pattern is safe in all usage contexts** ModuleCard is only rendered on program detail routes (`/mentorship/programs/[programKey]`), where the pathname doesn't already contain `/modules`. The constructed href (`${pathname}/modules/${module.key}`) correctly produces `/mentorship/programs/{programKey}/modules/{moduleKey}`, matching the actual route structure at `frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx`. There is no risk of duplicated segments. </blockquote></details> <details> <summary>frontend/src/components/ProgramForm.tsx (1)</summary><blockquote> `67-67`: **LGTM! Label simplification aligns with UI/UX improvements.** The label change from "Program Name *" to "Name *" removes redundancy and streamlines the form interface, consistent with the broader UI simplification pattern across the PR. </blockquote></details> <details> <summary>frontend/__tests__/unit/pages/CreateProgram.test.tsx (1)</summary><blockquote> `63-63`: **LGTM! Test selectors correctly updated to match UI changes.** All label references have been consistently updated from "Program Name *" to "Name *" across all test scenarios (initial render, success path, and error handling). Also applies to: 106-106, 130-131, 189-191 </blockquote></details> <details> <summary>frontend/src/components/CardDetailsPage.tsx (2)</summary><blockquote> `96-108`: **LGTM! EntityActions integration properly handles both program and module contexts.** The conditional rendering correctly distinguishes between program and module types, with appropriate permission checks for each context. --- `208-229`: **Good defensive rendering for Tags/Domains sections.** Wrapping the Tags/Domains grid in a conditional check prevents rendering empty sections when no data exists, improving the UI experience. </blockquote></details> <details> <summary>frontend/__tests__/unit/components/ProgramCard.test.tsx (2)</summary><blockquote> `109-123`: **LGTM! Tests correctly validate the new clickable card interaction.** The tests properly verify that cards trigger navigation when clicked, covering both admin and user access levels. Also applies to: 175-189 --- `255-291`: **Description line-clamp tests correctly updated.** All test assertions have been consistently updated to expect `line-clamp-8` instead of `line-clamp-6`, matching the UI change that allows more description text to be visible. </blockquote></details> <details> <summary>frontend/src/components/EntityActions.tsx (4)</summary><blockquote> `10-24`: **LGTM! Well-designed unified component interface.** The props structure cleanly supports both program and module contexts with appropriate optional fields. Making `status` and `setStatus` optional allows for flexible usage across different scenarios. --- `37-46`: **Good defensive programming for module actions.** Gating the module-specific routes behind `moduleKey` checks prevents navigation errors when the key is undefined. --- `48-55`: **Optional chaining correctly handles optional status updates.** Using `setStatus?.()` prevents runtime errors when `setStatus` is not provided, aligning with the optional prop design. --- `96-99`: **Good use of stopPropagation.** Stopping event propagation prevents the dropdown toggle from triggering parent click handlers (like the card click in ProgramCard). </blockquote></details> <details> <summary>frontend/src/components/SingleModuleCard.tsx (1)</summary><blockquote> `62-62`: **EntityActions integration simplifies the admin UI.** Replacing the inline dropdown with the unified EntityActions component reduces code duplication and ensures consistent behavior across program and module cards. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
|
The PR must be linked to an issue assigned to the PR author. |
|
The PR must be linked to an issue assigned to the PR author. |
|
The PR must be linked to an issue assigned to the PR author. |
Collaborator
Yes, that's my fault. I updated the check-issue-pr action. It should work fine now. |
Collaborator
|
Reopening. |
|
The PR must be linked to an issue assigned to the PR author. |
|
The PR must be linked to an issue assigned to the PR author. |
|
The PR must be linked to an issue assigned to the PR author. |
|
|
Please retry analysis of this Pull-Request directly on SonarQube Cloud |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
frontend/src/components/EntityActions.tsx (1)
10-77: EntityActions correctly centralizes program/module actions; consider minor TS & a11y polishThe routing and status-change logic for both
programandmoduletypes looks correct, and the outside‑click handling should keep the dropdown tidy.Two small follow‑ups you might consider:
- Tighten types by using
status?: ProgramStatusEnumandsetStatus?: (newStatus: ProgramStatusEnum) => voidto catch mismatches at compile time.- In a later pass, add basic keyboard support inside the menu (e.g., close on Escape, up/down to move focus) to complement the existing ARIA attributes.
Also applies to: 78-137
frontend/src/components/ProgramCard.tsx (1)
3-16: Link-wrapped card and embedded EntityActions behave correctly; watch nested interactive markupTurning the outer container into a
Linkwithhref={href}and moving admin controls intoEntityActionscleanly achieves “clickable card” while keeping status changes scoped to admins only.One thing to keep in mind for a future tidy‑up:
EntityActionsrenders a<button>inside the anchor produced by the Link, which is technically nested interactive content. It works in practice, but if you run into accessibility tooling complaints you may want to move the actions outside the anchor or restructure so the actions sit in a sibling container rather than inside the link.Also applies to: 56-61, 62-85, 99-103
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
frontend/__tests__/unit/components/ProgramCard.test.tsx(20 hunks)frontend/__tests__/unit/components/SingleModuleCard.test.tsx(9 hunks)frontend/__tests__/unit/pages/Program.test.tsx(2 hunks)frontend/src/app/mentorship/programs/page.tsx(2 hunks)frontend/src/app/my/mentorship/page.tsx(1 hunks)frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/issues/[issueId]/page.tsx(2 hunks)frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/mentees/[menteeKey]/page.tsx(3 hunks)frontend/src/components/EntityActions.tsx(1 hunks)frontend/src/components/ModuleCard.tsx(2 hunks)frontend/src/components/ProgramCard.tsx(4 hunks)frontend/src/components/SingleModuleCard.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/mentees/[menteeKey]/page.tsx
- frontend/src/app/mentorship/programs/page.tsx
🧰 Additional context used
🧠 Learnings (12)
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 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]/issues/[issueId]/page.tsxfrontend/__tests__/unit/pages/Program.test.tsxfrontend/__tests__/unit/components/ProgramCard.test.tsxfrontend/src/components/ProgramCard.tsxfrontend/src/app/my/mentorship/page.tsxfrontend/src/components/SingleModuleCard.tsxfrontend/src/components/ModuleCard.tsx
📚 Learning: 2025-09-29T06:02:35.566Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2178
File: frontend/src/components/SingleModuleCard.tsx:54-54
Timestamp: 2025-09-29T06:02:35.566Z
Learning: In the Module type from types/mentorship.ts, the experienceLevel field is required (experienceLevel: ExperienceLevelEnum), not optional, so null/undefined checks are not needed when accessing this field.
Applied to files:
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/issues/[issueId]/page.tsxfrontend/src/app/my/mentorship/page.tsxfrontend/src/components/ModuleCard.tsx
📚 Learning: 2025-07-13T07:31:06.511Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 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/[moduleKey]/issues/[issueId]/page.tsxfrontend/__tests__/unit/components/SingleModuleCard.test.tsxfrontend/src/components/SingleModuleCard.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 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/__tests__/unit/pages/Program.test.tsxfrontend/src/components/ProgramCard.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 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/__tests__/unit/pages/Program.test.tsx
📚 Learning: 2025-07-12T17:36:57.255Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: frontend/__tests__/unit/pages/createProgram.test.tsx:70-86
Timestamp: 2025-07-12T17:36:57.255Z
Learning: When testing React page components that use mocked form components, validation logic should be tested at the form component level, not the page level. Page-level tests should focus on authentication, role checking, submission handling, and navigation logic.
Applied to files:
frontend/__tests__/unit/pages/Program.test.tsxfrontend/__tests__/unit/components/ProgramCard.test.tsxfrontend/__tests__/unit/components/SingleModuleCard.test.tsx
📚 Learning: 2025-07-12T17:14:28.536Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 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/__tests__/unit/pages/Program.test.tsxfrontend/src/components/ProgramCard.tsxfrontend/src/app/my/mentorship/page.tsxfrontend/src/components/SingleModuleCard.tsxfrontend/src/components/ModuleCard.tsx
📚 Learning: 2025-11-17T16:47:05.578Z
Learnt from: anurag2787
Repo: OWASP/Nest PR: 2671
File: frontend/__tests__/unit/components/MultiSearch.test.tsx:427-427
Timestamp: 2025-11-17T16:47:05.578Z
Learning: In the frontend test files for the OWASP/Nest repository, `expect(true).toBe(true)` no-op assertions may be intentionally added as workarounds when ESLint rule jest/expect-expect doesn't detect expectations inside helper functions or waitFor callbacks. These can be resolved by configuring the ESLint rule's assertFunctionNames option to recognize custom assertion helpers instead of flagging them as redundant.
Applied to files:
frontend/__tests__/unit/pages/Program.test.tsxfrontend/__tests__/unit/components/ProgramCard.test.tsxfrontend/__tests__/unit/components/SingleModuleCard.test.tsx
📚 Learning: 2025-07-12T17:12:25.807Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: frontend/__tests__/unit/pages/ProgramDetails.test.tsx:35-0
Timestamp: 2025-07-12T17:12:25.807Z
Learning: In React applications, button elements should always have an explicit type attribute (type="button", type="submit", or type="reset") to prevent unintended form submission behavior, even when the button appears to be outside of a form context. The default type is "submit" which can cause unexpected behavior.
Applied to files:
frontend/src/components/ProgramCard.tsx
📚 Learning: 2025-04-30T13:41:20.846Z
Learnt from: codic-yeeshu
Repo: OWASP/Nest PR: 1444
File: frontend/src/components/NavDropDown.tsx:0-0
Timestamp: 2025-04-30T13:41:20.846Z
Learning: When implementing dropdown menus or similar interactive components, always include proper accessibility features: ARIA attributes (aria-expanded, aria-haspopup, aria-controls), keyboard navigation support (Enter, Space, Escape keys), and mechanisms to close dropdowns when clicking outside.
Applied to files:
frontend/src/components/ProgramCard.tsx
📚 Learning: 2025-07-13T11:29:25.245Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 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/__tests__/unit/components/SingleModuleCard.test.tsxfrontend/src/components/SingleModuleCard.tsx
📚 Learning: 2025-07-13T11:34:31.823Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 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/__tests__/unit/components/SingleModuleCard.test.tsxfrontend/src/components/SingleModuleCard.tsx
🧬 Code graph analysis (3)
frontend/src/components/ProgramCard.tsx (3)
frontend/src/types/mentorship.ts (1)
Program(7-22)backend/apps/mentorship/models/program.py (1)
Program(18-87)frontend/src/types/link.ts (1)
Link(1-7)
frontend/src/components/EntityActions.tsx (1)
backend/apps/mentorship/api/internal/nodes/enum.py (1)
ProgramStatusEnum(22-27)
frontend/src/components/ModuleCard.tsx (4)
frontend/src/types/mentorship.ts (1)
Module(37-51)frontend/src/components/TruncatedText.tsx (1)
TruncatedText(3-45)frontend/src/components/InfoItem.tsx (1)
TextInfoItem(37-52)frontend/src/utils/dateFormatter.ts (1)
formatDate(1-20)
🔇 Additional comments (8)
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/issues/[issueId]/page.tsx (2)
27-28: LGTM! Type-safe parameter extraction.The generic typing of
useParamsis the preferred TypeScript pattern and provides better type safety than runtime assertions.
269-269: Mentee route exists and href path is correct.The target route is properly defined at
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/mentees/[menteeKey]/page.tsx, which matches the href structure on line 269. The navigation update successfully points to the mentee page as intended.frontend/src/components/ModuleCard.tsx (1)
29-41: ModuleItem prop reshaping and admin-only labels look consistentPassing
{ module, isAdmin }intoModuleItemand gating label rendering onisAdmin && module.labels?.lengthcleanly separates public vs admin views and keeps the card API simple. No issues spotted here.Based on learnings, the non-optional
experienceLevelfield makes the direct access safe.Also applies to: 66-86
frontend/src/app/my/mentorship/page.tsx (1)
136-143: ProgramCard href usage matches new navigation modelSwitching to
href={/my/mentorship/programs/${p.key}}aligns with the updated ProgramCard API and keeps admin visibility driven byaccessLevel="admin"plusisAdmin={p.userRole === 'admin'}. Looks correct.frontend/__tests__/unit/components/SingleModuleCard.test.tsx (1)
3-69: Tests correctly cover pathname-based module links and simplified admin propsThe added
usePathnamemock and updated expectations for/my/mentorship/programs/test-program/modules/test-modulealign with the component’s new link logic, and the extra cases around admin props/undefined admins ensure the new props surface is well covered.Also applies to: 82-268
frontend/__tests__/unit/pages/Program.test.tsx (1)
56-103: Program detail navigation test matches Link-based cardsAsserting a link with
href="/mentorship/programs/program_1"instead of relying on router.push correctly reflects the new “card is a link” behaviour. The test looks solid.frontend/__tests__/unit/components/ProgramCard.test.tsx (1)
3-12: ProgramCard tests comprehensively exercise the new href + EntityActions behaviourThe updated suite does a nice job of validating:
- href wiring for both admin/user views,
- visibility of the actions button only for admins,
- that clicking “Edit” in the dropdown triggers the expected edit route,
- and the updated
line-clamp-8description styling and date fallbacks.Everything here looks consistent with the refactored ProgramCard.
Also applies to: 53-59, 62-75, 87-103, 129-168, 185-215, 221-257, 265-389, 391-444
frontend/src/components/SingleModuleCard.tsx (1)
1-3: SingleModuleCard’s navigation and admin actions wiring look correctUsing the current pathname to build the module link and extract
programKey, plus the stricter admin check against theadminslist and session login, cleanly supports both public linking and admin‑only actions viaEntityActions. The new props surface from ModuleCard and the updated tests line up with this behaviour.Also applies to: 7-9, 13-15, 25-35, 49-66
This was referenced Dec 27, 2025
This was referenced Jan 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Covers minor updates in #2701 except for a few things:
Checklist
make check-testlocally; all checks and tests passed.