feat(pg-migration): Add Postgres Migration Support for Funds & Fundraising Campaigns in orgDashboard #3591
Conversation
WalkthroughThis pull request updates several GraphQL mutations and queries along with React components and CSS. The changes rename mutation and query parameters, update type definitions, and restructure input objects across campaign and fund operations. In the UI components, adjustments include new validation logic, modified data mapping, and updated rendering identifiers. Additionally, pagination and error logging have been introduced in some queries, and a CSS adjustment increases the height of a drawer option list and refines its scrollbar styling. Changes
Possibly related PRs
Suggested labels
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (4)
src/style/app.module.css (4)
8497-8502: 🛠️ Refactor suggestionFix scrollbar styling for better cross-browser compatibility.
The
.optionListclass has inconsistent scrollbar styling across browsers:.optionList { height: 100%; overflow-y: hidden; - scrollbar-width: thin; - scrollbar-color: var(--table-bg) transparent; + /* Add webkit prefixes */ + -webkit-scrollbar-width: thin; + -webkit-scrollbar-color: var(--table-bg) transparent; + /* Standard syntax */ + scrollbar-width: thin; + scrollbar-color: var(--table-bg) transparent; transition: overflow 0.3s ease-in-out; }
1-9214:⚠️ Potential issueCritical: File size impacts performance.
The CSS file is over 9000 lines which can impact:
- Load time and performance
- Maintainability
- Code organization
Recommendations:
- Split into smaller component-specific modules
- Use CSS-in-JS or styled-components for better code organization
- Implement CSS code splitting
🧰 Tools
🪛 Biome (1.9.4)
[error] 2231-2231: Unexpected shorthand property padding after padding-bottom
(lint/suspicious/noShorthandPropertyOverrides)
[error] 5949-5949: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.(lint/correctness/noUnknownProperty)
525-532: 🛠️ Refactor suggestionConsolidate duplicate media queries.
Multiple identical media query breakpoints are scattered throughout the file. Consider consolidating them into a single location for better maintainability.
/* Create a dedicated media queries section */ @media screen and (max-width: 575.5px) { /* Combine all styles for this breakpoint */ .justifysp, .mainpageright, .btnsContainer { /* Common styles */ } }Also applies to: 1818-1829, 3405-3415
2184-2204: 🛠️ Refactor suggestionDeduplicate keyframe animations.
Multiple identical keyframe animations are defined. Consolidate them:
- @keyframes load8 { ... } - @-webkit-keyframes load8 { ... } + @keyframes load8 { + 0% { + transform: rotate(0deg); + } + 100% { + transform: rotate(360deg); + } + } + @-webkit-keyframes load8 { + 0% { + -webkit-transform: rotate(0deg); + } + 100% { + -webkit-transform: rotate(360deg); + } + }Also applies to: 3542-3564, 4492-4516, 4518-4542
🧹 Nitpick comments (15)
src/GraphQl/Queries/fundQueries.ts (4)
15-36: Consider making thefirst: 32argument more flexible.Using a hard-coded limit of 32 funds may hamper scalability. Exposing it as a query variable or referencing a global constant would improve maintainability and flexibility.
query FundsByOrganization($input: QueryOrganizationInput!, $limit: Int = 32) { organization(input: $input) { - funds(first: 32) { + funds(first: $limit) {
39-48: Fix potential docstring typo.The parameter
@params sratAtseems to be a misspelling ofstartAt:- * @params sratAt - The start date of the fund campaign to be fetched. + * @params startAt - The start date of the fund campaign to be fetched.
50-69: Parameterize the page size for campaigns.The query currently uses a fixed limit of 10 campaigns. Consider making it configurable or documented for easier pagination management.
72-91: Parameterize the page size for pledges.Similar to the previous query, the limit of 10 pledges can be parameterized or documented to handle larger data volumes gracefully.
src/GraphQl/Mutations/FundMutation.ts (3)
15-15: Use consistent docstring format.Consider including the other parameters (
name,organizationId) in the docstring for completeness. This helps avoid confusion for future contributors.
18-24: Address Prettier indentation errors.The static analysis hints recommend adding a newline or spacing for better readability. For instance:
createFund( - input: { # ✅ Ensure correct field names + input: { name: $name organizationId: $organizationId isTaxDeductible: $isTaxDeductible } ) {🧰 Tools
🪛 ESLint
[error] 18-18: Insert
⏎······(prettier/prettier)
29-29: Remove extra blank lines after the closing brace.Prettier also suggests deleting extra newlines for consistent formatting.
src/screens/OrganizationFundCampaign/CampaignModal.tsx (1)
186-192: Consider consolidating duplicate date validation logic.The date validation logic is duplicated in both create and update handlers. Consider extracting it into a reusable function.
+const validateCampaignDates = (startDate: Date, endDate: Date): string | null => { + if (dayjs(endDate).isBefore(dayjs())) { + return "End date must be in the future."; + } + if (dayjs(endDate).isSame(dayjs(startDate))) { + return "End date must be different from the start date."; + } + return null; +}; const createCampaignHandler = async (e: ChangeEvent<HTMLFormElement>) => { e.preventDefault(); - if (dayjs(formState.campaignEndDate).isBefore(dayjs())) { - toast.error("End date must be in the future."); - return; - } - if (dayjs(formState.campaignEndDate).isSame(dayjs(formState.campaignStartDate))) { - toast.error("End date must be different from the start date."); - return; + const error = validateCampaignDates(formState.campaignStartDate, formState.campaignEndDate); + if (error) { + toast.error(error); + return; }🧰 Tools
🪛 ESLint
[error] 188-188: Replace
"End·date·must·be·in·the·future."with'End·date·must·be·in·the·future.'(prettier/prettier)
src/screens/OrganizationFunds/OrganizationFunds.tsx (1)
158-160: Consider adding null checks for safer data access.The data access pattern could be more defensive to handle potential null values in the response.
const funds = useMemo(() => { - return fundData?.organization?.funds?.edges.map(edge => edge.node) ?? []; + return fundData?.organization?.funds?.edges?.map(edge => edge?.node).filter(Boolean) ?? []; }, [fundData]);🧰 Tools
🪛 ESLint
[error] 159-159: Replace
edgewith(edge)(prettier/prettier)
src/screens/OrganizationFundCampaign/OrganizationFundCampagins.tsx (2)
153-154: Remove development console logs before production.The added console logs are useful for development but should be removed before deploying to production.
- onCompleted: (data) => console.log("GraphQL Data Received:", data), - onError: (error) => console.error("GraphQL Error:", error), - console.log("compaignsData", compaignsData);Also applies to: 161-161
🧰 Tools
🪛 ESLint
[error] 153-153: Replace
"GraphQL·Data·Received:"with'GraphQL·Data·Received:'(prettier/prettier)
[error] 154-154: Replace
"GraphQL·Error:"with'GraphQL·Error:'(prettier/prettier)
157-160: Consider adding null checks for safer data access.Similar to the funds component, the data access pattern could be more defensive.
const compaignsData = useMemo(() => { - return campaignData?.fund?.campaigns?.edges.map(edge => edge.node) ?? []; + return campaignData?.fund?.campaigns?.edges?.map(edge => edge?.node).filter(Boolean) ?? []; }, [campaignData]);🧰 Tools
🪛 ESLint
[error] 158-158: Replace
edgewith(edge)(prettier/prettier)
src/screens/FundCampaignPledge/FundCampaignPledge.tsx (2)
361-367: Verify the implementation of the "donated" column.The "donated" column currently displays a hardcoded value of 0. This might indicate:
- An unimplemented feature
- Missing data mapping from the GraphQL query
Consider implementing the actual donation tracking if this is a required feature.
Would you like me to help implement the donation tracking feature or create an issue to track this task?
145-153: Run Prettier to fix formatting issues.The code has minor formatting inconsistencies that can be automatically fixed by running Prettier.
Run the following command to fix the formatting:
- useQuery(FUND_CAMPAIGN_PLEDGE, { - variables: { - input: { - id: fundCampaignId, - }, - pledgeOrderBy: sortBy, - }, - }); + useQuery(FUND_CAMPAIGN_PLEDGE, { + variables: { + input: { + id: fundCampaignId, + }, + pledgeOrderBy: sortBy, + }, + });🧰 Tools
🪛 ESLint
[error] 145-146: Delete
⏎···(prettier/prettier)
[error] 147-147: Delete
··(prettier/prettier)
[error] 148-148: Delete
··(prettier/prettier)
[error] 149-149: Replace
··········with········(prettier/prettier)
[error] 150-150: Delete
··(prettier/prettier)
[error] 151-151: Delete
··(prettier/prettier)
[error] 152-152: Delete
··(prettier/prettier)
[error] 153-153: Delete
··(prettier/prettier)
src/style/app.module.css (2)
497-500: Consider using CSS Grid for better layout control.The
.searchtitleclass uses fixed width which may not be responsive:- width: 60%; + display: grid; + grid-template-columns: minmax(0, 60%);
547-549: Optimize compound selectors for better performance.Multiple compound selectors using
:is()can impact performance. Consider simplifying:- .noOutline:is(:hover, :focus, :active, :focus-visible, .show) + .noOutline:hover, + .noOutline:focus, + .noOutline:active, + .noOutline:focus-visible, + .noOutline.showAlso applies to: 877-881, 930-934
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/GraphQl/Mutations/CampaignMutation.ts(2 hunks)src/GraphQl/Mutations/FundMutation.ts(1 hunks)src/GraphQl/Queries/fundQueries.ts(2 hunks)src/screens/FundCampaignPledge/FundCampaignPledge.tsx(1 hunks)src/screens/OrganizationFundCampaign/CampaignModal.tsx(3 hunks)src/screens/OrganizationFundCampaign/OrganizationFundCampagins.tsx(10 hunks)src/screens/OrganizationFunds/FundModal.tsx(3 hunks)src/screens/OrganizationFunds/OrganizationFunds.tsx(6 hunks)src/style/app.module.css(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
src/screens/OrganizationFunds/FundModal.tsx (2)
Learnt from: GlenDsza
PR: PalisadoesFoundation/talawa-admin#2064
File: src/screens/OrganizationFunds/OrganizationFunds.tsx:66-72
Timestamp: 2024-11-12T10:40:58.654Z
Learning: Renaming the Enum `Modal` to `ModalState` in the `organizationFunds` component is unnecessary and does not change the functionality.
Learnt from: GlenDsza
PR: PalisadoesFoundation/talawa-admin#2064
File: src/screens/OrganizationFunds/FundDeleteModal.tsx:1-2
Timestamp: 2024-11-12T10:41:04.351Z
Learning: The folder containing mutation queries in the codebase is named `GraphQl`, not `graphql`.
src/screens/OrganizationFunds/OrganizationFunds.tsx (1)
Learnt from: GlenDsza
PR: PalisadoesFoundation/talawa-admin#2064
File: src/screens/OrganizationFunds/FundDeleteModal.tsx:1-2
Timestamp: 2024-11-12T10:41:04.351Z
Learning: The folder containing mutation queries in the codebase is named `GraphQl`, not `graphql`.
🪛 ESLint
src/screens/OrganizationFunds/FundModal.tsx
[error] 138-139: Delete ⏎·····
(prettier/prettier)
src/GraphQl/Mutations/FundMutation.ts
[error] 18-18: Insert ⏎······
(prettier/prettier)
[error] 27-28: Delete ⏎
(prettier/prettier)
src/screens/FundCampaignPledge/FundCampaignPledge.tsx
[error] 145-146: Delete ⏎···
(prettier/prettier)
[error] 147-147: Delete ··
(prettier/prettier)
[error] 148-148: Delete ··
(prettier/prettier)
[error] 149-149: Replace ·········· with ········
(prettier/prettier)
[error] 150-150: Delete ··
(prettier/prettier)
[error] 151-151: Delete ··
(prettier/prettier)
[error] 152-152: Delete ··
(prettier/prettier)
[error] 153-153: Delete ··
(prettier/prettier)
src/screens/OrganizationFunds/OrganizationFunds.tsx
[error] 154-154: Insert ,
(prettier/prettier)
[error] 155-155: Insert ,
(prettier/prettier)
[error] 159-159: Replace edge with (edge)
(prettier/prettier)
src/screens/OrganizationFundCampaign/CampaignModal.tsx
[error] 110-110: Missing return type on function.
(@typescript-eslint/explicit-function-return-type)
[error] 115-115: Replace "End·date·must·be·in·the·future." with 'End·date·must·be·in·the·future.'
(prettier/prettier)
[error] 120-120: Replace dayjs(formState.campaignEndDate).isSame(dayjs(formState.campaignStartDate)) with ⏎······dayjs(formState.campaignEndDate).isSame(⏎········dayjs(formState.campaignStartDate),⏎······)⏎····
(prettier/prettier)
[error] 121-121: Replace "End·date·must·be·different·from·the·start·date." with 'End·date·must·be·different·from·the·start·date.'
(prettier/prettier)
[error] 165-165: Replace "End·date·must·be·in·the·future." with 'End·date·must·be·in·the·future.'
(prettier/prettier)
[error] 170-170: Replace dayjs(formState.campaignEndDate).isSame(dayjs(formState.campaignStartDate)) with ⏎······dayjs(formState.campaignEndDate).isSame(⏎········dayjs(formState.campaignStartDate),⏎······)⏎····
(prettier/prettier)
[error] 171-171: Replace "End·date·must·be·different·from·the·start·date." with 'End·date·must·be·different·from·the·start·date.'
(prettier/prettier)
[error] 188-188: Replace "End·date·must·be·in·the·future." with 'End·date·must·be·in·the·future.'
(prettier/prettier)
src/screens/OrganizationFundCampaign/OrganizationFundCampagins.tsx
[error] 150-150: Delete ·
(prettier/prettier)
[error] 153-153: Replace "GraphQL·Data·Received:" with 'GraphQL·Data·Received:'
(prettier/prettier)
[error] 154-154: Replace "GraphQL·Error:" with 'GraphQL·Error:'
(prettier/prettier)
[error] 158-158: Replace edge with (edge)
(prettier/prettier)
[error] 161-161: Replace "compaignsData" with 'compaignsData'
(prettier/prettier)
[error] 316-316: Replace ·handleOpenModal(params.row·as·InterfaceCampaignInfo,·'edit',) with ⏎············handleOpenModal(params.row·as·InterfaceCampaignInfo,·'edit')⏎··········
(prettier/prettier)
🪛 GitHub Actions: PR Workflow
src/screens/OrganizationFunds/FundModal.tsx
[warning] 1-1: Code style issues found. Run Prettier with --write to fix.
src/GraphQl/Mutations/FundMutation.ts
[warning] 1-1: Code style issues found. Run Prettier with --write to fix.
src/screens/FundCampaignPledge/FundCampaignPledge.tsx
[warning] 1-1: Code style issues found. Run Prettier with --write to fix.
src/screens/OrganizationFunds/OrganizationFunds.tsx
[warning] 1-1: Code style issues found. Run Prettier with --write to fix.
src/screens/OrganizationFundCampaign/CampaignModal.tsx
[warning] 1-1: Code style issues found. Run Prettier with --write to fix.
src/screens/OrganizationFundCampaign/OrganizationFundCampagins.tsx
[warning] 1-1: Code style issues found. Run Prettier with --write to fix.
src/GraphQl/Queries/fundQueries.ts
[error] 1-1: File contains eslint-disable statements. ESLint-disable check failed.
🔇 Additional comments (15)
src/GraphQl/Queries/fundQueries.ts (2)
8-11: Ensure docstring parameters align with the actual GraphQL input.The docstring references parameters (
name,creatorId,updaterId,isTaxDeductible) that may not separately appear in$input. Verify that these parameters match fields inQueryOrganizationInputor revise the docstring for clarity.
98-99: Verify front-end usage of newly returned fields.Fields
fundingGoal,startDate, andendDateare now returned bygetFundraisingCampaigns. Ensure front-end processes or UI components are updated to handle and display these fields properly.Also applies to: 102-104
src/GraphQl/Mutations/FundMutation.ts (2)
8-8: Confirm documentation matches the new parameter name.The docstring references
isTaxDeductible, which replacestaxDeductible. Ensure all references—such as front-end variables—are updated to maintain consistency.
39-42: Looks good.The updated
UpdateFundmutation is concise, aligns with the new input structure, and follows the naming conventions set for the updated schema.src/GraphQl/Mutations/CampaignMutation.ts (4)
8-11: Confirm parameter docstrings match your schema fields.The docstring now correctly reflects
goalAmount,startAt,endAt, andcurrencyCode. Double-check your GraphQL schema to verify these fields exist in the campaign input type.
16-31: Ensure switching fromFloattoIntforgoalAmountcovers your use cases.If your campaigns need partial funding amounts (e.g. 100.50), changing
goalAmounttoInt!might be restrictive. Verify currency decimals are not required.
44-47: Docstring alignment is good.The updated naming and type references in the docstring for campaign fields look consistent with the typed input.
52-54: Mutation signature update is clean and consistent.The
updateFundCampaignmutation now aligns neatly with the other updated GraphQL schemas. Good job!src/screens/OrganizationFunds/FundModal.tsx (1)
153-156: LGTM! Input object pattern improves mutation structure.The change to use an
inputobject for the mutation follows GraphQL best practices by encapsulating related fields.src/screens/OrganizationFundCampaign/CampaignModal.tsx (1)
114-123: LGTM! Added essential date validation.The added validation ensures that:
- End date is in the future
- End date differs from start date
This prevents invalid campaign date ranges.
🧰 Tools
🪛 ESLint
[error] 115-115: Replace
"End·date·must·be·in·the·future."with'End·date·must·be·in·the·future.'(prettier/prettier)
[error] 120-120: Replace
dayjs(formState.campaignEndDate).isSame(dayjs(formState.campaignStartDate))with⏎······dayjs(formState.campaignEndDate).isSame(⏎········dayjs(formState.campaignStartDate),⏎······)⏎····(prettier/prettier)
[error] 121-121: Replace
"End·date·must·be·different·from·the·start·date."with'End·date·must·be·different·from·the·start·date.'(prettier/prettier)
src/screens/OrganizationFunds/OrganizationFunds.tsx (1)
152-155: LGTM! Query structure follows GraphQL best practices.The change to use an
inputobject for the query parameters follows GraphQL best practices and improves maintainability.🧰 Tools
🪛 ESLint
[error] 154-154: Insert
,(prettier/prettier)
[error] 155-155: Insert
,(prettier/prettier)
src/screens/OrganizationFundCampaign/OrganizationFundCampagins.tsx (1)
214-222: LGTM! Improved render function readability.The simplified arrow function syntax for render functions improves code readability while maintaining functionality.
Also applies to: 310-321
src/screens/FundCampaignPledge/FundCampaignPledge.tsx (1)
145-153: LGTM! Query structure follows GraphQL best practices.The updated query structure using an
inputobject instead of awhereclause improves maintainability and follows modern GraphQL conventions.🧰 Tools
🪛 ESLint
[error] 145-146: Delete
⏎···(prettier/prettier)
[error] 147-147: Delete
··(prettier/prettier)
[error] 148-148: Delete
··(prettier/prettier)
[error] 149-149: Replace
··········with········(prettier/prettier)
[error] 150-150: Delete
··(prettier/prettier)
[error] 151-151: Delete
··(prettier/prettier)
[error] 152-152: Delete
··(prettier/prettier)
[error] 153-153: Delete
··(prettier/prettier)
src/style/app.module.css (2)
1-36: Excellent documentation of CSS methodology and guidelines.The comprehensive documentation at the start of the file provides clear guidance on:
- CSS reusability approach
- Naming conventions
- Examples of global classes
This helps maintain consistency across the codebase.
38-378: Excellent use of CSS variables for theming.The
:rootselector defines a comprehensive set of CSS variables for:
- Colors
- Spacing
- Typography
- Shadows
This enables consistent theming and easy customization.
src/GraphQl/Queries/fundQueries.ts
Outdated
| @@ -1,87 +1,89 @@ | |||
| /*eslint-disable*/ | |||
| /* eslint-disable */ | |||
There was a problem hiding this comment.
Remove or justify the eslint-disable to fix pipeline failure.
The pipeline flags this line for violating the ESLint-disable check. If there isn't a critical reason for disabling ESLint, removing this line ensures compliance with the style checks:
-/* eslint-disable */📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /* eslint-disable */ |
🧰 Tools
🪛 GitHub Actions: PR Workflow
[error] 1-1: File contains eslint-disable statements. ESLint-disable check failed.
| organizationId: orgId, | ||
| taxDeductible, | ||
| isTaxDeductible: taxDeductible, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the field name change from refrenceNumber to isTaxDeductible.
The field name change suggests a significant change in the data model. This could be a breaking change that affects other parts of the application.
Also applies to: 146-146
🏁 Script executed:
#!/bin/bash
# Search for any remaining usage of refrenceNumber
rg "refrenceNumber" --type ts --type tsxLength of output: 68
I'll re-run the search command using file globs (instead of relying on the unrecognized "tsx" file type) to check for any remaining occurrences of refrenceNumber. This will help ensure that the field name change has been consistently applied.
🏁 Script executed:
#!/bin/bash
rg "refrenceNumber" -g "*.ts" -g "*.tsx"Length of output: 1298
Inconsistent Field Name Usage Detected: "refrenceNumber" vs "isTaxDeductible".
- The change in FundModal.tsx (line 113) updated one instance to use
isTaxDeductible, yet other parts of the codebase—including FundModal.tsx itself (e.g.,fund?.refrenceNumber), the mocks in FundModal.spec.tsx, the GraphQL queries, and interface definitions in src/utils/interfaces.ts—still referencerefrenceNumber. - This inconsistency suggests that the field name update has not been propagated throughout the affected modules, which could indeed lead to breaking changes when interfacing with parts of the application expecting the old field name.
| currencyCode: formState.campaignCurrency, | ||
| goalAmount: parseInt(formState.campaignGoal, 10), | ||
| startAt: dayjs(formState.campaignStartDate).toISOString(), | ||
| endAt: dayjs(formState.campaignEndDate).toISOString(), | ||
| fundId, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the impact of renamed mutation variables.
The mutation variables have been renamed:
currency→currencyCodefundingGoal→goalAmountstartDate→startAtendDate→endAt
These changes might affect other components or API consumers.
🏁 Script executed:
#!/bin/bash
# Search for old variable names in the codebase
rg -A 2 "currency:|fundingGoal:|startDate:|endDate:" --type graphqlLength of output: 101
I'll generate a new shell script to search for the old mutation variable names specifically within GraphQL files (i.e. files ending with .graphql or .gql). This will help us verify if any references to the old names remain in the codebase.
🏁 Script executed:
#!/bin/bash
# Search for old mutation variable names in GraphQL files by using file globs.
rg -A 2 "currency:|fundingGoal:|startDate:|endDate:" -g "*.graphql"
rg -A 2 "currency:|fundingGoal:|startDate:|endDate:" -g "*.gql"Length of output: 1899
I'll initiate another search to see if any GraphQL operations in the codebase are still using the old variable names (e.g. fundingGoal) to determine potential impact on API consumers.
🏁 Script executed:
#!/bin/bash
# Search for any GraphQL operations referencing the old mutation variables.
rg "(fundingGoal:|startDate:|endDate:|currency:)" -n .Length of output: 52563
GraphQL Mutation Variable Mismatch: Update Needed
The UI now sends updated variable names (e.g., currencyCode, goalAmount, startAt, endAt), but our GraphQL mutations, schema, mocks, and tests still reference the old names (currency, fundingGoal, startDate, endDate). This inconsistency will likely cause payload mapping issues or mutation failures.
Action Items:
- Update GraphQL mutation definitions (and related schema) to use the new variable names.
- Revise tests and mocks to reflect these changes.
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
src/GraphQl/Queries/fundQueries.ts (2)
81-95:⚠️ Potential issueFix inconsistent field names.
The field names in this query are inconsistent with the mutations. Update them to match:
query GetFundraisingCampaigns( $where: CampaignWhereInput $campaignOrderBy: CampaignOrderByInput ) { getFundraisingCampaigns(where: $where, campaignOrderBy: $campaignOrderBy) { id name - currency - fundingGoal - startDate - endDate + currencyCode + goalAmount + startAt + endAt } }
97-122:⚠️ Potential issueFix inconsistent field names.
The field names in this query are inconsistent with other queries and mutations. Update them to match:
query GetPledgesByUserId( $userId: ID! $where: PledgeWhereInput $orderBy: PledgeOrderByInput ) { getPledgesByUserId(userId: $userId, where: $where, orderBy: $orderBy) { - _id + id amount - startDate - endDate + startAt + endAt campaign { - _id + id name - endDate + endAt } - currency + currencyCode users { - _id + id firstName lastName image } } }
🧹 Nitpick comments (3)
src/GraphQl/Queries/fundQueries.ts (3)
9-26: Consider enhancing pagination support.While adding pagination is good for performance, consider these improvements:
- Make the page size configurable via the input type
- Add cursor-based pagination support for efficient navigation through large datasets
query FundsByOrganization($input: QueryOrganizationInput!) { organization(input: $input) { - funds(first: 32) { + funds( + first: $input.pageSize + after: $input.after + before: $input.before + ) { edges { node { id name creator { id } createdAt } + cursor } + pageInfo { + hasNextPage + hasPreviousPage + startCursor + endCursor + } } } }
38-57: Consider enhancing pagination support for campaigns.Similar to the
FUND_LISTquery, consider making the page size configurable and adding cursor-based pagination support for campaigns.query GetFundById($input: QueryFundInput!) { fund(input: $input) { id name - campaigns(first: 10) { + campaigns( + first: $input.campaignsPageSize + after: $input.campaignsAfter + before: $input.campaignsBefore + ) { edges { node { id name startAt endAt currencyCode goalAmount } + cursor } + pageInfo { + hasNextPage + hasPreviousPage + startCursor + endCursor + } } } }
59-79: Consider enhancing pagination support for pledges.Similar to other queries, consider making the page size configurable and adding cursor-based pagination support for pledges.
query GetFundCampaignPledges($input: QueryFundCampaignInput!) { fundCampaign(input: $input) { id name - pledges(first: 10) { + pledges( + first: $input.pledgesPageSize + after: $input.pledgesAfter + before: $input.pledgesBefore + ) { edges { node { id amount note pledger { id name } } + cursor } + pageInfo { + hasNextPage + hasPreviousPage + startCursor + endCursor + } } } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/GraphQl/Mutations/CampaignMutation.ts(2 hunks)src/GraphQl/Mutations/FundMutation.ts(2 hunks)src/GraphQl/Queries/fundQueries.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/GraphQl/Mutations/FundMutation.ts
🧰 Additional context used
🪛 ESLint
src/GraphQl/Queries/fundQueries.ts
[error] 30-30: tsdoc-undefined-tag: The TSDoc tag "@params" is not defined in this configuration
(tsdoc/syntax)
[error] 31-31: tsdoc-undefined-tag: The TSDoc tag "@params" is not defined in this configuration
(tsdoc/syntax)
[error] 32-32: tsdoc-undefined-tag: The TSDoc tag "@params" is not defined in this configuration
(tsdoc/syntax)
[error] 33-33: tsdoc-undefined-tag: The TSDoc tag "@params" is not defined in this configuration
(tsdoc/syntax)
[error] 34-34: tsdoc-undefined-tag: The TSDoc tag "@params" is not defined in this configuration
(tsdoc/syntax)
[error] 35-35: tsdoc-undefined-tag: The TSDoc tag "@params" is not defined in this configuration
(tsdoc/syntax)
🔇 Additional comments (2)
src/GraphQl/Mutations/CampaignMutation.ts (2)
51-57: LGTM! Input type consolidation follows GraphQL best practices.The consolidation of parameters into a single input type (
MutationUpdateFundCampaignInput!) improves maintainability and follows GraphQL best practices.
15-37:✅ Verification successful
LGTM! Type changes improve precision and standardization.
The changes improve type safety and standardization:
- Using
Int!forgoalAmountis more appropriate for campaign goalsDateTime!provides more precise timestamps thanDate!Iso4217CurrencyCode!ensures standardized currency codes
🌐 Web query:
What is ISO 4217 and why is it used for currency codes?💡 Result:
ISO 4217 is an international standard that defines three-letter alphabetic codes (e.g., USD for US dollars) and three-digit numeric codes (e.g., 840 for USD) to uniquely identify currencies globally. Developed by the International Organization for Standardization (ISO), it ensures consistency in financial transactions, reduces ambiguity, and simplifies cross-border trade and banking operations[1][5][9].
Key Features of ISO 4217
Code Structure
- Alphabetic codes: Three letters (e.g., EUR for euros). The first two letters often align with the ISO 3166 country code (e.g., "US" for United States), while the third typically represents the currency’s initial letter[4][9].
- Numeric codes: Three digits (e.g., 978 for EUR), often matching the country’s ISO 3166-1 numeric code[1][9].
Scope
- Covers active currencies (e.g., JPY for Japanese yen), historical currencies (e.g., ATS for Austrian schilling), and special funds (e.g., XDR for IMF Special Drawing Rights)[1][5].
- Maintained by SIX Group on behalf of ISO[1].
Purpose and Usage
- Global Clarity: Eliminates confusion caused by similar currency names or symbols (e.g., distinguishing USD from AUD)[7][9].
- Financial Systems: Used in banking software, forex markets (e.g., EUR/USD pairs), and automated transactions where numeric codes streamline processing[9][10].
- Documentation: Found on airline tickets, international invoices, and financial reports to specify amounts unambiguously[1][5].
Historical Context
- Introduced in 1978 after collaboration between ISO Technical Committee 68 and UN/ECE experts[1][9].
- Updated regularly to reflect currency changes (e.g., redenominations like AZN replacing AZM for Azerbaijani manat)[1][3].
Examples of Active Codes
Code Numeric Currency Country/Region USD 840 US Dollar United States EUR 978 Euro Eurozone JPY 392 Japanese Yen Japan ISO 4217 remains indispensable for ensuring accuracy in international finance, trade, and data systems by providing a universal language for currencies[5][9].
Citations:
- 1: https://en.wikipedia.org/wiki/ISO_4217
- 2: https://www.iban.com/currency-codes
- 3: https://docs.oracle.com/cd/E15156_01/e1apps90pbr0/eng/psbooks/atpb/htm/atpb04.htm
- 4: https://www.dst.dk/en/Statistik/dokumentation/nomenklaturer/valuta-iso
- 5: https://www.iso.org/iso-4217-currency-codes.html
- 6: https://docs.1010data.com/1010dataReferenceManual/DataTypesAndFormats/currencyUnitCodes.html
- 7: https://currencysystem.com/codes/
- 8: https://www.youtube.com/watch?v=kwO5dnY-wLg
- 9: https://www.investopedia.com/terms/i/isocurrencycode.asp
- 10: https://www.newbridgefx.com/currency-codes-symbols/
LGTM! Type changes and naming conventions are spot-on.
- The updated parameter names and types improve consistency:
goalAmountasInt!is more suitable, as fractional campaign goals are uncommon.startAtandendAtasDateTime!provide improved timestamp precision.currencyCodeasIso4217CurrencyCode!leverages the ISO 4217 standard, ensuring globally recognized and unambiguous currency identifiers.- The mutation name and input changes further standardize the API design.
src/GraphQl/Queries/fundQueries.ts
Outdated
| /** | ||
| * Query to fetch a specific fund by its ID, along with its associated campaigns. | ||
| * @params id - The ID of the fund campaign to be fetched. | ||
| * @params name - The name of the fund campaign to be fetched. | ||
| * @params startAt - The start date of the fund campaign to be fetched. | ||
| * @params endAt - The end date of the fund campaign to be fetched. | ||
| * @params currencyCode - The currency code of the fund campaign to be fetched. | ||
| * @params goalAmount - The goal amount of the fund campaign to be fetched. | ||
| * @returns The fund campaign with the specified ID. | ||
| */ |
There was a problem hiding this comment.
Fix TSDoc syntax errors.
Replace @params with @param to fix the TSDoc syntax errors:
/**
* Query to fetch a specific fund by its ID, along with its associated campaigns.
- * @params id - The ID of the fund campaign to be fetched.
- * @params name - The name of the fund campaign to be fetched.
- * @params startAt - The start date of the fund campaign to be fetched.
- * @params endAt - The end date of the fund campaign to be fetched.
- * @params currencyCode - The currency code of the fund campaign to be fetched.
- * @params goalAmount - The goal amount of the fund campaign to be fetched.
+ * @param id - The ID of the fund campaign to be fetched.
+ * @param name - The name of the fund campaign to be fetched.
+ * @param startAt - The start date of the fund campaign to be fetched.
+ * @param endAt - The end date of the fund campaign to be fetched.
+ * @param currencyCode - The currency code of the fund campaign to be fetched.
+ * @param goalAmount - The goal amount of the fund campaign to be fetched.
* @returns The fund campaign with the specified ID.
*/📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Query to fetch a specific fund by its ID, along with its associated campaigns. | |
| * @params id - The ID of the fund campaign to be fetched. | |
| * @params name - The name of the fund campaign to be fetched. | |
| * @params startAt - The start date of the fund campaign to be fetched. | |
| * @params endAt - The end date of the fund campaign to be fetched. | |
| * @params currencyCode - The currency code of the fund campaign to be fetched. | |
| * @params goalAmount - The goal amount of the fund campaign to be fetched. | |
| * @returns The fund campaign with the specified ID. | |
| */ | |
| /** | |
| * Query to fetch a specific fund by its ID, along with its associated campaigns. | |
| * @param id - The ID of the fund campaign to be fetched. | |
| * @param name - The name of the fund campaign to be fetched. | |
| * @param startAt - The start date of the fund campaign to be fetched. | |
| * @param endAt - The end date of the fund campaign to be fetched. | |
| * @param currencyCode - The currency code of the fund campaign to be fetched. | |
| * @param goalAmount - The goal amount of the fund campaign to be fetched. | |
| * @returns The fund campaign with the specified ID. | |
| */ |
🧰 Tools
🪛 ESLint
[error] 30-30: tsdoc-undefined-tag: The TSDoc tag "@params" is not defined in this configuration
(tsdoc/syntax)
[error] 31-31: tsdoc-undefined-tag: The TSDoc tag "@params" is not defined in this configuration
(tsdoc/syntax)
[error] 32-32: tsdoc-undefined-tag: The TSDoc tag "@params" is not defined in this configuration
(tsdoc/syntax)
[error] 33-33: tsdoc-undefined-tag: The TSDoc tag "@params" is not defined in this configuration
(tsdoc/syntax)
[error] 34-34: tsdoc-undefined-tag: The TSDoc tag "@params" is not defined in this configuration
(tsdoc/syntax)
[error] 35-35: tsdoc-undefined-tag: The TSDoc tag "@params" is not defined in this configuration
(tsdoc/syntax)
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
src/GraphQl/Queries/fundQueries.ts (2)
81-95: 🛠️ Refactor suggestionStandardize date field naming across queries.
There's an inconsistency in date field naming between queries:
- FUND_CAMPAIGN uses
startAt/endAt- USER_FUND_CAMPAIGNS uses
startDate/endDateConsider standardizing the naming convention across all queries.
- fundingGoal - startDate - endDate + goalAmount + startAt + endAt
97-122: 🛠️ Refactor suggestionStandardize ID field naming across queries.
There's an inconsistency in ID field naming between queries:
- Other queries use
id- This query uses
_idConsider standardizing the ID field naming across all queries.
- _id + id - _id + id - _id + id
🧹 Nitpick comments (3)
src/GraphQl/Queries/fundQueries.ts (3)
9-26: Consider making the pagination limit configurable.The query implements pagination with a hardcoded limit of 32 items. Consider making this limit configurable through the input type to support different use cases.
- funds(first: 32) { + funds(first: $input.first) {
38-57: Consider making campaign pagination limit configurable.Similar to the previous query, the campaigns pagination limit is hardcoded to 10. Consider making this configurable through the input type.
- campaigns(first: 10) { + campaigns(first: $input.campaignsFirst) {
59-79: Consider making pledges pagination limit configurable.The pledges pagination limit is hardcoded to 10. Consider making this configurable through the input type for consistency with other queries.
- pledges(first: 10) { + pledges(first: $input.pledgesFirst) {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/GraphQl/Queries/fundQueries.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (2)
src/GraphQl/Queries/fundQueries.ts (2)
3-8: LGTM! Clear and accurate documentation.The documentation clearly describes the purpose and parameters of the query.
28-37: LGTM! Well-documented query parameters.The documentation clearly describes all parameters using correct TSDoc syntax.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/screens/OrganizationFunds/OrganizationFunds.tsx (1)
235-235: Consider displaying user-friendly creator information.Currently, only the creator's ID is displayed. Consider showing a more user-friendly representation (e.g., name) for better UX.
- return params.row.creator.id; + return params.row.creator.name ?? params.row.creator.id;src/utils/interfaces.ts (1)
1033-1042: Fix spacing in creator object definition.There's a missing space after the colon in the creator object definition.
- creator: { id: string}; + creator: { id: string };🧰 Tools
🪛 ESLint
[error] 1041-1041: Insert
·(prettier/prettier)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/screens/OrganizationFunds/OrganizationFunds.tsx(6 hunks)src/utils/interfaces.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/screens/OrganizationFunds/OrganizationFunds.tsx (1)
Learnt from: GlenDsza
PR: PalisadoesFoundation/talawa-admin#2064
File: src/screens/OrganizationFunds/FundDeleteModal.tsx:1-2
Timestamp: 2024-11-12T10:41:04.351Z
Learning: The folder containing mutation queries in the codebase is named `GraphQl`, not `graphql`.
🪛 ESLint
src/utils/interfaces.ts
[error] 1041-1041: Insert ·
(prettier/prettier)
🪛 GitHub Actions: PR Workflow
src/utils/interfaces.ts
[warning] 1-1: Code style issues found in the above file. Run Prettier with --write to fix.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (5)
src/screens/OrganizationFunds/OrganizationFunds.tsx (3)
144-152: LGTM! GraphQL query structure updated to use input object.The changes align with the PR objectives to support PostgreSQL migrations by updating the GraphQL query structure to use a standardized input object pattern.
Also applies to: 158-161
164-166: LGTM! Data access pattern updated to use edges/nodes pattern.The change to use the edges/nodes pattern is a good improvement as it:
- Follows GraphQL best practices for connection types
- Enables better pagination support
- Provides a consistent pattern across the application
368-368: LGTM! Row identification updated to use standardidfield.The changes to use
idinstead of_idalign with the PostgreSQL migration objectives and provide a more consistent approach to entity identification.Also applies to: 380-380
src/utils/interfaces.ts (2)
995-1011: LGTM! Updated interface to support GraphQL connections pattern.The changes to
InterfaceQueryOrganizationFundCampaignsproperly implement the GraphQL connections pattern with edges and nodes, which is a best practice for handling paginated lists.
1044-1051: LGTM! Updated campaign interface with PostgreSQL-compatible field names.The changes to field names (e.g.,
startAt,endAt,currencyCode,goalAmount) align with PostgreSQL naming conventions and provide better clarity.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/utils/interfaces.ts(2 hunks)
🔇 Additional comments (2)
src/utils/interfaces.ts (2)
995-1011: Well-structured interface update!The changes improve the interface by:
- Adopting the GraphQL edges/node pattern for better pagination support
- Standardizing field names to align with PostgreSQL conventions
- Maintaining consistency with other PG interfaces
1043-1051: Strong type definitions!The interface updates improve type safety and consistency by:
- Using proper types (number for goalAmount, Date for startAt/endAt)
- Following PostgreSQL naming conventions
- Maintaining consistency with fund-related interfaces
| export interface InterfaceFundInfo { | ||
| _id: string; | ||
| id: string; | ||
| name: string; | ||
| refrenceNumber: string; | ||
| taxDeductible: boolean; | ||
| isArchived: boolean; | ||
| isDefault: boolean; | ||
| createdAt: string; | ||
| organizationId: string; | ||
| creator: { _id: string; firstName: string; lastName: string }; | ||
| creator: { id: string }; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the impact of simplified creator object.
The creator object has been simplified to only include the id field. Ensure that any UI components or logic that previously relied on firstName and lastName fields have been updated accordingly.
🏁 Script executed:
#!/bin/bash
# Search for usage of creator.firstName or creator.lastName in fund-related components
rg -A 5 'creator\.(firstName|lastName)' src/Length of output: 8027
Action Required: Update Creator Object Usage in UI Components
The simplification of the creator object in InterfaceFundInfo to only include the id field now conflicts with multiple UI components still relying on creator.firstName and creator.lastName for display. In the following files, references to these fields were found:
- src/screens/EventVolunteers/VolunteerGroups/VolunteerGroupViewModal.tsx
- src/screens/OrgPost/OrgPost.tsx
- src/screens/UserPortal/Posts/Posts.tsx
- src/components/UsersTableItem/UsersTableItem.tsx
- src/components/OrgSettings/ActionItemCategories/OrgActionItemCategories.tsx
- src/components/UserPortal/CommentCard/CommentCard.tsx
- src/components/UserPortal/EventCard/EventCard.tsx
- src/components/UserPortal/PostCard/PostCard.tsx
Please verify and update these components so they no longer rely on the removed firstName and lastName fields—or alternatively, ensure that the necessary details are fetched from the correct source. This update is essential to align the UI with the new interface structure and avoid runtime errors or missing information.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/screens/OrganizationFundCampaign/CampaignModal.spec.tsx (1)
115-123: Update translation keys to match new property names.The test assertions are still using old translation keys (e.g.,
translations.fundingGoal) while the properties have been renamed (e.g.,goalAmount). Consider updating the translation keys to maintain consistency.Also applies to: 183-184, 209-210
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/screens/OrganizationFundCampaign/CampaignModal.spec.tsx(2 hunks)src/screens/OrganizationFunds/FundModal.spec.tsx(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/screens/OrganizationFunds/FundModal.spec.tsx (1)
Learnt from: GlenDsza
PR: PalisadoesFoundation/talawa-admin#2064
File: src/screens/OrganizationFunds/OrganizationFunds.tsx:66-72
Timestamp: 2024-11-12T10:40:58.654Z
Learning: Renaming the Enum `Modal` to `ModalState` in the `organizationFunds` component is unnecessary and does not change the functionality.
🪛 GitHub Actions: PR Workflow
src/screens/OrganizationFunds/FundModal.spec.tsx
[error] 55-55: Object literal may only specify known properties, and 'lastName' does not exist in type '{ id: string; }'.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (1)
src/screens/OrganizationFundCampaign/CampaignModal.spec.tsx (1)
54-61: LGTM! Property names updated consistently.The campaign object properties have been renamed to follow PostgreSQL naming conventions while maintaining consistency across create and edit modes.
Also applies to: 71-78
| creator: { | ||
| _id: 'creatorId1', | ||
| firstName: 'John', | ||
| id: 'creatorId1', | ||
| // firstName: 'John', | ||
| lastName: 'Doe', | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Fix type error in creator object.
The pipeline is failing due to a type mismatch in the creator object. The type definition only allows an id property, but the test data includes lastName and a commented-out firstName.
Either:
- Update the creator type definition to include
lastNameandfirstName, or - Remove the extra properties if they're no longer needed in the tests
If the creator's name properties are still needed, apply this diff:
creator: {
id: 'creatorId1',
- // firstName: 'John',
- lastName: 'Doe',
},Also applies to: 74-78
🧰 Tools
🪛 GitHub Check: Performs linting, formatting, type-checking, checking for different source and target branch
[failure] 55-55:
Object literal may only specify known properties, and 'lastName' does not exist in type '{ id: string; }'.
🪛 GitHub Actions: PR Workflow
[error] 55-55: Object literal may only specify known properties, and 'lastName' does not exist in type '{ id: string; }'.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
src/screens/OrganizationFunds/FundModal.tsx (4)
104-113: Simplify mutation variables and ensure consistent field naming.
- The destructured variables
isDefaultandisArchivedare passed to the mutation but could be removed from the destructuring since they're not used in the form anymore.- The field naming is inconsistent:
taxDeductiblein form state vsisTaxDeductiblein mutation.Apply this diff to improve consistency and remove unused variables:
- const { fundName, isDefault, taxDeductible, isArchived } = formState; + const { fundName, taxDeductible } = formState; try { await createFund({ variables: { name: fundName, organizationId: orgId, - isTaxDeductible: taxDeductible, - isArchived, - isDefault, + taxDeductible, }, });
141-141: Ensure consistent field naming.The field name
isTaxDeductiblein the mutation doesn't match thetaxDeductiblefield in the form state.Apply this diff:
- updatedFields.isTaxDeductible = taxDeductible; + updatedFields.taxDeductible = taxDeductible;
148-151: Remove unnecessary optional chaining in edit mode.Since this handler is only called in edit mode,
fundshould never be null, making the optional chaining onfund?.idunnecessary.Apply this diff:
input: { - id: fund?.id, + id: fund.id, ...updatedFields, },
213-219: Remove commented code.Instead of keeping commented code, it should be removed if it's no longer needed. If the fund ID field is meant to be read-only, consider displaying it as text instead of a disabled input field.
Apply this diff:
- // value={formState.fundRef} - // onChange={(e) => - // setFormState({ - // ...formState, - // fundRef: e.target.value, - // }) - // } + value={fund?.id ?? ''} + disabledsrc/utils/interfaces.ts (1)
1146-1146: Remove commented code.Instead of commenting out the
fundReffield, it should be removed if it's no longer needed.- // fundRef: string;
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/screens/OrganizationFundCampaign/CampaignModal.tsx(4 hunks)src/screens/OrganizationFundCampaign/OrganizationFundCampagins.tsx(13 hunks)src/screens/OrganizationFunds/FundModal.spec.tsx(4 hunks)src/screens/OrganizationFunds/FundModal.tsx(4 hunks)src/screens/OrganizationFunds/OrganizationFunds.tsx(8 hunks)src/utils/interfaces.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/screens/OrganizationFunds/FundModal.spec.tsx
- src/screens/OrganizationFundCampaign/CampaignModal.tsx
- src/screens/OrganizationFundCampaign/OrganizationFundCampagins.tsx
🧰 Additional context used
🧠 Learnings (2)
src/screens/OrganizationFunds/OrganizationFunds.tsx (1)
Learnt from: GlenDsza
PR: PalisadoesFoundation/talawa-admin#2064
File: src/screens/OrganizationFunds/FundDeleteModal.tsx:1-2
Timestamp: 2024-11-12T10:41:04.351Z
Learning: The folder containing mutation queries in the codebase is named `GraphQl`, not `graphql`.
src/screens/OrganizationFunds/FundModal.tsx (2)
Learnt from: GlenDsza
PR: PalisadoesFoundation/talawa-admin#2064
File: src/screens/OrganizationFunds/OrganizationFunds.tsx:66-72
Timestamp: 2024-11-12T10:40:58.654Z
Learning: Renaming the Enum `Modal` to `ModalState` in the `organizationFunds` component is unnecessary and does not change the functionality.
Learnt from: GlenDsza
PR: PalisadoesFoundation/talawa-admin#2064
File: src/screens/OrganizationFunds/FundDeleteModal.tsx:1-2
Timestamp: 2024-11-12T10:41:04.351Z
Learning: The folder containing mutation queries in the codebase is named `GraphQl`, not `graphql`.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (4)
src/utils/interfaces.ts (2)
995-1011: LGTM! Interface updated to support cursor-based pagination.The changes improve the interface by:
- Implementing the edges/node pattern for better pagination support
- Standardizing field names to align with PostgreSQL conventions
- Following GraphQL best practices for connection types
1032-1042: Update UI components to handle simplified creator object.The creator object has been simplified to only include the
idfield, which affects UI components that rely onfirstNameandlastNamefields.src/screens/OrganizationFunds/OrganizationFunds.tsx (2)
145-151: Well-structured GraphQL query implementation!The changes follow GraphQL best practices by using a standardized input object pattern and implementing the Relay connection specification with edges/nodes. This provides better type safety and scalability.
Also applies to: 158-160, 164-166
218-218: Good standardization of identifier usage!The changes consistently use
idinstead of_id, which aligns with PostgreSQL conventions. The simplified row handling improves maintainability.Also applies to: 310-310, 368-368, 380-380
| export interface InterfaceCampaignInfo { | ||
| _id: string; | ||
| id: string; | ||
| name: string; | ||
| fundingGoal: number; | ||
| startDate: Date; | ||
| endDate: Date; | ||
| goalAmount: number; | ||
| startAt: Date; | ||
| endAt: Date; | ||
| createdAt: string; | ||
| currency: string; | ||
| currencyCode: string; | ||
| } |
There was a problem hiding this comment.
Update InterfaceUserCampaign to match new field names.
The field names in InterfaceCampaignInfo have been standardized, but InterfaceUserCampaign (lines 1013-1020) still uses the old field names. This inconsistency should be resolved.
Apply this diff to update InterfaceUserCampaign:
export interface InterfaceUserCampaign {
- _id: string;
+ id: string;
name: string;
- fundingGoal: number;
- startDate: Date;
- endDate: Date;
- currency: string;
+ goalAmount: number;
+ startAt: Date;
+ endAt: Date;
+ currencyCode: string;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export interface InterfaceCampaignInfo { | |
| _id: string; | |
| id: string; | |
| name: string; | |
| fundingGoal: number; | |
| startDate: Date; | |
| endDate: Date; | |
| goalAmount: number; | |
| startAt: Date; | |
| endAt: Date; | |
| createdAt: string; | |
| currency: string; | |
| currencyCode: string; | |
| } | |
| export interface InterfaceUserCampaign { | |
| id: string; | |
| name: string; | |
| goalAmount: number; | |
| startAt: Date; | |
| endAt: Date; | |
| currencyCode: string; | |
| } |
| @@ -116,7 +116,7 @@ | |||
| } | |||
|
|
|||
| const [fund, setFund] = useState<InterfaceFundInfo | null>(null); | |||
| const [searchTerm, setSearchTerm] = useState<string>(''); | |||
| // const [searchTerm, setSearchTerm] = useState<string>(''); | |||
There was a problem hiding this comment.
Remove or implement the search functionality.
The search bar is rendered but non-functional due to commented-out code. This creates a confusing user experience.
Either:
- Remove the search functionality completely:
- // const [searchTerm, setSearchTerm] = useState<string>('');
- <SearchBar
- placeholder={tCommon('searchByName')}
- // onSearch={setSearchTerm}
- inputTestId="searchByName"
- buttonTestId="searchBtn"
- />- Or implement the search functionality properly by uncommenting and updating the code to work with the new data structure.
Also applies to: 327-327
| @@ -224,7 +232,7 @@ | |||
| sortable: false, | |||
| headerClassName: `${styles.tableHeader}`, | |||
| renderCell: (params: GridCellParams) => { | |||
| return params.row.creator.firstName + ' ' + params.row.creator.lastName; | |||
| return params.row.creator.id; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance creator information display.
Displaying only the creator's ID reduces readability. Consider showing both the ID and name for better user experience.
- return params.row.creator.id;
+ const creator = params.row.creator;
+ return (
+ <div title={creator.id}>
+ {creator.firstName} {creator.lastName}
+ </div>
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return params.row.creator.id; | |
| const creator = params.row.creator; | |
| return ( | |
| <div title={creator.id}> | |
| {creator.firstName} {creator.lastName} | |
| </div> | |
| ); |
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/screens/OrganizationFundCampaign/OrganizationFundCampagins.tsx (1)
119-119:⚠️ Potential issueFix unused variable warning.
The
searchTermstate is declared but never used in the component, causing a linter error.Either implement the search functionality or remove the unused state:
- const [searchTerm, setSearchTerm] = useState('');🧰 Tools
🪛 ESLint
[error] 119-119: 'searchTerm' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
🪛 GitHub Check: Performs linting, formatting, type-checking, checking for different source and target branch
[failure] 119-119:
'searchTerm' is assigned a value but never used🪛 GitHub Actions: PR Workflow
[error] 119-119: 'searchTerm' is assigned a value but never used @typescript-eslint/no-unused-vars
🧹 Nitpick comments (3)
src/screens/OrganizationFundCampaign/OrganizationFundCampagins.tsx (1)
153-154: Consider environment-based logging.The GraphQL callbacks are logging potentially sensitive data to the console. Consider using environment-based logging to prevent exposing sensitive information in production.
- onCompleted: (data) => console.log('GraphQL Data Received:', data), - onError: (error) => console.error('GraphQL Error:', error), + onCompleted: (data) => { + if (process.env.NODE_ENV === 'development') { + console.log('GraphQL Data Received:', data); + } + }, + onError: (error) => { + if (process.env.NODE_ENV === 'development') { + console.error('GraphQL Error:', error); + } + },src/screens/OrganizationFunds/OrganizationFunds.tsx (2)
164-166: Enhance type safety in data transformation.While the current implementation is correct, we can improve type safety and readability.
- const funds = useMemo(() => { - return fundData?.organization?.funds?.edges.map((edge) => edge.node) ?? []; - }, [fundData]); + const funds = useMemo(() => { + if (!fundData?.organization?.funds?.edges) return []; + return fundData.organization.funds.edges.map((edge) => edge.node); + }, [fundData]);
283-283: Remove commented-out code.The commented-out className
me-2 roundedshould either be removed or implemented.- // className="me-2 rounded"
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/screens/OrganizationFundCampaign/OrganizationFundCampagins.tsx(11 hunks)src/screens/OrganizationFunds/OrganizationFunds.tsx(6 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/screens/OrganizationFunds/OrganizationFunds.tsx (1)
Learnt from: GlenDsza
PR: PalisadoesFoundation/talawa-admin#2064
File: src/screens/OrganizationFunds/FundDeleteModal.tsx:1-2
Timestamp: 2024-11-12T10:41:04.351Z
Learning: The folder containing mutation queries in the codebase is named `GraphQl`, not `graphql`.
🪛 GitHub Actions: PR Workflow
src/screens/OrganizationFundCampaign/OrganizationFundCampagins.tsx
[error] 119-119: 'searchTerm' is assigned a value but never used @typescript-eslint/no-unused-vars
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (6)
src/screens/OrganizationFundCampaign/OrganizationFundCampagins.tsx (3)
157-159: LGTM! Good use of useMemo for performance optimization.The useMemo hook effectively prevents unnecessary recalculations of the campaigns data.
212-221: LGTM! Improved renderCell implementations.The simplified arrow functions and direct data access patterns improve code readability and reduce the chance of null pointer errors.
Also applies to: 230-232
373-378:❓ Verification inconclusive
Verify sorting functionality with the new data structure.
The sorting options reference fields like
fundingGoalbut the data structure usesgoalAmount. Additionally, thesortBystate doesn't seem to affect the data display.Also applies to: 388-396
🏁 Script executed:
#!/bin/bash # Check if the sorting field names match the GraphQL schema ast-grep --pattern 'type Campaign { $$$ goalAmount: Float $$$ }'Length of output: 72
Action Required: Verify Sorting Functionality with New Schema Fields
- The component still uses sorting options based on
fundingGoal(e.g.,fundingGoal_ASC/DESC), yet the GraphQL schema appears to define this field asgoalAmount. This mismatch means that the current sorting state (sortBy) may not properly influence data fetching or display.- Please manually verify that the GraphQL schema has been updated to use
goalAmountfor campaigns and update the sorting options accordingly.- Also, check lines 388–396 to ensure that the query construction correctly incorporates the new field names.
src/screens/OrganizationFunds/OrganizationFunds.tsx (3)
145-151: LGTM! GraphQL structure improvements.The changes follow GraphQL best practices by:
- Using a single input object for better maintainability
- Following the Relay specification with edges/nodes pattern for pagination support
Also applies to: 158-160
235-235: Enhance creator information display.Displaying only the creator's ID reduces readability. Consider showing both the ID and name for better user experience.
325-330:❓ Verification inconclusive
Verify search functionality with new data structure.
The search implementation needs verification with the updated GraphQL response structure.
🏁 Script executed:
#!/bin/bash # Check if the search functionality is properly integrated with the GraphQL query ast-grep --pattern 'const { data: fundData } = useQuery(FUND_LIST, { variables: { input: { $$$ searchTerm: $_, $$$ } } })'Length of output: 154
Attention: Manual Verification Required for Search Integration with New Data Structure
The automated check did not find the expected GraphQL query pattern incorporating the search term, so it's unclear whether the search functionality fully integrates with the updated GraphQL response structure. Please verify manually that:
- The search term provided by the SearchBar component is correctly mapped into the query variables.
- The GraphQL query handling (e.g., FUND_LIST) correctly processes the new data structure.
If discrepancies are found, update the GraphQL integration accordingly or adjust the query pattern as needed.
|
@palisadoes and @gautam-divyanshu Sir, I'm facing an error after modifying the FUND_LIST query to remove lastName and firstName. This change is affecting other sections, such as chat, due to dependencies on the organization fund. How can I resolve these dependency issues? I believe all the errors are similar. |
|
The API no longer supports firstName and lastName. Please update the code here appropriately |
|
|
|
@palisadoes Sir, I have already removed these variables. This error is occurring due to another query or mutation. Could you please guide me how i fix this issue? |
|
Look at the file that's generating the error. See whether the error is applicable |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/screens/OrganizationFunds/OrganizationFunds.tsx (1)
168-173: Consider implementing server-side filtering and pagination.The current implementation performs filtering on the client side, which may not scale well with large datasets. Since the GraphQL schema already uses the edges/nodes pattern, it's designed for pagination.
Consider updating the query to include:
- Server-side filtering
- Pagination support
- // Write now we use the search term in client side to filter the funds, we can use the search term in the query to filter the funds - const filteredFunds = useMemo(() => { - return funds.filter((fund) => - fund.name.toLowerCase().includes(searchTerm.toLowerCase()), - ); - }, [funds, searchTerm]); + const { data: fundData, fetchMore } = useQuery(FUND_LIST, { + variables: { + input: { + id: orgId, + filter: searchTerm, + first: 10, + }, + }, + });src/screens/OrganizationFunds/OrganizationFunds.spec.tsx (4)
120-128: Consider enhancing error handling test coverage.The test verifies error message display, but could be enhanced to also verify:
- Error toast notification
- Error state in the UI (disabled buttons, etc.)
it('should display error message when GraphQL query fails', async () => { vi.mocked(useParams).mockReturnValue({ orgId: 'orgId' }); renderOrganizationFunds(link2); await waitFor(() => { expect(screen.getByTestId('errorMsg')).toBeInTheDocument(); expect(screen.getByText('Mock graphql error')).toBeInTheDocument(); + expect(screen.getByTestId('createFundBtn')).toBeDisabled(); + expect(toast.error).toHaveBeenCalledWith('Mock graphql error'); }); });
130-148: Consider enhancing modal content verification.The test verifies modal opening/closing but could be enhanced to verify:
- Form field presence
- Initial field values
- Modal title and description
it('should open and close the Create Fund modal', async () => { vi.mocked(useParams).mockReturnValue({ orgId: 'orgId' }); renderOrganizationFunds(link1); const createFundBtn = await screen.findByTestId('createFundBtn'); expect(createFundBtn).toBeInTheDocument(); await userEvent.click(createFundBtn); await waitFor(() => { expect(screen.getByText(translations.fundCreate)).toBeInTheDocument(); + expect(screen.getByTestId('fundName')).toBeInTheDocument(); + expect(screen.getByTestId('fundDescription')).toBeInTheDocument(); + expect(screen.getByTestId('goalAmount')).toBeInTheDocument(); + expect(screen.getByTestId('startAt')).toBeInTheDocument(); }); const closeModalBtn = await screen.findByTestId('fundModalCloseBtn'); await userEvent.click(closeModalBtn); await waitFor(() => { expect(screen.queryByTestId('fundModalCloseBtn')).toBeNull(); + expect(screen.queryByTestId('fundName')).toBeNull(); }); });
170-183: Consider adding edge cases to search functionality test.The test could be enhanced to verify:
- Empty search results
- Special characters handling
- Case sensitivity
it('should filter funds by search term', async () => { vi.mocked(useParams).mockReturnValue({ orgId: 'orgId' }); renderOrganizationFunds(link1); const searchField = await screen.findByTestId('searchByName'); fireEvent.change(searchField, { target: { value: '2' } }); fireEvent.click(screen.getByTestId('searchBtn')); await waitFor(() => { expect(screen.getByText('Fund 2')).toBeInTheDocument(); expect(screen.queryByText('Fund 1')).toBeNull(); }); + + // Test empty search results + fireEvent.change(searchField, { target: { value: 'NonExistentFund' } }); + fireEvent.click(screen.getByTestId('searchBtn')); + await waitFor(() => { + expect(screen.getByText(translations.noFundsFound)).toBeInTheDocument(); + }); + + // Test case insensitivity + fireEvent.change(searchField, { target: { value: 'fund' } }); + fireEvent.click(screen.getByTestId('searchBtn')); + await waitFor(() => { + expect(screen.getAllByTestId('fundName')).toHaveLength(2); + }); });
255-261: Consider enhancing empty state test coverage.The test verifies the empty state message but could be enhanced to verify:
- Create fund button availability
- Search functionality behavior
- Sort functionality behavior
it('renders the empty fund component', async () => { vi.mocked(useParams).mockReturnValue({ orgId: 'orgId' }); renderOrganizationFunds(link3); await waitFor(() => expect(screen.getByText(translations.noFundsFound)).toBeInTheDocument(), ); + + // Verify create fund button is enabled + expect(screen.getByTestId('createFundBtn')).toBeEnabled(); + + // Verify search is disabled + expect(screen.getByTestId('searchByName')).toBeDisabled(); + + // Verify sort is disabled + expect(screen.getByTestId('filter')).toBeDisabled(); });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/screens/OrganizationFundCampaign/OrganizationFundCampagins.tsx(11 hunks)src/screens/OrganizationFunds/OrganizationFunds.spec.tsx(3 hunks)src/screens/OrganizationFunds/OrganizationFunds.tsx(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/screens/OrganizationFundCampaign/OrganizationFundCampagins.tsx
🧰 Additional context used
🧠 Learnings (1)
src/screens/OrganizationFunds/OrganizationFunds.tsx (1)
Learnt from: GlenDsza
PR: PalisadoesFoundation/talawa-admin#2064
File: src/screens/OrganizationFunds/FundDeleteModal.tsx:1-2
Timestamp: 2024-11-12T10:41:04.351Z
Learning: The folder containing mutation queries in the codebase is named `GraphQl`, not `graphql`.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Test Application
🔇 Additional comments (3)
src/screens/OrganizationFunds/OrganizationFunds.tsx (3)
225-226: LGTM! Consistent use ofidfield.The changes consistently use the
idfield instead of_idacross row identification and navigation logic.Also applies to: 317-318, 375-376
241-243: Enhance creator information display.Displaying only the creator's ID reduces readability. Consider showing both the ID and name for better user experience.
- return params.row.creator.id; + const creator = params.row.creator; + return ( + <div title={creator.id}> + {creator.firstName} {creator.lastName} + </div> + );
332-337: LGTM! Search functionality properly implemented.The search bar is now fully functional, providing a better user experience.
|
Ok sir |
What kind of change does this PR introduce?
Issue Number:
Fixes #3561
Snapshots/Videos:
(https://github.com/user-attachments/assets/ff0d25f7-3177-4a14-b30c-057cfca974f7)
If relevant, did you update the documentation?
Summary
Does this PR introduce a breaking change?
Checklist
CodeRabbit AI Review
Test Coverage
Other information
Have you read the contributing guide?
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Style