-
-
Notifications
You must be signed in to change notification settings - Fork 209
Migrate Apollo Client to v4 #2178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Migrate Apollo Client to v4 #2178
Conversation
Summary by CodeRabbit
WalkthroughMigrates frontend to Apollo Client v4: updates dependency, switches many imports to Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
Tests will fail. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 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’sprogram
prop 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
p
to 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
: Useundefined
for 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.ts
norfrontend/src/server/apolloClient.ts
defines a globalnotifyOnNetworkStatusChange
, so addto this query’s options to restore v3-style loading behavior.notifyOnNetworkStatusChange: false
frontend/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 intoApolloProvider
will 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 returningnull
and 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
null
leaks 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 === null
and no GraphQL error,isLoading
never 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-generatedGetProgramAdminsAndModulesDocument
and its correspondingGetProgramAdminsAndModulesQuery
/GetProgramAdminsAndModulesQueryVariables
types fromsrc/types/__generated__/moduleQueries.generated.ts
, then replace the legacyGET_PROGRAM_ADMINS_AND_MODULES
constant in youruseQuery
call 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
TypedDocumentNode
s, 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_DETAILS
with generated...Document
artifacts 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.
repositoryKey
isn’t referenced inside the effect; either remove it or includeorganizationKey
too 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
geolocationData
prop.Would you like me to help resolve the type issue with the
geolocationData
prop in theDetailsCard
component?frontend/src/app/about/page.tsx (2)
68-73
: Reduce state duplication — derive UI straight from query results.
projectMetadata
andtopContributors
mirrordata
and require effects to sync. Prefer readingprojectMetadataResponse?.project
andtopContributorsResponse?.topContributors
directly 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:user
is 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
. UseGetOrganizationDataQuery
to 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
’sloading
instead of a localisLoading
state.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
/renderChapterCard
take 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 string
casts 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/endedAt
areunknown
due to codegen config. Map your GraphQL Date/DateTime scalars tostring
in codegen to restore type-safety and drop the casts here.Example (codegen.yml):
config: scalars: DateTime: string Date: string
Then rerun codegen and remove the
as string
casts at call sites.
19-41
: Optional: derive UI state from Apollo instead of mirroring it.You can drop local
module/admins/isLoading
state and useconst { data, error, loading } = useQuery(...)
, then read fromdata
directly. Less state, fewer effects, fewer edge cases.frontend/src/app/snapshots/page.tsx (1)
16-26
: Optional: use Apollo’sloading
and 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 string
casts by mapping date scalars in codegen.Same rationale as other files; makes
formatDateForInput
calls 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: string
mapping in codegen.)
19-20
: Consistency nit: matchuseParams
typing 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?.project
and drop theuseEffect
+ twouseState
s to reduce re-renders and state sync risks. Happy to draft a refactor if you want.
58-64
: Guard scalar parsing instead ofunknown
→string
cast
createdAt
isunknown
(custom scalar). Prefer safe coercion or map the scalar tostring
in 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
string
in codegen.frontend/src/app/projects/dashboard/metrics/page.tsx (3)
89-93
: Nit: remove redundant||
fallback
currentOrdering
is 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,
notifyOnNetworkStatusChange
defaults 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: updateMetricsCard
prop types, remove TODODefine
metric
as 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.yaml
is excluded by!**/pnpm-lock.yaml
frontend/src/types/__generated__/userQueries.generated.ts
is 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.tsx
frontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx
frontend/src/app/mentorship/programs/[programKey]/page.tsx
frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx
frontend/src/app/chapters/[chapterKey]/page.tsx
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx
frontend/src/app/my/mentorship/page.tsx
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
frontend/src/app/projects/[projectKey]/page.tsx
frontend/src/app/organizations/[organizationKey]/page.tsx
frontend/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.tsx
frontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx
frontend/src/app/mentorship/programs/[programKey]/page.tsx
frontend/src/app/projects/dashboard/metrics/[projectKey]/page.tsx
frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx
frontend/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.tsx
frontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx
frontend/src/app/mentorship/programs/[programKey]/page.tsx
frontend/src/components/ModuleForm.tsx
frontend/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.tsx
frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx
frontend/src/app/my/mentorship/page.tsx
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details.
Applied to files:
frontend/src/app/mentorship/programs/[programKey]/page.tsx
frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx
frontend/src/app/my/mentorship/page.tsx
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx
frontend/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.tsx
frontend/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.tsx
frontend/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.tsx
frontend/src/app/projects/dashboard/metrics/page.tsx
frontend/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.tsx
frontend/src/app/my/mentorship/page.tsx
frontend/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)
const { data: projectMetadataResponse, error: projectMetadataRequestError } = useQuery( | ||
GET_PROJECT_METADATA, | ||
GetProjectMetadataDocument, | ||
{ | ||
variables: { key: projectKey }, | ||
} | ||
) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Fix: spinner can hang forever on errors — gate loading off hook loading
flags, not data presence.
Using !data
to infer loading causes an indefinite spinner when a request errors (no data ever arrives). Read loading
from each useQuery
and derive isLoading
from those flags.
Apply:
- const { data: projectMetadataResponse, error: projectMetadataRequestError } = useQuery(
+ const {
+ data: projectMetadataResponse,
+ error: projectMetadataRequestError,
+ loading: projectMetadataLoading,
+ } = useQuery(
GetProjectMetadataDocument,
{
variables: { key: projectKey },
}
)
...
- const { data: topContributorsResponse, error: topContributorsRequestError } = useQuery(
+ const {
+ data: topContributorsResponse,
+ error: topContributorsRequestError,
+ loading: topContributorsLoading,
+ } = useQuery(
GetTopContributorsDocument,
{
variables: {
excludedUsernames: Object.keys(leaders),
hasFullName: true,
key: projectKey,
limit: 24,
},
}
)
...
- const isLoading =
- !projectMetadataResponse ||
- !topContributorsResponse ||
- (projectMetadataRequestError && !projectMetadata) ||
- (topContributorsRequestError && !topContributors)
+ const isLoading = projectMetadataLoading || topContributorsLoading
Also applies to: 56-66, 95-103
🤖 Prompt for AI Agents
In frontend/src/app/about/page.tsx around lines 49-55 (and similarly for 56-66
and 95-103), the component currently infers loading state by checking for
absence of data, which causes the spinner to hang when a query errors; update
each useQuery call to also destructure its loading flag (e.g., { data, error,
loading }) and compute a combined isLoading by OR-ing those loading flags, then
gate the spinner/rendering off that isLoading value instead of !data so errors
don't leave the spinner stuck; ensure error handling still renders error UI when
an error exists.
const [chapter, setChapter] = useState<GetChapterDataQuery['chapter']>( | ||
{} as GetChapterDataQuery['chapter'] | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Avoid using type assertion for initial state.
Using {} as GetChapterDataQuery['chapter']
for initial state is unsafe as it creates an object that doesn't satisfy the type contract.
- const [chapter, setChapter] = useState<GetChapterDataQuery['chapter']>(
- {} as GetChapterDataQuery['chapter']
- )
+ const [chapter, setChapter] = useState<GetChapterDataQuery['chapter']>(null)
This change requires the state to explicitly handle null
, which is already handled in your conditional rendering (Line 44).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const [chapter, setChapter] = useState<GetChapterDataQuery['chapter']>( | |
{} as GetChapterDataQuery['chapter'] | |
) | |
const [chapter, setChapter] = useState<GetChapterDataQuery['chapter']>(null) |
🤖 Prompt for AI Agents
In frontend/src/app/chapters/[chapterKey]/page.tsx around lines 18 to 20, the
initial state uses an unsafe type assertion ({} as
GetChapterDataQuery['chapter']); change the state to explicitly allow null by
typing it as GetChapterDataQuery['chapter'] | null and initialize it to null,
then keep the existing conditional rendering that handles null (no other runtime
changes required).
const canvasRef = useRef<HTMLCanvasElement | null>(null) | ||
const theme = 'blue' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heatmap never draws when data arrives; dead canvasRef and stale effect.
Top-level canvasRef isn’t attached to any element; the draw effect depends on it and never runs. The inner Heatmap effect doesn’t react to data/username updates.
Apply this diff to remove dead code and fix dependencies:
- const canvasRef = useRef<HTMLCanvasElement | null>(null)
- const theme = 'blue'
@@
- useEffect(() => {
- if (canvasRef.current && data && data.years && data.years.length > 0) {
- drawContributions(canvasRef.current, { data, username, theme })
- const imageURL = canvasRef.current.toDataURL()
- setImageLink(imageURL)
- }
- }, [username, data])
+ // Removed: drawing happens inside Heatmap component tied to its canvas
And inside Heatmap:
- useEffect(() => {
+ useEffect(() => {
if (canvasRef.current && data?.years?.length) {
drawContributions(canvasRef.current, {
data,
username,
themeName: isDarkMode ? 'dark' : 'light',
})
const imageURL = canvasRef.current.toDataURL()
setImageLink(imageURL)
}
- }, [isDarkMode])
+ }, [isDarkMode, data, username])
Also applies to: 74-81, 139-155
{ label: 'Start Date', value: formatDate(module.startedAt as string) }, | ||
{ label: 'End Date', value: formatDate(module.endedAt as string) }, | ||
{ | ||
label: 'Duration', | ||
value: getSimpleDuration(module.startedAt, module.endedAt), | ||
value: getSimpleDuration(module.startedAt as string, module.endedAt as string), | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Normalize date handling; avoid unsafe casts and duration bugs.
startedAt/endedAt are generated as unknown. Casting to string breaks if backend sends epoch seconds. Use a helper or extend APIs to accept number|string.
Apply this minimal change here and harden helpers:
- { label: 'Start Date', value: formatDate(module.startedAt as string) },
- { label: 'End Date', value: formatDate(module.endedAt as string) },
+ { label: 'Start Date', value: formatDate(module.startedAt as unknown as number | string) },
+ { label: 'End Date', value: formatDate(module.endedAt as unknown as number | string) },
@@
- value: getSimpleDuration(module.startedAt as string, module.endedAt as string),
+ value: getSimpleDuration(
+ module.startedAt as unknown as number | string,
+ module.endedAt as unknown as number | string
+ ),
And update helpers outside this file:
--- a/frontend/src/components/ModuleCard.tsx
+++ b/frontend/src/components/ModuleCard.tsx
@@
-export const getSimpleDuration = (start: string, end: string): string => {
- const startDate = new Date(start)
- const endDate = new Date(end)
+export const getSimpleDuration = (start: number | string, end: number | string): string => {
+ const toDate = (v: number | string) =>
+ typeof v === 'number' ? new Date(v * 1000) : new Date(v)
+ const startDate = toDate(start)
+ const endDate = toDate(end)
if (isNaN(startDate.getTime()) || isNaN(endDate.getTime())) {
return 'Invalid duration'
}
const ms = endDate.getTime() - startDate.getTime()
const days = Math.floor(ms / (1000 * 60 * 60 * 24))
const weeks = Math.ceil(days / 7)
return `${weeks} week${weeks !== 1 ? 's' : ''}`
}
Optionally, make formatDate accept unknown:
--- a/frontend/src/utils/dateFormatter.ts
+++ b/frontend/src/utils/dateFormatter.ts
@@
-export const formatDate = (input: number | string) => {
+export const formatDate = (input: unknown) => {
if (!input) {
return ''
}
- const date =
- typeof input === 'number'
- ? new Date(input * 1000)
- : new Date(input)
+ const date =
+ typeof input === 'number' ? new Date(input * 1000) :
+ typeof input === 'string' ? new Date(input) :
+ new Date('invalid')
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
{ label: 'Start Date', value: formatDate(module.startedAt as string) }, | |
{ label: 'End Date', value: formatDate(module.endedAt as string) }, | |
{ | |
label: 'Duration', | |
value: getSimpleDuration(module.startedAt, module.endedAt), | |
value: getSimpleDuration(module.startedAt as string, module.endedAt as string), | |
}, | |
{ label: 'Start Date', value: formatDate(module.startedAt as unknown as number | string) }, | |
{ label: 'End Date', value: formatDate(module.endedAt as unknown as number | string) }, | |
{ | |
label: 'Duration', | |
value: getSimpleDuration( | |
module.startedAt as unknown as number | string, | |
module.endedAt as unknown as number | string | |
), | |
}, |
{ label: 'Start Date', value: formatDate(program.startedAt as string) }, | ||
{ label: 'End Date', value: formatDate(program.endedAt as string) }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify the type casting for date fields.
The as string
type assertions for startedAt
and endedAt
appear forced. According to the generated types, these fields are of type unknown
, which suggests a GraphQL scalar issue.
Based on the generated types showing these fields as unknown
, this appears to be a GraphQL scalar configuration issue. Consider defining a proper scalar type for dates in your GraphQL schema and codegen configuration to avoid unsafe type assertions.
🏁 Script executed:
#!/bin/bash
# Check the GraphQL schema definition for date fields
rg -n --type=ts "startedAt|endedAt" frontend/src/types/__generated__/
Length of output: 26369
Map GraphQL DateTime
scalar to a concrete type
The startedAt
/endedAt
fields are generated as unknown
, forcing as string
casts in lines 75–76. Update your GraphQL codegen scalar mapping (e.g. in codegen.yml
) to map DateTime
to string
(or Date
) so those fields are typed correctly and you can remove the unsafe casts.
🤖 Prompt for AI Agents
In frontend/src/app/mentorship/programs/[programKey]/page.tsx around lines 75–76
the GraphQL DateTime scalar is typed as unknown causing unsafe `as string` casts
on program.startedAt and program.endedAt; update your GraphQL Codegen config
(e.g., codegen.yml) to map the DateTime scalar to a concrete type (string or
Date), run the codegen to regenerate types, and then remove the `as string`
casts in those two lines so startedAt/endedAt are correctly typed and used.
let currentFilters = {} | ||
let currentOrdering = { | ||
score: 'DESC', | ||
score: Ordering.Desc, | ||
} | ||
const healthFilter = searchParams.get('health') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
❓ Verification inconclusive
Normalize and validate ordering to avoid enum/key mismatches
Right now, URL values (order
), dropdown item keys ('desc'|'asc'
), and the Ordering
enum could drift. Normalize to enum values, keep dropdown keys as enum values, and validate the URL param before use.
- let currentOrdering = {
- score: Ordering.Desc,
- }
+ let currentOrdering = { score: Ordering.Desc }
- const orderingParam = searchParams.get('order') as Ordering
+ const rawOrder = searchParams.get('order')
+ const isOrdering = (v: unknown): v is Ordering =>
+ v === Ordering.Asc || v === Ordering.Desc
+ const orderingParam = isOrdering(rawOrder) ? rawOrder : undefined
...
- if (orderingParam) {
- currentOrdering = {
- score: orderingParam,
- }
- }
+ if (orderingParam) currentOrdering = { score: orderingParam }
Use enum values for dropdown item keys and accept a string from the component, then validate/cast:
const orderingSections: DropDownSectionProps[] = [
{
title: '',
items: [
- { label: 'Descending', key: 'desc' },
- { label: 'Ascending', key: 'asc' },
+ { label: 'Descending', key: Ordering.Desc },
+ { label: 'Ascending', key: Ordering.Asc },
],
},
]
...
- <ProjectsDashboardDropDown
+ <ProjectsDashboardDropDown
...
- onAction={(key: Ordering) => {
+ onAction={(key: string) => {
// Reset pagination to the first page when changing ordering
setPagination({ offset: 0, limit: PAGINATION_LIMIT })
const newParams = new URLSearchParams(searchParams.toString())
- newParams.set('order', key)
- setOrdering({
- score: key,
- })
- setActiveOrdering([key])
+ if (isOrdering(key)) {
+ newParams.set('order', key)
+ setOrdering({ score: key })
+ setActiveOrdering([key])
+ } else {
+ newParams.delete('order')
+ setOrdering({ score: Ordering.Desc })
+ setActiveOrdering([Ordering.Desc])
+ }
router.replace(`/projects/dashboard/metrics?${newParams.toString()}`)
}}
/>
Run this to confirm enum values and the ProjectsDashboardDropDown
callback type:
Also applies to: 78-82, 128-135, 210-221
🏁 Script executed:
#!/bin/bash
# 1) Inspect Ordering enum literal values
rg -nC2 -g "types/**" -P "\b(enum|export\s+enum)\s+Ordering\b"
# 2) Check ProjectsDashboardDropDown onAction param type
fd -t f ProjectsDashboardDropDown | xargs -I{} rg -nC3 "onAction\s*:\s*\(" {}
Length of output: 753
🏁 Script executed:
#!/bin/bash
# Locate the Ordering enum definition and its literal values
rg -nP "\bexport\s+enum\s+Ordering\b" -C5
Length of output: 862
🏁 Script executed:
#!/bin/bash
# 3) Inspect DropDownSectionProps to verify item key type
rg -nC5 "export (interface|type) DropDownSectionProps" -g "frontend/src/types/**"
Length of output: 457
Normalize and validate ordering to avoid enum/key mismatches
Right now, URL order
, dropdown keys ('desc'
/'asc'
), and the Ordering
enum (in frontend/src/types/__generated__/graphql.ts
) can drift. Normalize to use enum values ('ASC'
/'DESC'
), validate the URL param before use, and update dropdown keys.
- let currentOrdering = {
- score: Ordering.Desc,
- }
+ let currentOrdering = { score: Ordering.Desc }
- const orderingParam = searchParams.get('order') as Ordering
+ const rawOrder = searchParams.get('order')
+ const isOrdering = (v: unknown): v is Ordering =>
+ v === Ordering.Asc || v === Ordering.Desc
+ const orderingParam = isOrdering(rawOrder) ? rawOrder : undefined
- if (orderingParam) {
- currentOrdering = {
- score: orderingParam,
- }
- }
+ if (orderingParam) currentOrdering = { score: orderingParam }
const orderingSections: DropDownSectionProps[] = [
{
title: '',
items: [
- { label: 'Descending', key: 'desc' },
- { label: 'Ascending', key: 'asc' },
+ { label: 'Descending', key: Ordering.Desc },
+ { label: 'Ascending', key: Ordering.Asc },
],
},
]
...
- <ProjectsDashboardDropDown
+ <ProjectsDashboardDropDown
...
- onAction={(key: Ordering) => {
+ onAction={(key: string) => {
setPagination({ offset: 0, limit: PAGINATION_LIMIT })
const newParams = new URLSearchParams(searchParams.toString())
- newParams.set('order', key)
- setOrdering({
- score: key,
- })
- setActiveOrdering([key])
+ if (isOrdering(key)) {
+ newParams.set('order', key)
+ setOrdering({ score: key })
+ setActiveOrdering([key])
+ } else {
+ newParams.delete('order')
+ setOrdering({ score: Ordering.Desc })
+ setActiveOrdering([Ordering.Desc])
+ }
router.replace(`/projects/dashboard/metrics?${newParams.toString()}`)
}}
/>
Also apply these changes to lines 78–82, 128–135, and 210–221.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
let currentFilters = {} | |
let currentOrdering = { | |
score: 'DESC', | |
score: Ordering.Desc, | |
} | |
const healthFilter = searchParams.get('health') | |
let currentFilters = {} | |
- let currentOrdering = { | |
- score: Ordering.Desc, | |
let currentOrdering = { score: Ordering.Desc } | |
const rawOrder = searchParams.get('order') | |
const isOrdering = (v: unknown): v is Ordering => | |
v === Ordering.Asc || v === Ordering.Desc | |
const orderingParam = isOrdering(rawOrder) ? rawOrder : undefined | |
if (orderingParam) currentOrdering = { score: orderingParam } | |
const healthFilter = searchParams.get('health') |
@@ -1,12 +1,11 @@ | |||
'use client' | |||
|
|||
import { useApolloClient } from '@apollo/client' | |||
import { useApolloClient } from '@apollo/client/react' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Apollo v4 React entrypoint import looks correct.
Good move to @apollo/client/react
for hooks in v4.
Run to find any remaining React hooks still imported from @apollo/client
:
🏁 Script executed:
#!/bin/bash
# List React Apollo hooks still imported from the root entrypoint
rg -nP -C1 --type=ts --type=tsx \
'import\s+\{[^}]*\b(useQuery|useLazyQuery|useMutation|useSubscription|useApolloClient|ApolloProvider)\b[^}]*\}\s+from\s*[\"\']@apollo/client[\"\']'
Length of output: 116
🏁 Script executed:
#!/usr/bin/env bash
# List React Apollo hooks still imported from the root entrypoint with corrected quoting
rg -nP -C1 --type=ts --type=tsx "import\s+{[^}]*\b(useQuery|useLazyQuery|useMutation|useSubscription|useApolloClient|ApolloProvider)\b[^}]*}\s+from\s+['\"]@apollo/client['\"]"
Length of output: 209
🏁 Script executed:
#!/usr/bin/env bash
# Search all .ts and .tsx files for Apollo hooks imported from the root entrypoint
rg -nP -C1 --glob '*.ts' --glob '*.tsx' "import\s+{[^}]*\b(useQuery|useLazyQuery|useMutation|useSubscription|useApolloClient|ApolloProvider)\b[^}]*}\s+from\s+['\"]@apollo/client['\"]"
Length of output: 5272
Migrate remaining Apollo hook imports in tests to @apollo/client/react
Several unit tests under frontend/tests/unit/pages/ still import React hooks (useQuery, useMutation, useApolloClient, etc.) from '@apollo/client'. Update those imports to '@apollo/client/react' to align with Apollo v4’s new entrypoint.
🤖 Prompt for AI Agents
In frontend/src/components/ModuleForm.tsx around line 2, the file currently
imports hooks from '@apollo/client'; update the import for React hooks (e.g.,
useApolloClient, useQuery, useMutation, etc.) to come from
'@apollo/client/react' instead. Replace the existing hook import source with
'@apollo/client/react' and verify any other test files under
frontend/__tests__/unit/pages/ use the same entrypoint so all Apollo v4 hook
imports are consistent.
@@ -1,8 +1,8 @@ | |||
import { useMutation } from '@apollo/client' | |||
import { useMutation } from '@apollo/client/react' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Repo-wide check for import consistency (optional).
Ensure no React hooks are still imported from @apollo/client after the v4 migration.
🏁 Script executed:
#!/bin/bash
# Find React hooks still imported from '@apollo/client' instead of '@apollo/client/react'
rg -nP --type ts --type tsx -C1 \
"import\s*{[^}]*\b(useQuery|useLazyQuery|useMutation|ApolloProvider)\b[^}]*}\s*from\s*['\"]@apollo/client['\"]"
# Find lingering v3 subpath imports
rg -nP --type ts --type tsx -C1 "from\s*['\"]@apollo/client/react/hooks['\"]"
Length of output: 289
🏁 Script executed:
#!/bin/bash
# Re-run search for any React hooks imported from '@apollo/client'
rg -nP -C1 "import\s*{[^}]*\b(useQuery|useLazyQuery|useMutation|ApolloProvider)\b[^}]*}\s*from\s*['\"]@apollo/client['\"]" --glob "*.ts" --glob "*.tsx"
# Re-run search for lingering v3 subpath imports
rg -nP -C1 "from\s*['\"]@apollo/client/react/hooks['\"]" --glob "*.ts" --glob "*.tsx"
Length of output: 5332
Update React hook imports in unit tests
Several unit tests under frontend/tests/unit/pages still import hooks from '@apollo/client'; migrate these to '@apollo/client/react' (e.g. replace import { useQuery, useMutation } from '@apollo/client'
with import { useQuery, useMutation } from '@apollo/client/react'
).
🤖 Prompt for AI Agents
In frontend/src/hooks/useDjangoSession.ts around line 1, unit tests are
importing React hooks from '@apollo/client' which should be updated to
'@apollo/client/react'; update all imports in this file (and other unit test
files under frontend/__tests__/unit/pages) replacing occurrences like "import {
useQuery, useMutation } from '@apollo/client'" with "import { useQuery,
useMutation } from '@apollo/client/react'" so tests import the hook
implementations from the React entrypoint.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
♻️ Duplicate comments (2)
frontend/src/app/snapshots/page.tsx (1)
40-42
: Prefer a readable item alias and letmap
infer the element type.Use a
SnapshotItem
alias ([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
/data
to simplify and avoid effect churn.
isLoading
andsnapshots
duplicateuseQuery
state and add unnecessary effects (can also double-fire toasts under React StrictMode). Derive fromuseQuery
instead.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
useState
from 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
key
on<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
/modules
in state and syncing viauseEffect
risks 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
/setModules
calls 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
DetailsCard
prop 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
DetailsCard
correctly and threadaccessLevel
fromisAdmin
. Want me to draft it?
2-2
: Import Apollo hooks from@apollo/client/react
everywhere. Test files are still importing hooks (e.g.useQuery
,useMutation
,useApolloClient
) from@apollo/client
; update those to the@apollo/client/react
subpath 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: true
alongsiderefetchQueries
—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
useQuery
from@apollo/client/react
is v4-compliant. No action needed.
7-10
: Good move to codegen documents and types.Using
GetCommunitySnapshotsDocument
andGetCommunitySnapshotsQuery
tightens 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: handleAppError
centralizes 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.
{ label: 'Start Date', value: formatDate(program.startedAt as string) }, | ||
{ label: 'End Date', value: formatDate(program.endedAt as string) }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Unsafe cast on date fields; harden against runtime errors and fix scalar typing.
startedAt/endedAt
are unknown
in generated types; casting to string
may break if the server returns a number or null, and formatDate
throws on invalid input. Guard and broaden the type, and map the scalar in codegen to avoid unknown
.
- { 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 number | string) : 'N/A' },
+ { label: 'End Date', value: program.endedAt ? formatDate(program.endedAt as number | string) : 'N/A' },
Additionally (outside this hunk), update codegen to map your Date/DateTime scalar:
# codegen.yml
config:
scalars:
DateTime: string
Date: string
🤖 Prompt for AI Agents
In frontend/src/app/my/mentorship/programs/[programKey]/page.tsx around lines
133-134, the code unsafely casts program.startedAt/program.endedAt to string
before calling formatDate; change to tolerate unknown scalar shapes by checking
their type and presence (e.g. if value is a string use formatDate, if number
convert to ISO/string or pass to formatDate accordingly, otherwise show a safe
fallback like '—' or 'TBD'), and broaden local typing to accept
string|number|null to avoid runtime throws. Also update GraphQL codegen config
to map Date/DateTime scalars to string (e.g. add scalars mapping in codegen.yml)
so generated types are not unknown.
@@ -38,7 +38,7 @@ const SnapshotDetailsPage: React.FC = () => { | |||
} | |||
}, [graphQLData, graphQLRequestError, snapshotKey]) | |||
|
|||
const renderProjectCard = (project: Project) => { | |||
const renderProjectCard = (project: GetSnapshotDetailsQuery['snapshot']['newProjects'][0]) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Fix parameter type: index into a NonNullable snapshot and use [number].
Indexing through GetSnapshotDetailsQuery['snapshot'] includes null and will fail under strictNullChecks.
- const renderProjectCard = (project: GetSnapshotDetailsQuery['snapshot']['newProjects'][0]) => {
+ const renderProjectCard = (
+ project: NonNullable<GetSnapshotDetailsQuery['snapshot']>['newProjects'][number],
+ ) => {
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const renderProjectCard = (project: GetSnapshotDetailsQuery['snapshot']['newProjects'][0]) => { | |
const renderProjectCard = ( | |
project: NonNullable<GetSnapshotDetailsQuery['snapshot']>['newProjects'][number], | |
) => { |
🤖 Prompt for AI Agents
In frontend/src/app/snapshots/[id]/page.tsx around line 41, the parameter type
currently indexes GetSnapshotDetailsQuery['snapshot'] which can be null; change
it to index a NonNullable snapshot and use [number] for the array item type
(e.g. NonNullable<GetSnapshotDetailsQuery['snapshot']>['newProjects'][number])
so the type is the project item (not a possibly-null snapshot) and satisfies
strictNullChecks.
@@ -66,7 +66,7 @@ | |||
) | |||
} | |||
|
|||
const renderChapterCard = (chapter: Chapter) => { | |||
const renderChapterCard = (chapter: GetSnapshotDetailsQuery['snapshot']['newChapters'][0]) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Same NonNullable fix for chapter param.
- const renderChapterCard = (chapter: GetSnapshotDetailsQuery['snapshot']['newChapters'][0]) => {
+ const renderChapterCard = (
+ chapter: NonNullable<GetSnapshotDetailsQuery['snapshot']>['newChapters'][number],
+ ) => {
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const renderChapterCard = (chapter: GetSnapshotDetailsQuery['snapshot']['newChapters'][0]) => { | |
const renderChapterCard = ( | |
chapter: NonNullable<GetSnapshotDetailsQuery['snapshot']>['newChapters'][number], | |
) => { |
🤖 Prompt for AI Agents
In frontend/src/app/snapshots/[id]/page.tsx around line 69, the chapter
parameter is typed as possibly nullable; update the parameter type to use
NonNullable<GetSnapshotDetailsQuery['snapshot']['newChapters'][0]> so the
function signature guarantees chapter is not null/undefined, and adjust any
callers if necessary to only pass defined chapters (or filter nulls beforehand).
@@ -120,7 +120,7 @@ | |||
<div className="flex items-center"> | |||
<FontAwesomeIcon icon={faCalendar} className="mr-1 h-4 w-4" /> | |||
<span> | |||
{formatDate(snapshot.startAt)} - {formatDate(snapshot.endAt)} | |||
{formatDate(snapshot.startAt as string)} - {formatDate(snapshot.endAt as string)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don’t force date scalars to string; allow number | string.
If the backend ever returns a Unix timestamp (seconds), casting to string yields an invalid Date. formatDate already accepts number | string.
- {formatDate(snapshot.startAt as string)} - {formatDate(snapshot.endAt as string)}
+ {formatDate(snapshot.startAt as number | string)} - {formatDate(snapshot.endAt as number | string)}
To eliminate these casts entirely, map GraphQL scalars in codegen (e.g., DateTime: string, Timestamp: number), so generated types aren’t unknown.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
{formatDate(snapshot.startAt as string)} - {formatDate(snapshot.endAt as string)} | |
{formatDate(snapshot.startAt as number | string)} - {formatDate(snapshot.endAt as number | string)} |
🤖 Prompt for AI Agents
In frontend/src/app/snapshots/[id]/page.tsx around line 123, remove the forced
"as string" casts when calling formatDate (i.e., stop casting snapshot.startAt
and snapshot.endAt to string) so you pass the actual values which may be number
| string; ensure formatDate is typed to accept number | string; for a
longer-term fix update GraphQL codegen scalars (e.g., map DateTime to string and
Timestamp to number) so generated types are accurate and you don't get unknown
types requiring casts.
<ChapterMapWrapper | ||
geoLocData={snapshot.newChapters} | ||
geoLocData={snapshot.newChapters} // TODO: update type | ||
showLocal={false} | ||
style={{ height: '400px', width: '100%', zIndex: '0' }} | ||
/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Pass location data, not full chapter nodes, to ChapterMapWrapper; use numeric zIndex.
This removes the TODO and aligns with typical map inputs.
- <ChapterMapWrapper
- geoLocData={snapshot.newChapters} // TODO: update type
- showLocal={false}
- style={{ height: '400px', width: '100%', zIndex: '0' }}
- />
+ <ChapterMapWrapper
+ geoLocData={snapshot.newChapters
+ .flatMap((c) => (c.geoLocation ? [{ key: c.key, name: c.name, ...c.geoLocation }] : []))}
+ showLocal={false}
+ style={{ height: '400px', width: '100%', zIndex: 0 }}
+ />
Run this to confirm the expected prop shape for ChapterMapWrapper and geoLocData:
🏁 Script executed:
#!/bin/bash
# Inspect ChapterMapWrapper prop types and geoLocData usage
fd -t f "ChapterMapWrapper" | tee /tmp/chapter_files.txt
while read -r f; do
echo "=== $f ==="
rg -n -C2 -e 'geoLocData' "$f" || true
rg -n -C2 -e 'interface\s+\w*Props' -e 'type\s+\w*Props' "$f" || true
done </tmp/chapter_files.txt
Length of output: 271
🏁 Script executed:
#!/bin/bash
# List all TSX files referencing ChapterMapWrapper within the frontend code
rg -l --glob "frontend/src/**/*.tsx" "ChapterMapWrapper"
# Show usage context for each occurrence
for file in $(rg -l --glob "frontend/src/**/*.tsx" "ChapterMapWrapper"); do
echo "=== $file ==="
rg -n -C2 "ChapterMapWrapper" "$file"
done
# Locate the definition of ChapterMapWrapper
def=$(rg -l --glob "frontend/src/**/*.tsx" -e "export (const|function|class) ChapterMapWrapper")
echo "=== Definition File: $def ==="
# Inspect its Props type declaration
rg -n -C3 "interface .*Props" "$def" || true
rg -n -C3 "type .*Props" "$def" || true
Length of output: 3961
Update ChapterMapWrapper to accept flattened location data and numeric zIndex
In frontend/src/app/snapshots/[id]/page.tsx, pass only location objects via snapshot.newChapters.flatMap(c ⇒ c.geoLocation ? [{ key: c.key, name: c.name, …c.geoLocation }] : []) and change style={{…, zIndex: 0 }}.
In frontend/src/components/ChapterMapWrapper.tsx, update the geoLocData prop type from Chapter[] to the flattened location shape (e.g. { key: string; name: string; latitude: number; longitude: number }[]), removing the // TODO.
🤖 Prompt for AI Agents
In frontend/src/app/snapshots/[id]/page.tsx around lines 137-141, the
ChapterMapWrapper is being passed snapshot.newChapters (Chapter[]) and a string
zIndex; change the first prop to a flattened array of location objects by using
snapshot.newChapters.flatMap(c => c.geoLocation ? [{ key: c.key, name: c.name,
...c.geoLocation }] : []) so it receives only { key, name, latitude, longitude }
entries, and update the style to use a numeric zIndex: style={{ height: '400px',
width: '100%', zIndex: 0 }}. Also update
frontend/src/components/ChapterMapWrapper.tsx prop typing to accept the
flattened location shape (e.g. { key: string; name: string; latitude: number;
longitude: number }[]) instead of Chapter[] and remove the TODO comment so the
component expects and uses the flattened location objects.
startAt={snapshot.startAt as string} | ||
endAt={snapshot.endAt as string} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Don’t cast unknown
date scalars to string
; fix scalar mapping in codegen.
startAt
/endAt
are unknown
in generated types and are force-cast to string
. Configure GraphQL Code Generator to map your datetime scalars to string
(or a branded type), then regenerate.
Example (codegen.yml):
generates:
frontend/src/types/__generated__/:
plugins:
- typescript
- typescript-operations
config:
scalars:
DateTime: string
Date: string
Time: string
After regenerating, you can remove the as string
assertions here.
🤖 Prompt for AI Agents
In frontend/src/app/snapshots/page.tsx around lines 56-57 the props startAt and
endAt are being force-cast from unknown to string; update your GraphQL Code
Generator config (e.g. codegen.yml) to map your datetime scalars to string (for
example scalars: DateTime: string, Date: string, Time: string), run the codegen
to regenerate types so startAt/endAt become string (or a branded string type),
then remove the `as string` assertions from these props in page.tsx.
type ProjectType = | ||
| Project | ||
| Issue | ||
| Committee | ||
| Chapter | ||
| GetSnapshotDetailsQuery['snapshot']['newChapters'][0] | ||
| GetSnapshotDetailsQuery['snapshot']['newProjects'][0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix nullable index access on GetSnapshotDetailsQuery['snapshot'].
snapshot
is ... | null
; indexing into ['newChapters']
/ ['newProjects']
can fail at the type level. Wrap with NonNullable
and prefer [number]
over [0]
for clarity.
-type ProjectType =
- | Project
- | Issue
- | Committee
- | Chapter
- | GetSnapshotDetailsQuery['snapshot']['newChapters'][0]
- | GetSnapshotDetailsQuery['snapshot']['newProjects'][0]
+type SnapshotNewChapter = NonNullable<GetSnapshotDetailsQuery['snapshot']>['newChapters'][number]
+type SnapshotNewProject = NonNullable<GetSnapshotDetailsQuery['snapshot']>['newProjects'][number]
+
+type ProjectType =
+ | Project
+ | Issue
+ | Committee
+ | Chapter
+ | SnapshotNewChapter
+ | SnapshotNewProject
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
type ProjectType = | |
| Project | |
| Issue | |
| Committee | |
| Chapter | |
| GetSnapshotDetailsQuery['snapshot']['newChapters'][0] | |
| GetSnapshotDetailsQuery['snapshot']['newProjects'][0] | |
type SnapshotNewChapter = NonNullable<GetSnapshotDetailsQuery['snapshot']>['newChapters'][number] | |
type SnapshotNewProject = NonNullable<GetSnapshotDetailsQuery['snapshot']>['newProjects'][number] | |
type ProjectType = | |
| Project | |
| Issue | |
| Committee | |
| Chapter | |
| SnapshotNewChapter | |
| SnapshotNewProject |
🤖 Prompt for AI Agents
In frontend/src/utils/utility.ts around lines 21 to 27, the ProjectType union
indexes into GetSnapshotDetailsQuery['snapshot'] which can be null; change those
members to use NonNullable<GetSnapshotDetailsQuery['snapshot']> and use [number]
instead of [0] (e.g.
NonNullable<GetSnapshotDetailsQuery['snapshot']>['newChapters'][number] and
NonNullable<GetSnapshotDetailsQuery['snapshot']>['newProjects'][number]) so the
type system no longer allows nullable index access and the intent is clearer.
Resolves #2120
Proposed change
TODOs
Checklist
make check-test
locally; all checks and tests passed.