Refactor facility export & Add sidebar collapse & expand test #9286
Conversation
ghost
self-requested a review
as a 
WalkthroughThe changes in this pull request enhance the testing suite for the Facility Homepage by adding a new test for sidebar functionality and refactoring the capacity export test. A constant for capacity exports is introduced, and the sidebar's collapse and expand functionality is tested to ensure proper visibility of icons and text. Additionally, the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
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. |
![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 and nitpick comments (2)
cypress/pageobject/Facility/FacilityHome.ts (1)
115-136: Consider adding error handling for missing attributes.The
verifySidebarElementsmethod assumes the data-text attribute is always present. Consider adding error handling for missing attributes.private verifySidebarElements( textVisibility: "be.visible" | "not.be.visible", ) { this.sidebarItems() .should("have.length.at.least", 1) .each(($item) => { const expectedText = $item.attr("data-text"); + if (!expectedText) { + throw new Error("Missing data-text attribute on sidebar item"); + } cy.wrap($item, { timeout: 10000 })cypress/e2e/facility_spec/FacilityHomepage.cy.ts (1)
142-153: LGTM! Comprehensive sidebar test coverage.The test case properly verifies both collapse and expand states of the sidebar, with clear steps and assertions.
Consider adding a test for the initial state of the sidebar:
it("Verify sidebar collapse and expand functionality", () => { + // Verify initial state + facilityHome.verifyIconsAndTextVisible(); + facilityHome.toggleSidebar(); facilityHome.verifyIconsAndTextVisible();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
cypress/e2e/facility_spec/FacilityHomepage.cy.ts(3 hunks)cypress/pageobject/Facility/FacilityHome.ts(2 hunks)src/components/Common/Sidebar/Sidebar.tsx(1 hunks)src/components/Common/Sidebar/SidebarItem.tsx(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/components/Common/Sidebar/Sidebar.tsx
🔇 Additional comments (9)
src/components/Common/Sidebar/SidebarItem.tsx (4)
36-37: LGTM! Well-structured test attributes.
The addition of data-testid and data-text attributes enhances testability while maintaining a clear separation between test hooks and component logic.
67-69: LGTM! Consistent test attribute naming.
The data-testid="sidebar-icon" follows the established naming pattern and properly wraps the icon element.
71-71: LGTM! Consistent test attribute placement.
The data-testid="sidebar-text" maintains consistency with other test attributes.
Line range hint 47-51: Review the resetHistory functionality.
The comment "On Review: Check if resetHistory is working as intended" suggests uncertainty about the functionality. This should be investigated to ensure proper behavior.
✅ Verification successful
Let me gather more information about the resetHistory implementation and usage.
resetHistory functionality is working as intended
The resetHistory function is properly implemented in the HistoryAPIProvider and is being correctly used in the SidebarItem component. It resets the navigation history to only keep the initial route by slicing the history array to its first element. This is appropriate behavior when clicking a sidebar item, as it helps maintain a clean navigation state when starting a new navigation path.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for resetHistory usage and implementation
ast-grep --pattern 'const { resetHistory } = useAppHistory()' -l
rg -A 5 'resetHistory\('
Length of output: 621
Script:
#!/bin/bash
# Search for useAppHistory hook implementation and resetHistory usage
ast-grep --pattern 'function useAppHistory() {
$$$
}' --lang typescript
# Search for resetHistory function definition
ast-grep --pattern 'const resetHistory = $$$' --lang typescript
# Search for all occurrences of resetHistory
rg "resetHistory" -B 2 -A 2
Length of output: 3480
cypress/pageobject/Facility/FacilityHome.ts (3)
7-8: LGTM! Clear and consistent selectors.
The selector methods follow the Page Object Model pattern and use the data-testid attributes effectively.
111-113: LGTM! Well-structured toggle method.
The toggleSidebar method includes proper visibility check before interaction.
138-144: LGTM! Clear and descriptive public methods.
The public methods provide a clean interface for verifying sidebar states.
cypress/e2e/facility_spec/FacilityHomepage.cy.ts (2)
22-22: LGTM! Consistent alias naming.
The capacitiesAlias constant follows the established naming pattern.
114-118: LGTM! Well-integrated capacity export test.
The capacity export test follows the same pattern as other export tests and includes proper API verification.
ghost
closed this
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
data-testidattributes to components for better testability.