Integrate graphql-codegen to generate GraphQL types #2155
Conversation
Summary by CodeRabbit
WalkthroughAdds GraphQL Codegen tooling and CI hooks, ignores generated types from linters/formatters, renames many GraphQL operation names, updates several queries’ selections (including removal of one query and small field edits), and integrates codegen into frontend Makefile and package scripts. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
Had to violate the DRY principle in This made it tricky to use that function and would make the code more complicated as node does not load environment variables from a |
|
|
||
| export const GET_PROGRAM_DATA = gql` | ||
| query GetPrograms($page: Int!, $search: String, $mentorUsername: String) { | ||
| allPrograms(page: $page, search: $search, mentorUsername: $mentorUsername) { |
There was a problem hiding this comment.
This was a query was not being used anywhere. It had syntax errors that were never caught due to it not being used.
The error:
Error 1: Cannot query field "allPrograms" on type "Query". Did you mean "myPrograms" or "getProgram"?
at /home/psyduck/repos/Nest/frontend/src/server/queries/programsQueries.ts:3:5
| } | ||
| login | ||
| avatarUrl | ||
| } |
There was a problem hiding this comment.
This query was not being used anywhere. Fixed syntax errors to run codegen. I can remove it entirely if required.
Error:
Error 0: Cannot query field "githubUser" on type "MentorNode".
at /home/psyduck/repos/Nest/frontend/src/server/queries/moduleQueries.ts:15:9
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
frontend/eslint.config.mjs (1)
30-39: Use a recursive ignore to be unambiguous.Minor: make the ignore explicitly recursive to avoid edge cases with some glob engines.
- 'src/types/__generated__', + 'src/types/__generated__/**',frontend/src/server/queries/moduleQueries.ts (1)
15-19: Align mentor field selections or extract a shared fragment.Inconsistency: GET_MODULES_BY_PROGRAM selects { id, login, avatarUrl } while GET_MODULE_BY_ID selects { login, name, avatarUrl }. Prefer one canonical shape or a fragment to keep types consistent.
Option A — add name to list view for parity:
mentors { - id - login - avatarUrl + id + login + name + avatarUrl }Option B — define a fragment for reuse (example):
fragment MentorSummary on Mentor { login name avatarUrl } # then use: mentors { ...MentorSummary }I can open a follow-up PR to introduce shared fragments if helpful.
Also applies to: 36-40
frontend/src/server/queries/repositoryQueries.ts (1)
3-85: Reduce duplication with fragments (optional).Author fields repeat across issues, releases, milestones, and PRs. Consider a shared fragment for consistency and smaller diffs when fields evolve.
Example (apply in this file and reuse elsewhere):
import { gql } from '@apollo/client' const USER_BASIC_FIELDS = gql` fragment UserBasicFields on User { avatarUrl login name } `Then replace repeated author selections with
...UserBasicFields.frontend/graphql-codegen.ts (3)
3-5: Type the CSRF cache.Avoid implicit any.
-let csrfToken = null +let csrfToken: string | null = null
6-22: Don’t duplicate CSRF fetch logic (optional).There’s an existing server util (src/server/fetchCsrfTokenServer.ts). Prefer importing/reusing to avoid drift, or centralize both behind a shared helper.
25-25: Exclude generated types from document scanning.Prevents unnecessary parsing and speeds up codegen.
- documents: ['src/**/*.{ts,tsx}'], + documents: ['src/**/*.{ts,tsx}', '!src/types/__generated__/**'],Also applies to: 27-27
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlfrontend/src/types/__generated__/graphql.tsis excluded by!**/__generated__/**
📒 Files selected for processing (13)
frontend/.prettierignore(1 hunks)frontend/eslint.config.mjs(1 hunks)frontend/graphql-codegen.ts(1 hunks)frontend/package.json(2 hunks)frontend/src/server/queries/chapterQueries.ts(2 hunks)frontend/src/server/queries/committeeQueries.ts(2 hunks)frontend/src/server/queries/moduleQueries.ts(3 hunks)frontend/src/server/queries/organizationQueries.ts(2 hunks)frontend/src/server/queries/programsQueries.ts(1 hunks)frontend/src/server/queries/projectsHealthDashboardQueries.ts(1 hunks)frontend/src/server/queries/repositoryQueries.ts(2 hunks)frontend/src/server/queries/snapshotQueries.ts(1 hunks)frontend/src/server/queries/userQueries.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/graphql-codegen.ts (1)
frontend/src/server/fetchCsrfTokenServer.ts (1)
fetchCsrfTokenServer(1-13)
🔇 Additional comments (15)
frontend/src/server/queries/committeeQueries.ts (1)
4-4: Consistent operation names — good; check for duplicates.These renames align with codegen. Verify uniqueness across the codebase to prevent duplicate interface/type names.
Use the duplicate-name scan provided in my earlier comment; it applies here too.
Also applies to: 28-28
frontend/src/server/queries/snapshotQueries.ts (1)
58-58: Rename to Metadata-suffixed op — good; confirm uniqueness.Same duplicate-name caution applies; ensure no other
GetSnapshotDetailsMetadataexists.Use the duplicate-name scan script from the first GraphQL comment.
frontend/src/server/queries/userQueries.ts (1)
4-4: The script ran without errors; please provide its output so I can inspect the queries.frontend/src/server/queries/chapterQueries.ts (2)
4-4: PascalCase operation names are consistent and clear.Renaming to GetChapterData/GetChapterMetadata improves consistency for codegen and tooling.
Also applies to: 29-29
4-4: No downstream dependencies on old operation names detected.frontend/eslint.config.mjs (1)
30-39: Ignoring generated types directory is correct.Excluding src/types/generated from lint keeps CI noise down and speeds linting.
frontend/src/server/queries/organizationQueries.ts (2)
4-4: Operation name standardization looks good.GetOrganizationData/GetOrganizationMetadata align with codegen conventions.
Also applies to: 98-98
4-4: Validate external monitoring and persisted query implications
Scan found no persisted‐query hints or operationName references in the codebase. Ensure any external alerting/analytics dashboards or persisted-query configurations referencingGetOrganizationDataorGetOrganizationMetadataare updated to match these changes.frontend/package.json (1)
72-74: Compatibility confirmed: @graphql-codegen/cli@^5, typescript@^4, and typescript-operations@^4 support graphql@^16. No changes required.frontend/src/server/queries/moduleQueries.ts (2)
4-4: Operation name updates LGTM.Consistent GetX naming aids codegen type exports and DX.
Also applies to: 25-25, 46-46
15-19: Drop githubUser wrapper warning
Search across.ts/.tsxfiles found zero references to.githubUser, and all mentors projections use flat fields (login,name, etc.), so no consumers expect the old nested shape.Likely an incorrect or invalid review comment.
frontend/src/server/queries/repositoryQueries.ts (2)
88-88: Operation rename verified: no occurrences of “GetRepository” remain in the frontend code.
4-4: No remaining references toGetRepositorydetected. Rename appears safe.frontend/graphql-codegen.ts (2)
35-42: skipTypeNameForRoot is a valid option
skipTypeNameForRoot: trueis supported by the @graphql-codegen/typescript-operations plugin to omit__typenameon root operation types; no changes needed.Likely an incorrect or invalid review comment.
49-56: Run GraphQL Codegen in the correct workspace
The pnpm error indicates no package.json at the repo root—please rerun the codegen command from the frontend package (e.g.cd frontend && pnpm graphql-codegen --config graphql-codegen.ts --verbose) or usepnpm --prefix frontend graphql-codegen …to verify the updated async export and added Origin header work as intended.
| "dev": "next dev --port 3000", | ||
| "format": "prettier --log-level warn --write .", | ||
| "format:check": "prettier --check .", | ||
| "graphql-codegen": "graphql-codegen --config graphql-codegen.ts", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Generate types automatically during builds.
Prevent stale/missing types by invoking codegen before build.
"scripts": {
- "build": "next build",
+ "build": "pnpm run graphql-codegen && next build",
"build:turbo": "next build --turbo",
@@
- "graphql-codegen": "graphql-codegen --config graphql-codegen.ts",
+ "graphql-codegen": "graphql-codegen --config graphql-codegen.ts",
+ "predev": "pnpm run graphql-codegen",
+ "pretest:unit": "pnpm run graphql-codegen",📝 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.
| "graphql-codegen": "graphql-codegen --config graphql-codegen.ts", | |
| "scripts": { | |
| "build": "pnpm run graphql-codegen && next build", | |
| "build:turbo": "next build --turbo", | |
| "graphql-codegen": "graphql-codegen --config graphql-codegen.ts", | |
| "predev": "pnpm run graphql-codegen", | |
| "pretest:unit": "pnpm run graphql-codegen", | |
| } |
🤖 Prompt for AI Agents
In frontend/package.json around line 11, the project invokes graphql-codegen via
a standalone script but does not ensure types are generated before building;
update package.json scripts to run codegen automatically before build—either add
a "prebuild" script that runs "graphql-codegen --config graphql-codegen.ts" or
prepend the build script with the codegen command (e.g., "graphql-codegen
--config graphql-codegen.ts && <existing build command>") so types are always
generated during CI/local builds.
There was a problem hiding this comment.
how is this possible if we run backend separately? if we invoke codegen before build it may fail as backend may or may not be available. Codegen requires a server to query the GraphQL endpoint
There was a problem hiding this comment.
@rudransh-shrivastava You're absolutely right! That's a critical oversight on my part. Running codegen during build would indeed fail when the backend isn't available, which is common in CI/CD pipelines or local development scenarios.
Here are better approaches to handle this:
Option 1: Use a static schema file
// In graphql-codegen.ts, replace the schema URL with a local file
- schema: `${PUBLIC_API_URL}/graphql/`,
+ schema: './schema.graphql',Option 2: Make codegen conditional
{
"scripts": {
"graphql-codegen": "graphql-codegen --config graphql-codegen.ts",
"graphql-codegen:safe": "graphql-codegen --config graphql-codegen.ts || echo 'Codegen skipped - server unavailable'",
"build": "pnpm run graphql-codegen:safe && next build"
}
}Option 3: Separate CI step
Run codegen only when backend is available (e.g., in a specific CI job) and commit the generated types.
Given that you've already committed the generated types, Option 3 seems to align with your current approach. The types would be generated when the backend is available and committed to the repo.
What's your preference for handling this in your deployment pipeline?
There was a problem hiding this comment.
- to get a
schema.graphqlfile, I run into the same problem. I need a backend server to generate that file.
I would like to know if there's a way to run graphql-codegen after backend is available? This is for local development purposes. I am assuming that for CI jobs -- all generated files are already present.
There was a problem hiding this comment.
- No changes needed to your current graphql-codegen.ts configuration
- Generated files remain committed for CI/CD as you intended
The first solution with wait-on + concurrently is probably your best bet since it handles the server dependency gracefully and provides a smooth development experience.
|
|
||
| export const GET_PROGRAM_ADMIN_DETAILS = gql` | ||
| query GetProgramDetails($programKey: String!) { | ||
| query GetProgramAdminDetails($programKey: String!) { |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Operation rename: verify no dependencies on old operation name (GetProgramDetails).
Check persisted queries, allowlists, dashboards, and tests that might assert on operationName.
🏁 Script executed:
#!/bin/bash
rg -nP -C2 --glob 'frontend/**' $'\\bquery\\s+GetProgramDetails\\b|operationName\\s*[:=]\\s*[\'"]GetProgramDetails[\'"]'Length of output: 512
Update GraphQL operation rename and dependencies
Old operation GetProgramDetails and its constant GET_PROGRAM_DETAILS remain in frontend/src/server/queries/programsQueries.ts (lines 21–24); rename or remove them to match the new GetProgramAdminDetails operation. Then search persisted queries, allowlists, dashboards, and tests for any GetProgramDetails occurrences to ensure no breaking references.
🤖 Prompt for AI Agents
In frontend/src/server/queries/programsQueries.ts around lines 21–24 and 81, the
code still defines the old GraphQL operation/constant
GetProgramDetails/GET_PROGRAM_DETAILS while the operation has been renamed to
GetProgramAdminDetails; update or remove the old constant so the exported query
name matches the new operation (rename GET_PROGRAM_DETAILS to
GET_PROGRAM_ADMIN_DETAILS or remove the stale constant), update any local
imports/usages in this file to reference the new name, then run a repo-wide
search for "GetProgramDetails" and "GET_PROGRAM_DETAILS" and update persisted
queries, allowlists, dashboards, and tests to reference
"GetProgramAdminDetails"/new constant to prevent broken references.
There was a problem hiding this comment.
This query is being used in places. we cannot remove it.
There was a problem hiding this comment.
🧩 Analysis chain
@rudransh-shrivastava You're absolutely right, and I apologize for the confusion. I can see now that GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two distinct queries serving different purposes:
GET_PROGRAM_DETAILS- fetches comprehensive program information (status, menteesLimit, experienceLevels, domains, tags, etc.)GET_PROGRAM_ADMIN_DETAILS- fetches only admin-related program details (id, key, name, and admins)
The rename in the diff only affected the operation name within GET_PROGRAM_ADMIN_DETAILS from "GetProgramDetails" to "GetProgramAdminDetails", which makes sense to avoid naming conflicts. The existing GET_PROGRAM_DETAILS query should remain as it serves a different use case.
Let me verify where GET_PROGRAM_DETAILS is being used to better understand its importance in the codebase.
🏁 Script executed:
#!/bin/bash
# Search for usage of GET_PROGRAM_DETAILS in the codebase
rg -nP --type=ts --type=tsx -C2 'GET_PROGRAM_DETAILS' frontend/
Length of output: 91
@rudransh-shrivastava Thank you for the clarification. You're correct that GET_PROGRAM_DETAILS is being used elsewhere and cannot be removed. The rename operation only affected the GET_PROGRAM_ADMIN_DETAILS query's operation name, which is the appropriate approach to avoid naming conflicts between the two distinct queries.
The GraphQL codegen integration correctly handles having multiple queries with different purposes, and maintaining both queries ensures backward compatibility with existing components that depend on GET_PROGRAM_DETAILS.
(\ /)
( ._.)
o_(")(") *thoughtful rabbit after learning something new*
🏁 Script executed:
#!/bin/bash
# Search for usage of GET_PROGRAM_DETAILS in the codebase (corrected file types)
rg -nP --type=ts -g '*.tsx' -C2 'GET_PROGRAM_DETAILS' frontend/
Length of output: 1202
🏁 Script executed:
#!/bin/bash
# Alternative search without file type restrictions
rg -nP -C2 'GET_PROGRAM_DETAILS' frontend/
Length of output: 1569
Perfect! The search results confirm exactly what you said. GET_PROGRAM_DETAILS is actively being used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for the program edit functionality (imported on line 10 and used in a useQuery hook on line 26).
This makes complete sense - the program edit page needs the comprehensive program information that GET_PROGRAM_DETAILS provides (status, menteesLimit, experienceLevels, domains, tags, etc.), while GET_PROGRAM_ADMIN_DETAILS serves a different, more limited purpose.
Thank you for the correction! The GraphQL codegen integration and the operation name rename for GET_PROGRAM_ADMIN_DETAILS (from "GetProgramDetails" to "GetProgramAdminDetails") was the right approach to avoid naming conflicts while preserving the existing functionality.
✏️ Learnings added
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.832Z
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.
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.832Z
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.
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.833Z
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.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
|
I'm yet to integrate this in the CI/CD pipeline |
|
I will also add generated types to |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
frontend/graphql-codegen.ts (1)
3-3: Load envs via dotenv or fail fast if PUBLIC_API_URL isn’t set (DRY + CI).This removes the need for duplicated CSRF logic elsewhere and avoids surprising localhost defaults in CI.
Option A (simple): add at top of file:
+import 'dotenv/config'Option B (strict): replace the fallback with a hard error:
-const PUBLIC_API_URL = process.env.PUBLIC_API_URL || 'http://localhost:8000' +const PUBLIC_API_URL = process.env.PUBLIC_API_URL +if (!PUBLIC_API_URL) throw new Error('PUBLIC_API_URL is required for graphql-codegen')
🧹 Nitpick comments (3)
frontend/graphql-codegen.ts (3)
18-18: Exclude generated dir from documents to avoid accidental reprocessing.- documents: ['src/**/*.{ts,tsx}'], + documents: ['src/**/*.{ts,tsx}', '!src/types/__generated__/**'],
36-37: Consider generating TypedDocumentNode for safer usage with Apollo.Adds strong typing for the actual gql documents; improves DX and narrows runtime mistakes.
- plugins: ['typescript', 'typescript-operations'], + plugins: ['typescript', 'typescript-operations', 'typed-document-node'],Follow-up: add devDependency @graphql-codegen/typed-document-node.
17-38: Pipeline strategy to decouple codegen from a live backend.Prefer using a schema artifact to make CI deterministic and faster:
- Backend job exports schema (SDL or introspection JSON) as an artifact.
- Frontend job consumes it: schema: './schema.graphql' (or schema.json) without any CSRF/network.
If desired, I can draft the CI steps and modify this config to accept a local schema path with a simple env flag.
📜 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 (1)
frontend/graphql-codegen.ts(1 hunks)
🔇 Additional comments (2)
frontend/graphql-codegen.ts (2)
21-35: Config defaults look solid.avoidOptionals, defaultScalarType, and typename settings align with Apollo usage.
39-42: LGTM on these flags.ignoreNoDocuments and overwrite are appropriate here.
frontend/graphql-codegen.ts
Outdated
| const createCodegenConfig = async (): Promise<CodegenConfig> => { | ||
| const response = await fetch(`${PUBLIC_API_URL}/csrf/`, { | ||
| credentials: 'include', | ||
| method: 'GET', | ||
| }) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add timeout; drop ineffective credentials: 'include'.
Node’s fetch has no cookie jar; credentials is no-op here. Add an AbortController to avoid hanging if /csrf/ is slow/down.
- const response = await fetch(`${PUBLIC_API_URL}/csrf/`, {
- credentials: 'include',
- method: 'GET',
- })
+ const controller = new AbortController()
+ const timeout = setTimeout(() => controller.abort(), 5000)
+ const response = await fetch(`${PUBLIC_API_URL}/csrf/`, {
+ method: 'GET',
+ signal: controller.signal,
+ })📝 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 createCodegenConfig = async (): Promise<CodegenConfig> => { | |
| const response = await fetch(`${PUBLIC_API_URL}/csrf/`, { | |
| credentials: 'include', | |
| method: 'GET', | |
| }) | |
| const createCodegenConfig = async (): Promise<CodegenConfig> => { | |
| const controller = new AbortController() | |
| const timeout = setTimeout(() => controller.abort(), 5000) | |
| const response = await fetch(`${PUBLIC_API_URL}/csrf/`, { | |
| method: 'GET', | |
| signal: controller.signal, | |
| }) |
🤖 Prompt for AI Agents
In frontend/graphql-codegen.ts around lines 5 to 10, the fetch call to
`${PUBLIC_API_URL}/csrf/` incorrectly includes credentials: 'include' (no-op in
Node) and lacks a timeout, risking hanging; remove the credentials option,
create an AbortController, pass its signal to fetch, set a short timeout (e.g.,
5s) that calls controller.abort(), and ensure you clear the timeout after fetch
completes and properly catch/handle the abort error so the function fails fast
when /csrf/ is slow or down.
| if (!response.ok) { | ||
| throw new Error(`Failed to fetch CSRF token: ${response.status} ${response.statusText}`) | ||
| } | ||
| const data = await response.json() | ||
| const csrfToken = data.csrftoken | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden CSRF extraction and clear timeout.
Be resilient to different payload shapes and fall back to Set-Cookie. Fail fast if still missing.
- if (!response.ok) {
+ clearTimeout(timeout)
+ if (!response.ok) {
throw new Error(`Failed to fetch CSRF token: ${response.status} ${response.statusText}`)
}
- const data = await response.json()
- const csrfToken = data.csrftoken
+ type CsrfResponse = { csrftoken?: string; csrfToken?: string; token?: string }
+ const data = (await response.json().catch(() => ({}))) as CsrfResponse
+ let csrfToken = data.csrftoken ?? data.csrfToken ?? data.token
+ if (!csrfToken) {
+ const setCookie = response.headers.get('set-cookie') ?? ''
+ const m = setCookie.match(/(?:^|;\s*)csrftoken=([^;]+)/i)
+ if (m) csrfToken = m[1]
+ }
+ if (!csrfToken) {
+ throw new Error('Failed to extract CSRF token from CSRF endpoint response')
+ }📝 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.
| if (!response.ok) { | |
| throw new Error(`Failed to fetch CSRF token: ${response.status} ${response.statusText}`) | |
| } | |
| const data = await response.json() | |
| const csrfToken = data.csrftoken | |
| clearTimeout(timeout) | |
| if (!response.ok) { | |
| throw new Error(`Failed to fetch CSRF token: ${response.status} ${response.statusText}`) | |
| } | |
| type CsrfResponse = { csrftoken?: string; csrfToken?: string; token?: string } | |
| const data = (await response.json().catch(() => ({}))) as CsrfResponse | |
| let csrfToken = data.csrftoken ?? data.csrfToken ?? data.token | |
| if (!csrfToken) { | |
| const setCookie = response.headers.get('set-cookie') ?? '' | |
| const m = setCookie.match(/(?:^|;\s*)csrftoken=([^;]+)/i) | |
| if (m) csrfToken = m[1] | |
| } | |
| if (!csrfToken) { | |
| throw new Error('Failed to extract CSRF token from CSRF endpoint response') | |
| } |
🤖 Prompt for AI Agents
In frontend/graphql-codegen.ts around lines 11 to 16, the code assumes the CSRF
token is at data.csrftoken and doesn't clear the fetch timeout or handle
alternate payloads; update the logic to: parse JSON safely (guarding for
data.csrftoken, data.csrfToken, data.token, or nested properties), if JSON
parsing fails or none of those fields exist, check the
response.headers.get('set-cookie') (or other cookie headers) for a csrf token
fallback, and ensure any timeout/abort controller is cleared or aborted cleanup
is performed; if after these attempts the token is still missing, throw an
explicit error immediately to fail fast.
| [`${PUBLIC_API_URL}/graphql/`]: { | ||
| headers: { | ||
| Cookie: `csrftoken=${csrfToken}`, | ||
| 'x-csrftoken': `${csrfToken}`, | ||
| }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Add Referer and normalize CSRF header casing (Django requires it over HTTPS).
Without a Referer that matches the target origin, Django will reject POST introspection requests even with a valid CSRF token. Also prefer the conventional header casing.
Apply:
headers: {
- Cookie: `csrftoken=${csrfToken}`,
- 'x-csrftoken': `${csrfToken}`,
+ Cookie: `csrftoken=${csrfToken}`,
+ 'X-CSRFToken': csrfToken,
+ Referer: PUBLIC_API_URL,
},📝 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.
| [`${PUBLIC_API_URL}/graphql/`]: { | |
| headers: { | |
| Cookie: `csrftoken=${csrfToken}`, | |
| 'x-csrftoken': `${csrfToken}`, | |
| }, | |
| }, | |
| }, | |
| [`${PUBLIC_API_URL}/graphql/`]: { | |
| headers: { | |
| Cookie: `csrftoken=${csrfToken}`, | |
| 'X-CSRFToken': csrfToken, | |
| Referer: PUBLIC_API_URL, | |
| }, | |
| }, |
🤖 Prompt for AI Agents
In frontend/graphql-codegen.ts around lines 43 to 49, the request headers are
missing a Referer and use a nonstandard CSRF header casing; add a Referer header
that matches the target origin (e.g. PUBLIC_API_URL with trailing slash) and
change the header name from 'x-csrftoken' to the conventional 'X-CSRFToken' so
Django accepts POST introspection over HTTPS; keep the Cookie csrftoken as-is
and ensure the Referer value is exactly the API origin expected by Django.
There was a problem hiding this comment.
django does not reject them, it works fine.
There was a problem hiding this comment.
@rudransh-shrivastava Thank you for clarifying! I appreciate you testing this and confirming it works with your current Django configuration.
My suggestion was based on Django's default CSRF protection behavior, particularly for HTTPS environments where Django typically requires a Referer header that matches the target origin. However, since you've verified it works fine in your setup, it's likely that:
- Your Django configuration has appropriate CSRF settings for your use case
- The GraphQL introspection might be using GET requests rather than POST
- Your development/deployment environment doesn't trigger Django's stricter CSRF validation
Since you've confirmed the current implementation works, there's no need to make these changes. Thanks for the feedback!
✏️ Learnings added
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/graphql-codegen.ts:0-0
Timestamp: 2025-08-31T13:48:09.785Z
Learning: In the OWASP/Nest project, Django's GraphQL endpoint accepts CSRF tokens via 'x-csrftoken' header (lowercase) without requiring a Referer header, working fine in their configuration for GraphQL codegen introspection.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/graphql-codegen.ts:52-52
Timestamp: 2025-08-30T12:52:32.180Z
Learning: When using GraphQL Codegen with async configuration that needs to fetch data (like CSRF tokens), export the async function directly instead of using top-level await: `export default async function()` rather than `export default await function()`.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
frontend/graphql-codegen.ts (3)
64-64: Replace top-level await with async default exportTop-level await is brittle across ESM/CJS and was reported as failing; GraphQL Codegen supports an async default export. Adopt the learned pattern.
-export default await createCodegenConfig() +export default async function codegenConfig(): Promise<CodegenConfig> { + return createCodegenConfig() +}
5-15: Fail fast and harden CSRF retrieval (timeout + robust parsing + fallback to Set-Cookie)Prevents hangs, clears timeout, and avoids assuming a single JSON shape.
-const createCodegenConfig = async (): Promise<CodegenConfig> => { - const response = await fetch(`${PUBLIC_API_URL}/csrf/`, { - method: 'GET', - }) - - if (!response.ok) { - throw new Error(`Failed to fetch CSRF token: ${response.status} ${response.statusText}`) - } - const data = await response.json() - const csrfToken = data.csrftoken +const createCodegenConfig = async (): Promise<CodegenConfig> => { + const controller = new AbortController() + const timeout = setTimeout(() => controller.abort(), 5000) + const response = await fetch(`${PUBLIC_API_URL}/csrf/`, { + method: 'GET', + signal: controller.signal, + }) + clearTimeout(timeout) + if (!response.ok) { + throw new Error(`Failed to fetch CSRF token: ${response.status} ${response.statusText}`) + } + type CsrfResponse = { csrftoken?: string; csrfToken?: string; token?: string } + const data = (await response.json().catch(() => ({}))) as CsrfResponse + let csrfToken = data.csrftoken ?? data.csrfToken ?? data.token + if (!csrfToken) { + const setCookie = response.headers.get('set-cookie') ?? '' + const m = setCookie.match(/(?:^|;\s*)csrftoken=([^;]+)/i) + if (m) csrfToken = m[1] + } + if (!csrfToken) { + throw new Error('Failed to extract CSRF token from CSRF endpoint response') + }
55-58: Add Referer for Django CSRF (HTTPS) and set AcceptDjango’s CSRF checks require a matching Referer; Accept helps with content negotiation.
headers: { Cookie: `csrftoken=${csrfToken}`, 'X-CSRFToken': csrfToken, + Referer: PUBLIC_API_URL, + Accept: 'application/json', },
🧹 Nitpick comments (3)
frontend/graphql-codegen.ts (3)
17-17: Broaden documents globs; exclude generated output directoryEnsures .graphql/.gql files are picked up and avoids scanning generated files.
- documents: ['src/**/*.{ts,tsx}'], + documents: ['src/**/*.{ts,tsx,graphql,gql}', '!src/types/__generated__/**'],
3-4: Normalize PUBLIC_API_URL to avoid accidental double slashesHandles env values with a trailing slash.
-const PUBLIC_API_URL = process.env.PUBLIC_API_URL || 'http://localhost:8000' +const PUBLIC_API_URL = (process.env.PUBLIC_API_URL || 'http://localhost:8000').replace(/\/+$/, '')
53-60: CI strategy: validate queries without committing generated filesOption A: commit schema (schema.graphql or schema.json) and generate types offline. Option B: spin up backend during CI and run codegen. Recommend adding a CI job.
Example (GitHub Actions step):
- name: Generate GraphQL types working-directory: frontend env: PUBLIC_API_URL: http://localhost:8000 run: pnpm run graphql-codegenIf you choose to commit schema instead, add a scheduled job to refresh it and keep generated types out of git.
📜 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 (18)
frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlfrontend/src/types/__generated__/apiKeyQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/authQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/chapterQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/committeeQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/graphql.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/homeQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/mentorshipQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/moduleMutations.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/moduleQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/organizationQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/programsMutations.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/programsQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/projectQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/projectsHealthDashboardQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/repositoryQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/snapshotQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/userQueries.generated.tsis excluded by!**/__generated__/**
📒 Files selected for processing (4)
frontend/.prettierignore(1 hunks)frontend/eslint.config.mjs(1 hunks)frontend/graphql-codegen.ts(1 hunks)frontend/package.json(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/eslint.config.mjs
- frontend/.prettierignore
- frontend/package.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-30T12:52:32.180Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/graphql-codegen.ts:52-52
Timestamp: 2025-08-30T12:52:32.180Z
Learning: When using GraphQL Codegen with async configuration that needs to fetch data (like CSRF tokens), export the async function directly instead of using top-level await: `export default async function()` rather than `export default await function()`.
Applied to files:
frontend/graphql-codegen.ts
🔇 Additional comments (1)
frontend/graphql-codegen.ts (1)
1-1: Remove runtime verification comment
Node v24.3.0 confirms global fetch support andgraphql-codegen --config graphql-codegen.tscorrectly resolves async exports.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
frontend/graphql-codegen.ts (3)
6-8: Add timeout and harden CSRF extraction (fail fast, resilient to payload/cookie variants).Prevent hangs with AbortController and clear the timer; parse JSON safely and fall back to Set-Cookie.
const response = await fetch(`${PUBLIC_API_URL}/csrf/`, { - method: 'GET', + method: 'GET', + signal: controller.signal, }) + clearTimeout(timeout) if (!response.ok) { throw new Error(`Failed to fetch CSRF token: ${response.status} ${response.statusText}`) } - const data = await response.json() - const csrfToken = data.csrftoken + type CsrfResponse = { csrftoken?: string; csrfToken?: string; token?: string } + const data = (await response.json().catch(() => ({}))) as CsrfResponse + let csrfToken = data.csrftoken ?? data.csrfToken ?? data.token + if (!csrfToken) { + const setCookie = response.headers.get('set-cookie') ?? '' + const m = setCookie.match(/(?:^|;\s*)csrftoken=([^;]+)/i) + if (m) csrfToken = m[1] + } + if (!csrfToken) { + throw new Error('Failed to extract CSRF token from CSRF endpoint response') + }Add the controller/timeout near the fetch:
+ const controller = new AbortController() + const timeout = setTimeout(() => controller.abort(), 5000)Also applies to: 10-10, 13-15
5-5: Export an async function instead of an async IIFE (aligns with our learning).Export the async function directly to avoid loader/TLA quirks and match GraphQL Codegen’s supported patterns. Applying your retrieved learning here.
-export default (async (): Promise<CodegenConfig> => { +export default async function codegenConfig(): Promise<CodegenConfig> { // ... -})() +}Also applies to: 62-62
54-58: Add Referer header; keep canonical CSRF header casing (Django HTTPS requirement).Without a matching Referer, Django may reject POST introspection even with a valid token.
headers: { Cookie: `csrftoken=${csrfToken}`, 'X-CSRFToken': csrfToken, + Referer: PUBLIC_API_URL, },
🧹 Nitpick comments (3)
frontend/graphql-codegen.ts (3)
17-18: Broaden documents glob to include .graphql/.gql files.Covers teams storing operations in SDL files alongside TS/TSX.
- documents: ['src/**/*.{ts,tsx}', '!src/types/__generated__/**'], + documents: ['src/**/*.{ts,tsx,gql,graphql}', '!src/types/__generated__/**'],
37-44: Verify near-operation-file paths won’t scatter outputs under src/*/types/generated.With folder set to '../../types/generated', outputs depend on each operation file’s depth (e.g., src/pages/foo → src/pages/types/generated). If your intent is a single central folder (src/types/generated), consider either:
- Dropping near-operation-file and generating a single ops artifact, or
- Keeping near-operation-file with a stable folder strategy per your directory layout.
If centralizing ops is desired, one option:
- './src/': { - // near-operation-file ... - }, + './src/types/__generated__/operations.ts': { + plugins: ['typescript-operations', 'typed-document-node'], + config: { importOperationTypesFrom: './graphql' } + },Please run pnpm graphql-codegen locally and confirm generated paths land where you expect.
1-62: CI integration and offline schema strategy.To meet “validate queries” reliably in CI:
- Add a job that starts the backend before codegen, or
- Commit an introspected schema (schema.graphql or schema.json) and point schema to that file; add a separate script to refresh it when backend changes.
📜 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 (1)
frontend/graphql-codegen.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-30T12:52:32.180Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/graphql-codegen.ts:52-52
Timestamp: 2025-08-30T12:52:32.180Z
Learning: When using GraphQL Codegen with async configuration that needs to fetch data (like CSRF tokens), export the async function directly instead of using top-level await: `export default async function()` rather than `export default await function()`.
Applied to files:
frontend/graphql-codegen.ts
🔇 Additional comments (1)
frontend/graphql-codegen.ts (1)
20-37: Config choices look solid.Good use of typed-document-node + operations, avoidOptionals, and typename settings. Order sensitivity called out—nice.
|
@arkid15r Hey, this is ready for review. I had to rename GraphQL queries for the codegen to work (unique names are required). I renamed them based on their variable name ( I say that we name the queries the same as their variable names but in PascalCase because these generated typed queries will be used instead of current query string variables (see Example code line 6). I also think that we should refactor variable and query names as they are very inconsistent. The only constraint we have is that they should be unique. Sources of documentation I followed: |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
frontend/Makefile (3)
30-32: Add friendly pass/fail output to match format/lint targets.Keep the UX consistent and surface failures clearly when running locally.
-graphql-codegen: - @cd frontend && pnpm run graphql-codegen +graphql-codegen: + @(cd frontend && pnpm run graphql-codegen >/dev/null 2>&1 \ + && (printf "pnpm run graphql-codegen"; for i in $$(seq 1 44); do printf "."; done; printf "\033[30;42mPassed\033[0m\n") \ + || (printf "pnpm run graphql-codegen"; for i in $$(seq 1 44); do printf "."; done; printf "\033[37;41mFailed\033[0m\n" \ + && pnpm run graphql-codegen))
30-32: Consider gating check-frontend on codegen or add a “check-graphql-codegen” target.Prevents stale types from slipping in before lint/tests.
If you want, I can propose a tiny Makefile snippet that runs codegen and fails on dirty git diff to ensure generated files are committed.
30-32: Allow environment override/documentation for PUBLIC_API_URL.Codegen hits the live schema/CSRF endpoint; make it obvious how to point at non-default backends.
Examples:
- Run against staging:
PUBLIC_API_URL=https://staging.example.com make graphql-codegen- Optional: declare a default at the top of the Makefile:
PUBLIC_API_URL ?= http://localhost:8000(env can still override).- Optional: mark the new target as phony:
.PHONY: graphql-codegen
📜 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 (1)
frontend/Makefile(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/graphql-codegen.ts:52-52
Timestamp: 2025-08-30T12:52:32.180Z
Learning: When using GraphQL Codegen with async configuration that needs to fetch data (like CSRF tokens), export the async function directly instead of using top-level await: `export default async function()` rather than `export default await function()`.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run frontend unit tests
- GitHub Check: Run backend tests
- GitHub Check: CodeQL (javascript-typescript)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
frontend/src/server/queries/userQueries.ts (1)
4-12: Consider renaming $key → $login for clarity.All three queries pass a user login;
$loginis clearer and avoids confusion with other “key” usages (programKey/moduleKey). Low-touch change; generated types will also read better (e.g., GetUserDataQueryVariables.login).Also applies to: 15-87, 89-97
frontend/src/server/queries/moduleQueries.ts (2)
25-25: Rename operation to match actual selector (key, not id).The operation is named GetModuleByID but takes moduleKey/programKey. Rename to avoid misleading generated type names.
Apply:
-export const GET_MODULE_BY_ID = gql` - query GetModuleByID($moduleKey: String!, $programKey: String!) { +export const GET_MODULE_BY_ID = gql` + query GetModuleByKey($moduleKey: String!, $programKey: String!) {Optional follow-up (breaking for importers): also rename the constant to keep names aligned.
-export const GET_MODULE_BY_ID = gql` +export const GET_MODULE_BY_KEY = gql`
15-19: Deduplicate repeated user fields with a fragment (optional).Admins/mentors repeat id/login/name/avatarUrl. A shared fragment will reduce drift and produce reusable fragment types from codegen.
Also applies to: 36-41, 50-55, 70-74
frontend/src/server/queries/programsQueries.ts (1)
84-97: Optional: extract admin user fields into a shared fragment.id/login/name/avatarUrl repeats across program queries; a fragment will reduce duplication and help codegen produce consistent types.
📜 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 (9)
frontend/src/server/queries/chapterQueries.ts(2 hunks)frontend/src/server/queries/committeeQueries.ts(2 hunks)frontend/src/server/queries/moduleQueries.ts(3 hunks)frontend/src/server/queries/organizationQueries.ts(2 hunks)frontend/src/server/queries/programsQueries.ts(1 hunks)frontend/src/server/queries/projectsHealthDashboardQueries.ts(2 hunks)frontend/src/server/queries/repositoryQueries.ts(2 hunks)frontend/src/server/queries/snapshotQueries.ts(1 hunks)frontend/src/server/queries/userQueries.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- frontend/src/server/queries/snapshotQueries.ts
- frontend/src/server/queries/repositoryQueries.ts
- frontend/src/server/queries/organizationQueries.ts
- frontend/src/server/queries/committeeQueries.ts
- frontend/src/server/queries/projectsHealthDashboardQueries.ts
- frontend/src/server/queries/chapterQueries.ts
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/graphql-codegen.ts:52-52
Timestamp: 2025-08-30T12:52:32.180Z
Learning: When using GraphQL Codegen with async configuration that needs to fetch data (like CSRF tokens), export the async function directly instead of using top-level await: `export default async function()` rather than `export default await function()`.
📚 Learning: 2025-08-31T13:47:15.832Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.832Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details. These queries cannot be removed or merged as they serve different use cases in the application.
Applied to files:
frontend/src/server/queries/programsQueries.tsfrontend/src/server/queries/moduleQueries.ts
📚 Learning: 2025-08-31T13:47:15.833Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.833Z
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/server/queries/programsQueries.tsfrontend/src/server/queries/moduleQueries.ts
📚 Learning: 2025-08-31T13:47:15.832Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.832Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details.
Applied to files:
frontend/src/server/queries/programsQueries.tsfrontend/src/server/queries/moduleQueries.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run frontend unit tests
- GitHub Check: Run backend tests
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (6)
frontend/src/server/queries/userQueries.ts (1)
4-4: Approve operation-name rename
Ran a workspace search for "GetUser" references; none found—no dashboards or persisted-query allowlists require updates.frontend/src/server/queries/moduleQueries.ts (4)
4-4: Unique operation name: LGTM.This aligns with codegen’s uniqueness requirement.
17-19: Schema fix for mentors: LGTM.Switching from the non-existent githubUser nesting to direct fields resolves the reported codegen error.
47-47: Rename to GetProgramAdminsAndModules: LGTM.Clearer intent and unique for codegen.
4-4: No external dependencies on old operation names found.frontend/src/server/queries/programsQueries.ts (1)
84-84: Rename to GetProgramAdminDetails: LGTM.Keeps it distinct from GET_PROGRAM_DETAILS (different purpose) and avoids name clashes in generated types.
.github/workflows/run-ci-cd.yaml
Outdated
| - name: Run pre-commit | ||
| env: | ||
| SKIP: graphql-codegen |
There was a problem hiding this comment.
This should probably be part of make check target if you skip it here.
Also this way we'll avoid adding local pre-commit hook
There was a problem hiding this comment.
I've made this change. Thank you
| "@fortawesome/free-regular-svg-icons": "^6.7.2", | ||
| "@fortawesome/free-solid-svg-icons": "^6.7.2", | ||
| "@fortawesome/react-fontawesome": "^3.0.1", | ||
| "@graphql-typed-document-node/core": "^3.2.0", |
There was a problem hiding this comment.
Does apollo pick up the generated types automagically based on the location?
There was a problem hiding this comment.
No, We need to import the types in each file where useQuery hook is used.
Example
Is it okay if I refactor all this in a different PR? I will need to update the tests too. I believe this refactor should complete the apollo migration.
For now, the generated files are unused.
It's recommended by apollo to separate out queries (one generated file for each query/mutation file) for larger applications. If we don't do this, all the queries and mutation are generated in the graphql.ts file.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
.pre-commit-config.yaml (1)
67-67: Anchor the exclude regex so all generated files are skipped.Current pattern may not exclude nested files reliably. Use an anchored directory regex to avoid the hook touching generated outputs.
- exclude: frontend/src/types/__generated__ + exclude: ^frontend/src/types/__generated__/frontend/Makefile (3)
5-6: Run codegen before lint to avoid “module not found”/type-resolver hiccups on clean clones.If eslint uses type-aware rules, missing generated types can cause false negatives. Safer to generate first, then lint.
check-frontend: \ format-frontend-code \ - lint-frontend-code \ - generate-graphql-types + generate-graphql-types \ + lint-frontend-code
31-33: Minor: keep subshell style consistent with other targets.Other targets use
(cd … && …). Align for readability.-graphql-codegen: - @cd frontend && pnpm run graphql-codegen +graphql-codegen: + @(cd frontend && pnpm run graphql-codegen)Additionally, declare new targets as phony to avoid filename collisions:
+.PHONY: graphql-codegen generate-graphql-types
46-51: Good UX: quiet run + on-failure re-run with logs.Nice touch. Consider forcing color for CI logs and a clearer non-zero exit on second failure (already implied, just making it explicit).
- @(cd frontend && pnpm run graphql-codegen >/dev/null 2>&1 \ + @(cd frontend && FORCE_COLOR=1 pnpm run graphql-codegen >/dev/null 2>&1 \ && (printf "pnpm run graphql-codegen"; for i in $$(seq 1 49); do printf "."; done; printf "\033[30;42mPassed\033[0m\n") \ || (printf "pnpm run graphql-codegen"; for i in $$(seq 1 49); do printf "."; done; printf "\033[37;41mFailed\033[0m\n" \ - && pnpm run graphql-codegen)) + && pnpm run graphql-codegen || exit $$?))
📜 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 (14)
frontend/src/types/__generated__/apiKeyQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/authQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/chapterQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/committeeQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/graphql.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/homeQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/moduleQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/organizationQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/programsQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/projectQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/projectsHealthDashboardQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/repositoryQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/snapshotQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/userQueries.generated.tsis excluded by!**/__generated__/**
📒 Files selected for processing (3)
.pre-commit-config.yaml(1 hunks)frontend/Makefile(3 hunks)frontend/src/server/queries/authQueries.ts(0 hunks)
💤 Files with no reviewable changes (1)
- frontend/src/server/queries/authQueries.ts
⏰ 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)
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
frontend/graphql-codegen.ts (3)
5-5: Export an async function instead of an async IIFE.Avoids immediate execution on import and aligns with Codegen’s async-config pattern used in this repo.
-export default (async (): Promise<CodegenConfig> => { +export default async function codegenConfig(): Promise<CodegenConfig> { ... -})() +}Also applies to: 69-69
9-16: Add a fetch timeout and fail fast; don’t return undefined config.Prevents hanging runs and surfaces a clear error instead of exporting
undefined.try { - response = await fetch(`${PUBLIC_API_URL}/csrf/`, { - method: 'GET', - }) - } catch { - /* eslint-disable no-console */ - console.log('Failed to fetch CSRF token: make sure the backend is running.') - return + const controller = new AbortController() + const timeout = setTimeout(() => controller.abort(), 5000) + response = await fetch(`${PUBLIC_API_URL}/csrf/`, { + method: 'GET', + signal: controller.signal, + }) + clearTimeout(timeout) + } catch (err: unknown) { + // Ensure we fail fast with a helpful message + const isAbort = typeof err === 'object' && err !== null && (err as any).name === 'AbortError' + throw new Error(`${isAbort ? 'Timed out' : 'Failed'} to fetch CSRF token from ${PUBLIC_API_URL}/csrf/`) }
21-21: Harden CSRF extraction; validate presence before proceeding.Handles alternate payloads and falls back to Set-Cookie.
- const csrfToken = (await response.json()).csrftoken + type CsrfResponse = { csrftoken?: string; csrfToken?: string; token?: string } + const data = (await response.json().catch(() => ({}))) as CsrfResponse + const csrfToken = + data.csrftoken ?? + data.csrfToken ?? + data.token ?? + (() => { + const setCookie = response.headers.get('set-cookie') ?? '' + const m = /(?:^|;\s*)csrftoken=([^;]+)/i.exec(setCookie) + return m ? m[1] : undefined + })() + if (!csrfToken) { + throw new Error('CSRF token missing in CSRF endpoint response') + }
🧹 Nitpick comments (3)
frontend/graphql-codegen.ts (3)
44-51: Prevent potential filename collisions from the flattened target folder.Set an explicit extension for near-operation-file outputs.
presetConfig: { // This should be the file generated by the "typescript" plugin above, // relative to the directory specified for this configuration baseTypesPath: './types/__generated__/graphql.ts', // Relative to the source files folder: '../../types/__generated__', + extension: '.generated.ts', },
3-3: Nit: avoid possible “//” in URLs when PUBLIC_API_URL has a trailing slash.Optionally normalize:
const API_URL = PUBLIC_API_URL.replace(/\/+$/, '')and use API_URL for/csrf/and/graphql/.
5-69: Reduce coupling to a live backend for codegen.Consider generating from a committed schema artifact (SDL or introspection JSON) updated via a CI job, so local/CI runs don’t depend on CSRF/network availability.
📜 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 (1)
frontend/graphql-codegen.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-30T12:52:32.180Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/graphql-codegen.ts:52-52
Timestamp: 2025-08-30T12:52:32.180Z
Learning: When using GraphQL Codegen with async configuration that needs to fetch data (like CSRF tokens), export the async function directly instead of using top-level await: `export default async function()` rather than `export default await function()`.
Applied to files:
frontend/graphql-codegen.ts
📚 Learning: 2025-08-31T13:48:09.785Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/graphql-codegen.ts:0-0
Timestamp: 2025-08-31T13:48:09.785Z
Learning: In the OWASP/Nest project, Django's GraphQL endpoint accepts CSRF tokens via 'x-csrftoken' header (lowercase) without requiring a Referer header, working fine in their configuration for GraphQL codegen introspection.
Applied to files:
frontend/graphql-codegen.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run frontend unit tests
- GitHub Check: Run backend tests
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (2)
frontend/graphql-codegen.ts (2)
27-41: Typegen config choices look solid.Good defaults: avoidOptionals.field=true, defaultScalarType=unknown, nonOptionalTypename, skipTypeNameForRoot.
61-67: CSRF headers OK.Header names are case-insensitive; this matches the project’s working setup.
* configure graphql-codegen * refactor query names to be unique * Update code (sort alphabetically) * ignore graphql-codegen generated files * Run graphql-codegen * remove fetchCsrfTokenServer and use a config constructor * Update code * use near-operation-file-preset * use typed-document-node plugin * Update code (sort alphabetically) * Update code * add graphql-codegen generated types to end-of-file-fixer * add hook for graphql-codegen for schema validation * Update code * skip graphql-codegen pre-commit hook * add graphql-codegen * add generate-graphql-types to make check * run make check * Update code --------- Co-authored-by: Arkadii Yakovets <[email protected]> Co-authored-by: Arkadii Yakovets <[email protected]>
* configure graphql-codegen * refactor query names to be unique * Update code (sort alphabetically) * ignore graphql-codegen generated files * Run graphql-codegen * remove fetchCsrfTokenServer and use a config constructor * Update code * use near-operation-file-preset * use typed-document-node plugin * Update code (sort alphabetically) * Update code * add graphql-codegen generated types to end-of-file-fixer * add hook for graphql-codegen for schema validation * Update code * skip graphql-codegen pre-commit hook * add graphql-codegen * add generate-graphql-types to make check * run make check * Update code --------- Co-authored-by: Arkadii Yakovets <[email protected]> Co-authored-by: Arkadii Yakovets <[email protected]>
2 participants
Resolves #1600
Proposed change
Configure graphql-codegen using this documentation
The codegen generates types and validates the queries.
Use
pnpm run graphql-codegento run the codegen.Checklist
make check-testlocally; all checks and tests passed.