Added dynamic breadcrumbs & Removed support for Removing Fund/Campaigns #2248
Conversation
WalkthroughThe changes involve the removal of GraphQL mutations for deleting campaigns and funds, reflecting a shift in functionality towards managing campaigns and funds without deletion capabilities. Additionally, enhancements were made to GraphQL queries to provide more detailed information about funds and campaigns. Mock data and interfaces were updated to align with these changes, ensuring consistency across the application. Changes
Assessment against linked issues
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 ProcessWe have these basic policies to make the approval process smoother for our volunteer team. Testing Your CodePlease make sure your code passes all tests. Our test code coverage system will fail if these conditions occur:
The process helps maintain the overall reliability of the code base and is a prerequisite for getting your PR approved. Assigned reviewers regularly review the PR queue and tend to focus on PRs that are passing. 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: 0
Outside diff range, codebase verification and nitpick comments (2)
src/screens/FundCampaignPledge/FundCampaignPledge.tsx (2)
Line range hint
133-146: Overall Assessment of Dynamic BreadcrumbsThe changes made to implement dynamic breadcrumbs are well-integrated within the component. The use of React's
useMemofor derivingfundNameand its subsequent use in theBreadcrumbscomponent enhances the application's usability by providing context-specific navigation paths.It's recommended to add further comments in the code explaining the choice of
history.go(-2)andhistory.back()for future maintainability.Also applies to: 397-408
Line range hint
133-146: Refactor Suggestion for useMemo HookWhile the current implementation of the
useMemohook is functional, consider refactoring it for better readability and efficiency. Specifically, the nested function calls and operations within the filter callback can be simplified or broken down into smaller, more manageable functions.Here's a suggested refactor for clarity:
- const { pledges, totalPledged, fundName } = useMemo(() => { - let totalPledged = 0; - const pledges = - pledgeData?.getFundraisingCampaigns[0].pledges.filter((pledge) => { - totalPledged += pledge.amount; - const search = searchTerm.toLowerCase(); - return pledge.users.some((user) => { - const fullName = `${user.firstName} ${user.lastName}`; - return fullName.toLowerCase().includes(search); - }); - }) ?? []; - const fundName = - pledgeData?.getFundraisingCampaigns[0].fundId.name ?? tCommon('Funds'); - return { pledges, totalPledged, fundName }; - }, [pledgeData, searchTerm]); + const calculatePledges = (pledges, searchTerm) => { + let total = 0; + const filteredPledges = pledges.filter((pledge) => { + total += pledge.amount; + const search = searchTerm.toLowerCase(); + return pledge.users.some((user) => { + const fullName = `${user.firstName} ${user.lastName}`; + return fullName.toLowerCase().includes(search); + }); + }); + return { filteredPledges, total }; + }; + const { pledges, totalPledged, fundName } = useMemo(() => { + const { filteredPledges, total } = calculatePledges( + pledgeData?.getFundraisingCampaigns[0].pledges ?? [], + searchTerm + ); + const fundName = + pledgeData?.getFundraisingCampaigns[0].fundId.name ?? tCommon('Funds'); + return { pledges: filteredPledges, totalPledged: total, fundName }; + }, [pledgeData, searchTerm]);This refactor separates the logic for filtering pledges and calculating the total pledged amount into a separate function, improving the readability and maintainability of the code.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (12)
- src/GraphQl/Mutations/CampaignMutation.ts (1 hunks)
- src/GraphQl/Mutations/FundMutation.ts (1 hunks)
- src/GraphQl/Queries/fundQueries.ts (2 hunks)
- src/screens/FundCampaignPledge/FundCampaignPledge.tsx (4 hunks)
- src/screens/FundCampaignPledge/PledgesMocks.ts (5 hunks)
- src/screens/OrganizationFundCampaign/OrganizationFundCampagins.tsx (8 hunks)
- src/screens/OrganizationFundCampaign/OrganizationFundCampaign.test.tsx (1 hunks)
- src/screens/OrganizationFundCampaign/OrganizationFundCampaignMocks.ts (10 hunks)
- src/screens/OrganizationFunds/OrganizationFunds.test.tsx (1 hunks)
- src/screens/OrganizationFunds/OrganizationFunds.tsx (7 hunks)
- src/screens/OrganizationFunds/OrganizationFundsMocks.ts (3 hunks)
- src/utils/interfaces.ts (2 hunks)
Files skipped from review due to trivial changes (3)
- src/screens/OrganizationFundCampaign/OrganizationFundCampaign.test.tsx
- src/screens/OrganizationFunds/OrganizationFunds.test.tsx
- src/screens/OrganizationFunds/OrganizationFundsMocks.ts
Additional context used
Learnings (5)
src/GraphQl/Mutations/FundMutation.ts (1)
Learnt from: GlenDsza PR: PalisadoesFoundation/talawa-admin#2064 File: src/screens/OrganizationFunds/FundDeleteModal.tsx:1-2 Timestamp: 2024-06-30T21:44:39.912Z Learning: The folder containing mutation queries in the codebase is named `GraphQl`, not `graphql`.src/GraphQl/Mutations/CampaignMutation.ts (1)
Learnt from: GlenDsza PR: PalisadoesFoundation/talawa-admin#2064 File: src/screens/OrganizationFundCampaign/CampaignDeleteModal.tsx:4-5 Timestamp: 2024-06-30T21:17:16.481Z Learning: The folder containing mutation queries in the codebase is named `GraphQl`, not `graphql`.src/screens/OrganizationFundCampaign/OrganizationFundCampaignMocks.ts (2)
Learnt from: GlenDsza PR: PalisadoesFoundation/talawa-admin#2064 File: src/screens/OrganizationFundCampaign/CampaignDeleteModal.tsx:4-5 Timestamp: 2024-06-30T21:17:16.481Z Learning: The folder containing mutation queries in the codebase is named `GraphQl`, not `graphql`.Learnt from: GlenDsza PR: PalisadoesFoundation/talawa-admin#2064 File: src/screens/OrganizationFunds/FundDeleteModal.tsx:1-2 Timestamp: 2024-06-30T21:44:39.912Z Learning: The folder containing mutation queries in the codebase is named `GraphQl`, not `graphql`.src/screens/OrganizationFunds/OrganizationFunds.tsx (4)
Learnt from: GlenDsza PR: PalisadoesFoundation/talawa-admin#2064 File: src/screens/OrganizationFunds/OrganizationFunds.tsx:66-72 Timestamp: 2024-06-30T21:27:17.104Z 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:30-42 Timestamp: 2024-06-30T21:45:15.681Z Learning: GlenDsza prefers using direct server response messages for error handling in toast notifications without additional contextual strings.Learnt from: GlenDsza PR: PalisadoesFoundation/talawa-admin#2064 File: src/screens/OrganizationFunds/FundDeleteModal.tsx:1-2 Timestamp: 2024-06-30T21:44:39.912Z Learning: The folder containing mutation queries in the codebase is named `GraphQl`, not `graphql`.Learnt from: GlenDsza PR: PalisadoesFoundation/talawa-admin#2064 File: src/screens/OrganizationFundCampaign/OrganizationFundCampagins.tsx:43-46 Timestamp: 2024-06-30T21:50:22.966Z Learning: GlenDsza prefers descriptive and consistent naming for enum values in modal states, as demonstrated in the PR PalisadoesFoundation/talawa-admin#2064.src/screens/OrganizationFundCampaign/OrganizationFundCampagins.tsx (2)
Learnt from: GlenDsza PR: PalisadoesFoundation/talawa-admin#2064 File: src/screens/OrganizationFunds/OrganizationFunds.tsx:66-72 Timestamp: 2024-06-30T21:27:17.104Z 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/OrganizationFundCampaign/OrganizationFundCampagins.tsx:43-46 Timestamp: 2024-06-30T21:50:22.966Z Learning: GlenDsza prefers descriptive and consistent naming for enum values in modal states, as demonstrated in the PR PalisadoesFoundation/talawa-admin#2064.
Additional comments not posted (23)
src/GraphQl/Mutations/FundMutation.ts (2)
Line range hint
8-20: LGTM!The
CREATE_FUND_MUTATIONis well-documented and correctly implemented.
Line range hint
22-38: LGTM!The
UPDATE_FUND_MUTATIONis well-documented and correctly implemented.src/GraphQl/Mutations/CampaignMutation.ts (2)
Line range hint
8-22: LGTM!The
CREATE_CAMPAIGN_MUTATIONis well-documented and correctly implemented.
Line range hint
24-40: LGTM!The
UPDATE_CAMPAIGN_MUTATIONis well-documented and correctly implemented.src/GraphQl/Queries/fundQueries.ts (3)
Line range hint
10-34: LGTM!The
FUND_LISTquery is well-documented and correctly implemented.
Line range hint
36-47: Enhanced Data Retrieval: Approved!The modifications to the
FUND_CAMPAIGNquery, including the addition ofnameandisArchivedfields, enhance the data retrieval capabilities and align with the PR objectives.
Line range hint
59-68: Enhanced Contextual Data: Approved!The modifications to the
FUND_CAMPAIGN_PLEDGEquery, including the addition of a nested fieldfundIdthat retrieves thenameof the associated fund, provide more context about the fundraising campaigns and enhance the overall utility of the query.src/screens/OrganizationFundCampaign/OrganizationFundCampaignMocks.ts (6)
20-21: Approved: Addition ofnameandisArchivedfields.The addition of
nameandisArchivedfields to the mock data is consistent with the PR's objectives to enhance the detail and usability of the data. Ensure that these fields are consistently used across all relevant mock data instances.
56-57: Approved: Consistent addition of fields in mock data.The
nameandisArchivedfields are consistently added here as in the first instance, which is crucial for maintaining uniform behavior across tests and components using this mock data.
84-85: Approved: Consistent addition of fields in mock data.The addition of
nameandisArchivedfields here is consistent with other instances, ensuring uniform behavior across the application's components and tests.
120-121: Approved: Consistent addition of fields in mock data.The consistent addition of
nameandisArchivedfields across all instances ensures that the mock data accurately reflects the intended changes and supports the dynamic nature of the application's UI.
156-157: Approved: Consistent addition of fields in mock data.The addition of
nameandisArchivedfields here is consistent with other instances, ensuring uniform behavior across the application's components and tests.
192-193: Approved: Consistent addition of fields in mock data.The addition of
nameandisArchivedfields here is consistent with other instances, ensuring uniform behavior across the application's components and tests.src/utils/interfaces.ts (2)
288-289: Approved: Interface additions for better data handling.The additions of
nameandisArchivedto theInterfaceQueryOrganizationFundCampaignsare well-aligned with the PR's objectives to enhance data representation. Ensure these new fields are utilized appropriately across the application.Run the following script to verify the usage of the new fields:
Verification successful
Verified: Fields
nameandisArchivedare well-integrated.The fields
nameandisArchivedare extensively used across the application, including in interfaces, test files, screen components, and GraphQL queries and mutations. This confirms their appropriate integration into the application's data model and logic. No further action is required.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `name` and `isArchived` fields in the application. # Test: Search for the usage of the new fields. Expect: References in queries and components. rg --type typescript -A 5 $'name' | rg --type typescript -A 5 $'isArchived'Length of output: 141
Script:
#!/bin/bash # Description: Verify the usage of `name` and `isArchived` fields in the application. # Corrected Test: Search for the usage of the new fields. Expect: References in queries and components. rg --type ts -A 5 'name' | rg --type ts -A 5 'isArchived'Length of output: 13979
309-311: Approved: Enhanced linkage between pledges and funds.The addition of the
fundIdobject with anameproperty in theInterfaceQueryFundCampaignsPledgesinterface enhances data clarity by explicitly linking pledges to their respective funds. Verify that this new structure is integrated correctly in the application's data handling.Run the following script to verify the integration of the new
fundIdstructure:Verification successful
Integration of
fundIdis consistent and successful across the application.The
fundIdstructure is well-integrated into the application's routing, data handling, and UI components. This includes its presence in GraphQL queries and mutations, test files, and various components. The integration aligns with the application's architecture and data handling strategy.
- Files with
fundIdreferences:
src/App.tsxsrc/GraphQl/Queries/fundQueries.tssrc/GraphQl/Mutations/CampaignMutation.tssrc/screens/OrganizationFundCampaign/CampaignModal.test.tsxsrc/screens/OrganizationFundCampaign/OrganizationFundCampaignMocks.tssrc/screens/OrganizationFunds/OrganizationFunds.tsxsrc/screens/FundCampaignPledge/FundCampaignPledge.tsxScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of `fundId` structure in the application. # Test: Search for the usage of `fundId`. Expect: References in queries and components. rg --type typescript -A 5 $'fundId'Length of output: 69
Script:
#!/bin/bash # Description: Verify the integration of `fundId` structure in the application. # Test: Search for the usage of `fundId`. Expect: References in queries and components. rg --type ts -A 5 $'fundId'Length of output: 28429
src/screens/FundCampaignPledge/PledgesMocks.ts (2)
65-67: Consistent addition offundId, but verify flexibility.The addition of
fundIdwith a hardcoded value 'Fund 1' across multiple instances in theMOCKSarray is consistent and appears correctly implemented. However, consider verifying if this hardcoded value limits the flexibility of the mock data, especially in diverse testing scenarios.Also applies to: 124-126, 183-185, 296-298
403-405: Consistent addition offundIdinEMPTY_MOCKS.The addition of
fundIdwith a hardcoded value 'Fund 1' in theEMPTY_MOCKSarray is consistent with the changes made in theMOCKSarray. This change enhances the structure of the mock data by associating a fund identifier with each campaign. However, ensure that this hardcoded value does not limit the flexibility of the mock data in testing scenarios.src/screens/FundCampaignPledge/FundCampaignPledge.tsx (2)
Line range hint
133-146: Dynamic Breadcrumbs ImplementationThe implementation of dynamic breadcrumbs using the
fundNamevariable is well done. TheuseMemohook efficiently calculatesfundNamebased on the availablepledgeData. This change aligns with the PR objectives to make breadcrumb navigation dynamic and context-aware.However, it's important to ensure that the fallback text (
tCommon('Funds')) is appropriate and that all translations are available for this text to maintain i18n standards.
Line range hint
397-408: Usage of Dynamic Breadcrumbs in JSXThe dynamic text for breadcrumbs is correctly implemented in the JSX. The
fundNameandcampaignInfo.nameare used to render the breadcrumb links dynamically. This is a crucial part of enhancing user navigation as per the PR objectives.Ensure that the
onClickhandlers for these links are correctly managing navigation history. The use ofhistory.go(-2)andhistory.back()might need careful testing to ensure they behave as expected across different browsers and navigation scenarios.Consider adding automated tests to verify the correct behavior of these navigation handlers across different user navigation paths and browser environments.
src/screens/OrganizationFunds/OrganizationFunds.tsx (2)
94-105: Approved: Updated modal handling logic.The
handleOpenModalfunction has been simplified to directly manage the modal state, making the component easier to maintain and understand. The use ofuseCallbackwith an empty dependency array is appropriate here, ensuring the function is not recreated unnecessarily.
43-47: Approved: Removal of deletion functionality and simplification of modal management.The changes correctly implement the PR's objectives by removing the deletion functionality and simplifying the modal management to a boolean state. The documentation has been updated accordingly.
Run the following script to ensure that no other parts of the codebase are attempting to use the removed
handleDeleteClickfunction orModalStateenum:Also applies to: 66-66, 94-94, 103-103, 372-373
src/screens/OrganizationFundCampaign/OrganizationFundCampagins.tsx (2)
94-105: Approved: Updated modal handling logic.The
handleOpenModalfunction has been simplified to directly manage the modal state, making the component easier to maintain and understand. The use ofuseCallbackwith an empty dependency array is appropriate here, ensuring the function is not recreated unnecessarily.
47-53: Approved: Removal of deletion functionality and simplification of modal management.The changes correctly implement the PR's objectives by removing the deletion functionality and simplifying the modal management to a boolean state. The documentation has been updated accordingly.
Run the following script to ensure that no other parts of the codebase are attempting to use the removed
handleDeleteClickfunction orModalStateenum:Also applies to: 60-60, 94-94, 103-103, 442-443
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2248 +/- ##
===========================================
- Coverage 97.45% 97.44% -0.02%
===========================================
Files 241 239 -2
Lines 6887 6841 -46
Branches 2000 2006 +6
===========================================
- Hits 6712 6666 -46
Misses 161 161
Partials 14 14 ☔ View full report in Codecov by Sentry. |
|
@tasneemkoushar Pls Review |
What kind of change does this PR introduce?
refactoring
Issue Number:
Fixes #2247
Did you add tests for your changes?
Yes
Snapshots/Videos:
breadcrumbs.mp4
Summary
Does this PR introduce a breaking change?
No
Have you read the contributing guide?
Yes
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores