Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added cypress tests for Facility cover image button functionality #9134

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions cypress/e2e/facility_spec/FacilityCoverImage.cy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import FacilityPage from "../../pageobject/Facility/FacilityCreation";
import FacilityManage from "../../pageobject/Facility/FacilityManage";

const roleUserFacilities = [
{ username: "devdistrictadmin", facilityName: "Dummy Facility 40" },
{ username: "devdoctor", facilityName: "Dummy Facility 4" },
];

roleUserFacilities.forEach(({ username, facilityName }) => {
describe("Facility Cover Image Functionality", () => {
const facilityManage = new FacilityManage();
const facilityPage = new FacilityPage();

before(() => {
cy.loginByApi(username, "Coronasafe@123");
cy.saveLocalStorage();
});
Comment on lines +14 to +17
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error handling for login failures

The login step should handle potential failures gracefully.

 before(() => {
-  cy.loginByApi(username, "Coronasafe@123");
+  cy.loginByApi(username, "Coronasafe@123").then((response) => {
+    expect(response.status).to.eq(200);
+  });
   cy.saveLocalStorage();
 });
📝 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.

Suggested change
before(() => {
cy.loginByApi(username, "Coronasafe@123");
cy.saveLocalStorage();
});
before(() => {
cy.loginByApi(username, "Coronasafe@123").then((response) => {
expect(response.status).to.eq(200);
});
cy.saveLocalStorage();
});


beforeEach(() => {
cy.viewport(1280, 720);
cy.restoreLocalStorage();
cy.clearLocalStorage(/filters--.+/);
cy.awaitUrl("/");
facilityPage.typeFacilitySearch(facilityName);
facilityPage.verifyFacilityBadgeContent(facilityName);
facilityPage.visitAlreadyCreatedFacility();
});
Comment on lines +19 to +27
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Implement API intercepts as specified in PR objectives

The PR objectives mention using cy.intercept() for API verification, but these are missing in the implementation.

Add API intercepts before navigation:

 beforeEach(() => {
   cy.viewport(1280, 720);
   cy.restoreLocalStorage();
   cy.clearLocalStorage(/filters--.+/);
+  cy.intercept('GET', '/api/v1/facility/*').as('getFacility');
+  cy.intercept('POST', '/api/v1/facility/*/cover_image').as('uploadCoverImage');
+  cy.intercept('DELETE', '/api/v1/facility/*/cover_image').as('deleteCoverImage');
   cy.awaitUrl("/");
   facilityPage.typeFacilitySearch(facilityName);
   facilityPage.verifyFacilityBadgeContent(facilityName);
   facilityPage.visitAlreadyCreatedFacility();
+  cy.wait('@getFacility');
 });
📝 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.

Suggested change
beforeEach(() => {
cy.viewport(1280, 720);
cy.restoreLocalStorage();
cy.clearLocalStorage(/filters--.+/);
cy.awaitUrl("/");
facilityPage.typeFacilitySearch(facilityName);
facilityPage.verifyFacilityBadgeContent(facilityName);
facilityPage.visitAlreadyCreatedFacility();
});
beforeEach(() => {
cy.viewport(1280, 720);
cy.restoreLocalStorage();
cy.clearLocalStorage(/filters--.+/);
cy.intercept('GET', '/api/v1/facility/*').as('getFacility');
cy.intercept('POST', '/api/v1/facility/*/cover_image').as('uploadCoverImage');
cy.intercept('DELETE', '/api/v1/facility/*/cover_image').as('deleteCoverImage');
cy.awaitUrl("/");
facilityPage.typeFacilitySearch(facilityName);
facilityPage.verifyFacilityBadgeContent(facilityName);
facilityPage.visitAlreadyCreatedFacility();
cy.wait('@getFacility');
});


it("Facility Cover Image button functionality", () => {
// It's only button functionality because we can't access S3 bucket in local
// Check: Cancel button functionality
facilityManage.clickCoverImage();
facilityManage.clickCancelButton();
facilityManage.verifyCoverImageModalClosed();
// Check: upload an image, verifying the API response.
facilityManage.clickCoverImage();
facilityManage.verifyUploadButtonVisible();
facilityManage.uploadCoverImage("facility-cover-image.jpg");
facilityManage.clickSaveCoverImage();
facilityManage.verifyCoverImageModalClosed();
// //Check: Edit the current cover image by uploading a new one, and verify the API response.
facilityManage.clickCoverImage();
facilityManage.verifyUploadButtonVisible();
facilityManage.uploadCoverImage("new-facility-cover-image.jpg");
facilityManage.clickSaveCoverImage();
facilityManage.verifyCoverImageModalClosed();
//Check: Delete the existing cover image, confirming the API response.
facilityManage.clickCoverImage();
facilityManage.clickDeleteCoverImage();
});
Comment on lines +29 to +50
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Enhance test assertions and API verifications

The test case lacks proper assertions after API calls and UI state verifications.

Add assertions and API verifications:

 it("Facility Cover Image button functionality", () => {
-  // It's only button functionality because we can't access S3 bucket in local
+  // Note: This test verifies UI interactions and API calls without actual S3 bucket operations
+  // as S3 access is not available in local development environment
   
   // Check: Cancel button functionality
   facilityManage.clickCoverImage();
   facilityManage.clickCancelButton();
   facilityManage.verifyCoverImageModalClosed();
   
   // Check: upload an image, verifying the API response.
   facilityManage.clickCoverImage();
   facilityManage.verifyUploadButtonVisible();
   facilityManage.uploadCoverImage("facility-cover-image.jpg");
   facilityManage.clickSaveCoverImage();
+  cy.wait('@uploadCoverImage').its('response.statusCode').should('eq', 200);
   facilityManage.verifyCoverImageModalClosed();
+  facilityManage.verifyCoverImageVisible();
   
   // Check: Edit the current cover image
   facilityManage.clickCoverImage();
   facilityManage.verifyUploadButtonVisible();
   facilityManage.uploadCoverImage("new-facility-cover-image.jpg");
   facilityManage.clickSaveCoverImage();
+  cy.wait('@uploadCoverImage').its('response.statusCode').should('eq', 200);
   facilityManage.verifyCoverImageModalClosed();
+  facilityManage.verifyCoverImageVisible();
   
   // Check: Delete the existing cover image
   facilityManage.clickCoverImage();
   facilityManage.clickDeleteCoverImage();
+  cy.wait('@deleteCoverImage').its('response.statusCode').should('eq', 200);
+  facilityManage.verifyCoverImageNotVisible();
 });

Committable suggestion skipped: line range outside the PR's diff.


afterEach(() => {
cy.saveLocalStorage();
});
Comment on lines +52 to +54
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add proper test cleanup

The test should clean up any resources it creates to maintain test isolation.

 afterEach(() => {
   cy.saveLocalStorage();
+  // Clean up any uploaded images to maintain test isolation
+  cy.get('@deleteCoverImage').then((interception) => {
+    if (interception) {
+      facilityManage.clickCoverImage();
+      facilityManage.clickDeleteCoverImage();
+      cy.wait('@deleteCoverImage');
+    }
+  });
 });

Committable suggestion skipped: line range outside the PR's diff.

});
});
8 changes: 0 additions & 8 deletions cypress/e2e/facility_spec/FacilityManage.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,6 @@ describe("Facility Manage Functions", () => {
facilityPage.visitAlreadyCreatedFacility();
});

