Improve Module Card: Show mentors/mentee avatars & change grid to 3 columns#3296
Conversation
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughModuleCard and SingleModuleCard now surface mentor and mentee avatars (filtered, capped at 4 with a "+N" overflow), swap lodash Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @frontend/src/components/ModuleCard.tsx:
- Around line 163-165: The current weeks calculation can produce 0 or negative
values for same-day or inverted date ranges; update the logic that computes
`weeks` from `days` (the block using `const weeks = Math.ceil(days / 7)` and the
subsequent return) to first validate the input (`days`) and handle `end < start`
as invalid, then clamp the result to a minimum of 1 (e.g., treat 0 or negative
as 1 week) before formatting the string; ensure any invalid date case either
returns a clear sentinel (e.g., "Invalid dates") or triggers the same clamp
depending on desired UX.
- Around line 68-83: The Link href can generate /modules/undefined because
ModuleItem uses module.key only while elsewhere the list key falls back to
module.id; update the Link generation in ModuleItem (the Link component using
pathname and module.key) to use the same fallback (module.key || module.id) so
the route always targets a valid identifier (i.e., replace usage of module.key
in the href with the fallback).
- Line 75: Replace the non-existent Tailwind utilities with standard ones:
change any "border-1" to "border", "border-l-1" to "border-l", and "shadow-xs"
to "shadow-sm" (or "shadow" if a larger shadow is intended). Update these
classnames in ModuleCard (the root div and other occurrences), and also sweep
RepositoryCard, ApiKeySkelton, Card, and any other components that use the same
invalid utilities; perform a project-wide search for "border-1", "border-l-1",
and "shadow-xs" and replace them with the appropriate standard Tailwind classes.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/components/ModuleCard.tsx
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
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.
📚 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/components/ModuleCard.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/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/components/ModuleCard.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/components/ModuleCard.tsx
🧬 Code graph analysis (1)
frontend/src/components/ModuleCard.tsx (3)
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 (3)
frontend/src/components/ModuleCard.tsx (3)
39-39: Grid change tolg:grid-cols-3matches issue objective.
2-2: Verifynext/imageremote configuration for GitHub avatars (and considersizes).This will only work if
next.config.*allows the avatar host viaimages.domains/images.remotePatterns. Also, since these render at 24×24, consider adding asizeshint (or keep as-is if you don’t rely on responsive image selection).Also applies to: 95-105, 118-128
82-82: ConfirmtoUpperCase()is the intended UX for “Level”.This changes presentation (e.g.,
BEGINNERvsBeginner). If you wanted “title case”, replace with a small formatter (no lodash needed).
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/src/components/ModuleCard.tsx (1)
120-142: Same URL pattern issue applies here (line 130).The mentees section has the same
&s=60URL construction concern as the mentors section.Additionally, the avatar rendering logic is duplicated between mentors (lines 100-117) and mentees (lines 123-140). Consider extracting to a reusable component if this pattern is used elsewhere.
♻️ Optional: Extract avatar list to helper component
const AvatarList = ({ contributors, label }: { contributors: Array<{ login: string; name?: string; avatarUrl: string }> label: string }) => ( <div className="flex-1 flex flex-col gap-2"> <span className="text-xs font-medium text-gray-400 uppercase tracking-wider">{label}</span> <div className="flex flex-wrap gap-1"> {contributors.slice(0, 4).map((contributor) => ( <Image key={contributor.login} alt={contributor.name || contributor.login} className="rounded-full border-1 border-gray-200 dark:border-gray-700" height={24} src={`${contributor.avatarUrl}${contributor.avatarUrl.includes('?') ? '&' : '?'}s=60`} title={contributor.name || contributor.login} width={24} /> ))} {contributors.length > 4 && ( <span className="text-xs font-medium text-gray-400 self-center"> +{contributors.length - 4} </span> )} </div> </div> )
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/components/ModuleCard.tsx
🧰 Additional context used
🧠 Learnings (5)
📚 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/components/ModuleCard.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/src/components/ModuleCard.tsx
📚 Learning: 2025-09-17T02:42:41.928Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 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
📚 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/components/ModuleCard.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/components/ModuleCard.tsx
🔇 Additional comments (5)
frontend/src/components/ModuleCard.tsx (5)
39-39: LGTM!Grid layout change from 4 to 3 columns on large screens matches the PR objectives for improved spacing and readability.
70-77: LGTM!Good defensive coding with fallback to empty arrays and proper filtering of contributors without avatar URLs, as specified in the PR objectives.
80-86: LGTM!The flexible height styling (
h-auto min-h-[12rem]) accommodates variable content from the new contributor sections, and the link href pattern is consistent with the key prop usage on line 41.
87-87: LGTM!Using
toUpperCase()is consistent with the uppercase label styling used elsewhere in this component (e.g., lines 99, 122). Based on learnings,experienceLevelis a required field in the Module type.
100-117: The URL construction concern does not apply to this code.The
avatarUrlvalues are restricted tohttps://avatars.githubusercontent.comby theremotePatternsconfiguration innext.config.js. GitHub avatar URLs always include query parameters (e.g.,?v=4), so appending&s=60produces a valid URL likehttps://avatars.githubusercontent.com/u/123?v=4&s=60. The hypothetical case of a URL lacking query parameters cannot occur given the domain whitelist.The Image domain configuration is correctly set up in
next.config.js.Likely an incorrect or invalid review comment.
|
@kasya, please review it and let me know if any changes are required. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @frontend/src/components/ModuleCard.tsx:
- Around line 104-114: The avatar URL concatenation in ModuleCard.tsx
incorrectly appends "&s=60" without checking for existing query params; update
the Image src construction for mentorsWithAvatars (and the corresponding mentees
block at the other occurrence) to append the size parameter correctly by
detecting whether contributor.avatarUrl contains a "?" and using "?" or "&"
accordingly (or use URL/URLSearchParams to set s=60), preserving existing query
params and keeping key/title/alt usage unchanged.
🧹 Nitpick comments (1)
frontend/src/components/ModuleCard.tsx (1)
123-149: Consider extracting avatar group rendering to reduce duplication.The mentor and mentee avatar sections have nearly identical rendering logic. While acceptable for this PR scope, consider extracting a reusable component for avatar groups if this pattern is used elsewhere.
♻️ Optional: Extract AvatarGroup component
const AvatarGroup = ({ contributors, label }: { contributors: Array<{ login: string; name?: string; avatarUrl: string }> label: string }) => ( <div className="flex flex-1 flex-col gap-2"> <span className="text-xs font-medium tracking-wider text-gray-400 uppercase"> {label} </span> <div className="flex flex-wrap gap-1"> {contributors.slice(0, 4).map((contributor) => ( <Image key={contributor.login} alt={contributor.name || contributor.login} className="rounded-full border-1 border-gray-200 dark:border-gray-700" height={24} src={`${contributor.avatarUrl}${contributor.avatarUrl.includes('?') ? '&' : '?'}s=60`} title={contributor.name || contributor.login} width={24} /> ))} {contributors.length > 4 && ( <span className="self-center text-xs font-medium text-gray-400"> +{contributors.length - 4} </span> )} </div> </div> )
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/components/ModuleCard.tsx
🧰 Additional context used
🧠 Learnings (4)
📚 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/components/ModuleCard.tsx
📚 Learning: 2025-09-17T02:42:41.928Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 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
📚 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/components/ModuleCard.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/components/ModuleCard.tsx
🧬 Code graph analysis (1)
frontend/src/components/ModuleCard.tsx (3)
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-21)
🔇 Additional comments (4)
frontend/src/components/ModuleCard.tsx (4)
1-2: LGTM!The import changes are appropriate:
capitalizecorrectly transforms experience levels (e.g., "BEGINNER" → "Beginner") for better readabilityImagefromnext/imageis the correct approach for avatar rendering in Next.js, providing automatic optimization
40-40: LGTM!Grid layout change from 4 to 3 columns on large screens matches the PR objectives for improved spacing and readability.
72-78: LGTM!The contributor filtering logic is well-implemented:
- Safe defaults prevent null/undefined errors
- Filtering by
avatarUrlcorrectly excludes contributors without valid avatar URLs (including empty strings)hasContributorsboolean efficiently guards the rendering block
81-88: Both module identifiers are guaranteed to be present.The
module.key || module.idfallback is safe since bothkeyandidare required fields in theModuletype. The flexible height (h-auto min-h-[12rem]) correctly accommodates variable content like the contributors section.
…tVerma109/Nest into pr/HarshitVerma109/3296
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @frontend/src/components/ModuleCard.tsx:
- Around line 93-98: Remove the fallback to module.id in the Link href: update
the Link that builds the URL (the component using pathname and
module.key/module.id) to use module.key only (e.g.,
`${pathname}/modules/${module.key}`) since the route expects moduleKey and
Module guarantees key is present; remove the fallback to module.id to avoid
generating invalid URLs and keep the TruncatedText usage for display unchanged.
🧹 Nitpick comments (1)
frontend/src/components/ModuleCard.tsx (1)
75-76: Consider using a type guard for better type safety.The filter ensures
avatarUrlexists at runtime, but TypeScript may still consider it potentially undefined. A type guard or non-null assertion would provide compile-time safety.♻️ Optional: Type-safe filter using type predicate
+type ContributorWithAvatar = { login: string; name?: string; avatarUrl: string } + +const hasAvatar = (m: { avatarUrl?: string } | null | undefined): m is ContributorWithAvatar => + Boolean(m?.avatarUrl) + -const mentorsWithAvatars = mentors.filter((m) => m?.avatarUrl) -const menteesWithAvatars = mentees.filter((m) => m?.avatarUrl) +const mentorsWithAvatars = mentors.filter(hasAvatar) +const menteesWithAvatars = mentees.filter(hasAvatar)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/components/ModuleCard.tsx
🧰 Additional context used
🧠 Learnings (4)
📚 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/components/ModuleCard.tsx
📚 Learning: 2025-09-17T02:42:41.928Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 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
📚 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/components/ModuleCard.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/components/ModuleCard.tsx
🔇 Additional comments (5)
frontend/src/components/ModuleCard.tsx (5)
1-2: LGTM!Good choice switching to
capitalize- it properly converts experience levels like "BEGINNER" to "Beginner" for better readability, whereasupperFirstwould leave them as all-caps. TheImageimport fromnext/imageis appropriate for the avatar rendering.
40-40: LGTM!Grid layout correctly changed to 3 columns on large screens as required. Minor note:
md:grid-cols-3 lg:grid-cols-3is now redundant since both use 3 columns, but this doesn't cause any issues.
71-89: Solid defensive coding for contributor handling.The logic correctly:
- Defaults to empty arrays for undefined mentors/mentees
- Filters out contributors without avatar URLs as required
- Handles malformed URLs gracefully in
getAvatarUrlWithSizeThe helper could be extracted to a shared utility if avatar sizing is needed elsewhere, but it's fine scoped here for now.
92-92: LGTM!Using
h-auto min-h-[12rem]instead of a fixed height correctly accommodates the variable content when contributor avatars are displayed while maintaining a consistent minimum size.
116-124: The Image component configuration for avatars is already properly handled. Theavatars.githubusercontent.comdomain is whitelisted in./frontend/next.config.ts(line 14), and thegetAvatarUrlWithSizefunction correctly modifies the avatar URL while preserving the domain. No action needed.
kasya
left a comment
kasya
left a comment
There was a problem hiding this comment.
@HarshitVerma109 This is great! One small request ⬇️
| <Image | ||
| key={contributor.login} | ||
| alt={contributor.name || contributor.login} | ||
| className="rounded-full border-1 border-gray-200 dark:border-gray-700" | ||
| height={24} | ||
| src={getAvatarUrlWithSize(contributor.avatarUrl)} | ||
| title={contributor.name || contributor.login} | ||
| width={24} | ||
| /> | ||
| ))} |
There was a problem hiding this comment.
@HarshitVerma109 can we add a link to a members page here on clicking the avatar?
There was a problem hiding this comment.
Hi @kasya, Should I add the members page link to both mentor and mentee avatars, or only to mentors?
| <Image | ||
| key={contributor.login} | ||
| alt={contributor.name || contributor.login} | ||
| className="rounded-full border-1 border-gray-200 dark:border-gray-700" | ||
| height={24} | ||
| src={getAvatarUrlWithSize(contributor.avatarUrl)} | ||
| title={contributor.name || contributor.login} | ||
| width={24} | ||
| /> |
There was a problem hiding this comment.
And here can we add a link to members/ page when we are on a public view /mentorship/... and to mentees/ page when we are on the private admin view /my/mentorship/.... ?
There was a problem hiding this comment.
Oh, you mentioned the mentee part here, sorry for the above question
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @frontend/src/components/ModuleCard.tsx:
- Around line 79-81: programKey can be empty causing malformed URLs; update the
code around isAdminView, programKey and moduleKey (and any call sites like
getMenteeUrl) to guard against an empty programKey by either deriving a safe
fallback or avoiding building the mentee URL when programKey is falsy: e.g.,
check programKey truthiness before concatenating into
`/my/mentorship/programs/${programKey}/modules/${moduleKey}` and if empty,
return a sensible alternative (skip the link, use a parent pathname segment, or
return null) so you never produce `//` in the path.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/components/ModuleCard.tsx
🧰 Additional context used
🧠 Learnings (4)
📚 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/components/ModuleCard.tsx
📚 Learning: 2025-09-17T02:42:41.928Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 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
📚 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/components/ModuleCard.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/components/ModuleCard.tsx
🔇 Additional comments (6)
frontend/src/components/ModuleCard.tsx (6)
40-40: LGTM!Grid layout correctly changed to 3 columns on large screens as per the PR objectives.
90-99: LGTM!Good defensive implementation with proper fallback for URLs that can't be parsed by the URL constructor. The 60px size for 24px display provides adequate quality for high-DPI screens.
102-102: LGTM!Using
h-auto min-h-[12rem]correctly allows cards to expand for the new contributors section while maintaining consistent minimum sizing.
186-193: LGTM!Admin labels section correctly renders only when the user is an admin and labels exist. The defensive check
module.labels && module.labels.length > 0prevents errors from undefined/empty arrays.
132-139: No action needed. The Next.js configuration infrontend/next.config.tsalready includesavatars.githubusercontent.comin theremotePatterns, which properly authorizes external GitHub avatar URLs for the Image component. The images are correctly configured and will be optimized by Next.js without requiring theunoptimizedprop.
1-2: The import change fromupperFirsttocapitalizeis correct and intentional.
capitalizeis the appropriate choice here as ExperienceLevelEnum values are stored in uppercase ('ADVANCED','BEGINNER','EXPERT','INTERMEDIATE'), and the code at line 109 correctly usescapitalize(module.experienceLevel)to format them for display (e.g.,'ADVANCED'→'Advanced'). This pattern is already established elsewhere in the codebase for the same purpose.
|
@kasya, please review it and let me know if any changes are required. |
kasya
left a comment
kasya
left a comment
There was a problem hiding this comment.
@HarshitVerma109 this works great! 👍🏼
Left one small request and also realized that we discussed another update that should be included in this PR. ⬇️
|
Also, please run |
|
Sorry |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@frontend/src/components/SingleModuleCard.tsx`:
- Around line 92-109: The admin-only branch renders MenteeContributorsList even
when programKey is empty, producing malformed links; change the conditional so
that the MenteeContributorsList is used only when
pathname?.startsWith('/my/mentorship') && programKey (non-empty) is truthy, and
fall back to TopContributorsList when programKey is falsy; update the ternary
around MenteeContributorsList/TopContributorsList (which currently checks
pathname) to include the programKey guard and keep props (module.mentees,
maxInitialDisplay, label) consistent.
♻️ Duplicate comments (2)
frontend/src/components/ModuleCard.tsx (2)
78-86: Avoid building admin mentee URLs whenprogramKeyis empty; dropmodule.idfallback.
programKeycan be empty if the path doesn’t contain/programs/, which yields malformed URLs. Also, the route expectsmoduleKey, so falling back tomodule.idcan generate invalid routes.🛠️ Suggested fix
- const moduleKey = module.key || module.id + const moduleKey = module.key ... - const getMenteeUrl = (login: string) => { - if (pathname?.startsWith('/my/mentorship')) { + const getMenteeUrl = (login: string) => { + if (pathname?.startsWith('/my/mentorship') && programKey) { return `/my/mentorship/programs/${programKey}/modules/${moduleKey}/mentees/${login}` } return `/members/${login}` }
100-164: Replace non-existent Tailwind utilities (border-1,border-l-1,shadow-xs).
These classes won’t generate CSS unless custom-extended. Prefer standard Tailwind utilities.🛠️ Proposed class updates
- <div className="flex h-auto min-h-[12rem] w-full flex-col gap-3 rounded-lg border-1 border-gray-200 p-4 shadow-xs ease-in-out hover:shadow-md dark:border-gray-700 dark:bg-gray-800"> + <div className="flex h-auto min-h-[12rem] w-full flex-col gap-3 rounded-lg border border-gray-200 p-4 shadow-sm ease-in-out hover:shadow-md dark:border-gray-700 dark:bg-gray-800"> ... - className="rounded-full border-1 border-gray-200 dark:border-gray-700" + className="rounded-full border border-gray-200 dark:border-gray-700" ... - className={`flex flex-1 flex-col gap-2 ${mentorsWithAvatars.length > 0 ? 'border-l-1 border-gray-100 pl-4 dark:border-gray-700' : ''}`} + className={`flex flex-1 flex-col gap-2 ${mentorsWithAvatars.length > 0 ? 'border-l border-gray-100 pl-4 dark:border-gray-700' : ''}`} ... - className="rounded-full border-1 border-gray-200 dark:border-gray-700" + className="rounded-full border border-gray-200 dark:border-gray-700"
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/components/ModuleCard.tsxfrontend/src/components/SingleModuleCard.tsx
🧰 Additional context used
🧠 Learnings (5)
📚 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/components/ModuleCard.tsx
📚 Learning: 2025-09-17T02:42:41.928Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 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
📚 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/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/components/ModuleCard.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/components/ModuleCard.tsx
🔇 Additional comments (5)
frontend/src/components/SingleModuleCard.tsx (1)
14-14: LGTM: mentee contributors import.frontend/src/components/ModuleCard.tsx (4)
1-2: LGTM: updated imports forcapitalizeandImage.
40-40: LGTM: grid shift to 3 columns on large screens.
88-97: LGTM: robust avatar size helper with URL fallback.
115-176: No documented "overlapping compact style" requirement exists for avatars.The current
flex flex-wrap gap-1pattern in ModuleCard.tsx is consistent with the codebase. A search for-space-x(negative spacing) returned no matches—overlapping avatars are not used elsewhere in the project. No design documentation, specification, or requirements file mentions this as a requirement. The suggested change to-space-x-2is a stylistic preference, not a requirement match.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| {module.mentees?.length > 0 && | ||
| (pathname?.startsWith('/my/mentorship') ? ( | ||
| <MenteeContributorsList | ||
| icon={HiUserGroup} | ||
| contributors={module.mentees} | ||
| maxInitialDisplay={6} | ||
| label="Mentees" | ||
| programKey={programKey} | ||
| moduleKey={module.key} | ||
| /> | ||
| ) : ( | ||
| <TopContributorsList | ||
| icon={HiUserGroup} | ||
| contributors={module.mentees} | ||
| maxInitialDisplay={6} | ||
| label="Mentees" | ||
| /> | ||
| ))} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the SingleModuleCard.tsx file structure
cd frontend/src/components && head -120 SingleModuleCard.tsx | tail -50Repository: OWASP/Nest
Length of output: 1395
🏁 Script executed:
# Also get the full context of the component to understand prop types
cat -n frontend/src/components/SingleModuleCard.tsx | head -30Repository: OWASP/Nest
Length of output: 1238
🏁 Script executed:
# Find where SingleModuleCard is used
rg "SingleModuleCard" frontend/src --type tsx -A 2Repository: OWASP/Nest
Length of output: 81
🏁 Script executed:
# Check routes that contain '/my/mentorship'
find frontend/src -type f \( -name "*.tsx" -o -name "*.ts" \) -exec grep -l "/my/mentorship" {} \;Repository: OWASP/Nest
Length of output: 804
🏁 Script executed:
# Check complete SingleModuleCard implementation to see where programKey is used
cat -n frontend/src/components/SingleModuleCard.tsxRepository: OWASP/Nest
Length of output: 4605
🏁 Script executed:
# Look for how SingleModuleCard is used in pages
rg "SingleModuleCard" frontend/src/app -B 5 -A 5 --type tsRepository: OWASP/Nest
Length of output: 36
🏁 Script executed:
# Check MenteeContributorsList implementation
cat -n frontend/src/components/MenteeContributorsList.tsxRepository: OWASP/Nest
Length of output: 3163
Guard admin mentee rendering when programKey is missing.
When accessing /my/mentorship without a selected program, programKey becomes an empty string, causing MenteeContributorsList to generate malformed links like /my/mentorship/programs//modules/.... Add a guard to fall back to the public list instead.
🛠️ Suggested fix
- {module.mentees?.length > 0 &&
- (pathname?.startsWith('/my/mentorship') ? (
+ {module.mentees?.length > 0 &&
+ (pathname?.startsWith('/my/mentorship') && programKey ? (
<MenteeContributorsList
icon={HiUserGroup}
contributors={module.mentees}
maxInitialDisplay={6}
label="Mentees"
programKey={programKey}
moduleKey={module.key}
/>
) : (
<TopContributorsList
icon={HiUserGroup}
contributors={module.mentees}
maxInitialDisplay={6}
label="Mentees"
/>
))}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {module.mentees?.length > 0 && | |
| (pathname?.startsWith('/my/mentorship') ? ( | |
| <MenteeContributorsList | |
| icon={HiUserGroup} | |
| contributors={module.mentees} | |
| maxInitialDisplay={6} | |
| label="Mentees" | |
| programKey={programKey} | |
| moduleKey={module.key} | |
| /> | |
| ) : ( | |
| <TopContributorsList | |
| icon={HiUserGroup} | |
| contributors={module.mentees} | |
| maxInitialDisplay={6} | |
| label="Mentees" | |
| /> | |
| ))} | |
| {module.mentees?.length > 0 && | |
| (pathname?.startsWith('/my/mentorship') && programKey ? ( | |
| <MenteeContributorsList | |
| icon={HiUserGroup} | |
| contributors={module.mentees} | |
| maxInitialDisplay={6} | |
| label="Mentees" | |
| programKey={programKey} | |
| moduleKey={module.key} | |
| /> | |
| ) : ( | |
| <TopContributorsList | |
| icon={HiUserGroup} | |
| contributors={module.mentees} | |
| maxInitialDisplay={6} | |
| label="Mentees" | |
| /> | |
| ))} |
🤖 Prompt for AI Agents
In `@frontend/src/components/SingleModuleCard.tsx` around lines 92 - 109, The
admin-only branch renders MenteeContributorsList even when programKey is empty,
producing malformed links; change the conditional so that the
MenteeContributorsList is used only when pathname?.startsWith('/my/mentorship')
&& programKey (non-empty) is truthy, and fall back to TopContributorsList when
programKey is falsy; update the ternary around
MenteeContributorsList/TopContributorsList (which currently checks pathname) to
include the programKey guard and keep props (module.mentees, maxInitialDisplay,
label) consistent.
|
kasya
left a comment
kasya
left a comment
There was a problem hiding this comment.
@HarshitVerma109 thank you for working on this! This looks great 👍🏼
I updated module card styling a little bit (bold text, text color and decided to hide labels).
2 participants
Proposed change
Resolves #3156
Improved Module Card to show mentors/mentee avatars and changed grid layout to 3 columns.
Changes:
Checklist
make check-testlocally: all warnings addressed, tests passed