Organization FE/BE implementation #1295
Conversation
Summary by CodeRabbit
WalkthroughThis pull request implements a full-stack solution to support organizations. On the backend, it adds new GraphQL nodes, queries, and an Algolia index for organizations, along with expanded indexing mixins and additional query filter capabilities using an organization parameter. On the frontend, new pages and components for displaying organization lists and details have been created, updates have been made to the search bar indexing, and navigation paths have been adjusted from "users" to "members." Numerous tests have been added and updated to ensure the new functionality performs as expected. Changes
Assessment against linked issues
Suggested labels
Suggested reviewers
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Dishant1804
changed the title
OrganizationsPage and OrganizationDetailsPage FE/BE implementation![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (13)
frontend/__tests__/unit/pages/OrganizationDetails.test.tsx (2)
48-60: Consider being more specific with loading spinner expectationsThe test verifies that loading spinners exist but doesn't assert the exact count. While using
toBeGreaterThan(0)provides some flexibility, consider asserting a specific count if the UI should consistently show a fixed number of spinners.- expect(loadingSpinner.length).toBeGreaterThan(0) + expect(loadingSpinner.length).toBe(expectedNumberOfSpinners)
31-46: Consider adding tests for navigation functionalityThe
navigateMockis set up but not tested. Consider adding a test that verifies navigation occurs correctly when relevant elements are clicked.test('navigates when organization name is clicked', async () => { render(<OrganizationDetailsPage />) await waitFor(() => { expect(screen.getByText('Test Organization')).toBeInTheDocument() }) // Assuming there's a clickable element with the organization name fireEvent.click(screen.getByText('Test Organization')) expect(navigateMock).toHaveBeenCalledWith('/expected/path') })frontend/src/components/UserCard.tsx (1)
42-47: Consider adding accessibility attributes to the members iconWhile the implementation correctly shows the members count when available, consider enhancing accessibility by adding descriptive attributes to the icon.
- <FontAwesomeIcon icon={faUsers} className="mr-1 h-4 w-4" /> + <FontAwesomeIcon + icon={faUsers} + className="mr-1 h-4 w-4" + aria-label="Members count" + title="Number of members" + />backend/tests/apps/github/graphql/nodes/organization_test.py (1)
47-82: Consider enhancing type assertions for GraphQL fieldsThe tests verify field existence and types, but some assertions use string comparisons which are fragile.
- assert str(top_contributors_field.type) == "[RepositoryContributorNode]" + from apps.github.graphql.nodes.repository import RepositoryContributorNode + assert isinstance(top_contributors_field.type, List) + assert top_contributors_field.type.of_type == RepositoryContributorNode - assert str(url_field.type) == "String" + from graphene import String + assert url_field.type == Stringfrontend/src/pages/Organization.tsx (1)
51-67: Layout implementation looks good but has a minor issue.The
SearchPageLayoutis properly configured with all necessary props. The responsive grid layout for organization cards is well-structured.However, there's a minor improvement suggested by the static analysis tool:
- {organizations && organizations.map(renderOrganizationCard)} + {organizations?.map(renderOrganizationCard)}Using optional chaining is more concise and equally safe for this use case.
🧰 Tools
🪛 Biome (1.9.4)
[error] 64-64: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
frontend/src/pages/OrganizationDetails.tsx (1)
26-39: Use optional chaining for more concise code.The conditional check can be simplified using optional chaining for better readability.
useEffect(() => { - if (data && data.organization) { + if (data?.organization) { setOrganization(data.organization) setIsLoading(false) } if (graphQLRequestError) { toaster.create({ description: 'Unable to complete the requested operation.', title: 'GraphQL Request Failed', type: 'error', }) setIsLoading(false) + setOrganization(null) // Explicitly set to null for consistency } }, [data, graphQLRequestError, organizationKey])🧰 Tools
🪛 Biome (1.9.4)
[error] 27-27: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
backend/apps/github/index/organization.py (1)
49-53: Consider adding organizational synonymsThe method is defined but doesn't add any organization-specific synonyms yet. Consider adding common synonym pairs for organizational terms (e.g., "org", "organization", "company", "foundation").
@staticmethod def update_synonyms(): """Update synonyms for the organizations index.""" - OrganizationIndex.reindex_synonyms("github", "organizations") + synonyms = [ + {"objectID": "org_synonyms", "type": "synonym", + "synonyms": ["org", "organization", "company", "foundation"]}, + {"objectID": "inc_synonyms", "type": "synonym", + "synonyms": ["inc", "incorporated", "corporation", "corp"]} + ] + OrganizationIndex.reindex_synonyms("github", "organizations", synonyms)frontend/__tests__/unit/data/mockOrganizationData.ts (1)
43-45: Consider handling potential line breaks in descriptionsThe multi-line description format used here is good for readability. Ensure that your components properly handle potential line breaks or HTML formatting in real descriptions.
backend/apps/github/graphql/nodes/organization.py (2)
58-60: Consider removing unnecessary resolver.This resolver simply returns the property value without any transformation or additional logic. Consider removing it as GraphQL can automatically resolve field values from object properties.
- def resolve_url(self, info): - """Resolve organization URL.""" - return self.url
110-138: Consider adding caching for top contributors.Computing top contributors across all repositories can be expensive for large organizations. Consider adding caching or batching this operation, especially if it's called frequently.
frontend/__tests__/e2e/pages/OrganizationDetails.spec.ts (2)
6-11: Add test cases for error states and empty data.All current tests are positive tests checking if elements are visible. Consider adding tests for error states (e.g., organization not found) and empty data scenarios (e.g., no repositories, issues, or contributors).
4-14: Consider using more specific route mocking.The test mocks all GraphQL requests with the same response. If your component makes different GraphQL queries that require different responses, consider more specific route mocking that matches specific queries.
// Example of more specific route mocking await page.route('**/graphql/', async (route) => { const body = JSON.parse(route.request().postData() || '{}'); if (body.query.includes('organization')) { await route.fulfill({ status: 200, json: { data: mockOrganizationDetailsData }, }); } else { // Handle other queries await route.continue(); } });backend/apps/github/models/mixins/organization.py (1)
39-43: Handle potential None value explicitly.While the ternary operator handles the None case for created_at, it would be clearer to use a more explicit check.
@property def idx_created_at(self): """Return created at for indexing.""" - return self.created_at.timestamp() if self.created_at else None + if self.created_at is None: + return None + return self.created_at.timestamp()
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
backend/apps/core/utils/index.py(1 hunks)backend/apps/github/graphql/nodes/organization.py(1 hunks)backend/apps/github/graphql/queries/__init__.py(2 hunks)backend/apps/github/graphql/queries/organization.py(1 hunks)backend/apps/github/index/__init__.py(1 hunks)backend/apps/github/index/organization.py(1 hunks)backend/apps/github/models/mixins/organization.py(1 hunks)backend/tests/apps/github/graphql/nodes/organization_test.py(1 hunks)backend/tests/apps/github/models/mixins/organization_test.py(1 hunks)frontend/__tests__/e2e/pages/Organization.spec.ts(1 hunks)frontend/__tests__/e2e/pages/OrganizationDetails.spec.ts(1 hunks)frontend/__tests__/unit/data/mockOrganizationData.ts(1 hunks)frontend/__tests__/unit/pages/Organization.test.tsx(1 hunks)frontend/__tests__/unit/pages/OrganizationDetails.test.tsx(1 hunks)frontend/src/App.tsx(2 hunks)frontend/src/api/queries/organizationQueries.ts(1 hunks)frontend/src/components/CardDetailsPage.tsx(3 hunks)frontend/src/components/MultiSearch.tsx(7 hunks)frontend/src/components/UserCard.tsx(2 hunks)frontend/src/pages/Home.tsx(1 hunks)frontend/src/pages/Organization.tsx(1 hunks)frontend/src/pages/OrganizationDetails.tsx(1 hunks)frontend/src/pages/index.ts(2 hunks)frontend/src/types/card.ts(1 hunks)frontend/src/types/organization.ts(1 hunks)frontend/src/types/search.ts(2 hunks)frontend/src/utils/constants.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (14)
backend/apps/github/index/__init__.py (1)
backend/apps/github/index/organization.py (1)
OrganizationIndex(8-61)
frontend/__tests__/e2e/pages/Organization.spec.ts (1)
frontend/__tests__/unit/data/mockOrganizationData.ts (1)
mockOrganizationData(1-35)
frontend/src/App.tsx (1)
frontend/src/pages/index.ts (2)
OrganizationPage(28-28)OrganizationDetailsPage(27-27)
frontend/__tests__/unit/pages/OrganizationDetails.test.tsx (2)
frontend/__tests__/unit/data/mockOrganizationData.ts (1)
mockOrganizationDetailsData(37-146)frontend/src/pages/index.ts (1)
OrganizationDetailsPage(27-27)
backend/apps/github/graphql/queries/__init__.py (1)
backend/apps/github/graphql/queries/organization.py (1)
OrganizationQuery(10-41)
backend/tests/apps/github/graphql/nodes/organization_test.py (1)
backend/apps/github/graphql/nodes/organization.py (2)
OrganizationNode(32-138)OrganizationStatsNode(22-29)
backend/tests/apps/github/models/mixins/organization_test.py (1)
backend/apps/github/models/mixins/organization.py (3)
is_indexable(8-10)idx_description(45-47)idx_collaborators_count(18-37)
frontend/src/components/UserCard.tsx (1)
frontend/src/types/card.ts (1)
UserCardProps(54-63)
frontend/src/pages/OrganizationDetails.tsx (2)
frontend/src/pages/index.ts (1)
OrganizationDetailsPage(27-27)frontend/src/api/queries/organizationQueries.ts (1)
GET_ORGANIZATION_DATA(3-63)
frontend/__tests__/unit/pages/Organization.test.tsx (1)
frontend/__tests__/unit/data/mockOrganizationData.ts (1)
mockOrganizationData(1-35)
backend/apps/github/graphql/queries/organization.py (1)
backend/apps/github/graphql/nodes/organization.py (1)
OrganizationNode(32-138)
frontend/src/types/search.ts (1)
frontend/src/types/organization.ts (1)
OrganizationTypeAlgolia(1-16)
frontend/src/components/MultiSearch.tsx (1)
frontend/src/types/organization.ts (1)
OrganizationTypeAlgolia(1-16)
frontend/__tests__/e2e/pages/OrganizationDetails.spec.ts (1)
frontend/__tests__/unit/data/mockOrganizationData.ts (1)
mockOrganizationDetailsData(37-146)
🪛 Biome (1.9.4)
frontend/src/pages/Organization.tsx
[error] 64-64: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
frontend/src/pages/OrganizationDetails.tsx
[error] 27-27: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Run frontend unit tests
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run backend tests
🔇 Additional comments (56)
frontend/src/utils/constants.ts (1)
12-12: Navigation link addition looks good!The addition of the "Organizations" link to the "Community" submenu is well-placed and follows the established pattern for navigation items. This properly integrates the new organization pages into the application's navigation structure.
frontend/__tests__/e2e/pages/Organization.spec.ts (1)
1-30: Well-structured organization page test suite.The test suite is well-organized with appropriate setup and assertions. It correctly mocks the API response for organization data and includes tests for both rendering and navigation functionality.
frontend/src/pages/Home.tsx (1)
140-140: Good addition of 'organizations' to the search indexes.Adding 'organizations' to the MultiSearchBar indexes enables users to search for organizations from the main page, which is one of the key objectives of this PR. The indexes are properly ordered alphabetically.
frontend/src/App.tsx (2)
8-9: Properly imported the new organization page components.The new organization components are correctly imported and maintain the alphabetical ordering convention.
54-55: Correctly added routes for organization pages.The routes follow the application's established pattern - a list route at
/organizationsand a details route at/organization/:organizationKeywith a parameter for the specific organization.backend/apps/core/utils/index.py (1)
118-130: Appropriate addition of organization attributes for indexing.The new case for "organizations" in the get_params_for_index function includes all the essential attributes needed for search functionality. The attributes are comprehensive and will allow for effective organization searching and display.
frontend/__tests__/unit/pages/OrganizationDetails.test.tsx (3)
1-46: Good test setup with comprehensive mocks!The test file is well-structured with properly mocked dependencies. Good use of before/after hooks, and clear separation of testing concerns.
62-80: Well-structured test with comprehensive assertionsThis test effectively verifies all the key organizational details are rendered correctly when data is available.
82-134: Tests cover all component sections thoroughlyThe tests for organization stats, issues, releases, contributors, and error handling are well-organized and complete. This comprehensive coverage ensures the component behaves correctly across various scenarios.
frontend/src/components/UserCard.tsx (2)
2-2: Good icon import for the new members functionalityThe additional FontAwesome icon import is appropriate for the new feature.
6-15: Props signature update looks goodThe component signature has been properly updated to include the new
membersproperty.frontend/src/types/search.ts (2)
3-3: Good import for the new organization typeThe import for
OrganizationTypeAlgoliais appropriate and follows the existing pattern.
15-22: Well-structured union type update for search suggestionsThe
OrganizationTypeAlgolia[]has been properly added to the union type, enabling organization search functionality.backend/tests/apps/github/graphql/nodes/organization_test.py (2)
1-46: Good test structure for OrganizationNodeThe tests are well-organized with clear descriptions and assertions to verify the inheritance hierarchy and field configuration.
84-96: Good test for OrganizationStatsNodeThe test effectively verifies that the OrganizationStatsNode has all the expected fields.
frontend/src/types/organization.ts (1)
1-16: Interface definition looks good.The
OrganizationTypeAlgoliainterface is well-structured with appropriate types and optional fields. This provides good type safety for organization data from Algolia search.One note: the timestamp fields (
created_atandupdated_at) are defined as numbers, implying Unix timestamps. This is fine, but make sure any component displaying these values handles the conversion to readable dates consistently.backend/apps/github/graphql/queries/__init__.py (1)
4-4: GraphQL organization query integration looks good.The
OrganizationQueryclass is properly imported and added to theGithubQueryinheritance list, extending the GraphQL schema with organization query capabilities. The alphabetical ordering of both imports and class inheritance is maintained, which is good practice.Also applies to: 14-14
frontend/src/pages/Organization.tsx (2)
1-7: Imports look good.All necessary imports are present for the component's functionality, including hooks, types, and UI components.
8-24: Component setup and hook usage looks good.The component properly uses the
useSearchPagehook with appropriate type parameters and configuration for organization data. The navigation hook is also correctly initialized.frontend/__tests__/unit/pages/Organization.test.tsx (4)
1-24: Test setup and mocks look good.The component imports and mocks are properly set up, including mocks for external dependencies like
fetchAlgoliaDataanduseNavigate. The pagination component mock is also well-implemented to simulate page navigation.
25-36: Test lifecycle hooks are properly implemented.The
beforeEachandafterEachhooks correctly set up the test environment and clean up after each test, which is a good practice.
37-45: Basic rendering test looks good.The test correctly verifies that organization names appear in the rendered component after data is loaded.
47-62: Pagination test is well-implemented.The test properly mocks
window.scrollToand verifies it's called with the expected parameters when changing pages. This ensures that the page scrolls to the top after navigation.frontend/src/components/CardDetailsPage.tsx (3)
78-79: LGTM! Statistics section now supports organizations.The conditional rendering logic has been appropriately extended to display statistics for organizations alongside users, projects, repositories, and committees.
124-127: LGTM! Recent Issues and Releases sections now support organizations.The conditional rendering has been properly updated to display the grid containing Recent Issues and Recent Releases for organizations, consistent with how it's handled for projects, repositories, and users.
142-147: LGTM! Repositories section now supports organizations.The code appropriately extends the Repositories section to be displayed for organizations when they have repositories, which is a logical addition since organizations are collections of repositories.
backend/apps/github/graphql/queries/organization.py (1)
1-42: LGTM! Well-structured GraphQL query implementation for organizations.The
OrganizationQueryclass is well-designed with appropriate fields for fetching a single organization by login and a list of organizations with filtering options. The resolver method includes proper error handling.frontend/src/pages/OrganizationDetails.tsx (4)
45-53: LGTM! Appropriate error handling when organization is not found.The component correctly displays a user-friendly error message when the organization doesn't exist, which provides a good user experience.
55-76: LGTM! Well-structured organization details.The organization details array is well-organized with appropriate fields including GitHub profile link, join date, followers count, and location with a fallback for missing data.
78-105: LGTM! Comprehensive statistics with appropriate icons.The statistics are well-structured with appropriate icons that visually represent each metric, and the pluralization handling for repositories is correctly implemented.
107-119: LGTM! Proper usage of DetailsCard component.The component correctly passes all necessary props to the DetailsCard component, ensuring consistent display of organization information.
frontend/src/api/queries/organizationQueries.ts (1)
3-63: LGTM! Comprehensive GraphQL query for organization data.The query is well-structured and retrieves all necessary data for displaying detailed organization information, including basic details, statistics, repositories, issues, releases, and top contributors.
backend/tests/apps/github/models/mixins/organization_test.py (5)
5-5: Good practice defining a constant for test valuesUsing a named constant
EXPECTED_CONTRIBUTOR_COUNTinstead of hardcoding the value directly in the test improves readability and maintainability.
19-30: LGTM - Comprehensive test for is_indexable propertyThe test correctly covers all possible combinations of name and login attributes, ensuring the organization is only indexable when both are present.
32-38: LGTM - Good test coverage for idx_description propertyBoth test cases (with value and empty) ensure the idx_description property correctly handles all scenarios.
40-63: Well-structured test for idx_collaborators_count with proper mockingThis test effectively uses mocking to verify both paths in the idx_collaborators_count implementation:
- When no repositories exist (returning collaborators_count)
- When repositories exist (returning count of distinct contributors)
The assertions verify both the return values and that the correct filter arguments were used.
11-11:❓ Verification inconclusive
Verify the removal of the "idx_company" test case
I notice that the parameterized test now only includes the "idx_name" attribute test case, removing what appears to have been an "idx_company" test. Make sure this removal was intentional and that you're not losing test coverage.
🏁 Script executed:
#!/bin/bash # Check if the organization model still has an idx_company property rg -A 2 -B 2 "def idx_company" backend/apps/github/models/mixins/organization.pyLength of output: 82
Action Required: Confirm Intentional Removal of "idx_company" Test Case
After running a search for any occurrences of
idx_companyin the Organization model, no definitions (e.g., adef idx_company) were found. This suggests that the removal of theidx_companytest case was likely intentional. However, please manually verify that there is indeed no dependent functionality or business logic requiring theidx_companyattribute to avoid any unintended loss of test coverage.
- Verification Points:
- Confirm that the removal of the
idx_companytest case aligns with the overall refactoring strategy.- Double-check any related documentation or recent changes that might reference the
idx_companyattribute.backend/apps/github/index/organization.py (6)
7-10: LGTM - Organization index class properly registeredThe
@register(Organization)decorator correctly associates this index class with the Organization model.
11-11: Consistent index namingThe index name "organizations" follows the naming convention used throughout the codebase for other entity types.
13-23: LGTM - Comprehensive field selection for organization indexingThe fields include all relevant organization properties needed for search and display in the UI, including metrics like followers and collaborators counts that will help with ranking results.
25-45: Well-configured search settings with appropriate ranking strategyThe search configuration:
- Sets a reasonable minimum proximity value
- Prioritizes organizations with more followers, contributors, and recent creation
- Uses standard Algolia ranking criteria
- Makes organization names and logins searchable in any order
This will provide relevant search results that prioritize popular and active organizations.
47-47: LGTM - Properly configured indexability criteriaSetting
should_index = "is_indexable"correctly uses the property tested inorganization_test.pyto determine which organizations should be indexed.
54-61: LGTM - Simple entity retrieval methodThe
get_entitiesmethod correctly retrieves all Organization objects for indexing.frontend/__tests__/unit/data/mockOrganizationData.ts (2)
1-35: LGTM - Well-structured mock organization list dataThe mock organization data provides realistic test values for all required fields and includes multiple organizations with different characteristics. This is excellent for testing the organizations list view.
37-146: LGTM - Comprehensive mock data for organization detailsThe mock data for organization details is thorough and includes:
- Basic organization information
- Statistics summary
- Repository listings with metrics
- Issue listings with authors
- Release information with linked repositories
- Top contributors with contribution counts
This mock data covers all the components needed for the organization details page.
frontend/src/components/MultiSearch.tsx (7)
8-8: LGTM - Appropriate icon selection for organizationsAdding the
faBuildingicon is a good choice to represent organizations in the UI.
18-18: LGTM - Type import for organization dataAdding the
OrganizationTypeAlgoliatype import ensures proper type checking for organization search results.
53-57: LGTM - Updated hits type to include organizationsThe type definition for
hitshas been properly updated to includeOrganizationTypeAlgolia[].
89-96: LGTM - Updated function signature to support organization typeThe
handleSuggestionClickfunction signature has been appropriately updated to includeOrganizationTypeAlgoliain the union type.
108-110: LGTM - Added navigation case for organizationsThe switch statement case for organizations correctly navigates to the organization detail page using the login as the identifier.
131-136: LGTM - Updated type cast in key handlerThe type cast in the key down handler has been properly updated to include
OrganizationTypeAlgolia.
216-217: LGTM - Added icon mapping for organizationsThe
getIconForIndexfunction now correctly returns thefaBuildingicon for organization search results.backend/apps/github/index/__init__.py (1)
4-4: Import looks good!The addition of
OrganizationIndeximport aligns with the PR objective of implementing organization-related features. This will make the organization indexing functionality available to the GitHub app.frontend/src/types/card.ts (1)
61-61: Good addition of the members property.Adding this optional property to the
UserCardPropsinterface allows the component to display organization member counts, which aligns well with the new organization pages being implemented.frontend/src/pages/index.ts (2)
11-12: Imports for new organization pages look good.These imports correctly bring in the new Organization and OrganizationDetails pages, aligning with the PR objective.
27-28: Proper export of new pages.The organization pages are correctly exported, making them available to the routing system and other parts of the application.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
backend/apps/github/graphql/nodes/organization.py (1)
91-96: Paging or limiting repository results.
Currently, the code retrieves the top 10 repositories according to stars inresolve_repositories, which is good. However, consider whether further pagination or filtering is necessary for organizations with more than 10 noteworthy repositories.frontend/src/app/organization/page.tsx (1)
64-66: Use optional chaining for safer access to organizations.Consider using optional chaining to safely access the organizations array.
- {organizations && organizations.map(renderOrganizationCard)} + {organizations?.map(renderOrganizationCard)}🧰 Tools
🪛 Biome (1.9.4)
[error] 65-65: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
frontend/src/app/organization/[organizationKey]/page.tsx (1)
27-43: Consider using optional chaining in the useEffect dependency check.The conditional check for data could be simplified using optional chaining.
- if (data && data.organization) { + if (data?.organization) {Also, good error handling with appropriate toast notifications and loading state management.
🧰 Tools
🪛 Biome (1.9.4)
[error] 28-28: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
backend/apps/github/graphql/nodes/organization.py(1 hunks)frontend/__tests__/e2e/pages/Organization.spec.ts(1 hunks)frontend/__tests__/e2e/pages/OrganizationDetails.spec.ts(1 hunks)frontend/__tests__/unit/pages/Organization.test.tsx(1 hunks)frontend/__tests__/unit/pages/OrganizationDetails.test.tsx(1 hunks)frontend/src/app/organization/[organizationKey]/page.tsx(1 hunks)frontend/src/app/organization/page.tsx(1 hunks)frontend/src/app/page.tsx(1 hunks)frontend/src/components/CardDetailsPage.tsx(3 hunks)frontend/src/components/MultiSearch.tsx(7 hunks)frontend/src/components/UserCard.tsx(2 hunks)frontend/src/server/queries/organizationQueries.ts(1 hunks)frontend/src/types/card.ts(1 hunks)frontend/src/utils/constants.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- frontend/src/utils/constants.ts
- frontend/src/types/card.ts
- frontend/src/components/UserCard.tsx
- frontend/tests/e2e/pages/Organization.spec.ts
- frontend/src/components/CardDetailsPage.tsx
- frontend/tests/unit/pages/Organization.test.tsx
- frontend/tests/unit/pages/OrganizationDetails.test.tsx
- frontend/tests/e2e/pages/OrganizationDetails.spec.ts
🧰 Additional context used
🧬 Code Definitions (4)
frontend/src/app/organization/[organizationKey]/page.tsx (3)
frontend/src/server/queries/organizationQueries.ts (1)
GET_ORGANIZATION_DATA(3-63)frontend/src/wrappers/ErrorWrapper.tsx (1)
ErrorDisplay(26-49)frontend/src/utils/dateFormatter.ts (1)
formatDate(1-16)
frontend/src/app/organization/page.tsx (2)
frontend/src/hooks/useSearchPage.ts (1)
useSearchPage(30-135)frontend/src/types/organization.ts (1)
OrganizationTypeAlgolia(1-16)
frontend/src/components/MultiSearch.tsx (4)
frontend/src/types/organization.ts (1)
OrganizationTypeAlgolia(1-16)frontend/src/types/chapter.ts (1)
ChapterTypeAlgolia(9-24)frontend/src/types/project.ts (1)
ProjectTypeAlgolia(25-46)frontend/src/types/event.ts (1)
EventType(1-10)
backend/apps/github/graphql/nodes/organization.py (7)
backend/apps/common/graphql/nodes.py (1)
BaseNode(6-10)backend/apps/github/graphql/nodes/issue.py (1)
IssueNode(7-19)backend/apps/github/graphql/nodes/release.py (1)
ReleaseNode(11-39)backend/apps/github/graphql/nodes/repository_contributor.py (1)
RepositoryContributorNode(6-14)backend/apps/github/models/organization.py (1)
Organization(12-88)backend/apps/github/models/repository.py (1)
Repository(22-332)backend/apps/github/models/repository_contributor.py (1)
RepositoryContributor(12-104)
🪛 Biome (1.9.4)
frontend/src/app/organization/[organizationKey]/page.tsx
[error] 28-28: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
frontend/src/app/organization/page.tsx
[error] 65-65: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (18)
backend/apps/github/graphql/nodes/organization.py (3)
6-16: Consider verifying permissions for private organizations.
While this GraphQL node effectively exposes organization data, verify whether any authorization checks or visibility rules are required to ensure that private or restricted organizations are not unintentionally exposed.
62-90: Great use of the Django aggregation functions for organization stats.
This implementation is a significant performance improvement over iterating Python-side for large data sets and aligns well with past review suggestions.
115-143: Efficient top contributors retrieval.
Good job leveraging theTOP_CONTRIBUTORS_LIMITto cap the query size, preventing excessive data loads for organizations with large contributor bases.frontend/src/app/page.tsx (1)
145-145: Confirm “organizations” index availability.
Including'organizations'in the search indexes is appropriate for the new organization feature. Ensure that this Algolia index is actually available and properly configured, or the search might fail.frontend/src/server/queries/organizationQueries.ts (1)
1-2: No immediate concerns with the GraphQL query definition.
All fields mesh well with the back-end schema, and the use of the$loginvariable is straightforward.frontend/src/app/organization/page.tsx (3)
9-69: Good implementation of the OrganizationPage component.The component structure follows best practices by separating concerns and reusing existing components. The
useSearchPagehook is appropriately configured for organizations, and the router navigation is implemented correctly.🧰 Tools
🪛 Biome (1.9.4)
[error] 65-65: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
26-50: Well-structured card rendering logic.The
renderOrganizationCardfunction is well-organized and handles navigation properly. The component encapsulation is clean, with a clear separation between navigation logic and presentation.
52-68: Good use of SearchPageLayout for consistent UX.Using the
SearchPageLayoutcomponent ensures consistent search experience across the application. The responsive grid layout with appropriate breakpoints is also well implemented.🧰 Tools
🪛 Biome (1.9.4)
[error] 65-65: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
frontend/src/app/organization/[organizationKey]/page.tsx (4)
19-25: Well-structured component initialization.Good use of React hooks for state management and GraphQL query setup. The component correctly extracts the organizationKey from URL parameters and sets up the appropriate initial states.
45-57: Good user experience handling for loading and error states.The component properly handles loading states with a spinner and displays a user-friendly error message when an organization isn't found. This ensures a good user experience even in edge cases.
59-109: Well-structured organization data formatting.The arrays for organization details and stats are well-structured and properly handle optional fields with fallbacks (e.g.,
organization.location || 'Not provided'). The pluralization for repositories is a nice touch for better UI text.
111-123: Clean rendering of DetailsCard with appropriate props.The component correctly passes all necessary data to the DetailsCard component, ensuring a consistent display of organization information. Setting the type to "organization" is important for the DetailsCard to render appropriately.
frontend/src/components/MultiSearch.tsx (6)
7-8: Good integration of organization search functionality.The import of the necessary icon and type for organizations is well implemented, maintaining consistency with the existing code structure.
Also applies to: 17-18
54-56: Well-integrated type update for hits array.Adding OrganizationTypeAlgolia to the hit types ensures proper type safety when handling organization search results.
88-96: Properly expanded function signature for suggestion handling.The updated function signature correctly includes OrganizationTypeAlgolia as a potential suggestion type, maintaining type safety.
108-113: Good implementation of organization navigation with type safety.The new case for organizations includes a proper type guard to safely access the login property. This ensures type safety and prevents potential runtime errors.
133-140: Consistent update to Enter key handler typecasting.The typecasting in the keydown handler has been properly updated to include OrganizationTypeAlgolia, ensuring consistency with the handleSuggestionClick function.
219-220: Well-implemented icon selection for organizations.Adding the building icon for organizations in the getIconForIndex function provides a clear visual indication of organization results in the search dropdown.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
frontend/src/app/organization/page.tsx (3)
26-50: Consider key placement and null handling for organization properties.The component key is provided in the UserCard instead of the root element of the mapped item. Also, some fields might benefit from improved null handling.
return ( + <div key={organization.objectID}> <UserCard button={SubmitButton} - key={organization.objectID} avatar={organization.avatar_url} - name={organization.name} + name={organization.name || organization.login} company={organization.company || ''} email={organization.email || ''} location={organization.location || `@${organization.login}`} members={organization.collaborators_count} className="h-64 w-80 bg-white p-6 text-left shadow-lg transition-transform duration-500 hover:scale-105 hover:shadow-xl dark:bg-gray-800 dark:shadow-gray-900/30" /> + </div> )
52-68: Consider using optional chaining for improved code readability.The code works as is, but could be improved by using optional chaining.
- {organizations && organizations.map(renderOrganizationCard)} + {organizations?.map(renderOrganizationCard)}🧰 Tools
🪛 Biome (1.9.4)
[error] 65-65: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
64-66: Consider adding a loading state and error handling for better UX.While the component passes
isLoadedto the SearchPageLayout, there isn't explicit handling of loading states or errors in the grid display itself.<div className="grid grid-cols-1 gap-6 sm:grid-cols-2 lg:grid-cols-3 xl:grid-cols-4"> - {organizations && organizations.map(renderOrganizationCard)} + {!isLoaded ? ( + <div className="col-span-full text-center">Loading organizations...</div> + ) : organizations?.length === 0 ? ( + <div className="col-span-full text-center">No organizations found</div> + ) : ( + organizations.map(renderOrganizationCard) + )} </div>🧰 Tools
🪛 Biome (1.9.4)
[error] 65-65: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/app/organization/page.tsx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
frontend/src/app/organization/page.tsx
[error] 65-65: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Run frontend unit tests
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run backend tests
- GitHub Check: CodeQL (python)
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (3)
frontend/src/app/organization/page.tsx (3)
1-8: Well-structured imports with good organization.The component properly uses 'use client' directive for Next.js client components, and has organized imports logically by grouping hooks, types, and UI components.
9-23: Good use of the custom hook for search functionality.The component effectively leverages the
useSearchPagehook with appropriate type parameters and configuration for the organization search. The clear destructuring of returned values makes the code readable.
71-72: Export is properly defined.The component is correctly exported as the default export.
There was a problem hiding this comment.
Let's start from:
- add recent PRs for org details page (extend pull_request graphql endpoint w/ organization filter)
- move issue resolver logic to existing recent issues endpoint (extend w/ filtering by org)
- move release resolver logic to existing recent releases endpoint (extend w/ filtering by org)
- move repositories resolver logic to existing repository endpoint (create a new one w/ filtering by org)
- move top contributors resolver logic to existing repository contributor endpoint (extend w/ filtering by org)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
backend/tests/apps/github/graphql/nodes/organization_test.py (1)
1-97: Good test structure for OrganizationNode and OrganizationStatsNode.The tests verify the structure, inheritance, and field configurations of both GraphQL nodes, which is essential for ensuring the schema is correctly defined.
Add tests for resolver methods
The current tests verify field types but don't test the actual resolver methods functionality. Consider adding tests that check if the resolvers return the expected data.
def test_resolve_url_returns_correct_value(self): """Test if resolve_url returns the correct URL.""" mock_organization = MagicMock() mock_organization.url = "https://github.com/test-org" node = OrganizationNode(mock_organization) result = node.resolve_url(None) assert result == "https://github.com/test-org"backend/apps/github/graphql/nodes/organization.py (1)
58-86: Optimized stats resolution with aggregation.The
resolve_statsmethod efficiently calculates organization statistics using Django's aggregation functions, which is a performant approach for handling large datasets.
🧹 Nitpick comments (4)
frontend/src/app/community/members/page.tsx (1)
27-27: URL path correctly updated from 'users' to 'members'.The navigation path update aligns with the new URL structure for accessing member profiles.
Consider renaming the component from
UsersPagetoMembersPageand updating other UI text references (like "OWASP Users" inpageTitleand "Search for OWASP users..." in the search placeholder) to maintain terminology consistency with the new URL path.frontend/__tests__/e2e/pages/Organizations.spec.ts (1)
1-38: Well-structured e2e tests for the new Organization page.The tests properly mock the API response, set up authentication, and verify both rendering and navigation functionality of the new Organizations page.
The navigation test on line 34-37 could be more specific with its assertion. Consider checking for a more specific URL pattern:
- expect(await page.url()).toContain('organizations') + expect(await page.url()).toMatch(/\/organizations\/[^\/]+$/)This would ensure navigation leads to a specific organization detail page, not just any URL containing "organizations".
frontend/src/app/organizations/[organizationKey]/page.tsx (1)
27-43: Use optional chaining for more concise data access.Consider using optional chaining for safer and more concise data access.
useEffect(() => { - if (data && data.organization) { + if (data?.organization) { setOrganization(data.organization) setIsLoading(false) } if (graphQLRequestError) { addToast({ description: 'Unable to complete the requested operation.', title: 'GraphQL Request Failed', timeout: 3000, shouldShowTimeoutProgress: true, color: 'danger', variant: 'solid', }) setIsLoading(false) } }, [data, graphQLRequestError, organizationKey])🧰 Tools
🪛 Biome (1.9.4)
[error] 28-28: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
backend/apps/github/graphql/nodes/organization.py (1)
87-142: Optimize repeated repository queries in resolver methods.Multiple resolver methods (
resolve_repositories,resolve_issues,resolve_releases, andresolve_top_contributors) all perform separate queries to fetch repositories for the organization.Consider caching the repository query in a single method or property to reduce database hits:
+ def get_repositories(self): + """Get repositories for the organization with caching.""" + if not hasattr(self, '_repositories'): + self._repositories = Repository.objects.filter(organization=self) + return self._repositories def resolve_repositories(self, info): """Resolve repositories for the organization.""" - return Repository.objects.filter(organization=self).order_by("-stars_count")[ + return self.get_repositories().order_by("-stars_count")[ :RECENT_REPOSITORIES_LIMIT ] def resolve_issues(self, info): """Resolve issues for the organization.""" - repositories = Repository.objects.filter(organization=self) + repositories = self.get_repositories() return ( Issue.objects.filter(repository__in=repositories) .select_related("repository", "author") .order_by("-created_at")[:RECENT_ISSUES_LIMIT] ) def resolve_releases(self, info): """Resolve releases for the organization.""" - repositories = Repository.objects.filter(organization=self) + repositories = self.get_repositories() return ( Release.objects.filter(repository__in=repositories) .select_related("repository", "author") .order_by("-published_at")[:RECENT_RELEASES_LIMIT] ) def resolve_top_contributors(self, info): """Resolve top contributors for the organization.""" top_contributors = ( - RepositoryContributor.objects.filter(repository__in=self.repository_set.all()) + RepositoryContributor.objects.filter(repository__in=self.get_repositories()) .values( "user__id", "user__login", "user__name", "user__avatar_url", ) .annotate(total_contributions=models.Sum("contributions_count")) .order_by("-total_contributions")[:TOP_CONTRIBUTORS_LIMIT] )This optimization will reduce the number of database queries needed for fetching the same set of repositories multiple times.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
backend/apps/common/management/commands/generate_sitemap.py(2 hunks)backend/apps/github/graphql/nodes/organization.py(1 hunks)backend/apps/slack/commands/community.py(1 hunks)backend/apps/slack/common/handlers/users.py(1 hunks)backend/tests/apps/github/graphql/nodes/organization_test.py(1 hunks)backend/tests/apps/slack/commands/community_test.py(2 hunks)frontend/__tests__/e2e/pages/About.spec.ts(1 hunks)frontend/__tests__/e2e/pages/OrganizationDetails.spec.ts(1 hunks)frontend/__tests__/e2e/pages/Organizations.spec.ts(1 hunks)frontend/__tests__/e2e/pages/UserDetails.spec.ts(1 hunks)frontend/__tests__/e2e/pages/Users.spec.tsx(3 hunks)frontend/__tests__/unit/data/mockAboutData.ts(1 hunks)frontend/__tests__/unit/pages/About.test.tsx(1 hunks)frontend/__tests__/unit/pages/Organization.test.tsx(1 hunks)frontend/__tests__/unit/pages/OrganizationDetails.test.tsx(1 hunks)frontend/__tests__/unit/pages/ProjectDetails.test.tsx(1 hunks)frontend/__tests__/unit/pages/RepositoryDetails.test.tsx(1 hunks)frontend/__tests__/unit/pages/UserDetails.test.tsx(1 hunks)frontend/__tests__/unit/pages/Users.test.tsx(2 hunks)frontend/src/app/about/page.tsx(1 hunks)frontend/src/app/community/members/page.tsx(1 hunks)frontend/src/app/organizations/[organizationKey]/page.tsx(1 hunks)frontend/src/app/organizations/page.tsx(1 hunks)frontend/src/components/ContributorAvatar.tsx(1 hunks)frontend/src/components/ItemCardList.tsx(1 hunks)frontend/src/components/LeadersList.tsx(1 hunks)frontend/src/components/MultiSearch.tsx(7 hunks)frontend/src/components/RecentReleases.tsx(1 hunks)frontend/src/components/TopContributors.tsx(1 hunks)frontend/src/components/UserCard.tsx(2 hunks)frontend/src/types/card.ts(1 hunks)frontend/src/utils/constants.ts(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- backend/apps/slack/common/handlers/users.py
- backend/apps/slack/commands/community.py
- frontend/tests/unit/pages/UserDetails.test.tsx
🚧 Files skipped from review as they are similar to previous changes (7)
- frontend/src/types/card.ts
- frontend/tests/unit/pages/Organization.test.tsx
- frontend/tests/unit/pages/OrganizationDetails.test.tsx
- frontend/src/components/UserCard.tsx
- frontend/src/utils/constants.ts
- frontend/src/components/MultiSearch.tsx
- frontend/tests/e2e/pages/OrganizationDetails.spec.ts
🧰 Additional context used
🧬 Code Graph Analysis (8)
backend/apps/common/management/commands/generate_sitemap.py (1)
frontend/src/types/user.ts (1)
user(3-17)
frontend/src/app/community/members/page.tsx (1)
frontend/src/types/user.ts (1)
user(3-17)
frontend/__tests__/e2e/pages/Organizations.spec.ts (1)
frontend/__tests__/unit/data/mockOrganizationData.ts (1)
mockOrganizationData(1-35)
backend/tests/apps/github/graphql/nodes/organization_test.py (6)
backend/apps/common/graphql/nodes.py (1)
BaseNode(6-10)backend/apps/github/graphql/nodes/issue.py (1)
IssueNode(7-19)backend/apps/github/graphql/nodes/organization.py (2)
OrganizationNode(32-141)OrganizationStatsNode(22-29)backend/apps/github/graphql/nodes/release.py (1)
ReleaseNode(11-39)backend/apps/github/graphql/nodes/repository.py (1)
RepositoryNode(15-81)backend/apps/github/models/organization.py (1)
Organization(12-88)
frontend/src/components/ContributorAvatar.tsx (1)
frontend/src/types/link.ts (1)
Link(1-6)
frontend/src/app/organizations/[organizationKey]/page.tsx (3)
frontend/src/server/queries/organizationQueries.ts (1)
GET_ORGANIZATION_DATA(3-63)frontend/src/wrappers/ErrorWrapper.tsx (1)
ErrorDisplay(26-49)frontend/src/utils/dateFormatter.ts (1)
formatDate(1-16)
frontend/src/app/organizations/page.tsx (2)
frontend/src/hooks/useSearchPage.ts (1)
useSearchPage(30-135)frontend/src/types/organization.ts (1)
OrganizationTypeAlgolia(1-16)
backend/apps/github/graphql/nodes/organization.py (7)
backend/apps/common/graphql/nodes.py (1)
BaseNode(6-10)backend/apps/github/graphql/nodes/issue.py (1)
IssueNode(7-19)backend/apps/github/graphql/nodes/release.py (1)
ReleaseNode(11-39)backend/apps/github/graphql/nodes/repository_contributor.py (1)
RepositoryContributorNode(6-14)backend/apps/github/models/organization.py (1)
Organization(12-88)backend/apps/github/models/repository.py (1)
Repository(22-332)backend/apps/github/models/repository_contributor.py (1)
RepositoryContributor(12-104)
🪛 Biome (1.9.4)
frontend/src/app/organizations/[organizationKey]/page.tsx
[error] 28-28: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
frontend/src/app/organizations/page.tsx
[error] 65-65: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run frontend unit tests
- GitHub Check: Run backend tests
🔇 Additional comments (33)
backend/tests/apps/slack/commands/community_test.py (2)
45-46: URL path updated to use "members" instead of "users".The URL has been updated to reflect the new routing structure from "users" to "members", which is consistent with the PR objectives of enhancing organization-related features.
62-62: Assertion updated to match new URL pattern.This assertion has been correctly updated to check for the new URL pattern, maintaining test integrity while reflecting the path change from "users" to "members".
backend/apps/common/management/commands/generate_sitemap.py (2)
27-27: Static route path updated from "users" to "members".The sitemap static route has been correctly updated to use the new path structure while maintaining the same change frequency and priority values.
152-157: Individual user paths updated to use "members" instead of "users".The dynamic path generation for individual user pages has been correctly updated to use the new URL structure. The change is consistent with updates in other files and properly utilizes the user's login property.
frontend/__tests__/e2e/pages/About.spec.ts (1)
85-85: URL path correctly updated from 'users' to 'members'.This change aligns with the updated URL structure for accessing member profiles throughout the application.
frontend/__tests__/e2e/pages/UserDetails.spec.ts (1)
20-20: URL path correctly updated from 'users' to 'members'.This change ensures the test navigates to the correct URL path, consistent with the updated routing structure.
frontend/src/components/ContributorAvatar.tsx (1)
51-51: URL path correctly updated from 'users' to 'members'.This change ensures contributors' profile links point to the correct URL path, consistent with the updated routing structure.
frontend/src/components/ItemCardList.tsx (1)
43-43: Path update aligns with application-wide change from "users" to "members".This change is part of a consistent update across the application to standardize the URL structure, changing user paths from
/community/users/to/community/members/. This modification supports the organization implementation by ensuring consistent navigation patterns throughout the application.frontend/src/components/RecentReleases.tsx (1)
34-34: Path update aligns with application-wide change from "users" to "members".This change is consistent with the URL structure standardization seen across the application, updating author profile links from
/community/users/to/community/members/. This ensures navigation consistency as part of the organization implementation.frontend/__tests__/unit/pages/About.test.tsx (1)
409-409: Test data URL updated to match new member path structure.The test data has been correctly updated to reflect the new URL structure, ensuring test cases remain valid with the application-wide change from
/community/users/to/community/members/.frontend/src/app/about/page.tsx (1)
175-176: Profile link in onclick handler updated to use "members" path.The profile navigation has been consistently updated from
/community/users/${username}to/community/members/${username}to align with the standardized URL structure throughout the application.frontend/src/components/LeadersList.tsx (1)
25-25: URL path updated to align with new member-centric routing structure.The change from "users" to "members" in the URL path is consistent with other changes in the codebase related to the organization implementation.
frontend/__tests__/e2e/pages/Users.spec.tsx (1)
23-23:❓ Verification inconclusive
URL paths updated correctly to the new member-centric routing structure.
The URL changes from
/community/usersto/community/membersare consistent with other changes in this PR.Verify if the test file name and message text should also be updated for consistency:
- The test file is still named
Users.spec.tsxbut tests/membersroutes- Line 39 still checks for "No Users Found" text - verify if the actual UI text has been updated
Also applies to: 38-39, 55-55
🏁 Script executed:
#!/bin/bash # Check if the UI text has been updated to "No Members Found" rg -i "No Members Found" --type=tsx --type=tsLength of output: 75
Verify UI Text and File Naming Consistency
The URL path changes to
/community/membersare correctly applied, but please verify the following manually:
- The test file is still named
Users.spec.tsxeven though the tested route is now/members.- The test still checks for "No Users Found" text (e.g., on line 39) – confirm if the UI has been updated to "No Members Found".
The above script is intended to check for any occurrences of "No Members Found" in
.tsxfiles. If no output is found, please manually verify if this discrepancy is intentional or if the test text and/or file name require updating.
🏁 Script executed:
#!/bin/bash # Re-run the search using a custom tsx file type so that .tsx files are included rg --type-add 'tsx:*.tsx' --type tsx -i "No Members Found" .Length of output: 184
Manual Verification Needed for UI Consistency
The URL changes to
/community/membersare correct, but there are two potential inconsistencies that need manual verification:
- The test file is still named
Users.spec.tsxeven though it now tests member-related routes.- The test still checks for "No Users Found" text (likely on or around line 39). Please verify if the UI text has been updated to "No Members Found" or if the test text should remain unchanged.
Please verify these points to ensure consistency across the codebase.
frontend/__tests__/unit/data/mockAboutData.ts (1)
29-29: Mock data URLs updated correctly to match the new routing structure.The changes consistently update all user URLs from
/community/users/{username}to/community/members/{username}in the mock data, which aligns with the other changes in this PR.Also applies to: 35-35, 41-41
frontend/src/components/TopContributors.tsx (1)
43-43: Navigation path updated to align with new member-centric routing structure.The change from "users" to "members" in the onClick navigation handler is consistent with the other routing changes across the codebase related to the organization implementation.
frontend/__tests__/unit/pages/RepositoryDetails.test.tsx (1)
135-135: URL path updated correctly.The updated path reflects the change from "users" to "members" in the URL routing structure, which is consistent with the organizational path changes mentioned in the PR objectives. This ensures tests pass after the URL structure update.
frontend/__tests__/unit/pages/ProjectDetails.test.tsx (1)
133-133: URL path updated correctly.The path has been updated from
/community/users/contributor1to/community/members/contributor1, aligning with the consistent URL structure changes throughout the application.frontend/__tests__/unit/pages/Users.test.tsx (2)
6-6: Path update correctly migrates from 'users' to 'members'.The import path has been updated to reflect the new routing structure, which aligns with the organization-related changes mentioned in the PR objectives.
133-133: Test assertion properly updated for new URL structure.The navigation test has been correctly updated to verify routing to the new path structure (
/community/members/user_1instead of/community/users/user_1), maintaining consistency with the import path changes.frontend/src/app/organizations/page.tsx (5)
1-8: LGTM: Component imports and client directive look good.The necessary imports are properly configured for the organization page component. The 'use client' directive is correctly placed at the top of the file to indicate client-side rendering.
9-23: Well-implemented search hook integration.The component properly utilizes the
useSearchPagehook with appropriate parameters for organization search. The implementation correctly handles pagination, search queries, and loading states.
26-50: Organization card rendering is well-structured.The
renderOrganizationCardfunction provides a clean implementation for displaying organization information. The navigation to organization details pages is correctly implemented using the router.
52-68: Consider adding null check for organizations array.The code is generally well-structured, but there's a potential issue when mapping over the organizations array.
- {organizations && organizations.map(renderOrganizationCard)} + {organizations?.map(renderOrganizationCard)}This change would use the optional chaining operator to safely access the
mapmethod, preventing errors iforganizationsis null.🧰 Tools
🪛 Biome (1.9.4)
[error] 65-65: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
71-71: Properly exported component.The component is correctly exported as the default export.
frontend/src/app/organizations/[organizationKey]/page.tsx (6)
1-18: LGTM: Imports and client directive properly set up.All necessary imports are correctly included for data fetching, UI components, and utilities. The 'use client' directive is appropriately placed at the top of the file.
19-26: Well-implemented query setup for organization data.The component correctly sets up the GraphQL query with the organization key from URL parameters. The local state is properly initialized for organization data and loading status.
45-57: Well-handled loading and error states.The component properly displays a loading spinner while data is being fetched and an appropriate error message if the organization is not found. This provides a good user experience during data loading and error scenarios.
59-81: Well-structured organization details formatting.The organization details are well-organized into a structured array for rendering. The use of the
formatDateutility function for date formatting is appropriate, and the optional rendering for location is handled correctly.
82-110: Comprehensive statistics display with appropriate icons.The statistics are well-organized with appropriate icons and units. The implementation handles pluralization properly with the
pluralizedNameproperty where needed.
111-126: Clean implementation of the details card rendering.The
DetailsCardcomponent is properly used with all necessary props passed correctly. The type is set to "organization" which allows the component to render organization-specific sections.backend/apps/github/graphql/nodes/organization.py (3)
1-21: LGTM: Clear imports and constants.The imports are organized well, and the constants for limiting results (RECENT_ISSUES_LIMIT, RECENT_RELEASES_LIMIT, RECENT_REPOSITORIES_LIMIT) are defined with appropriate values.
22-30: Well-defined organization stats node.The
OrganizationStatsNodeclass properly defines the fields needed for organization statistics. The use of GraphQL scalar types is appropriate for each field.
32-57: Well-structured organization node with comprehensive fields.The
OrganizationNodeclass correctly extendsBaseNodeand includes all necessary fields for organization data. The Meta class properly specifies the model and fields to expose through GraphQL.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
frontend/src/server/queries/organizationQueries.ts (1)
3-71: Consider adding pagination or cursors for top contributors.
You limit the number of pull requests, releases, organization repositories, and issues, but there's no limit or pagination mechanism fortopContributors. For large organizations, returning all contributors in one shot may degrade performance and slow down the UI.
🧹 Nitpick comments (4)
frontend/__tests__/unit/data/mockOrganizationData.ts (1)
1-35: Enhance test coverage with edge cases.
These datasets provide typical scenarios. Consider adding edge cases, e.g., an organization with no repositories, zero contributors, or missing fields, to verify the behavior under exceptional conditions.Also applies to: 37-164
backend/apps/github/graphql/queries/repository.py (1)
41-56: Consider adding error handling for non-existent organizationsThe resolver method correctly filters repositories by organization login and orders them by popularity (stars count). However, unlike
resolve_repository, there's no error handling if the organization doesn't exist.def resolve_organization_repositories(root, info, organization, limit): """Resolve repositories for an organization. Args: root (Any): The root query object. info (ResolveInfo): The GraphQL execution context. organization (str): The login of the organization. limit (int): Maximum number of repositories to return. Returns: QuerySet: Queryset containing the repositories for the organization. """ + # Check if organization exists first + from apps.github.models.organization import Organization + try: + org = Organization.objects.get(login=organization) + except Organization.DoesNotExist: + return [] + return Repository.objects.filter(organization__login=organization).order_by( "-stars_count" )[:limit]backend/apps/github/graphql/queries/repository_contributor.py (2)
35-107: Consider refactoring to reduce code duplication in node construction.There's duplication in the node construction logic between the organization-filtered and non-organization-filtered paths. Consider extracting this into a helper function to improve maintainability.
+def _create_contributor_node(contributor_data, with_project=True): + """Create a contributor node from query data. + + Args: + contributor_data (dict): Dictionary with contributor data. + with_project (bool): Whether to include project data. + + Returns: + RepositoryContributorNode: The constructed node. + """ + node = RepositoryContributorNode( + avatar_url=contributor_data["user__avatar_url"], + contributions_count=contributor_data.get("total_contributions", + contributor_data.get("contributions_count")), + login=contributor_data["user__login"], + name=contributor_data["user__name"], + ) + + if with_project and "project_name" in contributor_data: + node.project_name = contributor_data["project_name"] + node.project_url = contributor_data["project_url"] + + return node def resolve_top_contributors(root, info, limit, organization=None): # ... existing code ... if organization: # ... existing organization query code ... - return [ - RepositoryContributorNode( - avatar_url=trc["user__avatar_url"], - contributions_count=trc["total_contributions"], - login=trc["user__login"], - name=trc["user__name"], - ) - for trc in top_contributors - ] + return [_create_contributor_node(trc, with_project=False) for trc in top_contributors] # ... existing non-organization query code ... - return [ - RepositoryContributorNode( - avatar_url=trc["user__avatar_url"], - contributions_count=trc["contributions_count"], - login=trc["user__login"], - name=trc["user__name"], - project_name=trc["project_name"], - project_url=trc["project_url"], - ) - for trc in top_contributors - ] + return [_create_contributor_node(trc) for trc in top_contributors]
41-51:❓ Verification inconclusive
Consider adding pagination for better performance with large organizations.
For large organizations with many contributors, the aggregation might be expensive. Consider implementing pagination instead of just using a limit, to allow clients to fetch results in batches.
Run the following script to check if there are organizations with a large number of contributors:
🏁 Script executed:
#!/bin/bash # Check if there are any organizations with a high number of contributors echo "Checking for large organizations with many contributors..." # Count contributors per organization python -c " import django django.setup() from apps.github.models.repository_contributor import RepositoryContributor from django.db.models import Count # Get organizations with more than 50 contributors large_orgs = RepositoryContributor.objects.values('repository__organization__login').annotate( contributor_count=Count('user', distinct=True) ).filter(contributor_count__gt=50).order_by('-contributor_count') if large_orgs: print('Found organizations with many contributors:') for org in large_orgs: print(f'- {org[\"repository__organization__login\"]}: {org[\"contributor_count\"]} contributors') else: print('No organizations with more than 50 contributors found.') "Length of output: 978
Revisit Pagination for Scalability in Contributor Aggregation
It looks like the verification script couldn’t run due to a missing Django dependency, so we couldn’t conclusively determine if many organizations indeed require pagination. However, if production data reveals organizations with a high number of contributors, the current aggregation logic (lines 41–51 in
backend/apps/github/graphql/queries/repository_contributor.py) may suffer performance issues. Please verify manually in a properly configured Django environment if large contributor counts occur, and if so, consider updating the logic to use pagination for more efficient data retrieval.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
backend/apps/github/graphql/nodes/organization.py(1 hunks)backend/apps/github/graphql/queries/issue.py(2 hunks)backend/apps/github/graphql/queries/pull_request.py(2 hunks)backend/apps/github/graphql/queries/release.py(3 hunks)backend/apps/github/graphql/queries/repository.py(2 hunks)backend/apps/github/graphql/queries/repository_contributor.py(2 hunks)backend/tests/apps/github/graphql/nodes/organization_test.py(1 hunks)frontend/__tests__/e2e/pages/OrganizationDetails.spec.ts(1 hunks)frontend/__tests__/unit/data/mockOrganizationData.ts(1 hunks)frontend/__tests__/unit/pages/OrganizationDetails.test.tsx(1 hunks)frontend/src/app/organizations/[organizationKey]/page.tsx(1 hunks)frontend/src/components/CardDetailsPage.tsx(3 hunks)frontend/src/components/LeadersList.tsx(1 hunks)frontend/src/components/RecentReleases.tsx(1 hunks)frontend/src/server/queries/organizationQueries.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- frontend/tests/unit/pages/OrganizationDetails.test.tsx
- frontend/src/components/LeadersList.tsx
- frontend/src/components/RecentReleases.tsx
- frontend/src/components/CardDetailsPage.tsx
- backend/tests/apps/github/graphql/nodes/organization_test.py
- frontend/tests/e2e/pages/OrganizationDetails.spec.ts
🧰 Additional context used
🧬 Code Graph Analysis (5)
backend/apps/github/graphql/queries/repository.py (2)
backend/apps/github/graphql/nodes/repository.py (1)
RepositoryNode(15-81)backend/apps/github/models/repository.py (1)
Repository(22-332)
backend/apps/github/graphql/queries/repository_contributor.py (3)
backend/apps/github/graphql/nodes/repository_contributor.py (1)
RepositoryContributorNode(6-14)backend/apps/github/models/repository_contributor.py (1)
RepositoryContributor(12-104)backend/apps/github/models/managers/repository_contributor.py (4)
by_humans(12-16)by_humans(36-38)to_community_repositories(18-20)to_community_repositories(40-42)
backend/apps/github/graphql/nodes/organization.py (4)
backend/apps/common/graphql/nodes.py (1)
BaseNode(6-10)backend/apps/github/models/organization.py (1)
Organization(12-88)backend/apps/github/models/repository.py (1)
Repository(22-332)backend/apps/github/models/repository_contributor.py (1)
RepositoryContributor(12-104)
backend/apps/github/graphql/queries/pull_request.py (1)
backend/apps/github/models/pull_request.py (1)
PullRequest(11-125)
backend/apps/github/graphql/queries/issue.py (2)
backend/apps/owasp/graphql/nodes/project.py (1)
resolve_recent_issues(56-58)frontend/src/types/user.ts (1)
Issue(24-31)
⏰ Context from checks skipped due to timeout of 90000ms (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 (24)
backend/apps/github/graphql/nodes/organization.py (5)
1-20: Comprehensive definition of stats fields and clean imports.
All fields inOrganizationStatsNodeare clearly defined and the aggregator approach for stats is structurally consistent. No immediate issues are identified in these lines.
22-27: Approach to defining organization node fields looks good.
TheOrganizationNodesets up a clear GraphQL schema, withstatsandurlaccessible. This is in keeping with Graphene conventions.
28-42: Confirm exposure of email and other sensitive fields.
Including fields likeWould you like to review how these fields are accessed or implement any access restrictions or masking?
44-71: Efficient stats calculation matches recommended aggregator usage.
This resolves the prior review comment regarding improved database queries. The aggregate sums for stars, forks, and issues, along with distinct user count, are optimal for large data sets.
73-75:❓ Verification inconclusive
Verify underlying
urlproperty on the Organization model.
resolve_urlreturnsself.url, but theOrganizationmodel snippet does not explicitly list aurlfield or property. Ensure this is defined or inherited from a parent model so that it does not raise errors at runtime.
🏁 Script executed:
#!/bin/bash # Verify that the Organization model has a `url` property or # at least a fallback to a related field. rg --multiline --context 5 --case-sensitive 'class Organization'Length of output: 6718
Action: Confirm that the Organization model provides a valid
urlattributeThe
resolve_urlmethod inbackend/apps/github/graphql/nodes/organization.pyreturnsself.url. However, our search in the Organization model (i.e.backend/apps/github/models/organization.py) does not show an explicit definition for aurlproperty. Please verify that one of the inherited base classes (such asGenericUserModel,NodeModel, orOrganizationIndexMixin) supplies this attribute. If none of those or a fallback mechanism provides it, then an explicit definition should be added to the Organization model to prevent runtime errors.backend/apps/github/graphql/queries/repository.py (1)
18-22: Looks good: Added new GraphQL field for organization repositoriesThe new field
organization_repositorieshas been correctly defined to retrieve a list of repositories for a specific organization with a sensible default limit of 12.backend/apps/github/graphql/queries/issue.py (5)
19-19: Looks good: Added organization parameter to recent_issuesThe organization parameter has been correctly defined as an optional string parameter.
22-22: Looks good: Updated resolver method signatureThe resolver method signature has been properly updated to include the new organization parameter.
31-31: Looks good: Updated method documentationDocumentation has been properly updated to describe the new organization parameter.
39-39: Good optimization: Added repository to select_relatedAdding repository to the select_related call is a good optimization to reduce database queries when filtering by organization.
47-49: Looks good: Implemented organization filterThe filter correctly filters issues by the organization's login when the parameter is provided.
backend/apps/github/graphql/queries/release.py (4)
19-19: Looks good: Added organization parameter to recent_releasesThe organization parameter has been correctly defined as an optional string parameter.
22-22: Looks good: Updated resolver method signatureThe resolver method signature has been properly updated to include the new organization parameter.
31-31: Looks good: Updated method documentationDocumentation has been properly updated to describe the new organization parameter.
48-52: Looks good: Implemented organization filterThe filter correctly filters releases by the organization's login when the parameter is provided. The code formatting with proper indentation makes it consistent with the existing login filter above.
backend/apps/github/graphql/queries/pull_request.py (5)
19-19: Looks good: Added organization parameter to recent_pull_requestsThe organization parameter has been correctly defined as an optional string parameter.
22-24: Looks good: Updated resolver method signatureThe resolver method signature has been properly updated to include the new organization parameter.
33-33: Looks good: Updated method documentationDocumentation has been properly updated to describe the new organization parameter.
41-41: Good optimization: Added repository to select_relatedAdding repository to the select_related call is a good optimization to reduce database queries when filtering by organization.
49-51: Looks good: Implemented organization filterThe filter correctly filters pull requests by the organization's login when the parameter is provided.
backend/apps/github/graphql/queries/repository_contributor.py (4)
4-4: Good addition of the Sum import for the new aggregation logic.The addition of the
Sumimport is necessary and properly used for aggregating the total contributions in the organization filter path.
16-20: GraphQL field definition correctly extended with optional organization parameter.The addition of the optional organization parameter to the
top_contributorsfield is well-implemented:
- Made it non-required with
required=Falseto maintain backward compatibility- Kept the existing
limitparameter functionality- Parameter naming is clear and follows the API's conventions
22-34: Method signature and documentation properly updated.The resolver method signature has been correctly updated to include the new optional organization parameter, and the documentation has been updated to reflect this change. This is good practice for maintaining code readability and API documentation.
63-95: Good separation of concerns for organization and non-organization queries.The code clearly separates the organization-specific logic from the original query path, making it easier to understand and maintain. The comment at line 63 helps clarify the purpose of this section.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
frontend/src/server/queries/organizationQueries.ts (1)
3-72: GraphQL query structure looks good, but consider implementing paginationThe query is well-structured and fetches comprehensive organization data including basic information, statistics, contributors, repositories, and activity (PRs, issues, releases). The use of
limitparameters for recent activities is a good start.However, this query still lacks proper pagination mechanisms for repositories, issues, and releases. While you're using
limitparameters, there are no pagination cursors or offsets to load additional data beyond the initial set.Consider enhancing this query by:
- Adding cursor-based pagination arguments (using the GraphQL
afterpattern)- Implementing separate queries for different sections that can be loaded on demand
- Including total counts for paginated collections
export const GET_ORGANIZATION_DATA = gql` query GetOrganization($login: String!) { organization(login: $login) { # Basic info remains the same } topContributors(organization: $login) { # This section remains the same } - recentPullRequests(limit: 5, organization: $login) { + recentPullRequests(limit: 5, organization: $login, after: $pullRequestCursor) { title createdAt url author { login avatarUrl } + pageInfo { + hasNextPage + endCursor + } } # Apply similar pattern to other paginated sections } ` + export const GET_ORGANIZATION_REPOSITORIES = gql` + query GetOrganizationRepositories($login: String!, $limit: Int, $cursor: String) { + organizationRepositories(organization: $login, limit: $limit, after: $cursor) { + nodes { + name + url + contributorsCount + forksCount + openIssuesCount + starsCount + key + } + pageInfo { + hasNextPage + endCursor + } + } + } + `
🧹 Nitpick comments (2)
frontend/src/server/queries/organizationQueries.ts (2)
52-60: Consider adding more repository metadataThe repository data fetched is missing some potentially useful fields that could enhance the UI display and user experience.
Consider adding these additional fields:
description: Provides context about the repository's purposeprimaryLanguage: Shows the main technology usedvisibility: Whether the repo is public or privateupdatedAt: Last updated timestamporganizationRepositories(organization: $login, limit: 12) { name url contributorsCount forksCount openIssuesCount starsCount key + description + primaryLanguage { + name + color + } + visibility + updatedAt }
18-24: Consider expanding organization statisticsThe current stats are good, but consider adding more metrics that would be valuable for organization overview.
stats { totalRepositories totalContributors totalStars totalForks totalIssues + totalPullRequests + totalCommits + avgTimeToMerge + topLanguages { + name + color + percentage + } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/apps/github/graphql/queries/repository_contributor.py(2 hunks)frontend/src/server/queries/organizationQueries.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/apps/github/graphql/queries/repository_contributor.py
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Run frontend unit tests
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run backend tests
- GitHub Check: CodeQL (python)
- GitHub Check: CodeQL (javascript-typescript)
Hey @arkid15r these changes are implemented and ready for review :) |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
backend/apps/github/graphql/queries/organization.py (1)
30-33: Consider adding logging for non-existent organizations.While the error handling is appropriate, adding a log when an organization is not found would make it easier to track these lookup failures in production.
try: return Organization.objects.get(login=login) except Organization.DoesNotExist: + # Consider logging this failure with logger.debug or logger.info return Nonefrontend/src/app/organizations/page.tsx (3)
64-66: Improve code safety with optional chainingReplace the logical AND operator with optional chaining for a more concise and idiomatic approach.
- {organizations && organizations.map(renderOrganizationCard)} + {organizations?.map(renderOrganizationCard)}🧰 Tools
🪛 Biome (1.9.4)
[error] 65-65: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
45-46: Revisit location fallback logicUsing
@${organization.login}as a fallback for location seems unintuitive since location typically represents a physical place. Consider separating the login information or using a more descriptive fallback.- location={organization.location || `@${organization.login}`} + location={organization.location || ''} + username={organization.login}Note: This assumes the
UserCardcomponent supports ausernameprop. If not, you might need to update that component as well.
47-47: Extract Tailwind classes to a reusable constantThe long string of Tailwind classes reduces readability. Consider extracting this into a constant at the top of the component for better maintainability.
const OrganizationPage = () => { + const cardClassName = "h-64 w-80 bg-white p-6 text-left shadow-lg transition-transform duration-500 hover:scale-105 hover:shadow-xl dark:bg-gray-800 dark:shadow-gray-900/30" const { // ...Then update the component:
- className="h-64 w-80 bg-white p-6 text-left shadow-lg transition-transform duration-500 hover:scale-105 hover:shadow-xl dark:bg-gray-800 dark:shadow-gray-900/30" + className={cardClassName}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
backend/apps/github/graphql/queries/issue.py(3 hunks)backend/apps/github/graphql/queries/organization.py(1 hunks)backend/apps/github/graphql/queries/pull_request.py(3 hunks)backend/apps/github/graphql/queries/release.py(3 hunks)backend/apps/github/graphql/queries/repository.py(2 hunks)backend/apps/github/graphql/queries/repository_contributor.py(1 hunks)frontend/__tests__/unit/pages/Chapters.test.tsx(1 hunks)frontend/__tests__/unit/pages/Committees.test.tsx(1 hunks)frontend/__tests__/unit/pages/Contribute.test.tsx(1 hunks)frontend/__tests__/unit/pages/Projects.test.tsx(1 hunks)frontend/__tests__/unit/pages/Users.test.tsx(3 hunks)frontend/src/app/chapters/page.tsx(1 hunks)frontend/src/app/committees/page.tsx(1 hunks)frontend/src/app/community/members/page.tsx(2 hunks)frontend/src/app/organizations/[organizationKey]/page.tsx(1 hunks)frontend/src/app/organizations/page.tsx(1 hunks)frontend/src/app/projects/contribute/page.tsx(1 hunks)frontend/src/app/projects/page.tsx(1 hunks)frontend/src/components/RecentReleases.tsx(3 hunks)frontend/src/server/queries/organizationQueries.ts(1 hunks)frontend/src/types/card.ts(1 hunks)frontend/src/types/organization.ts(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- frontend/src/app/chapters/page.tsx
- frontend/src/app/projects/contribute/page.tsx
- frontend/src/app/committees/page.tsx
- frontend/src/app/projects/page.tsx
- frontend/tests/unit/pages/Contribute.test.tsx
🚧 Files skipped from review as they are similar to previous changes (11)
- frontend/src/types/card.ts
- frontend/src/components/RecentReleases.tsx
- backend/apps/github/graphql/queries/release.py
- frontend/src/app/community/members/page.tsx
- backend/apps/github/graphql/queries/pull_request.py
- frontend/src/server/queries/organizationQueries.ts
- frontend/src/types/organization.ts
- frontend/tests/unit/pages/Users.test.tsx
- frontend/src/app/organizations/[organizationKey]/page.tsx
- backend/apps/github/graphql/queries/repository_contributor.py
- backend/apps/github/graphql/queries/repository.py
🧰 Additional context used
🧬 Code Graph Analysis (2)
backend/apps/github/graphql/queries/organization.py (3)
backend/apps/common/graphql/queries.py (1)
BaseQuery(6-7)backend/apps/github/graphql/nodes/organization.py (1)
OrganizationNode(22-75)backend/apps/github/models/organization.py (1)
Organization(12-88)
backend/apps/github/graphql/queries/issue.py (1)
backend/apps/owasp/graphql/nodes/project.py (1)
resolve_recent_issues(56-58)
🪛 Biome (1.9.4)
frontend/src/app/organizations/page.tsx
[error] 65-65: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (10)
frontend/__tests__/unit/pages/Committees.test.tsx (1)
74-75: Placeholder text update aligns with UI standardization.This change correctly updates the test to match the new placeholder text in the Committees search bar, which is part of a broader effort to standardize placeholder texts across the application by removing specific references to "OWASP".
frontend/__tests__/unit/pages/Chapters.test.tsx (1)
112-112: Placeholder text updated to be more generic.The search placeholder has been updated from "Search for OWASP chapters..." to "Search for chapters...", which aligns with the broader updates in this PR to make search functionality more general and consistent across the application.
frontend/__tests__/unit/pages/Projects.test.tsx (1)
82-82: Placeholder text updated to be more generic.The search placeholder has been changed from "Search for OWASP projects..." to "Search for projects...", following the same pattern of generalizing search placeholders across the application. This is consistent with the other similar changes for chapters and matches the implementation of organization search functionality.
backend/apps/github/graphql/queries/organization.py (1)
1-34: Implementation looks good with clear organization query support.This new file provides a well-structured implementation of GraphQL queries for GitHub organizations. The code follows project conventions with clear docstrings and appropriate error handling. The
resolve_organizationmethod efficiently handles the lookup of an organization by its login and gracefully handles the case where the organization doesn't exist.backend/apps/github/graphql/queries/issue.py (4)
19-19: LGTM: Organization parameter properly added to GraphQL query.The addition of the organization parameter to filter issues by an organization's login is well-implemented.
22-22: Method signature correctly updated with organization parameter.The resolver method signature has been properly updated to include the new optional organization parameter with a default value of None.
31-31: Docstring properly updated with organization parameter description.Documentation has been updated to include the new parameter, maintaining code quality standards.
43-54: Filtering logic implemented well with performance considerations.The implementation efficiently filters issues by organization using appropriate select_related to optimize queries. The select_related for repository__organization is correctly placed inside the organization conditional block, minimizing unnecessary joins when not filtering by organization.
frontend/src/app/organizations/page.tsx (2)
38-48: Reusing UserCard for organizations may require reviewThe component reuses
UserCardfor displaying organizations, which might not be semantically accurate. Consider either:
- Renaming
UserCardto a more generic name likeProfileCardthroughout the application to accommodate both users and organizations- Creating a dedicated
OrganizationCardcomponent if organizations require different fields or behaviorsWhile this works functionally, it might cause confusion for future developers. Please verify that all
UserCardprops are appropriate for organizations.
9-69: LGTM - Well-structured componentThe
OrganizationPagecomponent is well-structured and correctly implements the organization listing functionality as described in the PR objectives. Good job using:
- The
useSearchPagehook for search and pagination- Proper component composition
- Clean separation of rendering logic
- Responsive grid layout
🧰 Tools
🪛 Biome (1.9.4)
[error] 65-65: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
|
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
frontend/__tests__/e2e/components/Header.spec.ts (1)
44-60: 🛠️ Refactor suggestionAdd test for the Organizations link in desktop view.
I notice that you've added an assertion for the 'Organizations' link in the mobile view tests, but there isn't a corresponding assertion in the desktop view tests. For complete test coverage, you should add a test for the Organizations link in the desktop dropdown as well.
// Assert dropdown options appear await expect(navbar.getByRole('link', { name: 'Chapters' })).toBeVisible() await expect(navbar.getByRole('link', { name: 'Members' })).toBeVisible() +await expect(navbar.getByRole('link', { name: 'Organizations' })).toBeVisible() await expect(navbar.getByRole('link', { name: 'Snapshots' })).toBeVisible()
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/__tests__/e2e/components/Header.spec.ts(2 hunks)frontend/__tests__/e2e/pages/Users.spec.ts(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run backend tests
- GitHub Check: Run frontend unit tests
🔇 Additional comments (1)
frontend/__tests__/e2e/components/Header.spec.ts (1)
58-58: LGTM - Navigation updates for members and organizations.The Header component tests have been properly updated to reflect the navigation changes. You've successfully added assertions for the new 'Members' link in both desktop and mobile views, as well as the 'Organizations' link in mobile view.
Also applies to: 109-112
| @@ -20,7 +20,7 @@ test.describe('Users Page', () => { | |||
| path: '/', | |||
| }, | |||
| ]) | |||
| await page.goto('/community/users') | |||
| await page.goto('/community/members') | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update file name to reflect the new "members" terminology.
The test file is correctly updated to use the new path /community/members instead of /community/users, but the file itself is still named Users.spec.ts. For consistency across the codebase, consider renaming this file to Members.spec.ts.
Additionally, the test descriptions and assertions still use "Users" terminology (e.g., 'Users Page', 'No Users Found') while testing "members" routes.
-test.describe('Users Page', () => {
+test.describe('Members Page', () => {And:
-await expect(page.getByText('No Users Found')).toBeVisible()
+await expect(page.getByText('No Members Found')).toBeVisible()This would ensure full terminology consistency across both routes and user-facing messages.
Also applies to: 38-38, 55-55
* OragnizationPage and OrganizationDetailsPage fe/be * checks and lint * removed unused field * migrated organization to nextjs components * checks and lint * adding member check for reliability * improved usercard * Update code * using resolvers with filtering * code rabbit suggestions * Update code * What conflict? * Update tests * Update code --------- Co-authored-by: Arkadii Yakovets <[email protected]>
* OragnizationPage and OrganizationDetailsPage fe/be * checks and lint * removed unused field * migrated organization to nextjs components * checks and lint * adding member check for reliability * improved usercard * Update code * using resolvers with filtering * code rabbit suggestions * Update code * What conflict? * Update tests * Update code --------- Co-authored-by: Arkadii Yakovets <[email protected]>
Resolves #1231
2025-04-07.13-29-22.mp4