-
-
Notifications
You must be signed in to change notification settings - Fork 864
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor Admin Login: MongoDB → PostgreSQL #3477
Refactor Admin Login: MongoDB → PostgreSQL #3477
Conversation
WalkthroughThis pull request introduces significant modifications to the GraphQL schema and user data handling across multiple components. The changes primarily focus on restructuring user and organization data types, updating role management, and modifying how user information is stored and retrieved. The modifications span the GraphQL schema, various React components, and utility interfaces, with a particular emphasis on enhancing user role and organization data representation. Changes
Assessment against linked issues
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
|
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
🧹 Nitpick comments (15)
src/screens/OrgList/OrgList.tsx (6)
22-24
: Fix missing trailing comma.
ESLint indicates a missing trailing comma in the import statement.Applying this diff aligns with Prettier formatting:
import type { - InterfaceCurrentUserTypePG, - InterfaceOrgConnectionInfoType + InterfaceCurrentUserTypePG, + InterfaceOrgConnectionInfoType, InterfaceOrgConnectionTypePG } from 'utils/interfaces';🧰 Tools
🪛 ESLint
[error] 24-24: Insert
,
(prettier/prettier)
🪛 GitHub Actions: PR Workflow
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
130-146
: Consider removing commented legacy code.
Leaving large commented blocks can create confusion. Remove or refactor them if you no longer plan to reuse theORGANIZATION_CONNECTION_LIST
approach.🧰 Tools
🪛 GitHub Actions: PR Workflow
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
305-308
: Fix commented-out code and format updateQuery properly.
- The
isLoadingMore
code is commented out; remove or uncomment if needed.- Prettier wants multiline definitions for
prev
andfetchMoreResult
.- // if (isLoadingMore || !hasMore) setIsLoadingMore(true); fetchMore({ variables: { - skip: orgsData?.edges?.length || 0, + skip: orgsData?.edges?.length ?? 0, }, updateQuery: ( - prev: { organizationsConnection: InterfaceOrgConnectionTypePG[] } | undefined, - { fetchMoreResult }: { fetchMoreResult: { organizationsConnection: InterfaceOrgConnectionTypePG[] } | undefined }, - ): { organizationsConnection: InterfaceOrgConnectionTypePG[] } | undefined => { + prev: { + organizationsConnection: InterfaceOrgConnectionTypePG[]; + } | undefined, + { + fetchMoreResult, + }: { + fetchMoreResult: { + organizationsConnection: InterfaceOrgConnectionTypePG[]; + } | undefined; + }, + ): { + organizationsConnection: InterfaceOrgConnectionTypePG[]; + } | undefined => { setIsLoadingMore(false); ...Also applies to: 311-317
🧰 Tools
🪛 GitHub Actions: PR Workflow
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
Line range hint
371-380
: Remove superfluous commented code for SuperAdmin creation button.
If you plan to restore the creation button soon, keep a TODO; otherwise, consider removing.🧰 Tools
🪛 GitHub Actions: PR Workflow
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
387-389
: Use strict equality for clarity.
When comparing lengths to zero, prefer=== 0
.- (!orgsData?.edges || orgsData.edges.length === 0) && + (!orgsData?.edges || orgsData.edges.length === 0) &&- orgsData?.edges.length == 0 && + orgsData?.edges.length === 0 &&Also applies to: 395-395
🧰 Tools
🪛 GitHub Actions: PR Workflow
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
Line range hint
497-507
: Remove or restore commentedOrganizationModal
.
It’s unclear if you plan to bring it back; consider removing if no longer required.🧰 Tools
🪛 GitHub Actions: PR Workflow
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
src/components/OrgListCard/OrgListCard.spec.tsx (1)
52-63
: Fix formatting issues to match Prettier standards.Apply these formatting changes:
const props: InterfaceOrgListCardPropsPG = { data: { id: 'xyz', name: 'Dogs Care', avatarURL: 'https://api.dicebear.com/5.x/initials/svg?seed=John%20Doe', - description: "Dog care center", + description: 'Dog care center', members: { - edges: [ - - ] + edges: [], }, - addressLine1: "Texas, USA" + addressLine1: 'Texas, USA', }, };🧰 Tools
🪛 ESLint
[error] 57-57: Replace
"Dog·care·center"
with'Dog·care·center'
(prettier/prettier)
[error] 59-60: Replace
⏎······]
with],
(prettier/prettier)
[error] 62-62: Replace
"Texas,·USA"
with'Texas,·USA',
(prettier/prettier)
🪛 GitHub Actions: PR Workflow
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
src/components/OrgListCard/OrgListCard.tsx (1)
115-117
: Remove commented admin count section.Remove the commented-out admin count section if it's no longer needed.
- {/* <div> - {tCommon('admins')}: <span>{admins.length}</span> - </div> */}🧰 Tools
🪛 GitHub Actions: PR Workflow
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
src/screens/OrgList/OrgListMocks.ts (2)
12-13
: Remove unused import.The
InterfaceOrgConnectionInfoType
import is defined but never used.- import type { - InterfaceOrgConnectionInfoType, - InterfaceOrgConnectionInfoTypePG, - InterfaceUserType, - } from 'utils/interfaces'; + import type { + InterfaceOrgConnectionInfoTypePG, + InterfaceUserType, + } from 'utils/interfaces';🧰 Tools
🪛 ESLint
[error] 12-12: 'InterfaceOrgConnectionInfoType' is defined but never used.
(@typescript-eslint/no-unused-vars)
🪛 GitHub Actions: PR Workflow
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
32-44
: Fix formatting issues in mock data.Apply these formatting changes:
const organizations: InterfaceOrgConnectionInfoTypePG[] = [ { id: 'xyz', name: 'Dogs Care', avatarURL: 'https://api.dicebear.com/5.x/initials/svg?seed=John%20Doe', - description: "Dog care center", + description: 'Dog care center', members: { - edges: [ - - ] + edges: [], }, - addressLine1: "Texas, USA" + addressLine1: 'Texas, USA', }, ];🧰 Tools
🪛 ESLint
[error] 37-37: Replace
"Dog·care·center"
with'Dog·care·center'
(prettier/prettier)
[error] 39-41: Replace
⏎········⏎······]
with],
(prettier/prettier)
[error] 43-43: Replace
"Texas,·USA"
with'Texas,·USA',
(prettier/prettier)
🪛 GitHub Actions: PR Workflow
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
src/components/ProfileDropdown/ProfileDropdown.spec.tsx (1)
102-102
: Standardize string quotes.Apply these formatting changes:
- setItem('role', "API Administrator"); + setItem('role', 'API Administrator'); - setItem('role', "administrator"); + setItem('role', 'administrator'); - setItem('role', "regular") + setItem('role', 'regular'); - setItem('role', "regular"); + setItem('role', 'regular');Also applies to: 113-113, 144-144, 169-169
🧰 Tools
🪛 ESLint
[error] 102-102: Replace
"API·Administrator"
with'API·Administrator'
(prettier/prettier)
🪛 GitHub Actions: PR Workflow
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
src/screens/OrgList/OrganizationModal.tsx (1)
6-6
: Fix import formatting according to Prettier standards.The import statement needs to be formatted according to the project's style guide.
Apply this diff to fix the formatting:
-import type { InterfaceAddress, InterfaceCurrentUserTypePG } from 'utils/interfaces'; +import type { + InterfaceAddress, + InterfaceCurrentUserTypePG, +} from 'utils/interfaces';🧰 Tools
🪛 ESLint
[error] 6-6: Replace
·InterfaceAddress,·InterfaceCurrentUserTypePG·
with⏎··InterfaceAddress,⏎··InterfaceCurrentUserTypePG,⏎
(prettier/prettier)
🪛 GitHub Actions: PR Workflow
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
src/utils/interfaces.ts (1)
119-143
: Consider adding JSDoc comments for the new interface.While the interface is well-structured, adding documentation would improve maintainability.
Add JSDoc comments to explain the purpose and structure:
+/** + * Represents an organization's connection information in PostgreSQL format. + * @property id - Unique identifier for the organization + * @property avatarURL - URL to the organization's avatar image + * @property name - Name of the organization + * @property members - Nested structure containing member information + * @property description - Description of the organization + * @property addressLine1 - Primary address line + */ export interface InterfaceOrgConnectionInfoTypePG {src/GraphQl/Queries/Queries.ts (1)
49-75
: Consider making the member limit configurable.The query hardcodes a limit of 32 members, which might not be suitable for all use cases.
Consider making the limit configurable:
- members(first: 32) { + members(first: $memberLimit) {And update the query parameters:
- query UserJoinedOrganizations($id: String!, $first: Int) { + query UserJoinedOrganizations($id: String!, $first: Int, $memberLimit: Int = 32) {src/screens/OrgList/OrgList.spec.tsx (1)
148-148
: Fix code style to match Prettier standards.Use single quotes for string literals to maintain consistency.
Apply this diff:
- setItem('role', "administrator"); + setItem('role', 'administrator');Also applies to: 171-171
🧰 Tools
🪛 ESLint
[error] 148-148: Replace
"administrator"
with'administrator'
(prettier/prettier)
🪛 GitHub Actions: PR Workflow
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
schema.graphql
(2 hunks)src/App.tsx
(1 hunks)src/GraphQl/Queries/Queries.ts
(1 hunks)src/components/OrgListCard/OrgListCard.spec.tsx
(4 hunks)src/components/OrgListCard/OrgListCard.tsx
(5 hunks)src/components/ProfileDropdown/ProfileDropdown.spec.tsx
(3 hunks)src/components/ProfileDropdown/ProfileDropdown.tsx
(2 hunks)src/components/SecuredRoute/SecuredRoute.tsx
(1 hunks)src/screens/LoginPage/LoginPage.tsx
(1 hunks)src/screens/OrgList/OrgList.spec.tsx
(4 hunks)src/screens/OrgList/OrgList.tsx
(10 hunks)src/screens/OrgList/OrgListMocks.ts
(8 hunks)src/screens/OrgList/OrganizationModal.tsx
(2 hunks)src/utils/interfaces.ts
(2 hunks)
🧰 Additional context used
🪛 ESLint
src/screens/LoginPage/LoginPage.tsx
[error] 305-305: Insert ;
(prettier/prettier)
[error] 306-306: Replace ||·"")······················································································································································································································································
with ·||·'');
(prettier/prettier)
src/screens/OrgList/OrganizationModal.tsx
[error] 6-6: Replace ·InterfaceAddress,·InterfaceCurrentUserTypePG·
with ⏎··InterfaceAddress,⏎··InterfaceCurrentUserTypePG,⏎
(prettier/prettier)
src/components/OrgListCard/OrgListCard.spec.tsx
[error] 57-57: Replace "Dog·care·center"
with 'Dog·care·center'
(prettier/prettier)
[error] 59-60: Replace ⏎······]
with ],
(prettier/prettier)
[error] 62-62: Replace "Texas,·USA"
with 'Texas,·USA',
(prettier/prettier)
src/components/ProfileDropdown/ProfileDropdown.spec.tsx
[error] 102-102: Replace "API·Administrator"
with 'API·Administrator'
(prettier/prettier)
[error] 113-113: Replace "administrator"
with 'administrator'
(prettier/prettier)
[error] 144-144: Replace "regular")
with 'regular');
(prettier/prettier)
[error] 169-169: Replace "regular"
with 'regular'
(prettier/prettier)
src/screens/OrgList/OrgListMocks.ts
[error] 12-12: 'InterfaceOrgConnectionInfoType' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 37-37: Replace "Dog·care·center"
with 'Dog·care·center'
(prettier/prettier)
[error] 39-41: Replace ⏎········⏎······]
with ],
(prettier/prettier)
[error] 43-43: Replace "Texas,·USA"
with 'Texas,·USA',
(prettier/prettier)
src/components/OrgListCard/OrgListCard.tsx
[error] 40-40: Insert ⏎···
(prettier/prettier)
[error] 65-66: Delete ⏎
(prettier/prettier)
[error] 108-110: Replace ⏎··················text={
${addressLine1}}⏎···············
with ·text={
${addressLine1}}
(prettier/prettier)
src/screens/OrgList/OrgList.tsx
[error] 24-24: Insert ,
(prettier/prettier)
[error] 161-162: Delete ⏎
(prettier/prettier)
[error] 247-247: Expected an assignment or function call and instead saw an expression.
(@typescript-eslint/no-unused-expressions)
[error] 311-311: Replace ·{·organizationsConnection:·InterfaceOrgConnectionTypePG[]·}
with ⏎··········|·{·organizationsConnection:·InterfaceOrgConnectionTypePG[]·}⏎·········
(prettier/prettier)
[error] 315-315: Replace ·{·organizationsConnection:·InterfaceOrgConnectionTypePG[]·}
with ⏎············|·{·organizationsConnection:·InterfaceOrgConnectionTypePG[]·}⏎···········
(prettier/prettier)
[error] 317-317: Replace ·{·organizationsConnection:·InterfaceOrgConnectionTypePG[]·}
with ⏎········|·{·organizationsConnection:·InterfaceOrgConnectionTypePG[]·}⏎·······
(prettier/prettier)
[error] 439-439: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 448-448: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
src/screens/OrgList/OrgList.spec.tsx
[error] 148-148: Replace "administrator"
with 'administrator'
(prettier/prettier)
[error] 171-171: Replace "administrator"
with 'administrator'
(prettier/prettier)
🪛 GitHub Actions: PR Workflow
src/screens/LoginPage/LoginPage.tsx
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
src/App.tsx
[error] Unauthorized file modification. This file is protected and requires the 'ignore-sensitive-files-pr' label to modify.
src/screens/OrgList/OrganizationModal.tsx
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
src/components/OrgListCard/OrgListCard.spec.tsx
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
src/components/ProfileDropdown/ProfileDropdown.spec.tsx
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
schema.graphql
[error] Unauthorized file modification. This file is protected and requires the 'ignore-sensitive-files-pr' label to modify.
src/screens/OrgList/OrgListMocks.ts
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
src/components/OrgListCard/OrgListCard.tsx
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
src/screens/OrgList/OrgList.tsx
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
src/screens/OrgList/OrgList.spec.tsx
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
🔇 Additional comments (12)
src/screens/OrgList/OrgList.tsx (2)
8-9
: Migration to the new queries looks correct.
Switching from previous queries toUSER_JOINED_ORGANIZATIONS_PG
andCURRENT_USER
is aligned with the updated data-fetching logic for user-based organization membership.🧰 Tools
🪛 GitHub Actions: PR Workflow
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
148-153
: Validate that user IDs are strings.
id: getItem('id')
often returns a string. Ensure that queries expecting a numeric type are handled appropriately.Also applies to: 155-155, 162-162
🧰 Tools
🪛 GitHub Actions: PR Workflow
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
src/components/SecuredRoute/SecuredRoute.tsx (1)
18-21
: Role-based routing is properly implemented.
Usingrole === 'administrator'
to guard admin routes is consistent with your updated role-based logic. The fallback to<PageNotFound />
ensures restricted access.src/components/ProfileDropdown/ProfileDropdown.tsx (2)
29-29
: User role retrieved from local storage is well-defined.
Ensure that'role'
is consistent across all updates to local storage.
92-92
: Check role naming consistency.
userRole === 'regular'
diverges from'administrator'
. Confirm that your role naming remains consistent across the codebase (e.g., is'regular'
the only valid label for non-admin?).src/components/OrgListCard/OrgListCard.spec.tsx (1)
89-91
: Consider adding more test assertions.The test only verifies the presence of text elements. Consider adding assertions for:
- Description content
- Member count display
- Organization image URL
🧰 Tools
🪛 GitHub Actions: PR Workflow
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
src/App.tsx (1)
Line range hint
1-1
: This file requires special permission to modify.The pipeline indicates this is a protected file. Please add the 'ignore-sensitive-files-pr' label to the PR to proceed with the changes.
🧰 Tools
🪛 GitHub Actions: PR Workflow
[error] Unauthorized file modification. This file is protected and requires the 'ignore-sensitive-files-pr' label to modify.
src/screens/OrgList/OrganizationModal.tsx (1)
54-54
: LGTM! Type update aligns with PostgreSQL migration.The update to use
InterfaceCurrentUserTypePG
aligns with the PR's objective of adapting to PostgreSQL endpoints.🧰 Tools
🪛 GitHub Actions: PR Workflow
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
src/utils/interfaces.ts (1)
10-17
: LGTM! Well-structured user interface for PostgreSQL.The new interface properly defines the user structure with essential fields.
schema.graphql (2)
1066-1066
: Protected file modification requires special handling.The schema changes look good and align with the login functionality requirements. However, this file is protected and requires the 'ignore-sensitive-files-pr' label to be modified.
Please add the 'ignore-sensitive-files-pr' label to the PR to proceed with these changes.
Also applies to: 1318-1346
🧰 Tools
🪛 GitHub Actions: PR Workflow
[error] Unauthorized file modification. This file is protected and requires the 'ignore-sensitive-files-pr' label to modify.
1654-1657
: LGTM! Role definitions align with login requirements.The UserRole enum correctly defines the administrator role needed for admin login functionality.
🧰 Tools
🪛 GitHub Actions: PR Workflow
[error] Unauthorized file modification. This file is protected and requires the 'ignore-sensitive-files-pr' label to modify.
src/screens/OrgList/OrgList.spec.tsx (1)
186-473
:⚠️ Potential issueCritical: Restore test coverage for organization management.
A large number of tests have been commented out, which significantly reduces test coverage for critical organization management functionality. These tests should be updated to work with the new role-based system rather than being disabled.
Let's verify the current test coverage:
✅ Verification successful
Critical: Restore test coverage for organization management functionality
The verification confirms that critical test coverage has been removed:
- 6 active tests only cover basic UI functionality (search, sort, alerts)
- Core organization management features are no longer tested:
- Organization creation workflow
- Form validation
- Plugin notification system
- Sample organization creation
These tests should be updated to work with the new role-based system rather than being disabled, as they cover essential functionality.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check test coverage for organization management functionality # Test: Look for organization-related test files and their coverage npm test -- --coverage --testPathPattern=src/screens/OrgListLength of output: 255
Script:
#!/bin/bash # Search for test files and active tests in OrgList echo "=== Active tests in OrgList.spec.tsx ===" rg "test\(" src/screens/OrgList/OrgList.spec.tsx -A 1 | grep -v "\/\/" echo -e "\n=== Commented tests in OrgList.spec.tsx ===" rg "\/\/\s*test\(" src/screens/OrgList/OrgList.spec.tsx -A 1 echo -e "\n=== Related test files ===" fd "test|spec" src/screens/OrgList/Length of output: 1203
🧰 Tools
🪛 GitHub Actions: PR Workflow
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
src/components/ProfileDropdown/ProfileDropdown.spec.tsx (1)
Line range hint
190-251
: Update remaining tests to use the new role system.These test cases still use the old role system (
setItem('SuperAdmin', true)
). They should be updated to use the new role-based approach for consistency.test('navigates to /member/:orgId for non-user roles when orgId is not present', async () => { window.history.pushState({}, 'Test page', '/orglist'); - setItem('SuperAdmin', true); // Set as admin + setItem('role', 'API Administrator'); setItem('id', '123'); // ... rest of the test }); test('navigates to /member/:userID for non-user roles', async () => { window.history.pushState({}, 'Test page', '/321'); - setItem('SuperAdmin', true); // Set as admin + setItem('role', 'API Administrator'); setItem('id', '123'); // ... rest of the test });
🧹 Nitpick comments (7)
src/components/ProfileDropdown/ProfileDropdown.spec.tsx (2)
102-102
: Clean up unused role-related setup code.The test correctly uses the new role-based approach. However, the
beforeEach
hook still sets the now-unusedSuperAdmin
andAdminFor
keys. Consider removing these legacy role-related setup lines frombeforeEach
to avoid confusion.beforeEach(() => { setItem('name', 'John Doe'); setItem( 'UserImage', 'https://api.dicebear.com/5.x/initials/svg?seed=John%20Doe', ); - setItem('SuperAdmin', false); - setItem('AdminFor', []); setItem('id', '123'); });Also applies to: 110-110
Line range hint
144-168
: Consider consolidating duplicate test cases.There are two nearly identical test cases that verify navigation to '/user/settings' for regular users. Consider consolidating these tests to reduce duplication and improve maintainability.
- describe('Member screen routing testing', () => { - test('member screen', async () => { - setItem('role', 'regular'); - - render( - <MockedProvider mocks={MOCKS} addTypename={false}> - <BrowserRouter> - <I18nextProvider i18n={i18nForTest}> - <ProfileDropdown /> - </I18nextProvider> - </BrowserRouter> - </MockedProvider>, - ); - - await act(async () => { - userEvent.click(screen.getByTestId('togDrop')); - }); - - await act(async () => { - userEvent.click(screen.getByTestId('profileBtn')); - }); - - expect(mockNavigate).toHaveBeenCalledWith('/user/settings'); - }); - }); - - test('navigates to /user/settings for a user', async () => { + describe('Navigation testing', () => { + test('navigates to /user/settings for regular users', async () => { setItem('role', 'regular'); render( <MockedProvider mocks={MOCKS} addTypename={false}> <BrowserRouter> <I18nextProvider i18n={i18nForTest}> <ProfileDropdown /> </I18nextProvider> </BrowserRouter> </MockedProvider>, ); await act(async () => { userEvent.click(screen.getByTestId('togDrop')); }); await act(async () => { userEvent.click(screen.getByTestId('profileBtn')); }); expect(mockNavigate).toHaveBeenCalledWith('/user/settings'); });Also applies to: 169-189
src/components/OrgListCard/OrgListCard.tsx (4)
32-32
: Update doc comment to reflect removal ofadmins
.
The doc comment still mentionsadmins
even though the property was removed. Consider removing or updating that mention to ensure the description stays accurate.
40-41
: Keep doc comments synchronous with destructured fields.
We have noadmins
property here, while the doc comment references admins. Please remove or revise any leftover references.
83-84
: Optional fallback improvement.
DisplayingavatarURL
is fine, but consider handling error states (e.g. broken image URLs) by falling back to<Avatar />
seamlessly.
111-113
: Remove obsolete commented-out admins code.
If the admins count is no longer needed, delete these lines.src/screens/OrgList/OrgListMocks.ts (1)
45-77
: Remove or restore the large commented-out code block.
If it's no longer relevant, removing it will clarify the final data model.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/components/OrgListCard/OrgListCard.spec.tsx
(4 hunks)src/components/OrgListCard/OrgListCard.tsx
(5 hunks)src/components/ProfileDropdown/ProfileDropdown.spec.tsx
(3 hunks)src/screens/LoginPage/LoginPage.tsx
(1 hunks)src/screens/OrgList/OrgList.spec.tsx
(4 hunks)src/screens/OrgList/OrgList.tsx
(10 hunks)src/screens/OrgList/OrgListMocks.ts
(7 hunks)src/screens/OrgList/OrganizationModal.tsx
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/screens/OrgList/OrgList.spec.tsx
- src/screens/LoginPage/LoginPage.tsx
- src/screens/OrgList/OrganizationModal.tsx
🧰 Additional context used
🪛 ESLint
src/screens/OrgList/OrgListMocks.ts
[error] 12-12: 'InterfaceOrgConnectionInfoType' is defined but never used.
(@typescript-eslint/no-unused-vars)
src/screens/OrgList/OrgList.tsx
[error] 246-246: Expected an assignment or function call and instead saw an expression.
(@typescript-eslint/no-unused-expressions)
[error] 444-444: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 453-453: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🪛 GitHub Actions: PR Workflow
src/components/OrgListCard/OrgListCard.tsx
[error] 51-51: 'navigate' is assigned a value but never used
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (32)
src/components/ProfileDropdown/ProfileDropdown.spec.tsx (1)
113-113
: LGTM! Test case correctly implements the new role-based approach.The test properly sets and verifies the administrator role using the new role-based system.
Also applies to: 121-121
src/components/OrgListCard/OrgListCard.tsx (11)
10-10
: Looks good.
The added import for the new interface is consistent with the PostgreSQL data model.
24-25
: Interface renaming is consistent.
DefiningInterfaceOrgListCardPropsPG
with thedata
property referencing the new interface is tidy.
38-38
: Signature looks good.
Switching toInterfaceOrgListCardPropsPG
aligns with the updated data structure.
46-46
: Parameter rename is correct.
Replacing_id
withid
is consistent with the new schema.
61-61
: Proper variable usage.
Passingid
toORGANIZATIONS_LIST
is consistent with the updated schema.
66-66
: Remove or implement the commented navigation code.
The pipeline warns thatnavigate
is never used, and the code block remains commented out.
68-68
: Same commented-out navigation code persists here.
This duplicates the previously flagged code. Remove or restore it if needed.
100-100
: Good usage of fallback text.
Usingdescription || ''
prevents runtime errors.
104-104
: Conditional address check is safe.
No issues, nicely avoids rendering empty elements.
106-106
: Skipping.
No concerns in this line.
115-115
: Handle potential null data.
members.edges.length
might throw an error ifmembers
oredges
is undefined. Consider a safe-guard if there's any chance of missing data.src/components/OrgListCard/OrgListCard.spec.tsx (5)
11-11
: Importing the updated interface.
Switching toInterfaceOrgListCardPropsPG
keeps the tests in sync with the refactor.
52-52
: Props object updated correctly.
Defining thedata
object to match the PostgreSQL-based fields is a clear improvement.
54-61
: Data structure aligns with new schema.
All fields (id
,avatarURL
,description
, etc.) reflect the updated interface.
89-89
: Test coverage for address field.
ValidatesaddressLine1
is rendered. Good approach to ensure the UI displays location info.
115-115
: Properly testing null avatar scenario.
Ensures fallback image behavior is correct.src/screens/OrgList/OrgListMocks.ts (10)
6-6
: Importing theCURRENT_USER
query.
No issues; aligns with the updated approach to retrieving user data.
8-8
: IntroducingUSER_JOINED_ORGANIZATIONS_PG
.
This new query is consistent with the PostgreSQL-based refactor.
32-32
: Correct data type usage.
organizations
now appropriately referencesInterfaceOrgConnectionInfoTypePG[]
.
34-41
: New fields introduced.
Switched from old_id
andimage
toid
andavatarURL
, matching the latest schema changes.
83-83
: Mock forUSER_JOINED_ORGANIZATIONS_PG
.
Tests the new query in a typical scenario.
100-100
: Mock request forCURRENT_USER
.
Ensures we fetch the super admin user details.
153-153
: Empty organizations mock.
Helps confirm the UI handles no results gracefully.
170-170
: Consistency withCURRENT_USER
.
No issues using the same approach as the standard mocks.
181-181
: Vehicles for error testing.
Mocks with valid data before injecting an error scenario is a sound strategy.
198-198
: Consistently retrieving user info.
No concerns; these lines maintain consistency with the other mocks.src/screens/OrgList/OrgList.tsx (5)
8-9
: LGTM! Query and type imports align with PostgreSQL migration.The imports reflect the transition from MongoDB to PostgreSQL endpoints, with appropriate type definitions.
Also applies to: 22-24
Line range hint
376-385
: Verify the impact of commented-out UI components.The organization creation functionality appears to be disabled through commented-out code. This seems inconsistent with the PR objectives.
Please confirm if this is intentional as part of the PostgreSQL migration. If not, consider:
- Removing the comments if the feature should be active
- Adding a TODO comment explaining why it's temporarily disabled
- Creating a follow-up issue to track the re-enablement
Also applies to: 502-512
246-246
:⚠️ Potential issueFix the no-op expression.
The
toggleModal
expression is a no-op and should be called as a function.- toggleModal; + toggleModal();Likely invalid or redundant comment.
🧰 Tools
🪛 ESLint
[error] 246-246: Expected an assignment or function call and instead saw an expression.
(@typescript-eslint/no-unused-expressions)
444-461
: 🛠️ Refactor suggestionRemove
any
from mapped items.Replace
any
with explicit types that match your edge structure.- ? orgsData?.edges.map((item: any) => { + ? orgsData?.edges.map((item: { node: InterfaceOrgConnectionInfoTypePG }) => {- orgsData?.edges.map((item: any) => { + orgsData?.edges.map((item: { node: InterfaceOrgConnectionInfoTypePG }) => {Likely invalid or redundant comment.
🧰 Tools
🪛 ESLint
[error] 444-444: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 453-453: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
130-162
: Verify data access pattern after query migration.The migration from
ORGANIZATION_CONNECTION_LIST
toUSER_JOINED_ORGANIZATIONS_PG
changes how organization data is accessed. Ensure thatorgsData.edges
exists in the new PostgreSQL response structure.
Please fix the failing tests and make code rabbit approve your PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/docs/auto-docs/screens/OrgList/OrgListMocks/variables/MOCKS_ADMIN.md (1)
7-9
: Consider enhancing documentation for better developer experience.While the type definition is accurate, consider adding:
- A brief description of the
MOCKS_ADMIN
variable's purpose- Examples of usage in tests
- Key differences between MongoDB and PostgreSQL data structures
- Migration notes for developers updating their tests
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (82)
docs/docs/auto-docs/GraphQl/Queries/Queries/variables/ADMIN_LIST.md
(1 hunks)docs/docs/auto-docs/GraphQl/Queries/Queries/variables/BLOCK_PAGE_MEMBER_LIST.md
(1 hunks)docs/docs/auto-docs/GraphQl/Queries/Queries/variables/EVENT_ATTENDEES.md
(1 hunks)docs/docs/auto-docs/GraphQl/Queries/Queries/variables/EVENT_CHECKINS.md
(1 hunks)docs/docs/auto-docs/GraphQl/Queries/Queries/variables/EVENT_DETAILS.md
(1 hunks)docs/docs/auto-docs/GraphQl/Queries/Queries/variables/EVENT_FEEDBACKS.md
(1 hunks)docs/docs/auto-docs/GraphQl/Queries/Queries/variables/EVENT_REGISTRANTS.md
(1 hunks)docs/docs/auto-docs/GraphQl/Queries/Queries/variables/GET_COMMUNITY_DATA.md
(1 hunks)docs/docs/auto-docs/GraphQl/Queries/Queries/variables/GET_COMMUNITY_SESSION_TIMEOUT_DATA.md
(1 hunks)docs/docs/auto-docs/GraphQl/Queries/Queries/variables/MEMBERSHIP_REQUEST.md
(1 hunks)docs/docs/auto-docs/GraphQl/Queries/Queries/variables/MEMBERS_LIST.md
(1 hunks)docs/docs/auto-docs/GraphQl/Queries/Queries/variables/ORGANIZATIONS_LIST.md
(1 hunks)docs/docs/auto-docs/GraphQl/Queries/Queries/variables/ORGANIZATIONS_MEMBER_CONNECTION_LIST.md
(1 hunks)docs/docs/auto-docs/GraphQl/Queries/Queries/variables/ORGANIZATION_CONNECTION_LIST.md
(1 hunks)docs/docs/auto-docs/GraphQl/Queries/Queries/variables/ORGANIZATION_DONATION_CONNECTION_LIST.md
(1 hunks)docs/docs/auto-docs/GraphQl/Queries/Queries/variables/ORGANIZATION_EVENT_CONNECTION_LIST.md
(1 hunks)docs/docs/auto-docs/GraphQl/Queries/Queries/variables/ORGANIZATION_EVENT_LIST.md
(1 hunks)docs/docs/auto-docs/GraphQl/Queries/Queries/variables/RECURRING_EVENTS.md
(1 hunks)docs/docs/auto-docs/GraphQl/Queries/Queries/variables/SIGNIN_QUERY.md
(1 hunks)docs/docs/auto-docs/GraphQl/Queries/Queries/variables/USERS_CONNECTION_LIST.md
(1 hunks)docs/docs/auto-docs/GraphQl/Queries/Queries/variables/USER_DETAILS.md
(1 hunks)docs/docs/auto-docs/GraphQl/Queries/Queries/variables/USER_JOINED_ORGANIZATIONS_PG.md
(1 hunks)docs/docs/auto-docs/GraphQl/Queries/Queries/variables/USER_LIST.md
(1 hunks)docs/docs/auto-docs/GraphQl/Queries/Queries/variables/USER_LIST_FOR_TABLE.md
(1 hunks)docs/docs/auto-docs/GraphQl/Queries/Queries/variables/USER_LIST_REQUEST.md
(1 hunks)docs/docs/auto-docs/GraphQl/Queries/Queries/variables/USER_ORGANIZATION_LIST.md
(1 hunks)docs/docs/auto-docs/components/OrgListCard/OrgListCard/functions/default.md
(1 hunks)docs/docs/auto-docs/components/OrgListCard/OrgListCard/interfaces/InterfaceOrgListCardProps.md
(0 hunks)docs/docs/auto-docs/components/OrgListCard/OrgListCard/interfaces/InterfaceOrgListCardPropsPG.md
(1 hunks)docs/docs/auto-docs/screens/OrgList/OrgList/functions/default.md
(1 hunks)docs/docs/auto-docs/screens/OrgList/OrgListMocks/variables/MOCKS.md
(1 hunks)docs/docs/auto-docs/screens/OrgList/OrgListMocks/variables/MOCKS_ADMIN.md
(1 hunks)docs/docs/auto-docs/screens/OrgList/OrgListMocks/variables/MOCKS_EMPTY.md
(1 hunks)docs/docs/auto-docs/screens/OrgList/OrgListMocks/variables/MOCKS_WITH_ERROR.md
(1 hunks)docs/docs/auto-docs/screens/OrgList/OrganizationModal/functions/default.md
(1 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceActionItemCategoryInfo.md
(2 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceActionItemCategoryList.md
(1 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceActionItemInfo.md
(3 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceActionItemList.md
(1 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceAddOnSpotAttendeeProps.md
(3 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceAddress.md
(1 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceAgendaItemCategoryInfo.md
(2 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceAgendaItemCategoryList.md
(1 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceAgendaItemInfo.md
(5 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceAgendaItemList.md
(1 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceBaseEvent.md
(2 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceCampaignInfo.md
(1 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceCreateFund.md
(1 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceCreatePledge.md
(1 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceCreateVolunteerGroup.md
(1 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceCurrentUserTypePG.md
(1 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceCustomFieldData.md
(1 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceEventVolunteerInfo.md
(3 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceFormData.md
(1 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceFundInfo.md
(2 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceMapType.md
(1 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceMemberInfo.md
(1 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceMembersList.md
(1 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceOrgConnectionInfoType.md
(4 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceOrgConnectionInfoTypePG.md
(1 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceOrgConnectionType.md
(1 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceOrgConnectionTypePG.md
(1 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceOrgInfoTypePG.md
(1 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfacePledgeInfo.md
(2 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfacePostCard.md
(5 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfacePostForm.md
(1 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceQueryBlockPageMemberListItem.md
(1 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceQueryFundCampaignsPledges.md
(2 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceQueryMembershipRequestsListItem.md
(1 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceQueryOrganizationAdvertisementListItem.md
(1 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceQueryOrganizationEventListItem.md
(11 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceQueryOrganizationFundCampaigns.md
(2 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceQueryOrganizationListObject.md
(4 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceQueryOrganizationPostListItem.md
(1 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceQueryOrganizationUserTags.md
(1 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceQueryOrganizationsListObject.md
(6 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceQueryUserListItem.md
(2 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceQueryUserTagChildTags.md
(2 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceQueryUserTagsAssignedMembers.md
(2 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceQueryUserTagsMembersToAssignTo.md
(1 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceQueryVenueListItem.md
(1 hunks)docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceTagData.md
(4 hunks)
⛔ Files not processed due to max files limit (14)
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceUserCampaign.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceUserEvents.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceUserInfo.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceUserTypePG.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceVolunteerGroupInfo.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceVolunteerMembership.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceVolunteerRank.md
- src/App.tsx
- src/components/OrgListCard/OrgListCard.tsx
- src/screens/LoginPage/LoginPage.tsx
- src/screens/OrgList/OrgList.spec.tsx
- src/screens/OrgList/OrgList.tsx
- src/screens/OrgList/OrgListMocks.ts
- src/utils/interfaces.ts
💤 Files with no reviewable changes (1)
- docs/docs/auto-docs/components/OrgListCard/OrgListCard/interfaces/InterfaceOrgListCardProps.md
✅ Files skipped from review due to trivial changes (75)
- docs/docs/auto-docs/GraphQl/Queries/Queries/variables/GET_COMMUNITY_SESSION_TIMEOUT_DATA.md
- docs/docs/auto-docs/GraphQl/Queries/Queries/variables/ORGANIZATIONS_MEMBER_CONNECTION_LIST.md
- docs/docs/auto-docs/GraphQl/Queries/Queries/variables/BLOCK_PAGE_MEMBER_LIST.md
- docs/docs/auto-docs/GraphQl/Queries/Queries/variables/MEMBERS_LIST.md
- docs/docs/auto-docs/GraphQl/Queries/Queries/variables/GET_COMMUNITY_DATA.md
- docs/docs/auto-docs/GraphQl/Queries/Queries/variables/ORGANIZATION_CONNECTION_LIST.md
- docs/docs/auto-docs/GraphQl/Queries/Queries/variables/USER_LIST_REQUEST.md
- docs/docs/auto-docs/GraphQl/Queries/Queries/variables/RECURRING_EVENTS.md
- docs/docs/auto-docs/GraphQl/Queries/Queries/variables/ADMIN_LIST.md
- docs/docs/auto-docs/GraphQl/Queries/Queries/variables/USERS_CONNECTION_LIST.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceMapType.md
- docs/docs/auto-docs/components/OrgListCard/OrgListCard/interfaces/InterfaceOrgListCardPropsPG.md
- docs/docs/auto-docs/GraphQl/Queries/Queries/variables/EVENT_DETAILS.md
- docs/docs/auto-docs/GraphQl/Queries/Queries/variables/EVENT_REGISTRANTS.md
- docs/docs/auto-docs/GraphQl/Queries/Queries/variables/USER_ORGANIZATION_LIST.md
- docs/docs/auto-docs/GraphQl/Queries/Queries/variables/EVENT_FEEDBACKS.md
- docs/docs/auto-docs/screens/OrgList/OrgList/functions/default.md
- docs/docs/auto-docs/GraphQl/Queries/Queries/variables/ORGANIZATION_EVENT_LIST.md
- docs/docs/auto-docs/GraphQl/Queries/Queries/variables/ORGANIZATIONS_LIST.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceOrgConnectionInfoTypePG.md
- docs/docs/auto-docs/screens/OrgList/OrganizationModal/functions/default.md
- docs/docs/auto-docs/GraphQl/Queries/Queries/variables/ORGANIZATION_DONATION_CONNECTION_LIST.md
- docs/docs/auto-docs/GraphQl/Queries/Queries/variables/ORGANIZATION_EVENT_CONNECTION_LIST.md
- docs/docs/auto-docs/GraphQl/Queries/Queries/variables/MEMBERSHIP_REQUEST.md
- docs/docs/auto-docs/GraphQl/Queries/Queries/variables/USER_JOINED_ORGANIZATIONS_PG.md
- docs/docs/auto-docs/GraphQl/Queries/Queries/variables/SIGNIN_QUERY.md
- docs/docs/auto-docs/GraphQl/Queries/Queries/variables/EVENT_ATTENDEES.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceAgendaItemList.md
- docs/docs/auto-docs/GraphQl/Queries/Queries/variables/USER_LIST.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceCurrentUserTypePG.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceActionItemCategoryList.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceQueryUserTagChildTags.md
- docs/docs/auto-docs/GraphQl/Queries/Queries/variables/EVENT_CHECKINS.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceMembersList.md
- docs/docs/auto-docs/GraphQl/Queries/Queries/variables/USER_DETAILS.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceQueryOrganizationPostListItem.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceOrgConnectionTypePG.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceCustomFieldData.md
- docs/docs/auto-docs/GraphQl/Queries/Queries/variables/USER_LIST_FOR_TABLE.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceQueryUserTagsMembersToAssignTo.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceCampaignInfo.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceQueryMembershipRequestsListItem.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceQueryUserListItem.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceCreatePledge.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceQueryOrganizationAdvertisementListItem.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceOrgInfoTypePG.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceOrgConnectionType.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceAddOnSpotAttendeeProps.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceActionItemCategoryInfo.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceTagData.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceMemberInfo.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceCreateVolunteerGroup.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceAgendaItemCategoryInfo.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceQueryVenueListItem.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceQueryUserTagsAssignedMembers.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceEventVolunteerInfo.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceCreateFund.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceActionItemList.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceQueryOrganizationFundCampaigns.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceQueryOrganizationUserTags.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceAgendaItemInfo.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceFormData.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceQueryOrganizationListObject.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfacePostCard.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceOrgConnectionInfoType.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceQueryBlockPageMemberListItem.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfacePostForm.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceAddress.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceQueryFundCampaignsPledges.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfacePledgeInfo.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceBaseEvent.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceFundInfo.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceQueryOrganizationEventListItem.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceQueryOrganizationsListObject.md
- docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceActionItemInfo.md
🧰 Additional context used
🪛 LanguageTool
docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceAgendaItemCategoryList.md
[uncategorized] ~13-~13: A punctuation mark might be missing here.
Context: ...agendaItemCategoriesByOrganization: [InterfaceAgendaItemCategoryInfo
](Interf...
(AI_EN_LECTOR_MISSING_PUNCTUATION)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Test Application
🔇 Additional comments (11)
docs/docs/auto-docs/screens/OrgList/OrgListMocks/variables/MOCKS_ADMIN.md (1)
7-7
: LGTM! Type change aligns with PostgreSQL migration.The update from
InterfaceOrgConnectionInfoType
toInterfaceOrgInfoTypePG
correctly reflects the database migration from MongoDB to PostgreSQL.docs/docs/auto-docs/screens/OrgList/OrgListMocks/variables/MOCKS.md (2)
7-7
: LGTM! Type definition updated for PostgreSQL.The type definition change from MongoDB to PostgreSQL (
InterfaceOrgInfoTypePG[]
) aligns with the PR objectives.Let's verify that the referenced interface documentation exists:
#!/bin/bash # Description: Verify that the referenced interface documentation exists # Expected: Should find the interface documentation file fd "InterfaceOrgInfoTypePG.md" docs/
9-9
: LGTM! Source file reference updated.The line number reference has been correctly updated to reflect the new location in the source file.
Let's verify the line number reference:
#!/bin/bash # Description: Verify that MOCKS is defined at the specified line # Expected: Should find MOCKS defined at line 79 rg -n "^export const MOCKS" src/screens/OrgList/OrgListMocks.tsdocs/docs/auto-docs/components/OrgListCard/OrgListCard/functions/default.md (3)
9-9
: LGTM! Line reference updated correctly.The source file reference has been updated to line 31, which aligns with the component restructuring mentioned in the PR objectives.
14-14
: LGTM! Property name updated for PostgreSQL schema.The documentation correctly reflects the change from "address" to "addressLine1", which is consistent with the PostgreSQL migration.
21-21
: Verify the interface documentation exists.The interface type has been updated to
InterfaceOrgListCardPropsPG
, but we should verify that its documentation exists and is accessible through the provided link.#!/bin/bash # Description: Check if the interface documentation file exists # and verify its content matches the new PostgreSQL schema # Check if the interface documentation file exists fd "InterfaceOrgListCardPropsPG.md" docs/docs/auto-docs # If found, check its content for PostgreSQL-specific fields if [ $? -eq 0 ]; then rg -A 5 "interface InterfaceOrgListCardPropsPG" docs/docs/auto-docs fidocs/docs/auto-docs/screens/OrgList/OrgListMocks/variables/MOCKS_WITH_ERROR.md (2)
7-7
: Verify type consistency across the codebase.The type change from
InterfaceOrgConnectionInfoType
toInterfaceOrgInfoTypePG
aligns with the PR's objective of migrating from MongoDB to PostgreSQL. Let's verify this change is consistent across the codebase.#!/bin/bash # Description: Verify type consistency and usage across the codebase # Check for any remaining references to the old MongoDB interface type echo "Checking for old interface references..." rg "InterfaceOrgConnectionInfoType" # Check for consistent usage of the new PostgreSQL interface type echo "Checking new interface usage..." rg "InterfaceOrgInfoTypePG" # Check for any potential type mismatches in GraphQL queries echo "Checking GraphQL queries..." rg -t graphql "type.*Org.*Connection"
9-9
: Verify documentation line number reference.The source file line number has changed from 192 to 177, which could indicate code reorganization. Let's verify this reference is accurate.
#!/bin/bash # Description: Verify the line number reference in the documentation # Check the actual line number in the source file echo "Checking line number in source file..." rg -n "MOCKS_WITH_ERROR" "src/screens/OrgList/OrgListMocks.ts"docs/docs/auto-docs/utils/interfaces/interfaces/InterfaceAgendaItemCategoryList.md (1)
7-15
: Documentation updates look good, verify line references.The documentation updates correctly reflect the interface's new location in the codebase.
Let's verify the accuracy of the line references in the source file:
#!/bin/bash # Description: Verify the line numbers for InterfaceAgendaItemCategoryList interface definition # Search for the interface definition and its property rg -n "interface InterfaceAgendaItemCategoryList" src/utils/interfaces.ts rg -n "agendaItemCategoriesByOrganization" src/utils/interfaces.ts🧰 Tools
🪛 LanguageTool
[uncategorized] ~13-~13: A punctuation mark might be missing here.
Context: ...agendaItemCategoriesByOrganization: [InterfaceAgendaItemCategoryInfo
](Interf...(AI_EN_LECTOR_MISSING_PUNCTUATION)
docs/docs/auto-docs/screens/OrgList/OrgListMocks/variables/MOCKS_EMPTY.md (2)
Line range hint
1-24
: LGTM! Documentation accurately reflects the type changes.The auto-generated documentation correctly captures:
- The union type structure for both organization list and current user queries
- The updated type definitions including the
InterfaceUserType
- The new source file location
Line range hint
1-24
: Verify the mock data structure aligns with PostgreSQL schema.Since this PR involves transitioning from MongoDB to PostgreSQL, ensure that:
- The mock data structure matches the PostgreSQL schema
- All required fields for PostgreSQL are included
Run this script to verify the schema alignment:
#!/bin/bash # Description: Verify that the mock data structure aligns with PostgreSQL schema # by checking the GraphQL schema and related type definitions # Check GraphQL schema for PostgreSQL types rg -A 10 "type.*User.*{" # Check interface definitions ast-grep --pattern 'interface InterfaceUserType { $$$ }' # Check mock implementations ast-grep --pattern 'const MOCKS_EMPTY = [ $$$ ]'
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #3477 +/- ##
====================================================
- Coverage 88.68% 88.63% -0.06%
====================================================
Files 341 341
Lines 8638 8579 -59
Branches 1925 1914 -11
====================================================
- Hits 7661 7604 -57
+ Misses 644 640 -4
- Partials 333 335 +2 ☔ View full report in Codecov by Sentry. |
@palisadoes PTAL In this PR, I have fixed the admin login; however, the SuperAdmin functionality is yet to be addressed. That's why a significant portion of code in OrgList.tsx is commented out. Could you let me know if anyone else is currently working on adapting the admin to GraphQL endpoints? This would help me identify which sections are already being worked on so I can pick a different area to focus on. |
working on documentation... |
Try to get the test code coverage for your patch up to 100% too |
yes |
@rahulch07 Are you working to fix SuperAdmin functionality, or may I take that up? |
I think superadmin login should be fixed in another issue to keep the changes seperated. I have created one issue regarding it, will fix this login issue for superadmin role there soon as it's in priority. @palisadoes what do you think? |
The super admin role no longer exists in the API |
@palisadoes Have read the docs throughly. Had a question what are the functionalities that API Administrator has that Administrator do not have? |
Only to:
|
Check slack. I DM'd you |
|
yes working on tests will be done by today. spent some time on understanding the API functionalities I think the backend also has the issue where administrator is able to promote regular users |
That feature should remain for now. If prefer not to modify too much in the API now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (7)
src/components/OrgListCard/OrgListCard.tsx (2)
28-28
: Improve props destructuring pattern.While the component name now follows React conventions, the props destructuring can be optimized.
Apply this diff to improve props destructuring:
-function OrgListCard(props: InterfaceOrgListCardPropsPG): JSX.Element { - const { avatarURL, addressLine1, name, description, members } = props.data; +function OrgListCard({ data }: InterfaceOrgListCardPropsPG): JSX.Element { + const { avatarURL, addressLine1, name, description, members } = data;Also applies to: 30-30
58-59
: LGTM! UI improvements with one cleanup needed.The changes improve the component's robustness with proper avatar fallback and updated organization details rendering. However, there's still some cleanup needed.
Remove the commented-out sample organization UI code (lines 99-108) since it's no longer needed according to the PR discussion about role changes.
Also applies to: 75-75, 79-81, 87-87, 99-108
docs/docs/docs/getting-started/installation.md (5)
289-293
: Clarify Note Formatting and Fix Typo
The note instructing users to replace "user-id" and org-id is helpful. However, the sentence contains a typo: change "You can obtain this form the postgres database via cloudbeaver" to "You can obtain this from the postgres database via cloudbeaver."- You can obtain this form the postgres database via cloudbeaver. + You can obtain this from the postgres database via cloudbeaver.
300-325
: GraphQL SignIn Mutation Example – Use Placeholders
The signIn mutation example is well-structured. For better security practices and to avoid confusion, consider using placeholder values (e.g.,<ADMIN_EMAIL>
and<ADMIN_PASSWORD>
) instead of hardcoded credentials.- input: { emailAddress: "[email protected]", password: "password" } + input: { emailAddress: "<ADMIN_EMAIL>", password: "<ADMIN_PASSWORD>" }
329-345
: UpdateUser Mutation Clarity
The mutation for assigning the Administrator role is clear. It might be useful to add a brief note about any expected error handling (e.g., what should a user look for if the update fails) to guide troubleshooting.
355-381
: Organization Membership Mutation – Additional Clarification
The mutation for assigning an administrator role to the user within an organization is well-structured. For added clarity, consider commenting (or documenting nearby) what type of error messages users might encounter if the membership creation fails.
383-383
: Final Instruction Rewording for Clarity
The final instruction could be rephrased for clarity. For example, "Now, sign in with your registered ADMIN account" more clearly communicates the action the user needs to take.- Now sign successfully in with your registered ADMIN. + Now, sign in with your registered ADMIN account.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/docs/auto-docs/components/OrgListCard/OrgListCard/functions/default.md
(1 hunks)docs/docs/auto-docs/components/OrgListCard/OrgListCard/interfaces/InterfaceOrgListCardPropsPG.md
(1 hunks)docs/docs/docs/getting-started/installation.md
(1 hunks)src/components/OrgListCard/OrgListCard.tsx
(5 hunks)src/components/SecuredRoute/securedRoute.spec.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- docs/docs/auto-docs/components/OrgListCard/OrgListCard/functions/default.md
- docs/docs/auto-docs/components/OrgListCard/OrgListCard/interfaces/InterfaceOrgListCardPropsPG.md
- src/components/SecuredRoute/securedRoute.spec.tsx
🧰 Additional context used
🪛 ESLint
src/components/OrgListCard/OrgListCard.tsx
[error] 30-30: Must use destructuring props assignment
(react/destructuring-assignment)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (6)
src/components/OrgListCard/OrgListCard.tsx (3)
7-7
: LGTM! Interface changes align with PostgreSQL migration.The updated interface names and imports correctly reflect the migration from MongoDB to PostgreSQL.
Also applies to: 14-15
33-37
: Remove commented query code.Since SuperAdmin functionality is no longer needed (as mentioned in PR comments), remove the commented-out query code to maintain code cleanliness.
41-43
: Remove commented code or implement navigation.The
handleClick
function contains commented-out navigation code. Either implement the navigation functionality or remove the comments if they're no longer needed.docs/docs/docs/getting-started/installation.md (3)
270-277
: Clear Introductory Section for Admin Sign-In
The new "First Time SignIn as Admin" section clearly explains that the PostgreSQL database is initially set up with only one Administrator. This gives new users a solid context for the admin registration process.
278-286
: Straightforward Registration Instructions
The steps for registering as an Admin using the frontend are clear and the optional image placeholder is a nice addition for visual guidance.
349-382
: Organization Creation Mutation – Country Code Format
In the mutation that creates an organization, thecountryCode
field is set asin
without quotes. If this field is defined as an enum in your GraphQL schema, consider following the convention by using uppercase (e.g.,IN
). If it is meant to be a string, then enclose it in quotes (e.g.,"in"
). Please verify that this aligns with the GraphQL schema.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
schema.graphql (1)
1365-1615
: EnumIso3166Alpha2CountryCode
Update
The enum listing detailed country codes has been extensively updated here. Given its size, you might consider whether it could be auto-generated or sourced from a maintained library to reduce future maintenance overhead.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
schema.graphql
(9 hunks)
🔇 Additional comments (11)
schema.graphql (11)
690-695
: UpdatedsendMessageToChat
Mutation: OptionalreplyTo
Field
The mutation now defines thereplyTo
argument as optional (ID
) instead of non-null. This change is appropriate if not every message is a reply. Please ensure that both the resolver implementation and client-side logic gracefully handle cases wherereplyTo
is omitted.
1074-1074
: Updateduser
Query Signature
Theuser
query now accepts an input object (QueryUserInput
) and returns the newUserPg
type. This aligns with the transition to PostgreSQL for user data retrieval. Please verify that all client queries and backend resolvers have been updated to adopt this new structure.
1326-1359
: Introduction of New TypeUserPg
The newUserPg
type provides a richer and more granular representation of the user data required for PostgreSQL operations. Key points to review:
- Field Naming & Mapping: Ensure that fields such as
avatarURL
,role
, and others exactly match the structure of the PostgreSQL backend.- Nullability: Confirm that fields are correctly marked as nullable or non-null as per business logic requirements.
Overall, this type lays a solid foundation for enhancing user queries under the new database backend.
1361-1363
: Addition ofQueryUserInput
Input Type
The new input typeQueryUserInput
encapsulates the user ID as a string for the updateduser
query. This pattern improves extensibility by allowing for future enhancements without altering the query signature.
1617-1634
: EnumUserEducationGrade
– Lowercase Formatting
The newUserEducationGrade
enum uses lowercase identifiers (e.g.,grade_1
,grade_2
) which differs from some existing enums (e.g.,EducationGrade
uses uppercase). Please verify that this naming convention is intentional and that any client-side code properly maps these values.
1636-1640
: EnumUserEmploymentStatus
Format Check
TheUserEmploymentStatus
enum now employs lowercase values. Ensure that any existing logic expecting values in a particular case (e.g., uppercase) is updated accordingly.
1642-1649
: EnumUserMaritalStatus
Consistency Consideration
Similarly, theUserMaritalStatus
enum uses a lowercase format. Confirm that this change is consistent with your overall naming strategy and that consumers of this enum (both front-end and back-end) are aware of the new format.
1651-1655
: Introduction of EnumUserNatalSex
The addition ofUserNatalSex
with valuesfemale
,intersex
, andmale
is straightforward and clear. Just ensure that any business logic or validations that depend on this field are updated to accommodate the new enum.
1657-1666
: New Connection Type for User Organizations
TheUserOrganizationsWhereMemberConnection
type and its edge definition provide a standardized approach to paginate organizations where a user is a member. This follows established GraphQL patterns and is well-integrated into the schema.
1667-1670
: EnumUserRole
– Case Sensitivity Check
TheUserRole
enum now uses lowercase values (administrator
andregular
). Please confirm that these values are consistent with the role-checking logic throughout your application and that any existing references have been updated.
1738-1738
: New Field inUserInput
:selectedOrganization
The required fieldselectedOrganization: ID!
has been added to theUserInput
type. Ensure that the sign-up flow, input validations, and related test cases are updated to reflect and enforce this new requirement.
@palisadoes PTAL
The approach i used is in the userJoinedOrganization query a get required info of all organizations together.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (5)
docs/docs/auto-docs/screens/OrganizationActionItems/OrganizationActionItem.mocks/variables/MOCKS_ERROR.md (1)
7-9
: Refactor Complex Union Type for Readability and MaintainabilityThe updated declaration for
MOCKS_ERROR
introduces a very complex union type that mixes several disparate type shapes. While it correctly reflects the various responses and error cases, the sheer length and complexity make it hard to read and maintain. Consider refactoring by splitting this union into several smaller, well-named type aliases. This will not only improve readability but also help enforce consistent naming conventions (e.g., unifying the use ofis_disabled
vs.isDisabled
).docs/docs/auto-docs/screens/OrganizationActionItems/OrganizationActionItem.mocks/variables/MOCKS.md (1)
Line range hint
7-100
: Enhance Readability through Better FormattingThe updated declaration of the
MOCKS
variable now includes very complex union types with deeply nested objects. To improve long‑term maintainability and readability, consider reformatting the code block by adding additional line breaks and consistent indentation for each nested level. This will make it easier for future developers to parse the structure at a glance.schema.graphql (3)
1365-1615
: Use uppercase values for ISO country codes.The ISO 3166-1 alpha-2 country codes are conventionally represented in uppercase.
Consider updating the enum values to uppercase for consistency with the ISO standard. For example:
enum Iso3166Alpha2CountryCode { - ad + AD - ae + AE # ... (remaining codes) }
1617-1634
: Use SCREAMING_SNAKE_CASE for enum values.The enum values should follow the convention of using uppercase with underscores.
Consider updating the enum values to SCREAMING_SNAKE_CASE for consistency:
enum UserEducationGrade { - grade_1 + GRADE_1 - grade_2 + GRADE_2 # ... (remaining grades) }
1636-1670
: Use SCREAMING_SNAKE_CASE for enum values.The enum values should follow the convention of using uppercase with underscores.
Consider updating the enum values to SCREAMING_SNAKE_CASE for consistency:
enum UserEmploymentStatus { - full_time + FULL_TIME - part_time + PART_TIME - unemployed + UNEMPLOYED } enum UserMaritalStatus { - divorced + DIVORCED # ... (remaining statuses) } enum UserNatalSex { - female + FEMALE # ... (remaining values) } enum UserRole { - administrator + ADMINISTRATOR - regular + REGULAR }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
docs/docs/auto-docs/screens/OrganizationActionItems/OrganizationActionItem.mocks/variables/MOCKS.md
(1 hunks)docs/docs/auto-docs/screens/OrganizationActionItems/OrganizationActionItem.mocks/variables/MOCKS_EMPTY.md
(1 hunks)docs/docs/auto-docs/screens/OrganizationActionItems/OrganizationActionItem.mocks/variables/MOCKS_ERROR.md
(1 hunks)docs/docs/auto-docs/screens/OrganizationFundCampaign/OrganizationFundCampagins/functions/default.md
(1 hunks)docs/docs/auto-docs/screens/OrganizationFunds/OrganizationFunds/functions/default.md
(1 hunks)schema.graphql
(2 hunks)src/components/OrgListCard/OrgListCard.tsx
(5 hunks)
✅ Files skipped from review due to trivial changes (3)
- docs/docs/auto-docs/screens/OrganizationFundCampaign/OrganizationFundCampagins/functions/default.md
- docs/docs/auto-docs/screens/OrganizationFunds/OrganizationFunds/functions/default.md
- docs/docs/auto-docs/screens/OrganizationActionItems/OrganizationActionItem.mocks/variables/MOCKS_EMPTY.md
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Test Application
- GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (11)
docs/docs/auto-docs/screens/OrganizationActionItems/OrganizationActionItem.mocks/variables/MOCKS.md (1)
Line range hint
7-100
: Clarify the Use ofundefined
Literal TypesSeveral properties (for example,
_id
within union types) are explicitly typed asundefined
. If this is intended to model data that may be missing, verify whether using the literalundefined
is the most semantically clear approach versus marking properties as optional (e.g., using_id?: string
). Please confirm that this accurately represents the expected behavior of the mock data.src/components/OrgListCard/OrgListCard.tsx (6)
7-7
: LGTM! Interface changes align with PostgreSQL migration.The imports and interface declarations correctly reflect the transition to PostgreSQL data types.
Also applies to: 14-16
28-30
: LGTM! Component declaration updated correctly.The component has been properly renamed to follow React conventions and the props destructuring aligns with the PostgreSQL schema.
57-58
: LGTM! Template updated to use PostgreSQL fields.The template has been correctly updated to use the new PostgreSQL fields and pagination structure.
Also applies to: 74-74, 78-81, 86-86
Line range hint
98-107
: Handle sample organization UI consistently.The flask icon section should be handled consistently with the sample organization query decision.
This section should be restored if the sample organization functionality is still needed (see previous verification task).
32-36
: Verify if sample organization functionality is still needed.The commented sample organization query might still be relevant. Please verify if this functionality should be maintained or removed.
Run the following script to check for sample organization usage:
✅ Verification successful
Subject: Sample Organization Functionality Remains Active
The search results show that the
IS_SAMPLE_ORGANIZATION_QUERY
(and similar identifiers) appear in multiple files—including test files, mocks, and query definition files—which confirms that the sample organization functionality is still in use. Before making any changes, please verify that any code removal or deprecation aligns with the expected behavior in tests and related components.Key points:
- The query is referenced in both production (e.g.,
src/components/OrgSettings/General/DeleteOrg/DeleteOrg.tsx
) and test files (e.g.,src/components/OrgListCard/OrgListCard.spec.tsx
,DeleteOrg.spec.tsx
).- Documentation and mock files also reference sample organization variables.
- The query definitions in
src/GraphQl/Queries/PlugInQueries.ts
andsrc/GraphQl/Queries/Queries.ts
indicate its ongoing relevance.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for sample organization functionality usage # Search for sample organization related code rg -l 'IS_SAMPLE_ORGANIZATION_QUERY|isSampleOrganization'Length of output: 478
40-42
:⚠️ Potential issueImplement the navigation functionality.
The navigation code is commented out but appears to be essential for managing organizations.
Apply this diff to restore the navigation functionality:
function handleClick(): void { - // const url = '/orgdash/' + id; - // Dont change the below two lines - // navigate(url); + const url = '/orgdash/' + id; + navigate(url); }Likely invalid or redundant comment.
schema.graphql (4)
1074-1074
: LGTM! User query updated to use input type.The query has been properly updated to use an input type and return the PostgreSQL user type.
1326-1359
: LGTM! UserPg type properly defined.The UserPg type includes all necessary fields with appropriate types and supports pagination for organizations.
1361-1363
: LGTM! QueryUserInput properly defined.The input type is appropriately defined with the necessary ID field.
1657-1665
: LGTM! Connection types properly implement Relay specification.The connection types correctly implement the Relay connection specification for proper pagination support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/components/OrgListCard/OrgListCard.spec.tsx (2)
11-11
: Consider documenting the interface naming convention.The suffix 'PG' in
InterfaceOrgListCardPropsPG
suggests PostgreSQL-specific interface. Consider adding a comment or documentation to explain the naming convention and the difference between this interface and the original one.Also applies to: 52-52
89-89
: Strengthen the address assertion.The current assertion
expect(screen.getByText(/Texas/i)).toBeDefined();
is too lenient. Consider:
- Testing the exact address string "Texas, USA"
- Adding assertions for the description field
- expect(screen.getByText(/Texas/i)).toBeDefined(); + expect(screen.getByText('Texas, USA')).toBeDefined(); + expect(screen.getByText('Dog care center')).toBeDefined();
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/OrgListCard/OrgListCard.spec.tsx
(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Test Application
🔇 Additional comments (1)
src/components/OrgListCard/OrgListCard.spec.tsx (1)
54-61
: Verify test data coverage.The test data structure has been simplified, but please ensure:
- All required fields from the PostgreSQL schema are covered in the test data.
- Edge cases are tested (e.g., empty
edges
array is tested, but what about non-empty array?).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/screens/OrgList/OrgListMocks.ts (1)
31-41
: LGTM! Consider adding more test cases.The mock data structure correctly reflects the PostgreSQL schema with appropriate fields and relationship patterns. However, consider adding more varied test cases to cover different scenarios.
Add more organization objects with different member counts and descriptions to test pagination and filtering:
const organizations: InterfaceOrgInfoTypePG[] = [ { id: 'xyz', name: 'Dogs Care', avatarURL: 'https://api.dicebear.com/5.x/initials/svg?seed=John%20Doe', description: 'Dog care center', members: { edges: [], }, addressLine1: 'Texas, USA', }, + { + id: 'abc', + name: 'Cat Shelter', + avatarURL: 'https://api.dicebear.com/5.x/initials/svg?seed=Cat%20Shelter', + description: 'Shelter for stray cats', + members: { + edges: [ + { node: { id: 'member1' } }, + { node: { id: 'member2' } } + ], + }, + addressLine1: 'California, USA', + } ];
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docs/docs/auto-docs/screens/OrgList/OrgListMocks/variables/MOCKS.md
(1 hunks)docs/docs/auto-docs/screens/OrgList/OrgListMocks/variables/MOCKS_ADMIN.md
(1 hunks)docs/docs/auto-docs/screens/OrgList/OrgListMocks/variables/MOCKS_EMPTY.md
(1 hunks)docs/docs/auto-docs/screens/OrgList/OrgListMocks/variables/MOCKS_WITH_ERROR.md
(1 hunks)src/screens/OrgList/OrgList.spec.tsx
(5 hunks)src/screens/OrgList/OrgListMocks.ts
(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- docs/docs/auto-docs/screens/OrgList/OrgListMocks/variables/MOCKS_ADMIN.md
- docs/docs/auto-docs/screens/OrgList/OrgListMocks/variables/MOCKS_WITH_ERROR.md
- src/screens/OrgList/OrgList.spec.tsx
- docs/docs/auto-docs/screens/OrgList/OrgListMocks/variables/MOCKS_EMPTY.md
- docs/docs/auto-docs/screens/OrgList/OrgListMocks/variables/MOCKS.md
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Test Application
- GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (1)
src/screens/OrgList/OrgListMocks.ts (1)
6-14
: LGTM! Import changes align with PostgreSQL migration.The new imports and type definitions appropriately reflect the migration from MongoDB to PostgreSQL endpoints.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🔭 Outside diff range comments (3)
src/screens/OrgList/OrgListMocks.ts (3)
Line range hint
159-175
: Fix empty response structure.The empty response structure doesn't match the query type. For
USER_JOINED_ORGANIZATIONS_PG
, the response should maintain the same structure as non-empty responses.result: { data: { - organizationsConnection: [], + user: { + organizationsWhereMember: { + pageInfo: { + hasNextPage: false, + hasPreviousPage: false, + startCursor: null, + endCursor: null + }, + edges: [] + } + } }, },
Line range hint
187-203
: Fix successful response structure in error mocks.The successful response structure in
MOCKS_WITH_ERROR
should match the query type.result: { data: { - organizationsConnection: organizations, + user: { + organizationsWhereMember: { + pageInfo: { + hasNextPage: false, + hasPreviousPage: false, + startCursor: null, + endCursor: null + }, + edges: organizations.map(org => ({ node: org })) + } + } }, },
Line range hint
218-255
: Migrate admin mocks to PostgreSQL queries.The
MOCKS_ADMIN
array still uses MongoDB queries (ORGANIZATION_CONNECTION_LIST
,USER_ORGANIZATION_LIST
). Update it to use the new PostgreSQL queries.const MOCKS_ADMIN = [ { request: { - query: ORGANIZATION_CONNECTION_LIST, + query: USER_JOINED_ORGANIZATIONS_PG, variables: { + id: '123', first: 8, skip: 0, filter: '', orderBy: 'createdAt_ASC', notifyOnNetworkStatusChange: true, }, }, result: { data: { - organizationsConnection: organizations, + user: { + organizationsWhereMember: { + pageInfo: { + hasNextPage: false, + hasPreviousPage: false, + startCursor: null, + endCursor: null + }, + edges: organizations.map(org => ({ node: org })) + } + } }, }, }, { request: { - query: USER_ORGANIZATION_LIST, + query: CURRENT_USER, variables: { userId: '123' }, }, result: { data: { user: adminUser }, }, }, ];
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docs/docs/auto-docs/screens/OrgList/OrgListMocks/variables/MOCKS.md
(1 hunks)docs/docs/auto-docs/screens/OrgList/OrgListMocks/variables/MOCKS_ADMIN.md
(1 hunks)docs/docs/auto-docs/screens/OrgList/OrgListMocks/variables/MOCKS_EMPTY.md
(1 hunks)docs/docs/auto-docs/screens/OrgList/OrgListMocks/variables/MOCKS_WITH_ERROR.md
(1 hunks)src/screens/OrgList/OrgList.spec.tsx
(5 hunks)src/screens/OrgList/OrgListMocks.ts
(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- docs/docs/auto-docs/screens/OrgList/OrgListMocks/variables/MOCKS_ADMIN.md
- docs/docs/auto-docs/screens/OrgList/OrgListMocks/variables/MOCKS_WITH_ERROR.md
- src/screens/OrgList/OrgList.spec.tsx
- docs/docs/auto-docs/screens/OrgList/OrgListMocks/variables/MOCKS_EMPTY.md
- docs/docs/auto-docs/screens/OrgList/OrgListMocks/variables/MOCKS.md
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Test Application
🔇 Additional comments (1)
src/screens/OrgList/OrgListMocks.ts (1)
31-41
: LGTM! Well-structured mock data.The organization mock data follows GraphQL connection patterns and provides a clear example of the expected PostgreSQL data structure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
src/screens/OrgList/OrgListMocks.ts (1)
Line range hint
200-238
: Update empty and error mock responses to match PostgreSQL schema.The empty and error mock responses still use MongoDB structure. Update them to match the PostgreSQL schema:
{ request: { query: USER_JOINED_ORGANIZATIONS_PG, variables: { first: 8, skip: 0, filter: '', orderBy: 'createdAt_ASC', }, notifyOnNetworkStatusChange: true, }, result: { data: { - organizationsConnection: [], + user: { + organizationsWhereMember: { + pageInfo: { + hasNextPage: false, + hasPreviousPage: false + }, + edges: [] + } + } }, }, },
🧹 Nitpick comments (3)
src/screens/OrgList/OrgListMocks.ts (1)
97-154
: Standardize pagination variables in mock data.The mock for
USER_JOINED_ORGANIZATIONS_PG
should include all pagination variables for complete test coverage:{ request: { query: USER_JOINED_ORGANIZATIONS_PG, variables: { id: '123', first: 8, + skip: 0, + filter: '', + orderBy: 'createdAt_ASC', }, }, result: { data: { user: { organizationsWhereMember: { pageInfo: { hasNextPage: true, + hasPreviousPage: false, + startCursor: "cursor1", + endCursor: "cursor2" },src/screens/OrgList/OrgList.tsx (2)
430-437
: Improve type annotation in organization mapping.Use a more specific type for the map callback:
- (item: InterfaceOrgConnectionInfoTypePG) => { + (item: { node: InterfaceOrgConnectionInfoTypePG }) => {
Line range hint
268-272
: Improve error handling and user experience.Consider adding more specific error handling:
if (errorList || errorUser) { + const error = errorList || errorUser; + if (error?.message?.includes('unauthorized')) { + errorHandler(t, 'Session expired. Please login again.'); + } else { errorHandler(t, errorList || errorUser); + } localStorage.clear(); window.location.assign('/'); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
docs/docs/auto-docs/screens/OrgList/OrgList/functions/default.md
(1 hunks)docs/docs/auto-docs/screens/OrgList/OrgListMocks/variables/MOCKS.md
(1 hunks)docs/docs/auto-docs/screens/OrgList/OrgListMocks/variables/MOCKS_ADMIN.md
(1 hunks)docs/docs/auto-docs/screens/OrgList/OrgListMocks/variables/MOCKS_EMPTY.md
(1 hunks)docs/docs/auto-docs/screens/OrgList/OrgListMocks/variables/MOCKS_WITH_ERROR.md
(1 hunks)src/screens/OrgList/OrgList.spec.tsx
(5 hunks)src/screens/OrgList/OrgList.tsx
(10 hunks)src/screens/OrgList/OrgListMocks.ts
(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- docs/docs/auto-docs/screens/OrgList/OrgList/functions/default.md
- docs/docs/auto-docs/screens/OrgList/OrgListMocks/variables/MOCKS_WITH_ERROR.md
- src/screens/OrgList/OrgList.spec.tsx
- docs/docs/auto-docs/screens/OrgList/OrgListMocks/variables/MOCKS.md
- docs/docs/auto-docs/screens/OrgList/OrgListMocks/variables/MOCKS_ADMIN.md
- docs/docs/auto-docs/screens/OrgList/OrgListMocks/variables/MOCKS_EMPTY.md
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Test Application
🔇 Additional comments (3)
src/screens/OrgList/OrgListMocks.ts (2)
31-41
: LGTM! Organization data structure aligns with PostgreSQL schema.The mock data structure correctly reflects the PostgreSQL schema with proper field names and nested edges for members.
6-10
: 🛠️ Refactor suggestionClean up query imports.
Since we're migrating to PostgreSQL, remove the MongoDB-specific query imports:
import { CURRENT_USER, - ORGANIZATION_CONNECTION_LIST, USER_JOINED_ORGANIZATIONS_PG, - USER_ORGANIZATION_LIST, } from 'GraphQl/Queries/Queries';Likely invalid or redundant comment.
src/screens/OrgList/OrgList.tsx (1)
141-154
: LGTM! Query and type changes align with PostgreSQL migration.The changes correctly:
- Use the new PostgreSQL query
USER_JOINED_ORGANIZATIONS_PG
- Update the type to match the new schema
- Handle the response data structure appropriately
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/screens/OrgList/OrgList.spec.tsx
(7 hunks)src/screens/OrgList/OrgList.tsx
(10 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/screens/OrgList/OrgList.spec.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Test Application
🔇 Additional comments (2)
src/screens/OrgList/OrgList.tsx (2)
141-154
: Verify query variables and pagination logic.The
USER_JOINED_ORGANIZATIONS_PG
query is missing pagination variables that were present in the old query:
- No
skip
parameter for offset- No
filter
for search- No
orderBy
for sorting✅ Verification successful
Review Verification: Pagination Variables in USER_JOINED_ORGANIZATIONS_PG
- The
USER_JOINED_ORGANIZATIONS_PG
query only acceptsid
andfirst
as variables.- Pagination is handled through the
first
parameter along withpageInfo.hasNextPage
, which is consistent with the simplified pagination requirements for a user's joined organizations.- The pagination variables
skip
,filter
, andorderBy
are available in the separateORGANIZATION_CONNECTION_LIST
query intended for organization lists that require filtering and sorting.Based on the inspection, the absence of additional pagination variables in
USER_JOINED_ORGANIZATIONS_PG
is intentional and appropriate for its use case.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the GraphQL schema supports these parameters ast-grep --pattern 'type Query { $$$ userJoinedOrganizationsPg($$$): $$$ $$$ }'Length of output: 87
Script:
#!/bin/bash # Search for the definition or usage of USER_JOINED_ORGANIZATIONS_PG in the codebase rg "USER_JOINED_ORGANIZATIONS_PG" -n .Length of output: 687
Script:
#!/bin/bash # Print the definition of USER_JOINED_ORGANIZATIONS_PG from its file to inspect its query parameters. rg "export const USER_JOINED_ORGANIZATIONS_PG" -n ./src/GraphQl/Queries/Queries.tsLength of output: 136
Script:
#!/bin/bash # Print out the entire GraphQL query definition for USER_JOINED_ORGANIZATIONS_PG from its file rg -A 50 "export const USER_JOINED_ORGANIZATIONS_PG = gql\`" ./src/GraphQl/Queries/Queries.tsLength of output: 1166
430-456
: 🛠️ Refactor suggestionSimplify organization rendering logic.
The current implementation has unnecessary complexity:
- Commented-out admin checks that are no longer needed
- Duplicate mapping logic for superAdmin vs regular users
- Type annotations that could be more specific
Apply this diff to simplify the rendering logic:
- {userData && superAdmin - ? orgsData?.edges.map( - (item: InterfaceOrgConnectionInfoTypePG) => { - return ( - <div - key={item.node.id} - className={styles.itemCardOrgList} - > - <OrgListCard data={item.node} /> - </div> - ); - }, - ) - : // userData && - // adminFor.length > 0 && - orgsData?.edges.map( - (item: InterfaceOrgConnectionInfoTypePG) => { - // if (isAdminForCurrentOrg(item)) { - return ( - <div - key={item.node.id} - className={styles.itemCardOrgList} - > - <OrgListCard data={item.node} /> - </div> - ); - // } - }, - )} + {orgsData?.edges.map( + (item: { node: InterfaceOrgConnectionInfoTypePG }) => ( + <div + key={item.node.id} + className={styles.itemCardOrgList} + > + <OrgListCard data={item.node} /> + </div> + ) + )}Likely invalid or redundant comment.
4d41ad3
into
PalisadoesFoundation:develop-postgres
|
Yes , starting to work on other components that require refactore |
What kind of change does this PR introduce?
Recently we have started to talawa admin to adapt to postgres engpoints. This PR Admins/administrators to Login
Issue Number:
Fixes #3476
Snapshots/Videos:
Screencast from 2025-01-29 08-48-00.webm
If relevant, did you update the documentation?
Summary
Does this PR introduce a breaking change?
No
Checklist
CodeRabbit AI Review
Test Coverage
Other information
Have you read the contributing guide?
Summary by CodeRabbit
Release Notes
New Features
SecuredRoute
component to validate user role handling.Bug Fixes
Refactor
Chores