feat: implement community page#3873
Conversation
Summary by CodeRabbit
WalkthroughAdds a Community feature: backend GraphQL field to fetch recent OWASP-related organizations; new frontend Community page, layout, metadata, GraphQL query, components, and tests; breadcrumb hook updated to insert a "Community" segment for related routes. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 18 files
Confidence score: 4/5
- This PR looks generally safe to merge with a small, contained risk from one medium‑severity issue.
frontend/src/app/community/page.tsxstringifieschapter.leaders, which can lose structure and may change howLeadersListrenders leader data.- Pay close attention to
frontend/src/app/community/page.tsx- ensureLeadersListreceives the leaders array directly to avoid lossy conversion.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="frontend/src/app/community/page.tsx">
<violation number="1" location="frontend/src/app/community/page.tsx:168">
P2: `String(chapter.leaders)` converts an `Array<string>` to a comma-separated string. This is a lossy conversion — pass the array directly to the `LeadersList` component instead of stringifying it.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@backend/tests/apps/github/api/internal/queries/organization_test.py`:
- Around line 52-63: The test for OrganizationQuery.recent_organizations is not
verifying that the queryset is sliced; change mock_qs.order_by to return another
Mock instead of a list (e.g., next_qs = Mock(); mock_qs.order_by.return_value =
next_qs) and assert that next_qs.__getitem__ was called with slice(None, 2) to
ensure the [:limit] operation occurs; additionally add a separate test calling
OrganizationQuery().recent_organizations with limit=0 (or an invalid limit) and
assert it returns [] to cover the normalize_limit → None branch.
In `@frontend/src/app/community/page.tsx`:
- Around line 194-202: The org avatar URL is built incorrectly by always
appending "&s=80" to org.avatarUrl; update the Image src construction (the JSX
using org.avatarUrl inside page.tsx) to conditionally append a size param like
the contributor avatar does—i.e., if org.avatarUrl includes a "?" use "&s=80",
otherwise use "?s=80"—so the src becomes a properly formed URL when rendering
the Image component for org avatars.
- Around line 59-61: The current render early-return uses "if (isLoading ||
!graphQLData) return <LoadingSpinner />", which causes a permanent spinner when
the GraphQL query errored; update the conditional in page.tsx to explicitly
check graphQLRequestError (the error variable returned by your GraphQL hook)
before falling back to LoadingSpinner, and render an error/empty state (or a
friendly message/component) when graphQLRequestError is present instead of
treating it as a loading state; ensure references to isLoading, graphQLData, and
graphQLRequestError are used so the UI shows a spinner only while loading and an
error/empty view when the request fails.
🧹 Nitpick comments (5)
frontend/__tests__/unit/hooks/useBreadcrumbs.test.tsx (1)
80-86: Consider adding a test for the/communitypath itself.To ensure no duplicate "Community" breadcrumb appears when navigating directly to
/community, a test asserting[{ title: 'Home', path: '/' }, { title: 'Community', path: '/community' }]would be a useful edge-case addition.frontend/src/app/community/page.tsx (1)
63-63: Redundant alias.
const data = graphQLDataadds no value sincegraphQLDatais already descriptively named. Consider usinggraphQLDatadirectly or destructuring the query result asdatafrom the start.frontend/__tests__/unit/pages/Community.test.tsx (1)
53-70: Error test doesn't assert the rendered UI state.The test verifies the toast is triggered but doesn't assert what the user actually sees. Given the permanent-spinner bug noted in the page component, this test would pass even with that broken UX. Consider adding an assertion for the rendered output after error (e.g., verifying no loading spinner or showing an error message).
frontend/__tests__/e2e/pages/Community.spec.ts (1)
40-45: Fragile CSS class selector for navigation cards.
page.locator('.grid.grid-cols-2')is tied to Tailwind utility classes and will break if styling changes. Consider using adata-testidattribute or a more semantic locator.frontend/__tests__/e2e/data/mockCommunityData.ts (1)
59-69:topContributorsentries are missing theidfield.The GraphQL query requests
idfortopContributors, and the unit test mock (mockCommunityGraphQLData) includes it, but this e2e fixture omits it. While this doesn't affect rendering (the page usesloginas the key), it's an inconsistency with the actual query response shape.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@frontend/src/app/community/page.tsx`:
- Around line 248-266: The contributors list always renders the Image even when
contributor.avatarUrl is null, causing broken src like "null?s=60"; update the
contributors rendering to guard avatar rendering the same way as orgs: only
render the <Image> when contributor.avatarUrl is truthy, e.g., wrap the Image in
a conditional check on contributor.avatarUrl (or use {contributor.avatarUrl &&
(...)}) and keep getMemberUrl, Link and the text rendering unchanged; also
normalize optional chaining by removing unnecessary contributor?. usages inside
the .map callback where contributor is guaranteed to be defined (e.g., use
contributor.name, contributor.login, contributor.avatarUrl consistently).
🧹 Nitpick comments (2)
backend/tests/apps/github/api/internal/queries/organization_test.py (1)
68-82: Good edge-case coverage for invalid limits.Both tests correctly verify the early-return
[]path and confirm no database query is issued.Consider also adding a test with
limit > MAX_LIMIT(e.g.,limit=200) to verify the clamping behavior ofnormalize_limit, ensuring the slice usesMAX_LIMIT(100) rather than the raw input.frontend/src/app/community/page.tsx (1)
73-73: Redundant alias:const data = graphQLData.
datais just a reassignment ofgraphQLDatawith no transformation. You could either rename the destructured variable directly in theuseQuerycall or usegraphQLDatathroughout.♻️ Suggested simplification
const { - data: graphQLData, + data, error: graphQLRequestError, loading: isLoading, } = useQuery(GetCommunityPageDataDocument) ... - if (graphQLRequestError || !graphQLData) { + if (graphQLRequestError || !data) { ... } - - const data = graphQLData
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@frontend/src/app/community/page.tsx`:
- Around line 75-92: The code constructs statsData by accessing
data.statsOverview fields directly and also calls millify later, which will
throw if statsOverview is null/undefined; update the code to null-safe these
accesses by using optional chaining and sensible defaults (e.g.,
data.statsOverview?.activeChaptersStats ?? 0 or a placeholder) when building
statsData and before calling millify, or skip/render a fallback when the value
is missing; refer to the statsData array and any millify(...) call to add these
guards so runtime TypeErrors are prevented.
🧹 Nitpick comments (2)
frontend/src/app/community/page.tsx (2)
73-73: Redundant aliasconst data = graphQLData.This intermediate variable adds no value since
graphQLDatais already descriptively named and narrowed to non-null by the guard above. Consider usinggraphQLDatadirectly or destructuring the needed fields.
95-95: Hardcoded dark-mode background color.
dark:bg-[#212529]is a magic value. If this color is part of the design system, consider defining it as a theme token to keep dark-mode colors consistent and maintainable across pages.
HarshitVerma109
force-pushed
the
feat/community-page
branch
from
586075f to
4312e2c
Compare
|
@cubic-dev-ai review this pull request |
@HarshitVerma109 I have started the AI code review. It will take a few minutes to complete. |
HarshitVerma109
dismissed stale reviews from cubic-dev-ai[bot] and coderabbitai[bot]
via
13c3ecb
|
|
@arkid15r, I’ve resolved the merge conflicts. Please review the changes and let me know if any further adjustments are needed. |
|
Closing in favor of #3912 |
2 participants
Proposed change
Resolves #3862
I implemented the new Community Hub page at
/community, which serves as a central place to discover recent chapters, organizations, top contributors, and key community stats.Screen.Recording.2026-02-09.190054.1.mp4
Checklist
make check-testlocally: all warnings addressed, tests passed