added ids to frontend graphql queries #2161
Conversation
Summary by CodeRabbit
WalkthroughAdds Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
frontend/src/server/queries/snapshotQueries.ts (1)
63-70: Rename metadata GraphQL operation to avoid name collisionRename the GraphQL operation in
GET_SNAPSHOT_DETAILS_METADATAtoGetSnapshotDetailsMetadatato ensure each operation name is unique.export const GET_SNAPSHOT_DETAILS_METADATA = gql` - query GetSnapshotDetails($key: String!) { + query GetSnapshotDetailsMetadata($key: String!) { snapshot(key: $key) { id title } } `frontend/src/server/queries/organizationQueries.ts (1)
109-118: Rename duplicate GraphQL operation name
Infrontend/src/server/queries/organizationQueries.ts, bothGET_ORGANIZATION_DATAandGET_ORGANIZATION_METADATAusequery GetOrganization, which will collide at runtime. Change the metadata query’s operation name to something unique, for example:export const GET_ORGANIZATION_METADATA = gql` - query GetOrganization($login: String!) { + query GetOrganizationMetadata($login: String!) { organization(login: $login) { id description login name } } `frontend/src/server/queries/userQueries.ts (1)
3-12: Rename GraphQL operation names to match constants
In frontend/src/server/queries/userQueries.ts, update eachquery GetUserto a unique name that mirrors its export, e.g.:export const GET_LEADER_DATA = gql` - query GetUser($key: String!) { + query GetLeaderData($key: String!) { user(login: $key) { id avatarUrl login name } } ` export const GET_USER_DATA = gql` - query GetUser($key: String!) { + query GetUserData($key: String!) { … } ` export const GET_USER_METADATA = gql` - query GetUser($key: String!) { + query GetUserMetadata($key: String!) { user(login: $key) { id bio login name } } `Prevents codegen/type collisions and aids debugging.
🧹 Nitpick comments (8)
frontend/src/server/queries/homeQueries.ts (1)
6-6: Broad ID coverage — looks good.This should stabilize list rendering keys and Apollo normalization for home widgets.
Consider extracting a shared author fragment (if your schema exposes a common interface/type for authors) to remove repetition across issues/PRs/releases/milestones.
Also applies to: 16-16, 24-24, 32-32, 38-41, 52-55, 66-69, 81-81, 95-95, 106-109
frontend/src/server/queries/repositoryQueries.ts (1)
6-6: Repository graph selections now include stable IDs — LGTM.IDs on
repository,issues,releases,recentMilestones,topContributors,recentPullRequests, and nestedauthorobjects align with Apollo best practices.If multiple queries repeat
author { id avatarUrl login name }, consider a fragment to DRY them up when the underlying type is shared.Also applies to: 14-17, 31-31, 35-35, 40-43, 60-63, 77-77, 83-86, 102-102
frontend/src/server/queries/projectQueries.ts (1)
31-44: Optional: Add ids for list items and repository.organization for full normalization.
Helps Apollo identify list items and nested orgs, reducing cache warnings/duplicates.recentIssues { + id author { id avatarUrl login name url } createdAt organizationName repositoryName title url } recentReleases { + id author { id avatarUrl login name } name organizationName publishedAt repositoryName tagName url } repositories { id contributorsCount forksCount key name openIssuesCount organization { + id login } starsCount subscribersCount url } recentMilestones(limit: 5) { + id author { id avatarUrl login name } title openIssuesCount closedIssuesCount repositoryName organizationName createdAt url } recentPullRequests { + id author { id avatarUrl login name } createdAt organizationName repositoryName title url }Also applies to: 45-58, 80-94, 95-107, 66-69
frontend/src/server/queries/moduleQueries.ts (1)
40-44: Optional: Normalize “user” shapes via fragments to reduce duplication and drift.
If mentors/admins share the same GraphQL type, consider a common fragment.Example fragment (add once in a shared queries file or alongside these docs):
export const USER_BASIC_FIELDS = gql` fragment UserBasicFields on User { id login name avatarUrl } `Then use:
mentors { - id - login - name - avatarUrl + ...UserBasicFields }Note: Only apply if mentors/admins are of type User; otherwise keep as-is.
Also applies to: 73-77
frontend/src/server/queries/programsQueries.ts (1)
54-58: Optional: Consider a shared User fragment to keep selections consistent across modules.Example:
export const USER_BASIC_FIELDS = gql` fragment UserBasicFields on User { id login name avatarUrl } `And replace repeated fields with
...UserBasicFieldsin admins/mentors blocks where the type is User.Also applies to: 77-81, 108-112
frontend/src/server/queries/snapshotQueries.ts (1)
29-34: Deduplicate repeated contributor selections with a fragment (optional).Same fields for topContributors appear multiple times. Consider a fragment for maintainability.
+const CONTRIBUTOR_BASIC = gql` + fragment ContributorBasic on User { + id + avatarUrl + login + name + } +`; ... - topContributors { - id - avatarUrl - login - name - } + topContributors { ...ContributorBasic } ... - topContributors { - id - avatarUrl - login - name - } + topContributors { ...ContributorBasic }Also applies to: 45-49
frontend/src/server/queries/organizationQueries.ts (1)
35-40: Use an author fragment to DRY repeated fields (optional).author has the same four fields in three places; consider a fragment.
const AUTHOR_BASIC = gql` fragment AuthorBasic on User { id avatarUrl login name } `;Then replace author blocks with
{ ...AuthorBasic }.Also applies to: 50-54, 65-69
frontend/src/server/queries/userQueries.ts (1)
53-66: Optional: introduce fragments for repository and user basics.Repeated blocks (repository basics and user basics) can use shared fragments to reduce drift and ease future changes.
Also applies to: 68-85
📜 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 (13)
frontend/src/server/queries/apiKeyQueries.ts(2 hunks)frontend/src/server/queries/authQueries.ts(2 hunks)frontend/src/server/queries/chapterQueries.ts(3 hunks)frontend/src/server/queries/committeeQueries.ts(3 hunks)frontend/src/server/queries/homeQueries.ts(7 hunks)frontend/src/server/queries/moduleQueries.ts(3 hunks)frontend/src/server/queries/organizationQueries.ts(6 hunks)frontend/src/server/queries/programsQueries.ts(4 hunks)frontend/src/server/queries/projectQueries.ts(10 hunks)frontend/src/server/queries/projectsHealthDashboardQueries.ts(3 hunks)frontend/src/server/queries/repositoryQueries.ts(5 hunks)frontend/src/server/queries/snapshotQueries.ts(4 hunks)frontend/src/server/queries/userQueries.ts(5 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 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
📚 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
⏰ 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 (15)
frontend/src/server/queries/committeeQueries.ts (1)
6-6: IDs added for Apollo normalization — LGTM.Adding
idtocommittee,topContributors, and metadata should resolve cache identity issues.If you previously had custom typePolicies for these types, confirm they still key by
id(or remove overrides) to avoid duplicate entities.Also applies to: 21-21, 32-32
frontend/src/server/queries/apiKeyQueries.ts (1)
7-7: No custom cache policy found; Apollo Client defaults to usingidfor normalization. The codebase contains notypePoliciesorkeyFieldsforApiKey, soInMemoryCachewill automatically generate identifiers asApiKey:<id>by default (unpkg.com).frontend/src/server/queries/projectsHealthDashboardQueries.ts (1)
28-28: Health metric IDs added — verify they identify snapshots, not projects.For
healthMetricsLatestandhealthMetricsList, ensureidrefers to the metric entry/snapshot. If it’s the projectid, Apollo will collapse distinct timepoints into one cache entry.If unsure, quickly check server schema/resolvers for the
ProjectHealthMetrictype to confirmiduniqueness per snapshot.Also applies to: 43-46, 62-62
frontend/src/server/queries/authQueries.ts (3)
6-6: LGTM: Adding id to logoutUser payload improves Apollo normalization.
20-21: LGTM: Adding user.id is correct for cache identity.
6-6: Verify backend schema exposesidfor logoutUser and auth userFrontend queries already request
id(frontend/src/server/queries/authQueries.ts — logoutUser { id, ... } and the auth response user { id, ... }); repository search did not locate server-side SDL/resolver definitions that explicitly declareidon the logout response or the auth user type. Confirm backend GraphQL types/resolvers returnidfor logoutUser and githubAuth.user to avoid runtime query errors.frontend/src/server/queries/projectQueries.ts (1)
6-6: LGTM: Consistent id additions across project, metrics, nested authors, repositories, milestones, PRs, and topContributors.
These will stabilize Apollo cache keys without altering inputs.Also applies to: 17-17, 33-33, 47-47, 60-60, 82-82, 97-97, 110-110, 121-121, 129-129, 153-153
frontend/src/server/queries/chapterQueries.ts (1)
6-6: LGTM: Added ids for chapter and topContributors selections.
Matches the PR’s objective and should aid cache identity.Also applies to: 22-22, 33-33
frontend/src/server/queries/moduleQueries.ts (1)
40-44: LGTM: Adding id to mentors/admins selections is consistent and beneficial.Also applies to: 54-58, 73-77
frontend/src/server/queries/programsQueries.ts (1)
54-58: LGTM: ids added to admins and mentors selections across queries.
This aligns with the cache-fix objective without changing variables.Also applies to: 77-81, 92-96, 108-112
frontend/src/server/queries/snapshotQueries.ts (1)
6-6: LGTM on adding ids (improves Apollo normalization).Adding id to snapshot, newReleases, newProjects (plus topContributors), newChapters (plus topContributors), and snapshots list is correct and low-risk.
Also applies to: 12-17, 19-35, 37-58, 75-80
frontend/src/server/queries/organizationQueries.ts (2)
6-26: LGTM on id additions across organization data.Adding id to organization, lists (issues, PRs, releases, milestones), authors, repositories, and organization within repositories aligns with Apollo cache best practices.
Also applies to: 28-32, 34-46, 48-61, 63-77, 79-91, 93-105
6-6: No typePolicies or keyFields overrides detected
Verified bothInMemoryCacheinstances infrontend/src/server/apolloClient.tsandfrontend/src/utils/helpers/apolloClient.tshave no customtypePoliciesorkeyFields, so default__typename+idnormalization applies.frontend/src/server/queries/userQueries.ts (2)
6-10: LGTM on id additions for user, activity lists, and repositories.This should resolve Apollo warnings and improves cache normalization. Remember to regenerate GraphQL types/artifacts if you use codegen.
Also applies to: 17-23, 25-33, 35-41, 43-51, 53-66, 68-85, 91-95
3-12: Verify GraphQL operation name uniqueness: The automation scripts weren’t reliably extracting names—manually ensure allquery,mutation, andsubscriptionoperation names infrontend/src/server/queries/*.tsare unique.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
backend/tests/apps/github/api/internal/queries/repository_contributor_test.py (2)
20-21: Add assertions to verifyidis propagated to result nodes.Mock data now includes
id, but tests don’t assert it’s carried through by the resolver. Add quick checks to prevent regressions.Python snippets to add:
# In test_top_contributors_basic, after other assertions: assert result[0].id == "1" assert isinstance(result[0].id, str) assert result[1].id == "2" # In test_top_contributors_with_limit, after asserting login: assert result[0].id == "1"Also applies to: 29-30
129-131: Also assertidon the constructed node.This test validates field mapping; include
idto fully cover the new field.# After node = result[0] and existing field asserts: assert node.id == "3" assert isinstance(node.id, str)
📜 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 (2)
backend/tests/apps/github/api/internal/queries/repository_contributor_test.py(3 hunks)backend/tests/apps/github/models/repository_contributor_test.py(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- backend/tests/apps/github/models/repository_contributor_test.py
⏰ 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)
kasya
left a comment
kasya
left a comment
There was a problem hiding this comment.
Hi @Piyushrathoree ! Thanks for working on this.
once started the server I noticed there were some issues on compiling the home page. Apparently, one node still missed an id on the backend.
So I fixed that (and tests) and pushed the update to your PR.
The rest works great!
Thanks!
@kasya can you share some more details because I'm not seeing any issue |
|
* added ids to frontend graphql queries * Add top contributors ID * Fix tests --------- Co-authored-by: Kate <kate@kgthreads.com> Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com>
3 participants
Proposed change
Resolves #2121
added Id field in the frontend graphQL queries to resolved apollo error.
Checklist
make check-testlocally; all checks and tests passed.