UI/ux mentorship admin #2288
UI/ux mentorship admin #2288
Conversation
Summary by CodeRabbit
WalkthroughAdds programKey props to ProgramActions and DetailsCard, introduces isAdmin on ProgramCard and pages, centralizes status updates via a new useUpdateProgramStatus hook, includes status in GET_MY_PROGRAMS, updates Program/Module form styling, and adjusts tests and Jest navigation/hook mocks. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (1)
107-121: Don’t force admin UI for non‑admins.Passing accessLevel="admin" unconditionally can expose admin‑only UI to non‑admins. Derive from isAdmin.
Apply:
- accessLevel="admin" + accessLevel={isAdmin ? 'admin' : 'user'}
♻️ Duplicate comments (1)
frontend/__tests__/unit/components/ProgramCard.test.tsx (1)
157-164: Same as above: setisAdmin={false}for user View Details test.Apply the same change here.
🧹 Nitpick comments (23)
frontend/src/types/card.ts (1)
58-60: MakeprogramKeyrequired when status updates are enabled.
DetailsCardProps.programKeyis optional, butProgramActionsexpects a non‑optionalprogramKey: string. This can surface runtime bugs ifcanUpdateStatusis true butprogramKeyis undefined. Either enforce at the call sites or tighten the type here.Apply this doc hint now (and consider a discriminated union follow‑up):
admins?: Contributor[] projectName?: string + /** Required when canUpdateStatus is true. Used by ProgramActions/status updates. */ programKey?: stringLonger‑term, prefer:
type DetailsCardProps = Base & ({ canUpdateStatus: true; programKey: string; setStatus: (s: string) => void } | { canUpdateStatus?: false; programKey?: never; setStatus?: never })frontend/src/components/ActionButton.tsx (1)
17-31: Minor: redundant tooltip data attributes with HeroUI Tooltip.
data-tooltip-id/data-tooltip-contentaren’t needed with<Tooltip>wrapper and can be removed to reduce noise.frontend/src/server/queries/programsQueries.ts (1)
22-44: Reduce duplication with fragments.
GET_PROGRAM_DETAILSandGET_PROGRAM_AND_MODULESrepeat the same program fields. Extract afragment ProgramCoreFields on Programand reuse.Also applies to: 45-82
frontend/src/components/ProgramForm.tsx (1)
211-215: Optional: use Next.js router for navigation.
history.back()works, butrouter.back()keeps navigation concerns consistent with other components/tests.Outside this hunk, add:
import { useRouter } from 'next/navigation' const router = useRouter()Then:
-onClick={() => history.back()} +onClick={() => router.back()}frontend/jest.setup.ts (1)
46-60: Harden the Next.js router mock (missingrefresh).Several components call
router.refresh(). Add it to prevent test failures; keep parity with Next 13+.const prefetch = jest.fn() const back = jest.fn() const forward = jest.fn() - const mockRouter = { push, replace, prefetch, back, forward } + const refresh = jest.fn() + const mockRouter = { push, replace, prefetch, back, forward, refresh } return { ...actual, useRouter: jest.fn(() => mockRouter), useSearchParams: jest.fn(() => new URLSearchParams()), useParams: jest.fn(() => ({})), }frontend/src/hooks/useUpdateProgramStatus.ts (1)
7-12: LoosenrefetchQueriestyping.The current
{ variables: { programKey: string } }[]is too narrow for queries like lists/pagination. Prefer Apollo’s native type or a generic variables shape.- refetchQueries?: { query: DocumentNode; variables: { programKey: string } }[] + refetchQueries?: { query: DocumentNode; variables?: Record<string, unknown> }[]frontend/__tests__/unit/pages/Program.test.tsx (1)
15-17: Good isolation of side effects via hook mock.Mocking
useUpdateProgramStatuskeeps tests deterministic.You can centralize this mock in
jest.setup.tsto reduce repetition, and override per‑test when needed.frontend/src/app/mentorship/programs/page.tsx (1)
37-37: Prop is redundant; removeisAdmin={false}.ProgramCard defaults to non-admin. Dropping this reduces noise without behavior change.
Apply:
- isAdmin={false}frontend/__tests__/unit/components/ProgramActions.test.tsx (2)
55-61: Strengthen assertion: verify exact route for Add Module.Assert
pushis called with the program-specific path.Apply:
- expect(mockPush).toHaveBeenCalled() + expect(mockPush).toHaveBeenCalledWith( + '/my/mentorship/programs/test-program/modules/create' + )
64-70: Add explicit non-navigation assertion for Publish.Keep ensuring no navigation and status set correctly.
Apply:
fireEvent.click(screen.getByRole('menuitem', { name: /publish program/i })) expect(setStatus).toHaveBeenCalledWith('PUBLISHED') - expect(mockPush).not.toHaveBeenCalled() + expect(mockPush).not.toHaveBeenCalled()Optional: also add an “Edit Program navigates” test here for completeness.
frontend/src/components/ProgramActions.tsx (3)
10-13: Keep strong typing for status/setStatus.Avoid widening to
string; use the Program status union/enum to prevent invalid updates.Apply:
-import type React from 'react' +import type React from 'react' +import type { ProgramStatusEnum } from 'types/mentorship' @@ interface ProgramActionsProps { programKey: string - status: string - setStatus: (newStatus: string) => void + status: ProgramStatusEnum + setStatus: (newStatus: ProgramStatusEnum) => void }
23-27: Normalize action keys (no spaces) for maintainability.Use stable identifiers; labels can stay human-readable.
Apply:
- case 'edit Program': + case 'edit': router.push(`/my/mentorship/programs/${programKey}/edit`) break case 'create_module': router.push(`/my/mentorship/programs/${programKey}/modules/create`) break @@ - const options = [ - { key: 'edit Program', label: 'Edit Program' }, + const options = [ + { key: 'edit', label: 'Edit Program' }, { key: 'create_module', label: 'Add Module' },Also applies to: 41-49
67-76: Improve a11y and hit target for actions button.Add padding and ARIA attributes; expose menu role for items.
Apply:
<button data-testid="program-actions-button" type="button" onClick={() => setDropdownOpen((prev) => !prev)} - className="rounded hover:bg-gray-200 dark:hover:bg-gray-700" + className="rounded p-2 hover:bg-gray-200 dark:hover:bg-gray-700" + aria-label="Program actions" + aria-haspopup="menu" + aria-expanded={dropdownOpen} > <FontAwesomeIcon icon={faEllipsisV} /> </button> {dropdownOpen && ( - <div className="absolute right-0 z-20 mt-2 w-40 rounded-md border border-gray-200 bg-white shadow-lg dark:border-gray-700 dark:bg-gray-800"> + <div + role="menu" + className="absolute right-0 z-20 mt-2 w-40 rounded-md border border-gray-200 bg-white shadow-lg dark:border-gray-700 dark:bg-gray-800" + >frontend/__tests__/unit/components/ProgramCard.test.tsx (3)
230-232: This assertion can’t work: it searches text for CSS classes.Assert absence of any role badge text instead.
Apply:
- // Should not render badge when userRole is undefined - expect(screen.queryByText(/bg-/)).not.toBeInTheDocument() + // Should not render a role badge when userRole is undefined + expect(screen.queryByText(/^(admin|mentor|unknown)$/i)).not.toBeInTheDocument()
376-387: Good: actions trigger presence. Consider adding absence check for user view.After adjusting
isAdminin user tests, also assert the actions button is not rendered for users.Example:
expect(screen.queryByTestId('program-actions-button')).not.toBeInTheDocument()
404-416: Nice coverage. Consider asserting the exact edit route.Mirror the ProgramActions test: ensure the path includes the program key.
expect(useRouter().push).toHaveBeenCalledWith('/my/mentorship/programs/test-program/edit')frontend/src/components/ProgramCard.tsx (4)
67-74: Avoid empty role pill when userRole is undefined.Guard rendering so we don’t show an empty badge.
Apply:
- {accessLevel === 'admin' && ( + {accessLevel === 'admin' && program.userRole && ( <span className={`rounded-full px-2 py-1 text-xs font-medium capitalize ${ roleClass[program.userRole] ?? roleClass.default }`} > {program.userRole} </span> )}
18-23: Use shared date formatter for consistency.Reuse utils/dateFormatter (already used elsewhere) instead of a local formatter.
Apply (within this hunk):
- const formatDate = (d: string) => - new Date(d).toLocaleDateString('en-US', { - month: 'short', - day: 'numeric', - year: 'numeric', - })And add this import (outside the changed hunk):
import { formatDate } from 'utils/dateFormatter'
44-44: Softer card border to match the updated visual tone.Minor: consider slightly lighter borders to align with “less visually heavy” direction.
Apply:
- <div className="flex h-64 w-80 flex-col justify-between rounded-[5px] border border-gray-400 bg-white p-6 text-left transition-transform duration-300 hover:scale-[1.02] hover:brightness-105 dark:border-gray-600 dark:bg-gray-800"> + <div className="flex h-64 w-80 flex-col justify-between rounded-[5px] border border-gray-300 bg-white p-6 text-left transition-transform duration-300 hover:scale-[1.02] hover:brightness-105 dark:border-gray-700 dark:bg-gray-800">
51-56: Let ProgramActions size itself.Padding here may bloat the icon button and misalign the header. Prefer relying on ProgramActions’ internal sizing or use compact padding.
Apply:
- <ProgramActions - className={'px-4 py-2'} + <ProgramActions + className="p-2" programKey={program.key} status={program.status} setStatus={updateProgramStatus} />frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (3)
95-104: Type‑safety: ensure program is non‑null before deref.TS may still complain about possible null after the earlier checks. Add a guard for clarity.
Apply:
if (!program && !isLoading) { return ( <ErrorDisplay statusCode={404} title="Program Not Found" message="Sorry, the program you're looking for doesn't exist." /> ) } + + if (!program) return null
59-71: Clean query param without window dependency.Prefer usePathname for SSR‑safe path retrieval and clearer intent.
Apply (within this hunk):
- const cleaned = params.toString() - router.replace(cleaned ? `?${cleaned}` : window.location.pathname, { scroll: false }) + const cleaned = params.toString() + const pathname = window.location.pathname + router.replace(cleaned ? `${pathname}?${cleaned}` : pathname, { scroll: false })And ideally replace window with Next’s hook (outside this hunk):
import { usePathname } from 'next/navigation' const pathname = usePathname() router.replace(cleaned ? `${pathname}?${cleaned}` : pathname, { scroll: false })
52-57: Stale capture of programName (low risk).If the name changes in‑page, an older closure could show an outdated toast. Non‑blocking; optional to read name at call time.
Possible tweak:
const { updateProgramStatus } = useUpdateProgramStatus({ programKey, programName: program?.name ?? '', isAdmin, refetchQueries: [{ query: GET_PROGRAM_AND_MODULES, variables: { programKey } }], })
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
frontend/__tests__/unit/components/ProgramActions.test.tsx(5 hunks)frontend/__tests__/unit/components/ProgramCard.test.tsx(10 hunks)frontend/__tests__/unit/pages/MyMentorship.test.tsx(1 hunks)frontend/__tests__/unit/pages/Program.test.tsx(1 hunks)frontend/jest.setup.ts(1 hunks)frontend/src/app/mentorship/programs/page.tsx(2 hunks)frontend/src/app/my/mentorship/page.tsx(1 hunks)frontend/src/app/my/mentorship/programs/[programKey]/page.tsx(3 hunks)frontend/src/components/ActionButton.tsx(1 hunks)frontend/src/components/CardDetailsPage.tsx(2 hunks)frontend/src/components/ProgramActions.tsx(2 hunks)frontend/src/components/ProgramCard.tsx(2 hunks)frontend/src/components/ProgramForm.tsx(8 hunks)frontend/src/hooks/useUpdateProgramStatus.ts(1 hunks)frontend/src/server/queries/programsQueries.ts(1 hunks)frontend/src/types/card.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
📚 Learning: 2025-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
📚 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/hooks/useUpdateProgramStatus.tsfrontend/src/server/queries/programsQueries.tsfrontend/__tests__/unit/pages/Program.test.tsxfrontend/src/components/ProgramActions.tsxfrontend/src/components/CardDetailsPage.tsxfrontend/src/app/my/mentorship/page.tsxfrontend/src/components/ProgramCard.tsxfrontend/src/app/mentorship/programs/page.tsxfrontend/__tests__/unit/components/ProgramCard.test.tsxfrontend/src/app/my/mentorship/programs/[programKey]/page.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details. These queries cannot be removed or merged as they serve different use cases in the application.
Applied to files:
frontend/src/server/queries/programsQueries.tsfrontend/src/components/ProgramCard.tsxfrontend/src/app/mentorship/programs/page.tsxfrontend/src/app/my/mentorship/programs/[programKey]/page.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details.
Applied to files:
frontend/src/server/queries/programsQueries.tsfrontend/src/components/ProgramCard.tsxfrontend/src/app/mentorship/programs/page.tsxfrontend/src/app/my/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/__tests__/unit/components/ProgramCard.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/ProgramCard.test.tsx
🧬 Code graph analysis (3)
frontend/src/hooks/useUpdateProgramStatus.ts (2)
frontend/src/server/mutations/programsMutations.ts (1)
UPDATE_PROGRAM_STATUS_MUTATION(43-50)frontend/src/app/global-error.tsx (1)
handleAppError(66-86)
frontend/src/components/ProgramCard.tsx (3)
frontend/src/types/mentorship.ts (1)
Program(22-37)frontend/src/hooks/useUpdateProgramStatus.ts (1)
useUpdateProgramStatus(14-61)frontend/src/server/queries/programsQueries.ts (1)
GET_PROGRAM_AND_MODULES(45-82)
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (2)
frontend/src/hooks/useUpdateProgramStatus.ts (1)
useUpdateProgramStatus(14-61)frontend/src/server/queries/programsQueries.ts (1)
GET_PROGRAM_AND_MODULES(45-82)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run frontend unit tests
- GitHub Check: Run backend tests
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (14)
frontend/src/components/ProgramForm.tsx (1)
209-215: Borders match the “border‑1, softer color” requirement.
border-t border-gray-200and updated button borders deliver the intended visual weight. Looks good.frontend/src/hooks/useUpdateProgramStatus.ts (2)
24-34: Guardrails are client‑side only; server must enforce auth.The admin check and toast are good UX, but ensure the server mutation also validates admin privileges.
24-46: Keepname(it's in the schema) — addawaitRefetchQueries.
- Generated types include UpdateProgramStatusInput.name (keep
name; do not remove it).- Add
awaitRefetchQueries: trueto theupdateProgramMutationcall so the success toast waits for refetch to finish.Likely an incorrect or invalid review comment.
frontend/__tests__/unit/pages/MyMentorship.test.tsx (1)
32-34: Hook mock aligns with new admin flow.Looks good and consistent with other suites.
frontend/src/server/queries/programsQueries.ts (1)
12-12: LGTM — verify production schema & regen types
- GET_MY_PROGRAMS now requests
status(frontend/src/server/queries/programsQueries.ts).- Backend ProgramNode defines
status(backend/apps/mentorship/api/internal/nodes/program.py).- Generated query types for GetMyPrograms lack
status(frontend/src/types/generated/programsQueries.generated.ts) — run GraphQL codegen to update.- Confirm production GraphQL exposes
Program.statusonmyPrograms.programsbefore deploying to avoid runtime errors.frontend/src/app/mentorship/programs/page.tsx (1)
56-56: Status filter casing may diverge from enum usage elsewhere.Other code/tests use
PUBLISHED; here it's'published'. Normalize to avoid silent mismatches.Apply:
- {programs && programs.filter((p) => p.status === 'published').map(renderProgramCard)} + {programs && + programs + .filter((p) => String(p.status).toUpperCase() === 'PUBLISHED') + .map(renderProgramCard)}frontend/src/app/my/mentorship/page.tsx (1)
143-144: LGTM: gating admin UI off role.Passing
isAdmin={p?.userRole === 'admin'}aligns ProgramCard behavior with user role.frontend/src/components/ProgramActions.tsx (1)
3-5: Icon name check.Confirm
faEllipsisVexists in your installed FontAwesome version; some packages exposefaEllipsisVertical.frontend/__tests__/unit/components/ProgramCard.test.tsx (1)
109-124: Router mock present — no action required.
jest.setup.ts mocks 'next/navigation' and returns a mockRouter with push = jest.fn(), so router.push is spyable and the test assertion is valid.frontend/src/components/ProgramCard.tsx (4)
25-30: Hook wiring + refetch looks good.Passing programKey, programName, and refetchQueries into useUpdateProgramStatus is correct and aligns with the query contract.
80-83: LGTM: single primary action.“Preview” vs “View Details” simplifies the CTA while keeping admin context.
50-57: ProgramActions: required admin actions are implemented.
Contains Edit (navigates to /my/mentorship/programs/${programKey}/edit), create_module (add module), and status actions including Move to Draft / Publish Program / Mark as Completed.
10-17: Verified — all ProgramCard call sites include isAdmin. Repository-wide search across .tsx/.jsx found no usages missing the isAdmin prop.frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (1)
30-38: LGTM: query config and loading orchestration.notifyOnNetworkStatusChange + local isRefetching yields smooth refresh UX.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/__tests__/unit/components/ProgramCard.test.tsx (1)
229-232: Broken assertion: querying class names via text always passes
queryByText(/bg-/)searches text, not classes, so this test can never fail even if a badge renders. Assert on visible labels or a test id.Apply either:
Option A (preferred if you can expose a test id on the badge in ProgramCard):
- // Should not render badge when userRole is undefined - expect(screen.queryByText(/bg-/)).not.toBeInTheDocument() + // Should not render badge when userRole is undefined + expect(screen.queryByTestId('user-role-badge')).toBeNull()Option B (no component changes):
- // Should not render badge when userRole is undefined - expect(screen.queryByText(/bg-/)).not.toBeInTheDocument() + // Should not render badge when userRole is undefined + expect(screen.queryByText('admin')).not.toBeInTheDocument() + expect(screen.queryByText('mentor')).not.toBeInTheDocument() + expect(screen.queryByText('unknown')).not.toBeInTheDocument()
♻️ Duplicate comments (3)
frontend/src/components/ProgramForm.tsx (2)
79-79: Borders + focus states on form fields — LGTM.Consistent 1px borders and added visible focus rings hit the UX goal and restore keyboard focus visibility.
Confirm this translation aligns with the issue’s “border-1” ask (Tailwind’s
borderis 1px; there’s no defaultborder-1).Also applies to: 94-94, 117-117, 131-131, 146-146, 169-169, 183-183
201-201: Add visible focus styles to the Admin GitHub Usernames input.This field still removes the default outline without adding a custom ring.
Apply:
- className="w-full rounded-lg border border-gray-300 bg-gray-50 px-4 py-3 text-gray-800 focus:outline-none dark:border-gray-600 dark:bg-gray-800 dark:text-gray-200" + className="w-full rounded-lg border border-gray-300 bg-gray-50 px-4 py-3 text-gray-800 focus:border-[#0D6EFD] focus:outline-none focus-visible:ring-1 focus-visible:ring-[#0D6EFD] dark:border-gray-600 dark:bg-gray-800 dark:text-gray-200"frontend/__tests__/unit/components/ProgramCard.test.tsx (1)
142-149: Good fix: user-view tests now render with isAdmin={false}This addresses the earlier concern about weakening user-only scenarios.
Also applies to: 157-164, 130-136
🧹 Nitpick comments (10)
frontend/src/components/ProgramForm.tsx (6)
110-118: Prevent start/end date inversions with native constraints.Use HTML min/max to block invalid ranges at input-time.
Apply:
<input id="program-start-date" type="date" name="startedAt" value={formData.startedAt} onChange={handleInputChange} required + max={formData.endedAt}<input id="program-end-date" type="date" name="endedAt" value={formData.endedAt} onChange={handleInputChange} required + min={formData.startedAt}Also applies to: 124-132
214-214: Add a visible focus ring to the Cancel button.Currently no custom focus ring; add
focus-visiblestyles for keyboard users.Apply:
- className="rounded-lg border border-gray-300 px-6 py-3 font-medium text-gray-600 hover:bg-gray-50 dark:border-gray-600 dark:text-gray-300 dark:hover:bg-gray-700" + className="rounded-lg border border-gray-300 px-6 py-3 font-medium text-gray-600 hover:bg-gray-50 focus:outline-none focus:border-[#0D6EFD] focus-visible:ring-2 focus-visible:ring-[#0D6EFD] ring-offset-2 dark:ring-offset-[#212529] dark:border-gray-600 dark:text-gray-300 dark:hover:bg-gray-700"
219-224: Add focus ring + disabled affordances to the Submit button.Improve a11y feedback on focus/disabled and preserve brand color focus.
Apply:
- <button - type="submit" - disabled={loading} - className="flex items-center justify-center gap-2 rounded-md border border-[#0D6EFD] px-4 py-2 text-[#0D6EFD] hover:bg-[#0D6EFD] hover:text-white dark:text-sky-600 dark:hover:bg-sky-100" - > + <button + type="submit" + disabled={loading} + aria-busy={loading} + className="flex items-center justify-center gap-2 rounded-md border border-[#0D6EFD] px-4 py-2 text-[#0D6EFD] hover:bg-[#0D6EFD] hover:text-white focus:outline-none focus-visible:ring-2 focus-visible:ring-[#0D6EFD] ring-offset-2 dark:ring-offset-[#212529] disabled:opacity-50 disabled:cursor-not-allowed dark:text-sky-600 dark:hover:bg-sky-100" + >
189-196: Rename mismatched IDs (mentor → admin) for clarity.Label says “Admin”, but
id/htmlForusesmentor-….Apply:
- <label - htmlFor="mentor-github-usernames" + <label + htmlFor="admin-github-usernames" className="mb-2 block text-sm font-medium" > Admin GitHub Usernames </label> <input - id="mentor-github-usernames" + id="admin-github-usernames" type="text"
79-79: Deduplicate repeated input classes.Reduce drift by centralizing the shared input class string.
Example:
+ const inputBase = + "w-full rounded-lg border border-gray-300 bg-gray-50 px-4 py-3 text-gray-800 focus:border-[#0D6EFD] focus:outline-none focus-visible:ring-1 focus-visible:ring-[#0D6EFD] dark:border-gray-600 dark:bg-gray-800 dark:text-gray-200"; ... - <input ... className="w-full rounded-lg border border-gray-300 ..." /> + <input ... className={inputBase} />Also applies to: 94-94, 117-117, 131-131, 146-146, 169-169, 183-183, 201-201
214-214: Standardize color tokens for buttons (brand vs Tailwind named colors).Mixed
#0D6EFDandsky-*can drift across themes. Prefer a single brand token or consistent palette.Do your design tokens define a primary brand color (e.g., CSS var) to replace both
#0D6EFDandsky-*here?Also applies to: 221-221
frontend/__tests__/unit/components/ProgramCard.test.tsx (4)
10-17: Avoid hoisting pitfalls in FontAwesome mockThe mock compares
iconagainstfaEye/faEditfrom the test module. Becausejest.mockis hoisted, those identifiers may be undefined when the factory runs. Require icons inside the factory to make it safe.-jest.mock('@fortawesome/react-fontawesome', () => ({ - FontAwesomeIcon: ({ icon, className }: { icon: unknown; className?: string }) => ( - <span - data-testid={`icon-${icon === faEye ? 'eye' : icon === faEdit ? 'edit' : 'unknown'}`} - className={className} - /> - ), -})) +jest.mock('@fortawesome/react-fontawesome', () => { + const { faEye } = require('@fortawesome/free-regular-svg-icons') + const { faEdit } = require('@fortawesome/free-solid-svg-icons') + return { + FontAwesomeIcon: ({ icon, className }: { icon: unknown; className?: string }) => ( + <span + data-testid={`icon-${icon === faEye ? 'eye' : icon === faEdit ? 'edit' : 'unknown'}`} + className={className} + /> + ), + } +})
376-387: Add regression guard: ensure legacy inline Edit button is gone in admin viewTo lock the UI change, assert the old inline Edit control is absent.
it('renders actions button for admin menu', () => { render( <ProgramCard program={baseMockProgram} isAdmin={true} onView={mockOnView} accessLevel="admin" /> ) expect(screen.getByTestId('program-actions-button')).toBeInTheDocument() }) + + it('does not render inline Edit button/icon in admin view', () => { + render( + <ProgramCard program={baseMockProgram} isAdmin={true} onView={mockOnView} accessLevel="admin" /> + ) + expect(screen.queryByText('Edit')).not.toBeInTheDocument() + expect(screen.queryByTestId('icon-edit')).not.toBeInTheDocument() + })
414-416: Prefer userEvent over fireEvent for clicksuserEvent better simulates real interactions (bubbling, pointer events). Optional, but it reduces false positives.
-import { fireEvent, render, screen } from '@testing-library/react' +import { render, screen } from '@testing-library/react' +import userEvent from '@testing-library/user-event' ... - fireEvent.click(previewButton!) + await userEvent.click(previewButton!) ... - fireEvent.click(screen.getByTestId('program-actions-button')) - fireEvent.click(screen.getByText('Edit Program')) + await userEvent.click(screen.getByTestId('program-actions-button')) + await userEvent.click(screen.getByText('Edit Program'))Also applies to: 120-122, 103-105
404-416: Optional: assert other admin actions appear in the menuThe issue asks to include all actions in the three-dot menu (e.g., “Manage Program”). Add one more expectation to lock it in.
fireEvent.click(screen.getByTestId('program-actions-button')) expect(screen.getByText('Edit Program')).toBeInTheDocument() + expect(screen.getByText(/Manage Program/i)).toBeInTheDocument()If label differs, adjust the regex accordingly.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
frontend/__tests__/unit/components/ProgramCard.test.tsx(10 hunks)frontend/src/app/mentorship/programs/page.tsx(1 hunks)frontend/src/components/ProgramForm.tsx(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/app/mentorship/programs/page.tsx
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
📚 Learning: 2025-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
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
Applied to files:
frontend/__tests__/unit/components/ProgramCard.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/ProgramCard.test.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/__tests__/unit/components/ProgramCard.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). (3)
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run frontend unit tests
- GitHub Check: Run backend tests
🔇 Additional comments (3)
frontend/src/components/ProgramForm.tsx (1)
209-209: Softer section divider — LGTM.
border-t border-gray-200 dark:border-gray-700matches the “less visually heavy” brief.frontend/__tests__/unit/components/ProgramCard.test.tsx (2)
312-315: Harden date assertions against locale/timezone driftHard-coded "Jan 1, 2024" / "Dec 31, 2024" strings can flake across locales/timezones — compute expectations with the same formatter used by the component or pin Jest's TZ to UTC.
Example tweak:
- expect( - screen.getByText((t) => t.includes('Jan 1, 2024') && t.includes('Dec 31, 2024')) - ).toBeInTheDocument() + const fmt = (d: string) => + new Date(d).toLocaleDateString('en-US', { month: 'short', day: 'numeric', year: 'numeric' }) + expect( + screen.getByText((t) => t.includes(fmt('2024-01-01T00:00:00Z')) && t.includes(fmt('2024-12-31T23:59:59Z'))) + ).toBeInTheDocument()And/or set TZ in Jest setup:
// jest.setup.ts process.env.TZ = 'UTC'Verify TZ is pinned:
#!/bin/bash rg -n "process\.env\.TZ" -SSearch for process.env.TZ returned no matches; cannot confirm TZ is pinned in the repo — verify Jest setup or adopt the formatter approach. Also applies to lines 328, 343, 358.
108-124: Incorrect — global Jest mock returns singleton routerfrontend/jest.setup.ts (lines 46–59) defines a single mockRouter (push = jest.fn()) and useRouter: jest.fn(() => mockRouter), so the component and test already share the same push spy. The suggested local override is unnecessary.
Likely an incorrect or invalid review comment.
* 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]>
There was a problem hiding this comment.
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 (3)
frontend/src/components/ProgramCard.tsx (3)
71-82: Remove legacy admin CTAs (Edit/Preview) — conflicts with new 3‑dot menu and breaks build.This block reintroduces the Edit button and
onEditeven though:
onEditwas removed from props (type error).faEditisn’t imported (compile error).- Actions must be consolidated into the top‑right menu (
ProgramActions) per PR objective.Delete this block and rely on the bottom single CTA plus
ProgramActions.Apply this diff:
- <div className="flex gap-2"> - {accessLevel === 'admin' ? ( - <> - <ActionButton onClick={() => onView(program.key)}> - <FontAwesomeIcon icon={faEye} className="mr-1" /> - Preview - </ActionButton> - <ActionButton onClick={() => onEdit(program.key)}> - <FontAwesomeIcon icon={faEdit} className="mr-1" /> - Edit - </ActionButton> - </> - ) : ( - <ActionButton onClick={() => onView(program.key)}> - <FontAwesomeIcon icon={faEye} className="mr-1" /> - View Details - </ActionButton> - )} - </div>
56-63: Fix unclosed — CI failing (Prettier/parse error).There’s a missing closing tag for the
<span>opened on Line 57.Apply this diff:
<div className="mb-2 flex flex-wrap items-center gap-2 text-xs text-gray-600 dark:text-gray-400"> - <span> + <span> {program.startedAt && program.endedAt ? `${formatDate(program.startedAt)} – ${formatDate(program.endedAt)}` : program.startedAt ? `Started: ${formatDate(program.startedAt)}` : 'No dates set'} - </div> + </span> + </div>
71-82: Fix undefined onEdit & missing faEdit import; verify ProgramCard usages for isAdmin
- frontend/src/components/ProgramCard.tsx (≈ lines 71–82): onEdit(...) is called but not declared in ProgramCardProps — add onEdit: (key: string) => void or remove/replace the call.
- frontend/src/components/ProgramCard.tsx (same area): faEdit is referenced but not imported — add the appropriate import.
- Repo-wide search for ProgramCard usages returned no results in the verification run; confirm all call sites pass isAdmin or the component provides a safe default.
🧹 Nitpick comments (7)
frontend/src/components/ProgramCard.tsx (4)
42-55: Border token alignment with spec (“border-1” + softer color).The container uses
border+border-gray-400/dark:border-gray-600. The spec requestsborder-1and a softer, less‑contrasting shade. Ifborder-1is a project token, consider something likeborder-1 border-gray-300 dark:border-gray-700(or your design‑system equivalents).Apply this diff if tokens exist:
- <div className="flex h-64 w-80 flex-col justify-between rounded-[5px] border border-gray-400 bg-white p-6 text-left transition-transform duration-300 hover:scale-[1.02] hover:brightness-105 dark:border-gray-600 dark:bg-gray-800"> + <div className="flex h-64 w-80 flex-col justify-between rounded-[5px] border-1 border-gray-300 bg-white p-6 text-left transition-transform duration-300 hover:scale-[1.02] hover:brightness-105 dark:border-gray-700 dark:bg-gray-800">If
border-1isn’t defined in Tailwind config, confirm which token maps to 1px and adjust accordingly.
15-18: Make isAdmin optional with a safe default.To reduce call‑site churn and avoid accidental omissions, default
isAdmintofalse.Apply this diff:
interface ProgramCardProps { program: Program onView: (key: string) => void accessLevel: 'admin' | 'user' - isAdmin: boolean + isAdmin?: boolean } -const ProgramCard: React.FC<ProgramCardProps> = ({ program, onView, accessLevel, isAdmin }) => { +const ProgramCard: React.FC<ProgramCardProps> = ({ program, onView, accessLevel, isAdmin = false }) => {
33-37: Remove unused roleClass.Leftover constant increases bundle size and confuses readers.
Apply this diff:
- const roleClass = { - admin: 'bg-blue-100 text-blue-800 dark:bg-blue-900 dark:text-blue-200', - mentor: 'bg-green-100 text-green-800 dark:bg-green-900 dark:text-green-200', - default: 'bg-gray-100 text-gray-800 dark:bg-gray-900 dark:text-gray-200', - }
4-4: Drop unused Tooltip import.No usage after header simplification.
Apply this diff:
-import { Tooltip } from '@heroui/tooltip'frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (3)
5-5: Remove unused imports (upperFirst, ProgramStatusEnum).Not referenced after migrating to the hook and
titleCaseWord.Apply this diff:
-import upperFirst from 'lodash/upperFirst' @@ -import { ProgramStatusEnum } from 'types/mentorship'Also applies to: 13-13
49-53: Minor: Simplify canUpdateStatus.Same truthiness with less code.
Apply this diff:
- const canUpdateStatus = useMemo(() => { - if (!isAdmin || !program?.status) return false - return true - }, [isAdmin, program]) + const canUpdateStatus = useMemo(() => Boolean(isAdmin && program?.status), [isAdmin, program])
61-74: Optional: UseusePathname()instead ofwindow.location.pathname.Keeps routing SSR‑safe and avoids direct window usage.
Apply this diff:
-import { useParams, useSearchParams, useRouter } from 'next/navigation' +import { useParams, useSearchParams, useRouter, usePathname } from 'next/navigation' @@ - const shouldRefresh = searchParams.get('refresh') === 'true' + const shouldRefresh = searchParams.get('refresh') === 'true' + const pathname = usePathname() @@ - router.replace(cleaned ? `?${cleaned}` : window.location.pathname, { scroll: false }) + router.replace(cleaned ? `?${cleaned}` : pathname, { scroll: false })
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
frontend/__tests__/unit/components/ProgramCard.test.tsx(11 hunks)frontend/src/app/mentorship/programs/page.tsx(1 hunks)frontend/src/app/my/mentorship/page.tsx(1 hunks)frontend/src/app/my/mentorship/programs/[programKey]/page.tsx(3 hunks)frontend/src/components/CardDetailsPage.tsx(2 hunks)frontend/src/components/ProgramCard.tsx(3 hunks)frontend/src/components/ProgramForm.tsx(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- frontend/src/app/mentorship/programs/page.tsx
- frontend/tests/unit/components/ProgramCard.test.tsx
- frontend/src/app/my/mentorship/page.tsx
- frontend/src/components/CardDetailsPage.tsx
- frontend/src/components/ProgramForm.tsx
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
Applied to files:
frontend/src/components/ProgramCard.tsxfrontend/src/app/my/mentorship/programs/[programKey]/page.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details. These queries cannot be removed or merged as they serve different use cases in the application.
Applied to files:
frontend/src/components/ProgramCard.tsxfrontend/src/app/my/mentorship/programs/[programKey]/page.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details.
Applied to files:
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
🧬 Code graph analysis (2)
frontend/src/components/ProgramCard.tsx (3)
frontend/src/types/mentorship.ts (1)
Program(22-37)frontend/src/hooks/useUpdateProgramStatus.ts (1)
useUpdateProgramStatus(14-61)frontend/src/server/queries/programsQueries.ts (1)
GET_PROGRAM_AND_MODULES(45-82)
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (2)
frontend/src/hooks/useUpdateProgramStatus.ts (1)
useUpdateProgramStatus(14-61)frontend/src/server/queries/programsQueries.ts (1)
GET_PROGRAM_AND_MODULES(45-82)
🪛 Biome (2.1.2)
frontend/src/components/ProgramCard.tsx
[error] 57-57: Expected corresponding JSX closing tag for 'span'.
Opening tag
closing tag
(parse)
🪛 GitHub Actions: Run CI/CD
frontend/src/components/ProgramCard.tsx
[error] 57-57: Prettier formatting failed. SyntaxError: JSX element 'span' has no corresponding closing tag. (57:12)
🔇 Additional comments (5)
frontend/src/components/ProgramCard.tsx (3)
48-55: Good: Admin actions consolidated in header via ProgramActions.Conditional render matches access rules and aligns with the three‑dot menu requirement.
26-31: Good: Status updates routed through useUpdateProgramStatus with scoped refetch.Refetching
GET_PROGRAM_AND_MODULESfor the specific program is appropriate.
93-96: Good: Single bottom CTA (“Preview” for admin, “View Details” for user).Matches the spec to keep the card footer simple.
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (2)
54-59: Good: Centralized status mutation via useUpdateProgramStatus with scoped refetch.Admin‑gate lives in the hook; refetch targets the same query used by the page.
109-123: Good: DetailsCard wired with programKey and hook‑backed setStatus.Props align with the expanded DetailsCard API; status control remains admin‑gated.
| <span> | ||
| {program.startedAt && program.endedAt | ||
| ? `${formatDate(program.startedAt)} – ${formatDate(program.endedAt)}` | ||
| : program.startedAt |
There was a problem hiding this comment.
@Rajgupta36 Are you still working on resolving these? I don't think you're done... 🤔
There are duplicate blocks right now and unclosed element that causes page to crash. I tried to update some of it to at least load the page, but that's not ok:
Also the page misses faEdit icon import.
There was a problem hiding this comment.
I am working on resolving it @kasya
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
frontend/src/components/ProgramCard.tsx (6)
25-35: Status change may not reflect on list immediately — add list query to refetch or update cache.Only GET_PROGRAM_AND_MODULES is refetched; the card/list backed by GET_MY_PROGRAMS may remain stale until a page-level refresh.
Apply:
- import { GET_PROGRAM_AND_MODULES } from 'server/queries/programsQueries' + import { GET_PROGRAM_AND_MODULES, GET_MY_PROGRAMS } from 'server/queries/programsQueries' @@ const { updateProgramStatus } = useUpdateProgramStatus({ @@ - refetchQueries: [ + refetchQueries: [ { query: GET_PROGRAM_AND_MODULES, variables: { programKey: program.key }, }, + { query: GET_MY_PROGRAMS }, ], })If GET_MY_PROGRAMS needs variables, include them accordingly. Alternatively, perform an optimistic update or writeQuery on mutation success.
63-70: Confirm ProgramActions.setStatus contract (setter vs async updater).You’re passing an async mutation function as setStatus. If ProgramActions expected a React state setter, selection highlighting may not update instantly. Consider renaming the prop to onChangeStatus or ensure ProgramActions awaits the promise and handles pending state.
71-78: Handle endedAt-only case and avoid timezone drift in dates.Ended-only programs currently render “No dates set”. Also, toLocaleDateString without a fixed timeZone can off-by-one in some locales.
Apply:
- {program.startedAt && program.endedAt - ? `${formatDate(program.startedAt)} – ${formatDate(program.endedAt)}` - : program.startedAt - ? `Started: ${formatDate(program.startedAt)}` - : 'No dates set'} + {program.startedAt && program.endedAt + ? `${formatDate(program.startedAt)} – ${formatDate(program.endedAt)}` + : program.startedAt + ? `Started: ${formatDate(program.startedAt)}` + : program.endedAt + ? `Ended: ${formatDate(program.endedAt)}` + : 'No dates set'}Additionally (outside this range), fix the formatter:
- const formatDate = (d: string) => - new Date(d).toLocaleDateString('en-US', { + const formatDate = (d: string) => + new Date(d).toLocaleDateString('en-US', { month: 'short', day: 'numeric', year: 'numeric', - }) + timeZone: 'UTC', + })
81-86: Guard against undefined userRole to avoid “Undefined” badge; tighten typing.Render the badge only when userRole exists and narrow the roleClass index.
Apply:
- {accessLevel === 'admin' && ( - <span - className={`rounded-full px-2 py-1 text-xs font-medium capitalize ${ - roleClass[program.userRole] ?? roleClass.default - }`} - > - {program.userRole} - </span> - )} + {accessLevel === 'admin' && program.userRole && ( + <span + className={`rounded-full px-2 py-1 text-xs font-medium capitalize ${ + roleClass[program.userRole as keyof typeof roleClass] ?? roleClass.default + }`} + > + {program.userRole} + </span> + )}
37-41: AI summary mismatch: mentor style still present.Summary claims mentor mapping was removed from roleClass; code still includes it. Update the summary or remove the unused mapping if truly not needed.
97-100: Add accessible label to the action button.Improves a11y and makes tests more resilient.
Apply:
- <ActionButton onClick={() => onView(program.key)}> + <ActionButton + aria-label={accessLevel === 'admin' ? 'Preview program' : 'View program details'} + onClick={() => onView(program.key)} + >
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx(3 hunks)frontend/src/components/ProgramCard.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
Applied to files:
frontend/src/components/ProgramCard.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/components/ProgramCard.tsx
🧬 Code graph analysis (1)
frontend/src/components/ProgramCard.tsx (3)
frontend/src/types/mentorship.ts (1)
Program(22-37)frontend/src/hooks/useUpdateProgramStatus.ts (1)
useUpdateProgramStatus(14-61)frontend/src/server/queries/programsQueries.ts (1)
GET_PROGRAM_AND_MODULES(45-82)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run backend tests
- GitHub Check: Run frontend unit tests
🔇 Additional comments (2)
frontend/src/components/ProgramCard.tsx (2)
4-10: Hook + query imports wired correctly.Using useUpdateProgramStatus and ProgramActions here is appropriate for the new admin actions UI.
11-16: Public API change (isAdmin) — verified: all call-sites updated; no onEdit props remain.
All ProgramCard usages include isAdmin and no occurrences of onEdit were found in pages or unit tests.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
frontend/src/components/ProgramForm.tsx (1)
79-79: Restore accessible focus styles and drop non‑standard class.
focus:outline-hiddenisn’t a Tailwind utility; add explicit, visible focus states (ring + border) for inputs/textarea/select.Example change (apply everywhere shown in the ranges):
- className="... focus:outline-hidden ..." + className="... focus:outline-none focus-visible:ring-2 focus-visible:ring-[#1D7BD7] focus:border-[#1D7BD7] dark:focus-visible:ring-[#1D7BD7] ..."Quick repo sweep:
#!/bin/bash rg -n -C1 -g '!**/dist/**' -S 'focus:outline-hidden'Check Tailwind validity:
Is "focus:outline-hidden" a valid Tailwind CSS utility in v3.x? If not, what’s the recommended way to provide visible focus styles?Also applies to: 94-94, 117-117, 131-131, 146-146, 169-169, 183-183, 201-201
🧹 Nitpick comments (16)
frontend/src/components/ProgramCard.tsx (6)
25-35: Refetch the list query as well (stale status on cards).Only refetching GET_PROGRAM_AND_MODULES may leave the “My Programs” list stale if that view uses GET_MY_PROGRAMS. Either include GET_MY_PROGRAMS in refetchQueries or add a cache update in the mutation.
Apply this diff here if you prefer refetch (adjust import accordingly):
- const { updateProgramStatus } = useUpdateProgramStatus({ + const { updateProgramStatus } = useUpdateProgramStatus({ programKey: program.key, programName: program.name, isAdmin, refetchQueries: [ { query: GET_PROGRAM_AND_MODULES, variables: { programKey: program.key }, }, + // Also refresh the listing so the status badge/menu reflect immediately. + { query: GET_MY_PROGRAMS }, ], })Additional import needed:
+import { GET_MY_PROGRAMS } from 'server/queries/programsQueries'
50-58: Fix non-standard width class and simplify boolean.Tailwind doesn’t include w-88 by default; also the ternary isn’t needed.
- className="w-88" - isDisabled={program.name.length > 50 ? false : true} + className="max-w-[22rem]" + isDisabled={program.name.length <= 50}
63-69: Guard against double status updates; expose loading to ProgramActions.updateProgramStatus doesn’t provide loading state; users can trigger rapid repeated mutations from the menu.
- Extend useUpdateProgramStatus to return loading from useMutation.
- Pass loading to ProgramActions and disable menu items while true.
I can prep a small patch in the hook and ProgramActions if helpful.
71-78: Date rendering: handle invalid/partial dates consistently.If backend occasionally returns null/invalid strings, new Date(d) results vary and can cause SSR/CSR mismatch. Prefer a safe parser or guard.
Example:
- const formatDate = (d: string) => - new Date(d).toLocaleDateString('en-US', { month: 'short', day: 'numeric', year: 'numeric' }) + const formatDate = (d?: string) => { + if (!d) return '—' + const dt = new Date(d) + return Number.isNaN(dt.getTime()) + ? '—' + : dt.toLocaleDateString('en-US', { month: 'short', day: 'numeric', year: 'numeric' }) + }
81-86: Type-safety for roleClass index; avoid rendering “undefined”.program.userRole is string | undefined; indexing roleClass with a generic string is unsafe in TS and may compile-error. Also don’t render an undefined badge.
Minimal change:
- <span - className={`ml-2 rounded-full px-2 py-1 text-xs font-medium capitalize ${ - roleClass[program.userRole] ?? roleClass.default - }`} - > - {program.userRole} - </span> + {program.userRole && ( + <span + className={`ml-2 rounded-full px-2 py-1 text-xs font-medium capitalize ${ + roleClass[(program.userRole as 'admin' | 'mentor')] ?? roleClass.default + }`} + > + {program.userRole} + </span> + )}Better (outside this snippet): define a typed helper
const getRoleClass = (r?: string) => r === 'admin' ? roleClass.admin : r === 'mentor' ? roleClass.mentor : roleClass.defaultand use getRoleClass(program.userRole).
97-100: Add accessible label to the action button.Improves SR usability; text changes with role.
- <ActionButton onClick={() => onView(program.key)}> + <ActionButton onClick={() => onView(program.key)} aria-label={accessLevel === 'admin' ? 'Preview program' : 'View program details'}>frontend/src/components/ProgramForm.tsx (5)
187-203: Avoid controlled/uncontrolled warning; remove duplicate text color.
adminLoginsis optional; default to empty string, and drop the duplicatetext-gray-600 text-gray-800.- value={formData.adminLogins} - className="w-full rounded-lg border border-gray-600 bg-gray-50 px-4 py-3 text-gray-600 text-gray-800 focus:outline-hidden sm:text-base dark:bg-gray-800 dark:text-gray-400" + value={formData.adminLogins ?? ''} + className="w-full rounded-lg border border-gray-600 bg-gray-50 px-4 py-3 text-gray-800 focus:outline-none focus-visible:ring-2 focus-visible:ring-[#1D7BD7] focus:border-[#1D7BD7] sm:text-base dark:bg-gray-800 dark:text-gray-400"
79-79: Border color is heavier than “softer look”.
border-gray-600reads darker; considergray-300(light) and a darker variant in dark mode.- className="... border border-gray-600 ..." + className="... border border-gray-300 dark:border-gray-700 ..."Confirm with design if
gray-300/700matches the intended contrast.Also applies to: 94-94, 117-117, 131-131, 146-146, 169-169, 183-183, 201-201
172-172: Remove empty className.- <div className=""> + <div>
212-215: Use Next.js router for consistency (matches ModuleForm).- onClick={() => history.back()} + onClick={() => router.back()}And add:
+ import { useRouter } from 'next/navigation' ... const ProgramForm = ({ ... }: ProgramFormProps) => { + const router = useRouter()
121-133: Optional: basic date constraint.Prevent selecting an end date before start date with native HTML constraint.
<input id="program-end-date" type="date" name="endedAt" value={formData.endedAt} onChange={handleInputChange} required + min={formData.startedAt}frontend/src/components/ModuleForm.tsx (5)
171-171: Invalid Tailwind color tokenbg-gray-30.Tailwind uses 50/100/200…;
30won’t apply. Usebg-gray-50for parity with other fields.- className="bg-gray-30 w-full rounded-lg border border-gray-600 px-4 py-3 text-gray-800 focus:outline-hidden dark:bg-gray-800 dark:text-gray-200" + className="w-full rounded-lg border border-gray-600 bg-gray-50 px-4 py-3 text-gray-800 focus:outline-none focus-visible:ring-2 focus-visible:ring-[#1D7BD7] focus:border-[#1D7BD7] dark:bg-gray-800 dark:text-gray-200"
281-284: Don’t throw inside debounced async; log and surface error instead.
throw new Error('...', err)drops the original error and can cause unhandled promise rejections. Prefer logging and setting UI error.- } catch (err) { + } catch (err) { setRawResults([]) setShowSuggestions(false) - throw new Error('Error fetching suggestions:', err) + console.error('Error fetching suggestions', err) + setError('Failed to load projects. Please try again.') }If you want an error cause:
- throw new Error('Error fetching suggestions:', err) + throw new Error('Error fetching suggestions', { cause: err as unknown })…but avoid throwing here since the result isn’t awaited.
358-372: A11y: add listbox/option roles to suggestions; consider keyboard nav.- <div className="absolute z-10 mt-1 w-full rounded-md border ..."> + <div role="listbox" aria-label="Project suggestions" + className="absolute z-10 mt-1 w-full rounded-md border ..."> {suggestions.map((project) => ( - <button + <button role="option" aria-selected="false" key={project.id} type="button" ...Follow‑up: arrow‑key navigation and
aria-activedescendantwould further improve UX.
343-356: A11y: link error text to input.- <input + <input id="projectSelector" ... + aria-invalid={!!error} + aria-describedby={error ? 'projectSelector-error' : undefined} /> - {error && <p className="mt-1 text-sm text-red-600">{error}</p>} + {error && <p id="projectSelector-error" className="mt-1 text-sm text-red-600">{error}</p>}Also applies to: 374-374
121-132: Optional: native date constraint.Keep end date after start date with an HTML
min.<input type="date" name="endedAt" id="endedAt" value={formData.endedAt} onChange={handleInputChange} required + min={formData.startedAt}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
frontend/src/components/ModuleForm.tsx(14 hunks)frontend/src/components/ProgramCard.tsx(3 hunks)frontend/src/components/ProgramForm.tsx(10 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
📚 Learning: 2025-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.tsxfrontend/src/components/ProgramForm.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/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/components/ProgramForm.tsxfrontend/src/components/ProgramCard.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/components/ProgramCard.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/components/ProgramCard.tsx
🧬 Code graph analysis (1)
frontend/src/components/ProgramCard.tsx (3)
frontend/src/types/mentorship.ts (1)
Program(22-37)frontend/src/hooks/useUpdateProgramStatus.ts (1)
useUpdateProgramStatus(14-61)frontend/src/server/queries/programsQueries.ts (1)
GET_PROGRAM_AND_MODULES(45-82)
⏰ 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 backend tests
- GitHub Check: Run frontend unit tests
- GitHub Check: Run frontend e2e tests
🔇 Additional comments (5)
frontend/src/components/ProgramCard.tsx (3)
4-10: Imports look good; confirm path aliases resolve in tsconfig.If baseUrl/paths aren’t configured, these absolute imports will break in Jest and Storybook.
15-18: Public API change: added isAdmin prop.LGTM. Ensure all callsites/tests pass isAdmin to avoid runtime conditionals evaluating incorrectly.
63-69: UI/UX goal met: three-dot menu + single primary action.ProgramActions in header for admins and single “Preview/View Details” button below matches the PR objective to consolidate actions. Nice cleanup.
Also applies to: 97-100
frontend/src/components/ProgramForm.tsx (1)
67-67: Styling alignment looks good.Borders standardized to 1px and labels to semibold; sections read cleaner and match the “softer form” direction.
Also applies to: 101-101, 209-209
frontend/src/components/ModuleForm.tsx (1)
68-68: Cohesive visual pass.Form sections, borders, and labels are now consistent with ProgramForm; looks good.
Also applies to: 105-105, 220-220
arkid15r
force-pushed
the
feature/mentorship-portal
branch
from
c02929e to
b87a4c1
Compare
* 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]>
arkid15r
force-pushed
the
feature/mentorship-portal
branch
from
b87a4c1 to
ec16079
Compare
* 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 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 (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]> * 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]>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/__tests__/unit/components/ProgramCard.test.tsx (2)
228-240: Vacuous assertion: searching for a CSS class via text will always pass.
screen.queryByText(/bg-/)looks for visible text "bg-" and doesn’t validate badge absence.Prefer one of:
- Add a test id to the badge and assert absence:
- expect(screen.queryByText(/bg-/)).not.toBeInTheDocument() + expect(screen.queryByTestId('user-role-badge')).not.toBeInTheDocument()
- Or use the container to ensure no badge-like element exists (brittle but no component change):
- expect(screen.queryByText(/bg-/)).not.toBeInTheDocument() + const { container } = render( /* same props */ ) + expect(container.querySelector('[class*="bg-"][class*="text-"]')).toBeNull()If you opt for the first, please add
data-testid="user-role-badge"to the badge in ProgramCard.
49-51: Pin TZ in Jest setup and make test dates TZ-robustjest.setup.ts exists (it mocks next/navigation) but does not set process.env.TZ — add a TZ pin and make fixtures timezone-robust to prevent CI flakes.
Actions:
- Add at top of jest.setup.ts:
+// jest.setup.ts (top) +process.env.TZ = 'UTC'
- Update the test fixture timestamps (frontend/tests/unit/components/ProgramCard.test.tsx — around lines 49–51) from midnight/23:59:59Z to midday UTC:
- startedAt: '2024-01-01T00:00:00Z', - endedAt: '2024-12-31T23:59:59Z', + startedAt: '2024-01-01T12:00:00Z', + endedAt: '2024-12-31T12:00:00Z',Alternative: mock the component’s date formatter in tests for deterministic output.
🧹 Nitpick comments (6)
frontend/__tests__/unit/components/ProgramCard.test.tsx (6)
149-157: Assert the admin actions menu is absent for user access.Strengthen the user-view test by asserting the 3‑dot actions button is not rendered.
render( <ProgramCard isAdmin={false} program={baseMockProgram} onView={mockOnView} accessLevel="user" /> ) expect(screen.getByText('View Details')).toBeInTheDocument() expect(screen.queryByText('Preview')).not.toBeInTheDocument() expect(screen.queryByText('Edit')).not.toBeInTheDocument() + expect(screen.queryByTestId('program-actions-button')).not.toBeInTheDocument()
257-258: Duplicate expectation — remove redundancy.Back-to-back identical
getByText(longDescription)assertions add no value.- expect(screen.getByText(longDescription)).toBeInTheDocument() - expect(screen.getByText(longDescription)).toBeInTheDocument() + expect(screen.getByText(longDescription)).toBeInTheDocument()
111-111: Prefer userEvent over fireEvent for closer-to-user interactions.Dropdowns/menus often rely on pointer/keyboard semantics; userEvent is more representative and reduces flakiness.
-import { fireEvent, render, screen } from '@testing-library/react' +import { render, screen } from '@testing-library/react' +import userEvent from '@testing-library/user-event' ... - fireEvent.click(previewButton!) + await userEvent.click(previewButton!) ... - fireEvent.click(screen.getByTestId('program-actions-button')) - fireEvent.click(screen.getByText('Edit Program')) + await userEvent.click(screen.getByTestId('program-actions-button')) + await userEvent.click(screen.getByText('Edit Program')) ... - fireEvent.click(viewButton!) + await userEvent.click(viewButton!) ... - fireEvent.click(screen.getByTestId('program-actions-button')) + await userEvent.click(screen.getByTestId('program-actions-button'))Note: mark affected tests
async.Also applies to: 128-129, 175-175, 427-428
417-429: Add assertions for all admin menu items required by #2196.PR goal: the 3‑dot menu must include all actions (Edit program, Manage program, etc.). Only "Edit Program" is asserted here.
Extend the test to open the menu and assert presence of the full set (labels as implemented), e.g., "Manage program", "Put into draft", etc.
389-401: Regression guard: ensure no standalone “Edit” button remains.To enforce the removal of the old Edit button, add a negative assertion in both admin and user contexts.
Example (admin block after render):
expect(screen.getByTestId('program-actions-button')).toBeInTheDocument() +expect(screen.queryByRole('button', { name: /^Edit$/i })).not.toBeInTheDocument()You already assert absence of "Edit" for user; mirror it for admin.
Also applies to: 149-157
2-2: Minor: drop faEdit from the test if not asserted.
faEditis only used to label a hypotheticalicon-editin the mock, but no test asserts it. Consider simplifying to reduce noise.-import { faEdit } from '@fortawesome/free-solid-svg-icons' ... - data-testid={`icon-${icon === faEye ? 'eye' : icon === faEdit ? 'edit' : 'unknown'}`} + data-testid={`icon-${icon === faEye ? 'eye' : 'unknown'}`}If other suites rely on
icon-edit, ignore this.Also applies to: 10-17
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/__tests__/unit/components/ProgramCard.test.tsx(11 hunks)frontend/src/app/my/mentorship/programs/[programKey]/page.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
Applied to files:
frontend/__tests__/unit/components/ProgramCard.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/ProgramCard.test.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/__tests__/unit/components/ProgramCard.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). (3)
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run frontend unit tests
- GitHub Check: Run backend tests
🔇 Additional comments (2)
frontend/__tests__/unit/components/ProgramCard.test.tsx (2)
86-99: Good fix on isAdmin usage in user contexts.Previous feedback about user-view tests incorrectly passing
isAdmin={true}is resolved; user sections now passisAdmin={false}consistently.Also applies to: 135-144
116-132: Unstable router mock — assert against the same mock instanceI couldn't verify jest.setup.ts in this environment; ensure one of the fixes below is applied.
- Ensure your global Jest setup returns a stable, shared router object (single
pushmock).- Or locally stub
next/navigationin the test with a shared mock:+const pushMock = jest.fn() +jest.mock('next/navigation', () => ({ + useRouter: () => ({ push: pushMock }), +})) ... - const router = useRouter() ... - expect(router.push).toHaveBeenCalledWith('/my/mentorship/programs/test-program/edit') + expect(pushMock).toHaveBeenCalledWith('/my/mentorship/programs/test-program/edit')
| type="button" | ||
| onClick={() => setDropdownOpen((prev) => !prev)} | ||
| className="rounded px-4 py-2 hover:bg-gray-200 dark:hover:bg-gray-700" | ||
| className="rounded hover:bg-gray-200 dark:hover:bg-gray-700" |
There was a problem hiding this comment.
@Rajgupta36 I believe the hover effect should be not on the button but on the icon itself. If you look closely, right now on hover the background of the icon changes color weirdly, making it look as if those three dots gets stretched.. or have a shadow or something.
I do believe adding the hover effect on the icon itself would work better here:
<FontAwesomeIcon icon={faEllipsisV} className="text-gray-400 hover:text-gray-500 dark:hover:text-gray-200" />
Maybe something like this?
arkid15r
force-pushed
the
feature/mentorship-portal
branch
from
300ee4a to
bf2a2bf
Compare
|
@Rajgupta36 could you resolve these conflicts again? Thanks! |
|
* 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 #2196
Description
Media