User Profile Replacement/Removal #9382
Conversation
WalkthroughThe changes in this pull request involve significant modifications to the user management test suite and related components. The Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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
CodeRabbit Configuration File (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
- Removed logic to test profile in user creation (already done elsewhere) - Removed ununsed logic in UserProfilePage - Makes uses of logic in ManageUsers instead since it's the same page - Removed UserFacilities (logic related to sidebar for managing facilities/no longer used)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (8)
cypress/e2e/users_spec/UsersProfile.cy.ts (4)
Line range hint
33-33: Update hardcoded URL in beforeEachThe
cy.awaitUrl("/user/profile")still references the old profile route which has been removed.- cy.awaitUrl("/user/profile"); + cy.awaitUrl("/user/details");
37-44: Extract test data to constants or fixturesConsider extracting test data like "Devo", "Doctoro", and date formats to constants or fixtures for better maintainability.
const TEST_DATA = { firstName: "Devo", lastName: "Doctoro", dateFormat: "M/D/YYYY" };
55-91: Improve test organization and readabilityThis test case handles multiple concerns (qualifications, working hours, video link). Consider:
- Breaking it into smaller, focused test cases
- Using test data builders or fixtures
- Extracting repeated patterns into custom commands
Example refactor:
describe('Professional Information', () => { it('should update qualification details', () => { // qualification specific test }); it('should update working hours', () => { // working hours specific test }); it('should update video connect link', () => { // video link specific test }); });
70-71: Consider moving date formatting logic to a helperThe date manipulation logic should be extracted to a helper function for reusability and easier testing.
// utils/dateHelpers.ts export const calculateExperienceDate = (yearsOfExperience: string): string => { const date = dayjs().subtract(parseInt(yearsOfExperience), "year"); return date.format("YYYY-MM-DD"); };src/components/Users/UserSoftwareUpdate.tsx (2)
12-17: Add TypeScript types for better type safetyConsider adding TypeScript interface for the update status state.
+interface UpdateStatus { + isChecking: boolean; + isUpdateAvailable: boolean; +} export default function UserSoftwareUpdate() { - const [updateStatus, setUpdateStatus] = useState({ + const [updateStatus, setUpdateStatus] = useState<UpdateStatus>({ isChecking: false, isUpdateAvailable: false, });
38-71: Enhance accessibility for update status buttonsThe buttons need ARIA labels and roles for better accessibility.
- <Button disabled> + <Button + disabled + aria-label={t("checking_for_update")} + role="status" + > <div className="flex items-center gap-4"> <CareIcon icon="l-sync" className="text-2xl animate-spin" /> {t("checking_for_update")} </div> </Button> - <Button disabled> + <Button + disabled + aria-label={t("update_available")} + role="alert" + > <div className="flex items-center gap-4"> <CareIcon icon="l-exclamation" className="text-2xl text-warning" /> {t("update_available")} </div> </Button> - <Button variant="primary" onClick={checkUpdates}> + <Button + variant="primary" + onClick={checkUpdates} + aria-label={t("check_for_update")} + >cypress/e2e/users_spec/UsersCreation.cy.ts (2)
Line range hint
89-127: Consider adding test coverage for new user details page integration.While the user creation test is comprehensive, consider adding assertions to verify:
- Successful redirection to the new user details page after creation
- Proper tab selection in the new interface
userCreationPage.clickSaveUserButton(); cy.wait("@createUser").its("response.statusCode").should("eq", 201); cy.verifyNotification("User added successfully"); +// Verify redirection to user details page +cy.url().should('include', '/user/details'); +// Verify active tab +cy.get('[data-testid="user-details-tab"]').should('have.class', 'active');
Line range hint
129-134: Consider expanding mandatory field validation test.The current test only verifies error messages. Consider adding positive validation cases to ensure the error messages clear correctly when valid data is entered.
userCreationPage.clickSaveUserButton(); cy.get(".error-text", { timeout: 10000 }).should("be.visible"); cy.verifyErrorMessages(EXPECTED_ERROR_MESSAGES); +// Add validation for error message clearance +userCreationPage.typeUserName(username); +cy.contains("Please enter the username").should("not.exist");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (11)
cypress/e2e/users_spec/UsersCreation.cy.ts(1 hunks)cypress/e2e/users_spec/UsersManage.cy.ts(0 hunks)cypress/e2e/users_spec/UsersProfile.cy.ts(2 hunks)cypress/pageobject/Users/ManageUserPage.ts(3 hunks)cypress/pageobject/Users/UserProfilePage.ts(1 hunks)src/Routers/routes/UserRoutes.tsx(1 hunks)src/components/Users/ManageUsers.tsx(0 hunks)src/components/Users/UserHome.tsx(3 hunks)src/components/Users/UserProfile.tsx(0 hunks)src/components/Users/UserSoftwareUpdate.tsx(1 hunks)src/components/Users/UserSummary.tsx(2 hunks)
💤 Files with no reviewable changes (3)
- cypress/e2e/users_spec/UsersManage.cy.ts
- src/components/Users/UserProfile.tsx
- src/components/Users/ManageUsers.tsx
🔇 Additional comments (7)
src/components/Users/UserHome.tsx (1)
41-42: LGTM! Clean implementation of user-aware navigation.
The changes effectively handle URL construction and breadcrumb display based on whether the user is viewing their own profile or another user's profile.
Also applies to: 101-105, 132-132
cypress/pageobject/Users/ManageUserPage.ts (1)
153-162: LGTM! Well-structured test helper methods.
The new methods follow best practices:
- Proper scrolling before interactions
- Consistent verification approach
- Clear separation of edit and verify operations
Also applies to: 184-197, 222-226
src/components/Users/UserSoftwareUpdate.tsx (1)
1-10: Verify UpdatableApp integration and implementation
The component relies on UpdatableApp and checkForUpdate from Common components. Let's verify their implementation to ensure proper integration.
✅ Verification successful
Let me gather more information about the UpdatableApp component since the previous command only showed the checkForUpdate function.
Let me try a different approach to find the UpdatableApp component implementation.
UpdatableApp integration looks properly implemented
The verification shows that both UpdatableApp and checkForUpdate are properly implemented in the Common components:
checkForUpdateis a well-structured async function that handles version checking and update detectionUpdatableAppis a React component that manages update state and integrates with the check functionality- The implementation includes proper error handling, version management via localStorage, and UI feedback
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check implementation of UpdatableApp and checkForUpdate
ast-grep --pattern 'export const checkForUpdate = $_'
ast-grep --pattern 'export default function UpdatableApp($_) {
$$$
}'
Length of output: 2300
Script:
#!/bin/bash
# Search for UpdatableApp component implementation and its usage
rg -A 10 "function UpdatableApp" src/components/Common/UpdatableApp.tsx
Length of output: 73
Script:
#!/bin/bash
# Search for UpdatableApp component using different patterns
rg -A 20 "export default.*UpdatableApp" src/components/Common/UpdatableApp.tsx
rg -A 20 "const UpdatableApp" src/components/Common/UpdatableApp.tsx
Length of output: 1230
src/components/Users/UserSummary.tsx (1)
211-216: Verify scope of software update feature
The addition of the software update feature appears to be outside the scope of the PR objectives, which focus on profile page redirection and removal. Please clarify if this is an intentional addition or should be moved to a separate PR.
cypress/e2e/users_spec/UsersCreation.cy.ts (3)
9-9: LGTM! Import changes align with profile removal.
The simplified imports reflect the removal of profile-related functionality while maintaining necessary utilities for user creation.
Line range hint 136-147: Verify facility page redirection with new user details page.
The facility page user redirection test should be updated to verify proper navigation to the new user details page interface.
Line range hint 11-57: Verify test data constants against new user details page.
The test data constants and error messages should be verified to ensure they match the new user details page implementation.
✅ Verification successful
Error messages in test suite match the actual form validation
Based on the search results, the error messages defined in the test suite (EXPECTED_ERROR_MESSAGES) match the actual form validation messages used in the codebase:
- The user form validation messages are defined in
src/components/Users/UserFormValidations.tsxandsrc/components/Users/UserAddEditForm.tsx - The error messages cover all required fields:
- User type selection
- Phone number validation
- Username validation
- Date format validation
- Password requirements
- First/Last name validation
- Email validation
- Gender selection
- State/District/Local body selection
The test data constants accurately reflect the validation messages shown to users when form fields are invalid or missing.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for field validations in the new user details component
ast-grep --pattern 'const EXPECTED_ERROR_MESSAGES = [
$$$
]'
# Search for form field definitions
rg -A 5 'interface.*FormValues|type.*FormValues'
Length of output: 1297
Script:
#!/bin/bash
# Search for form validation messages in component files
rg -A 1 "Please select the User Type|Please enter valid phone number|Please enter the username|Please enter date in DD/MM/YYYY format|Password is required|Confirm password is required|First Name is required|Last Name is required|Please enter a valid email address|Please select the Gender|Please select the state|Please select the district|Please select the local body" --type ts --type tsx
# Search for form field validation logic
ast-grep --pattern 'const validate = {
$$$
}'
# Search for form field validation functions
ast-grep --pattern 'function validate$_($$$) {
$$$
}'
Length of output: 528
Script:
#!/bin/bash
# Search for validation messages in all files, fixing the file type flag
rg -A 1 "Please select the User Type|Please enter valid phone number|Please enter the username|Please enter date in DD/MM/YYYY format|Password is required|Confirm password is required|First Name is required|Last Name is required|Please enter a valid email address|Please select the Gender|Please select the state|Please select the district|Please select the local body" --type typescript
# Search for form validation in component files
rg -A 3 "validation|errorMessage|error message" --type typescript src/
# Look for user creation form component
fd -e ts -e tsx "UserCreation" src/
Length of output: 568
Script:
#!/bin/bash
# First find all TypeScript files that might contain validation messages
fd -e ts -e tsx . | xargs rg -l "validation|error|message"
# Find user creation related components and their validation logic
fd -e ts -e tsx . | xargs rg -A 3 "createUser|UserCreation|addUser"
# Search for form field validation patterns in all TypeScript files
fd -e ts -e tsx . | xargs rg -A 2 "required|invalid|Please"
Length of output: 70818
| <> | ||
| <UserColumns | ||
| heading={t("language_selection")} | ||
| note={t("set_your_local_language")} | ||
| Child={LanguageSelector} | ||
| childProps={userColumnsData} | ||
| /> | ||
| <UserColumns | ||
| heading={t("software_update")} | ||
| note={t("check_for_available_update")} | ||
| Child={UserSoftwareUpdate} | ||
| childProps={userColumnsData} | ||
| /> | ||
| </> |
There was a problem hiding this comment.
💡 Codebase verification
Translation keys missing in non-English locales
The software update section's translation keys need to be added to all non-English locale files:
software_updatekey missing in hi.json, kn.json, ml.json, mr.json, ta.jsoncheck_for_available_updatekey missing in hi.json, kn.json, ml.json, mr.json, ta.json
🔗 Analysis chain
Verify translation keys for software update section
The new software update section uses translation keys that need to be verified in the i18n files.
Translation keys are properly defined in English locale but missing in others
The verification shows that both translation keys software_update and check_for_available_update are defined in the English locale file (en.json), but might be missing in other locale files. Let me verify other locales.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if translation keys exist in i18n files
rg -g '*.json' '"software_update":'
rg -g '*.json' '"check_for_available_update":'
Length of output: 232
Script:
#!/bin/bash
# List all locale files
fd -e json -p public/locale/
# Check content of all locale files for these keys
for locale in $(fd -e json -p public/locale/); do
echo "=== $locale ==="
jq '.software_update, .check_for_available_update' "$locale"
done
Length of output: 1443
|
👋 Hi, @Jacobjeevan, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
Jacobjeevan
added
needs testing
needs review
and removed
merge conflict
|
👋 Hi, @Jacobjeevan, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
ManageUserscomponent by removing user facilities management logic.Chores