Skip to content

feat: optimize the about page queries#3626

Closed
Utkarsh-0304 wants to merge 5 commits intoOWASP:mainfrom
Utkarsh-0304:feat/optimize_about_page_queries
Closed

feat: optimize the about page queries#3626
Utkarsh-0304 wants to merge 5 commits intoOWASP:mainfrom
Utkarsh-0304:feat/optimize_about_page_queries

Conversation

@Utkarsh-0304
Copy link
Contributor

@Utkarsh-0304 Utkarsh-0304 commented Jan 28, 2026

Proposed change

Resolves #3592

This PR optimises the about page queries by combining them into a single query. Unit and e2e tests are also modified to handle these changes. Some changes related to a separate new query field, users, have been added to handle leaders' data and follow the DRY principle.

Checklist

  • Required: I followed the contributing workflow
  • Required: I verified that my code works as intended and resolves the issue as described
  • Required: I ran make check-test locally: all warnings addressed, tests passed
  • I used AI for code, documentation, tests, or communication related to this PR

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 28, 2026

Walkthrough

Adds a backend GraphQL field to fetch multiple users by login, consolidates multiple frontend About-page GraphQL requests into a single query and updates the About page and tests to use the unified response, and renames local docker-compose volumes by appending _3592.

Changes

Cohort / File(s) Summary
Backend Query Enhancement
backend/apps/github/api/internal/queries/user.py
Adds users(self, logins: list[str]) -> list[UserNode] that filters by login__in and has_public_member_page, and preserves input order via a Case/When ordering expression.
Frontend About query & usage
frontend/src/server/queries/aboutQueries.ts, frontend/src/app/about/page.tsx
Adds GET_ABOUT_PAGE_DATA and replaces multiple per-section queries in the About page with a single request; derives project, topContributors, and users from the unified response and removes prior per-query hooks/logic.
Frontend tests & mock data
frontend/__tests__/e2e/pages/About.spec.ts, frontend/__tests__/mockData/mockAboutData.ts, frontend/__tests__/unit/pages/About.test.tsx
Mocks and test data updated to return/use consolidated about payload; mockAboutData.users changed from keyed object to array; unit and e2e tests refactored to use single mocked response and adjusted assertions/failure scenarios.
Docker Compose volumes
docker-compose/local/compose.yaml
All local volume names and references renamed to append _3592 (services and top-level volumes map).
Backend tests
backend/tests/apps/github/api/internal/queries/user_test.py
Adds unit tests for UserQuery.users covering filtering, empty input, and preservation of input order via ordering behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~28 minutes

Possibly related PRs

Suggested reviewers

  • arkid15r
  • kasya
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: optimizing about page queries by consolidating them.
Linked Issues check ✅ Passed The PR successfully implements the objective from #3592: combines multiple GraphQL queries into a single GetAboutPageDataDocument query and updates frontend to use it.
Out of Scope Changes check ✅ Passed All changes are directly related to the core objective of query optimization and include supporting changes to tests and the new users field.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description check ✅ Passed The PR description clearly relates to the changeset by explaining the optimization of About page queries into a single query and mentioning the new users field.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
frontend/__tests__/e2e/pages/About.spec.ts (1)

7-22: Add a fallback for unexpected operations to avoid hanging tests.

Right now non-matching GraphQL operations never resolve, which can stall the test suite. Fail fast or pass through explicitly.

🛠️ Proposed fix (fail fast)
-      if (postData.operationName === 'GetAboutPageData') {
+      if (postData.operationName === 'GetAboutPageData') {
         await route.fulfill({
           status: 200,
           json: {
             data: {
               project: mockAboutData.project,
               topContributors: mockAboutData.topContributors,
               users: mockAboutData.users,
             },
           },
         })
+        return
       }
+      await route.fulfill({
+        status: 400,
+        json: { errors: [{ message: `Unexpected operation: ${postData.operationName}` }] },
+      })
🤖 Fix all issues with AI agents
In `@backend/apps/github/api/internal/queries/user.py`:
- Around line 56-67: The users resolver currently returns a list and uses
login__in which loses input order and prevents Strawberry/Django optimizer
hooks; change the users method to return a QuerySet (not list) from
User.objects.filter(login__in=logins, has_public_member_page=True) and apply an
explicit ordering that preserves the order of the incoming logins using Django's
Case/When (build When conditions for each login and order_by the Case
expression) so results match the input sequence while keeping a QuerySet for
optimizer integration.

In `@frontend/__tests__/unit/pages/About.test.tsx`:
- Around line 417-421: The test "handles null project in data response
gracefully" is mocking useQuery incorrectly by returning project at the top
level; update the mock for useQuery (used in this test) to return an object
shaped like the real hook: { data: { ...mockAboutData, project: null }, loading:
false, error: null } so the component receives data.project === null and follows
the intended code path; keep the mockAboutData reference and the test name to
locate the test and adjust only the mockReturnValue shape.

In `@frontend/src/app/about/page.tsx`:
- Around line 77-83: The variable leadersData can be undefined when
aboutPageDataResponse?.users is missing, causing downstream components to break;
update the leadersData assignment (the leadersData constant that builds from
aboutPageDataResponse?.users) to default to an empty array and only map when
users exist — e.g., treat aboutPageDataResponse?.users || [] (or use a
conditional fallback) so leadersData always yields an array of objects using
leaders[leader.login], memberName and member.

coderabbitai[bot]
coderabbitai bot previously approved these changes Jan 28, 2026
@Utkarsh-0304 Utkarsh-0304 marked this pull request as ready for review January 28, 2026 14:39
@sonarqubecloud
Copy link

@Utkarsh-0304 Utkarsh-0304 marked this pull request as ready for review January 29, 2026 13:23
name
}

users(logins: $leaderLogins) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a better approach long term 👍

@arkid15r
Copy link
Collaborator

Closing in favor of #3604

@arkid15r arkid15r closed this Jan 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Optimize the About page queries

2 participants

Comments