-
-
Notifications
You must be signed in to change notification settings - Fork 649
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
Redesign FundPledge Screen & Add Search/Sort #2040
Conversation
WalkthroughThe recent changes overhaul the Fund Campaign Pledge screen by redesigning the interface, adding enhanced sorting and search functionalities, and allowing admin users to create pledges on behalf of others. This update refines various user interactions, introduces new modal dialogs for creating and editing pledges, and aligns the look and feel with the existing design system. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Admin
participant PledgeScreen
participant GraphQL
participant Modal
User->>+PledgeScreen: Open Pledge Screen
PledgeScreen->>+GraphQL: Fetch Pledges
GraphQL-->>-PledgeScreen: Return Pledges
User->>PledgeScreen: Sort/Filter/Search Pledges
PledgeScreen->>+GraphQL: Fetch Sorted/Filtered/Queried Pledges
GraphQL-->>-PledgeScreen: Return Requested Pledges
Admin->>+PledgeScreen: Open Create/Edit Pledge Modal
PledgeScreen->>+Modal: Display Modal
Admin->>Modal: Enter Pledge Details
Modal->>+GraphQL: Save Pledge
GraphQL-->>-Modal: Confirm Save
Modal-->>-Admin: Close Modal
PledgeScreen->>+GraphQL: Refresh Pledge List
GraphQL-->>-PledgeScreen: Return Updated Pledges
Assessment against linked issues
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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
|
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: 1
Outside diff range and nitpick comments (5)
src/screens/FundCampaignPledge/PledgeModal.test.tsx (1)
49-49
: The functionrenderFundCampaignPledge
is well implemented, encapsulating the rendering logic for theFundCampaignPledge
component within a test environment. However, consider adding a brief comment describing its purpose and parameters for better maintainability.src/screens/FundCampaignPledge/FundCampaignPledge.test.tsx (1)
49-49
: The functionrenderFundCampaignPledge
is well implemented, encapsulating the rendering logic for theFundCampaignPledge
component within a test environment. However, consider adding a brief comment describing its purpose and parameters for better maintainability.src/screens/LoginPage/LoginPage.tsx (3)
Line range hint
85-88
: Consider using regular expression literals instead of theRegExp
constructor for simplicity and performance.- lowercaseCharRegExp: new RegExp('[a-z]'), - uppercaseCharRegExp: new RegExp('[A-Z]'), - numericalValueRegExp: new RegExp('\\d'), - specialCharRegExp: new RegExp('[!@#$%^&*()_+{}\\[\\]:;<>,.?~\\\\/-]'), + lowercaseCharRegExp: /[a-z]/, + uppercaseCharRegExp: /[A-Z]/, + numericalValueRegExp: /\d/, + specialCharRegExp: /[!@#$%^&*()_+{}\\[\\]:;<>,.?~\\\\/-]/,Using literals simplifies the code and avoids some of the escaping required in string literals, enhancing readability and maintainability.
Also applies to: 205-205
Line range hint
160-160
: Clarify the use ofvoid
in union types or consider replacing it withundefined
for better clarity in type definitions.- type PasswordValidation = { - lowercaseChar: boolean | void; - uppercaseChar: boolean | void; - numericValue: boolean | void; - specialChar: boolean | void; - }; + type PasswordValidation = { + lowercaseChar: boolean | undefined; + uppercaseChar: boolean | undefined; + numericValue: boolean | undefined; + specialChar: boolean | undefined; + };This change aligns with TypeScript best practices by clearly indicating that the properties can explicitly be
undefined
.
Line range hint
350-350
: Ensure all image elements have meaningful alternative text to improve accessibility for screen reader users.- <img src={logo} /> + <img src={logo} alt={tag + " logo"} />Adding
alt
attributes provides context and improves accessibility.Also applies to: 361-361
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (18)
- public/locales/en/translation.json (1 hunks)
- public/locales/fr/translation.json (1 hunks)
- public/locales/hi/translation.json (1 hunks)
- public/locales/sp/translation.json (1 hunks)
- public/locales/zh/translation.json (1 hunks)
- src/GraphQl/Mutations/PledgeMutation.ts (1 hunks)
- src/GraphQl/Queries/fundQueries.ts (2 hunks)
- src/screens/FundCampaignPledge/FundCampaignPledge.module.css (3 hunks)
- src/screens/FundCampaignPledge/FundCampaignPledge.test.tsx (2 hunks)
- src/screens/FundCampaignPledge/FundCampaignPledge.tsx (4 hunks)
- src/screens/FundCampaignPledge/PledgeDeleteModal.test.tsx (1 hunks)
- src/screens/FundCampaignPledge/PledgeDeleteModal.tsx (2 hunks)
- src/screens/FundCampaignPledge/PledgeModal.test.tsx (1 hunks)
- src/screens/FundCampaignPledge/PledgeModal.tsx (1 hunks)
- src/screens/FundCampaignPledge/PledgesMocks.ts (7 hunks)
- src/screens/LoginPage/LoginPage.tsx (1 hunks)
- src/utils/currency.ts (1 hunks)
- src/utils/interfaces.ts (4 hunks)
Files skipped from review due to trivial changes (4)
- public/locales/en/translation.json
- public/locales/fr/translation.json
- public/locales/sp/translation.json
- src/screens/FundCampaignPledge/FundCampaignPledge.module.css
Additional context used
Biome
src/screens/LoginPage/LoginPage.tsx
[error] 160-160: void is confusing inside a union type. (lint/suspicious/noConfusingVoidType)
Unsafe fix: Use undefined instead.
[error] 350-350: Provide a text alternative through the alt, aria-label or aria-labelledby attribute (lint/a11y/useAltText)
Meaningful alternative text on elements helps users relying on screen readers to understand content's purpose within a page.
[error] 361-361: Provide a text alternative through the alt, aria-label or aria-labelledby attribute (lint/a11y/useAltText)
Meaningful alternative text on elements helps users relying on screen readers to understand content's purpose within a page.
[error] 85-85: Use a regular expression literal instead of the RegExp constructor. (lint/complexity/useRegexLiterals)
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.
[error] 86-86: Use a regular expression literal instead of the RegExp constructor. (lint/complexity/useRegexLiterals)
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.
[error] 87-87: Use a regular expression literal instead of the RegExp constructor. (lint/complexity/useRegexLiterals)
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.
[error] 88-88: Use a regular expression literal instead of the RegExp constructor. (lint/complexity/useRegexLiterals)
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.
[error] 205-205: Use a regular expression literal instead of the RegExp constructor. (lint/complexity/useRegexLiterals)
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.
Additional comments not posted (29)
src/GraphQl/Queries/fundQueries.ts (1)
Line range hint
50-64
: The modifications to theFUND_CAMPAIGN_PLEDGE
query, including the addition of theorderBy
parameter and new fields in theusers
object, correctly align with the PR's objectives to enhance the functionality of the pledge management system.src/GraphQl/Mutations/PledgeMutation.ts (1)
55-60
: The addition of the$users
parameter to theUPDATE_PLEDGE
mutation enhances the system's flexibility and supports the PR's goal of allowing admins to manage pledges on behalf of users.src/screens/FundCampaignPledge/PledgeDeleteModal.tsx (1)
4-42
: The updates to thePledgeDeleteModal
component, including the use of hooks, streamlined interface, and new deletion handler, significantly enhance the component's readability and functionality.src/screens/FundCampaignPledge/PledgeDeleteModal.test.tsx (1)
1-108
: The updates to the test file forPledgeDeleteModal
, including the use of new testing utilities and updated mocks, correctly align with the changes in the component and ensure comprehensive coverage of the new functionalities.src/screens/FundCampaignPledge/PledgeModal.test.tsx (2)
1-1
: The file imports and setup look good and are correctly updated to reflect the new dependencies and changes in the project structure.Also applies to: 4-4, 6-6, 15-15, 18-18, 26-26, 44-44, 45-47
114-114
: The test cases are comprehensive and cover various functionalities of thePledgeModal
component, including form field updates, modal operations, and error handling. Ensure all new functionalities introduced in the PR are covered by these tests.Also applies to: 122-122, 126-126, 141-141, 149-149, 157-157, 165-165, 173-173, 186-186, 219-219, 238-238, 257-257, 276-276
src/utils/interfaces.ts (1)
262-262
: The addition of theInterfacePledgeVolunteer
and its usage in various interfaces ensures a consistent structure for volunteer data across the application. This change enhances maintainability and type safety. Ensure all components using these interfaces are updated accordingly.Also applies to: 289-289, 433-433, 455-460
Verification successful
The search results indicate that the
InterfacePledgeVolunteer
interface is used in multiple locations within the codebase, includingsrc/utils/interfaces.ts
,src/screens/FundCampaignPledge/FundCampaignPledge.tsx
, andsrc/screens/FundCampaignPledge/PledgeModal.tsx
. This suggests that the interface is indeed being utilized in various components.
src/utils/interfaces.ts
: Lines whereInterfacePledgeVolunteer
is defined and used.src/screens/FundCampaignPledge/FundCampaignPledge.tsx
: Lines whereInterfacePledgeVolunteer
is imported and used.src/screens/FundCampaignPledge/PledgeModal.tsx
: Lines whereInterfacePledgeVolunteer
is imported and used.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all components using these interfaces are updated accordingly. # Test: Search for usage of `InterfacePledgeVolunteer`. Expect: Components using this interface should be updated. rg --type python "InterfacePledgeVolunteer" src/Length of output: 79
Script:
#!/bin/bash # Description: Verify all components using these interfaces are updated accordingly. # Test: Search for usage of `InterfacePledgeVolunteer`. Expect: Components using this interface should be updated. rg --type typescript "InterfacePledgeVolunteer" src/Length of output: 87
Script:
#!/bin/bash # Description: Verify all components using these interfaces are updated accordingly. # Test: Search for usage of `InterfacePledgeVolunteer`. Expect: Components using this interface should be updated. rg --type-add 'ts:*.ts' --type-add 'tsx:*.tsx' "InterfacePledgeVolunteer" src/Length of output: 865
src/screens/FundCampaignPledge/FundCampaignPledge.test.tsx (2)
4-4
: The file imports and setup look good and are correctly updated to reflect the new dependencies and changes in the project structure.Also applies to: 6-6, 15-15, 18-18, 26-26, 44-44, 45-47
114-114
: The test cases are comprehensive and cover various functionalities of theFundCampaignPledge
component, including form field updates, modal operations, and error handling. Ensure all new functionalities introduced in the PR are covered by these tests.Also applies to: 122-122, 126-126, 141-141, 149-149, 157-157, 165-165, 173-173, 186-186, 219-219, 238-238, 257-257, 276-276
src/screens/FundCampaignPledge/PledgeModal.tsx (1)
1-1
: The imports and component setup inPledgeModal.tsx
are correctly structured, and the use of TypeScript interfaces enhances type safety and code readability. The component structure follows best practices for modularity and reusability.Also applies to: 2-2, 3-3, 4-4, 5-5, 6-10, 11-11, 12-12, 13-13, 14-14, 15-15, 16-16, 17-24, 26-26, 28-37
src/screens/FundCampaignPledge/PledgesMocks.ts (7)
6-6
: The import ofMEMBERS_LIST
from the GraphQL queries module is appropriate for the expanded functionality of the pledge system, which now involves handling member data.
22-22
: The updated end dates for the pledges align with the requirements specified in the PR to ensure correct data representation.Also applies to: 71-71, 120-120, 169-169
29-29
: The code correctly implements the sorting of pledges by end date, which is a new feature intended to enhance user experience.Also applies to: 78-78, 93-93, 142-142, 176-176, 191-191
34-35
: The addition oflastName
andimage
fields to the user objects within pledges is consistent with the enhanced user interface requirements for displaying more detailed volunteer information.Also applies to: 49-50, 83-84, 98-99, 131-132, 146-147, 181-182, 196-197
285-290
: The new variables for creating pledges (campaignId
,startDate
,endDate
,userIds
) are correctly defined and align with the updated functionality of the pledge system.Also applies to: 494-499
435-510
: ThePLEDGE_MODAL_MOCKS
are well structured and provide comprehensive mock data for testing the member list and pledge operations, which is crucial for ensuring robust functionality.
14-15
: Ensure consistency in the use of IDs for fund campaign references across the application to avoid potential mismatches or errors.Also applies to: 63-64, 112-113, 161-162, 228-229, 418-419
Verification successful
Ensure consistency in the use of IDs for fund campaign references across the application to avoid potential mismatches or errors.
- The usage of 'fundCampaignId' is consistent across multiple files:
src/App.tsx
src/screens/FundCampaignPledge/PledgesMocks.ts
src/screens/FundCampaignPledge/FundCampaignPledge.test.tsx
src/screens/FundCampaignPledge/FundCampaignPledge.tsx
src/screens/FundCampaignPledge/PledgeModal.test.tsx
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistent use of 'fundCampaignId' across the application. # Test: Search for the variable usage. Expect: Consistent usage of 'fundCampaignId'. rg --type typescript $'fundCampaignId'Length of output: 72
Script:
#!/bin/bash # Description: Verify consistent use of 'fundCampaignId' across the application. # Test: Search for the variable usage. Expect: Consistent usage of 'fundCampaignId'. rg --type ts 'fundCampaignId'Length of output: 1454
src/screens/FundCampaignPledge/FundCampaignPledge.tsx (9)
1-2
: The imports are correctly organized and necessary for the functionality of the FundCampaignPledge component, including new features like search and sorting.Also applies to: 6-7, 9-9, 11-13, 14-17, 20-20
35-38
: The handling of missingfundCampaignId
ororgId
by redirecting to the home page is a good safety measure to ensure that the component operates with all required data.
45-47
: The state management for modal visibility and sorting preferences is implemented correctly, aligning with the component's requirements for dynamic user interaction.Also applies to: 49-53
73-74
: The use ofsortBy
state variable to dynamically set the order of pledges fetched from the server is a good implementation of the new sorting feature.
84-92
: The filtering logic for pledges based on the search term is correctly implemented and should effectively filter the pledges based on user input.
95-97
: The use ofuseEffect
to refetch pledges data when sorting preferences change is a good practice to ensure that the UI is always up to date with the latest data based on user interactions.
107-111
: ThehandleOpenModal
function is well-implemented usinguseCallback
for optimization, and correctly manages the state for opening modals based on the pledge and mode.
140-271
: The configuration for theDataGrid
columns is comprehensive and well-defined, particularly the custom render cells that enhance the display of complex data such as volunteers and pledge amounts.
273-409
: The main return block of the component is well-structured, providing a clear and functional user interface. The integration of modals and the DataGrid within the user interface is effectively handled.src/utils/currency.ts (1)
166-166
: The addition of type annotations improves type safety and code readability.public/locales/zh/translation.json (1)
398-404
: Ensure new localization keys are consistent with existing standards and accurately reflect the intended functionality.The new keys for sorting and searching functionalities are consistent with the overall structure and naming conventions used throughout the file. It's important to verify that these translations are accurate and culturally appropriate.
public/locales/hi/translation.json (1)
398-404
: Ensure the translations accurately reflect the intended functionality and are consistent with the terminology used in other languages.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2040 +/- ##
===========================================
+ Coverage 98.39% 98.43% +0.03%
===========================================
Files 222 221 -1
Lines 5932 5945 +13
Branches 1736 1729 -7
===========================================
+ Hits 5837 5852 +15
+ Misses 89 87 -2
Partials 6 6 ☔ View full report in Codecov by Sentry. |
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
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- src/screens/FundCampaignPledge/PledgeModal.tsx (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/screens/FundCampaignPledge/PledgeModal.tsx
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
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- src/screens/FundCampaignPledge/PledgeModal.tsx (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/screens/FundCampaignPledge/PledgeModal.tsx
@aashimawadhwa @Tajcore Can you review this PR? |
What kind of change does this PR introduce?
feature, refactoring
Issue Number:
Fixes #1979
Did you add tests for your changes?
Yes
Snapshots/Videos:
pledge_redesign.mp4
Summary
Does this PR introduce a breaking change?
No
Have you read the contributing guide?
Yes
Summary by CodeRabbit
New Features
PledgeModal
component for creating and editing pledges with comprehensive form elements.Bug Fixes
LoginPage
component to prevent errors.Refactor
FundCampaignPledge
component to use DataGrid for better data handling and UI.Style
FundCampaignPledge
module.Tests
PledgeModal
andPledgeDeleteModal
components.Chores