Cleanup capacity related fields from facility #9436
Conversation
WalkthroughThis pull request involves the removal of several components related to facility management, specifically targeting bed capacity, staff capacity, and associated functionalities. The changes simplify the facility management interface by eliminating components like Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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. |
Deploying care-fe with
|
| Latest commit: |
cfb743a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0aa56617.care-fe.pages.dev |
| Branch Preview URL: | https://cleanup-facility.care-fe.pages.dev |
CARE
|
||||||||||||||||||||||||||||
| Project |
CARE
|
| Branch Review |
cleanup-facility
|
| Run status |
|
| Run duration | 05m 20s |
| Commit |
|
| Committer | Bodhish Thomas |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
139
|
| View all changes introduced in this branch ↗︎ | |
|
@nihal467 can you cleap up the spec ☝️ |
|
can you remove all the related exports as well from the facility page |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
cypress/support/index.ts (1)
49-53: LGTM! Consider adding JSDoc comments.The new
typeIntoFieldmethod is well-designed with clear typing and follows Cypress's chainable pattern. The optionalclearBeforeTypingflag provides good flexibility.Consider adding JSDoc comments to document the method's purpose and parameters:
+ /** + * Types a value into a field identified by the selector + * @param selector - The selector to identify the field + * @param value - The value to type into the field + * @param options - Optional configuration + * @param options.clearBeforeTyping - Whether to clear the field before typing + * @returns Chainable<Element> + */ typeIntoField( selector: string, value: string, options?: { clearBeforeTyping?: boolean }, ): Chainable<Element>;cypress/support/commands.ts (1)
244-261: LGTM! Consider enhancing robustness with additional options.The new
typeIntoFieldcommand is well-implemented with proper TypeScript types and follows Cypress best practices. It effectively combines clear, click, and type operations with an optional clearing feature.Consider these enhancements for improved robustness:
Cypress.Commands.add( "typeIntoField", ( selector: string, value: string, - options: { clearBeforeTyping?: boolean } = {}, + options: { + clearBeforeTyping?: boolean; + timeout?: number; + force?: boolean; + validateInput?: boolean; + } = {}, ) => { - const { clearBeforeTyping = false } = options; + const { + clearBeforeTyping = false, + timeout = 4000, + force = false, + validateInput = true, + } = options; + + if (validateInput && !value && value !== '') { + throw new Error('Invalid input value'); + } + const inputField = cy.get(selector); if (clearBeforeTyping) { inputField.clear(); // Clear the input field } - inputField.click().type(value); // Click and type the new value + inputField + .should('exist', { timeout }) + .click({ force }) + .type(value, { force }); // Click and type the new value }, );These changes add:
- Configurable timeout for element existence
- Force option for hidden elements
- Input validation with toggle
src/components/Facility/FacilityHome.tsx (1)
Line range hint
214-450: Consider architectural improvements for better maintainability.
- Extract the facility details section into a separate component to improve code organization and reusability.
- Consider moving the dropdown menu items to a configuration array.
Example implementation for the dropdown menu items:
const FACILITY_MANAGEMENT_ACTIONS = [ { id: 'update-facility', label: 'update_facility', icon: 'l-edit-alt', href: (facilityId: string) => `/facility/${facilityId}/update`, authorizeFor: NonReadOnlyUsers }, // ... other menu items ] as const; // Usage in component {FACILITY_MANAGEMENT_ACTIONS.map(action => ( <DropdownItem key={action.id} id={action.id} onClick={() => navigate(action.href(facilityId))} authorizeFor={action.authorizeFor} icon={<CareIcon icon={action.icon} className="text-lg" />} > {t(action.label)} </DropdownItem> ))}cypress/pageobject/Facility/FacilityCreation.ts (1)
349-349: Consider removing redundantcy.closeNotification()callSince
cy.closeNotification()is already called withinverifyFacilityCreatedNotification(), the explicit call tocy.closeNotification()afterclickSaveFacilityButton()increateNewFacilitymay be redundant. Removing it can reduce unnecessary code duplication.Apply this diff to remove the redundant call:
this.clickSaveFacilityButton(); - cy.closeNotification(); this.verifyfacilitynewurl(); return this;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
cypress/e2e/facility_spec/FacilityCreation.cy.ts(2 hunks)cypress/e2e/facility_spec/FacilityHomepage.cy.ts(0 hunks)cypress/e2e/facility_spec/FacilityManage.cy.ts(0 hunks)cypress/pageobject/Facility/FacilityCreation.ts(5 hunks)cypress/pageobject/utils/constants.ts(1 hunks)cypress/support/commands.ts(1 hunks)cypress/support/index.ts(1 hunks)package.json(1 hunks)src/components/Facility/FacilityHome.tsx(1 hunks)
💤 Files with no reviewable changes (2)
- cypress/e2e/facility_spec/FacilityHomepage.cy.ts
- cypress/e2e/facility_spec/FacilityManage.cy.ts
🧰 Additional context used
📓 Learnings (2)
cypress/e2e/facility_spec/FacilityCreation.cy.ts (3)
Learnt from: Jacobjeevan
PR: ohcnetwork/care_fe#9145
File: cypress/e2e/facility_spec/FacilityCreation.cy.ts:177-180
Timestamp: 2024-11-18T10:44:30.303Z
Learning: In `cypress/e2e/facility_spec/FacilityCreation.cy.ts`, when testing bed and staff capacity individually for additional error verification, we prefer to use separate method calls to handle bed and staff capacity, rather than using `facilityPage.createNewFacility(testFacilityData)` which also handles bed management.
Learnt from: kihan2518B
PR: ohcnetwork/care_fe#8956
File: cypress/e2e/facility_spec/FacilityCreation.cy.ts:261-261
Timestamp: 2024-12-04T18:58:47.241Z
Learning: In `cypress/e2e/facility_spec/FacilityCreation.cy.ts`, the test case titled "Create a new facility with no bed and doctor capacity" includes steps to select bed types and specializations before validation. This is done intentionally to verify that error messages are shown properly when no capacity is specified.
Learnt from: Jacobjeevan
PR: ohcnetwork/care_fe#9145
File: cypress/e2e/facility_spec/FacilityHomepage.cy.ts:229-317
Timestamp: 2024-11-18T10:46:56.271Z
Learning: In `cypress/e2e/facility_spec/FacilityHomepage.cy.ts`, when verifying the bed capacity badge reflection, the badge remains the same during multiple actions and only updates after a bed is attached to a patient during consultation. Therefore, it's appropriate to keep the test case consolidated rather than splitting it into smaller tests.
cypress/pageobject/Facility/FacilityCreation.ts (2)
Learnt from: Jacobjeevan
PR: ohcnetwork/care_fe#9145
File: cypress/e2e/facility_spec/FacilityCreation.cy.ts:177-180
Timestamp: 2024-11-18T10:44:30.303Z
Learning: In `cypress/e2e/facility_spec/FacilityCreation.cy.ts`, when testing bed and staff capacity individually for additional error verification, we prefer to use separate method calls to handle bed and staff capacity, rather than using `facilityPage.createNewFacility(testFacilityData)` which also handles bed management.
Learnt from: kihan2518B
PR: ohcnetwork/care_fe#8956
File: cypress/e2e/facility_spec/FacilityCreation.cy.ts:261-261
Timestamp: 2024-12-04T18:58:47.241Z
Learning: In `cypress/e2e/facility_spec/FacilityCreation.cy.ts`, the test case titled "Create a new facility with no bed and doctor capacity" includes steps to select bed types and specializations before validation. This is done intentionally to verify that error messages are shown properly when no capacity is specified.
🔇 Additional comments (13)
src/components/Facility/FacilityHome.tsx (2)
214-217: LGTM: Clean implementation of the facility details card.
The grid layout changes look good and align with the removal of capacity-related fields.
Line range hint 1-100: Verify complete removal of capacity-related components.
Let's ensure there are no remaining references to the removed components in the codebase.
✅ Verification successful
Let me check the remaining references in more detail to understand their context.
Remaining references found only in test files - Component removal verified
The capacity-related components have been properly removed from the main application code. The only remaining references are in Cypress test files that need to be updated:
cypress/pageobject/Facility/FacilityManage.ts: Contains test helper methods for bed capacitycypress/e2e/facility_spec/FacilityHomepage.cy.ts: Contains bed capacity related test cases
These test files should be updated to align with the new changes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for any remaining references to removed components
# Check for any remaining imports or usage of removed components
echo "Checking for BedCapacity component references..."
rg -l "BedCapacity"
echo "Checking for FacilityStaffList component references..."
rg -l "FacilityStaffList"
echo "Checking for any bed or capacity related types/interfaces..."
ast-grep --pattern 'interface $_ {
$$$
capacity
$$$
}'
echo "Checking for any bed or capacity related fields in facility queries..."
rg -l "bed.*capacity|staff.*capacity" --type ts
Length of output: 807
Script:
#!/bin/bash
# Check the context of remaining BedCapacity references in test files
echo "Checking BedCapacity references in cypress tests..."
rg -C 5 "BedCapacity" "cypress/pageobject/Facility/FacilityManage.ts"
echo "Checking capacity references in facility homepage tests..."
rg -C 5 "bed.*capacity|staff.*capacity" "cypress/e2e/facility_spec/FacilityHomepage.cy.ts"
Length of output: 1571
cypress/pageobject/Facility/FacilityCreation.ts (9)
38-39: Refactored typeFacilityName method enhances code clarity
The method typeFacilityName has been appropriately refactored to use the utility function cy.typeIntoField with the optional clearBeforeTyping parameter. This improves code clarity and reusability.
51-52: Consistent refactoring in typeFacilityAddress method
The typeFacilityAddress method now utilizes cy.typeIntoField, aligning it with the refactored pattern and enhancing consistency across input methods.
55-59: Improved typeFacilityPhoneNumber method with optional clearing
The typeFacilityPhoneNumber method is updated to use cy.typeIntoField, maintaining consistency with other input methods and providing the flexibility to clear the field before typing.
62-63: Added clickSaveFacilityButton method for better modularity
The new method clickSaveFacilityButton encapsulates the action of clicking the "Save Facility" button, enhancing code modularity and readability.
66-68: Added verifyFacilityCreatedNotification method for clarity
The method verifyFacilityCreatedNotification encapsulates the verification of the success notification after creating a facility, improving test clarity and maintainability.
71-73: Introduced verifyFacilityUpdatedNotification for consistency
The verifyFacilityUpdatedNotification method provides a consistent way to verify the success notification after updating a facility, enhancing code reuse.
88-90: Added clickUpdateFacilityButton method for abstraction
The clickUpdateFacilityButton method abstracts the action of clicking the "Update Facility" button, improving code maintainability and readability.
357-357: Updated fillBasicDetails to use typeFacilityName
The fillBasicDetails method now calls typeFacilityName, aligning with the refactored naming conventions and input methods.
368-369: Consistent use of typeFacilityAddress and typeFacilityPhoneNumber
The methods typeFacilityAddress and typeFacilityPhoneNumber are utilized in fillBasicDetails, maintaining consistency with the refactored input handling approach.
cypress/e2e/facility_spec/FacilityCreation.cy.ts (1)
Line range hint 1-122: Test cases updated to reflect the removal of bed and staff capacities
The test cases have been correctly adjusted to remove references to bed and staff capacities, aligning with the PR objectives. The use of dynamic data generation functions like generateFacilityName, generatePhoneNumber, and generateRandomAddress enhances test reliability and maintainability.
package.json (1)
82-82: Verify the necessity of downgrading the Cypress version
The cypress dependency has been downgraded from ^13.16.0 to ^13.15.2. Please ensure that this change is intentional and verify that it does not remove any required features or important bug fixes introduced in the newer version.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/components/Facility/FacilityCreate.tsx (3)
Line range hint
52-69: Consider using strict types for required fieldsThe
FacilityFormtype could be improved by marking required fields with non-optional types. This would provide better type safety at compile time.type FacilityForm = { - facility_type?: string; + facility_type: string; name: string; state: number; district: number; local_body: number; features: number[]; ward: number; kasp_empanelled: string; address: string; phone_number: string; latitude: string; longitude: string; pincode: string; };
Line range hint
396-419: Enhance error handling in form submissionThe form submission error handling could be improved by:
- Adding specific error messages for different failure scenarios
- Handling network errors explicitly
} else { + const errorMessage = requestData?.detail || 'Failed to save facility'; Notification.Success({ - msg: "Facility updated successfully", + msg: errorMessage, }); } } catch (error) { + Notification.Error({ + msg: "Network error occurred while saving facility", + }); } finally { setIsLoading(false); }
438-443: Improve accessibility of the formConsider adding ARIA labels and roles to improve accessibility:
<Page title={headerText} + aria-label="Facility Creation Form" crumbsReplacements={{ [facilityId || "????"]: { name: state.form.name }, }} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
cypress/e2e/facility_spec/FacilityHomepage.cy.ts(0 hunks)cypress/pageobject/Facility/FacilityHome.ts(0 hunks)cypress/pageobject/Facility/FacilityManage.ts(0 hunks)src/components/Facility/FacilityCreate.tsx(5 hunks)
💤 Files with no reviewable changes (3)
- cypress/pageobject/Facility/FacilityHome.ts
- cypress/e2e/facility_spec/FacilityHomepage.cy.ts
- cypress/pageobject/Facility/FacilityManage.ts
🔇 Additional comments (1)
src/components/Facility/FacilityCreate.tsx (1)
Line range hint 1-643: Verify removal of capacity-related fields
The changes successfully remove all facility capacity and beds fields as intended. The form maintains its core functionality while being simplified.
✅ Verification successful
The search results show that there are still many capacity-related fields and components in the codebase, particularly in the Facility directory. Let me verify if these are actually being used or referenced in the current file.
Let me try a different approach to verify the changes in the FacilityCreate component and related models.
Based on the inspection of the models file and the lack of any capacity-related fields in the FacilityCreate.tsx search results, I can now generate the final response.
Capacity-related fields have been successfully removed
The changes in FacilityCreate.tsx are correct. The code no longer contains any references to capacity-related fields (beds, oxygen, cylinders), and the FacilityForm interface only includes the essential facility information fields. The remaining capacity-related types in models.tsx are used by other components and don't affect the facility creation form.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify that all capacity-related fields have been removed
# and no references remain in the codebase
# Check for any remaining capacity-related fields
rg -i "capacity|beds|oxygen|cylinders" src/components/Facility/
Length of output: 11853
Script:
#!/bin/bash
# Check if any of these capacity-related components are imported or used in FacilityCreate.tsx
rg -l "import.*Bed|capacity|oxygen|cylinders" src/components/Facility/FacilityCreate.tsx
# Check for any capacity-related fields in the form interfaces and models
ast-grep --pattern 'interface FacilityForm {
$$$
}' src/components/Facility/FacilityCreate.tsx
ast-grep --pattern 'interface FacilityRequest {
$$$
}' src/components/Facility/models.tsx
Length of output: 283
Script:
#!/bin/bash
# Check the content of the models file to see facility-related interfaces
cat src/components/Facility/models.tsx
# Check for any capacity-related field references in FacilityCreate.tsx
rg -i "capacity|bed|oxygen|cylinder" src/components/Facility/FacilityCreate.tsx
Length of output: 17814
| <GLocationPicker | ||
| lat={Number(state.form.latitude)} | ||
| lng={Number(state.form.longitude)} | ||
| handleOnChange={handleLocationChange} | ||
| handleOnClose={() => null} | ||
| handleOnSelectCurrentLocation={ | ||
| handleSelectCurrentLocation | ||
| } | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for location picker
The location picker should handle cases where geolocation fails or is not supported:
<GLocationPicker
lat={Number(state.form.latitude)}
lng={Number(state.form.longitude)}
handleOnChange={handleLocationChange}
handleOnClose={() => null}
- handleOnSelectCurrentLocation={handleSelectCurrentLocation}
+ handleOnSelectCurrentLocation={(setCenter) => {
+ try {
+ if (!navigator.geolocation) {
+ Notification.Error({ msg: "Geolocation is not supported by your browser" });
+ return;
+ }
+ handleSelectCurrentLocation(setCenter);
+ } catch (error) {
+ Notification.Error({ msg: "Failed to get current location" });
+ }
+ }}
/>📝 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.
| <GLocationPicker | |
| lat={Number(state.form.latitude)} | |
| lng={Number(state.form.longitude)} | |
| handleOnChange={handleLocationChange} | |
| handleOnClose={() => null} | |
| handleOnSelectCurrentLocation={ | |
| handleSelectCurrentLocation | |
| } | |
| /> | |
| <GLocationPicker | |
| lat={Number(state.form.latitude)} | |
| lng={Number(state.form.longitude)} | |
| handleOnChange={handleLocationChange} | |
| handleOnClose={() => null} | |
| handleOnSelectCurrentLocation={(setCenter) => { | |
| try { | |
| if (!navigator.geolocation) { | |
| Notification.Error({ msg: "Geolocation is not supported by your browser" }); | |
| return; | |
| } | |
| handleSelectCurrentLocation(setCenter); | |
| } catch (error) { | |
| Notification.Error({ msg: "Failed to get current location" }); | |
| } | |
| }} | |
| /> |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
cypress/e2e/facility_spec/FacilityCreation.cy.ts (3)
71-101: Consider adding wait assertions for better test reliability.The test flow is well-structured, but consider adding explicit wait assertions before verification steps to ensure UI updates are complete.
facilityPage.clickUpdateFacilityButton(); facilityPage.verifyFacilityUpdatedNotification(); +cy.wait(500); // Add a small delay for UI updates // verify the facility card updated info cy.verifyContentPresence("#facility-details-card", [
124-139: Consider enhancing error message verification.While the error handling is comprehensive, consider extracting the error messages to constants for better maintainability and reusability.
+const PERMISSION_ERROR = "You do not have permission to perform this action."; +const ACCESS_ERROR = "You don't have permission to perform this action. Contact the admin"; facilityPage.clickSaveFacilityButton(); -facilityPage.verifyErrorNotification( - "You do not have permission to perform this action.", -); +facilityPage.verifyErrorNotification(PERMISSION_ERROR);
Line range hint
141-150: LGTM! Thorough access restriction testing.Good coverage of access restrictions for all non-admin roles. Consider adding a test log to indicate which role is being tested for better debugging.
nonAdminRoles.forEach((role) => { + cy.log(`Testing access restriction for role: ${role}`); loginPage.loginByRole(role);src/components/Facility/FacilityCreate.tsx (2)
417-420: Enhance error handling in form submissionConsider adding specific error messages for different failure scenarios during form submission.
} else { + const errorMessage = requestData?.detail || "Failed to save facility"; Notification.Success({ - msg: "Facility updated successfully", + msg: errorMessage, }); }
457-577: Consider extracting form sections into separate componentsThe form has grown quite large and could benefit from being split into smaller, more manageable components. Consider extracting logical sections like:
- Basic Information (facility type, name, features)
- Location Information (pincode, state, district, etc.)
- Contact Information (address, phone)
Example structure:
// BasicInformationSection.tsx export const BasicInformationSection = ({ formState, onChange }: FormSectionProps) => { return ( <> <SelectFormField name="facility_type" required options={FACILITY_TYPES} // ... other props /> {/* Other fields */} </> ); }; // Usage in FacilityCreate.tsx <BasicInformationSection formState={state.form} onChange={handleChange} />cypress/pageobject/Facility/FacilityCreation.ts (1)
38-39: Great improvements to method naming and implementation!The renamed methods (
typeFacilityName,typeFacilityAddress,typeFacilityPhoneNumber) are more descriptive and the addition ofclearBeforeTypingparameter provides better control over input field handling.Consider adding JSDoc comments to document the
clearBeforeTypingparameter:/** * Types the facility name into the name field * @param name The facility name to type * @param clearBeforeTyping Whether to clear the field before typing (default: false) */ typeFacilityName(name: string, clearBeforeTyping: boolean = false) { cy.typeIntoField("#name", name, { clearBeforeTyping }); }Also applies to: 51-52, 55-59
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cypress/e2e/facility_spec/FacilityCreation.cy.ts(2 hunks)cypress/pageobject/Facility/FacilityCreation.ts(5 hunks)src/components/Facility/FacilityCreate.tsx(5 hunks)
🧰 Additional context used
📓 Learnings (2)
cypress/e2e/facility_spec/FacilityCreation.cy.ts (2)
Learnt from: Jacobjeevan
PR: ohcnetwork/care_fe#9145
File: cypress/e2e/facility_spec/FacilityCreation.cy.ts:177-180
Timestamp: 2024-11-18T10:44:30.303Z
Learning: In `cypress/e2e/facility_spec/FacilityCreation.cy.ts`, when testing bed and staff capacity individually for additional error verification, we prefer to use separate method calls to handle bed and staff capacity, rather than using `facilityPage.createNewFacility(testFacilityData)` which also handles bed management.
Learnt from: kihan2518B
PR: ohcnetwork/care_fe#8956
File: cypress/e2e/facility_spec/FacilityCreation.cy.ts:261-261
Timestamp: 2024-12-04T18:58:47.241Z
Learning: In `cypress/e2e/facility_spec/FacilityCreation.cy.ts`, the test case titled "Create a new facility with no bed and doctor capacity" includes steps to select bed types and specializations before validation. This is done intentionally to verify that error messages are shown properly when no capacity is specified.
cypress/pageobject/Facility/FacilityCreation.ts (2)
Learnt from: Jacobjeevan
PR: ohcnetwork/care_fe#9145
File: cypress/e2e/facility_spec/FacilityCreation.cy.ts:177-180
Timestamp: 2024-11-18T10:44:30.303Z
Learning: In `cypress/e2e/facility_spec/FacilityCreation.cy.ts`, when testing bed and staff capacity individually for additional error verification, we prefer to use separate method calls to handle bed and staff capacity, rather than using `facilityPage.createNewFacility(testFacilityData)` which also handles bed management.
Learnt from: kihan2518B
PR: ohcnetwork/care_fe#8956
File: cypress/e2e/facility_spec/FacilityCreation.cy.ts:261-261
Timestamp: 2024-12-04T18:58:47.241Z
Learning: In `cypress/e2e/facility_spec/FacilityCreation.cy.ts`, the test case titled "Create a new facility with no bed and doctor capacity" includes steps to select bed types and specializations before validation. This is done intentionally to verify that error messages are shown properly when no capacity is specified.
🔇 Additional comments (8)
cypress/e2e/facility_spec/FacilityCreation.cy.ts (2)
Line range hint 1-69: LGTM! Clean test data setup aligned with PR objectives.
The imports and test data structure appropriately reflect the removal of capacity-related fields, with a clean focus on basic facility details. Good use of utility functions for generating test data.
104-121: LGTM! Well-structured test for mandatory fields.
The test case effectively covers facility creation with mandatory fields and deletion functionality, properly aligned with the PR's objective of removing capacity-related fields.
src/components/Facility/FacilityCreate.tsx (2)
614-622: Add error handling for location picker
The location picker should handle cases where geolocation fails or is not supported.
20-20: Verify unused imports after capacity fields removal
Let's verify if all imported types and constants are still in use after removing capacity-related fields.
Also applies to: 36-36
✅ Verification successful
Imports are correctly used in FacilityCreate.tsx
The verification shows that both FACILITY_TYPES and FACILITY_FEATURE_TYPES are actively used in FacilityCreate.tsx for facility type selection and feature selection dropdowns. The DistrictModel and FacilityRequest imports from line 20 are also necessary as they are used for type definitions in the form.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for unused imports
ast-grep --pattern 'import { $_ } from "@/components/Facility/models"'
# Check usage of FACILITY_FEATURE_TYPES and FACILITY_TYPES
rg "FACILITY_FEATURE_TYPES|FACILITY_TYPES" -A 5
Length of output: 17759
cypress/pageobject/Facility/FacilityCreation.ts (4)
Line range hint 3-19: LGTM! Interface cleanup aligns with PR objectives.
The simplified FacilityData interface correctly removes all capacity-related fields while maintaining essential facility information.
66-73: Well-structured notification verification methods!
The new notification verification methods provide clear, reusable functionality for checking success messages.
349-351: Clean implementation of facility creation flow!
The simplified facility creation process correctly removes capacity-related steps while maintaining a clear structure for basic and location details. The implementation aligns with the established pattern of handling capacity separately, as noted in the retrieved learnings.
Also applies to: 357-369
Line range hint 1-385: Verify complete removal of capacity-related code
Let's ensure there are no remaining references to the removed capacity-related functionality.
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
cypress/e2e/resource_spec/ResourcesHomepage.cy.ts (1)
Line range hint
73-81: Add URL existence check before navigation.The tests using
createdResourceURL should verify its existence before navigation to prevent flaky tests.it("Update the status of resource", () => { + expect(createdResource).to.exist; cy.awaitUrl(createdResource); resourcePage.clickUpdateStatus(); // ... rest of the test }); it("Post comment for a resource", () => { + expect(createdResource).to.exist; cy.awaitUrl(createdResource); resourcePage.addCommentForResource("Test comment"); // ... rest of the test });Also applies to: 83-88
🧹 Nitpick comments (2)
cypress/pageobject/Resource/ResourcePage.ts (1)
31-33: Rename method for better grammar and add return type.The method implementation is good, but let's improve its signature.
- navigationToResourcePage() { + navigateToResourcePage(): Cypress.Chainable<string> { cy.awaitUrl("/resource"); }cypress/e2e/resource_spec/ResourcesHomepage.cy.ts (1)
Line range hint
31-38: Consider splitting multiple assertions into separate test cases.This test verifies both completed and active resource states. Consider splitting it into two separate test cases for better isolation and maintenance:
- "Should switch to completed resources"
- "Should switch to active resources"
This follows the single responsibility principle for tests and makes failures more specific.
it("Should switch to completed resources", () => { resourcePage.navigationToResourcePage(); resourcePage.spyResourceApi(); resourcePage.clickCompletedResources(); resourcePage.verifyCompletedResources(); }); it("Should switch to active resources", () => { resourcePage.navigationToResourcePage(); resourcePage.spyResourceApi(); resourcePage.clickActiveResources(); resourcePage.verifyActiveResources(); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
cypress/e2e/patient_spec/PatientHomepage.cy.ts(0 hunks)cypress/e2e/resource_spec/ResourcesHomepage.cy.ts(4 hunks)cypress/pageobject/Facility/FacilityHome.ts(1 hunks)cypress/pageobject/Resource/ResourcePage.ts(1 hunks)src/Utils/request/api.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- cypress/e2e/patient_spec/PatientHomepage.cy.ts
🔇 Additional comments (2)
src/Utils/request/api.tsx (1)
711-711: Verify interface compatibility for LocalBodyModel
The response types for local body endpoints have been updated to use LocalBodyModel. Let's verify that this change is backward compatible.
Also applies to: 716-716
✅ Verification successful
Let me try a different approach to gather the necessary information.
Let me try one final search to find any references to the old interfaces in the codebase.
Based on the gathered information, I can now provide the final response:
Interface change to LocalBodyModel is compatible
The change from ILocalBodies[] and ILocalBodyByDistrict[] to LocalBodyModel[] is verified to be safe because:
LocalBodyModelis properly defined insrc/components/Facility/models.tsxwith all essential fields (id, name, body_type, localbody_code, district)- No references to the old interfaces (
ILocalBodies,ILocalBodyByDistrict) were found in the codebase - The endpoints are used in multiple components (UserAddEditForm, PatientRegister, LocalBodyAutocompleteFormField, FacilityCreate) which will now correctly use the
LocalBodyModeltype
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check interface compatibility and usage
# 1. Find the old interface definitions
echo "Searching for old interface definitions..."
ast-grep --pattern 'interface $INTERFACE {
$$$
}' --pattern-ids INTERFACE=ILocalBodies,ILocalBodyByDistrict
# 2. Find all usages of these endpoints
echo "Searching for endpoint usages..."
rg -A 3 "getAllLocalBodyByDistrict|getLocalbodyByDistrict"
# 3. Find LocalBodyModel definition
echo "Searching for LocalBodyModel definition..."
ast-grep --pattern 'type LocalBodyModel = {
$$$
}'
Length of output: 2434
Script:
#!/bin/bash
# 1. Find LocalBodyModel definition with interface pattern
echo "Searching for LocalBodyModel definition..."
ast-grep --pattern 'interface LocalBodyModel {
$$$
}'
# 2. Try finding old interfaces with rg
echo "Searching for old interface definitions..."
rg -A 10 "interface ILocalBodies|interface ILocalBodyByDistrict"
# 3. Find type definition of LocalBodyModel using rg
echo "Searching for LocalBodyModel type..."
rg -A 10 "type LocalBodyModel"
Length of output: 829
Script:
#!/bin/bash
# Search for any file containing ILocalBodies or ILocalBodyByDistrict
echo "Searching for old interface references..."
rg "ILocalBodies|ILocalBodyByDistrict" --type ts --type tsx
# Search for any imports of these interfaces
echo "Searching for imports of old interfaces..."
ast-grep --pattern 'import { $$$ILocalBodies$$$ } from'
ast-grep --pattern 'import { $$$ILocalBodyByDistrict$$$ } from'
Length of output: 401
cypress/pageobject/Facility/FacilityHome.ts (1)
Line range hint 147-149: Verify if occupancy-related test methods should be removed
Given that this PR removes facility capacity fields, please verify if these occupancy-related test methods should also be removed:
verifyOccupancyBadgeVisibility()assertFacilityBadgeContent(occupied: string, total: string)assertFacilityBadgeBackgroundColor(color: string)
Also applies to: 164-171, 173-179
|
@bodhish @rithviknishad @nihal467 Your efforts have helped advance digital healthcare and TeleICU systems. 🚀 Thank you for taking the time out to make CARE better. We hope you continue to innovate and contribute; your impact is immense! 🙌 |
Proposed Changes
Lets remove facility capacity and beds.
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Chores