Migrate Apollo Client to v4 #2178
Conversation
Summary by CodeRabbit
WalkthroughMigrate frontend to Apollo Client v4 and generated GraphQL artifacts: update dependencies, switch to @apollo/client/react and HttpLink, replace local GET_*/mutation constants with codegen-generated DocumentNodes and enums, relax many TypeScript model fields, add ISO date helper, and update tests to new imports/types. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 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 |
|
Tests will fail. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (10)
frontend/src/components/ModuleForm.tsx (1)
272-285: Fix invalid Error construction and avoid rethrow inside debounced async.
new Error('...', err)is invalid (Error ctor takes a single message). Rethrowing here can cause unhandled promise rejections and noisy UI. Prefer logging + user-facing error state.Apply:
try { const { data } = await client.query({ query: SearchProjectNamesDocument, variables: { query }, }) setRawResults(data.searchProjects || []) setShowSuggestions(true) } catch (err) { setRawResults([]) setShowSuggestions(false) - throw new Error('Error fetching suggestions:', err) + console.error('Error fetching suggestions:', err) + setError('Failed to fetch project names. Please try again.') }frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx (1)
125-137: Bug: Spinner masks the 403 view; check denied before showing the loaderDenied users currently see a spinner because the loader return runs first. Show the denied view first, then the loader only when allowed-but-not-ready.
Apply:
- if (accessStatus === 'checking' || !formData) { - return <LoadingSpinner /> - } - - if (accessStatus === 'denied') { + if (accessStatus === 'denied') { return ( <ErrorDisplay statusCode={403} title="Access Denied" message="You do not have permission to edit this module. You will be redirected." /> ) } + + if (accessStatus === 'checking' || (accessStatus === 'allowed' && !formData)) { + return <LoadingSpinner /> + }frontend/src/app/my/mentorship/page.tsx (2)
139-146: Resolve the TODO by aligning ProgramCard’sprogramprop to generated typesThe prop is currently a TODO; lock this down to the generated node type (or an adapter). Two options:
- Update ProgramCard’s prop to accept
MyProgram(preferred).- Map
pto a view-model that matches ProgramCard’s expected shape.If ProgramCard uses dates, this also depends on fixing the scalar mapping above.
I can adjust ProgramCard’s props and add a simple adapter if needed.
54-62: Useundefinedfor emptysearch; consider disablingnotifyOnNetworkStatusChange
- In
frontend/src/app/my/mentorship/page.tsx(around line 54), change- variables: { search: debouncedQuery, page, limit: 24 }, + variables: { search: debouncedQuery || undefined, page, limit: 24 },- Neither
frontend/src/utils/helpers/apolloClient.tsnorfrontend/src/server/apolloClient.tsdefines a globalnotifyOnNetworkStatusChange, so addto this query’s options to restore v3-style loading behavior.notifyOnNetworkStatusChange: falsefrontend/src/app/my/mentorship/programs/create/page.tsx (1)
76-85: Harden error handling in catch (narrow unknown to ApolloError/Error).Avoid relying on
any/optional chaining onunknown. Show GraphQL/network details when available.Apply:
-import { useMutation } from '@apollo/client/react' +import { useMutation } from '@apollo/client/react' +import { isApolloError } from '@apollo/client' @@ - } catch (err) { + } catch (err: unknown) { + const message = + isApolloError(err) + ? err.message + : err instanceof Error + ? err.message + : 'Unable to complete the requested operation.' addToast({ - description: err?.message || 'Unable to complete the requested operation.', + description: message, title: 'GraphQL Request Failed', timeout: 3000, shouldShowTimeoutProgress: true, color: 'danger', variant: 'solid', })frontend/src/wrappers/provider.tsx (1)
27-31: Guard against null Apollo client to prevent runtime crash.
createApolloClient()can returnnull(see utils/helpers/apolloClient.ts). Passing that intoApolloProviderwill throw.Minimal fix here:
+import { ErrorDisplay } from 'app/global-error' @@ export function Providers({ children }: { children: React.ReactNode }) { return ( <Suspense> <SessionProvider> <HeroUIProvider> <NextThemesProvider attribute="class" defaultTheme="dark"> <ToastProvider /> - <ApolloProvider client={apolloClient}> + {!apolloClient ? ( + <ErrorDisplay + statusCode={500} + title="GraphQL client unavailable" + message="Missing or invalid GRAPHQL_URL configuration." + /> + ) : ( + <ApolloProvider client={apolloClient}> <AppInitializer /> {children} - </ApolloProvider> + </ApolloProvider> + )} </NextThemesProvider> </HeroUIProvider> </SessionProvider> </Suspense> ) }Alternative (also valid): make
createApolloClient()throw instead of returningnulland surface a global error boundary.frontend/src/utils/helpers/apolloClient.ts (1)
7-11: Avoid returning null from factory; fail fast or provide a clear fallback.Returning
nullleaks into consumers (see Providers) and causes crashes if unguarded.Option A (fail fast):
- if (!GRAPHQL_URL) { - const error = new AppError(500, 'Missing GraphQL URL') - handleAppError(error) - return null - } + if (!GRAPHQL_URL) { + const error = new AppError(500, 'Missing GraphQL URL') + handleAppError(error) + throw error + }If you keep returning
null, ensure every consumer guards (see suggested Providers fix).frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx (1)
29-38: Fix: loading can hang forever when module is missing (return 404).If the query resolves with
getModule === nulland no GraphQL error,isLoadingnever flips tofalse, so the spinner never stops and the 404 UI never renders.Apply this minimal patch:
- useEffect(() => { - if (data?.getModule) { - setModule(data.getModule) - setAdmins(data.getProgram.admins) - setIsLoading(false) - } else if (error) { - handleAppError(error) - setIsLoading(false) - } - }, [data, error]) + useEffect(() => { + if (data) { + setModule(data.getModule ?? null) + setAdmins(data.getProgram.admins) + setIsLoading(false) + } else if (error) { + handleAppError(error) + setIsLoading(false) + } + }, [data, error])frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx (1)
29-38: Spinner never stops when module is missing (404 case).You only clear isLoading when data.getModule exists or when error is set. If the query returns successfully with getModule = null, the spinner persists forever.
Apply this diff:
- useEffect(() => { - if (data?.getModule) { - setModule(data.getModule) - setAdmins(data.getProgram.admins) - setIsLoading(false) - } else if (error) { - handleAppError(error) - setIsLoading(false) - } - }, [data, error]) + useEffect(() => { + if (data) { + setModule(data.getModule) + setAdmins(data.getProgram.admins) + setIsLoading(false) + } + if (error) { + handleAppError(error) + setIsLoading(false) + } + }, [data, error])Also applies to: 40-50
frontend/src/app/members/[memberKey]/page.tsx (1)
82-104: Crash when user.bio is empty/undefined.map is called on possibly undefined (result of user?.bio?.split(...)). This throws at runtime.
Apply this diff:
- const formattedBio = user?.bio?.split(' ').map((word, index) => { + const formattedBio = user?.bio?.split(' ')?.map((word, index) => {Optionally, default to null to avoid rendering an empty
:
- const formattedBio = user?.bio?.split(' ')?.map(...) + const formattedBio = user?.bio ? user.bio.split(' ').map(...) : null
🧹 Nitpick comments (52)
frontend/src/components/ModuleForm.tsx (1)
272-279: Optional: force fresh search results and guard against cache staleness.Search suggestions usually benefit from bypassing cache.
Apply:
- const { data } = await client.query({ + const { data } = await client.query({ query: SearchProjectNamesDocument, variables: { query }, + fetchPolicy: 'network-only', })frontend/src/hooks/useDjangoSession.ts (3)
25-28: Nit: coerce shouldSync to a boolean.As written, shouldSync is boolean | string due to accessToken. Coerce to boolean for clarity.
- const shouldSync = - status === 'authenticated' && - (session as ExtendedSession)?.accessToken && - !sessionStorage.getItem(SYNC_STATUS_KEY) + const shouldSync = + status === 'authenticated' && + Boolean((session as ExtendedSession)?.accessToken) && + !sessionStorage.getItem(SYNC_STATUS_KEY)
45-49: Avoid possible TS nullability complaint on session.user.Inside this block session is present, but TS won’t narrow. Assert via cast (or pre-destructure) to keep types happy.
- update({ - user: { - ...session.user, - isOwaspStaff: githubAuth.user?.isOwaspStaff, - }, - }).then(() => {}) + { + const { user } = session as ExtendedSession + update({ + user: { + ...user, + isOwaspStaff: githubAuth.user?.isOwaspStaff, + }, + }).then(() => {}) + }
50-59: Toast may be lost due to signOut redirect.signOut() redirects by default, so the toast likely won’t be visible. Either show the toast before signing out, or use redirect: false.
- } else { - signOut() // Invalidate Next.js session if not ok. - addToast({ + } else { + addToast({ color: 'danger', description: githubAuth?.message, shouldShowTimeoutProgress: true, timeout: 4000, title: 'Authentication Failed', variant: 'bordered', }) + void signOut({ redirect: false }) // Keep user on page so toast is visible. }frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx (3)
37-70: Avoid orphaned redirect timer; add cleanup in effectIf the component unmounts or dependencies change before the timeout fires, the pending redirect persists. Track and clear the timer.
Apply:
useEffect(() => { if (sessionStatus === 'loading' || queryLoading) { return } + let timeoutId: ReturnType<typeof setTimeout> | undefined + if ( queryError || !data?.getProgram || !data?.getModule || sessionStatus === 'unauthenticated' ) { setAccessStatus('denied') return } const currentUserLogin = (sessionData as ExtendedSession)?.user?.login const isAdmin = data.getProgram.admins?.some( (admin: { login: string }) => admin.login === currentUserLogin ) if (isAdmin) { setAccessStatus('allowed') } else { setAccessStatus('denied') addToast({ title: 'Access Denied', description: 'Only program admins can edit modules.', color: 'danger', variant: 'solid', timeout: 4000, }) - setTimeout(() => router.replace(`/my/mentorship/programs/${programKey}`), 1500) + timeoutId = setTimeout( + () => router.replace(`/my/mentorship/programs/${programKey}`), + 1500 + ) } -}, [sessionStatus, sessionData, queryLoading, data, programKey, queryError, router]) + return () => { + if (timeoutId) clearTimeout(timeoutId) + } +}, [sessionStatus, sessionData, queryLoading, data, programKey, queryError, router])
81-86: Default projectName to an empty string to keep inputs controlledPrevents potential uncontrolled-to-controlled input warnings if projectName is undefined.
Apply:
- projectName: m.projectName, + projectName: m.projectName || '',
27-35: Use generated typed documents for this query
Import the code-generatedGetProgramAdminsAndModulesDocumentand its correspondingGetProgramAdminsAndModulesQuery/GetProgramAdminsAndModulesQueryVariablestypes fromsrc/types/__generated__/moduleQueries.generated.ts, then replace the legacyGET_PROGRAM_ADMINS_AND_MODULESconstant in youruseQuerycall with the generated document and supply the proper generics. This will ensure full type safety and consistency with the rest of the codebase.frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (4)
29-30: Avoid duplicating server data in local state; prevent a 404 flicker on refetchBecause you set isRefetching=false before the useQuery data updates, users can briefly hit the 404 branch. Either derive program/modules directly from useQuery or set state from the awaited refetch() result.
Option A (simpler): derive
const { data } = useQuery(...) const program = data?.getProgram ?? null const modules = data?.getProgramModules ?? []Option B (keep state): hydrate from refetch result
const { data: d } = await refetch() setProgram(d?.getProgram ?? null) setModules(d?.getProgramModules ?? [])
33-36: Replace TODO with fully typed mutation via codegenType the mutation to catch server/variable drift at compile time.
- // TODO: update type - const [updateProgram] = useMutation(UPDATE_PROGRAM_STATUS_MUTATION, { - onError: handleAppError, - }) + const [updateProgram] = useMutation< + UpdateProgramStatusMutation, + UpdateProgramStatusMutationVariables + >(UpdateProgramStatusDocument, { onError: handleAppError })Add the import (outside this hunk):
import { UpdateProgramStatusDocument, UpdateProgramStatusMutation, UpdateProgramStatusMutationVariables, } from 'types/__generated__/programsMutations.generated'
81-81: Ensure UI waits for refetch before showing successTo avoid toasts firing before the UI updates, wait for refetch completion or use onQueryUpdated.
await updateProgram({ variables: { inputData: { key: program.key, name: program.name, status: newStatus, }, }, - refetchQueries: [{ query: GetProgramAndModulesDocument, variables: { programKey } }], + refetchQueries: [{ query: GetProgramAndModulesDocument, variables: { programKey } }], + awaitRefetchQueries: true, })In Apollo Client v4, refetchQueries supports passing DocumentNodes and variables; awaitRefetchQueries is still available, and returning a Promise from onQueryUpdated is the preferred alternative. (apollographql.com)
134-135: Don’t assert dates to string; guard or map scalar to avoid runtime “Invalid date”Short-term guard (safer if nulls slip through):
- { label: 'Start Date', value: formatDate(program.startedAt as string) }, - { label: 'End Date', value: formatDate(program.endedAt as string) }, + { label: 'Start Date', value: program.startedAt ? formatDate(program.startedAt as unknown as number | string) : 'N/A' }, + { label: 'End Date', value: program.endedAt ? formatDate(program.endedAt as unknown as number | string) : 'N/A' },Long-term: map your Date/DateTime scalar in codegen (e.g., DateTime: string) so these casts disappear.
frontend/src/app/my/mentorship/page.tsx (1)
34-34: Introduce a named alias for readability and reuseThe inline indexed access type is correct but hard to scan and reuse.
Apply:
-const [programs, setPrograms] = useState<GetMyProgramsQuery['myPrograms']['programs']>([]) +const [programs, setPrograms] = useState<MyProgram[]>([])Add near the imports:
type MyProgram = GetMyProgramsQuery['myPrograms']['programs'][number]frontend/src/app/page.tsx (1)
49-52: Tighten types and loading checks (optional).
- State typing: allow null in the state to match the initial value.
- Loading gate: consider waiting on Algolia data too to avoid map flashing.
Apply:
- const [data, setData] = useState<MainPageData>(null) + const [data, setData] = useState<MainPageData | null>(null) - if (isLoading || !graphQLData || !geoLocData) { + if (isLoading || !graphQLData || geoLocData.length === 0) { return <LoadingSpinner /> }Also applies to: 94-96
frontend/src/components/CardDetailsPage.tsx (1)
38-38: Convert the TODO into a concrete typing task.Two low-risk wins:
- Narrow prop types on SocialLinks.
- Ensure props with
.length(e.g.,healthMetricsData) are typed as arrays.Apply:
-export const SocialLinks = ({ urls }) => { +export const SocialLinks = ({ urls }: { urls: string[] }) => {If you’d like, I can open a follow-up issue to “type tighten” DetailsCardProps and children.
frontend/src/hooks/useLogout.ts (1)
1-1: Import path is correct; consider using useApolloClient for cache ops.Sticking to the React entrypoint is right. For cache clearing, AC v4 recommends using the client directly via
useApolloClient()for imperative work, which keeps the mutation tuple focused on UI state. UseclearStore()(no refetch) orresetStore()(refetch active queries) per your UX. (apollographql.com)Apply:
-import { useMutation } from '@apollo/client/react' +import { useMutation, useApolloClient } from '@apollo/client/react' @@ export const useLogout = () => { - const [logoutUser, { loading, client }] = useMutation(LOGOUT_DJANGO_MUTATION) + const [logoutUser, { loading }] = useMutation(LOGOUT_DJANGO_MUTATION) + const client = useApolloClient() @@ - await client.clearStore() // Removes Apollo cache + await client.clearStore() // Removes Apollo cache (no refetch)Note: per past learnings, some call sites intentionally invoke
logout()andsignOut()both—don’t dedupe unless you plan to adjust those call sites too.frontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx (1)
2-2: Correct migration: import hooks from @apollo/client/react.Matches AC v4 guidance. If/when you switch this page to codegen
TypedDocumentNodes, AC v4 will infer variable/result types from the document directly. (apollographql.com)If you want, I can prep a follow-up patch to replace
CREATE_MODULE/GET_PROGRAM_ADMIN_DETAILSwith generated...Documentartifacts for stronger typing.frontend/src/app/organizations/[organizationKey]/repositories/[repositoryKey]/page.tsx (2)
70-75: Use an anchor for external URLs (accessibility + expected behavior).Next.js Link is intended for internal routes.
- <Link href={repository.url} className="text-blue-400 hover:underline"> - {repository.url} - </Link> + <a + href={repository.url} + className="text-blue-400 hover:underline" + target="_blank" + rel="noopener noreferrer" + > + {repository.url} + </a>
30-41: Tidy useEffect deps.
repositoryKeyisn’t referenced inside the effect; either remove it or includeorganizationKeytoo for symmetry.- }, [data, graphQLRequestError, repositoryKey]) + }, [data, graphQLRequestError])frontend/src/utils/helpers/apolloClient.ts (1)
18-27: Optional: centralize GraphQL error logging with an errorLink.Helps unify toasts/logging instead of per-call try/catch.
+import { onError } from '@apollo/client/link/error' @@ - const authLink = setContext(async (_, { headers }) => { + const authLink = setContext(async (_, { headers }) => { const csrfToken = await getCsrfToken() @@ }) + + const errorLink = onError(({ graphQLErrors, networkError }) => { + if (graphQLErrors?.length || networkError) { + handleAppError(new AppError(500, 'GraphQL Request Failed')) + } + }) @@ - return new ApolloClient({ + return new ApolloClient({ cache: new InMemoryCache(), - link: authLink.concat(httpLink), + link: errorLink.concat(authLink).concat(httpLink), })frontend/src/app/chapters/[chapterKey]/page.tsx (1)
70-70: Address the TODO comment.The TODO indicates a type issue with the
geolocationDataprop.Would you like me to help resolve the type issue with the
geolocationDataprop in theDetailsCardcomponent?frontend/src/app/about/page.tsx (2)
68-73: Reduce state duplication — derive UI straight from query results.
projectMetadataandtopContributorsmirrordataand require effects to sync. Prefer readingprojectMetadataResponse?.projectandtopContributorsResponse?.topContributorsdirectly to avoid extra renders and stale state.If you want, I can push a minimal diff that inlines these and drops both effects.
Also applies to: 75-94, 101-113
273-275: Tighten type:useris non-null here.After the null-guard above, make the parameter non-null to prevent accidental misuse elsewhere.
- const handleButtonClick = (user: GetLeaderDataQuery['user']) => { + const handleButtonClick = (user: NonNullable<GetLeaderDataQuery['user']>) => { router.push(`/members/${user.login}`) }frontend/src/app/organizations/[organizationKey]/page.tsx (2)
14-14: Add strong types for local state from codegen.State is
any-ish asnull. UseGetOrganizationDataQueryto keep props/consumers type-safe.-import { GetOrganizationDataDocument } from 'types/__generated__/organizationQueries.generated' +import { + GetOrganizationDataDocument, +} from 'types/__generated__/organizationQueries.generated' +import type { GetOrganizationDataQuery } from 'types/__generated__/organizationQueries.generated' - const [organization, setOrganization] = useState(null) - const [issues, setIssues] = useState(null) - const [milestones, setMilestones] = useState(null) - const [pullRequests, setPullRequests] = useState(null) - const [releases, setReleases] = useState(null) - const [repositories, setRepositories] = useState(null) - const [topContributors, setTopContributors] = useState(null) + const [organization, setOrganization] = + useState<GetOrganizationDataQuery['organization'] | null>(null) + const [issues, setIssues] = + useState<GetOrganizationDataQuery['recentIssues'] | null>(null) + const [milestones, setMilestones] = + useState<GetOrganizationDataQuery['recentMilestones'] | null>(null) + const [pullRequests, setPullRequests] = + useState<GetOrganizationDataQuery['recentPullRequests'] | null>(null) + const [releases, setReleases] = + useState<GetOrganizationDataQuery['recentReleases'] | null>(null) + const [repositories, setRepositories] = + useState<GetOrganizationDataQuery['repositories'] | null>(null) + const [topContributors, setTopContributors] = + useState<GetOrganizationDataQuery['topContributors'] | null>(null)Also applies to: 20-27
28-30: Optional: rely onuseQuery’sloadinginstead of a localisLoadingstate.This simplifies logic and prevents edge cases where local state and hook state drift.
I can provide a follow-up diff if you want this simplified.
Also applies to: 32-47, 49-54
frontend/src/app/snapshots/[id]/page.tsx (2)
169-172: Keys and nullable dates.
- Use a stable key (
release.id) instead oftagName-index.- Guard
publishedAt(nullable) to avoid “Released: ” with empty date.- {snapshot.newReleases.map((release, index) => ( + {snapshot.newReleases.map((release) => ( <div - key={`${release.tagName}-${index}`} + key={release.id} className="overflow-hidden rounded-lg border-1 border-gray-200 bg-white shadow-xs transition-all hover:shadow-md dark:border-gray-700 dark:bg-gray-800" > ... - Released: {formatDate(release.publishedAt as string)} + Released:{' '} + {release.publishedAt + ? formatDate(release.publishedAt as string) + : 'TBD'}Also applies to: 190-191
13-15: Unify types with codegen to drop casts and TODOs.
renderProjectCard/renderChapterCardtake app-local types while you pass codegen objects; plus you cast scalars tostring. Prefer codegen types for the render helpers.-import type { Chapter } from 'types/chapter' -import type { Project } from 'types/project' +// Use codegen types to match the query payload +type SnapshotProject = NonNullable<GetSnapshotDetailsQuery['snapshot']>['newProjects'][number] +type SnapshotChapter = NonNullable<GetSnapshotDetailsQuery['snapshot']>['newChapters'][number] - const renderProjectCard = (project: Project) => { + const renderProjectCard = (project: SnapshotProject) => { ... - const renderChapterCard = (chapter: Chapter) => { + const renderChapterCard = (chapter: SnapshotChapter) => {Consider mapping GraphQL scalars in codegen (e.g., DateTime ->
string) to eliminateas stringcasts project-wide.Also applies to: 43-69, 71-97, 153-163
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx (3)
17-17: Prefer casting over a generic foruseParams(Next.js types aren’t generic).Keeps consistency with other files and avoids type errors across Next versions.
- const { programKey, moduleKey } = useParams<{ programKey: string; moduleKey: string }>() + const { programKey, moduleKey } = useParams() as { programKey: string; moduleKey: string }
54-59: Reduce “as string” casts by mapping date scalars in codegen.
startedAt/endedAtareunknowndue to codegen config. Map your GraphQL Date/DateTime scalars tostringin codegen to restore type-safety and drop the casts here.Example (codegen.yml):
config: scalars: DateTime: string Date: stringThen rerun codegen and remove the
as stringcasts at call sites.
19-41: Optional: derive UI state from Apollo instead of mirroring it.You can drop local
module/admins/isLoadingstate and useconst { data, error, loading } = useQuery(...), then read fromdatadirectly. Less state, fewer effects, fewer edge cases.frontend/src/app/snapshots/page.tsx (1)
16-26: Optional: use Apollo’sloadingand avoid localisLoading/effect bookkeeping.Simplifies control flow and removes a class of race conditions around manual loading flags.
- const [snapshots, setSnapshots] = useState<GetCommunitySnapshotsQuery['snapshots'] | null>(null) - const [isLoading, setIsLoading] = useState<boolean>(true) - - const { data: graphQLData, error: graphQLRequestError } = useQuery(GetCommunitySnapshotsDocument) + const { data: graphQLData, error: graphQLRequestError, loading } = + useQuery(GetCommunitySnapshotsDocument) + const snapshots = graphQLData?.snapshots ?? null @@ - if (graphQLData) { - setSnapshots(graphQLData.snapshots) - setIsLoading(false) - } if (graphQLRequestError) { addToast({ @@ - setIsLoading(false) } }, [graphQLData, graphQLRequestError]) @@ - if (isLoading) { + if (loading) { return <LoadingSpinner /> }Also applies to: 63-66
frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx (2)
77-79: Dropas stringcasts by mapping date scalars in codegen.Same rationale as other files; makes
formatDateForInputcalls type-safe.- startedAt: formatDateForInput(program.startedAt as string), - endedAt: formatDateForInput(program.endedAt as string), + startedAt: formatDateForInput(program.startedAt), + endedAt: formatDateForInput(program.endedAt),(After adding
DateTime: stringmapping in codegen.)
19-20: Consistency nit: matchuseParamstyping style used across pages.Here you cast; in modules page you used a generic. Pick one (casting is safer with Next types) to avoid drift.
frontend/src/app/projects/[projectKey]/page.tsx (2)
21-21: Prefer Apollo’s loading state over bespoke isLoading.Leverage the loading flag from useQuery to avoid manual state bookkeeping and edge cases on param changes.
Apply this diff:
- const [isLoading, setIsLoading] = useState<boolean>(true) + // Rely on Apollo's loading flag const { data, error: graphQLRequestError } = useQuery(GetProjectDocument, { variables: { key: projectKey }, }) useEffect(() => { if (data) { setProject(data.project) setTopContributors(data.topContributors) - setIsLoading(false) } if (graphQLRequestError) { handleAppError(graphQLRequestError) - setIsLoading(false) } }, [data, graphQLRequestError, projectKey]) - if (isLoading) { + // Show spinner while the query is loading + if (!project && !graphQLRequestError) { return <LoadingSpinner /> }Also applies to: 27-37, 39-41
27-37: Unnecessary dependency.projectKey in the effect dependencies isn’t used inside the effect and will cause extra runs on param changes.
- }, [data, graphQLRequestError, projectKey]) + }, [data, graphQLRequestError])frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx (2)
19-19: Type admins state explicitly.Avoid implicit any; align with generated types.
- const [admins, setAdmins] = useState(null) + const [admins, setAdmins] = useState< + GetProgramAdminsAndModulesQuery['getProgram']['admins'] + >(null)
22-27: Use Apollo loading instead of manual isLoading.Same suggestion as other pages for consistency and fewer states.
- const [isLoading, setIsLoading] = useState(true) - const { data, error } = useQuery(GetProgramAdminsAndModulesDocument, { + const { data, error, loading } = useQuery(GetProgramAdminsAndModulesDocument, { variables: { programKey, moduleKey }, }) @@ - if (isLoading) return <LoadingSpinner /> + if (loading) return <LoadingSpinner />Also applies to: 40-41
frontend/src/app/members/[memberKey]/page.tsx (1)
23-31: Minor state/flow cleanups.
- Initialize user to null to avoid undefined in unions.
- Prefer Apollo loading flag over manual isLoading for consistency.
- const [user, setUser] = useState<GetUserDataQuery['user'] | null>() + const [user, setUser] = useState<GetUserDataQuery['user'] | null>(null) @@ - const [isLoading, setIsLoading] = useState<boolean>(true) + // Rely on Apollo 'loading' @@ - const { data: graphQLData, error: graphQLRequestError } = useQuery(GetUserDataDocument, { + const { data: graphQLData, error: graphQLRequestError, loading } = useQuery(GetUserDataDocument, { variables: { key: memberKey }, }) @@ - if (isLoading) { + if (loading) { return <LoadingSpinner /> } @@ - if (!isLoading && user == null) { + if (!loading && user == null) { return (Also applies to: 106-118, 211-225
frontend/src/app/settings/api-keys/page.tsx (10)
37-37: Small type alias to reduce repetitionDefine an alias for readability and reuse.
-const [keyToRevoke, setKeyToRevoke] = useState<GetApiKeysQuery['apiKeys'][0] | null>(null) +type ApiKey = GetApiKeysQuery['apiKeys'][number] +const [keyToRevoke, setKeyToRevoke] = useState<ApiKey | null>(null)
39-42: Don’t block on errors when errorPolicy='all'With errorPolicy: 'all', you may have both data and error; current ternary hides partial data. Gate the error block on !data.
- {error ? ( + {error && !data ? ( <div className="rounded-md bg-red-50 p-4 text-red-700 dark:bg-red-900/20 dark:text-red-400"> Error loading API keys </div> ) : loading ? (Optionally surface a non-blocking banner when error && data.
Also applies to: 217-229
105-114: Validate expiry and send ISO string to avoid TZ ambiguityEnsure date isn’t in the past and serialize as ISO (servers usually expect strings).
- const variables: { name: string; expiresAt: Date } = { - name: newKeyName.trim(), - expiresAt: new Date(newKeyExpiry), - } - createApiKey({ variables }) + const expiry = new Date(newKeyExpiry) + const today = new Date(); today.setHours(0,0,0,0) + if (isNaN(expiry.getTime()) || expiry < today) { + addToast({ title: 'Error', description: 'Expiration must be today or later', color: 'danger' }) + return + } + createApiKey({ + variables: { name: newKeyName.trim(), expiresAt: expiry.toISOString() }, + })Consider mapping scalars in codegen so DateTime/UUID are typed as strings (see separate comment).
68-76: Disable revoke button while mutation is in flightPrevents double submissions and accidental repeats.
-const [revokeApiKey] = useMutation(RevokeApiKeyDocument, { +const [revokeApiKey, { loading: revokeLoading }] = useMutation(RevokeApiKeyDocument, { onCompleted: () => {- <Button color="danger" onPress={handleRevokeKey}> + <Button color="danger" onPress={handleRevokeKey} isDisabled={revokeLoading}> Revoke Key </Button>Also applies to: 430-432
80-81: Compute default expiry on open to avoid stale “30 days from today”Minor UX nit if the page stays open across midnight.
-const defaultExpiryDate = format(addDays(new Date(), 30), 'yyyy-MM-dd') ... - setNewKeyExpiry(defaultExpiryDate) + setNewKeyExpiry(format(addDays(new Date(), 30), 'yyyy-MM-dd'))Also applies to: 137-140
242-252: Prefer stable string types and avoid casts; consider codegen scalar mappingUse id (string) for React key, and coerce with String(...) over as-casts. Also, map GraphQL scalars to real TS types to remove unknowns.
-{data.apiKeys.map((key: GetApiKeysQuery['apiKeys'][0]) => ( +{data.apiKeys.map((key: GetApiKeysQuery['apiKeys'][number]) => ( <tr - key={key.uuid as string} + key={key.id} className="border-b border-b-gray-200 dark:border-b-gray-700" > <td className="py-3">{key.name}</td> - <td className="py-3 font-mono text-sm">{key.uuid as string}</td> - <td className="py-3">{format(new Date(key.createdAt as string), 'PP')}</td> + <td className="py-3 font-mono text-sm">{String(key.uuid)}</td> + <td className="py-3">{format(new Date(String(key.createdAt)), 'PP')}</td> <td className="py-3"> - {key.expiresAt ? format(new Date(key.expiresAt as string), 'PP') : 'Never'} + {key.expiresAt ? format(new Date(String(key.expiresAt)), 'PP') : 'Never'} </td>Codegen (typescript plugin) suggestion to remove unknowns:
# codegen.yml generates: frontend/src/types/__generated__/: plugins: - typescript - typescript-operations config: scalars: DateTime: input: string output: string UUID: input: string output: string
234-234: Tailwind class nit: 'border-b-1' isn’t a default utilityUse 'border-b' or an arbitrary value 'border-b-[1px]'.
-<tr className="border-b-1 border-b-gray-200 dark:border-b-gray-700"> +<tr className="border-b border-b-gray-200 dark:border-b-gray-700">
192-195: Wording: use “revoke” to match the actionAlign the guidance with the button/action label.
-You've reached the maximum number of API keys. Delete an existing key to create +You've reached the maximum number of API keys. Revoke an existing key to create
150-158: Type safety when passing uuid to mutationuuid is typed unknown in generated types; explicit string coercion avoids TS complaints and accidental non-string values.
- await revokeApiKey({ variables: { uuid: keyToRevoke.uuid } }) + await revokeApiKey({ variables: { uuid: String(keyToRevoke.uuid) } })
248-251: Potential date off-by-one in some timezonesIf backend stores midnight UTC, local formatting can show the prior day. Consider storing/displaying local midnight or using date-fns-tz’s formatInTimeZone to render in UTC.
Would you like a patch using date-fns-tz (formatInTimeZone) to render consistently?
frontend/src/app/projects/dashboard/metrics/[projectKey]/page.tsx (2)
42-52: Optional: avoid mirroring server data in local stateYou can render directly from
data?.projectand drop theuseEffect+ twouseStates to reduce re-renders and state sync risks. Happy to draft a refactor if you want.
58-64: Guard scalar parsing instead ofunknown→stringcast
createdAtisunknown(custom scalar). Prefer safe coercion or map the scalar tostringin codegen.- const labels = - metricsList?.map((m) => - new Date(m.createdAt as string).toLocaleString('default', { + const labels = + metricsList?.map(({ createdAt }) => + new Date(String(createdAt)).toLocaleString('default', { month: 'short', day: 'numeric', }) ) || []Alternatively, map your GraphQL Date/DateTime scalar to
stringin codegen.frontend/src/app/projects/dashboard/metrics/page.tsx (3)
89-93: Nit: remove redundant||fallback
currentOrderingis always truthy. Drop the||to simplify.- const [ordering, setOrdering] = useState( - currentOrdering || { - score: Ordering.Desc, - } - ) + const [ordering, setOrdering] = useState(currentOrdering)
98-115: Consider explicit error handling policy with v4 defaultsIn v4,
notifyOnNetworkStatusChangedefaults totrue, and error handling semantics are unified. If you expect partial data alongside errors, seterrorPolicy: 'all'. Otherwise, you may see more loading transitions. (apollographql.com)} = useQuery(GetProjectHealthMetricsDocument, { variables: { filters, pagination: { offset: 0, limit: PAGINATION_LIMIT }, ordering: [ ordering, { // eslint-disable-next-line @typescript-eslint/naming-convention project_Name: Ordering.Asc, }, ], }, + // Optional: opt into partial data + errors + // errorPolicy: 'all', })
244-245: Follow-up: updateMetricsCardprop types, remove TODODefine
metricas the generated item type to get full type coverage.Example (in MetricsCard.tsx):
import type { GetProjectHealthMetricsQuery } from 'types/__generated__/projectsHealthDashboardQueries.generated' type MetricT = GetProjectHealthMetricsQuery['projectHealthMetrics'][number]; export interface MetricsCardProps { metric: MetricT; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlfrontend/src/types/__generated__/userQueries.generated.tsis excluded by!**/__generated__/**
📒 Files selected for processing (32)
frontend/package.json(2 hunks)frontend/src/app/about/page.tsx(6 hunks)frontend/src/app/chapters/[chapterKey]/page.tsx(2 hunks)frontend/src/app/committees/[committeeKey]/page.tsx(2 hunks)frontend/src/app/members/[memberKey]/page.tsx(3 hunks)frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx(2 hunks)frontend/src/app/mentorship/programs/[programKey]/page.tsx(3 hunks)frontend/src/app/my/mentorship/page.tsx(5 hunks)frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx(3 hunks)frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx(1 hunks)frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx(2 hunks)frontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx(1 hunks)frontend/src/app/my/mentorship/programs/[programKey]/page.tsx(5 hunks)frontend/src/app/my/mentorship/programs/create/page.tsx(1 hunks)frontend/src/app/organizations/[organizationKey]/page.tsx(3 hunks)frontend/src/app/organizations/[organizationKey]/repositories/[repositoryKey]/page.tsx(1 hunks)frontend/src/app/page.tsx(1 hunks)frontend/src/app/projects/[projectKey]/page.tsx(2 hunks)frontend/src/app/projects/dashboard/metrics/[projectKey]/page.tsx(4 hunks)frontend/src/app/projects/dashboard/metrics/page.tsx(6 hunks)frontend/src/app/projects/dashboard/page.tsx(3 hunks)frontend/src/app/settings/api-keys/page.tsx(5 hunks)frontend/src/app/snapshots/[id]/page.tsx(5 hunks)frontend/src/app/snapshots/page.tsx(2 hunks)frontend/src/components/CardDetailsPage.tsx(1 hunks)frontend/src/components/ModuleForm.tsx(2 hunks)frontend/src/hooks/useDjangoSession.ts(2 hunks)frontend/src/hooks/useLogout.ts(1 hunks)frontend/src/server/apolloClient.ts(2 hunks)frontend/src/server/queries/userQueries.ts(1 hunks)frontend/src/utils/helpers/apolloClient.ts(2 hunks)frontend/src/wrappers/provider.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (11)
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
Applied to files:
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsxfrontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsxfrontend/src/app/mentorship/programs/[programKey]/page.tsxfrontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsxfrontend/src/app/chapters/[chapterKey]/page.tsxfrontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsxfrontend/src/app/my/mentorship/page.tsxfrontend/src/app/my/mentorship/programs/[programKey]/page.tsxfrontend/src/app/projects/[projectKey]/page.tsxfrontend/src/app/organizations/[organizationKey]/page.tsxfrontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx
📚 Learning: 2025-07-13T07:31:06.511Z
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: frontend/src/components/ModuleCard.tsx:53-55
Timestamp: 2025-07-13T07:31:06.511Z
Learning: In Next.js 13+ app router, useRouter from 'next/navigation' does not provide asPath or query properties. Use useParams to extract route parameters and usePathname to get the current pathname instead.
Applied to files:
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsxfrontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsxfrontend/src/app/mentorship/programs/[programKey]/page.tsxfrontend/src/app/projects/dashboard/metrics/[projectKey]/page.tsxfrontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsxfrontend/src/app/organizations/[organizationKey]/page.tsx
📚 Learning: 2025-07-13T11:29:25.245Z
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: frontend/src/app/mentorship/programs/page.tsx:59-61
Timestamp: 2025-07-13T11:29:25.245Z
Learning: In Next.js 13+ app router, components with the 'use client' directive run entirely on the client side and don't require window object existence checks or SSR hydration considerations. Direct access to window.location and other browser APIs is safe in client components.
Applied to files:
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsxfrontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsxfrontend/src/app/mentorship/programs/[programKey]/page.tsxfrontend/src/components/ModuleForm.tsxfrontend/src/app/my/mentorship/programs/[programKey]/page.tsx
📚 Learning: 2025-07-11T15:46:58.233Z
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: frontend/src/components/UserMenu.tsx:56-60
Timestamp: 2025-07-11T15:46:58.233Z
Learning: In the UserMenu component's handleLogout function, both logout() and signOut() calls are intentionally kept despite the useLogout hook already calling signOut internally, as this is necessary for the specific implementation requirements.
Applied to files:
frontend/src/hooks/useLogout.ts
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details. These queries cannot be removed or merged as they serve different use cases in the application.
Applied to files:
frontend/src/app/mentorship/programs/[programKey]/page.tsxfrontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsxfrontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsxfrontend/src/app/my/mentorship/page.tsxfrontend/src/app/my/mentorship/programs/[programKey]/page.tsxfrontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details.
Applied to files:
frontend/src/app/mentorship/programs/[programKey]/page.tsxfrontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsxfrontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsxfrontend/src/app/my/mentorship/page.tsxfrontend/src/app/my/mentorship/programs/[programKey]/page.tsxfrontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx
📚 Learning: 2025-07-08T17:24:36.501Z
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: backend/apps/mentorship/graphql/mutations/program.py:41-44
Timestamp: 2025-07-08T17:24:36.501Z
Learning: In the mentorship program GraphQL mutations, date validation is handled at the GraphQL schema/node level in the input types (CreateProgramInput, UpdateProgramInput), preventing null values from reaching the mutation logic where date comparisons are performed.
Applied to files:
frontend/src/app/mentorship/programs/[programKey]/page.tsxfrontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx
📚 Learning: 2025-07-12T17:14:28.536Z
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: frontend/src/app/mentorship/programs/[programKey]/edit/page.tsx:90-128
Timestamp: 2025-07-12T17:14:28.536Z
Learning: Both ProgramForm (programCard.tsx) and ModuleForm (mainmoduleCard.tsx) components already implement HTML validation using the `required` attribute on form fields. The browser's native validation prevents form submission and displays error messages when required fields are empty, eliminating the need for additional JavaScript validation before GraphQL mutations.
Applied to files:
frontend/src/app/mentorship/programs/[programKey]/page.tsx
📚 Learning: 2025-06-20T16:12:59.256Z
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.
Applied to files:
frontend/src/app/projects/dashboard/metrics/[projectKey]/page.tsxfrontend/src/app/projects/dashboard/metrics/page.tsx
📚 Learning: 2025-06-20T16:12:59.256Z
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.
Applied to files:
frontend/src/app/projects/dashboard/metrics/[projectKey]/page.tsxfrontend/src/app/projects/dashboard/metrics/page.tsxfrontend/src/app/projects/dashboard/page.tsx
📚 Learning: 2025-07-13T11:34:31.823Z
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: frontend/src/app/mentorship/programs/page.tsx:14-14
Timestamp: 2025-07-13T11:34:31.823Z
Learning: In the Next.js frontend mentorship application, there are two distinct types for authentication-related data: ExtendedSession for useSession hook (containing accessToken and user.login properties) and UserRolesData for useUserRoles hook (containing currentUserRoles.roles array). The correct access pattern for GitHub username is `(session as ExtendedSession)?.user?.login`.
Applied to files:
frontend/src/app/members/[memberKey]/page.tsxfrontend/src/app/my/mentorship/page.tsxfrontend/src/app/my/mentorship/programs/[programKey]/page.tsx
🧬 Code graph analysis (20)
frontend/src/app/mentorship/programs/[programKey]/page.tsx (2)
frontend/src/types/__generated__/programsQueries.generated.ts (2)
GetProgramAndModulesDocument(37-37)GetProgramAndModulesQuery(25-25)frontend/src/utils/dateFormatter.ts (1)
formatDate(1-20)
frontend/src/components/ModuleForm.tsx (1)
frontend/src/types/__generated__/projectQueries.generated.ts (1)
SearchProjectNamesDocument(39-39)
frontend/src/app/projects/dashboard/metrics/[projectKey]/page.tsx (1)
frontend/src/types/__generated__/projectsHealthDashboardQueries.generated.ts (2)
GetProjectHealthMetricsDetailsQuery(23-23)GetProjectHealthMetricsDetailsDocument(28-28)
frontend/src/app/settings/api-keys/page.tsx (1)
frontend/src/types/__generated__/apiKeyQueries.generated.ts (4)
GetApiKeysQuery(7-7)GetApiKeysDocument(25-25)CreateApiKeyDocument(26-26)RevokeApiKeyDocument(27-27)
frontend/src/hooks/useDjangoSession.ts (1)
frontend/src/types/__generated__/authQueries.generated.ts (1)
SyncDjangoSessionDocument(18-18)
frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx (2)
frontend/src/types/__generated__/programsQueries.generated.ts (1)
GetProgramDetailsDocument(36-36)frontend/src/utils/dateFormatter.ts (1)
formatDateForInput(63-70)
frontend/src/app/members/[memberKey]/page.tsx (1)
frontend/src/types/__generated__/userQueries.generated.ts (2)
GetUserDataQuery(16-16)GetUserDataDocument(27-27)
frontend/src/app/projects/dashboard/metrics/page.tsx (1)
frontend/src/types/__generated__/projectsHealthDashboardQueries.generated.ts (2)
GetProjectHealthMetricsQuery(16-16)GetProjectHealthMetricsDocument(27-27)
frontend/src/app/about/page.tsx (2)
frontend/src/types/__generated__/projectQueries.generated.ts (4)
GetProjectMetadataDocument(37-37)GetTopContributorsDocument(38-38)GetProjectMetadataQuery(16-16)GetTopContributorsQuery(26-26)frontend/src/types/__generated__/userQueries.generated.ts (2)
GetLeaderDataDocument(26-26)GetLeaderDataQuery(9-9)
frontend/src/app/chapters/[chapterKey]/page.tsx (2)
frontend/src/types/__generated__/chapterQueries.generated.ts (2)
GetChapterDataQuery(9-9)GetChapterDataDocument(19-19)frontend/src/types/contributor.ts (1)
Contributor(1-8)
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx (3)
frontend/src/types/__generated__/moduleQueries.generated.ts (2)
GetProgramAdminsAndModulesQuery(25-25)GetProgramAdminsAndModulesDocument(30-30)frontend/src/utils/dateFormatter.ts (1)
formatDate(1-20)frontend/src/components/ModuleCard.tsx (1)
getSimpleDuration(98-110)
frontend/src/app/snapshots/[id]/page.tsx (2)
frontend/src/types/__generated__/snapshotQueries.generated.ts (2)
GetSnapshotDetailsQuery(9-9)GetSnapshotDetailsDocument(24-24)frontend/src/utils/dateFormatter.ts (1)
formatDate(1-20)
frontend/src/app/my/mentorship/page.tsx (1)
frontend/src/types/__generated__/programsQueries.generated.ts (2)
GetMyProgramsQuery(11-11)GetMyProgramsDocument(35-35)
frontend/src/app/projects/dashboard/page.tsx (1)
frontend/src/types/__generated__/projectsHealthDashboardQueries.generated.ts (2)
GetProjectHealthStatsQuery(7-7)GetProjectHealthStatsDocument(26-26)
frontend/src/app/snapshots/page.tsx (2)
backend/apps/owasp/api/internal/queries/snapshot.py (2)
snapshots(25-31)snapshot(14-22)frontend/src/types/__generated__/snapshotQueries.generated.ts (2)
GetCommunitySnapshotsQuery(21-21)GetCommunitySnapshotsDocument(26-26)
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (2)
frontend/src/types/__generated__/programsQueries.generated.ts (2)
GetProgramAndModulesQuery(25-25)GetProgramAndModulesDocument(37-37)frontend/src/utils/dateFormatter.ts (1)
formatDate(1-20)
frontend/src/app/projects/[projectKey]/page.tsx (1)
frontend/src/types/__generated__/projectQueries.generated.ts (2)
GetProjectQuery(9-9)GetProjectDocument(36-36)
frontend/src/app/committees/[committeeKey]/page.tsx (3)
backend/apps/owasp/api/internal/queries/committee.py (1)
committee(14-28)frontend/src/types/__generated__/committeeQueries.generated.ts (2)
GetCommitteeDataQuery(9-9)GetCommitteeDataDocument(19-19)frontend/src/types/contributor.ts (1)
Contributor(1-8)
frontend/src/app/organizations/[organizationKey]/page.tsx (1)
frontend/src/types/__generated__/organizationQueries.generated.ts (1)
GetOrganizationDataDocument(19-19)
frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx (3)
frontend/src/types/__generated__/moduleQueries.generated.ts (2)
GetProgramAdminsAndModulesQuery(25-25)GetProgramAdminsAndModulesDocument(30-30)frontend/src/utils/dateFormatter.ts (1)
formatDate(1-20)frontend/src/components/ModuleCard.tsx (1)
getSimpleDuration(98-110)
🪛 Biome (2.1.2)
frontend/src/app/snapshots/[id]/page.tsx
[error] 147-147: Wrap comments inside children within braces.
Unsafe fix: Wrap the comments with braces
(lint/suspicious/noCommentText)
⏰ 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 backend tests
- GitHub Check: Run frontend unit tests
- GitHub Check: CodeQL (javascript-typescript)
frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (2)
frontend/src/app/snapshots/page.tsx (1)
40-42: Prefer a readable item alias and letmapinfer the element type.Use a
SnapshotItemalias ([number]index) and drop the explicit element type inmap. This mirrors prior feedback.Apply this diff:
- const handleButtonClick = (snapshot: GetCommunitySnapshotsQuery['snapshots'][0]) => { + const handleButtonClick = (snapshot: SnapshotItem) => { router.push(`/snapshots/${snapshot.key}`) }- const renderSnapshotCard = (snapshot: GetCommunitySnapshotsQuery['snapshots'][0]) => { + const renderSnapshotCard = (snapshot: SnapshotItem) => {- snapshots.map((snapshot: GetCommunitySnapshotsQuery['snapshots'][0]) => ( + snapshots.map((snapshot) => ( <div key={snapshot.key}>{renderSnapshotCard(snapshot)}</div> ))Add this alias near the imports (outside the diffed ranges):
type SnapshotItem = GetCommunitySnapshotsQuery['snapshots'][number]Also applies to: 44-50, 73-75
frontend/src/app/snapshots/[id]/page.tsx (1)
144-145: Remove stray whitespace text node to satisfy Biome rule.This is the same issue previously flagged; the {' '} child is unnecessary here.
- {' '} {snapshot.newChapters.filter((chapter) => chapter.isActive).map(renderChapterCard)}
🧹 Nitpick comments (14)
frontend/src/utils/utility.ts (2)
7-7: Use type-only import to avoid emitting runtime code.Switch to a type import for the generated GraphQL type.
-import { GetSnapshotDetailsQuery } from 'types/__generated__/snapshotQueries.generated' +import type { GetSnapshotDetailsQuery } from 'types/__generated__/snapshotQueries.generated'
29-36: Harden createdAt handling and tighten types for params/reduce accumulator.
- Accept only known icon keys in
params.- Support both numeric (sec/ms) and string timestamps for
createdAt.- Type the reducer accumulator as
Partial<Icon>to avoid{}-as-Icon casts leaking.-export const getFilteredIcons = (project: ProjectType, params: string[]): Icon => { - const filteredIcons = params.reduce((acc: Icon, key) => { +export const getFilteredIcons = (project: ProjectType, params: readonly IconKeys[]): Icon => { + const filteredIcons = params.reduce<Partial<Icon>>((acc, key) => { if (ICONS[key as IconKeys] && project[key as keyof typeof project] !== undefined) { if (key === 'createdAt') { - acc[key] = dayjs.unix(project[key as keyof ProjectType] as unknown as number).fromNow() + const raw = project[key as keyof ProjectType] as unknown + const d = + typeof raw === 'number' + ? (raw > 1_000_000_000_000 ? dayjs(raw) : dayjs.unix(raw)) + : dayjs(raw as string) + if (d.isValid()) acc[key] = d.fromNow() } else { - acc[key] = project[key as keyof typeof project] as unknown as number + acc[key] = project[key as keyof ProjectType] as unknown as number } } return acc - }, {}) + }, {}) - return filteredIcons + return filteredIcons as Icon }Also applies to: 41-41
frontend/src/app/snapshots/page.tsx (2)
15-19: Remove mirrored state; rely on Apollo’sloading/datato simplify and avoid effect churn.
isLoadingandsnapshotsduplicateuseQuerystate and add unnecessary effects (can also double-fire toasts under React StrictMode). Derive fromuseQueryinstead.Apply this diff:
- const [snapshots, setSnapshots] = useState<GetCommunitySnapshotsQuery['snapshots'] | null>(null) - const [isLoading, setIsLoading] = useState<boolean>(true) - - const { data: graphQLData, error: graphQLRequestError } = useQuery(GetCommunitySnapshotsDocument) + const { data, error, loading } = useQuery(GetCommunitySnapshotsDocument) + const snapshots = data?.snapshots ?? []- useEffect(() => { - if (graphQLData) { - setSnapshots(graphQLData.snapshots) - setIsLoading(false) - } - if (graphQLRequestError) { - addToast({ - description: 'Unable to complete the requested operation.', - title: 'GraphQL Request Failed', - timeout: 3000, - shouldShowTimeoutProgress: true, - color: 'danger', - variant: 'solid', - }) - setIsLoading(false) - } - }, [graphQLData, graphQLRequestError]) + useEffect(() => { + if (error) { + addToast({ + description: 'Unable to complete the requested operation.', + title: 'GraphQL Request Failed', + timeout: 3000, + shouldShowTimeoutProgress: true, + color: 'danger', + variant: 'solid', + }) + } + }, [error])- if (isLoading) { + if (loading) { return <LoadingSpinner /> }- {!snapshots?.length ? ( + {!snapshots.length ? ( <div className="col-span-full py-8 text-center">No Snapshots found</div> ) : ( snapshots.map((snapshot: GetCommunitySnapshotsQuery['snapshots'][0]) => ( <div key={snapshot.key}>{renderSnapshotCard(snapshot)}</div> )) )}Additionally, remove
useStatefrom the React import:- import React, { useState, useEffect } from 'react' + import React, { useEffect } from 'react'Also applies to: 20-36, 62-64, 70-76
53-53: Avoid double keys and an extra wrapper node.You set
keyon<SnapshotCard>and also on the wrapping<div>. Keep one. Easiest: remove the wrapper and map directly to the card-returning renderer.Apply this diff:
- snapshots.map((snapshot) => ( - <div key={snapshot.key}>{renderSnapshotCard(snapshot)}</div> - )) + snapshots.map(renderSnapshotCard)Also applies to: 73-75
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (5)
29-31: Avoid mirroring Apollo data in local state.Storing
program/modulesin state and syncing viauseEffectrisks staleness and extra renders. Prefer deriving directly fromdata.Example (outside this hunk): after the
useQuery(...)call, derive:const program = data?.getProgram ?? null const modules = data?.getProgramModules ?? []Then remove the
setProgram/setModulescalls in the effect.
41-45: Consider cache-and-network for snappier UX.If acceptable, fetch from cache first while updating in background.
} = useQuery(GetProgramAndModulesDocument, { variables: { programKey }, skip: !programKey, notifyOnNetworkStatusChange: true, + fetchPolicy: 'cache-and-network', })
144-144: Resolve TODO: type modules prop and align accessLevel with actual role.
- Update
DetailsCardprop to acceptGetProgramAndModulesQuery['getProgramModules']to drop the TODO.- Consider
accessLevel={isAdmin ? 'admin' : 'read'}to avoid overexposing actions when the viewer isn’t an admin.I can open a follow-up to type
DetailsCardcorrectly and threadaccessLevelfromisAdmin. Want me to draft it?
2-2: Import Apollo hooks from@apollo/client/reacteverywhere. Test files are still importing hooks (e.g.useQuery,useMutation,useApolloClient) from@apollo/client; update those to the@apollo/client/reactsubpath to maintain repo-wide consistency and avoid mixed entry points.
71-81: Add awaitRefetchQueries to the mutation call
Ensure the mutation’s returned promise waits for the refetch by addingawaitRefetchQueries: truealongsiderefetchQueries—the input shape already includesname, so no changes needed there.frontend/src/app/snapshots/[id]/page.tsx (5)
9-12: Good switch to generated Document and typed query.This enables end-to-end type inference for useQuery.
If possible, also export and use the generated GetSnapshotDetailsQueryVariables for extra safety on variables.
21-22: Tighten snapshot state type.GetSnapshotDetailsQuery['snapshot'] already includes null, so the extra "| null" is redundant.
- const [snapshot, setSnapshot] = useState<GetSnapshotDetailsQuery['snapshot'] | null>(null) + const [snapshot, setSnapshot] = useState<GetSnapshotDetailsQuery['snapshot']>(null)
26-28: Guard the query until the route param is available; consider relying on useQuery.loading.Add skip: !snapshotKey to avoid a spurious request with undefined variables. Optional: favor loading from useQuery over a parallel isLoading state/effect to reduce race conditions.
- const { data: graphQLData, error: graphQLRequestError } = useQuery(GetSnapshotDetailsDocument, { - variables: { key: snapshotKey }, - }) + const { data: graphQLData, error: graphQLRequestError } = useQuery(GetSnapshotDetailsDocument, { + variables: { key: snapshotKey }, + skip: !snapshotKey, + })
186-186: Handle nullable publishedAt without string cast; prefer a readable fallback.Also consider using release.id as the React key instead of tag/index (outside this hunk).
- Released: {formatDate(release.publishedAt as string)} + Released: {release.publishedAt ? formatDate(release.publishedAt as number | string) : 'TBD'}Outside this hunk, change the item key to a stable id:
key={release.id}
9-12: Codegen scalars mapping to remove “unknown” dates (one-time config).Map server date/timestamp scalars to concrete TS types so casts disappear and type-safety improves.
Add to codegen config:
generates: frontend/src/types/__generated__/: plugins: - typescript - typescript-operations config: scalars: DateTime: string Timestamp: number Date: stringThen re-run codegen and replace the remaining as number | string casts.
Also applies to: 123-123, 186-186
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx(5 hunks)frontend/src/app/snapshots/[id]/page.tsx(7 hunks)frontend/src/app/snapshots/page.tsx(4 hunks)frontend/src/utils/utility.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-07-13T07:31:06.511Z
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: frontend/src/components/ModuleCard.tsx:53-55
Timestamp: 2025-07-13T07:31:06.511Z
Learning: In Next.js 13+ app router, useRouter from 'next/navigation' does not provide asPath or query properties. Use useParams to extract route parameters and usePathname to get the current pathname instead.
Applied to files:
frontend/src/app/snapshots/[id]/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 is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
Applied to files:
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details. These queries cannot be removed or merged as they serve different use cases in the application.
Applied to files:
frontend/src/app/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
📚 Learning: 2025-07-08T17:24:36.501Z
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: backend/apps/mentorship/graphql/mutations/program.py:41-44
Timestamp: 2025-07-08T17:24:36.501Z
Learning: In the mentorship program GraphQL mutations, date validation is handled at the GraphQL schema/node level in the input types (CreateProgramInput, UpdateProgramInput), preventing null values from reaching the mutation logic where date comparisons are performed.
Applied to files:
frontend/src/app/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/src/app/my/mentorship/programs/[programKey]/page.tsx
📚 Learning: 2025-07-13T11:34:31.823Z
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: frontend/src/app/mentorship/programs/page.tsx:14-14
Timestamp: 2025-07-13T11:34:31.823Z
Learning: In the Next.js frontend mentorship application, there are two distinct types for authentication-related data: ExtendedSession for useSession hook (containing accessToken and user.login properties) and UserRolesData for useUserRoles hook (containing currentUserRoles.roles array). The correct access pattern for GitHub username is `(session as ExtendedSession)?.user?.login`.
Applied to files:
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
🧬 Code graph analysis (4)
frontend/src/utils/utility.ts (3)
frontend/src/types/project.ts (1)
Project(17-45)frontend/src/types/chapter.ts (1)
Chapter(3-19)frontend/src/types/__generated__/snapshotQueries.generated.ts (1)
GetSnapshotDetailsQuery(9-9)
frontend/src/app/snapshots/[id]/page.tsx (2)
frontend/src/types/__generated__/snapshotQueries.generated.ts (2)
GetSnapshotDetailsQuery(9-9)GetSnapshotDetailsDocument(24-24)frontend/src/utils/dateFormatter.ts (1)
formatDate(1-20)
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (4)
frontend/src/types/__generated__/programsQueries.generated.ts (2)
GetProgramAndModulesQuery(25-25)GetProgramAndModulesDocument(37-37)frontend/src/types/__generated__/programsMutations.generated.ts (1)
UpdateProgramStatusDocument(28-28)frontend/src/app/global-error.tsx (1)
handleAppError(66-86)frontend/src/utils/dateFormatter.ts (1)
formatDate(1-20)
frontend/src/app/snapshots/page.tsx (2)
backend/apps/owasp/api/internal/queries/snapshot.py (2)
snapshots(25-31)snapshot(14-22)frontend/src/types/__generated__/snapshotQueries.generated.ts (2)
GetCommunitySnapshotsQuery(21-21)GetCommunitySnapshotsDocument(26-26)
⏰ 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 backend tests
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run frontend unit tests
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (5)
frontend/src/app/snapshots/page.tsx (2)
2-2: Apollo v4 import path is correct.Importing
useQueryfrom@apollo/client/reactis v4-compliant. No action needed.
7-10: Good move to codegen documents and types.Using
GetCommunitySnapshotsDocumentandGetCommunitySnapshotsQuerytightens type-safety.frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (2)
9-14: Switch to codegen documents/types — LGTM.Using generated DocumentNodes and enums tightens type-safety and aligns with the migration plan. Also compatible with the prior learning that GET_PROGRAM_DETAILS remains in use elsewhere; no conflict here.
33-35: Mutation hook wiring — LGTM.
onError: handleAppErrorcentralizes error handling; keep it.frontend/src/app/snapshots/[id]/page.tsx (1)
2-2: Apollo v4 React import looks good.Using the React-only subpath is compatible with v4 and keeps the bundle lean.
rudransh-shrivastava
force-pushed
the
refactor/migrate-apollo-3-to-4
branch
from
13c6f52 to
b8a110c
Compare
There was a problem hiding this comment.
Actionable comments posted: 10
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/snapshots/[id]/layout.tsx (1)
10-16: Incorrect params typing/await in generateMetadata—use plain object.Next.js passes params as a plain object; remove Promise typing/await to match runtime and typings.
Apply this diff:
export async function generateMetadata({ params, }: { - params: Promise<{ id: string }> + params: { id: string } }): Promise<Metadata> { - const { id: snapshotKey } = await params + const { id: snapshotKey } = params const { data } = await apolloClient.query({ query: GetSnapshotDetailsMetadataDocument, variables: { key: snapshotKey }, })
🧹 Nitpick comments (20)
frontend/src/types/chapter.ts (1)
5-18: Avoid blanket-optionalizing Chapter; prefer query-specific/generated types or local Partial/Pick.Most Chapter fields shouldn't be blanket-optional — keep Chapter strict and either use GraphQL‑codegen selection types per query (Apollo v4) or introduce a derived partial for call sites. Suggested helper: export type ChapterPartial = Partial
frontend/src/types/committee.ts (1)
16-16: Make topContributors required (or explicitly nullable)GraphQL codegen shows topContributors as a non-null array — prefer a required array and normalise upstream to [] (or use explicit null union if the schema can return null).
Location: frontend/src/types/committee.ts:16
Suggested change:
- topContributors?: Contributor[] + topContributors: Contributor[] // ensure upstream maps undefined/null to []Call sites/tests to update: frontend/tests/unit/components/CardDetailsPage.test.tsx:1074 and 1195-1196 (these pass topContributors={undefined}).
frontend/src/types/pullRequest.ts (2)
3-5: Document why author is nullable.A brief note will prevent churn and clarify expectations for consumers.
Apply:
export type PullRequest = { - author?: User + // author may be undefined if not returned by the API or omitted from the selection set + author?: User
3-10: Optional: keep domain types strict; narrow at query boundaries.If this nullability is only due to specific queries, consider mapping API results to a stricter UI/domain type (with a fallback author) at the data layer instead of loosening the shared type. Given the migration focus, feel free to defer.
frontend/src/types/healthMetrics.ts (1)
30-30: Self-documentcreatedAtformat (optional)If this is ISO‑8601, consider a branded alias to improve readability and reduce misuse.
Apply:
+export type ISODateString = string // e.g., 2025-09-22T12:34:56Z export type HealthMetricsProps = { ... - createdAt?: string + createdAt?: ISODateString ... }frontend/src/types/mentorship.ts (1)
2-3: Import these enums as types; optionally re-export to avoid eslint disableGenerated file exports runtime enums, but this file uses them only in type positions — switch to a type-only import.
Apply this minimal diff:
-// eslint-disable-next-line no-restricted-imports -import { ExperienceLevelEnum, ProgramStatusEnum } from './__generated__/graphql' +// eslint-disable-next-line no-restricted-imports +import type { ExperienceLevelEnum, ProgramStatusEnum } from './__generated__/graphql'Optional follow-up (separate change): create frontend/src/types/graphql.ts
// re-export types to avoid importing from __generated__ all over export type { ExperienceLevelEnum, ProgramStatusEnum } from './__generated__/graphql'Then update this file:
-// eslint-disable-next-line no-restricted-imports -import type { ExperienceLevelEnum, ProgramStatusEnum } from './__generated__/graphql' +import type { ExperienceLevelEnum, ProgramStatusEnum } from 'types/graphql'frontend/src/types/issue.ts (1)
7-7: Immutable labels: consider ReadonlyArrayMinor: if labels are not mutated, convey intent and help the compiler.
- labels?: string[] + labels?: ReadonlyArray<string>frontend/src/app/projects/dashboard/metrics/[projectKey]/page.tsx (1)
56-56: Date casting should be verified for safety.The cast of
createdAtto string in the date formatting should be verified against the actual data type returned by the GraphQL query, especially given the scalar mapping configuration.Based on the codegen configuration that maps Date/DateTime to
'string | number', this cast might be unsafe if the backend returns epoch seconds:- new Date(m.createdAt as string).toLocaleString('default', { + new Date(typeof m.createdAt === 'number' ? m.createdAt * 1000 : m.createdAt as string).toLocaleString('default', {frontend/src/utils/structuredData.ts (2)
4-15: formatISODate: undefined and ms/sec handling; avoid throwing.If createdAt/updatedAt are absent or already in milliseconds, current logic can throw or mis-convert. Make the helper accept undefined and auto-detect seconds vs milliseconds; return undefined instead of throwing so callers can omit the field.
Apply this diff:
-export const formatISODate = (input: number | string): string => { - const date = - typeof input === 'number' - ? new Date(input * 1000) // Unix timestamp in seconds - : new Date(input) // ISO date string - - if (isNaN(date.getTime())) { - throw new Error('Invalid date') - } - - return date.toISOString() -} +export const formatISODate = (input: number | string | undefined): string | undefined => { + if (input == null) return undefined + if (typeof input === 'number') { + // Heuristic: > 1e12 likely ms; else seconds. + const ms = input > 1_000_000_000_000 ? input : input * 1000 + const d = new Date(ms) + return isNaN(d.getTime()) ? undefined : d.toISOString() + } + const d = new Date(input) + return isNaN(d.getTime()) ? undefined : d.toISOString() +}
58-59: Avoid emitting sameAs: [undefined].Only include sameAs when URL exists.
Apply this diff:
- sameAs: [user.url], + ...(user.url && { sameAs: [user.url] }),frontend/src/app/members/[memberKey]/page.tsx (1)
29-29: Initialize user state to null to avoid triple-state (undefined | null | User).Current call without an initial value makes state possibly undefined and complicates checks.
Apply this diff:
- const [user, setUser] = useState<User | null>() + const [user, setUser] = useState<User | null>(null)frontend/src/app/settings/api-keys/page.tsx (3)
20-26: Use generated GraphQL types; drop legacy ApiKey type to prevent drift.Stay consistent with codegen outputs to avoid mismatches and unsafe casts.
Apply this diff:
import { CreateApiKeyDocument, GetApiKeysDocument, RevokeApiKeyDocument, } from 'types/__generated__/apiKeyQueries.generated' -import { ApiKey } from 'types/apiKey' +import type { GetApiKeysQuery } from 'types/__generated__/apiKeyQueries.generated'
37-37: Key-to-revoke state should use generated item type.Aligns state with the query result shape.
Apply this diff:
- const [keyToRevoke, setKeyToRevoke] = useState<ApiKey | null>(null) + const [keyToRevoke, setKeyToRevoke] = useState<GetApiKeysQuery['apiKeys'][0] | null>(null)
242-252: Drop explicit ApiKey annotation and unsafe casts; rely on codegen types.Remove the manual type on map and string casts—codegen already types uuid/createdAt/expiresAt as strings.
Apply this diff:
- {data.apiKeys.map((key: ApiKey) => ( + {data.apiKeys.map((key) => ( <tr - key={key.uuid as string} + key={key.uuid} className="border-b border-b-gray-200 dark:border-b-gray-700" > <td className="py-3">{key.name}</td> - <td className="py-3 font-mono text-sm">{key.uuid as string}</td> - <td className="py-3">{format(new Date(key.createdAt as string), 'PP')}</td> + <td className="py-3 font-mono text-sm">{key.uuid}</td> + <td className="py-3">{format(new Date(key.createdAt), 'PP')}</td> <td className="py-3"> - {key.expiresAt ? format(new Date(key.expiresAt as string), 'PP') : 'Never'} + {key.expiresAt ? format(new Date(key.expiresAt), 'PP') : 'Never'} </td>frontend/src/app/snapshots/[id]/page.tsx (3)
138-142: Use numeric zIndex.Style props expect a number;
'0'is a string.Apply:
- style={{ height: '400px', width: '100%', zIndex: '0' }} + style={{ height: '400px', width: '100%', zIndex: 0 }}
145-147: Remove stray text node.The lone
{' '}is noise and can trip linters.Apply:
- {' '} {snapshot.newChapters.filter((chapter) => chapter.isActive).map(renderChapterCard)}
12-12: Import SnapshotDetails as a typetsconfig.json (frontend/tsconfig.json) does not set preserveValueImports/verbatimModuleSyntax/importsNotUsedAsValues — still use an explicit type import to avoid runtime import issues.
File: frontend/src/app/snapshots/[id]/page.tsx
-import { SnapshotDetails } from 'types/snapshot' +import type { SnapshotDetails } from 'types/snapshot'frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (3)
13-13: Align local state types to generated query types to avoid drift.Prefer types derived from GetProgramAndModulesQuery so state matches the actual selection set.
-import { Program, Module } from 'types/mentorship' +import type { GetProgramAndModulesQuery } from 'types/__generated__/graphql'And later:
-const [program, setProgram] = useState<Program | null>(null) -const [modules, setModules] = useState<Module[]>([]) +type ProgramT = NonNullable<GetProgramAndModulesQuery['getProgram']> +type ModuleT = GetProgramAndModulesQuery['getProgramModules'][number] +const [program, setProgram] = useState<ProgramT | null>(null) +const [modules, setModules] = useState<ModuleT[]>([])
31-33: Resolve the TODO or remove it to keep migration clean.If you plan to rely on refetchQueries, drop the TODO; otherwise implement an update/optimisticResponse for cache sync.
I can wire an optimistic response for status flips if you want a follow‑up patch.
2-2: Prefer root Apollo hooks export ('@apollo/client') instead of '@apollo/client/react'.Project uses @apollo/[email protected] (frontend/pnpm-lock.yaml); change the import in frontend/src/app/my/mentorship/programs/[programKey]/page.tsx from
import { useMutation, useQuery } from '@apollo/client/react'
to
import { useMutation, useQuery } from '@apollo/client'.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (14)
frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlfrontend/src/types/__generated__/apiKeyQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/graphql.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/homeQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/moduleMutations.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/moduleQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/organizationQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/programsMutations.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/programsQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/projectQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/projectsHealthDashboardQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/repositoryQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/snapshotQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/userQueries.generated.tsis excluded by!**/__generated__/**
📒 Files selected for processing (40)
frontend/graphql-codegen.ts(2 hunks)frontend/package.json(2 hunks)frontend/src/app/about/page.tsx(4 hunks)frontend/src/app/chapters/[chapterKey]/page.tsx(1 hunks)frontend/src/app/committees/[committeeKey]/page.tsx(3 hunks)frontend/src/app/members/[memberKey]/page.tsx(3 hunks)frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx(2 hunks)frontend/src/app/mentorship/programs/[programKey]/page.tsx(3 hunks)frontend/src/app/mentorship/programs/page.tsx(2 hunks)frontend/src/app/my/mentorship/page.tsx(3 hunks)frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx(2 hunks)frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx(2 hunks)frontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx(2 hunks)frontend/src/app/my/mentorship/programs/[programKey]/page.tsx(4 hunks)frontend/src/app/my/mentorship/programs/create/page.tsx(2 hunks)frontend/src/app/organizations/[organizationKey]/layout.tsx(1 hunks)frontend/src/app/organizations/[organizationKey]/repositories/[repositoryKey]/layout.tsx(2 hunks)frontend/src/app/organizations/[organizationKey]/repositories/[repositoryKey]/page.tsx(2 hunks)frontend/src/app/page.tsx(3 hunks)frontend/src/app/projects/[projectKey]/layout.tsx(2 hunks)frontend/src/app/projects/[projectKey]/page.tsx(2 hunks)frontend/src/app/projects/dashboard/metrics/[projectKey]/page.tsx(4 hunks)frontend/src/app/projects/dashboard/metrics/page.tsx(6 hunks)frontend/src/app/projects/dashboard/page.tsx(3 hunks)frontend/src/app/settings/api-keys/page.tsx(6 hunks)frontend/src/app/snapshots/[id]/layout.tsx(2 hunks)frontend/src/app/snapshots/[id]/page.tsx(4 hunks)frontend/src/app/snapshots/page.tsx(2 hunks)frontend/src/types/chapter.ts(1 hunks)frontend/src/types/committee.ts(1 hunks)frontend/src/types/healthMetrics.ts(1 hunks)frontend/src/types/issue.ts(1 hunks)frontend/src/types/mentorship.ts(3 hunks)frontend/src/types/milestone.ts(1 hunks)frontend/src/types/organization.ts(1 hunks)frontend/src/types/project.ts(1 hunks)frontend/src/types/pullRequest.ts(1 hunks)frontend/src/types/release.ts(1 hunks)frontend/src/types/user.ts(1 hunks)frontend/src/utils/structuredData.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- frontend/src/app/organizations/[organizationKey]/layout.tsx
🚧 Files skipped from review as they are similar to previous changes (15)
- frontend/src/app/mentorship/programs/[programKey]/page.tsx
- frontend/src/app/projects/dashboard/page.tsx
- frontend/package.json
- frontend/src/app/my/mentorship/programs/create/page.tsx
- frontend/src/app/committees/[committeeKey]/page.tsx
- frontend/src/app/about/page.tsx
- frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx
- frontend/src/app/projects/[projectKey]/page.tsx
- frontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx
- frontend/src/app/snapshots/page.tsx
- frontend/src/app/projects/dashboard/metrics/page.tsx
- frontend/src/app/organizations/[organizationKey]/repositories/[repositoryKey]/page.tsx
- frontend/src/app/chapters/[chapterKey]/page.tsx
- frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx
- frontend/src/app/page.tsx
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2178
File: frontend/src/app/snapshots/[id]/page.tsx:0-0
Timestamp: 2025-09-21T17:04:48.154Z
Learning: User rudransh-shrivastava confirmed that suggested type safety improvements during Apollo Client migration were no longer relevant, reinforcing their preference to keep migration PRs focused on core migration changes rather than additional improvements.
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2178
File: frontend/src/app/about/page.tsx:47-53
Timestamp: 2025-09-21T11:34:33.377Z
Learning: User rudransh-shrivastava prefers to keep Apollo Client migration PRs focused solely on migration-related changes and not include additional bug fixes or improvements that aren't directly related to the migration itself.
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
Applied to files:
frontend/src/app/mentorship/programs/page.tsxfrontend/src/app/my/mentorship/programs/[programKey]/page.tsxfrontend/src/app/my/mentorship/page.tsxfrontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details. These queries cannot be removed or merged as they serve different use cases in the application.
Applied to files:
frontend/src/app/mentorship/programs/page.tsxfrontend/src/app/my/mentorship/programs/[programKey]/page.tsxfrontend/src/app/my/mentorship/page.tsxfrontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details.
Applied to files:
frontend/src/app/mentorship/programs/page.tsxfrontend/src/app/my/mentorship/programs/[programKey]/page.tsxfrontend/src/app/my/mentorship/page.tsxfrontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx
📚 Learning: 2025-07-13T07:31:06.511Z
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: frontend/src/components/ModuleCard.tsx:53-55
Timestamp: 2025-07-13T07:31:06.511Z
Learning: In Next.js 13+ app router, useRouter from 'next/navigation' does not provide asPath or query properties. Use useParams to extract route parameters and usePathname to get the current pathname instead.
Applied to files:
frontend/src/app/mentorship/programs/page.tsxfrontend/src/app/projects/dashboard/metrics/[projectKey]/page.tsx
📚 Learning: 2025-06-20T16:12:59.256Z
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.
Applied to files:
frontend/src/app/projects/dashboard/metrics/[projectKey]/page.tsxfrontend/src/types/healthMetrics.ts
📚 Learning: 2025-06-20T16:12:59.256Z
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.
Applied to files:
frontend/src/app/projects/dashboard/metrics/[projectKey]/page.tsxfrontend/src/types/healthMetrics.ts
📚 Learning: 2025-07-08T17:24:36.501Z
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: backend/apps/mentorship/graphql/mutations/program.py:41-44
Timestamp: 2025-07-08T17:24:36.501Z
Learning: In the mentorship program GraphQL mutations, date validation is handled at the GraphQL schema/node level in the input types (CreateProgramInput, UpdateProgramInput), preventing null values from reaching the mutation logic where date comparisons are performed.
Applied to files:
frontend/src/app/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/src/app/my/mentorship/programs/[programKey]/page.tsx
📚 Learning: 2025-09-21T17:04:48.154Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2178
File: frontend/src/app/snapshots/[id]/page.tsx:0-0
Timestamp: 2025-09-21T17:04:48.154Z
Learning: User rudransh-shrivastava confirmed that suggested type safety improvements during Apollo Client migration were no longer relevant, reinforcing their preference to keep migration PRs focused on core migration changes rather than additional improvements.
Applied to files:
frontend/src/app/snapshots/[id]/page.tsxfrontend/src/app/my/mentorship/page.tsx
📚 Learning: 2025-09-21T11:34:33.377Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2178
File: frontend/src/app/about/page.tsx:47-53
Timestamp: 2025-09-21T11:34:33.377Z
Learning: User rudransh-shrivastava prefers to keep Apollo Client migration PRs focused solely on migration-related changes and not include additional bug fixes or improvements that aren't directly related to the migration itself.
Applied to files:
frontend/src/app/snapshots/[id]/page.tsx
📚 Learning: 2025-07-13T11:34:31.823Z
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: frontend/src/app/mentorship/programs/page.tsx:14-14
Timestamp: 2025-07-13T11:34:31.823Z
Learning: In the Next.js frontend mentorship application, there are two distinct types for authentication-related data: ExtendedSession for useSession hook (containing accessToken and user.login properties) and UserRolesData for useUserRoles hook (containing currentUserRoles.roles array). The correct access pattern for GitHub username is `(session as ExtendedSession)?.user?.login`.
Applied to files:
frontend/src/app/members/[memberKey]/page.tsxfrontend/src/app/my/mentorship/page.tsx
🧬 Code graph analysis (21)
frontend/src/types/pullRequest.ts (1)
frontend/src/types/user.ts (1)
User(10-32)
frontend/src/types/committee.ts (1)
frontend/src/types/contributor.ts (1)
Contributor(1-8)
frontend/src/app/projects/[projectKey]/layout.tsx (1)
frontend/src/types/__generated__/projectQueries.generated.ts (1)
GetProjectMetadataDocument(37-37)
frontend/src/app/mentorship/programs/page.tsx (1)
backend/apps/mentorship/api/internal/nodes/enum.py (1)
ProgramStatusEnum(22-27)
frontend/src/types/chapter.ts (1)
frontend/src/types/contributor.ts (1)
Contributor(1-8)
frontend/src/types/milestone.ts (1)
frontend/src/types/user.ts (1)
User(10-32)
frontend/src/types/user.ts (4)
backend/apps/github/models/user.py (1)
User(19-159)frontend/src/types/issue.ts (1)
Issue(3-19)frontend/src/types/release.ts (1)
Release(3-13)frontend/src/types/project.ts (1)
RepositoryCardProps(52-62)
frontend/src/app/organizations/[organizationKey]/repositories/[repositoryKey]/layout.tsx (1)
frontend/src/types/__generated__/repositoryQueries.generated.ts (1)
GetRepositoryMetadataDocument(22-22)
frontend/src/app/settings/api-keys/page.tsx (1)
frontend/src/types/__generated__/apiKeyQueries.generated.ts (3)
GetApiKeysDocument(25-25)CreateApiKeyDocument(26-26)RevokeApiKeyDocument(27-27)
frontend/src/app/snapshots/[id]/layout.tsx (1)
frontend/src/types/__generated__/snapshotQueries.generated.ts (1)
GetSnapshotDetailsMetadataDocument(25-25)
frontend/src/types/release.ts (1)
frontend/src/types/user.ts (1)
User(10-32)
frontend/src/app/projects/dashboard/metrics/[projectKey]/page.tsx (2)
frontend/src/types/healthMetrics.ts (1)
HealthMetricsProps(26-54)frontend/src/types/__generated__/projectsHealthDashboardQueries.generated.ts (1)
GetProjectHealthMetricsDetailsDocument(28-28)
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (3)
frontend/src/app/global-error.tsx (1)
handleAppError(66-86)frontend/src/types/__generated__/programsQueries.generated.ts (1)
GetProgramAndModulesDocument(37-37)frontend/src/utils/dateFormatter.ts (1)
formatDate(1-20)
frontend/src/types/mentorship.ts (2)
backend/apps/mentorship/api/internal/nodes/enum.py (2)
ProgramStatusEnum(22-27)ExperienceLevelEnum(12-18)frontend/src/types/contributor.ts (1)
Contributor(1-8)
frontend/src/app/snapshots/[id]/page.tsx (3)
frontend/src/types/snapshot.ts (1)
SnapshotDetails(5-13)frontend/src/types/__generated__/snapshotQueries.generated.ts (1)
GetSnapshotDetailsDocument(24-24)frontend/src/utils/dateFormatter.ts (1)
formatDate(1-20)
frontend/src/types/project.ts (2)
frontend/src/types/healthMetrics.ts (1)
HealthMetricsProps(26-54)frontend/src/types/contributor.ts (1)
Contributor(1-8)
frontend/src/types/issue.ts (1)
frontend/src/types/user.ts (2)
User(10-32)RepositoryDetails(5-8)
frontend/src/app/members/[memberKey]/page.tsx (2)
frontend/src/types/user.ts (1)
User(10-32)frontend/src/types/__generated__/userQueries.generated.ts (1)
GetUserDataDocument(27-27)
frontend/src/app/my/mentorship/page.tsx (1)
frontend/src/types/__generated__/programsQueries.generated.ts (1)
GetMyProgramsDocument(35-35)
frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx (4)
frontend/src/types/mentorship.ts (1)
Module(42-54)frontend/src/types/__generated__/moduleQueries.generated.ts (1)
GetProgramAdminsAndModulesDocument(30-30)frontend/src/utils/dateFormatter.ts (1)
formatDate(1-20)frontend/src/components/ModuleCard.tsx (1)
getSimpleDuration(98-110)
frontend/src/utils/structuredData.ts (2)
frontend/src/types/user.ts (1)
User(10-32)frontend/src/types/profilePageStructuredData.ts (1)
ProfilePageStructuredData(1-31)
🔇 Additional comments (34)
frontend/src/types/chapter.ts (2)
3-3: No generated Chapter type found under generated; verify codegen outputs or rename local typerg shows a local
export type Chapter = …in frontend/src/types/chapter.ts and no matches under generated; confirm whether codegen emits aChaptertype elsewhere—if it does, re‑export the generated type; otherwise rename this local type (e.g., DomainChapter) or mark it @deprecated to avoid confusion.
5-5: createdAt/updatedAt OK as number for Chapter — no change required.Generated types declare ChapterNode.createdAt/updatedAt as number (e.g. frontend/src/types/generated/snapshotQueries.generated.ts and frontend/src/types/generated/homeQueries.generated.ts), so keep createdAt?: number in frontend/src/types/chapter.ts unless you change the GraphQL scalar mapping.
frontend/src/types/committee.ts (1)
8-8: Require Committee.key or justify optionality
- File: frontend/src/types/committee.ts — don't weaken the primary type. If production always provides the identifier, revert:
- key?: string + key: string
- If some payloads legitimately omit it, introduce a separate partial/preview type (e.g., CommitteePreview) instead of loosening the main Committee type.
- Avoid spreading Committee into JSX (React reserves
key); if spreads are needed, rename the field (e.g.,committeeKey) in a follow‑up.frontend/src/types/healthMetrics.ts (1)
26-54: Optionalizing most fields — confirm null vs undefined semantics and downstream usageFindings: frontend/tsconfig.json sets "strict": false and does not set strictNullChecks or exactOptionalPropertyTypes. I could not locate GraphQL codegen config or package.json in the repo root.
- Verify GraphQL codegen nullability (avoidOptionals / maybeValue / nonOptionalTypename) — does codegen emit null (Maybe) or omit fields (undefined)?
- Scan consumers for arithmetic/sorting/filters and React key usage on newly-optional fields (ageDays, lastCommitDays, score, projectKey, starsCount, forksCount, openIssuesCount, totalIssuesCount) and add runtime guards or adapt types.
- If aligning types, match codegen semantics (e.g., export type Maybe = T | null) instead of mixing null and undefined.
frontend/src/types/mentorship.ts (1)
52-53: Guard optional domains/tags at call sites or default to []domains/tags are now optional — ensure no code does array ops on undefined. Review/adjust these call-sites (guard or default to []):
- frontend/src/app/mentorship/programs/[programKey]/page.tsx — tags={program.tags} (line ~86), domains={program.domains} (line ~87)
- frontend/src/app/my/mentorship/programs/[programKey]/page.tsx — tags={program.tags} (149), domains={program.domains} (150)
- frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx — tags={module.tags} (65), domains={module.domains} (66)
- frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx — tags={module.tags} (65), domains={module.domains} (66)
- frontend/src/components/ModuleForm.tsx — value={formData.domains} (168), value={formData.tags} (182)
- frontend/src/components/ProgramForm.tsx — value={formData.tags} (166), value={formData.domains} (180)
- create/edit pages use parseCommaSeparated(formData.*) — confirm parseCommaSeparated handles undefined (create/page.tsx, modules/create/page.tsx, edit/page.tsx).
- Already guarded (OK): frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx — (program.tags || []).join(', '), (program.domains || []).join(', ') (lines ~79–80); modules edit page has similar guards.
Fix patterns: (x ?? []).map(...), x?.length ?? 0, or default array when forwarding props (e.g., tags={program.tags ?? []}).
frontend/graphql-codegen.ts (2)
34-34: Scalar type configuration aligned with Apollo Client v4 migration.The change from
'unknown'to'any'for the default scalar type follows a practical approach for the Apollo Client v4 migration, where unknown scalar types need to be handled more flexibly during the transition phase.
53-60: Good scalar mapping for Date and DateTime fields.The scalar configuration correctly maps GraphQL Date and DateTime scalars to
'string | number', which aligns with the broader migration pattern seen across the codebase where date fields are being cast to strings for formatting functions.frontend/src/app/mentorship/programs/page.tsx (2)
5-5: Consistent enum usage adoption.Good migration from string literals to the generated
ProgramStatusEnum- this improves type safety and aligns with the broader pattern of using generated GraphQL types throughout the codebase.
55-56: Verify enum value consistency with backendfrontend/src/app/mentorship/programs/page.tsx (lines 55–56):
{programs && programs.filter((p) => p.status === ProgramStatusEnum.Published).map(renderProgramCard)}ast-grep returned no matches for ProgramStatusEnum or generated GraphQL ProgramStatusEnum in the repo. Confirm that ProgramStatusEnum.Published serializes to the backend value 'published'; if it does not, replace with the correct string or update the enum mapping.
frontend/src/app/organizations/[organizationKey]/repositories/[repositoryKey]/layout.tsx (1)
4-4: Successfully migrated to generated GraphQL document.The import and usage update from
GET_REPOSITORY_METADATAtoGetRepositoryMetadataDocumentcorrectly follows the Apollo Client v4 migration pattern with generated document nodes.Also applies to: 17-17
frontend/src/app/projects/[projectKey]/layout.tsx (1)
4-4: Clean migration to generated GraphQL documents.The replacement of
GET_PROJECT_METADATAwithGetProjectMetadataDocumentis consistent with the project-wide migration to generated GraphQL types and maintains the same functional behavior.Also applies to: 16-16
frontend/src/types/organization.ts (1)
2-16: Type flexibility improvements for GraphQL integration.Making most Organization properties optional (except
login) aligns well with GraphQL's nullable field handling and provides better compatibility with generated types. This change supports partial data scenarios common in GraphQL queries.frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx (4)
2-2: Apollo v4 React hooks import correctly updated.The import path change to
@apollo/client/reactaligns with Apollo Client v4's new entrypoint structure.
7-7: Generated GraphQL document migration completed.The migration from
GET_PROGRAM_ADMINS_AND_MODULEStoGetProgramAdminsAndModulesDocumentfollows the established pattern throughout the codebase.Also applies to: 20-20
15-15: Good route parameter typing improvement.Adding explicit types to
useParams<{ programKey: string; moduleKey: string }>()improves type safety.
52-57: Date handling casts are fragile.The type casting of
startedAt/endedAttostringis potentially unsafe if the backend returns epoch seconds. Based on the codegen configuration showing Date/DateTime scalars mapped to'string | number', these fields could be numbers.frontend/src/app/my/mentorship/page.tsx (2)
2-2: **Apollo Client v4 React hooks import is correct.**The import from@apollo/client/reactis correct for Apollo Client v4. React hooks now live in the @apollo/client/react entrypoint, allowing developers to use Apollo Client with any JavaScript framework without React dependencies.
11-11: Successful migration to generated GraphQL document.The replacement of
GET_MY_PROGRAMSwithGetMyProgramsDocumentfollows the established pattern throughout the codebase and maintains the same functionality.Also applies to: 56-56
frontend/src/app/projects/dashboard/metrics/[projectKey]/page.tsx (3)
2-2: Apollo Client v4 React hooks import is correct.The updated import path aligns with Apollo Client v4's new module structure.
17-17: GraphQL document migration completed successfully.The replacement of
GET_PROJECT_HEALTH_METRICS_DETAILSwithGetProjectHealthMetricsDetailsDocumentfollows the established migration pattern.Also applies to: 34-34
27-27: Good route parameter typing enhancement.Adding explicit typing to
useParams<{ projectKey: string }>()improves type safety.frontend/src/types/milestone.ts (1)
4-12: Optionalizing many Milestone fields—verify downstream null-safety.author, body, closedIssuesCount, createdAt, openIssuesCount, repositoryName, and state are now optional. Please double-check all consumers (rendering, sorting, formatting) for unchecked property access and add guards where needed to avoid runtime undefined issues.
frontend/src/types/user.ts (1)
14-31: createdAt/updatedAt widened to number | string and many fields optional—audit call sites.Ensure all usages (e.g., date formatters, comparisons, arithmetic) handle both string and number, and that optional fields are null‑safe. This change can silently break UI if any code assumed required numbers.
frontend/src/types/release.ts (1)
4-5: LGTM: author/isPreRelease made optional to match varying data.Change aligns with broader optionality adjustments and should be safe with null‑checks in consumers.
frontend/src/types/project.ts (1)
19-39: Project surface relaxed—verify consumers for null/shape assumptions.Many fields are now optional (and organizations is a string). Please sanity‑check renderers, filters, and sorters that previously assumed presence or array/object shapes.
frontend/src/app/snapshots/[id]/page.tsx (5)
9-9: Good move to codegen documents.Using
GetSnapshotDetailsDocumentaligns with Apollo v4 + codegen.
22-22: CorrectuseParamstyping.Explicit param typing is appropriate here.
27-29: useQuery usage matches Apollo v4 expectations.Document + variables wiring looks correct.
124-125: Don’t force date scalars to string; pass through as-is.
formatDatealready acceptsnumber | string. The casts are unnecessary and can hide type issues.Apply:
- {formatDate(snapshot.startAt as string)} - {formatDate(snapshot.endAt as string)} + {formatDate(snapshot.startAt)} - {formatDate(snapshot.endAt)}
2-2: Approve: Apollo hook imports are consistent in frontend/src.
Search shows React hook imports use '@apollo/client/react' across frontend/src; remaining '@apollo/client' usages are server-side (ApolloClient/gql) and expected.frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (4)
10-11: LGTM: generated enums and documents are correctly adopted.Using ProgramStatusEnum and the typed GetProgramAndModulesDocument aligns with Apollo v4 + codegen best practices.
36-44: LGTM: useQuery with typed document + skip flag.Typed variables and notifyOnNetworkStatusChange fit the migration; skip guards undefined params.
79-79: Apollo v4 note: refetchQueries are not awaited; verify UX expectations.If the toast/navigation should occur after data is refreshed, explicitly await refetch() or manually await the returned refs; otherwise current behavior is fine.
Minimal alternative:
- await updateProgram({ + await updateProgram({ variables: { /* ... */ }, - refetchQueries: [{ query: GetProgramAndModulesDocument, variables: { programKey } }], + refetchQueries: [{ query: GetProgramAndModulesDocument, variables: { programKey } }], }) + await refetch()
130-133: Harden date formatting and humanize enum; avoid unsafe casts.
- Casting to string can yield "Invalid date" for numeric epochs and can still throw in formatDate. Pass number|string with null guards.
- ProgramStatusEnum values are typically UPPER_CASE; upperFirst keeps them shouting. Lowercase then title‑case for display.
- const programDetails = [ - { label: 'Status', value: upperFirst(program.status) }, - { label: 'Start Date', value: formatDate(program.startedAt as string) }, - { label: 'End Date', value: formatDate(program.endedAt as string) }, + const programDetails = [ + { label: 'Status', value: upperFirst(program.status.toLowerCase()) }, + { label: 'Start Date', value: program.startedAt ? formatDate(program.startedAt as number | string) : 'N/A' }, + { label: 'End Date', value: program.endedAt ? formatDate(program.endedAt as number | string) : 'N/A' },Additionally (elsewhere in this file), for the toast:
- title: `Program status updated to ${upperFirst(newStatus)}`, + title: `Program status updated to ${upperFirst(newStatus.toLowerCase())}`,Optional follow‑up: map Date/DateTime scalars to string in codegen.yml to avoid unknown and casts.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
frontend/src/app/snapshots/[id]/page.tsx (3)
2-2: Confirm import path choice for Apollo v4 hooks.Both '@apollo/client' and '@apollo/client/react' work in v4. The docs generally show root imports; mixing entrypoints across files can increase bundle duplication. If consistency is a goal, consider the root import.
- import { useQuery } from '@apollo/client/react' + import { useQuery } from '@apollo/client'
12-12: Make SnapshotDetails a type-only import.Avoids creating a runtime import for a type-only symbol.
- import { SnapshotDetails } from 'types/snapshot' + import type { SnapshotDetails } from 'types/snapshot'
145-146: Remove stray whitespace text node in JSX.The {' '} fragment adds an unnecessary text child and may trip linters.
- {' '} {snapshot.newChapters.filter((chapter) => chapter.isActive).map(renderChapterCard)}frontend/src/app/mentorship/programs/[programKey]/page.tsx (2)
8-8: Use type‑only import to avoid bundling the types modulePrevents accidental runtime import and improves tree‑shaking.
-import { Program, Module } from 'types/mentorship' +import type { Program, Module } from 'types/mentorship'
14-14: Prefer typed useParams over assertion (match modules page)Consistent, safer typing without a cast.
-const { programKey } = useParams() as { programKey: string } +const { programKey } = useParams<{ programKey: string }>()frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx (2)
8-8: Type‑only import for ModuleAvoids emitting a runtime import.
-import { Module } from 'types/mentorship' +import type { Module } from 'types/mentorship'
17-17: Optional: type admins stateIf DetailsCard expects a Contributor[] (likely), type it to preserve inference.
-const [admins, setAdmins] = useState(null) +import type { Contributor } from 'types/mentorship' +const [admins, setAdmins] = useState<Contributor[] | null>(null)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx(1 hunks)frontend/src/app/mentorship/programs/[programKey]/page.tsx(2 hunks)frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx(2 hunks)frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx(1 hunks)frontend/src/app/projects/dashboard/metrics/[projectKey]/page.tsx(3 hunks)frontend/src/app/settings/api-keys/page.tsx(5 hunks)frontend/src/app/snapshots/[id]/page.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/src/app/settings/api-keys/page.tsx
- frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx
- frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2178
File: frontend/src/app/snapshots/[id]/page.tsx:0-0
Timestamp: 2025-09-21T17:04:48.154Z
Learning: User rudransh-shrivastava confirmed that suggested type safety improvements during Apollo Client migration were no longer relevant, reinforcing their preference to keep migration PRs focused on core migration changes rather than additional improvements.
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2178
File: frontend/src/app/about/page.tsx:47-53
Timestamp: 2025-09-21T11:34:33.377Z
Learning: User rudransh-shrivastava prefers to keep Apollo Client migration PRs focused solely on migration-related changes and not include additional bug fixes or improvements that aren't directly related to the migration itself.
📚 Learning: 2025-09-21T17:04:48.154Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2178
File: frontend/src/app/snapshots/[id]/page.tsx:0-0
Timestamp: 2025-09-21T17:04:48.154Z
Learning: User rudransh-shrivastava confirmed that suggested type safety improvements during Apollo Client migration were no longer relevant, reinforcing their preference to keep migration PRs focused on core migration changes rather than additional improvements.
Applied to files:
frontend/src/app/snapshots/[id]/page.tsx
📚 Learning: 2025-09-21T11:34:33.377Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2178
File: frontend/src/app/about/page.tsx:47-53
Timestamp: 2025-09-21T11:34:33.377Z
Learning: User rudransh-shrivastava prefers to keep Apollo Client migration PRs focused solely on migration-related changes and not include additional bug fixes or improvements that aren't directly related to the migration itself.
Applied to files:
frontend/src/app/snapshots/[id]/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 is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
Applied to files:
frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsxfrontend/src/app/mentorship/programs/[programKey]/page.tsxfrontend/src/app/projects/dashboard/metrics/[projectKey]/page.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details. These queries cannot be removed or merged as they serve different use cases in the application.
Applied to files:
frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsxfrontend/src/app/mentorship/programs/[programKey]/page.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details.
Applied to files:
frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsxfrontend/src/app/mentorship/programs/[programKey]/page.tsx
📚 Learning: 2025-07-08T17:24:36.501Z
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: backend/apps/mentorship/graphql/mutations/program.py:41-44
Timestamp: 2025-07-08T17:24:36.501Z
Learning: In the mentorship program GraphQL mutations, date validation is handled at the GraphQL schema/node level in the input types (CreateProgramInput, UpdateProgramInput), preventing null values from reaching the mutation logic where date comparisons are performed.
Applied to files:
frontend/src/app/mentorship/programs/[programKey]/page.tsx
📚 Learning: 2025-07-13T07:31:06.511Z
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: frontend/src/components/ModuleCard.tsx:53-55
Timestamp: 2025-07-13T07:31:06.511Z
Learning: In Next.js 13+ app router, useRouter from 'next/navigation' does not provide asPath or query properties. Use useParams to extract route parameters and usePathname to get the current pathname instead.
Applied to files:
frontend/src/app/mentorship/programs/[programKey]/page.tsxfrontend/src/app/projects/dashboard/metrics/[projectKey]/page.tsx
📚 Learning: 2025-07-13T11:29:25.245Z
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: frontend/src/app/mentorship/programs/page.tsx:59-61
Timestamp: 2025-07-13T11:29:25.245Z
Learning: In Next.js 13+ app router, components with the 'use client' directive run entirely on the client side and don't require window object existence checks or SSR hydration considerations. Direct access to window.location and other browser APIs is safe in client components.
Applied to files:
frontend/src/app/mentorship/programs/[programKey]/page.tsx
📚 Learning: 2025-06-20T16:12:59.256Z
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.
Applied to files:
frontend/src/app/projects/dashboard/metrics/[projectKey]/page.tsx
📚 Learning: 2025-06-20T16:12:59.256Z
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.
Applied to files:
frontend/src/app/projects/dashboard/metrics/[projectKey]/page.tsx
🧬 Code graph analysis (4)
frontend/src/app/snapshots/[id]/page.tsx (2)
frontend/src/types/snapshot.ts (1)
SnapshotDetails(5-13)frontend/src/types/__generated__/snapshotQueries.generated.ts (1)
GetSnapshotDetailsDocument(24-24)
frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx (2)
frontend/src/types/mentorship.ts (1)
Module(42-54)frontend/src/types/__generated__/moduleQueries.generated.ts (1)
GetProgramAdminsAndModulesDocument(30-30)
frontend/src/app/mentorship/programs/[programKey]/page.tsx (1)
frontend/src/types/__generated__/programsQueries.generated.ts (1)
GetProgramAndModulesDocument(37-37)
frontend/src/app/projects/dashboard/metrics/[projectKey]/page.tsx (2)
frontend/src/types/healthMetrics.ts (1)
HealthMetricsProps(26-54)frontend/src/types/__generated__/projectsHealthDashboardQueries.generated.ts (1)
GetProjectHealthMetricsDetailsDocument(28-28)
🔇 Additional comments (12)
frontend/src/app/snapshots/[id]/page.tsx (3)
9-9: Good: switched to generated Document for typed queries.Using GetSnapshotDetailsDocument aligns with Apollo v4 + codegen best practices.
22-22: Typed route params look correct.useParams<{ id: string }>() is appropriate for /snapshots/[id].
27-29: LGTM on useQuery with generated document.Inference from the typed DocumentNode obviates generics; variables usage is correct.
frontend/src/app/mentorship/programs/[programKey]/page.tsx (2)
2-2: Apollo v4 hook import — LGTMCorrectly importing React hooks from @apollo/client/react.
7-7: Switch to generated document — LGTM; Date/DateTime scalars emitted as any in generated types (fix required)graphql-codegen.ts maps Date/DateTime -> 'string | number' for ./src/types/generated/graphql.ts, but operation-generated files still emit startedAt/endedAt as any.
Affected: frontend/src/types/generated/programsQueries.generated.ts, programsMutations.generated.ts, moduleQueries.generated.ts, moduleMutations.generated.ts.
Action: apply the scalars mapping to the operation outputs (or use a global mapping), re-run codegen, and verify startedAt/endedAt are typed 'string | number' (not any).frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx (3)
2-2: Apollo v4 hook import — LGTM
7-7: Generated document + useQuery wiring — LGTMDocument and variables match the query signature.
Also applies to: 20-25
15-15: Typed useParams — LGTMGood switch to generic typing for route params.
frontend/src/app/projects/dashboard/metrics/[projectKey]/page.tsx (4)
27-27: TypeduseParamsfor App Router is spot on.Matches prior learning about extracting params in Next 13+. Thanks for keeping this scoped to migration.
34-36:useQuerywith the generated document and inline variables — LGTM.Types are inferred from the DocumentNode; no generics needed.
17-17: Approve — generated DocumentNode usage is correct; legacy constant only defined, not referenced elsewhereGetProjectHealthMetricsDetailsDocument is imported/used in frontend/src/app/projects/dashboard/metrics/[projectKey]/page.tsx; the legacy GET_PROJECT_HEALTH_METRICS_DETAILS only exists at frontend/src/server/queries/projectsHealthDashboardQueries.ts and has no other references — safe to remove the legacy constant if you want to clean up.
2-2: Switch to@apollo/client/react— app code migrated; tests need updatesApp pages/hooks/components now import from
@apollo/client/react(v4 migration looks correct). Server-side modules (frontend/src/server/** and utils/helpers/apolloClient.ts) correctly keep top-level@apollo/clientfor non‑React exports (ApolloClient, gql, link/context).
Many unit tests still import hooks from the top-level@apollo/client(e.g. frontend/tests/unit/pages/About.test.tsx, ProjectDetails.test.tsx, CreateModule.test.tsx, EditModule.test.tsx, ApiKeysPage.test.tsx and other files under frontend/tests/unit/pages/**) — update those tests or their jest.mock targets to import/mock@apollo/client/reactso hooks resolve to the React entrypoint.
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
frontend/__tests__/unit/pages/About.test.tsx (2)
532-541: Do not compare to legacy GET_ if the component uses generated documents.*Align this condition with operation-name matching or generated documents.
116-133: Fix fragile useQuery mock in About.test.tsx — match generated documents or operation namefrontend/src/app/about/page.tsx uses generated documents (GetProjectMetadataDocument, GetTopContributorsDocument, GetLeaderDataDocument); the test's mock in frontend/tests/unit/pages/About.test.tsx (lines 116–133) currently compares against GET_* and will break. Update the mock to either match the operation name (document.definitions[0].name.value) or import and compare against the same generated Document objects used by the component.
frontend/__tests__/unit/pages/ApiKeysPage.test.tsx (1)
67-76: Replace legacy mutation identity checks with generated DocumentNodesfrontend/src/app/settings/api-keys/page.tsx imports CreateApiKeyDocument and RevokeApiKeyDocument; the test currently compares against CREATE_API_KEY/REVOKE_API_KEY and will break. Update the mock in frontend/tests/unit/pages/ApiKeysPage.test.tsx (lines 67–76) to compare against CreateApiKeyDocument / RevokeApiKeyDocument (or match by operation name) instead of the old constants.
🧹 Nitpick comments (31)
frontend/__tests__/unit/utils/structuredData.test.ts (3)
1-1: Use type‑only import for clarity and to avoid accidental value import.
Switch toimport typesinceUseris a type alias.-import { User } from 'types/user' +import type { User } from 'types/user'
10-19: Fix mockUser date fields to match expected normalized ISO timestamps.
Your expectations use epoch+2020s and +2021s (1970-01-01T00:33:40Z / 00:33:41Z). Update the fixture to numbers to avoid mismatch.- createdAt: '2020-01-01T00:00:00Z', + createdAt: 2020, email: '[email protected]', @@ - updatedAt: '2021-02-03T00:00:00Z', + updatedAt: 2021,If
formatISODatetreats numbers as seconds (not ms), this will pass. If it expects ms, use2000and2001seconds × 1000 respectively. Please run the unit tests to confirm.
61-72: Fix minimalUser date fields to match expected normalization.
Mirror the same numeric dates so the second test remains consistent.- createdAt: '2020-01-01T00:00:00Z', + createdAt: 2020, @@ - updatedAt: '2021-02-03T00:00:00Z', + updatedAt: 2021,frontend/__tests__/unit/pages/MyMentorship.test.tsx (1)
33-33: Casting via unknown works; optionally prefer jest.MockedFunction typing.
Keeps types tighter without double cast.Apply if desired:
-const mockUseQuery = useQuery as unknown as jest.Mock +const mockUseQuery = useQuery as jest.MockedFunction<typeof useQuery>If this trips on generics/overloads in your tsconfig, keep the current cast to stay unblocked for migration.
frontend/__tests__/unit/pages/ProjectsHealthDashboardMetrics.test.tsx (1)
64-69: Optional: DRY the useQuery mock castAvoid repeating
(useQuery as unknown as jest.Mock)by defining a single typed alias and reusing it.Add once near the imports:
const mockedUseQuery = useQuery as unknown as jest.MockThen update the call sites:
- ;(useQuery as unknown as jest.Mock).mockReturnValue({ + mockedUseQuery.mockReturnValue({(Apply the same replacement at Lines 75, 88, and 177.)
Also applies to: 75-80, 88-93, 177-182
frontend/__tests__/unit/pages/ProjectDetails.test.tsx (2)
46-46: Remove repetitive double-casts by aliasing the mock once.Define a typed alias once and use it, improving readability and avoiding repeated casts.
Apply this diff for the repeated usage lines:
- ;(useQuery as unknown as jest.Mock).mockReturnValue({ + useQueryMock.mockReturnValue({Add this alias near the top (after imports):
const useQueryMock = useQuery as unknown as jest.MockAlso applies to: 58-58, 72-72, 89-89, 156-156, 171-171, 197-197, 218-218, 264-264, 280-280
58-61: Set loading explicitly for the loading-state test.To avoid relying on component internals, return loading: true when asserting the spinner.
- ;(useQuery as unknown as jest.Mock).mockReturnValue({ - data: null, - error: null, - }) + ;(useQuery as unknown as jest.Mock).mockReturnValue({ + data: null, + loading: true, + error: null, + })frontend/__tests__/unit/pages/EditModule.test.tsx (1)
26-29: Preserve all exports when mocking @apollo/client.Spread the actual module to avoid unintentionally nulling other exports used transitively (e.g., gql, types, utility fns).
-jest.mock('@apollo/client', () => ({ - useApolloClient: jest.fn(), - gql: jest.requireActual('@apollo/client').gql, -})) +jest.mock('@apollo/client', () => ({ + ...jest.requireActual('@apollo/client'), + useApolloClient: jest.fn(), +}))frontend/__tests__/unit/pages/ModuleDetails.test.tsx (1)
34-34: Casting approach is fine; optional stronger typing.Current cast is acceptable. Optionally, tighten typing if TS allows:
- const mockUseQuery = useQuery as unknown as jest.Mock + const mockUseQuery = useQuery as unknown as jest.MockedFunction<typeof useQuery>frontend/__tests__/unit/pages/SnapshotDetails.test.tsx (4)
8-11: Mocking the React entrypoint looks good; reduce repeated casts.To avoid repeating
(useQuery as unknown as jest.Mock)everywhere, create a single alias:Add near the imports:
const mockUseQuery = useQuery as unknown as jest.MockThen replace usages with
mockUseQuery.mockReturnValue(...).
51-56: Fix brittle loading assertion (getAllByAltText evaluated too early).
getAllByAltTextis called before thewaitFor, so the test can fail immediately if the spinner renders asynchronously. UsefindAllByAltTextor move the query insidewaitFor.Replace the assertion block with one of:
// Option A (preferred) const loadingSpinner = await screen.findAllByAltText('Loading indicator') expect(loadingSpinner.length).toBeGreaterThan(0) // Option B await waitFor(() => { expect(screen.getAllByAltText('Loading indicator').length).toBeGreaterThan(0) })
66-69: Be explicit about loading=false to avoid flakiness.Not required, but adding
loading: falsemakes intent clear if the component branches onloading.
83-86: Use an Error/ApolloError instance for the error shape.Right now
erroris{ error: Error }. If the component checkserror.messageor expects an ApolloError, this may misbehave.Consider:
import { ApolloError } from '@apollo/client' // ... const mockError = new ApolloError({ errorMessage: 'GraphQL error' }) // ... (useQuery as unknown as jest.Mock).mockReturnValue({ data: null, error: mockError, })Or, minimally:
const mockError = new Error('GraphQL error') // ... error: mockError,frontend/__tests__/unit/pages/ProgramDetailsMentorship.test.tsx (1)
23-23: DRY the repeated casts by defining a single typed mock for useQuery.Keeps casts in one place and improves readability.
Apply this change here:
- ;(useQuery as unknown as jest.Mock).mockReturnValue({ + mockedUseQuery.mockReturnValue({ data: mockProgramDetailsData, loading: false, refetch: jest.fn(), })Add this once near the imports (outside describe):
// after imports const mockedUseQuery = useQuery as unknown as jest.MockedFunction<typeof useQuery>frontend/__tests__/unit/pages/CreateProgram.test.tsx (1)
37-37: Prefer typed mock helper over double-castUse jest.mocked for cleaner typing and readability.
- ;(useMutation as unknown as jest.Mock).mockReturnValue([mockCreateProgram, { loading: false }]) + jest.mocked(useMutation).mockReturnValue([mockCreateProgram, { loading: false }])frontend/__tests__/unit/pages/ModuleDetailsPage.test.tsx (1)
34-34: Prefer precise typing overunknown as jest.Mock.Use
jest.MockedFunctionfor safer typings.Apply:
- const mockUseQuery = useQuery as unknown as jest.Mock + const mockUseQuery = useQuery as jest.MockedFunction<typeof useQuery>frontend/__tests__/unit/pages/CommitteeDetails.test.tsx (1)
39-49: Fix potential flakiness in loading-spinner assertion.Capturing elements before waiting can throw; use findAllBy* or assert inside waitFor.
Apply:
- const loadingSpinner = screen.getAllByAltText('Loading indicator') - await waitFor(() => { - expect(loadingSpinner.length).toBeGreaterThan(0) - }) + const loadingSpinner = await screen.findAllByAltText('Loading indicator') + expect(loadingSpinner.length).toBeGreaterThan(0)frontend/__tests__/unit/pages/ProjectHealthDashboardMetricsDetails.test.tsx (1)
40-44: Avoid repeating casts; create a local mock alias for useQueryDefine a typed alias once and use it across tests to reduce noise.
Apply this diff to the current line:
- ;(useQuery as unknown as jest.Mock).mockReturnValue({ + ;mockUseQuery.mockReturnValue({Add this helper near the top (e.g., after the Apollo mock):
// place once near the mocks const mockUseQuery = useQuery as unknown as jest.Mock;frontend/__tests__/unit/pages/ProjectsHealthDashboardOverview.test.tsx (4)
35-37: Mock error shape should be an Error (or ApolloError), not{ error: Error }Use a direct Error to better match
useQuery’serrortype and typical component checks.Apply:
-const mockError = { - error: new Error('GraphQL error'), -} +const mockError = new Error('GraphQL error')
41-46: Casting works; optional: tighten typing with jest.MockedFunctionThis avoids double-casting to
unknownand preservesuseQuery’s call signature in mocks.Apply:
-;(useQuery as unknown as jest.Mock).mockReturnValue({ +;(useQuery as unknown as jest.MockedFunction<typeof useQuery>).mockReturnValue({ data: mockProjectsDashboardOverviewData, loading: false, error: null, })
52-56: Same optional typing improvement for the loading state mockKeeps mocks consistently typed.
Apply:
-;(useQuery as unknown as jest.Mock).mockReturnValue({ +;(useQuery as unknown as jest.MockedFunction<typeof useQuery>).mockReturnValue({ data: null, loading: true, error: null, })
65-69: Error state mock: align with direct Error instanceAfter adjusting
mockErrortoError, this will better reflect runtime behavior and component expectations.Optionally, use
.mockReturnValueOnce(...)here to avoid cross-test bleed and rely on the default set inbeforeEach.frontend/__tests__/unit/pages/UserDetails.test.tsx (1)
47-47: Explicitly setloadingin alluseQuerymocks; fix the loading-state test; optional typed helper to reduce casts.
- Some overrides omit
loading. Be explicit to avoid fragile tests if component logic changes.- The “renders loading state” test should mock
loading: trueto reflect intent.Apply this diff for the loading test:
- ;(useQuery as unknown as jest.Mock).mockReturnValue({ - data: null, - error: null, - }) + ;(useQuery as unknown as jest.Mock).mockReturnValue({ + data: null, + error: null, + loading: true, + })Apply this diff for the error test:
- ;(useQuery as unknown as jest.Mock).mockReturnValue({ - data: null, - error: mockError, - }) + ;(useQuery as unknown as jest.Mock).mockReturnValue({ + data: null, + error: mockError, + loading: false, + })Optional (to reduce repeated casts):
// near the top of the file const mockedUseQuery = useQuery as unknown as jest.Mock; // usage mockedUseQuery.mockReturnValue({ data: ..., loading: false, error: null })Also applies to: 63-66, 76-83, 102-106, 123-127, 141-145, 159-163, 181-185, 205-209, 232-236, 255-258, 270-273, 339-343, 357-361, 376-380, 395-398, 412-416, 430-433, 452-456, 477-481, 492-495
frontend/__tests__/unit/pages/CreateModule.test.tsx (2)
59-66: Prefer precise mock typing over unknown casts.Use jest.MockedFunction to avoid unsafe unknown as jest.Mock casts.
- ;(useQuery as unknown as jest.Mock).mockReturnValue({ + ;(useQuery as jest.MockedFunction<typeof useQuery>).mockReturnValue({ data: { getProgram: { admins: [{ login: 'admin-user' }], }, }, loading: false, })
67-70: Type the mutation mock tuple precisely.This keeps the mock shape aligned with Apollo’s useMutation return type.
- ;(useMutation as unknown as jest.Mock).mockReturnValue([ + ;(useMutation as jest.MockedFunction<typeof useMutation>).mockReturnValue([ mockCreateModule.mockResolvedValue({}), { loading: false }, ])frontend/__tests__/unit/pages/RepositoryDetails.test.tsx (2)
49-60: Setloading: trueand avoid pre-querying beforewaitForin the loading test.
Currently the mock omitsloading, andgetAllByAltTextis called before the UI can update.Apply this diff to set loading explicitly:
- ;(useQuery as unknown as jest.Mock).mockReturnValue({ - data: null, - error: null, - }) + ;(useQuery as unknown as jest.Mock).mockReturnValue({ + data: null, + loading: true, + error: null, + })Optionally, prefer an async query to reduce flakiness:
// Replace the getAllByAltText + waitFor with: const loadingSpinner = await screen.findAllByAltText('Loading indicator') expect(loadingSpinner.length).toBeGreaterThan(0)
82-86: Use anError(orApolloError) directly for theerrorfield.
Passing{ error: Error }nests the error and may not match expectations.Apply this diff to pass the actual error:
- error: mockError, + error: mockError.error,Alternatively, construct an ApolloError if you want closer parity with v4:
import { ApolloError } from '@apollo/client' // ... error: new ApolloError({ networkError: new Error('GraphQL error') }),frontend/__tests__/unit/pages/EditProgram.test.tsx (1)
38-38: Prefer typed mocking to avoidunknown as jest.MockcastsUse
jest.mocked(useQuery)orMockedFunction<typeof useQuery>for type-safe returns.Example:
import type { MockedFunction } from 'jest-mock' const mockUseQuery = useQuery as unknown as MockedFunction<typeof useQuery> mockUseQuery.mockReturnValue({ loading: true } as any)frontend/__tests__/unit/pages/ProgramDetails.test.tsx (1)
23-27: Avoid repeated casts by extracting a typed alias once.Define a single const and reuse to reduce repetition.
Apply this diff here and reuse the alias throughout the file:
describe('ProgramDetailsPage', () => { + const mockUseQuery = useQuery as unknown as jest.Mock beforeEach(() => { - ;(useQuery as unknown as jest.Mock).mockReturnValue({ + mockUseQuery.mockReturnValue({ data: mockProgramDetailsData, loading: false, refetch: jest.fn(), }) })Change other occurrences similarly (see Lines 35 and 48).
frontend/__tests__/unit/pages/Home.test.tsx (1)
73-78: Extract a single mock alias to avoid repeated casts.Define mockUseQuery once and reuse.
Apply:
describe('Home', () => { let mockRouter: { push: jest.Mock } beforeEach(() => { - ;(useQuery as unknown as jest.Mock).mockReturnValue({ + const mockUseQuery = useQuery as unknown as jest.Mock + mockUseQuery.mockReturnValue({ data: mockGraphQLData, loading: false, error: null, })Replicate for other occurrences below (Lines 88, 113, 196, 248).
frontend/__tests__/unit/pages/ApiKeysPage.test.tsx (1)
51-52: Typed aliases for mocks improve clarity.Good pattern; consider jest.MockedFunction<typeof useQuery/useMutation> for tighter typing (optional).
-const mockUseQuery = useQuery as unknown as jest.Mock -const mockUseMutation = useMutation as unknown as jest.Mock +const mockUseQuery = useQuery as jest.MockedFunction<typeof useQuery> +const mockUseMutation = useMutation as jest.MockedFunction<typeof useMutation>
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
frontend/__tests__/unit/components/ProgramCard.test.tsx(3 hunks)frontend/__tests__/unit/components/SingleModuleCard.test.tsx(2 hunks)frontend/__tests__/unit/data/mockProgramData.ts(2 hunks)frontend/__tests__/unit/pages/About.test.tsx(14 hunks)frontend/__tests__/unit/pages/ApiKeysPage.test.tsx(3 hunks)frontend/__tests__/unit/pages/ChapterDetails.test.tsx(7 hunks)frontend/__tests__/unit/pages/CommitteeDetails.test.tsx(7 hunks)frontend/__tests__/unit/pages/CreateModule.test.tsx(3 hunks)frontend/__tests__/unit/pages/CreateProgram.test.tsx(2 hunks)frontend/__tests__/unit/pages/EditModule.test.tsx(5 hunks)frontend/__tests__/unit/pages/EditProgram.test.tsx(5 hunks)frontend/__tests__/unit/pages/Home.test.tsx(7 hunks)frontend/__tests__/unit/pages/ModuleDetails.test.tsx(3 hunks)frontend/__tests__/unit/pages/ModuleDetailsPage.test.tsx(3 hunks)frontend/__tests__/unit/pages/MyMentorship.test.tsx(2 hunks)frontend/__tests__/unit/pages/OrganizationDetails.test.tsx(9 hunks)frontend/__tests__/unit/pages/ProgramDetails.test.tsx(4 hunks)frontend/__tests__/unit/pages/ProgramDetailsMentorship.test.tsx(4 hunks)frontend/__tests__/unit/pages/ProjectDetails.test.tsx(11 hunks)frontend/__tests__/unit/pages/ProjectHealthDashboardMetricsDetails.test.tsx(5 hunks)frontend/__tests__/unit/pages/ProjectsHealthDashboardMetrics.test.tsx(5 hunks)frontend/__tests__/unit/pages/ProjectsHealthDashboardOverview.test.tsx(4 hunks)frontend/__tests__/unit/pages/RepositoryDetails.test.tsx(7 hunks)frontend/__tests__/unit/pages/SnapshotDetails.test.tsx(9 hunks)frontend/__tests__/unit/pages/Snapshots.test.tsx(6 hunks)frontend/__tests__/unit/pages/UserDetails.test.tsx(25 hunks)frontend/__tests__/unit/utils/structuredData.test.ts(2 hunks)
✅ Files skipped from review due to trivial changes (3)
- frontend/tests/unit/pages/ChapterDetails.test.tsx
- frontend/tests/unit/pages/Snapshots.test.tsx
- frontend/tests/unit/pages/OrganizationDetails.test.tsx
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2178
File: frontend/src/app/snapshots/[id]/page.tsx:0-0
Timestamp: 2025-09-21T17:04:48.154Z
Learning: User rudransh-shrivastava confirmed that suggested type safety improvements during Apollo Client migration were no longer relevant, reinforcing their preference to keep migration PRs focused on core migration changes rather than additional improvements.
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2178
File: frontend/src/app/about/page.tsx:47-53
Timestamp: 2025-09-21T11:34:33.377Z
Learning: User rudransh-shrivastava prefers to keep Apollo Client migration PRs focused solely on migration-related changes and not include additional bug fixes or improvements that aren't directly related to the migration itself.
📚 Learning: 2025-07-13T07:31:06.511Z
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: frontend/src/components/ModuleCard.tsx:53-55
Timestamp: 2025-07-13T07:31:06.511Z
Learning: In Next.js 13+ app router, useRouter from 'next/navigation' does not provide asPath or query properties. Use useParams to extract route parameters and usePathname to get the current pathname instead.
Applied to files:
frontend/__tests__/unit/pages/ModuleDetailsPage.test.tsxfrontend/__tests__/unit/pages/EditProgram.test.tsxfrontend/__tests__/unit/pages/CreateModule.test.tsxfrontend/__tests__/unit/pages/ModuleDetails.test.tsxfrontend/__tests__/unit/pages/EditModule.test.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/pages/ProgramDetails.test.tsxfrontend/__tests__/unit/pages/EditProgram.test.tsxfrontend/__tests__/unit/pages/ProgramDetailsMentorship.test.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/__tests__/unit/pages/ProgramDetails.test.tsxfrontend/__tests__/unit/pages/ProgramDetailsMentorship.test.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/__tests__/unit/pages/ProgramDetails.test.tsxfrontend/__tests__/unit/pages/ProgramDetailsMentorship.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/pages/ProgramDetails.test.tsxfrontend/__tests__/unit/pages/MyMentorship.test.tsxfrontend/__tests__/unit/pages/ApiKeysPage.test.tsxfrontend/__tests__/unit/pages/ProjectDetails.test.tsxfrontend/__tests__/unit/pages/CreateProgram.test.tsxfrontend/__tests__/unit/pages/ProgramDetailsMentorship.test.tsxfrontend/__tests__/unit/pages/CommitteeDetails.test.tsx
📚 Learning: 2025-07-13T11:34:31.823Z
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: frontend/src/app/mentorship/programs/page.tsx:14-14
Timestamp: 2025-07-13T11:34:31.823Z
Learning: In the Next.js frontend mentorship application, there are two distinct types for authentication-related data: ExtendedSession for useSession hook (containing accessToken and user.login properties) and UserRolesData for useUserRoles hook (containing currentUserRoles.roles array). The correct access pattern for GitHub username is `(session as ExtendedSession)?.user?.login`.
Applied to files:
frontend/__tests__/unit/components/SingleModuleCard.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/pages/EditProgram.test.tsxfrontend/__tests__/unit/pages/CreateModule.test.tsxfrontend/__tests__/unit/pages/EditModule.test.tsx
📚 Learning: 2025-06-20T16:12:59.256Z
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.
Applied to files:
frontend/__tests__/unit/pages/ProjectsHealthDashboardMetrics.test.tsx
📚 Learning: 2025-06-20T16:12:59.256Z
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.
Applied to files:
frontend/__tests__/unit/pages/ProjectsHealthDashboardMetrics.test.tsx
🧬 Code graph analysis (4)
frontend/__tests__/unit/components/SingleModuleCard.test.tsx (1)
backend/apps/mentorship/api/internal/nodes/enum.py (2)
ProgramStatusEnum(22-27)ExperienceLevelEnum(12-18)
frontend/__tests__/unit/data/mockProgramData.ts (1)
backend/apps/mentorship/api/internal/nodes/enum.py (1)
ProgramStatusEnum(22-27)
frontend/__tests__/unit/utils/structuredData.test.ts (1)
frontend/src/types/user.ts (1)
User(10-32)
frontend/__tests__/unit/components/ProgramCard.test.tsx (1)
backend/apps/mentorship/api/internal/nodes/enum.py (1)
ProgramStatusEnum(22-27)
🔇 Additional comments (80)
frontend/__tests__/unit/utils/structuredData.test.ts (2)
5-5: LGTM: test fixture typed withUser.
Type aligns withtypes/user.ts.
26-31: Confirm units for date normalization (seconds vs milliseconds).
The expected values imply seconds-based normalization. If the helper uses ms, adjust inputs (or expectations) accordingly.frontend/__tests__/unit/pages/MyMentorship.test.tsx (2)
1-1: Switching hook import to '@apollo/client/react' is correct for Apollo v4.
Matches the new recommended subpath for React hooks.
8-14: Ensure the component imports hooks from the same @apollo/client subpath you're mocking
You mock '@apollo/client/react' — that will not stub imports from '@apollo/client'. Either mock '@apollo/client' instead or change the component to import from '@apollo/client/react'. Automated search didn't locate the MyMentorship page in this repo; confirm the component's import path.frontend/__tests__/unit/pages/ProjectsHealthDashboardMetrics.test.tsx (2)
1-1: LGTM: switched hooks to @apollo/client/reactCorrect entrypoint for Apollo v4 React hooks.
Please confirm the page under test also imports hooks from '@apollo/client/react' so this mock reliably intercepts calls.
6-9: Mocking strategy looks correctSpreading the real module and overriding useQuery with a jest.fn is appropriate for these tests.
frontend/__tests__/unit/pages/ProjectDetails.test.tsx (2)
1-1: Apollo v4 hooks entrypoint looks correct.Good switch to '@apollo/client/react'. Please ensure the component under test imports hooks from the same entrypoint; otherwise the mock won’t intercept.
8-11: Mock target updated to the react entrypoint — LGTM.Spreading the actual exports and overriding useQuery keeps behavior consistent.
frontend/__tests__/unit/pages/EditModule.test.tsx (4)
20-24: React-surface mocks look correct.Spreading actual and overriding
useQuery/useMutationis appropriate for this test.
63-82: Query mock shape aligns with expected component usage.Admins and module fields look consistent for rendering and form population.
83-86: Mutation mock wiring is fine.Returning
[fn, { loading: false }]matches Apollo hook tuple.
127-127: Loading-state mock is fine.This will exercise the spinner path as intended.
frontend/__tests__/unit/pages/ModuleDetails.test.tsx (2)
13-16: Mocking the React entrypoint is aligned with the new import path.Good partial mock retaining real exports and overriding only useQuery.
1-1: Correct Apollo v4 hook import path.LGTM — verified no React hooks are imported from the top-level '@apollo/client'; hooks are imported from '@apollo/client/react' in tests and app code, so the mock will intercept correctly.
frontend/__tests__/unit/pages/SnapshotDetails.test.tsx (6)
39-44: Default mock shape LGTM.Covers data/loading/error for the happy path.
103-106: LGTM.Happy path mock for navigation test is fine.
122-125: LGTM.Chapter navigation test wiring is consistent.
141-144: LGTM.Release rendering test wiring matches the new import surface.
157-167: LGTM.Good negative-case coverage by zeroing the new* arrays.
1-1: Correct v4 import path — approved.Client hooks are imported from '@apollo/client/react' across the repo; server-side imports from '@apollo/client' (ApolloClient/gql/links) are intentional. No stray hook imports found.
frontend/__tests__/unit/components/SingleModuleCard.test.tsx (1)
7-7: Confirmed: generated enums export ExperienceLevelEnum and ProgramStatusEnumfrontend/src/types/generated/graphql.ts exports ExperienceLevelEnum and ProgramStatusEnum — the imports in the tests are correct.
frontend/__tests__/unit/pages/ProgramDetailsMentorship.test.tsx (4)
8-12: Mock target updated to @apollo/client/react — aligned with the migration.Good move to mock the React entrypoint so it matches the new hook imports.
35-38: Same refactor applies: reuse the typed mock helper.- ;(useQuery as unknown as jest.Mock).mockReturnValue({ + mockedUseQuery.mockReturnValue({ loading: true, data: null, })
48-51: Same refactor applies here as well.- ;(useQuery as unknown as jest.Mock).mockReturnValue({ + mockedUseQuery.mockReturnValue({ loading: false, data: { program: null }, })
1-1: Confirmed — mock will apply (both test and page use '@apollo/client/react').frontend/tests/unit/pages/ProgramDetailsMentorship.test.tsx mocks useQuery from '@apollo/client/react' and frontend/src/app/my/mentorship/programs/[programKey]/page.tsx imports useMutation/useQuery from '@apollo/client/react'; no hook imports from '@apollo/client' found in mentorship pages.
frontend/__tests__/unit/pages/CreateProgram.test.tsx (1)
1-1: Correct Apollo v4 React entrypoint for hooksUsing '@apollo/client/react' aligns with Apollo Client v4’s React entrypoint and the PR’s migration approach.
frontend/__tests__/unit/pages/ModuleDetailsPage.test.tsx (1)
13-16: Mock wiring looks correct.Mock path matches the import; spreading
requireActualpreserves other exports.frontend/__tests__/unit/pages/CommitteeDetails.test.tsx (7)
8-11: Mocking the React entry and overriding useQuery looks good.Preserves actual exports while replacing useQuery with a jest.fn.
29-33: Sane default mock shape for useQuery.Provides data and error defaults expected by the component.
64-67: Good: error-path mock for “not found”.Matches the component’s expected error handling.
94-97: Good: covers incomplete contributor names.Validates graceful fallback to login.
114-118: Good: GraphQL error case exercised.Asserts user-facing message on request failure.
127-130: Good: sponsor block expectation remains explicit.Keeps behavior pinned during migration.
1-1: Correct import path for Apollo v4 hooks.Hook imports use '@apollo/client/react' now — verified: no remaining useQuery/useMutation/useApolloClient/etc. imports from '@apollo/client'.
frontend/__tests__/unit/pages/ProjectHealthDashboardMetricsDetails.test.tsx (3)
14-16: Module mock updated to @apollo/client/react looks goodPreserving actual exports and overriding useQuery is correct for Apollo v4.
52-56: Same DRY suggestion as above for repeated castsUse the mockUseQuery alias here as well.
- ;(useQuery as unknown as jest.Mock).mockReturnValue({ + ;mockUseQuery.mockReturnValue({
65-69: Same DRY suggestion as above for repeated castsUse the mockUseQuery alias here too.
- ;(useQuery as unknown as jest.Mock).mockReturnValue({ + ;mockUseQuery.mockReturnValue({frontend/__tests__/unit/pages/ProjectsHealthDashboardOverview.test.tsx (1)
1-1: Correct: import React hooks from @apollo/client/react in v4This aligns with Apollo Client v4 module structure.
frontend/__tests__/unit/pages/UserDetails.test.tsx (2)
11-12: LGTM: Mocking the React entrypoint.Mock path matches the new hook import path.
1-1: LGTM — Apollo v4 hook imports are correct.Searched repo (excluding tests) for useQuery/useLazyQuery/useMutation/useSubscription imported from '@apollo/client' — no matches found; hooks use '@apollo/client/react'.
frontend/__tests__/unit/pages/CreateModule.test.tsx (2)
1-2: Correct v4 hook import path.Importing React hooks from @apollo/client/react matches Apollo v4 module boundaries.
19-25: Mocks aligned with v4 entrypoint.Stubbing useQuery/useMutation/useApolloClient from @apollo/client/react is the right approach for Apollo v4.
frontend/__tests__/unit/pages/RepositoryDetails.test.tsx (6)
37-41: CastinguseQuerytojest.Mockis fine here.
Keeps TS quiet during migration.
63-67: Data-available mock looks fine.
149-152: No-data case mock is reasonable.
198-211: Zero-stats mock reads well for the “missing stats” scenario.
1-1: Align imports to @apollo/client/react so the test mock appliesfrontend/tests/unit/pages/RepositoryDetails.test.tsx imports useQuery from '@apollo/client/react'; if the component still imports from '@apollo/client' the mock won’t intercept — verify and update all useQuery/useLazyQuery imports to '@apollo/client/react' across the repo.
8-11: Mock target aligns with import path — sandbox couldn't confirm absence of '@apollo/client' hook imports.
Run locally to list any remaining root-package hook imports and paste the output:
rg -n --hidden -S "import\s+{[^}]\buse(?:Query|LazyQuery)\b[^}]}\s+from\s+['"]@apollo/client(?:/react)?['"]" -g '!node_modules' -g '!dist' -C2
If hits are found, standardize imports to '@apollo/client/react' or add a jest.mock for '@apollo/client' as well.frontend/__tests__/unit/pages/EditProgram.test.tsx (3)
14-21: Module mock switched to '@apollo/client/react' — LGTMThe requireActual + override pattern is correct and aligns with the new import surface.
50-57: Mock is correct — component expectsdata.getProgram
The component referencesdata?.getProgramand destructuresconst { getProgram: program } = datain frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx, so the test mock usingdata.getProgramis correct.
71-86: Dates: mocks are correct — no change required.
formatDateForInput(new Date(dateStr) -> toISOString().slice(0,10)) accepts '2025-01-01' and '2025-12-31' and will produce the expected 'YYYY-MM-DD' values for the date inputs.frontend/__tests__/unit/pages/ProgramDetails.test.tsx (4)
1-1: Correct Apollo v4 hook import path.Good switch to @apollo/client/react for hooks.
8-12: Mocking the React entrypoint is correct.Targeting @apollo/client/react keeps mocks aligned with Apollo v4.
35-38: Same aliasing suggestion as above.Use the mockUseQuery alias instead of re‑casting.
48-51: Same aliasing suggestion as above.Use the mockUseQuery alias instead of re‑casting.
frontend/__tests__/unit/pages/Home.test.tsx (6)
11-14: Mocks aligned with Apollo v4 module path.Looks good.
88-92: Same aliasing suggestion as above.Use a mockUseQuery alias instead of re‑casting.
113-116: Same aliasing suggestion as above.Use a mockUseQuery alias instead of re‑casting.
196-201: Same aliasing suggestion as above.Use a mockUseQuery alias instead of re‑casting.
248-255: Same aliasing suggestion as above.Use a mockUseQuery alias instead of re‑casting.
1-1: Approved — hooks correctly imported from '@apollo/client/react'Repo-wide scan found no direct imports of useQuery/useMutation/etc. from '@apollo/client'. Server-side imports of gql/ApolloClient remain from '@apollo/client' (expected).
frontend/__tests__/unit/pages/About.test.tsx (12)
1-1: Correct Apollo v4 hook import path.Good update to @apollo/client/react.
12-15: Mocks target the right module.Mocking @apollo/client/react is correct for Apollo v4.
175-185: Heads-up: ensure this mock matches the component’s document.If the page uses generated documents, prefer operation-name matching or comparing to the generated document as suggested above.
308-320: Same note on document identity.Apply the operation-name or generated-document approach here as well.
339-351: LGTM.This branch keys off variables only and doesn’t rely on document identity.
371-377: LGTM.This mock variation is fine and doesn’t depend on query identity.
393-401: LGTM.No identity checks; safe.
442-451: LGTM.No identity checks; safe.
465-476: LGTM.No identity checks; safe.
488-497: LGTM.No identity checks; safe.
510-519: LGTM.No identity checks; safe.
558-567: Same issue as above for topContributors.Use operation-name or generated-document comparison.
frontend/__tests__/unit/data/mockProgramData.ts (2)
1-1: Switch to generated enum source looks correct.Using types/generated/graphql aligns with codegen outputs.
20-20: Enum member casing updated to match codegen.ProgramStatusEnum.Draft aligns with typical codegen (value 'DRAFT'); UI can still render “Draft”.
Confirm other tests referencing ProgramStatusEnum have updated member casing to avoid type errors.
frontend/__tests__/unit/components/ProgramCard.test.tsx (3)
5-5: Importing ProgramStatusEnum from generated types is consistent.Good alignment with codegen.
Ensure the Program type (types/mentorship) is compatible with the generated enum type to avoid TS widening or mismatch.
36-36: Updated enum member casing is correct.ProgramStatusEnum.Published matches codegen style.
299-299: Updated enum member casing is correct.ProgramStatusEnum.Draft matches codegen style.
frontend/__tests__/unit/pages/ApiKeysPage.test.tsx (2)
2-2: Correct Apollo v4 hook import path.useQuery/useMutation from @apollo/client/react is the right entrypoint.
11-15: Mocks aligned to @apollo/client/react.Looks good.
frontend/__tests__/unit/pages/ProjectHealthDashboardMetricsDetails.test.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (13)
frontend/__tests__/unit/utils/structuredData.test.ts (2)
1-1: Useimport typefor type-only imports.Prevents emitting a runtime import and avoids TS 5
verbatimModuleSyntaxissues.Apply this diff:
-import { User } from 'types/user' +import type { User } from 'types/user'
29-30: Derive expected ISO timestamps instead of hardcoding.000Z.Reduces brittleness if formatting changes upstream.
Apply this diff:
it('should generate valid ProfilePage structured data', () => { const result = generateProfilePageStructuredData(mockUser) + const expectedDateCreated = new Date(mockUser.createdAt!).toISOString() + const expectedDateModified = new Date(mockUser.updatedAt!).toISOString() + expect(result).toEqual({ '@context': 'https://schema.org', '@type': 'ProfilePage', - dateCreated: '2020-01-01T00:00:00.000Z', - dateModified: '2021-02-03T00:00:00.000Z', + dateCreated: expectedDateCreated, + dateModified: expectedDateModified, mainEntity: {frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (4)
31-34: Use generated mutation document; remove the TODO.Replace the legacy mutation constant with the generated document to get end-to-end typing and consistency with the query update.
- const [updateProgram] = useMutation(UPDATE_PROGRAM_STATUS_MUTATION, { - // TODO: update - onError: handleAppError, - }) + const [updateProgram] = useMutation(UpdateProgramStatusDocument, { + onError: handleAppError, + })Add the import (outside this hunk):
import { UpdateProgramStatusDocument } from 'types/__generated__/programsMutations.generated'
83-83: Fix: calling toLowerCase on enum values can fail type-checking.Wrap the enum in String() for safe formatting.
- title: `Program status updated to ${upperFirst(newStatus.toLowerCase())}`, + title: `Program status updated to ${upperFirst(String(newStatus).toLowerCase())}`,
131-131: Same enum-string formatting nit as above.- { label: 'Status', value: upperFirst(program.status.toLowerCase()) }, + { label: 'Status', value: upperFirst(String(program.status).toLowerCase()) },
13-13: Optional: type state from generated query to avoid drift.Derive Program/modules state types from GetProgramAndModulesQuery to keep in lockstep with schema changes.
Example (outside this hunk):
import type { GetProgramAndModulesQuery } from 'types/__generated__/graphql' const [program, setProgram] = useState<GetProgramAndModulesQuery['getProgram'] | null>(null) const [modules, setModules] = useState<NonNullable<GetProgramAndModulesQuery['getProgramModules']>>([])frontend/__tests__/unit/pages/EditModule.test.tsx (1)
25-27: Avoid over‑mocking @apollo/client root to only gql (can break other imports).This stub drops all other exports from @apollo/client at runtime. If the component (or its deps) import anything else (e.g., ApolloError), tests may break. Prefer pass‑through to the actual module.
Apply this diff:
-jest.mock('@apollo/client', () => ({ - gql: jest.requireActual('@apollo/client').gql, -})) +jest.mock('@apollo/client', () => jest.requireActual('@apollo/client'))Please confirm the component under test doesn’t rely on other @apollo/client exports at runtime.
frontend/src/components/SingleModuleCard.tsx (2)
40-40: Avoidwindowat render time; useusePathname()to build URLsReferencing
windowcan break SSR and hydration. PreferusePathname()fromnext/navigation.Apply within the selected lines:
- router.push(`${window.location.pathname}/modules/${module.key}`) + router.push(`${pathname}/modules/${module.key}`)- router.push(`${window.location.pathname}/modules/${module.key}/edit`) + router.push(`${pathname}/modules/${module.key}/edit`)- router.push(`${window.location.pathname}/modules/create`) + router.push(`${pathname}/modules/create`)- href={`${window.location.pathname}/modules/${module.key}`} + href={`${pathname}/modules/${module.key}`}And outside these ranges, also update:
// imports import { useRouter, usePathname } from 'next/navigation' // after const router = useRouter() const pathname = usePathname()Note: Aware of your preference to keep migration PRs focused. Consider as a small follow-up if out of scope.
Also applies to: 45-45, 50-50, 76-76
1-1: Confirm client component usage or add 'use client'This file uses React/Next client hooks. Ensure it's only imported by client components, or add the directive.
'use client'frontend/__tests__/unit/pages/About.test.tsx (4)
15-18: Type the mocked useQuery to avoidunknown as jest.MockUse a typed alias so you don’t repeat casts and you get signatures checked.
Add once near the imports:
// At the top (after imports) const mockedUseQuery = useQuery as unknown as jest.MockedFunction<typeof useQuery>Then replace usages like:
-;(useQuery as unknown as jest.Mock).mockImplementation((query, options) => { +;mockedUseQuery.mockImplementation((query, options) => {(Apply similarly where you reassign the implementation in individual tests.)
119-136: DRY up repeateduseQuerymocksYou’re re-declaring similar mockImplementations many times. Extracting small helpers will reduce duplication and ease future changes.
Example helper (place near the top of the file):
type Key = string | undefined; const byKey = (key: Key, expected: Key) => key === expected; function defaultAboutUseQueryMock(query: any, options?: { variables?: { key?: string } }) { const key = options?.variables?.key; if (query === GetProjectMetadataDocument && byKey(key, 'nest')) return mockProjectData; if (query === GetTopContributorsDocument && byKey(key, 'nest')) return mockTopContributorsData; if (query === GetLeaderDataDocument) return mockUserData(key); return { loading: true }; }Then in beforeEach:
-;(useQuery as unknown as jest.Mock).mockImplementation((query, options) => { ... }) +mockedUseQuery.mockImplementation(defaultAboutUseQueryMock)And in specialized tests, override only the differing branch, delegating the rest to the default to avoid breaking unrelated queries.
Also applies to: 178-187, 311-323, 342-354, 373-381, 395-405, 445-454, 468-480, 491-501, 513-523, 535-545, 561-571
178-187: Fix incorrect error shape in mockYou return
{ error: mockError }, wheremockErroritself is{ error: Error }. Apollo expectserrorto be an Error/ApolloError, not a nested object.Apply:
- } else if (options?.variables?.key === 'arkid15r') { - return { data: null, loading: false, error: mockError } + } else if (options?.variables?.key === 'arkid15r') { + return { data: null, loading: false, error: new Error('GraphQL error') }Alternatively, redefine
mockErrorto be an Error:// near line 112 -const mockError = { - error: new Error('GraphQL error'), -} +const mockError = new Error('GraphQL error')
140-142: Replace jest.clearAllMocks() with jest.resetAllMocks()clearAllMocks() doesn't reset mock implementations and can leak state across tests; resetAllMocks() clears calls and restores default implementations.
- jest.clearAllMocks() + jest.resetAllMocks()rg found 80 occurrences under frontend/tests; update this file and consider a project-wide replace or set "resetMocks" in Jest config — verify any tests that rely on preserved mock implementations before changing.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
frontend/__tests__/unit/data/mockProgramData.ts(2 hunks)frontend/__tests__/unit/pages/About.test.tsx(13 hunks)frontend/__tests__/unit/pages/ApiKeysPage.test.tsx(5 hunks)frontend/__tests__/unit/pages/CreateModule.test.tsx(3 hunks)frontend/__tests__/unit/pages/EditModule.test.tsx(5 hunks)frontend/__tests__/unit/utils/structuredData.test.ts(3 hunks)frontend/src/app/mentorship/programs/[programKey]/page.tsx(3 hunks)frontend/src/app/my/mentorship/programs/[programKey]/page.tsx(4 hunks)frontend/src/components/SingleModuleCard.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/tests/unit/pages/CreateModule.test.tsx
- frontend/tests/unit/pages/ApiKeysPage.test.tsx
- frontend/src/app/mentorship/programs/[programKey]/page.tsx
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2178
File: frontend/src/app/snapshots/[id]/page.tsx:0-0
Timestamp: 2025-09-21T17:04:48.154Z
Learning: User rudransh-shrivastava confirmed that suggested type safety improvements during Apollo Client migration were no longer relevant, reinforcing their preference to keep migration PRs focused on core migration changes rather than additional improvements.
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2178
File: frontend/src/app/about/page.tsx:47-53
Timestamp: 2025-09-21T11:34:33.377Z
Learning: User rudransh-shrivastava prefers to keep Apollo Client migration PRs focused solely on migration-related changes and not include additional bug fixes or improvements that aren't directly related to the migration itself.
📚 Learning: 2025-07-13T07:31:06.511Z
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: frontend/src/components/ModuleCard.tsx:53-55
Timestamp: 2025-07-13T07:31:06.511Z
Learning: In Next.js 13+ app router, useRouter from 'next/navigation' does not provide asPath or query properties. Use useParams to extract route parameters and usePathname to get the current pathname instead.
Applied to files:
frontend/__tests__/unit/pages/EditModule.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/pages/EditModule.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 is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
Applied to files:
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details. These queries cannot be removed or merged as they serve different use cases in the application.
Applied to files:
frontend/src/app/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
📚 Learning: 2025-07-08T17:24:36.501Z
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: backend/apps/mentorship/graphql/mutations/program.py:41-44
Timestamp: 2025-07-08T17:24:36.501Z
Learning: In the mentorship program GraphQL mutations, date validation is handled at the GraphQL schema/node level in the input types (CreateProgramInput, UpdateProgramInput), preventing null values from reaching the mutation logic where date comparisons are performed.
Applied to files:
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
🧬 Code graph analysis (4)
frontend/__tests__/unit/utils/structuredData.test.ts (1)
frontend/src/types/user.ts (1)
User(10-32)
frontend/__tests__/unit/data/mockProgramData.ts (1)
backend/apps/mentorship/api/internal/nodes/enum.py (1)
ProgramStatusEnum(22-27)
frontend/__tests__/unit/pages/About.test.tsx (2)
frontend/src/types/__generated__/projectQueries.generated.ts (2)
GetProjectMetadataDocument(37-37)GetTopContributorsDocument(38-38)frontend/src/types/__generated__/userQueries.generated.ts (1)
GetLeaderDataDocument(26-26)
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (2)
frontend/src/app/global-error.tsx (1)
handleAppError(66-86)frontend/src/types/__generated__/programsQueries.generated.ts (1)
GetProgramAndModulesDocument(37-37)
🔇 Additional comments (17)
frontend/__tests__/unit/utils/structuredData.test.ts (2)
5-5: Switch toUsertype looks correct.Matches the exported type in
types/user.ts.
61-61: LGTM on minimal user typing.Covers required fields; optional fields omitted as intended.
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (5)
2-2: Apollo v4 hooks import looks good.Switching hooks to '@apollo/client/react' aligns with the v4 migration.
10-11: Good move to generated enums/documents.Using ProgramStatusEnum and GetProgramAndModulesDocument is consistent with codegen-driven v4 usage.
40-40: Query migration to generated document is correct.Typed variables + skip usage are appropriate.
79-79: RefetchQuery shape is valid in v4; consider awaiting if UI depends on it.Only add
awaitRefetchQueries: trueif you need the refetch to complete before proceeding (e.g., before reading cache/UI that depends on fresh data).
132-133: Duplicate of earlier feedback: guard/normalize date scalar before formatting.Previously flagged: formatDate may throw if startedAt/endedAt aren’t reliably strings. Consider a presence check or scalar mapping in codegen to ensure these are strings.
frontend/__tests__/unit/pages/EditModule.test.tsx (5)
1-1: Good: Hooks imported from @apollo/client/react align with Apollo v4 and your mocks.
19-23: Good: Mocking @apollo/client/react directly avoids the prior import/mock mismatch.
61-80: Mock shape for useQuery looks correct for this test flow.
81-84: Mutation mock wiring is correct and verifies the submit path.
125-125: Loading state mock aligns with the spinner assertion.frontend/__tests__/unit/data/mockProgramData.ts (1)
1-1: Confirm generated enum export and Jest path alias
- Confirm ProgramStatusEnum is emitted as an exported enum (not generated as a type via enumsAsTypes).
- Confirm the 'types/*' tsconfig path alias resolves under Jest (moduleNameMapper/ts-jest); check frontend/tests/unit/data/mockProgramData.ts — import { ProgramStatusEnum } from 'types/generated/graphql'
frontend/__tests__/unit/pages/About.test.tsx (4)
1-1: Apollo v4 hooks import path is correctSwitching to '@apollo/client/react' aligns with Apollo Client v4. Looks good.
535-545: LGTM: Project error toast path is validated correctlyMatching the query by GetProjectMetadataDocument and asserting addToast payload looks solid.
561-571: LGTM: Top contributors error toast path is validatedSame pattern as the project error case; assertions look correct.
9-14: Good move — About page uses generated DocumentNodesfrontend/src/app/about/page.tsx imports and calls GetProjectMetadataDocument, GetTopContributorsDocument, and GetLeaderDataDocument with useQuery, so tests' identity checks remain stable.
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 (1)
frontend/src/hooks/useLogout.ts (1)
21-23: Type‑safe error handling in catch block.
error.messagecan be unsafe if the catch variable isunknown. Narrow before accessing or coerce to string.- } catch (error) { + } catch (error: unknown) { await signOut({ callbackUrl: '/' }) - throw new Error('Logout failed: ' + error.message) + const message = error instanceof Error ? error.message : String(error) + throw new Error('Logout failed: ' + message) }
🧹 Nitpick comments (6)
frontend/src/app/chapters/[chapterKey]/layout.tsx (1)
7-13: Nit: simplifyparamstyping (non-migration).Next.js passes a plain object for
params; no need to type it asPromiseorawaitit. Safe to defer.Based on learnings
export async function generateMetadata({ params, }: { - params: Promise<{ chapterKey: string }> + params: { chapterKey: string } }): Promise<Metadata> { - const { chapterKey } = await params + const { chapterKey } = paramsfrontend/src/hooks/useLogout.ts (2)
12-12: Tuple destructure is fine; have a fallback ifclienttyping disagrees.
useMutation(LogoutDjangoDocument)infers types correctly in v4, andresult.clientis available. If TS complains, preferuseApolloClientfrom@apollo/client/reactand keeploadingfrom the mutation result.+import { useApolloClient } from '@apollo/client/react' ... -const [logoutUser, { loading, client }] = useMutation(LogoutDjangoDocument) +const [logoutUser, { loading }] = useMutation(LogoutDjangoDocument) +const client = useApolloClient()
19-21: Optional: ensure cache clear runs before redirect.
signOut({ callbackUrl: '/' })may trigger a redirect immediately, preventingclient.clearStore()from running. Consider clearing the cache first, or useredirect: falsethen navigate. Defer if out of scope for this migration.Based on learnings
frontend/src/app/api/auth/[...nextauth]/route.ts (3)
13-13: Drop the unnecessary await on apolloClient.apolloClient resolves to a client instance; awaiting it is redundant and can be removed for clarity.
- const client = await apolloClient + const client = apolloClientAlso applies to: 27-27
20-22: Fix error construction to preserve cause.new Error(message, err) doesn’t set the cause (and may not type-check depending on TS settings). Use the ErrorOptions form.
- } catch (err) { - throw new Error('Failed to fetch project leader status Error', err) - } + } catch (err) { + throw new Error('Failed to fetch project leader status', { cause: err }) + }- } catch (err) { - throw new Error('Failed to fetch mentor status Error', err) - } + } catch (err) { + throw new Error('Failed to fetch mentor status', { cause: err }) + }If your TS lib target is below ES2022, we can instead rethrow a normalized error (err instanceof Error ? err : new Error(String(err))). Based on learnings.
Also applies to: 34-36
78-81: Optional: run the two role checks in parallel to reduce sign‑in latency.This keeps behavior identical but halves the network wait.
- const isLeader = await checkIfProjectLeader(login) - const isMentor = await checkIfMentor(login) + const [isLeader, isMentor] = await Promise.all([ + checkIfProjectLeader(login), + checkIfMentor(login), + ]) token.isLeader = isLeader token.isMentor = isMentorGiven the author’s preference to keep migration PRs focused, consider doing this in a follow‑up. Based on learnings.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
frontend/src/app/api/auth/[...nextauth]/route.ts(2 hunks)frontend/src/app/chapters/[chapterKey]/layout.tsx(2 hunks)frontend/src/app/committees/[committeeKey]/layout.tsx(2 hunks)frontend/src/app/members/[memberKey]/layout.tsx(3 hunks)frontend/src/hooks/useLogout.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2178
File: frontend/src/app/snapshots/[id]/page.tsx:0-0
Timestamp: 2025-09-21T17:04:48.154Z
Learning: User rudransh-shrivastava confirmed that suggested type safety improvements during Apollo Client migration were no longer relevant, reinforcing their preference to keep migration PRs focused on core migration changes rather than additional improvements.
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2178
File: frontend/src/app/about/page.tsx:47-53
Timestamp: 2025-09-21T11:34:33.377Z
Learning: User rudransh-shrivastava prefers to keep Apollo Client migration PRs focused solely on migration-related changes and not include additional bug fixes or improvements that aren't directly related to the migration itself.
📚 Learning: 2025-08-10T11:08:47.258Z
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: frontend/src/app/api/auth/[...nextauth]/route.ts:13-25
Timestamp: 2025-08-10T11:08:47.258Z
Learning: In the OWASP Nest codebase (frontend/src/app/api/auth/[...nextauth]/route.ts), input validation and string trimming for authentication-related queries like `isProjectLeader` and `isMentor` are handled in the backend rather than the frontend. The backend is responsible for sanitizing and validating input parameters.
Applied to files:
frontend/src/app/api/auth/[...nextauth]/route.ts
📚 Learning: 2025-07-13T11:34:31.823Z
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: frontend/src/app/mentorship/programs/page.tsx:14-14
Timestamp: 2025-07-13T11:34:31.823Z
Learning: In the Next.js frontend mentorship application, there are two distinct types for authentication-related data: ExtendedSession for useSession hook (containing accessToken and user.login properties) and UserRolesData for useUserRoles hook (containing currentUserRoles.roles array). The correct access pattern for GitHub username is `(session as ExtendedSession)?.user?.login`.
Applied to files:
frontend/src/app/api/auth/[...nextauth]/route.ts
📚 Learning: 2025-07-11T15:46:58.233Z
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: frontend/src/components/UserMenu.tsx:56-60
Timestamp: 2025-07-11T15:46:58.233Z
Learning: In the UserMenu component's handleLogout function, both logout() and signOut() calls are intentionally kept despite the useLogout hook already calling signOut internally, as this is necessary for the specific implementation requirements.
Applied to files:
frontend/src/hooks/useLogout.ts
🧬 Code graph analysis (5)
frontend/src/app/members/[memberKey]/layout.tsx (1)
frontend/src/types/__generated__/userQueries.generated.ts (2)
GetUserMetadataDocument(28-28)GetUserDataDocument(27-27)
frontend/src/app/chapters/[chapterKey]/layout.tsx (1)
frontend/src/types/__generated__/chapterQueries.generated.ts (1)
GetChapterMetadataDocument(20-20)
frontend/src/app/committees/[committeeKey]/layout.tsx (1)
frontend/src/types/__generated__/committeeQueries.generated.ts (1)
GetCommitteeMetadataDocument(20-20)
frontend/src/app/api/auth/[...nextauth]/route.ts (2)
frontend/src/server/apolloClient.ts (1)
apolloClient(36-37)frontend/src/types/__generated__/mentorshipQueries.generated.ts (2)
IsProjectLeaderDocument(19-19)IsMentorDocument(20-20)
frontend/src/hooks/useLogout.ts (1)
frontend/src/types/__generated__/authQueries.generated.ts (1)
LogoutDjangoDocument(17-17)
⏰ 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 backend tests
- GitHub Check: Run frontend unit tests
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (11)
frontend/src/app/committees/[committeeKey]/layout.tsx (1)
4-15: Nice swap to the generated document.Passing the codegen-provided
GetCommitteeMetadataDocumentkeeps this query strongly typed, and Apollo Client 4 explicitly acceptsTypedDocumentNodeobjects anywhere aDocumentNodeis expected, so this change aligns perfectly with the migration goals. (apollographql.com)frontend/src/app/chapters/[chapterKey]/layout.tsx (2)
14-18: Correct use of the generated document inapolloClient.query.Inference for
data/variableswill flow from the TDN. No issues.
4-4: Approve client migration; no stale imports in this PRThe client side no longer imports or uses
GET_CHAPTER_METADATA. The only remaining definition is infrontend/src/server/queries/chapterQueries.ts, which lies outside this change.frontend/src/hooks/useLogout.ts (2)
4-4: Good switch to generated TypedDocumentNode.Using
LogoutDjangoDocumentenables typed variables/results and better tree‑shaking. No issues here.
1-1: Confirm correct v4 import; update verification script for .ts/.tsx
ImportinguseMutationfrom@apollo/client/reactis correct for Apollo Client v4. To verify no legacy React hooks/imports remain, rerun the check with file-glob patterns:#!/bin/bash # Fail if React hooks/components still come from "@apollo/client" rg -nP -g '*.ts' -g '*.tsx' -C0 \ 'from\s+["'"'@apollo/client"'"']\s*.*\{\s*[^}]*\b(useQuery|useMutation|useLazyQuery|useSubscription|useApolloClient|ApolloProvider|ApolloConsumer)\b' \ || echo "OK: no v3-style React imports"Ensure there are no matches.
frontend/src/app/members/[memberKey]/layout.tsx (2)
4-7: Good switch to generated TypedDocumentNodes.Importing
GetUserDataDocumentandGetUserMetadataDocumentfrom codegen is the right move for Apollo v4 and tight typing. Nothing else needed here.
47-47: App layout query updated correctly.Using
GetUserDataDocumentaligns this layout with the generated operations; downstreamdata.usershould now be fully typed.frontend/src/app/api/auth/[...nextauth]/route.ts (4)
4-7: Good switch to TypedDocumentNode imports.Using generated IsMentorDocument/IsProjectLeaderDocument is correct for Apollo v4 and preserves strong typing.
15-18: Query call correctly uses the generated document.Passing IsProjectLeaderDocument with variables and fetchPolicy: 'no-cache' is appropriate.
29-32: Same here — query usage looks right.IsMentorDocument with no-cache aligns with auth use cases.
4-7: Sanity check passed: no references to IS_PROJECT_LEADER_QUERY or IS_MENTOR_QUERY outside their exports in frontend/src/server/queries/mentorshipQueries.ts.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (1)
130-132: Harden status/date display against enum casing and nullable/number dates.
- Enum label: normalize before capitalizing.
- Dates: guard null/undefined and accept number|string per migration notes.
- { label: 'Status', value: capitalize(program.status) }, - { label: 'Start Date', value: formatDate(program.startedAt) }, - { label: 'End Date', value: formatDate(program.endedAt) }, + { label: 'Status', value: capitalize(program.status.toLowerCase()) }, + { label: 'Start Date', value: program.startedAt ? formatDate(program.startedAt as number | string) : 'N/A' }, + { label: 'End Date', value: program.endedAt ? formatDate(program.endedAt as number | string) : 'N/A' },Also ensure codegen maps your date scalars to
string | numberso these types flow correctly.Based on learnings
🧹 Nitpick comments (3)
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (3)
4-4: Avoid full lodash import for a single util (bundle size).Prefer per-method import to prevent pulling most of lodash in CJS builds.
-import { capitalize } from 'lodash' +import capitalize from 'lodash/capitalize'Based on learnings
31-31: Refetch timing and UX after mutation.Right now the toast shows immediately, but UI may display the old status until refetch finishes. Two low-impact options:
- Await the refetch:
- await updateProgram({ + await updateProgram({ variables: { /* ... */ }, - refetchQueries: [{ query: GetProgramAndModulesDocument, variables: { programKey } }], + refetchQueries: [{ query: GetProgramAndModulesDocument, variables: { programKey } }], + awaitRefetchQueries: true, })
- Or optimistically update local state (snappier UI):
await updateProgram({ /* ... */ }) + setProgram((prev) => (prev ? { ...prev, status: newStatus } : prev))Also applies to: 78-78
82-82: Status label casing consistency.To match other pages and avoid odd casing, normalize first:
- title: `Program status updated to ${capitalize(newStatus)}`, + title: `Program status updated to ${capitalize(newStatus.toLowerCase())}`,
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
frontend/src/app/mentorship/programs/[programKey]/page.tsx(3 hunks)frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx(2 hunks)frontend/src/app/my/mentorship/programs/[programKey]/page.tsx(4 hunks)frontend/src/components/ModuleCard.tsx(2 hunks)frontend/src/components/SingleModuleCard.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/components/SingleModuleCard.tsx
- frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2178
File: frontend/src/app/snapshots/[id]/page.tsx:0-0
Timestamp: 2025-09-21T17:04:48.154Z
Learning: User rudransh-shrivastava confirmed that suggested type safety improvements during Apollo Client migration were no longer relevant, reinforcing their preference to keep migration PRs focused on core migration changes rather than additional improvements.
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2178
File: frontend/src/app/about/page.tsx:47-53
Timestamp: 2025-09-21T11:34:33.377Z
Learning: User rudransh-shrivastava prefers to keep Apollo Client migration PRs focused solely on migration-related changes and not include additional bug fixes or improvements that aren't directly related to the migration itself.
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
Applied to files:
frontend/src/app/mentorship/programs/[programKey]/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. These queries cannot be removed or merged as they serve different use cases in the application.
Applied to files:
frontend/src/app/mentorship/programs/[programKey]/page.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details.
Applied to files:
frontend/src/app/mentorship/programs/[programKey]/page.tsx
🧬 Code graph analysis (3)
frontend/src/components/ModuleCard.tsx (1)
frontend/src/components/InfoItem.tsx (1)
TextInfoItem(37-52)
frontend/src/app/mentorship/programs/[programKey]/page.tsx (1)
frontend/src/types/__generated__/programsQueries.generated.ts (1)
GetProgramAndModulesDocument(37-37)
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (3)
frontend/src/types/__generated__/programsMutations.generated.ts (1)
UpdateProgramStatusDocument(28-28)frontend/src/app/global-error.tsx (1)
handleAppError(66-86)frontend/src/types/__generated__/programsQueries.generated.ts (1)
GetProgramAndModulesDocument(37-37)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Run frontend unit tests
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run backend tests
🔇 Additional comments (4)
frontend/src/components/ModuleCard.tsx (1)
9-9: Verify thecapitalizetransformation is intentional
- Swapping
upperFirstforcapitalizewill lowercase the rest of the enum string (e.g.'ADVANCED'→'Advanced'instead of'ADVANCED'); confirm this display change matches design requirements.- This formatting change lives in both
ModuleCard.tsxandSingleModuleCard.tsxbut isn’t tied to the Apollo v4 migration—if it isn’t needed for generated enum formatting, move it to a standalone PR.frontend/src/app/mentorship/programs/[programKey]/page.tsx (2)
2-2: LGTM! Clean Apollo v4 migration.The migration changes are correct:
- Updated import path to
@apollo/client/react- Switched to generated
GetProgramAndModulesDocument- Proper type-only imports
Also applies to: 7-8, 22-22
3-3: Verify updated status display formatting
Switching fromupperFirsttocapitalizetitle-cases UPPERCASE enum values (e.g.COMPLETED→Completed). Confirm this is the intended UX for allProgramStatusEnumvalues and update any affected tests or snapshots that expect the old all-caps labels.frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (1)
2-2: Apollo v4 + TypedDocumentNode adoption — LGTMImports from generated documents and enums align with the migration goals. No issues spotted.
Also applies to: 9-11
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
frontend/src/utils/structuredData.ts (1)
33-34: Conditionally include date fields to omit them when absent.With the refactored helper returning
undefinedfor missing dates, these fields will be set toundefinedin the JSON-LD object. Schema.org structured data should omit absent fields rather than include them withundefinedvalues.Apply this diff to conditionally include the date fields:
return { '@context': 'https://schema.org', '@type': 'ProfilePage', - dateCreated: formatISODate(user.createdAt), - dateModified: formatISODate(user.updatedAt), + ...(user.createdAt && { + dateCreated: formatISODate(user.createdAt), + }), + ...(user.updatedAt && { + dateModified: formatISODate(user.updatedAt), + }), mainEntity: {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/utils/structuredData.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2178
File: frontend/src/app/snapshots/[id]/page.tsx:0-0
Timestamp: 2025-09-21T17:04:48.154Z
Learning: User rudransh-shrivastava confirmed that suggested type safety improvements during Apollo Client migration were no longer relevant, reinforcing their preference to keep migration PRs focused on core migration changes rather than additional improvements.
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2178
File: frontend/src/app/about/page.tsx:47-53
Timestamp: 2025-09-21T11:34:33.377Z
Learning: User rudransh-shrivastava prefers to keep Apollo Client migration PRs focused solely on migration-related changes and not include additional bug fixes or improvements that aren't directly related to the migration itself.
🧬 Code graph analysis (1)
frontend/src/utils/structuredData.ts (2)
frontend/src/types/user.ts (1)
User(10-32)frontend/src/types/profilePageStructuredData.ts (1)
ProfilePageStructuredData(1-31)
⏰ 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
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
frontend/src/utils/structuredData.ts (1)
4-15: Handle undefined inputs to prevent runtime crashes.The function signature accepts optional input (
input?) but doesn't guard againstundefinedbefore constructing the Date object. Wheninputisundefined,new Date(undefined)creates an Invalid Date, which triggers theTypeErroron line 11. SinceUser.createdAtandUser.updatedAtare optional fields, this will crash at runtime when dates are missing.Apply this diff to safely handle missing dates:
-export const formatISODate = (input?: number | string): string => { +export const formatISODate = (input?: number | string): string | undefined => { + if (input === undefined || input === null) { + return undefined + } + const date = typeof input === 'number' ? new Date(input * 1000) // Unix timestamp in seconds : new Date(input) // ISO date string if (Number.isNaN(date.getTime())) { throw new TypeError('Invalid date') } return date.toISOString() }
🧹 Nitpick comments (1)
frontend/src/utils/structuredData.ts (1)
33-34: Conditionally include date fields to omit absent values.Once
formatISODateis updated to returnstring | undefined, these fields will be set toundefinedwhen dates are missing. To align with structured data best practices and the existing spread operator pattern used elsewhere in this file (lines 37-39, 43-51, 60-65), conditionally include these fields only when defined.Apply this diff:
return { '@context': 'https://schema.org', '@type': 'ProfilePage', - dateCreated: formatISODate(user.createdAt), - dateModified: formatISODate(user.updatedAt), + ...(formatISODate(user.createdAt) && { + dateCreated: formatISODate(user.createdAt), + }), + ...(formatISODate(user.updatedAt) && { + dateModified: formatISODate(user.updatedAt), + }), mainEntity: {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/utils/structuredData.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2178
File: frontend/src/app/snapshots/[id]/page.tsx:0-0
Timestamp: 2025-09-21T17:04:48.154Z
Learning: User rudransh-shrivastava confirmed that suggested type safety improvements during Apollo Client migration were no longer relevant, reinforcing their preference to keep migration PRs focused on core migration changes rather than additional improvements.
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2178
File: frontend/src/app/about/page.tsx:47-53
Timestamp: 2025-09-21T11:34:33.377Z
Learning: User rudransh-shrivastava prefers to keep Apollo Client migration PRs focused solely on migration-related changes and not include additional bug fixes or improvements that aren't directly related to the migration itself.
🧬 Code graph analysis (1)
frontend/src/utils/structuredData.ts (2)
frontend/src/types/user.ts (1)
User(10-32)frontend/src/types/profilePageStructuredData.ts (1)
ProfilePageStructuredData(1-31)
⏰ 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 e2e tests
- GitHub Check: Run frontend unit tests
🔇 Additional comments (1)
frontend/src/utils/structuredData.ts (1)
2-2: LGTM: Type change aligns with Apollo v4 migration.The change from
UserDetailstoUsertype is appropriate for the migration, accommodating the new optional date fields (createdAt?andupdatedAt?) that can be eithernumber | stringas per the migrated GraphQL types.Also applies to: 27-27
| ...(formatISODate(user.updatedAt) && { | ||
| dateModified: formatISODate(user.updatedAt), | ||
| }), | ||
| mainEntity: { |
There was a problem hiding this comment.
maybe the spread is not required, createdAt and updatedAt are optional in type but should always exist.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
frontend/src/utils/structuredData.ts (1)
4-7: Fix return type mismatch.Line 6 returns
undefined, but the function signature declares the return type asstring. This creates a type safety violation.Apply this diff to correct the return type:
-export const formatISODate = (input?: number | string): string => { +export const formatISODate = (input?: number | string): string | undefined => { if (input == null) { return undefined }
🧹 Nitpick comments (1)
frontend/src/utils/structuredData.ts (1)
37-42: Avoid redundant calls to formatISODate.Each field calls
formatISODatetwice—once in the condition and once for the value. Store the result in a variable to eliminate the redundant computation.Apply this diff:
+ const dateCreated = formatISODate(user.createdAt) + const dateModified = formatISODate(user.updatedAt) + return { '@context': 'https://schema.org', '@type': 'ProfilePage', - ...(formatISODate(user.createdAt) && { - dateCreated: formatISODate(user.createdAt), - }), - ...(formatISODate(user.updatedAt) && { - dateModified: formatISODate(user.updatedAt), - }), + ...(dateCreated && { dateCreated }), + ...(dateModified && { dateModified }),
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/utils/structuredData.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2178
File: frontend/src/app/snapshots/[id]/page.tsx:0-0
Timestamp: 2025-09-21T17:04:48.154Z
Learning: User rudransh-shrivastava confirmed that suggested type safety improvements during Apollo Client migration were no longer relevant, reinforcing their preference to keep migration PRs focused on core migration changes rather than additional improvements.
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2178
File: frontend/src/app/about/page.tsx:47-53
Timestamp: 2025-09-21T11:34:33.377Z
Learning: User rudransh-shrivastava prefers to keep Apollo Client migration PRs focused solely on migration-related changes and not include additional bug fixes or improvements that aren't directly related to the migration itself.
📚 Learning: 2025-09-21T17:04:48.154Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2178
File: frontend/src/app/snapshots/[id]/page.tsx:0-0
Timestamp: 2025-09-21T17:04:48.154Z
Learning: User rudransh-shrivastava confirmed that suggested type safety improvements during Apollo Client migration were no longer relevant, reinforcing their preference to keep migration PRs focused on core migration changes rather than additional improvements.
Applied to files:
frontend/src/utils/structuredData.ts
📚 Learning: 2025-07-10T10:04:36.224Z
Learnt from: ahmedxgouda
PR: OWASP/Nest#1718
File: frontend/src/components/MetricsCard.tsx:25-25
Timestamp: 2025-07-10T10:04:36.224Z
Learning: In the OWASP Nest project, createdAt fields are auto-created in the backend, so they will always exist and not be null. This makes null checks unnecessary when displaying these dates in the frontend.
Applied to files:
frontend/src/utils/structuredData.ts
🧬 Code graph analysis (1)
frontend/src/utils/structuredData.ts (2)
frontend/src/types/user.ts (1)
User(10-32)frontend/src/types/profilePageStructuredData.ts (1)
ProfilePageStructuredData(1-31)
|
This PR is ready for review. There are so many changes, I need help testing pages that use Enums and Mutations. Apart from that, I will also need to make followup PRs to do things mentioned in the description. |
|
|
||
| const programDetails = [ | ||
| { label: 'Status', value: upperFirst(program.status) }, | ||
| { label: 'Status', value: capitalize(program.status) }, |
There was a problem hiding this comment.
Not sure if that's better.
There was a problem hiding this comment.
It's not better but I think it's required as
program.status is UPPERCASE
so instead of doing upperFirst(program.status.toLower()) , I used capitalize().
|
2 participants
Resolves #2120
Proposed change
Migrate Apollo Client to v4
apollo/clientto 4.0.0typed-document-nodequeriescreatedAt/updatedAt) fields to benumber | stringenumsto beUPPERCASEas per convention by using generated EnumsFollow up changes [TODO]
Checklist
make check-testlocally; all checks and tests passed.