it("Facility Cover Image button functionality", () => {
// It's only button functionality because we can't access S3 bucket in local
facilityManage.clickCoverImage();
facilityManage.verifyUploadButtonVisible();
facilityManage.uploadCoverImage("facility-cover-image.jpg");
facilityManage.clickSaveCoverImage();
});

it("Configure Facility Middleware", () => {
facilityPage.clickManageFacilityDropdown();
facilityManage.clickFacilityConfigureButton();
Expand Down
Binary file modified cypress/fixtures/facility-cover-image.jpg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added cypress/fixtures/facility-cover-image1.jpg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added cypress/fixtures/new-facility-cover-image.jpg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
24 changes: 21 additions & 3 deletions cypress/pageobject/Facility/FacilityManage.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,20 @@
/// <reference types="cypress" />

class FacilityManage {
clickCoverImage() {
cy.get("#facility-coverimage").click();
}

clickCancelButton() {
cy.get("#cancel").scrollIntoView().should("be.visible").click();
}
Comment on lines +8 to +10
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Use a more specific selector for the cancel button

The current selector '#cancel' is too generic and could match unintended elements on the page. Consider using a more specific selector that includes the context of the cover image modal.

-    cy.get("#cancel").scrollIntoView().should("be.visible").click();
+    cy.get("[data-testid='cover-image-modal-cancel']").scrollIntoView().should("be.visible").click();
📝 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.

Suggested change
clickCancelButton() {
cy.get("#cancel").scrollIntoView().should("be.visible").click();
}
clickCancelButton() {
cy.get("[data-testid='cover-image-modal-cancel']").scrollIntoView().should("be.visible").click();
}


verifyCoverImageModalClosed() {
cy.get("#upload-cover-image").should("not.exist");
}
Comment on lines +12 to +14
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Verify modal closure using the modal container

The current implementation checks for the upload button's non-existence, which might not accurately represent whether the modal is closed. Consider checking the modal container element instead.

-    cy.get("#upload-cover-image").should("not.exist");
+    cy.get("[data-testid='cover-image-modal']").should("not.exist");

Committable suggestion skipped: line range outside the PR's diff.


verifyUploadButtonVisible() {
cy.get("#upload-cover-image").should("be.visible");
cy.get("#upload-cover-image").scrollIntoView().should("be.visible");
}

uploadCoverImage(fileName: string) {
Expand All @@ -14,8 +24,16 @@ class FacilityManage {
}

clickSaveCoverImage() {
cy.get("#save-cover-image").scrollIntoView();
cy.get("#save-cover-image").click();
cy.intercept("POST", "**/api/v1/facility/**").as("uploadCoverImage");
cy.get("#save-cover-image").scrollIntoView().click();
cy.wait("@uploadCoverImage").its("response.statusCode").should("eq", 200);
}
Comment on lines +27 to +30
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve API intercept and error handling

The current implementation has several areas for improvement:

  1. The API route wildcard could match unintended requests
  2. No handling of failed uploads
  3. Multiple responsibilities in a single method
-    cy.intercept("POST", "**/api/v1/facility/**").as("uploadCoverImage");
-    cy.get("#save-cover-image").scrollIntoView().click();
-    cy.wait("@uploadCoverImage").its("response.statusCode").should("eq", 200);
+    cy.intercept("POST", "**/api/v1/facility/*/cover_image").as("uploadCoverImage");
+    cy.get("#save-cover-image").scrollIntoView().should("be.visible").click();
+    cy.wait("@uploadCoverImage").then((interception) => {
+      if (interception.response?.statusCode !== 200) {
+        throw new Error(`Failed to upload cover image: ${interception.response?.statusCode}`);
+      }
+    });
📝 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.

Suggested change
cy.intercept("POST", "**/api/v1/facility/**").as("uploadCoverImage");
cy.get("#save-cover-image").scrollIntoView().click();
cy.wait("@uploadCoverImage").its("response.statusCode").should("eq", 200);
}
cy.intercept("POST", "**/api/v1/facility/*/cover_image").as("uploadCoverImage");
cy.get("#save-cover-image").scrollIntoView().should("be.visible").click();
cy.wait("@uploadCoverImage").then((interception) => {
if (interception.response?.statusCode !== 200) {
throw new Error(`Failed to upload cover image: ${interception.response?.statusCode}`);
}
});


clickDeleteCoverImage() {
cy.intercept("DELETE", "**/api/v1/facility/**").as("deleteCoverImage");
cy.get("#delete-cover-image").scrollIntoView();
cy.get("#delete-cover-image").click();
cy.wait("@deleteCoverImage").its("response.statusCode").should("eq", 204);
Comment on lines +32 to +36
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Fix duplicate scrollIntoView and improve error handling

The current implementation has several issues:

  1. Duplicate scrollIntoView calls
  2. The API route wildcard could match unintended requests
  3. No handling of failed deletions
-    cy.intercept("DELETE", "**/api/v1/facility/**").as("deleteCoverImage");
-    cy.get("#delete-cover-image").scrollIntoView();
-    cy.get("#delete-cover-image").click();
-    cy.wait("@deleteCoverImage").its("response.statusCode").should("eq", 204);
+    cy.intercept("DELETE", "**/api/v1/facility/*/cover_image").as("deleteCoverImage");
+    cy.get("#delete-cover-image").scrollIntoView().should("be.visible").click();
+    cy.wait("@deleteCoverImage").then((interception) => {
+      if (interception.response?.statusCode !== 204) {
+        throw new Error(`Failed to delete cover image: ${interception.response?.statusCode}`);
+      }
+    });
📝 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.

Suggested change
clickDeleteCoverImage() {
cy.intercept("DELETE", "**/api/v1/facility/**").as("deleteCoverImage");
cy.get("#delete-cover-image").scrollIntoView();
cy.get("#delete-cover-image").click();
cy.wait("@deleteCoverImage").its("response.statusCode").should("eq", 204);
clickDeleteCoverImage() {
cy.intercept("DELETE", "**/api/v1/facility/*/cover_image").as("deleteCoverImage");
cy.get("#delete-cover-image").scrollIntoView().should("be.visible").click();
cy.wait("@deleteCoverImage").then((interception) => {
if (interception.response?.statusCode !== 204) {
throw new Error(`Failed to delete cover image: ${interception.response?.statusCode}`);
}
});

}

verifyTotalDoctorCapacity(expectedCapacity: string) {
Expand Down
1 change: 1 addition & 0 deletions src/components/Common/AvatarEditModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,7 @@ const AvatarEditModal = ({
{imageUrl && (
<ButtonV2
variant="danger"
id="delete-cover-image"
onClick={deleteAvatar}
disabled={isProcessing}
>
Expand Down
Loading