-
-
Notifications
You must be signed in to change notification settings - Fork 217
UI/ux mentorship program update #2244
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
UI/ux mentorship program update #2244
Conversation
Summary by CodeRabbit
WalkthroughReplaces lodash upperFirst with utils/capitalize.titleCaseWord for status and experience labels; adds tooltips and fixed title height with clamped descriptions on ProgramCard; updates ActionButton and several button styles to #1D7BD7; makes module dropdown admin-only and removes "View Module"; adjusts grid gaps/breakpoints; updates tests and mocks. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (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)
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: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
frontend/src/components/ItemCardList.tsx (1)
74-82
: Prevent reverse tabnabbing and expose full title on hover.External links opened in a new tab should set rel to noopener noreferrer. Adding a title also surfaces the full text for truncated headings.
Apply this diff:
- <Link - className="text-blue-400 hover:underline" - href={item?.url || ''} - target="_blank" - > + <Link + className="text-blue-400 hover:underline" + href={item?.url || ''} + title={item?.title || item?.name} + target="_blank" + rel="noopener noreferrer" + >frontend/src/components/Footer.tsx (1)
56-63
: Add rel="noopener noreferrer" to external links opened in a new tab.Prevents reverse tabnabbing and improves security/SEO.
- <Link - target="_blank" + <Link + target="_blank" + rel="noopener noreferrer" className="text-slate-600 hover:text-slate-900 dark:text-slate-400 dark:hover:text-slate-100" href={link.href || '/'} >frontend/src/components/Modal.tsx (1)
21-29
: aria-labelledby points to non-existent idaria-labelledby="modal-title" has no matching element id in the modal content. Screen readers will miss the title.
Apply:
- <ModalHeader className="mb-1 flex-col ..."> + <ModalHeader id="modal-title" className="mb-1 flex-col ...">frontend/src/components/MetricsScoreCircle.tsx (1)
1-68
: Replace invalid Tailwind utility classes across the repo
- Swap
bg-linear-to-br
forbg-gradient-to-br
- Change
backdrop-blur-xs
to a valid size (e.g.backdrop-blur-sm
)- Replace
focus-visible:outline-hidden
withfocus-visible:outline-none
(and update affected tests)- Use
whitespace-nowrap
instead oftext-nowrap
andwhitespace-normal
(orbreak-words
) instead oftext-wrap
Re-runrg -nP 'bg-linear-to-br|backdrop-blur-xs|focus-visible:outline-hidden|\btext-wrap\b|\btext-nowrap\b'
to confirm no invalid classes remain.frontend/src/components/Search.tsx (1)
69-95
: Fix inverted isLoaded check (UI shows when not loaded; skeleton shows when loaded).Swap the ternary so the search UI renders when loaded.
- {!isLoaded ? ( + {isLoaded ? ( <> <FontAwesomeIcon icon={faSearch} className="pointer-events-none absolute left-3 top-1/2 h-4 w-4 -translate-y-1/2 text-gray-400" /> <input ref={inputRef} type="text" value={searchQuery} onChange={handleSearchChange} placeholder={placeholder} className="focus:outline-hidden h-12 w-full rounded-lg border-1 border-gray-300 pl-10 pr-10 text-lg text-black focus:border-blue-500 focus:ring-2 focus:ring-blue-500 dark:border-gray-600 dark:bg-gray-800 dark:text-white dark:focus:border-blue-300 dark:focus:ring-blue-300" /> {searchQuery && ( <button - className="focus:outline-hidden absolute right-2 top-1/2 -translate-y-1/2 rounded-full p-1 hover:bg-gray-100 focus:ring-2 focus:ring-gray-300" + className="focus:outline-hidden absolute right-2 top-1/2 -translate-y-1/2 rounded-full p-1 hover:bg-gray-100 focus:ring-2 focus:ring-gray-300" onClick={handleClearSearch} + aria-label="Clear search" > <FontAwesomeIcon icon={faTimes} /> </button> )} </> ) : ( <Skeleton className="h-12 rounded-lg" /> )}frontend/src/components/SingleModuleCard.tsx (1)
1-1
: Add 'use client' directive to SingleModuleCard.tsx
This component uses React hooks and thewindow
API and must be a Client Component; insert'use client';
at the top offrontend/src/components/SingleModuleCard.tsx
.frontend/src/components/ProgramCard.tsx (1)
109-112
: Guard optional onEdit to avoid runtime error
onEdit
is optional in props; clicking “Edit” will throw if it’s undefined. Guard it and optionally disable the button when absent.- <ActionButton onClick={() => onEdit(program.key)}> + <ActionButton onClick={() => onEdit?.(program.key)} disabled={!onEdit}> <FontAwesomeIcon icon={faEdit} className="mr-1" /> Edit </ActionButton>
🧹 Nitpick comments (21)
frontend/src/app/globals.css (1)
335-345
: Improve dropdown accessibility and interaction (keyboard + pointer events).Currently visibility is toggled only on hover; keyboard users can’t open it, and invisible menus can still intercept clicks. Add focus-within support, disable pointer events when hidden, and ensure stacking with z-index.
Apply this diff:
.dropdown-menu { - @apply absolute left-0 top-full mt-2 w-48 rounded-lg bg-white p-3 shadow-lg dark:bg-gray-800; - @apply invisible opacity-0 transition-all duration-200 ease-in-out; + @apply absolute left-0 top-full z-20 mt-2 w-48 rounded-lg bg-white p-3 shadow-lg dark:bg-gray-800; + @apply invisible opacity-0 pointer-events-none transition-all duration-200 ease-in-out; @apply flex flex-col gap-2; /* Stack items vertically */ } -.dropdown:hover .dropdown-menu { - @apply visible opacity-100; +.dropdown:hover .dropdown-menu, +.dropdown:focus-within .dropdown-menu { + @apply visible opacity-100 pointer-events-auto; }frontend/src/components/InfoBlock.tsx (1)
30-30
: Silence decorative icon for screen readers.Prevent duplicate announcements by marking the icon as decorative.
Apply this diff:
- <FontAwesomeIcon icon={icon} className="mr-3 mt-1 w-5" /> + <FontAwesomeIcon icon={icon} className="mr-3 mt-1 w-5" aria-hidden="true" focusable="false" />frontend/src/components/SortBy.tsx (1)
61-61
: Make sort toggle accessible and safe in forms.Add button type, ARIA pressed state, and an explicit label/title.
Apply this diff:
- <button - onClick={() => onOrderChange(selectedOrder === 'asc' ? 'desc' : 'asc')} - className="shadow-xs inline-flex h-9 w-9 items-center justify-center rounded-lg bg-gray-200 p-0 hover:bg-gray-300 dark:bg-[#323232] dark:text-gray-300 dark:hover:bg-[#454545]" - > + <button + type="button" + aria-pressed={selectedOrder === 'desc'} + aria-label={selectedOrder === 'asc' ? 'Set descending order' : 'Set ascending order'} + title={selectedOrder === 'asc' ? 'Set descending order' : 'Set ascending order'} + onClick={() => onOrderChange(selectedOrder === 'asc' ? 'desc' : 'asc')} + className="shadow-xs inline-flex h-9 w-9 items-center justify-center rounded-lg bg-gray-200 p-0 hover:bg-gray-300 dark:bg-[#323232] dark:text-gray-300 dark:hover:bg-[#454545]" + >frontend/src/components/Footer.tsx (1)
51-54
: Prefer stable keys over array index.Reduces unnecessary re-renders on list mutations.
- {section.links.map((link, index) => ( - <div key={index} className="py-1"> + {section.links.map((link) => ( + <div key={link.href ?? link.text} className="py-1">frontend/src/components/Card.tsx (1)
70-79
: Micro-optimization: avoid recomputing filtered icons inside the loop.Compute once before map to reduce repeated work.
- <div className="mt-3 flex flex-wrap"> - {Object.keys(ICONS).map((key, index) => - icons[key] ? ( - <DisplayIcon - key={`${key}-${index}`} - item={key} - icons={Object.fromEntries(Object.entries(icons).filter(([_, value]) => value))} - /> - ) : null - )} - </div> + <div className="mt-3 flex flex-wrap"> + {(() => { + const filtered = Object.fromEntries(Object.entries(icons).filter(([, v]) => v)) + return Object.keys(ICONS).map((key, index) => + icons[key] ? <DisplayIcon key={`${key}-${index}`} item={key} icons={filtered} /> : null + ) + })()} + </div>frontend/src/components/skeletons/ApiKeySkelton.tsx (1)
26-26
: Define customshadow-xs
or useshadow-sm
Noshadow-xs
utility is defined in your Tailwind config, so this class is a no-op. Either add aboxShadow: { 'xs': … }
entry undertheme.extend
or replace it with a built-in shadow (e.g.shadow-sm
):- <div className="shadow-xs mb-6 rounded-lg border-1 border-gray-200 bg-white p-6 dark:border-gray-700 dark:bg-gray-800"> + <div className="shadow-sm mb-6 rounded-lg border-1 border-gray-200 bg-white p-6 dark:border-gray-700 dark:bg-gray-800">frontend/src/components/ProgramForm.tsx (1)
79-79
: Replace invalidfocus:outline-hidden
and add visible focus rings
focus:outline-hidden
isn’t defined in Tailwind (and isn’t in your config), so it’s ignored. Swap it forfocus:outline-none
plus ring utilities to keep focus visible. For example:- className="focus:outline-hidden w-full rounded-lg border-2 bg-gray-50 px-4 py-3 text-gray-800 dark:bg-gray-800 dark:text-gray-200" + className="focus:outline-none focus:ring-2 focus:ring-offset-2 focus:ring-[#0D6EFD] dark:focus:ring-slate-400 w-full rounded-lg border-2 bg-gray-50 px-4 py-3 text-gray-800 dark:bg-gray-800 dark:text-gray-200"Apply this update to all input/textarea elements at lines 79, 95, 117, 131, 146, 169, 183, and 201.
frontend/src/components/ActionButton.tsx (2)
14-16
: Hard-coded brand color may fail contrast in dark modeSwitching to text-[#1D7BD7] is brand-accurate, but ensure WCAG contrast on dark surfaces. Prefer theme token (e.g., text-primary) or CSS var.
Minimal change if tokens exist:
- '... border-[#1D7BD7] ... text-[#1D7BD7] hover:bg-[#1D7BD7] ...' + '... border-primary ... text-primary hover:bg-primary ...'If not, define a CSS var and use text-[var(--brand-primary)] consistently.
17-31
: Remove leftover data-tooltip- attrs*Using @heroui/tooltip makes data-tooltip-id/content redundant (from react-tooltip). Keep one tooltip system to avoid confusion.
- data-tooltip-id="button-tooltip" - data-tooltip-content={tooltipLabel}frontend/src/components/ModuleCard.tsx (2)
72-74
: Avoid window.location in app routerUsing window.location.pathname can break in SSR contexts. Prefer usePathname() from next/navigation.
- const handleClick = () => { router.push(`${window.location.pathname}/modules/${details.key}`) } + import { usePathname } from 'next/navigation' + const pathname = usePathname() + const handleClick = () => { router.push(`${pathname}/modules/${details.key}`) }
98-110
: Handle negative or zero durationsIf endedAt < startedAt, weeks becomes <= 0. Clamp and/or return a clearer label.
- const weeks = Math.ceil(days / 7) - return `${weeks} week${weeks !== 1 ? 's' : ''}` + const weeks = Math.max(0, Math.ceil(days / 7)) + return weeks > 0 ? `${weeks} week${weeks !== 1 ? 's' : ''}` : 'Less than a week'frontend/src/components/MetricsScoreCircle.tsx (1)
36-48
: Keyboard handler casts KeyboardEvent to MouseEventType cast hides a mismatch. Unify activation via a single onActivate callback.
- onClick?: (e: MouseEvent<HTMLDivElement>) => void + onActivate?: () => voidThen call onActivate() from both onClick and onKeyDown handlers.
frontend/src/components/UserMenu.tsx (2)
68-75
: Restore visible focus and label the avatar button for a11y.Currently removes focus outline without a replacement and lacks a descriptive label. Prefer focus-visible rings and add an aria-label.
- <button + <button onClick={() => setIsOpen((prev) => !prev)} aria-expanded={isOpen} aria-haspopup="true" aria-controls={dropdownId} - className="focus:outline-hidden w-auto" + aria-label="User menu" + className="focus:outline-none focus-visible:ring-2 focus-visible:ring-blue-500 focus-visible:ring-offset-2 focus-visible:ring-offset-white dark:focus-visible:ring-offset-slate-900 w-auto" disabled={isLoggingOut} >
26-34
: Add Escape-to-close for the dropdown.Improves keyboard UX and matches our accessibility learnings.
useEffect(() => { - const handleClickOutside = (event: MouseEvent) => { + const handleClickOutside = (event: MouseEvent) => { if (dropdownRef.current && !dropdownRef.current.contains(event.target as Node)) { setIsOpen(false) } } + const handleKeyDown = (event: KeyboardEvent) => { + if (event.key === 'Escape') setIsOpen(false) + } document.addEventListener('mousedown', handleClickOutside) - return () => document.removeEventListener('mousedown', handleClickOutside) + document.addEventListener('keydown', handleKeyDown) + return () => { + document.removeEventListener('mousedown', handleClickOutside) + document.removeEventListener('keydown', handleKeyDown) + } }, [])frontend/src/components/ScrollToTop.tsx (1)
38-38
: Nit: trailing space and non-standard duration.
- Remove trailing space at the end of the class string.
- Verify duration-400 exists in Tailwind config; default scales don’t include 400.
frontend/src/components/SingleModuleCard.tsx (1)
49-53
: Use titleCaseWord for experience level to normalize UPPERCASE API values.Aligns with program pages and avoids rendering BEGINNER/ADVANCED in all caps.
-import upperFirst from 'lodash/upperFirst' +import { titleCaseWord } from 'utils/capitalize' ... - { label: 'Experience Level', value: upperFirst(module.experienceLevel) }, + { label: 'Experience Level', value: titleCaseWord(module.experienceLevel) },frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (1)
83-89
: Use titleCaseWord in the success toast for consistency.upperFirst('OPEN') => 'OPEN'. titleCaseWord will render 'Open'.
- addToast({ - title: `Program status updated to ${upperFirst(newStatus)}`, + addToast({ + title: `Program status updated to ${titleCaseWord(newStatus)}`, description: 'The status has been successfully updated.',If upperFirst is unused after this, drop its import.
-import upperFirst from 'lodash/upperFirst'
frontend/src/components/ModuleForm.tsx (1)
282-285
: Avoid throwing inside debounced async; use cause or log instead.
new Error('msg', err)
ignores the second arg; throwing here creates unhandled rejections. Prefer logging and graceful fallback.- } catch (err) { + } catch (err) { setRawResults([]) setShowSuggestions(false) - throw new Error('Error fetching suggestions:', err) + console.error('Error fetching project suggestions', err) + // Optionally surface UI feedback here if desired. }frontend/src/components/ProgramCard.tsx (3)
18-35
: Prefer matchMedia/useMediaQuery over resize listenerUsing
window.matchMedia('(min-width: …)')
(or a shareduseBreakpoint
hook) aligns with Tailwind breakpoints and avoids continuous resize work.- useEffect(() => { - const updateScreenSize = () => { - const width = window.innerWidth - if (width < 768) { - setScreenSize('sm') - } else if (width < 1024) { - setScreenSize('md') - } else { - setScreenSize('lg') - } - } - updateScreenSize() - window.addEventListener('resize', updateScreenSize) - return () => window.removeEventListener('resize', updateScreenSize) - }, []) + useEffect(() => { + const mqMd = window.matchMedia('(min-width: 768px)') + const mqLg = window.matchMedia('(min-width: 1024px)') + const update = () => setScreenSize(mqLg.matches ? 'lg' : mqMd.matches ? 'md' : 'sm') + update() + mqMd.addEventListener('change', update) + mqLg.addEventListener('change', update) + return () => { + mqMd.removeEventListener('change', update) + mqLg.removeEventListener('change', update) + } + }, [])
52-57
: Heuristic length thresholds may misclassify overflowString length ≠ rendered overflow (fonts, widths, locale). Consider measuring overflow via CSS (line-clamp with tooltip only on
:has(+ .line-clamp-2[aria-hidden])
) or compute based on container width. If keeping thresholds, centralize them to a constant to avoid drift.
37-43
: Unify date formattingThere’s a shared
utils/dateFormatter.ts
. Using it here keeps formats consistent and adds invalid-date protection.- const formatDate = (d: string) => - new Date(d).toLocaleDateString('en-US', { - month: 'short', - day: 'numeric', - year: 'numeric', - }) + // reuse: import { formatDate } from 'utils/dateFormatter'
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (43)
frontend/src/app/about/page.tsx
(1 hunks)frontend/src/app/global-error.tsx
(1 hunks)frontend/src/app/globals.css
(1 hunks)frontend/src/app/mentorship/programs/[programKey]/page.tsx
(2 hunks)frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
(2 hunks)frontend/src/app/page.tsx
(3 hunks)frontend/src/app/snapshots/[id]/page.tsx
(1 hunks)frontend/src/app/snapshots/page.tsx
(1 hunks)frontend/src/components/ActionButton.tsx
(1 hunks)frontend/src/components/Card.tsx
(2 hunks)frontend/src/components/CardDetailsPage.tsx
(1 hunks)frontend/src/components/Footer.tsx
(1 hunks)frontend/src/components/GeneralCompliantComponent.tsx
(1 hunks)frontend/src/components/Header.tsx
(3 hunks)frontend/src/components/InfoBlock.tsx
(1 hunks)frontend/src/components/ItemCardList.tsx
(1 hunks)frontend/src/components/LoginPageContent.tsx
(1 hunks)frontend/src/components/LogoCarousel.tsx
(3 hunks)frontend/src/components/MetricsCard.tsx
(1 hunks)frontend/src/components/MetricsScoreCircle.tsx
(1 hunks)frontend/src/components/Modal.tsx
(2 hunks)frontend/src/components/ModeToggle.tsx
(1 hunks)frontend/src/components/ModuleCard.tsx
(1 hunks)frontend/src/components/ModuleForm.tsx
(10 hunks)frontend/src/components/MultiSearch.tsx
(2 hunks)frontend/src/components/NavButton.tsx
(1 hunks)frontend/src/components/NavDropDown.tsx
(1 hunks)frontend/src/components/ProgramCard.tsx
(4 hunks)frontend/src/components/ProgramForm.tsx
(9 hunks)frontend/src/components/Release.tsx
(1 hunks)frontend/src/components/RepositoriesCard.tsx
(1 hunks)frontend/src/components/ScrollToTop.tsx
(1 hunks)frontend/src/components/Search.tsx
(1 hunks)frontend/src/components/SearchPageLayout.tsx
(1 hunks)frontend/src/components/SingleModuleCard.tsx
(1 hunks)frontend/src/components/SnapshotCard.tsx
(1 hunks)frontend/src/components/SortBy.tsx
(1 hunks)frontend/src/components/TopContributorsList.tsx
(1 hunks)frontend/src/components/TruncatedText.tsx
(1 hunks)frontend/src/components/UserCard.tsx
(1 hunks)frontend/src/components/UserMenu.tsx
(2 hunks)frontend/src/components/skeletons/ApiKeySkelton.tsx
(2 hunks)frontend/src/components/skeletons/Card.tsx
(2 hunks)
🧰 Additional context used
🧠 Learnings (8)
📚 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
frontend/src/app/mentorship/programs/[programKey]/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]/page.tsx
📚 Learning: 2025-04-30T13:41:20.846Z
Learnt from: codic-yeeshu
PR: OWASP/Nest#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/UserMenu.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/components/ModuleForm.tsx
frontend/src/components/ProgramForm.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/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/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/mentorship/programs/[programKey]/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/mentorship/programs/[programKey]/page.tsx
🧬 Code graph analysis (4)
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (1)
frontend/src/utils/capitalize.ts (1)
titleCaseWord
(1-4)
frontend/src/components/ProgramCard.tsx (1)
frontend/src/utils/dateFormatter.ts (1)
formatDate
(1-20)
frontend/src/components/Footer.tsx (3)
frontend/src/utils/constants.ts (1)
footerSections
(52-104)frontend/src/types/section.ts (1)
Section
(3-6)frontend/src/types/button.ts (1)
Button
(4-9)
frontend/src/app/mentorship/programs/[programKey]/page.tsx (2)
frontend/src/utils/capitalize.ts (1)
titleCaseWord
(1-4)frontend/src/utils/dateFormatter.ts (1)
formatDate
(1-20)
⏰ 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). (1)
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (37)
frontend/src/app/about/page.tsx (1)
109-109
: LGTM: class reordering is a no-op.No functional or visual regression expected.
frontend/src/components/Footer.tsx (1)
23-24
: LGTM on class reordering.Pure Tailwind token reordering; no functional change.
frontend/src/app/snapshots/page.tsx (1)
66-66
: LGTM on wrapper class reorder.No behavior change.
frontend/src/app/snapshots/[id]/page.tsx (1)
115-115
: LGTM on container class reorder.Styling tokens unchanged; no functional impact.
frontend/src/components/Card.tsx (1)
32-32
: LGTM on Tailwind token reordering.No behavior change detected.
Also applies to: 57-57
frontend/src/components/TruncatedText.tsx (1)
40-40
: LGTM on class order tweak.No change in behavior.
frontend/src/components/NavDropDown.tsx (1)
69-69
: LGTM — class reordering only.
No behavioral change.frontend/src/app/global-error.tsx (1)
43-43
: LGTM — class reordering only.
No functional impact.frontend/src/components/GeneralCompliantComponent.tsx (1)
30-30
: LGTM — positional utilities reordered.
No change in centering behavior.frontend/src/components/Release.tsx (1)
54-54
: LGTM — truncation/title styles order only.
No behavioral change.frontend/src/components/LoginPageContent.tsx (1)
65-65
: LGTM — container class order change only.
No functional impact.frontend/src/components/TopContributorsList.tsx (1)
57-57
: LGTM — Link class reorder.
No change to layout or behavior.frontend/src/components/ProgramForm.tsx (1)
54-54
: LGTM — wrapper class reorder.
No functional difference.frontend/src/components/Modal.tsx (1)
54-59
: Close button focus styles: OKOrder change preserves focus ring and offset. No functional impact.
frontend/src/components/ActionButton.tsx (1)
33-37
: Button variant: OKShared baseStyles across Link and Button reads fine; aria-label is provided when tooltipLabel exists.
frontend/src/components/ModuleCard.tsx (1)
76-83
: Class reordering onlyNo behavioral change in ModuleItem container.
frontend/src/components/MetricsScoreCircle.tsx (1)
54-61
: Class reorders: OKTypography swaps (uppercase/tracking-wide, font/leading) don’t alter rendering.
frontend/src/app/page.tsx (2)
143-147
: Hero paragraph reorder: OK
372-390
: CTA container reorder: OKfrontend/src/components/RepositoriesCard.tsx (1)
39-65
: Class reordering onlyRepositoryItem container reorder preserves styles.
frontend/src/components/SearchPageLayout.tsx (1)
43-71
: Root wrapper reorder: OKToken order change only; no behavior difference.
frontend/src/components/UserMenu.tsx (1)
88-92
: Class reorder only — no behavior change.Looks good.
frontend/src/components/Search.tsx (1)
74-74
: Class reorders are fine.No semantic changes from the class name movements.
Also applies to: 82-82, 86-86
frontend/src/components/UserCard.tsx (1)
41-45
: Class reorder only — safe.Token order changes don’t affect rendering here.
Also applies to: 44-49, 54-57, 60-63
frontend/src/components/SingleModuleCard.tsx (1)
77-77
: Class reorder only — safe.No behavioral change.
frontend/src/components/MultiSearch.tsx (1)
231-233
: Class reorders only — safe.No logic changes; a11y on clear button already present.
Also applies to: 241-242, 246-252
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (1)
15-16
: Good swap to titleCaseWord for display.This better formats enum-like values for users.
Also applies to: 132-139
frontend/src/components/ModuleForm.tsx (1)
57-58
: Class reorders only — safe.No semantic changes; consistent with PR-wide cleanup.
Also applies to: 81-82, 96-97, 118-119, 132-133, 144-145, 172-173, 186-187, 214-215, 358-358
frontend/src/app/mentorship/programs/[programKey]/page.tsx (2)
9-9
: Switch to local titleCase utility — good moveImporting and using
titleCaseWord
avoids a lodash dependency and normalizes casing consistently across pages.
73-73
: Status capitalization change looks correctDisplaying
titleCaseWord(program.status)
improves readability without changing semantics.frontend/src/components/LogoCarousel.tsx (1)
25-25
: Tailwind class reorder onlyNo functional impact; animation and links remain unchanged.
Also applies to: 56-56, 62-62, 75-75
frontend/src/components/skeletons/Card.tsx (1)
20-20
: Skeleton class order tweaks are harmlessStyles are equivalent; no rendering or accessibility changes.
Also applies to: 60-60
frontend/src/components/ProgramCard.tsx (1)
63-73
: Tooltip integration reads wellClean conditional rendering with consistent typography; good accessibility defaults from HeroUI.
Also applies to: 90-99
frontend/src/components/Header.tsx (3)
58-58
: Class reorders are benignLayout/behavior unchanged; slide-in menu logic still matches selectors used in outside-click handler.
Also applies to: 152-152, 157-157
139-139
: Confirm visible focus state for the mobile menu buttonAdding
focus:outline-hidden
can suppress the default focus indicator. Ensure an alternate visible focus style remains for keyboard users (WCAG 2.4.7). If HeroUI already applies a ring, considerfocus-visible:ring
instead of removing outline.
166-166
: Explicit mobile logo height — good
h-16
stabilizes layout and reduces CLS on slow networks.Also applies to: 174-174
frontend/src/components/NavButton.tsx (1)
24-24
: ClassName reorder; focus-visible kept — looks goodRetains ring on focus-visible and hover rings; no behavior 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.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/__tests__/unit/pages/ProgramDetailsMentorship.test.tsx (1)
60-71
: Guard against undefined experienceLevels and add a test
- In
frontend/src/app/mentorship/programs/[programKey]/page.tsx
andfrontend/src/app/my/mentorship/programs/[programKey]/page.tsx
, replacewithvalue: program.experienceLevels?.map(level => titleCaseWord(level)).join(', ') || 'N/A',value: (program.experienceLevels ?? []).map(level => titleCaseWord(level)).join(', ') || 'N/A',- In
frontend/__tests__/unit/pages/ProgramDetailsMentorship.test.tsx
, add a test withexperienceLevels: undefined
to assert graceful rendering (e.g. ‘N/A’).
♻️ Duplicate comments (1)
frontend/__tests__/unit/pages/ProgramDetails.test.tsx (1)
60-71
: Same undefined experienceLevels guard applies here.See mentorship test comment for the upstream fix and optional test addition.
🧹 Nitpick comments (6)
frontend/__tests__/unit/components/ProgramCard.test.tsx (3)
27-33
: Make Tooltip mock closer to real API (support ReactNode, mark ESM, add a11y role).Prevents type drift if content is a ReactNode and avoids interop issues; adds role for clearer queries.
-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 }: { children: React.ReactNode; content?: React.ReactNode }) => ( + <div + data-testid="tooltip" + role="tooltip" + aria-label={typeof content === 'string' ? content : undefined} + > + {children} + </div> + ), +}))
189-201
: Reduce brittleness: avoid 300-char exact match; target the description element directly.Querying by an enormous exact string couples tests to content and can be flaky with whitespace or element splitting. Prefer a stable handle.
- // Check that the full description is rendered (CSS handles the visual truncation) - expect(screen.getByText(longDescription)).toBeInTheDocument() - - // Check that the paragraph has the line-clamp-4 class - const descriptionElement = screen.getByText(longDescription) - expect(descriptionElement).toHaveClass('line-clamp-4') + // Prefer a stable handle (add data-testid="program-description" in ProgramCard) + const descriptionElement = screen.getByTestId('program-description') + expect(descriptionElement).toHaveTextContent(longDescription) + expect(descriptionElement).toHaveClass('line-clamp-4')If ProgramCard doesn’t expose a test id yet, would you like a tiny patch adding
data-testid="program-description"
on the description?
203-214
: Short description test: assert via stable handle and keep class check.Same rationale as above; keeps intent while reducing coupling to literal text nodes.
- expect(screen.getByText('Short description')).toBeInTheDocument() - - // Check that it still has line-clamp-4 class for consistency - const descriptionElement = screen.getByText('Short description') - expect(descriptionElement).toHaveClass('line-clamp-4') + const descriptionElement = screen.getByTestId('program-description') + expect(descriptionElement).toHaveTextContent('Short description') + expect(descriptionElement).toHaveClass('line-clamp-4')frontend/__tests__/unit/components/SingleModuleCard.test.tsx (3)
191-197
: Rename for accuracy.This test isn’t using “minimal props” (the module has full data). Consider renaming for clarity.
- it('renders correctly with minimal props', () => { + it('renders correctly with default props', () => {
199-211
: Assert that admin UI is actually absent.Since admin menu/options were removed, strengthen the test by asserting the absence of any menu trigger or menu role.
) - expect(screen.getByText('Test Module')).toBeInTheDocument() + expect(screen.getByText('Test Module')).toBeInTheDocument() + // No admin menu/trigger should exist + expect(screen.queryByRole('menu')).not.toBeInTheDocument() + expect( + screen.queryByRole('button', { name: /more options|menu|actions/i }) + ).not.toBeInTheDocument()Heads-up: Your FontAwesomeIcon mock labels any non-users icon as "ellipsis", which can mask regressions. Prefer absence checks by role/name over icon test IDs.
247-253
: Avoid asserting an h1 inside a card; assert the heading by name instead.Cards typically shouldn’t contain page-level h1s. Relax the assertion to the heading’s accessible name to avoid enforcing dubious document structure.
- const moduleTitle = screen.getByRole('heading', { level: 1 }) - expect(moduleTitle).toBeInTheDocument() - expect(moduleTitle).toHaveTextContent('Test Module') + const moduleTitle = screen.getByRole('heading', { name: /test module/i }) + expect(moduleTitle).toBeInTheDocument()
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
frontend/__tests__/unit/components/ProgramCard.test.tsx
(2 hunks)frontend/__tests__/unit/components/SingleModuleCard.test.tsx
(4 hunks)frontend/__tests__/unit/pages/ProgramDetails.test.tsx
(1 hunks)frontend/__tests__/unit/pages/ProgramDetailsMentorship.test.tsx
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-12T17:36:57.255Z
Learnt from: Rajgupta36
PR: OWASP/Nest#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/components/SingleModuleCard.test.tsx
⏰ 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: Run frontend checks
- GitHub Check: CodeQL (python)
🔇 Additional comments (5)
frontend/__tests__/unit/pages/ProgramDetailsMentorship.test.tsx (1)
70-70
: Casing expectation update — OK.The assertion now matches title-cased levels. No issues spotted.
frontend/__tests__/unit/pages/ProgramDetails.test.tsx (1)
70-70
: Casing expectation update — OK.Matches the new title-cased rendering.
frontend/__tests__/unit/components/SingleModuleCard.test.tsx (3)
2-2
: LGTM: move to screen queries.Using screen.* aligns with Testing Library guidance and keeps queries consistent.
228-233
: LGTM: covers undefined admins gracefully.Good guard to ensure optional admin props don’t break rendering.
237-245
: Do not change the href assertion to a single slash
The component prependswindow.location.pathname
(default/
in tests), yielding'//modules/test-module'
, so updating to'/modules/test-module'
will break the test. If you want to avoid the double-slash, either stubwindow.location.pathname
in the test or assert dynamically withexpect(moduleLink).toHaveAttribute( 'href', `${window.location.pathname}/modules/${mockModule.key}` )You may still opt-in to using
getByRole('link', { name: /test module/i })
for an a11y-focused query, and confirm the intendedtarget="_blank"
behavior.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
♻️ Duplicate comments (3)
frontend/__tests__/unit/components/SingleModuleCard.test.tsx (3)
165-172
: Fix link expectation: single leading slash and query by role (dup of earlier comment).Use “/modules/…” (not “//…”) and prefer
getByRole
with accessible name.- const moduleLink = screen.getByTestId('module-link') - expect(moduleLink).toHaveAttribute('href', '//modules/test-module') + const moduleLink = screen.getByRole('link', { name: /test module/i }) + expect(moduleLink).toHaveAttribute('href', '/modules/test-module')
175-187
: Apply the same link fix here: role-based query + single-slash href.- const moduleLink = screen.getByTestId('module-link') - expect(moduleLink).toHaveAttribute('href', '//modules/test-module') + const moduleLink = screen.getByRole('link', { name: /test module/i }) + expect(moduleLink).toHaveAttribute('href', '/modules/test-module')
236-244
: Accessibility check: role-based query + single-slash href; also reconsider target=_blank for internal route.Keep assertions accessible and correct the href. Opening internal pages in a new tab is atypical in Next.js—confirm if product wants this.
- const moduleLink = screen.getByTestId('module-link') - expect(moduleLink).toBeInTheDocument() - expect(moduleLink).toHaveAttribute('href', '//modules/test-module') - expect(moduleLink).toHaveAttribute('target', '_blank') - expect(moduleLink).toHaveAttribute('rel', 'noopener noreferrer') + const moduleLink = screen.getByRole('link', { name: /test module/i }) + expect(moduleLink).toBeInTheDocument() + expect(moduleLink).toHaveAttribute('href', '/modules/test-module') + expect(moduleLink).toHaveAttribute('target', '_blank') + expect(moduleLink).toHaveAttribute('rel', 'noopener noreferrer')If internal navigation should not open a new tab, drop these:
- expect(moduleLink).toHaveAttribute('target', '_blank') - expect(moduleLink).toHaveAttribute('rel', 'noopener noreferrer')
🧹 Nitpick comments (3)
frontend/__tests__/unit/components/SingleModuleCard.test.tsx (3)
36-47
: Remove test-id from Link mock; assert via role instead.Prefer accessible, role-based queries. The custom
data-testid="module-link"
isn’t needed and encourages fragile tests.- <a - href={href} - target={target} - rel={rel} - className={className} - {...props} - data-testid="module-link" - > + <a href={href} target={target} rel={rel} className={className} {...props}>
246-252
: Don’t assert H1 in a card; assert by accessible name to avoid page-level heading conflicts.Card-level H1s harm document outline when multiple cards render on a page.
- const moduleTitle = screen.getByRole('heading', { level: 1 }) - expect(moduleTitle).toBeInTheDocument() - expect(moduleTitle).toHaveTextContent('Test Module') + const moduleTitle = screen.getByRole('heading', { name: /test module/i }) + expect(moduleTitle).toBeInTheDocument()
259-261
: Avoid brittle Tailwind class assertions.Class names are implementation details; assert semantics/visibility instead.
- const moduleTitle = screen.getByText('Test Module') - expect(moduleTitle).toHaveClass('sm:break-normal', 'sm:text-lg', 'lg:text-2xl') + const moduleTitle = screen.getByRole('heading', { name: /test module/i }) + expect(moduleTitle).toBeVisible()
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/__tests__/unit/components/SingleModuleCard.test.tsx
(4 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 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/__tests__/unit/components/SingleModuleCard.test.tsx
📚 Learning: 2025-07-12T17:36:57.255Z
Learnt from: Rajgupta36
PR: OWASP/Nest#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/components/SingleModuleCard.test.tsx
🧬 Code graph analysis (1)
frontend/__tests__/unit/components/SingleModuleCard.test.tsx (1)
frontend/src/wrappers/testUtil.tsx (1)
render
(14-14)
⏰ 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: Run frontend e2e tests
- GitHub Check: Run frontend unit tests
🔇 Additional comments (2)
frontend/__tests__/unit/components/SingleModuleCard.test.tsx (2)
137-139
: Confirm ellipsis icon expectation post-menu removal.If the dropdown/menu is gone, the ellipsis might be removed too. Adjust assertion accordingly.
- expect(screen.getByTestId('icon-ellipsis')).toBeInTheDocument() + // If menu/ellipsis was removed, drop this assertion.
12-14
: Ignore pruning the router mock:SingleModuleCard.tsx
imports and callsuseRouter()
(line 29), so the existingjest.mock('next/navigation')
setup in the test is required and should not be removed.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.
Left comments ⬇️
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
frontend/src/components/ModuleCard.tsx (1)
95-107
: Handle negative or zero-length durations gracefully.If end < start, the current logic yields negative weeks. Guard and return a clear message.
const ms = endDate.getTime() - startDate.getTime() + if (ms < 0) { + return 'Invalid duration' + } const days = Math.floor(ms / (1000 * 60 * 60 * 24))frontend/src/components/ModuleForm.tsx (1)
282-285
: Don’t throw inside debounced async; fix Error construction.
new Error('msg', err)
is invalid, and rethrowing here leads to unhandled rejections. Set error state and log instead.- throw new Error('Error fetching suggestions:', err) + // Avoid unhandled Promise rejections from a debounced async. + console.error('Error fetching suggestions:', err) + setError('Failed to fetch project suggestions. Please try again.')frontend/src/components/ProgramCard.tsx (1)
17-23
: Reuse shared date formatter to avoid duplication and edge-case drift.Import the existing utility which validates inputs and formats consistently.
-import type React from 'react' +import type React from 'react' +import { formatDate } from 'utils/dateFormatter' @@ - const formatDate = (d: string) => - new Date(d).toLocaleDateString('en-US', { - month: 'short', - day: 'numeric', - year: 'numeric', - }) + // Using shared formatter for consistency across the app
🧹 Nitpick comments (3)
frontend/src/components/ModuleCard.tsx (1)
78-82
: Non-standard Tailwind classes (“h-46”, “border-1”).These aren’t in Tailwind’s default scale and may be ignored. Prefer standard scales or arbitrary values.
- <div className="flex h-46 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-48 w-full flex-col gap-3 rounded-lg border border-gray-200 p-4 shadow-sm transition-shadow ease-in-out hover:shadow-md dark:border-gray-700 dark:bg-gray-800">frontend/src/components/ProgramForm.tsx (1)
212-215
: Prefer router.back() for consistency with Next.js usage elsewhere.Minor consistency win; ModuleForm already uses router.back().
- onClick={() => history.back()} + onClick={() => (window.history?.back ? history.back() : null)}Or import and use Next’s router:
const router = useRouter(); router.back()
.frontend/src/components/ProgramCard.tsx (1)
33-35
: Consider fluid width to play nicer with responsive grids.Fixed widths (w-72/w-80/w-96) can cause overflow/misalignment at certain breakpoints. Optional: use w-full with a max-w per breakpoint.
- <div className="h-72 w-72 rounded-lg border border-gray-400 bg-white p-6 text-left transition-transform duration-300 hover:scale-[1.02] hover:brightness-105 md:h-80 md:w-80 lg:h-80 lg:w-96 dark:border-gray-600 dark:bg-gray-800"> + <div className="h-72 w-full max-w-[18rem] rounded-lg border border-gray-400 bg-white p-6 text-left transition-transform duration-300 hover:scale-[1.02] hover:brightness-105 md:h-80 md:max-w-[20rem] lg:h-80 lg:max-w-[24rem] dark:border-gray-600 dark:bg-gray-800">
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
frontend/__tests__/unit/components/SingleModuleCard.test.tsx
(4 hunks)frontend/src/app/mentorship/programs/page.tsx
(1 hunks)frontend/src/app/my/mentorship/page.tsx
(1 hunks)frontend/src/components/ModuleCard.tsx
(2 hunks)frontend/src/components/ModuleForm.tsx
(1 hunks)frontend/src/components/ProgramCard.tsx
(3 hunks)frontend/src/components/ProgramForm.tsx
(1 hunks)frontend/src/components/SingleModuleCard.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/tests/unit/components/SingleModuleCard.test.tsx
- frontend/src/components/SingleModuleCard.tsx
🧰 Additional context used
🧠 Learnings (2)
📚 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/components/ProgramForm.tsx
frontend/src/components/ModuleForm.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/app/mentorship/programs/page.tsx
frontend/src/components/ProgramCard.tsx
🧬 Code graph analysis (2)
frontend/src/components/ModuleCard.tsx (1)
frontend/src/components/TruncatedText.tsx (1)
TruncatedText
(3-45)
frontend/src/components/ProgramCard.tsx (1)
frontend/src/utils/dateFormatter.ts (1)
formatDate
(1-20)
⏰ 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 unit tests
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run backend tests
🔇 Additional comments (3)
frontend/src/app/my/mentorship/page.tsx (1)
132-132
: Approve: grid simplification aligns with ProgramCard sizing
Grid update matches the new ProgramCard dimensions; note the lg:w-96 class in ProgramCard to prevent overflow on narrow xl containers.frontend/src/components/ProgramCard.tsx (1)
66-69
: Confirm hover tooltip for full description
ProgramCard.tsx (lines 66–69) still usesline-clamp-4
without any tooltip; if spec #2194 requires showing the full description on hover, wrap the<p>
in<Tooltip content={description}>…</Tooltip>
.frontend/src/app/mentorship/programs/page.tsx (1)
53-53
: LGTM: unified spacing and breakpoint simplification.Matches the updated card sizing and keeps grids consistent with “My Mentorship” page.
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/components/CardDetailsPage.tsx (1)
88-96
: Fix stray apostrophe and replace non-Tailwind class.
- Remove the stray
'
afterdark:hover:text-white
(it breaks the class token).- Replace
text-nowrap
(Bootstrap) with Tailwind’swhitespace-nowrap
.- className="dark:hover:text-white' flex items-center justify-center gap-2 rounded-md border border-[#1D7BD7] bg-transparent px-2 py-2 text-nowrap text-[#1D7BD7] hover:bg-[#1D7BD7] hover:text-white" + className="flex items-center justify-center gap-2 whitespace-nowrap rounded-md border border-[#1D7BD7] bg-transparent px-2 py-2 text-[#1D7BD7] hover:bg-[#1D7BD7] hover:text-white"
🧹 Nitpick comments (2)
frontend/src/components/CardDetailsPage.tsx (2)
91-93
: Avoidwindow.location
in Next.js routing; useusePathname()
.This keeps it SSR/edge-friendly and testable.
- import { useRouter } from 'next/navigation' + import { useRouter, usePathname } from 'next/navigation'const { data } = useSession() - const router = useRouter() + const router = useRouter() + const pathname = usePathname()- router.push(`${window.location.pathname}/edit`) + router.push(`${pathname}/edit`)
136-143
: Consider aligning with project-wide title-casing helper.Elsewhere in this PR, status/experience switched to a custom title-casing helper. For consistency and to reduce lodash surface area, consider using the same helper here or dropping the
upperFirst
import if not needed.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/components/CardDetailsPage.tsx
(1 hunks)
⏰ 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
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
🧹 Nitpick comments (1)
frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx (1)
52-52
: Normalize experienceLevel display — use a label-safe title-case helper that splits on "_" and "-" and title-cases each token.titleCaseWord/upperFirst are used inconsistently (page.tsx uses titleCaseWord, other components use upperFirst); repo search shows no current underscore-containing values but enum sources vary (upper/lowercase), so normalize now to avoid cases like "VERY_ADVANCED" rendering as "Very_advanced".
Files to update:
- frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx — currently: { label: 'Experience Level', value: titleCaseWord(module.experienceLevel) }
- frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx, frontend/src/components/SingleModuleCard.tsx, frontend/src/components/ModuleCard.tsx — currently use upperFirst(...)
Suggested change:
-import { titleCaseWord } from 'utils/capitalize' +import { titleCaseLabel } from 'utils/capitalize' @@ - { label: 'Experience Level', value: titleCaseWord(module.experienceLevel) }, + { label: 'Experience Level', value: titleCaseLabel(module.experienceLevel) },Helper (frontend/src/utils/capitalize.ts):
export function titleCaseLabel(value: string): string { if (!value) return value as unknown as string return value .replace(/[_-]+/g, ' ') .split(' ') .map(titleCaseWord) .join(' ') }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx
(2 hunks)frontend/src/components/ProgramCard.tsx
(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/components/ProgramCard.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/mentorship/programs/[programKey]/modules/[moduleKey]/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/mentorship/programs/[programKey]/modules/[moduleKey]/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/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx
🧬 Code graph analysis (1)
frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx (1)
frontend/src/utils/capitalize.ts (1)
titleCaseWord
(1-4)
⏰ 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 unit tests
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run backend tests
import { ErrorDisplay, handleAppError } from 'app/global-error' | ||
import { GET_PROGRAM_ADMINS_AND_MODULES } from 'server/queries/moduleQueries' | ||
import type { Module } from 'types/mentorship' | ||
import { titleCaseWord } from 'utils/capitalize' |
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.
💡 Verification agent
🧩 Analysis chain
Import path alias check for utils/capitalize
Using utils/capitalize
assumes a baseUrl or alias; this can break in isolated runners (Jest/Storybook) if not configured. Verify aliasing or switch to the project’s standard alias/relative path.
Run:
🏁 Script executed:
#!/bin/bash
# Confirm baseUrl/paths config
rg -n -C2 '"baseUrl"|"paths"' frontend/tsconfig*.json
# See if this alias is used elsewhere consistently
rg -nP -C2 "from ['\"]utils/capitalize['\"]" frontend/src
Length of output: 2094
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== frontend/tsconfig*.json files ==="
ls -la frontend/tsconfig*.json 2>/dev/null || true
echo
echo "=== frontend/tsconfig*.json: baseUrl / paths / extends ==="
rg -n -C2 '"baseUrl"|"paths"|"extends"' frontend/tsconfig*.json || true
echo
echo "=== repo-wide tsconfig paths occurrences ==="
rg -n -C2 '"paths"\s*:' || true
echo
echo "=== frontend/package.json (first 200 lines) ==="
if [ -f frontend/package.json ]; then
sed -n '1,200p' frontend/package.json
else
echo "no frontend/package.json"
fi
echo
echo "=== Search for Jest/Vitest/moduleNameMapper/tsconfig-paths/module-alias/module-resolver ==="
rg -n -C3 "moduleNameMapper|module-alias|tsconfig-paths|module-resolver|vitest|jest.config|jest\\.config" || true
echo
echo "=== Search for Storybook config (.storybook) and webpack resolve/alias usage ==="
rg -n -C3 "\\.storybook|webpackFinal|resolve\\s*:\\s*\\{|resolve\\.|alias\\s*:" || true
echo
echo "=== Search for next.config.js or Next webpack customizations ==="
rg -n -C3 "next.config|nextConfig|webpack\\s*:\\s*\\(" || true
echo
echo "=== Look for definitions/locations of titleCaseWord / capitalize file ==="
rg -n -C3 "titleCaseWord|utils/capitalize" || true
echo
echo "=== Search for any explicit 'utils/' path mappings in configs or code ==="
rg -n -C3 "\"utils/|\\butils\\/" || true
echo
Length of output: 23641
Resolve 'utils/capitalize' import — use project alias or add path mapping
frontend/tsconfig.json has baseUrl "." and no "utils/" path mapping; jest only maps "^@/(.)$" → "/src/$1". The file exists at frontend/src/utils/capitalize.ts but imports use "utils/capitalize" in:
- frontend/src/app/mentorship/programs/[programKey]/page.tsx
- frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
- frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx
Either:
- Change imports to the project alias: '@/utils/capitalize', or
- Add "utils/": ["src/utils/"] to frontend/tsconfig.json and add a matching entry in frontend/jest.config.ts moduleNameMapper.
🤖 Prompt for AI Agents
frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx
around line 9: the import "utils/capitalize" is unresolved because
frontend/tsconfig.json has no "utils/*" path mapping; replace the import with
the project alias import "@/utils/capitalize" (and update the same pattern in
the other files listed) so the path resolves correctly, or alternatively add
"utils/*": ["src/utils/*"] to frontend/tsconfig.json and add the corresponding
moduleNameMapper entry in frontend/jest.config.ts to keep Jest consistent.
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.
@Rajgupta36 I pushed some changes for styling, but the rest looks good 👌🏼
|
c9244ca
into
OWASP:feature/mentorship-portal
* 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 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 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 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]>
* 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 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 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]>
* 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 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]>
* 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 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 #2194
Media






