Fix Display of Address Field Labels in Create Organization Form #3754
Conversation
WalkthroughThe changes introduce a comprehensive test suite for the organization modal within the OrgList component. The new tests cover modal open/close interactions, form input handling, and error management via toast notifications on organization creation failures. In addition, a comment has been added in the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant O as OrgList Component
participant M as Modal
U->>O: Click "Create Organization" button
O->>M: Open Modal (display header)
M->>U: Render Modal
U->>M: Click "Close" button
M->>O: Close Modal
sequenceDiagram
participant U as User
participant O as OrgList Component
participant G as GraphQL Server
participant T as Toast Notification
U->>O: Submit organization form
O->>G: Trigger create organization mutation
G-->>O: Return error response
O->>T: Invoke toast.error
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🔇 Additional comments (5)
✨ Finishing Touches
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: 0
🔭 Outside diff range comments (1)
public/locales/fr/translation.json (1)
865-868: 🛠️ Refactor suggestionInconsistent Key Naming in OrgUpdate Section
In the
"orgUpdate"block, the address fields still use the old keys"line1"and"line2"(displaying “Ligne 1” and “Ligne 2”). For consistency across the application—and to fully meet the PR objective of a unified, user-friendly display—it is recommended to update these keys to"addressLine1"and"addressLine2".For example, you can apply the following diff:
- "line1": "Ligne 1", - "line2": "Ligne 2", + "addressLine1": "Ligne 1", + "addressLine2": "Ligne 2",This will ensure consistency across all forms showing address fields.
🧹 Nitpick comments (2)
src/screens/OrgList/OrgList.spec.tsx (2)
613-673: Complex close button detection logic could be simplified.The modal close button detection logic is unnecessarily complex with multiple fallback strategies. This suggests the test might be fragile if the UI structure changes in the future.
Consider simplifying the close button detection by adding a specific test ID to the close button in the component:
- // Close the modal using the header's close button - // Find the close button within the modal header - const modalHeader = screen.getByTestId('modalOrganizationHeader'); - const closeButton = modalHeader.querySelector('.btn-close'); - - if (closeButton) { - await userEvent.click(closeButton); - } else { - const closeButtons = modalHeader.querySelectorAll('button'); - if (closeButtons.length > 0) { - // Click the last button which is typically the close button - fireEvent.click(closeButtons[closeButtons.length - 1]); - } else { - // If no buttons found, try to click on any element that might be the close button - const closeElements = - modalHeader.querySelectorAll('.close, .btn-close'); - if (closeElements.length > 0) { - fireEvent.click(closeElements[0]); - } - } - } + // Close the modal using the close button + const closeButton = screen.getByTestId('modalCloseButton'); + await userEvent.click(closeButton);And ensure the corresponding button in the component has this test ID.
758-847: Error handling test could be more specific.While the test correctly verifies that the toast.error function is called when an error occurs, it doesn't check for any specific error message content that would be shown to the user.
Consider enhancing the test to verify specific error message content:
// Verify error handling was triggered await waitFor(() => { expect(toast.error).toHaveBeenCalled(); + // Verify the error message passed to toast.error + expect(toast.error).toHaveBeenCalledWith( + expect.stringContaining('Failed to create organization'), + expect.anything() + ); });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
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/screens/OrgList/OrgList.spec.tsx(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Test Application
🔇 Additional comments (8)
public/locales/zh/translation.json (1)
149-150: Updated Address Field Keys for Chinese Organization ListThe keys have been renamed from the older internal names to
"addressLine1"and"addressLine2", ensuring that the end user sees friendly labels (e.g. “1号线” and “2号线”). This change aligns well with the PR objective of fixing the display issue.public/locales/fr/translation.json (1)
149-150: Renamed Address Field Keys in OrgList SectionThe keys for the address fields in the organization list have been updated from
"line1"and"line2"to"addressLine1"and"addressLine2", with the corresponding user-facing values now displayed as “Ligne 1” and “Ligne 2”. This correctly addresses the bug by ensuring that more user-friendly labels are shown.public/locales/en/translation.json (1)
149-150: Clear Key Renaming for Address FieldsThe keys
"addressLine1"and"addressLine2"now replace the earlier internal names. This update enhances clarity by providing user-friendly labels ("Line 1" and "Line 2") for the address fields in the Create Organization form.public/locales/sp/translation.json (1)
154-155: Consistent Renaming in Spanish LocalizationThe Spanish translation now uses
"addressLine1"and"addressLine2"(with labels "Línea 1" and "Línea 2") instead of the older keys. This aligns with the changes in other locale files and ensures that end users see the friendly labels.public/locales/hi/translation.json (1)
149-150: Key change looks good.The renaming of
line1andline2toaddressLine1andaddressLine2while preserving the Hindi translations is correctly implemented. This change aligns with the PR's goal of fixing the display of address field labels in the Create Organization form.src/screens/OrgList/OrgList.spec.tsx (3)
20-36: Import and mock setup is well implemented.The additions for ToastContainer and toast from react-toastify are properly imported and mocked for testing purposes. The mock implementation is comprehensive, covering both success and error scenarios.
600-611: Test data structure aligns with the updated field names.The formData object correctly uses the new
addressLine1andaddressLine2properties instead of the previousline1andline2, ensuring consistency with the translation key changes.
675-756: Form input testing is thorough and well-structured.The test properly validates that each form field can be filled with input and checks that the values are correctly stored. The use of the new field IDs (
modalOrganizationAddressLine1andmodalOrganizationAddressLine2) is consistent with the translation key changes.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #3754 +/- ##
====================================================
+ Coverage 86.07% 86.24% +0.16%
====================================================
Files 372 372
Lines 9223 9219 -4
Branches 1955 1951 -4
====================================================
+ Hits 7939 7951 +12
+ Misses 903 888 -15
+ Partials 381 380 -1 ☔ View full report in Codecov by Sentry. |
|
@palisadoes PTAL |
|
@Nivedita-Chhokar added some comments please have a look |
|
@coderabbitai Full Review |
✅ Actions performedFull review triggered. |
palisadoes
merged commit d5e60f2
into
PalisadoesFoundation:develop-postgres
What kind of change does this PR introduce?
Bugfix
Issue Number:
Fixes #3743
Snapshots/Videos:
Before:
After:
If relevant, did you update the documentation?
No
Summary
This PR fixes the issue where the address input fields in the "Create Organization" form displayed internal variable names instead of user-friendly labels. It updates the labels to "Line 1" and "Line 2" for a clearer user experience.
Does this PR introduce a breaking change?
No
Checklist
CodeRabbit AI Review
Test Coverage
Other information
None
Have you read the contributing guide?
Yes
Summary by CodeRabbit