Add Test suit for Image Upload Functionality #9373
Conversation
WalkthroughThis pull request introduces comprehensive Cypress end-to-end testing for facility cover image functionality. The changes span multiple files, including a new test specification for facility cover image upload, enhancements to the Changes
Assessment against linked issues
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. |
|
👋 Hi, @JavidSumra, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
cypress/e2e/facility_spec/FacilityCoverImage.cy.ts (1)
12-21: Consider expanding role coverage in test data.While the current test data covers basic roles, consider adding more role variations (e.g., stateAdmin, facilityStaff) to ensure comprehensive testing of permissions and edge cases.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
cypress/fixtures/facility-cover-image-1.jpgis excluded by!**/*.jpgcypress/fixtures/facility-cover-image.jpgis excluded by!**/*.jpg
📒 Files selected for processing (4)
cypress/e2e/facility_spec/FacilityCoverImage.cy.ts(1 hunks)cypress/pageobject/Facility/FacilityManage.ts(2 hunks)public/locale/en.json(2 hunks)src/components/Common/AvatarEditModal.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- public/locale/en.json
🔇 Additional comments (5)
cypress/e2e/facility_spec/FacilityCoverImage.cy.ts (2)
1-10: LGTM! Well-structured imports and interface definition.
The imports are appropriate for the test requirements, and the interface provides good type safety for role validation.
36-43: 🛠️ Refactor suggestion
Add error handling for navigation failures.
The navigation step cy.awaitUrl("/") should include error handling to ensure the test fails gracefully if navigation fails.
- cy.awaitUrl("/");
+ cy.awaitUrl("/").then((url) => {
+ if (!url.includes('/')) {
+ throw new Error('Navigation to home page failed');
+ }
+ });⛔ Skipped due to learnings
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.
cypress/pageobject/Facility/FacilityManage.ts (1)
21-27: LGTM! Well-implemented UI interaction methods.
The click handler methods are well-implemented using the custom verifyAndClickElement helper.
src/components/Common/AvatarEditModal.tsx (2)
Line range hint 285-291: LGTM! Well-implemented delete functionality.
The delete button implementation is clean and includes proper disabled state handling during processing.
Line range hint 1-1: Verify API endpoint consistency across the codebase.
Let's verify that the API endpoints used in the tests match the actual endpoints used in the application.
✅ Verification successful
Let me verify the HTTP methods used with these endpoints by checking the component implementation.
Let me check the actual implementation of the avatar/image upload functionality in the component.
API endpoint consistency verification completed
Based on the search results, the API endpoints for cover image operations are consistently defined and used across the codebase:
- API definition in
src/Utils/request/api.tsx:/api/v1/facility/{id}/cover_image/ - Usage in
src/components/Facility/FacilityHome.tsx:/api/v1/facility/${facilityId}/cover_image/ - Test intercepts in
cypress/pageobject/Facility/FacilityManage.tsmatch the pattern:/api/v1/facility/*/cover_image/
The HTTP methods are also consistent:
- POST for upload (FacilityManage.ts)
- DELETE for removal (api.tsx and FacilityManage.ts)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for API endpoint definitions and usage
rg -A 2 "api/v1/facility/.*/cover_image" --type ts
Length of output: 945
Script:
#!/bin/bash
# Search for the implementation in AvatarEditModal.tsx
rg -A 5 "cover_image" src/components/Common/AvatarEditModal.tsx
Length of output: 63
Script:
#!/bin/bash
# Search for upload-related code in AvatarEditModal
ast-grep --pattern 'const $_ = async ($$$) => {
$$$
upload$$$
$$$
}' src/components/Common/AvatarEditModal.tsx
Length of output: 118
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
cypress/e2e/facility_spec/FacilityCoverImage.cy.ts (2)
12-21: Consider moving test data to fixtures.While the test data structure is clear, hardcoded facility names could make tests brittle. Consider:
- Moving test data to a separate fixture file
- Using dynamic facility creation or a test data factory pattern
Example refactor:
// cypress/fixtures/facility-roles.json { "rolesAndFacility": [ { "role": "districtAdmin", "facilityName": "Dummy Facility 40" }, { "role": "devDoctor", "facilityName": "Dummy Facility 4" } ] } // In test file let rolesAndFacility: IRoleAndFacility[]; before(() => { cy.fixture('facility-roles.json').then((data) => { rolesAndFacility = data.rolesAndFacility; }); });
23-29: Consider extracting configuration values.Move configuration values like viewport dimensions and notification text to a central configuration file or Cypress config for better maintainability.
Example:
// cypress/config/test-config.ts export const VIEWPORT = { width: 1920, height: 1080 }; export const NOTIFICATIONS = { coverImageUpdate: "Cover image updated." };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cypress/e2e/facility_spec/FacilityCoverImage.cy.ts(1 hunks)
🔇 Additional comments (2)
cypress/e2e/facility_spec/FacilityCoverImage.cy.ts (2)
1-10: LGTM! Well-structured imports and type definitions.
The imports are well-organized, and the interface definition uses TypeScript features effectively for type safety.
45-73: Add negative test cases and image validation tests.
The current test suite only covers happy paths.
|
👋 Hi, @JavidSumra, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
|
Closing due to no activity |
|
Hey @rithviknishad got some college work so due to that not able to contribute can you please open it again? |
|
Hey @nihal467, I have a doubt regarding this test suite. Since the upload and edit cover image procedures use the same API call, can we replace the edit procedure test with a test for an invalid image width instead? |
You can do that: verify the error message, replace the existing image with a new one, and complete the test. |
…Sumra/care_fe into issues/9109/image-upload-test-suit
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
cypress/pageobject/Facility/FacilityManage.ts (1)
111-131: 🛠️ Refactor suggestionEnhance network request handling reliability.
The network request interception methods need improvements for better reliability and flexibility.
- Add timeouts to prevent test hanging:
verifyImageUploadRequest(expectedCode: number = 200) { - cy.wait("@imageUploadReq") + cy.wait("@imageUploadReq", { timeout: 10000 }) .its("response.statusCode") .should("eq", expectedCode); } verifyImageDeleteRequest() { - cy.wait("@imageDeleteReq").its("response.statusCode").should("eq", 204); + cy.wait("@imageDeleteReq", { timeout: 10000 }) + .its("response.statusCode") + .should("eq", 204); }
- Add error handling for failed requests:
verifyImageUploadRequest(expectedCode: number = 200) { cy.wait("@imageUploadReq", { timeout: 10000 }) - .its("response.statusCode") - .should("eq", expectedCode); + .then((interception) => { + if (interception.error) { + throw new Error(`Request failed: ${interception.error}`); + } + expect(interception.response?.statusCode).to.equal(expectedCode); + }); }
- Make the delete status code configurable like upload:
- verifyImageDeleteRequest() { + verifyImageDeleteRequest(expectedCode: number = 204) { cy.wait("@imageDeleteReq", { timeout: 10000 }) .its("response.statusCode") - .should("eq", 204); + .should("eq", expectedCode); }
🧹 Nitpick comments (1)
cypress/pageobject/Facility/FacilityManage.ts (1)
29-51: Add visibility checks for capacity-related elements.The capacity management methods should verify element visibility before interaction to prevent flaky tests.
Consider adding visibility checks:
verifyTotalDoctorCapacity(expectedCapacity: string) { - cy.get("#facility-doctor-totalcapacity").contains(expectedCapacity); + cy.get("#facility-doctor-totalcapacity") + .should('be.visible') + .contains(expectedCapacity); } clickEditFacilityDoctorCapacity() { - cy.get("#edit-facility-doctorcapacity").click(); + cy.get("#edit-facility-doctorcapacity") + .should('be.visible') + .click(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cypress/e2e/facility_spec/FacilityCoverImage.cy.ts(1 hunks)cypress/pageobject/Facility/FacilityManage.ts(2 hunks)src/components/Common/AvatarEditModal.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/Common/AvatarEditModal.tsx
- cypress/e2e/facility_spec/FacilityCoverImage.cy.ts
🔇 Additional comments (2)
cypress/pageobject/Facility/FacilityManage.ts (2)
21-27: LGTM! Well-structured cover image management methods.
The methods are concise, follow the page object pattern, and make good use of the custom helper for element verification.
101-109: LGTM! Well-implemented type management methods.
Good practice using scrollIntoView to ensure element visibility before interaction.
rithviknishad
added
needs review
and removed
question
…Sumra/care_fe into issues/9109/image-upload-test-suit
|
no longer relevant in new architecture cc: @nihal467 |
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Enhancements