Created Module and program pages#1717
Conversation
Summary by CodeRabbit
WalkthroughIntroduces mentorship program/module keys with slugging and uniqueness, extensive Strawberry GraphQL API for mentorship (nodes, queries, mutations), Algolia indexing for programs with cache clearing via signals, OWASP project queries additions, GraphQL schema wiring, and a comprehensive frontend: authentication role flags, admin/user mentorship pages, forms, components, queries/mutations, types/utils, middleware, and unit tests. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90+ minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 31
🔭 Outside diff range comments (1)
frontend/src/app/api/auth/[...nextauth]/route.ts (1)
44-50: Consider eliminating duplicate GraphQL calls.The same
GITHUB_AUTH_MUTATIONis called in bothsignInandjwtcallbacks, which is redundant and could cause performance issues.Consider removing the GraphQL call from the
signIncallback since thejwtcallback already handles this:async signIn({ account }) { if (account?.provider === 'github' && account.access_token) { - try { - const { data } = await apolloClient.mutate({ - mutation: GITHUB_AUTH_MUTATION, - variables: { - accessToken: account.access_token, - }, - }) - if (!data?.githubAuth?.authUser) throw new Error('User sync failed') - return true - // eslint-disable-next-line @typescript-eslint/no-unused-vars - } catch (error) { - throw new Error('GitHub authentication failed') - } + return true } return true },Also applies to: 66-79
🧹 Nitpick comments (26)
frontend/src/utils/constants.ts (1)
15-18: Fix text capitalization for consistency.The navigation link text should be capitalized to match the existing pattern. All other navigation items use proper capitalization (Community, Projects, Contribute, About).
{ - text: 'mentorship', + text: 'Mentorship', href: '/mentorship/programs', },frontend/src/components/Card.tsx (1)
79-86: Consider adding error handling for invalid dates.The timeline feature implementation looks good, but consider adding error handling for invalid dates to prevent potential runtime errors.
{/* Timeline Section (Optional) */} {timeline?.start && timeline?.end && ( <div className="mt-2 text-sm text-gray-500 dark:text-gray-400"> <FontAwesomeIcon icon={faCalendarDays} className="mr-1 inline-block h-4 w-4" /> - {new Date(timeline.start).toLocaleDateString()} →{' '} - {new Date(timeline.end).toLocaleDateString()} + {new Date(timeline.start).toLocaleDateString()} →{' '} + {new Date(timeline.end).toLocaleDateString()} </div> )}Or add validation:
+ {timeline?.start && timeline?.end && !isNaN(new Date(timeline.start).getTime()) && !isNaN(new Date(timeline.end).getTime()) && ( - {timeline?.start && timeline?.end && (backend/apps/nest/graphql/mutations/user.py (1)
66-69: Consider adding logging for role changes.When a user's role is updated, it would be helpful to log this change for audit purposes.
if not created and auth_user.role != role: + logger.info( + "User role changed: %s (%s -> %s)", + auth_user.username, + auth_user.role, + role + ) auth_user.role = role auth_user.save(update_fields=["role"])frontend/src/utils/helpers/apolloClient.ts (1)
40-42: Clarify variable naming for better understanding.The variable name
apolloClientPromiseis misleading sincecreateApolloClient()returns anApolloClientinstance ornull, not a Promise.-const apolloClientPromise = createApolloClient() +const apolloClient = createApolloClient() -export default apolloClientPromise +export default apolloClientIf the intention is to make this async in the future, consider documenting that or implementing it now.
frontend/src/app/mentorship/programs/create/page.tsx (1)
59-70: Optimize array processing for better performance.The comma-separated string processing could be more efficient and handle edge cases better.
- tags: formData.tags - .split(',') - .map((t) => t.trim()) - .filter(Boolean), - domains: formData.domains - .split(',') - .map((d) => d.trim()) - .filter(Boolean), - adminLogins: formData.adminLogins - .split(',') - .map((login) => login.trim()) - .filter(Boolean), + tags: formData.tags.split(',').map(t => t.trim()).filter(Boolean), + domains: formData.domains.split(',').map(d => d.trim()).filter(Boolean), + adminLogins: formData.adminLogins.split(',').map(login => login.trim()).filter(Boolean),Consider extracting this logic into a utility function since it's reused multiple times.
backend/apps/mentorship/graphql/queries/program.py (2)
16-31: Optimize database queries and add input validation.The query filtering logic is good, but consider adding input validation and potentially using Q objects for more complex filters.
@strawberry.field def all_programs( self, page: int = 1, search: str = "", mentor_username: str | None = None, ) -> PaginatedPrograms: """Get all programs with optional pagination, search, and mentor filtering.""" + # Validate pagination parameters + if page < 1: + page = 1 + if search and len(search.strip()) < 2: + search = "" # Avoid inefficient short searches + queryset = Program.objects.prefetch_related("admins__github_user").all()
39-59: Extract ProgramNode creation logic to reduce duplication.The
ProgramNodecreation logic is duplicated betweenall_programsandprogrammethods. Consider extracting it into a helper method.+ def _create_program_node(self, program: Program) -> ProgramNode: + """Create a ProgramNode from a Program instance.""" + return ProgramNode( + id=program.id, + key=program.key, + name=program.name, + description=program.description, + admins=list(program.admins.all()), + domains=program.domains, + ended_at=program.ended_at, + experience_levels=( + [ExperienceLevelEnum(level) for level in program.experience_levels] + if program.experience_levels + else [] + ), + mentees_limit=program.mentees_limit, + started_at=program.started_at, + status=ProgramStatusEnum(program.status), + tags=program.tags, + ) - programs = [ - ProgramNode( - # ... existing fields - ) - for program in programs_qs - ] + programs = [self._create_program_node(program) for program in programs_qs]frontend/src/app/mentorship/programs/[programKey]/edit/page.tsx (1)
99-149: Extract shared form submission logic.The form submission logic is nearly identical to the create page. Consider extracting this into a shared utility function or custom hook.
Create a shared utility function:
// utils/programFormHelpers.ts export const prepareProgramInput = (formData: any, programKey?: string) => { const input = { name: formData.name, description: formData.description, menteesLimit: Number(formData.menteesLimit), startedAt: formData.startedAt, endedAt: formData.endedAt, experienceLevels: formData.experienceLevels, tags: formData.tags.split(',').map(t => t.trim()).filter(Boolean), domains: formData.domains.split(',').map(d => d.trim()).filter(Boolean), adminLogins: formData.adminLogins.split(',').map(login => login.trim()).filter(Boolean), status: formData.status, } if (programKey) { input.key = programKey } return input }This would reduce code duplication and make maintenance easier.
frontend/src/app/mentorship/programs/[programKey]/page.tsx (1)
72-81: Consider using ProgramStatusEnum constant instead of hardcoded string.The publish mutation uses a hardcoded 'PUBLISHED' string instead of the enum value for consistency.
await updateProgram({ variables: { inputData: { key: program.key, name: program.name, - status: 'PUBLISHED', + status: ProgramStatusEnum.PUBLISHED, }, }, })frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx (2)
50-72: Optimize access control logic with early returns.The access control logic runs even when the query is still loading or session is not authenticated. Consider adding early returns for better performance.
useEffect(() => { - if (sessionStatus !== 'authenticated' || queryLoading) return + if (sessionStatus !== 'authenticated') return + if (queryLoading || !data) return const programAdmins = data?.program?.admins || [] const isAdmin = programAdmins.some( (admin: { login: string }) => admin.login === currentUserLogin )
105-118: Consider adding input validation for comma-separated values.The string splitting logic assumes well-formed input. Consider adding validation to handle edge cases.
domains: formData.domains .split(',') - .map((d: string) => d.trim()) - .filter(Boolean), + .map((d: string) => d.trim()) + .filter((d: string) => d.length > 0),frontend/src/components/ModuleCard.tsx (2)
53-55: Consider using Next.js router.push with relative path construction.The current navigation depends on
window.location.pathnamewhich may not be reliable in all environments.const handleClick = () => { - router.push(`${window.location.pathname}/modules/${details.key}`) + router.push(`modules/${details.key}`) }
78-87: Add input validation to the duration calculation function.The function doesn't validate that the dates are valid or that end date is after start date.
export const getSimpleDuration = (start: string, end: string): string => { const startDate = new Date(start) const endDate = new Date(end) + if (isNaN(startDate.getTime()) || isNaN(endDate.getTime())) { + return 'Invalid dates' + } const ms = endDate.getTime() - startDate.getTime() + if (ms < 0) { + return 'Invalid duration' + } + const days = Math.floor(ms / (1000 * 60 * 60 * 24)) const weeks = Math.ceil(days / 7) return `${weeks} week${weeks !== 1 ? 's' : ''}` }frontend/src/app/mentorship/programs/[programKey]/createModule/page.tsx (3)
37-37: Consider adding null safety for session data.The type assertion could fail if session data doesn't match expected structure.
- const currentUserLogin = (sessionData as SessionWithRole)?.user?.username || '' + const currentUserLogin = (sessionData as SessionWithRole | null)?.user?.username || ''
76-90: Consider adding validation for array parsing.The comma-separated value parsing could benefit from additional validation to handle edge cases.
domains: formData.domains .split(',') .map((d) => d.trim()) - .filter(Boolean), + .filter((d) => d.length > 0), tags: formData.tags .split(',') .map((t) => t.trim()) - .filter(Boolean), + .filter((t) => t.length > 0),
94-103: Improve error handling specificity.The error handling could be more specific about different types of errors.
} catch (err) { + const errorMessage = err instanceof Error ? err.message : 'Something went wrong while creating the module.' addToast({ title: 'Creation Failed', - description: err.message || 'Something went wrong while creating the module.', + description: errorMessage, color: 'danger', variant: 'solid', timeout: 3000, shouldShowTimeoutProgress: true, }) }frontend/src/app/mentorship/programs/page.tsx (2)
44-55: Consider optimizing URL update logic.The URL synchronization could be optimized to avoid unnecessary comparisons.
const params = new URLSearchParams() if (searchQuery) params.set('q', searchQuery) if (page > 1) params.set('page', page.toString()) const queryString = params.toString() const newUrl = queryString ? `?${queryString}` : window.location.pathname - if (window.location.search !== (queryString ? `?${queryString}` : '')) { + const currentSearch = window.location.search + const newSearch = queryString ? `?${queryString}` : '' + if (currentSearch !== newSearch) { router.push(newUrl, { scroll: false }) }
128-131: Consider preserving scroll position on page change.The scroll behavior on page change could be smoother.
onPageChange={(p) => { setPage(p) - window.scrollTo({ top: 0, behavior: 'auto' }) + window.scrollTo({ top: 0, behavior: 'smooth' }) }}backend/apps/mentorship/graphql/nodes/program.py (1)
21-26: Consider making datetime fields optional for better flexibility.The
ended_atandstarted_atfields are typed as required but programs might not always have defined end dates.- ended_at: datetime + ended_at: datetime | None = None - started_at: datetime + started_at: datetime | None = Nonefrontend/src/types/program.ts (1)
23-23: Consider making menteesLimit nullable for consistency.The backend schema suggests this field can be null, but it's typed as required number here.
- menteesLimit: number + menteesLimit: number | nullfrontend/src/components/mainmoduleCard.tsx (2)
219-219: Use router navigation instead of history.back().For consistency with Next.js routing patterns, use router navigation.
+ import { useRouter } from 'next/navigation' + + const router = useRouter() + <button type="button" - onClick={() => history.back()} + onClick={() => router.back()} className="rounded-lg border px-6 py-3 font-medium text-gray-600 hover:bg-gray-50 dark:text-gray-300 dark:hover:bg-gray-700" > Cancel </button>
182-195: Add validation hint for Project ID field.The Project ID field is required but lacks guidance on what constitutes a valid ID.
<label htmlFor="projectId" className="mb-2 block text-sm font-medium"> - Project ID + Project ID * </label> <input id="projectId" type="text" name="projectId" value={formData.projectId} onChange={handleInputChange} - placeholder="Project UUID" + placeholder="Enter the project UUID" required className="w-full rounded-lg border-2 bg-gray-50 px-4 py-3 text-gray-800 focus:outline-none dark:bg-gray-800 dark:text-gray-200" /> + <p className="mt-1 text-xs text-gray-500 dark:text-gray-400"> + Enter a valid project UUID from the available projects. + </p>frontend/src/components/programCard.tsx (2)
276-280: Consider using router navigation instead of history.back().Using
history.back()might not navigate to the expected page if users arrive via direct link or bookmarks. Consider using Next.js router for predictable navigation.+import { useRouter } from 'next/navigation' const ProgramForm = ({ ... }) => { + const router = useRouter() // ... rest of component <button type="button" - onClick={() => history.back()} + onClick={() => router.push('/mentorship/programs')} className="rounded-lg border px-6 py-3 font-medium text-gray-600 hover:bg-gray-50 dark:text-gray-300 dark:hover:bg-gray-700" > Cancel </button>
226-248: Add guidance for comma-separated inputs.The tags and domains fields accept comma-separated values but lack user guidance and validation. Consider adding helper text and input validation.
<label htmlFor="program-tags" className="mb-2 block text-sm font-medium"> Tags + <span className="ml-1 text-xs text-gray-500">(comma-separated)</span> </label> <input id="program-tags" type="text" name="tags" value={formData.tags} onChange={handleInputChange} - placeholder="javascript, react" + placeholder="e.g., javascript, react, typescript" className="w-full rounded-lg border-2 bg-gray-50 px-4 py-3 text-gray-800 focus:outline-none dark:bg-gray-800 dark:text-gray-200" />Apply similar changes to the domains field.
frontend/src/components/CardDetailsPage.tsx (1)
107-116: Use consistent ActionButton component for module editing.The module edit button uses different styling and implementation compared to program buttons. Consider using the same ActionButton component for consistency.
{type === 'module' && ( - <button - className="flex items-center justify-center gap-2 text-nowrap rounded-md border border-[#0D6EFD] bg-transparent px-2 py-2 text-[#0D6EFD] text-blue-600 transition-all hover:bg-[#0D6EFD] hover:text-white dark:border-sky-600 dark:text-sky-600 dark:hover:bg-sky-100" + <ActionButton onClick={() => { router.push(`${window.location.pathname}/edit`) }} > Edit Module - </button> + </ActionButton> )}frontend/src/server/queries/programsQueries.ts (1)
118-131: Consider removing redundant query.The
GET_PROGRAM_ADMIN_DETAILSquery fetches a subset of data already available inGET_PROGRAM_DETAILS. Consider reusing the existing query to reduce code duplication and network requests.If you need a lighter query for performance reasons, consider using GraphQL fragments to share field selections between queries instead of duplicating them.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (48)
backend/apps/mentorship/graphql/mutations/__init__.py(1 hunks)backend/apps/mentorship/graphql/mutations/module.py(1 hunks)backend/apps/mentorship/graphql/mutations/program.py(1 hunks)backend/apps/mentorship/graphql/nodes/__init__.py(1 hunks)backend/apps/mentorship/graphql/nodes/enum.py(1 hunks)backend/apps/mentorship/graphql/nodes/mentor.py(1 hunks)backend/apps/mentorship/graphql/nodes/modules.py(1 hunks)backend/apps/mentorship/graphql/nodes/program.py(1 hunks)backend/apps/mentorship/graphql/queries/__init__.py(1 hunks)backend/apps/mentorship/graphql/queries/module.py(1 hunks)backend/apps/mentorship/graphql/queries/program.py(1 hunks)backend/apps/mentorship/migrations/0002_module_key_program_key.py(1 hunks)backend/apps/mentorship/models/module.py(2 hunks)backend/apps/mentorship/models/program.py(2 hunks)backend/apps/mentorship/utils/user.py(1 hunks)backend/apps/nest/graphql/mutations/user.py(2 hunks)backend/apps/nest/graphql/nodes/user.py(1 hunks)backend/apps/nest/migrations/0002_user_role.py(1 hunks)backend/apps/nest/models/user.py(2 hunks)backend/apps/owasp/models/__init__.py(1 hunks)backend/pyproject.toml(1 hunks)backend/settings/graphql.py(1 hunks)frontend/package.json(2 hunks)frontend/src/app/api/auth/[...nextauth]/route.ts(4 hunks)frontend/src/app/mentorship/programs/[programKey]/createModule/page.tsx(1 hunks)frontend/src/app/mentorship/programs/[programKey]/edit/page.tsx(1 hunks)frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx(1 hunks)frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx(1 hunks)frontend/src/app/mentorship/programs/[programKey]/page.tsx(1 hunks)frontend/src/app/mentorship/programs/create/page.tsx(1 hunks)frontend/src/app/mentorship/programs/page.tsx(1 hunks)frontend/src/components/Card.tsx(3 hunks)frontend/src/components/CardDetailsPage.tsx(9 hunks)frontend/src/components/InfoItem.tsx(1 hunks)frontend/src/components/ModuleCard.tsx(1 hunks)frontend/src/components/SearchPageLayout.tsx(1 hunks)frontend/src/components/mainmoduleCard.tsx(1 hunks)frontend/src/components/programCard.tsx(1 hunks)frontend/src/server/apolloClient.ts(1 hunks)frontend/src/server/queries/moduleQueries.ts(1 hunks)frontend/src/server/queries/programsQueries.ts(1 hunks)frontend/src/types/card.ts(3 hunks)frontend/src/types/contributor.ts(1 hunks)frontend/src/types/program.ts(1 hunks)frontend/src/utils/capitalize.ts(1 hunks)frontend/src/utils/constants.ts(1 hunks)frontend/src/utils/dateFormatter.ts(1 hunks)frontend/src/utils/helpers/apolloClient.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (9)
frontend/src/types/contributor.ts (1)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1703
File: frontend/src/types/healthMetrics.ts:6-24
Timestamp: 2025-07-03T03:03:38.713Z
Learning: In TypeScript, optional properties without explicit type annotations (like `fill?` and `meta?`) implicitly default to the `any` type. This is valid syntax and doesn't cause compilation errors. `fill?` is equivalent to `fill?: any`.
frontend/src/components/Card.tsx (2)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
backend/apps/mentorship/graphql/nodes/enum.py (1)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1676
File: backend/apps/owasp/graphql/filters/project_health_metrics.py:17-22
Timestamp: 2025-06-29T00:41:32.198Z
Learning: In the OWASP Nest codebase, when implementing GraphQL filters that convert string values to enums (like ProjectLevel), do not catch ValueError exceptions for invalid values. Let the errors propagate to provide proper error responses to GraphQL clients rather than silently ignoring invalid input.
backend/apps/nest/graphql/mutations/user.py (1)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1676
File: backend/apps/owasp/graphql/filters/project_health_metrics.py:17-22
Timestamp: 2025-06-29T00:41:32.198Z
Learning: In the OWASP Nest codebase, when implementing GraphQL filters that convert string values to enums (like ProjectLevel), do not catch ValueError exceptions for invalid values. Let the errors propagate to provide proper error responses to GraphQL clients rather than silently ignoring invalid input.
frontend/src/types/card.ts (2)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
frontend/src/components/SearchPageLayout.tsx (1)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1703
File: frontend/src/types/healthMetrics.ts:6-24
Timestamp: 2025-07-03T03:03:38.713Z
Learning: In TypeScript, optional properties without explicit type annotations (like `fill?` and `meta?`) implicitly default to the `any` type. This is valid syntax and doesn't cause compilation errors. `fill?` is equivalent to `fill?: any`.
frontend/src/app/api/auth/[...nextauth]/route.ts (1)
Learnt from: Rajgupta36
PR: OWASP/Nest#1634
File: backend/apps/nest/graphql/mutations/user.py:24-48
Timestamp: 2025-06-18T21:00:33.024Z
Learning: GitHub access tokens use specific prefixes to identify token types: OAuth tokens start with `gho_`, classic personal access tokens with `ghp_`, fine-grained personal access tokens with `github_pat_`, GitHub App user tokens with `ghu_`, GitHub App installation tokens with `ghs_`, and refresh tokens with `ghr_`. This is documented in GitHub's official authentication documentation and engineering blog.
frontend/src/components/ModuleCard.tsx (2)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
frontend/src/components/CardDetailsPage.tsx (2)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
🧬 Code Graph Analysis (17)
backend/apps/owasp/models/__init__.py (3)
backend/apps/owasp/admin.py (1)
project(186-188)backend/apps/github/models/repository.py (1)
project(158-160)backend/apps/owasp/models/project.py (1)
Project(26-354)
backend/apps/mentorship/graphql/mutations/__init__.py (2)
backend/apps/mentorship/graphql/mutations/module.py (1)
ModuleMutation(19-170)backend/apps/mentorship/graphql/mutations/program.py (1)
ProgramMutation(17-151)
backend/apps/nest/graphql/nodes/user.py (1)
backend/apps/nest/models/user.py (1)
User(9-47)
backend/apps/mentorship/models/module.py (3)
backend/apps/mentorship/models/common/experience_level.py (1)
ExperienceLevel(6-25)backend/apps/mentorship/models/common/matching_attributes.py (1)
MatchingAttributes(6-24)backend/apps/mentorship/models/common/start_end_range.py (1)
StartEndRange(6-13)
backend/apps/mentorship/models/program.py (3)
backend/apps/mentorship/models/common/experience_level.py (1)
ExperienceLevel(6-25)backend/apps/mentorship/models/common/matching_attributes.py (1)
MatchingAttributes(6-24)backend/apps/mentorship/models/common/start_end_range.py (1)
StartEndRange(6-13)
frontend/src/server/apolloClient.ts (1)
frontend/src/app/api/auth/[...nextauth]/route.ts (1)
session(84-96)
frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx (6)
frontend/src/types/program.ts (1)
Module(32-44)frontend/src/server/queries/moduleQueries.ts (1)
GET_PROGRAM_ADMINS_AND_MODULES(91-118)frontend/src/app/global-error.tsx (2)
handleAppError(66-86)ErrorDisplay(28-51)frontend/src/utils/capitalize.ts (1)
capitalize(1-4)frontend/src/utils/dateFormatter.ts (1)
formatDate(1-20)frontend/src/components/ModuleCard.tsx (1)
getSimpleDuration(78-87)
backend/apps/nest/graphql/mutations/user.py (2)
backend/apps/nest/models/user.py (1)
User(9-47)backend/apps/github/models/user.py (1)
User(14-134)
frontend/src/utils/helpers/apolloClient.ts (2)
frontend/src/app/api/auth/[...nextauth]/route.ts (1)
session(84-96)frontend/src/utils/utility.ts (1)
getCsrfToken(51-65)
backend/apps/mentorship/graphql/queries/program.py (4)
backend/apps/mentorship/graphql/nodes/enum.py (2)
ExperienceLevelEnum(9-15)ProgramStatusEnum(19-24)backend/apps/mentorship/graphql/nodes/program.py (2)
PaginatedPrograms(30-35)ProgramNode(12-26)backend/apps/mentorship/models/program.py (1)
Program(16-79)frontend/src/types/program.ts (1)
Program(16-30)
frontend/src/app/api/auth/[...nextauth]/route.ts (1)
frontend/src/server/apolloClient.ts (1)
apolloClient(42-43)
backend/apps/mentorship/graphql/nodes/program.py (2)
backend/apps/mentorship/graphql/nodes/enum.py (2)
ExperienceLevelEnum(9-15)ProgramStatusEnum(19-24)backend/apps/mentorship/graphql/nodes/mentor.py (2)
MentorNode(7-25)name(18-20)
frontend/src/components/ModuleCard.tsx (5)
frontend/src/types/program.ts (1)
Module(32-44)frontend/src/components/TruncatedText.tsx (1)
TruncatedText(3-45)frontend/src/components/InfoItem.tsx (1)
TextInfoItem(33-48)frontend/src/utils/capitalize.ts (1)
capitalize(1-4)frontend/src/utils/dateFormatter.ts (1)
formatDate(1-20)
frontend/src/types/program.ts (4)
backend/apps/mentorship/graphql/nodes/enum.py (2)
ExperienceLevelEnum(9-15)ProgramStatusEnum(19-24)backend/apps/mentorship/models/program.py (1)
Program(16-79)frontend/src/types/contributor.ts (1)
Contributor(1-8)backend/apps/mentorship/models/module.py (1)
Module(15-75)
backend/apps/mentorship/graphql/mutations/module.py (5)
backend/apps/mentorship/graphql/nodes/modules.py (3)
CreateModuleInput(31-43)ModuleNode(13-27)UpdateModuleInput(47-59)backend/apps/mentorship/models/mentor.py (1)
Mentor(11-40)backend/apps/mentorship/models/module.py (2)
Module(15-75)save(68-75)backend/apps/mentorship/models/program.py (1)
Program(16-79)backend/apps/mentorship/utils/user.py (1)
get_authenticated_user(10-37)
frontend/src/components/CardDetailsPage.tsx (1)
frontend/src/types/program.ts (1)
SessionWithRole(46-52)
backend/apps/mentorship/graphql/nodes/modules.py (4)
backend/apps/mentorship/graphql/nodes/enum.py (1)
ExperienceLevelEnum(9-15)backend/apps/mentorship/graphql/nodes/mentor.py (2)
MentorNode(7-25)name(18-20)backend/apps/mentorship/graphql/queries/program.py (1)
program(68-92)backend/apps/mentorship/graphql/nodes/program.py (1)
ProgramNode(12-26)
🪛 Flake8 (7.2.0)
backend/apps/owasp/models/__init__.py
[error] 1-1: '.project.Project' imported but unused
(F401)
backend/apps/mentorship/graphql/mutations/__init__.py
[error] 3-3: '.module.ModuleMutation' imported but unused
(F401)
[error] 4-4: '.program.ProgramMutation' imported but unused
(F401)
backend/apps/mentorship/graphql/queries/__init__.py
[error] 3-3: '.module.ModuleQuery' imported but unused
(F401)
[error] 4-4: '.program.ProgramQuery' imported but unused
(F401)
🪛 Biome (1.9.4)
frontend/src/components/CardDetailsPage.tsx
[error] 102-102: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 92-92: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
[error] 96-96: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
[error] 102-102: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
🔇 Additional comments (49)
backend/apps/mentorship/graphql/nodes/__init__.py (1)
1-2: LGTM!Standard Python package initialization file with appropriate comment.
frontend/package.json (2)
25-25: LGTM!Appropriate addition of date picker dependency for the mentorship features.
34-34: LGTM!Good addition of internationalized date handling utility to support the date picker functionality.
backend/apps/owasp/models/__init__.py (1)
1-1: LGTM!This import follows the standard Python pattern of exposing models at the package level. The static analysis warning is a false positive - this is the correct way to make the Project model accessible from the package.
backend/pyproject.toml (1)
134-136: LGTM!Reasonable addition of exception handling linting rules to ignore. These rules (EM101, TRY002, TRY003) can be overly restrictive in Django applications and the flexibility will help maintain code quality without unnecessary noise.
backend/apps/nest/graphql/nodes/user.py (1)
8-8: LGTM! Role field exposure through GraphQL.The addition of the "role" field to the GraphQL node correctly exposes the user role information that was added to the User model, enabling role-based functionality in the frontend.
frontend/src/components/SearchPageLayout.tsx (1)
15-15: LGTM! Flexible empty state handling.Making the
emptyprop optional improves component flexibility, allowing consumers to omit the empty state message when not needed. The usage on line 58 handles undefined values gracefully.backend/apps/mentorship/models/module.py (1)
8-12: LGTM - Import statement formatting improved.The multi-line import style enhances readability and follows Python best practices.
frontend/src/utils/capitalize.ts (1)
3-3: Widespread usage ofcapitalize—please verify behavior changeThe updated implementation lowercases the entire remainder of the string, which is indeed a breaking change from the previous behavior that preserved the original casing. We found the following call sites and imports across the codebase:
• frontend/src/utils/capitalize.ts
• frontend/tests/unit/utils/capitalize.test.ts
• frontend/src/app/page.tsx
• frontend/src/app/projects/[projectKey]/page.tsx
• frontend/src/app/mentorship/programs/[programKey]/page.tsx
• frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx
• frontend/src/components/TopContributorsList.tsx
• frontend/src/components/ModuleCard.tsx
• frontend/src/components/CardDetailsPage.tsx
• frontend/src/components/BreadCrumbs.tsxNext steps:
- Confirm that all affected snapshots/tests have been updated to expect the new lowercase remainder.
- Verify no UI labels or dynamic strings rely on preserving original casing beyond the first character (e.g., acronyms or mixed-case identifiers).
- If any call site requires the old behavior, consider adding a separate helper or flag to preserve casing.
frontend/src/types/contributor.ts (1)
3-3: LGTM - Improved type flexibility.Making
contributionsCountandprojectKeyoptional enhances type safety by accurately reflecting their actual usage patterns across the mentorship components where these fields may not always be present.Also applies to: 6-6
backend/apps/mentorship/models/program.py (1)
9-13: LGTM - Import statement formatting improved.The multi-line import style enhances readability and follows Python best practices.
backend/apps/nest/migrations/0002_user_role.py (1)
1-23: LGTM - Well-formed Django migration.The migration correctly adds a
rolefield with appropriate choices, default value, and constraints. The field design supports the role-based user management system being implemented.frontend/src/components/InfoItem.tsx (1)
33-48: LGTM! Clean and well-structured component.The
TextInfoItemcomponent is well-implemented with proper TypeScript typing, consistent styling, and clear purpose. The component follows React best practices and integrates well with the existing codebase.frontend/src/components/Card.tsx (1)
1-1: LGTM! Good addition of calendar icon import.The import of
faCalendarDaysfrom FontAwesome regular icons is appropriate for the timeline feature.backend/apps/nest/models/user.py (2)
12-15: LGTM! Well-structured role choices definition.The
ROLE_CHOICEStuple is properly defined with clear key-value pairs for "contributor" and "mentor" roles.
33-39: LGTM! Proper Django model field implementation.The
rolefield is correctly implemented with appropriate constraints, default value, and choices. The field configuration follows Django best practices.backend/apps/mentorship/graphql/nodes/enum.py (1)
8-24: Well-implemented enum definitions.The enum implementations follow proper Python enum patterns with consistent lowercase string values and correct Strawberry GraphQL decorators. The structure aligns with GraphQL best practices for strongly typed enum fields.
backend/apps/mentorship/utils/user.py (1)
16-16: Python Version Compatibility ConfirmedThe CI workflows specify
python-version: '3.13'(both in.github/workflows/test-schema.yamlandrun-ci-cd.yaml), which is ≥ 3.9. Sincestr.removeprefixwas introduced in Python 3.9, it’s safe to use here—no changes needed.backend/settings/graphql.py (2)
6-8: Clean GraphQL schema integration.The import structure and module organization follow good practices for separating concerns between different app modules.
12-25: Proper strawberry GraphQL decorators and inheritance.The use of
@strawberry.typedecorators and multi-line inheritance improves readability and follows strawberry conventions correctly.frontend/src/app/mentorship/programs/[programKey]/page.tsx (1)
108-117: LGTM! Well-structured program details formatting.The program details are properly formatted with null safety and appropriate data transformations.
frontend/src/types/card.ts (2)
11-11: LGTM! Proper import of Module type.The Module type import is correctly added to support the new modules property.
26-29: LGTM! Well-structured type extensions.The new optional properties are consistently added following the existing pattern and support the mentorship program functionality.
Also applies to: 44-44, 49-50, 52-53, 60-60, 67-67
frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx (1)
94-126: LGTM! Good error handling and form submission logic.The form submission properly handles errors and transforms the data correctly for the GraphQL mutation.
frontend/src/components/ModuleCard.tsx (1)
58-74: LGTM! Well-structured module item display.The ModuleItem component properly displays module information with appropriate styling and accessibility.
frontend/src/app/api/auth/[...nextauth]/route.ts (1)
88-94: LGTM! Proper session data assignment.The session callback correctly assigns role and username from the JWT token to the session object.
frontend/src/app/mentorship/programs/[programKey]/createModule/page.tsx (1)
42-64: LGTM! Excellent access control implementation.The access control logic properly verifies admin privileges before allowing module creation, with appropriate user feedback and navigation.
frontend/src/app/mentorship/programs/page.tsx (2)
31-42: LGTM! Proper debouncing implementation.The debounce setup with cleanup is correctly implemented to optimize search performance.
142-157: LGTM! Excellent role-based UI controls.The mentor role checks and button logic are well-implemented with proper state management.
backend/apps/mentorship/graphql/nodes/program.py (3)
11-27: LGTM! Well-structured GraphQL node definition.The ProgramNode type definition is comprehensive and properly typed with appropriate nullable fields.
29-36: LGTM! Proper pagination structure.The PaginatedPrograms type follows standard pagination patterns with current page, total pages, and results.
46-51: GraphQL input correctly mirrors non-nullable model fields
TheCreateProgramInputrequiresstarted_atandended_at, matching their definitions inStartEndRange(non-nullableDateTimeFields). Only if your intention is to allow draft programs without dates should you:
- update
backend/apps/mentorship/models/common/start_end_range.pytoDateTimeField(null=True, blank=True)- make
started_atand/orended_atoptional inCreateProgramInputOtherwise, no changes are needed.
frontend/src/types/program.ts (3)
2-7: LGTM! Enum values match backend schema.The ExperienceLevelEnum values are consistent with the backend enum definition.
9-13: LGTM! Program status enum is properly defined.The ProgramStatusEnum values match the backend schema and provide type safety.
46-52: LGTM! Session type extension is well-defined.The SessionWithRole type properly extends session data with role and username for access control.
frontend/src/components/mainmoduleCard.tsx (3)
25-30: LGTM! Well-defined experience level options.The experience level constants are properly structured and match the backend enum values.
48-92: LGTM! Excellent accessibility and responsive design.The form structure uses semantic HTML with proper labels, ARIA attributes, and responsive grid layout.
196-211: LGTM! Proper conditional rendering for edit mode.The mentor login field is correctly shown only in edit mode, preventing confusion during module creation.
backend/apps/mentorship/graphql/nodes/modules.py (2)
12-28: Well-structured GraphQL node definition!The
ModuleNodetype is properly defined with appropriate field types and optionality. The required fields (name, experience_level, mentors, dates) ensure data consistency while optional fields provide flexibility.
30-44: Verify the project_id requirement inconsistency.The
project_idfield is required inCreateModuleInput(line 41) but optional inModuleNode(line 25). Please verify if this is intentional - modules might be created without projects after initial creation, or this could be an oversight.frontend/src/components/CardDetailsPage.tsx (1)
251-332: Well-implemented program and module sections!The new sections for tags, domains, admins, mentors, and modules are properly structured with appropriate conditional rendering and responsive layouts. The reuse of existing components like ToggleableList and TopContributorsList maintains consistency.
frontend/src/server/queries/programsQueries.ts (1)
75-140: Well-structured mutations with good separation of concerns!The mutations are properly organized with
UPDATE_PROGRAMfor general updates,CREATE_PROGRAMfor new programs, andUPDATE_PROGRAM_STATUS_MUTATIONfor specific status changes. This separation provides flexibility and follows the single responsibility principle.backend/apps/mentorship/graphql/mutations/module.py (3)
45-50: Fix date validation to ensure end date is strictly after start date.The current validation allows the end date to be equal to the start date, which doesn't make logical sense for a time range.
- if ( - input_data.ended_at is not None - and input_data.started_at is not None - and input_data.ended_at <= input_data.started_at - ): - raise Exception("End date must be after start date") + if ( + input_data.ended_at is not None + and input_data.started_at is not None + and input_data.ended_at < input_data.started_at + ): + raise Exception("End date must be after start date")Likely an incorrect or invalid review comment.
77-90: Include project_id in the returned ModuleNode.The
ModuleNodetype includes aproject_idfield, but it's not being set in the return value. This is inconsistent with theget_modulequery which correctly includes it.return ModuleNode( id=module.id, key=module.key, name=module.name, description=module.description, domains=module.domains, ended_at=module.ended_at, experience_level=module.experience_level, mentors=list(module.mentors.all()), program=module.program, - project_id=module.project.id if module.project else None, + project_id=module.project_id, started_at=module.started_at, tags=module.tags, )Likely an incorrect or invalid review comment.
111-116: Fix date validation to ensure end date is strictly after start date.Same issue as in the create_module method - the validation should use
<instead of<=.- if ( - input_data.ended_at is not None - and input_data.started_at is not None - and input_data.ended_at <= input_data.started_at - ): - raise Exception("End date must be after start date") + if ( + input_data.ended_at is not None + and input_data.started_at is not None + and input_data.ended_at < input_data.started_at + ): + raise Exception("End date must be after start date")Likely an incorrect or invalid review comment.
backend/apps/mentorship/graphql/queries/module.py (1)
76-101: LGTM!The
get_modulemethod is well-implemented with proper error handling, query optimization, and correctly includes all fields in the returnedModuleNode.backend/apps/mentorship/graphql/mutations/program.py (3)
125-136: Excellent use of case-insensitive GitHub user lookup!The implementation correctly uses
login__iexactfor case-insensitive matching, which is consistent throughout this file and prevents issues with username case variations.
29-30: Fix date validation to ensure end date is strictly after start date.The validation allows the end date to equal the start date, which is inconsistent with the module mutations and doesn't make sense for a time range.
- if input_data.ended_at <= input_data.started_at: - raise Exception("End date must be after start date") + if input_data.ended_at < input_data.started_at: + raise Exception("End date must be after start date")Likely an incorrect or invalid review comment.
95-100: Fix date validation to ensure end date is strictly after start date.Same issue as in create_program - should use
<instead of<=.- if ( - input_data.ended_at is not None - and input_data.started_at is not None - and input_data.ended_at <= input_data.started_at - ): - raise Exception("End date must be after start date") + if ( + input_data.ended_at is not None + and input_data.started_at is not None + and input_data.ended_at < input_data.started_at + ): + raise Exception("End date must be after start date")Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
frontend/src/components/CardDetailsPage.tsx (1)
353-373: Add TypeScript type annotation to exported component.The SocialLinks component should have proper type annotations for its props.
Apply this diff:
-export const SocialLinks = ({ urls }) => { +interface SocialLinksProps { + urls: string[] +} + +export const SocialLinks = ({ urls }: SocialLinksProps) => {
♻️ Duplicate comments (3)
frontend/src/server/apolloClient.ts (1)
4-4: Replace getSession with getServerSession for server-side usage.This duplicates a previous review comment but remains unaddressed. The
getSessionfunction fromnext-auth/reactis optimized for client-side usage and makes an extra API call when used server-side. For server-side Apollo client setup, usegetServerSessionfromnext-auth/nextinstead.#!/bin/bash # Description: Check if getServerSession is available and properly imported in the codebase # Expected: Find usage patterns and import statements for getServerSession rg -A 3 -B 3 "getServerSession" --type ts --type tsxfrontend/src/utils/helpers/apolloClient.ts (1)
21-24: Improve type safety and error handling.The type casting
(session as SessionWithRole)?.accessTokenbypasses TypeScript's type safety. Consider using proper typing and add error handling for session retrieval.Apply this diff to improve type safety and error handling:
- const session = await getSession() - const accessToken = (session as SessionWithRole)?.accessToken - const csrfToken = await getCsrfToken() + try { + const session = await getSession() as SessionWithRole | null + const accessToken = session?.accessToken + const csrfToken = await getCsrfToken()Also ensure error handling is added at the end of the authLink:
headers: { ...headers, // eslint-disable-next-line @typescript-eslint/naming-convention Authorization: accessToken ? `Bearer ${accessToken}` : '', 'X-CSRFToken': csrfToken || '', }, + } catch (error) { + console.error('Failed to set authentication headers:', error) + return { headers } + }frontend/src/components/CardDetailsPage.tsx (1)
86-108: Simplify admin authorization check and fix React children prop usage.The admin check can be simplified using optional chaining, and the ActionButton components should use JSX children instead of props.
Apply this diff:
-{type === 'program' && - admins?.some( - (admin) => admin.login === ((data as SessionWithRole)?.user?.username as string) - ) && ( +{type === 'program' && + admins?.some( + (admin) => admin.login === (data as SessionWithRole)?.user?.username + ) && ( <div className="flex gap-2"> <ActionButton onClick={() => { router.push(`${window.location.pathname}/edit`) }} - children="Edit Program" - /> + > + Edit Program + </ActionButton> <ActionButton - children=" Add Module" onClick={() => { router.push(`${window.location.pathname}/createModule`) }} - /> - {isDraft && ( - <ActionButton children="Publish" onClick={() => setPublish && setPublish()} /> - )} + > + Add Module + </ActionButton> + {isDraft && ( + <ActionButton onClick={() => setPublish?.()}> + Publish + </ActionButton> + )} </div> )}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
backend/pyproject.toml(1 hunks)frontend/package.json(1 hunks)frontend/src/components/Card.tsx(3 hunks)frontend/src/components/CardDetailsPage.tsx(9 hunks)frontend/src/server/apolloClient.ts(1 hunks)frontend/src/types/card.ts(3 hunks)frontend/src/utils/helpers/apolloClient.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (3)
frontend/src/components/Card.tsx (2)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
frontend/src/types/card.ts (2)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
frontend/src/components/CardDetailsPage.tsx (2)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
🧬 Code Graph Analysis (4)
frontend/src/server/apolloClient.ts (2)
frontend/src/app/api/auth/[...nextauth]/route.ts (1)
session(84-96)frontend/src/types/program.ts (1)
SessionWithRole(46-52)
frontend/src/utils/helpers/apolloClient.ts (3)
frontend/src/app/api/auth/[...nextauth]/route.ts (1)
session(84-96)frontend/src/types/program.ts (1)
SessionWithRole(46-52)frontend/src/utils/utility.ts (1)
getCsrfToken(51-65)
frontend/src/types/card.ts (7)
frontend/src/types/contributor.ts (1)
Contributor(1-8)frontend/src/types/pullRequest.ts (1)
PullRequest(3-10)frontend/src/types/issue.ts (1)
Issue(3-19)frontend/src/types/milestone.ts (1)
Milestone(3-15)frontend/src/types/release.ts (1)
Release(3-14)frontend/src/types/project.ts (1)
RepositoryCardProps(51-61)frontend/src/types/program.ts (1)
Module(32-44)
frontend/src/components/CardDetailsPage.tsx (1)
frontend/src/types/program.ts (1)
SessionWithRole(46-52)
🪛 Biome (1.9.4)
frontend/src/components/CardDetailsPage.tsx
[error] 105-105: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 95-95: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
[error] 99-99: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
[error] 105-105: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
[error] 111-116: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CodeQL (javascript-typescript)
- GitHub Check: CodeQL (python)
🔇 Additional comments (7)
backend/pyproject.toml (1)
135-137: Re-evaluate the blanket suppression of EM101, TRY002 & TRY003Adding these three rules to the global ignore list disables Ruff checks that flag dubious exception-handling patterns (
flake8-errmsgandtryceratops). That can silently permit:• Poorly formatted or non-parameterised error messages (EM101)
• Broad/bareexceptblocks,passinexcept, or forgottenraise … from excchains (TRY002/TRY003)Before merging, confirm that:
- The new mentorship code actually needs these rules suppressed across the entire codebase.
- Per-file/per-line
# noqaor a narrower per-file ignore list wouldn’t be sufficient.Keeping these checks active helps catch real bugs early.
frontend/package.json (1)
24-26: Peer-dependency compatibility verified for@heroui/date-picker@2.3.22No further action required—this release declares wide React support (including React 19):
- react: “>=18 || >=19.0.0-rc.0”
- react-dom: “>=18 || >=19.0.0-rc.0”
frontend/src/server/apolloClient.ts (3)
6-6: Good type safety improvement.The import of
SessionWithRoletype addresses the previous concern about type safety. This is a good improvement over casting toany.
12-13: Session retrieval needs server-side optimization.While the type casting improvement is good, the underlying issue with using
getSessionin a server-side context remains. This will cause performance issues due to unnecessary API calls.Apply this diff to fix the server-side session retrieval:
-import { getSession } from 'next-auth/react' +import { getServerSession } from 'next-auth/next' +import { authOptions } from '../app/api/auth/[...nextauth]/route' -const session = await getSession() +const session = await getServerSession(authOptions)Note: You may need to adjust the authOptions import path based on your NextAuth configuration location.
20-21: Authorization header implementation looks correct.The Bearer token authorization setup is properly implemented with conditional header inclusion and appropriate eslint disable comment for the naming convention.
frontend/src/components/Card.tsx (1)
79-86: LGTM! Timeline section is well-implemented.The timeline display correctly:
- Validates both dates exist before rendering
- Uses locale-aware date formatting
- Has appropriate styling for light/dark modes
frontend/src/types/card.ts (1)
26-29: Type definitions look good.The new properties added to CardProps and DetailsCardProps are well-structured:
- Timeline dates support both string and Date types for flexibility
- All new DetailsCardProps fields are appropriately optional
- Types are properly imported from their respective modules
Also applies to: 44-69
There was a problem hiding this comment.
Actionable comments posted: 11
♻️ Duplicate comments (5)
backend/apps/mentorship/graphql/nodes/mentor.py (1)
7-10: Add type annotation for github_user attribute.The
MentorNodeclass accessesself.github_userin field methods but doesn't declare this attribute with proper typing, which could lead to IDE warnings and reduced type safety.Consider adding the missing type annotation:
+from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from apps.github.models import User as GithubUser + @strawberry.type class MentorNode: """A GraphQL node representing a mentorship mentor.""" id: strawberry.ID + github_user: "GithubUser"backend/apps/mentorship/utils/user.py (2)
25-27: Restrict lookup to login only to avoid false positives.The current query uses both login and name for matching, which can cause false positives since display names are not unique and may change.
- try: - github_user = GithubUser.objects.get(models.Q(login__iexact=login.lower())) - except GithubUser.DoesNotExist as err: - raise Exception("No GithubUser found matching this login or name") from err + try: + github_user = GithubUser.objects.get(login__iexact=login) + except GithubUser.DoesNotExist as err: + raise Exception("No GithubUser found matching this login") from err
17-22: Improve exception handling specificity.The broad exception handling masks specific GitHub API errors that could be handled more appropriately.
try: github = Github(access_token) gh_user = github.get_user() login = gh_user.login - except Exception as err: - raise Exception("GitHub token is invalid or expired") from err + except github.BadCredentialsException as err: + raise Exception("Invalid GitHub credentials") from err + except github.RateLimitExceededException as err: + raise Exception("GitHub API rate limit exceeded") from err + except Exception as err: + raise Exception("GitHub API error occurred") from errfrontend/src/app/mentorship/programs/create/page.tsx (1)
49-97: Add form validation before submission.The form submission lacks validation for required fields, which could result in unnecessary GraphQL calls and poor user experience.
const handleSubmit = async (e: React.FormEvent) => { e.preventDefault() + // Validate required fields + if (!formData.name.trim()) { + addToast({ + title: 'Validation Error', + description: 'Program name is required.', + color: 'danger', + variant: 'solid', + timeout: 3000, + shouldShowTimeoutProgress: true, + }) + return + } + + if (!formData.description.trim()) { + addToast({ + title: 'Validation Error', + description: 'Program description is required.', + color: 'danger', + variant: 'solid', + timeout: 3000, + shouldShowTimeoutProgress: true, + }) + return + } try { const input = { // ... existing code }backend/apps/mentorship/graphql/mutations/module.py (1)
66-70: Fix inconsistent GitHub user lookup in create_module.The GitHub user lookup uses case-sensitive matching while the
update_modulemethod uses case-insensitive matching. This inconsistency could cause issues when different case variations of the same username are used.- try: - github_user = GithubUser.objects.get(login=login) - except GithubUser.DoesNotExist as err: - raise Exception("GitHub username not found.") from err + try: + github_user = GithubUser.objects.get(login__iexact=login.lower()) + except GithubUser.DoesNotExist as err: + raise Exception("GitHub username not found.") from err
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (11)
backend/apps/mentorship/graphql/mutations/module.py(1 hunks)backend/apps/mentorship/graphql/nodes/mentor.py(1 hunks)backend/apps/mentorship/graphql/queries/module.py(1 hunks)backend/apps/mentorship/utils/user.py(1 hunks)backend/apps/nest/graphql/mutations/user.py(2 hunks)frontend/src/app/api/auth/[...nextauth]/route.ts(4 hunks)frontend/src/app/mentorship/programs/create/page.tsx(1 hunks)frontend/src/components/ModuleCard.tsx(1 hunks)frontend/src/server/apolloClient.ts(1 hunks)frontend/src/types/card.ts(3 hunks)frontend/src/utils/dateFormatter.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
frontend/src/components/ModuleCard.tsx (3)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
Learnt from: codic-yeeshu
PR: OWASP/Nest#1444
File: frontend/src/components/NavDropDown.tsx:0-0
Timestamp: 2025-04-30T13:41:20.846Z
Learning: Use React's useId() hook rather than manually generating random strings when creating accessibility identifiers for UI components. This creates stable, unique IDs without causing hydration mismatches.
frontend/src/types/card.ts (2)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
frontend/src/app/api/auth/[...nextauth]/route.ts (1)
Learnt from: Rajgupta36
PR: OWASP/Nest#1634
File: backend/apps/nest/graphql/mutations/user.py:24-48
Timestamp: 2025-06-18T21:00:33.024Z
Learning: GitHub access tokens use specific prefixes to identify token types: OAuth tokens start with `gho_`, classic personal access tokens with `ghp_`, fine-grained personal access tokens with `github_pat_`, GitHub App user tokens with `ghu_`, GitHub App installation tokens with `ghs_`, and refresh tokens with `ghr_`. This is documented in GitHub's official authentication documentation and engineering blog.
🧬 Code Graph Analysis (4)
frontend/src/components/ModuleCard.tsx (5)
frontend/src/types/program.ts (1)
Module(32-44)frontend/src/components/TruncatedText.tsx (1)
TruncatedText(3-45)frontend/src/components/InfoItem.tsx (1)
TextInfoItem(33-48)frontend/src/utils/capitalize.ts (1)
capitalize(1-4)frontend/src/utils/dateFormatter.ts (1)
formatDate(1-20)
frontend/src/types/card.ts (9)
frontend/src/types/chapter.ts (1)
Chapter(3-19)frontend/src/types/healthMetrics.ts (1)
HealthMetricsProps(1-14)frontend/src/types/contributor.ts (1)
Contributor(1-8)frontend/src/types/pullRequest.ts (1)
PullRequest(3-10)frontend/src/types/issue.ts (1)
Issue(3-19)frontend/src/types/milestone.ts (1)
Milestone(3-15)frontend/src/types/release.ts (1)
Release(3-14)frontend/src/types/project.ts (1)
RepositoryCardProps(51-61)frontend/src/types/program.ts (1)
Module(32-44)
backend/apps/nest/graphql/mutations/user.py (2)
backend/apps/nest/models/user.py (1)
User(9-47)backend/apps/github/models/user.py (1)
User(14-134)
frontend/src/app/api/auth/[...nextauth]/route.ts (1)
frontend/src/server/apolloClient.ts (1)
apolloClient(43-44)
🪛 Biome (1.9.4)
frontend/src/components/ModuleCard.tsx
[error] 31-34: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 59-62: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
🔇 Additional comments (10)
frontend/src/utils/dateFormatter.ts (1)
63-70: Implementation correctly handles invalid dates.The function properly validates date inputs and throws an error for invalid dates, following the same pattern as existing functions in the file. The past review comment concern has been addressed.
frontend/src/types/card.ts (2)
11-11: Import statement syntax is correct.The import statement uses proper TypeScript type-only import syntax and is correctly formatted. The past review comment concern has been resolved.
26-29: Well-structured type extensions for mentorship features.The new optional properties added to
CardPropsandDetailsCardPropsproperly support the mentorship program functionality. The types are consistent with the existing patterns and integrate well with the imported types.Also applies to: 44-45, 50-51, 53-55, 61-62, 69-69
frontend/src/components/ModuleCard.tsx (1)
25-27: Key usage is correctly implemented.The component properly uses unique identifiers (
module.key || module.id) as React keys instead of array indices, which addresses the past review comment concern and ensures stable rendering.backend/apps/mentorship/graphql/nodes/mentor.py (1)
15-31: LGTM - Proper defensive programming implemented.The field methods now correctly handle cases where
github_usermight be None, preventing potential AttributeError exceptions. The implementation returns appropriate empty strings as fallbacks.backend/apps/nest/graphql/mutations/user.py (2)
47-53: Excellent performance optimization with database queries.The role determination logic now uses efficient database queries instead of nested loops, significantly improving performance for large datasets. The
Qobject usage for OR conditions is appropriate for checking both login and name fields.
64-66: Good role synchronization logic.The code properly handles role updates for existing users, ensuring that changes in project leadership are reflected in user roles without unnecessary database writes.
frontend/src/app/mentorship/programs/create/page.tsx (1)
32-47: LGTM - Proper access control with early return.The access control logic correctly implements early returns to prevent unauthorized users from accessing the create program functionality. The loading state check and role verification are properly implemented.
frontend/src/app/api/auth/[...nextauth]/route.ts (1)
24-33: Well-structured GraphQL mutation constant.The extracted
GITHUB_AUTH_MUTATIONconstant improves code reusability and maintainability by eliminating duplication between thesignInandjwtcallbacks.backend/apps/mentorship/graphql/queries/module.py (1)
22-26: Excellent query optimization.The use of
select_related("project")andprefetch_related("mentors__github_user")demonstrates good understanding of Django ORM optimization to avoid N+1 query problems.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
frontend/src/app/mentorship/programs/[programKey]/page.tsx (1)
47-56: Consider adding runtime type validation for session data.The admin check relies on the username from session data, but the type casting may not guarantee the username exists at runtime.
- const username = (session as SessionWithRole)?.user?.username + const username = (session as SessionWithRole)?.user?.username || null const isAdmin = useMemo( - () => !!program?.admins?.some((admin) => admin.login === username), + () => !!username && !!program?.admins?.some((admin) => admin.login === username), [program, username] )
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
frontend/src/app/mentorship/programs/[programKey]/page.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). (4)
- GitHub Check: Run frontend unit tests
- GitHub Check: Run backend tests
- GitHub Check: Run frontend e2e tests
- GitHub Check: CodeQL (javascript-typescript)
arkid15r
left a comment
arkid15r
left a comment
There was a problem hiding this comment.
Could you make it CI/CD green before sending for review?
4 participants
Resolve(#1716)
Tasks -
Media
I have fixed publish program logic
https://github.com/user-attachments/assets/ac1cac1b-9b34-4921-b691-dbc652e3683e