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 shifting request #9035

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open
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
94 changes: 94 additions & 0 deletions cypress/e2e/shifting_spec/ShiftingRequest.cy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import LoginPage from "pageobject/Login/LoginPage";
import { PatientConsultationPage } from "pageobject/Patient/PatientConsultation";
import { PatientPage } from "pageobject/Patient/PatientCreation";
import ShiftCreation from "pageobject/Shift/ShiftCreation";
import { ShiftDetails } from "pageobject/Shift/ShiftDetails";
import { ShiftingBoard } from "pageobject/Shift/ShiftingBoard";
import ShiftUpdate from "pageobject/Shift/ShiftUpdate";

describe("New shifting request", () => {
const loginPage = new LoginPage();
const shiftCreation = new ShiftCreation();
const shiftingBoard = new ShiftingBoard();
const shiftDetails = new ShiftDetails();
const shiftUpdate = new ShiftUpdate();
const patientPage = new PatientPage();
const patientConsultationPage = new PatientConsultationPage();

const patientName = "Dummy Patient 16";
const currentFacilityPerson = "new";
const currentFacilityPhone = "9465666768";
const shiftReasonInitial = "emmergency";
const ambulanceDriverNameInitial = "Rahul";
const ambulancePhoneInitial = "9865666768";
const ambulanceNumberInitial = "1";
const commentInitial = "Some comment";

const shiftReasonUpdated = "new reason";
const ambulanceDriverNameUpdated = "Ramesh";
const ambulancePhoneUpdated = "9755443232";
const ambulanceNumberUpdated = "2";
const commentUpdated = "New comment";
Comment on lines +18 to +31
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

Move test data to a separate constants file and fix typo.

  1. Move these test data constants to a separate file (e.g., cypress/fixtures/shiftingRequest.json) for better maintainability and reusability.
  2. Fix the typo: emmergencyemergency
- const shiftReasonInitial = "emmergency";
+ const shiftReasonInitial = "emergency";

Create a new file cypress/fixtures/shiftingRequest.json:

{
  "patient": {
    "name": "Dummy Patient 16"
  },
  "initial": {
    "currentFacilityPerson": "new",
    "currentFacilityPhone": "9465666768",
    "shiftReason": "emergency",
    "ambulanceDriverName": "Rahul",
    "ambulancePhone": "9865666768",
    "ambulanceNumber": "1",
    "comment": "Some comment"
  },
  "updated": {
    "shiftReason": "new reason",
    "ambulanceDriverName": "Ramesh",
    "ambulancePhone": "9755443232",
    "ambulanceNumber": "2",
    "comment": "New comment"
  }
}


before(() => {
loginPage.loginAsDistrictAdmin();
cy.saveLocalStorage();
});

beforeEach(() => {
cy.restoreLocalStorage();
});

it("Shift Request Creation and Update", () => {
cy.visit('/patients');
patientPage.visitPatient(patientName);
patientConsultationPage.clickShiftPatientButton();

shiftCreation.typeCurrentFacilityPerson(currentFacilityPerson);
shiftCreation.typeCurrentFacilityPhone(currentFacilityPhone);
shiftCreation.typeShiftReason(shiftReasonInitial);
shiftCreation.typeAmbulanceDriverName(ambulanceDriverNameInitial);
shiftCreation.typeAmbulancePhone(ambulancePhoneInitial);
shiftCreation.typeAmbulanceNumber(ambulanceNumberInitial);
shiftCreation.typeComment(commentInitial);
shiftCreation.interceptShiftCreationRequest();
shiftCreation.submitShiftForm();

cy.wait("@shiftRequest").its("response.statusCode").should("eq", 201);
Comment on lines +54 to +57
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

Move API intercept before form interactions.

The intercept should be placed before any form interactions to ensure no requests are missed.

+   shiftCreation.interceptShiftCreationRequest();
    shiftCreation.typeCurrentFacilityPerson(currentFacilityPerson);
    shiftCreation.typeCurrentFacilityPhone(currentFacilityPhone);
    // ... other form interactions ...
-   shiftCreation.interceptShiftCreationRequest();
    shiftCreation.submitShiftForm();
📝 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
shiftCreation.interceptShiftCreationRequest();
shiftCreation.submitShiftForm();
cy.wait("@shiftRequest").its("response.statusCode").should("eq", 201);
shiftCreation.interceptShiftCreationRequest();
shiftCreation.typeCurrentFacilityPerson(currentFacilityPerson);
shiftCreation.submitShiftForm();
cy.wait("@shiftRequest").its("response.statusCode").should("eq", 201);
🧰 Tools
🪛 eslint

[error] 56-56: Delete ····

(prettier/prettier)


cy.visit("/shifting/board");
cy.contains(patientName).should("exist");

Comment on lines +59 to +61
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 initial data after creation.

After creating the shift request, verify that all initial data is correctly reflected in the shift details page before proceeding with updates.

cy.visit("/shifting/board");
cy.contains(patientName).should("exist");

// Verify initial data
shiftingBoard.openDetails(patientName);
cy.get('[data-testid="shift-details-name"]')
  .should("be.visible")
  .and("have.text", patientName);
// Add more verifications for other fields
🧰 Tools
🪛 eslint

[error] 61-61: Delete ··

(prettier/prettier)

Comment on lines +42 to +61
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

Split creation and update into separate test cases.

The current test combines creation and update flows. This makes the test harder to maintain and debug. Consider splitting into separate test cases:

  1. "Should create a new shift request"
  2. "Should update an existing shift request"
🧰 Tools
🪛 eslint

[error] 43-43: Replace '/patients' with "/patients"

(prettier/prettier)


[error] 46-46: Delete ····

(prettier/prettier)


[error] 56-56: Delete ····

(prettier/prettier)


[error] 61-61: Delete ··

(prettier/prettier)

shiftingBoard.openDetails(patientName);
shiftDetails.clickUpdateStatusButton();

shiftUpdate.typeShiftReason(shiftReasonUpdated);
shiftUpdate.typeAmbulanceDriverName(ambulanceDriverNameUpdated);
shiftUpdate.typeAmbulancePhone(ambulancePhoneUpdated);
shiftUpdate.typeAmbulanceNumber(ambulanceNumberUpdated);
shiftUpdate.typeComment(commentUpdated);
shiftUpdate.interceptShiftUpdateRequest();
shiftUpdate.submitShiftForm();

Copy link
Member

Choose a reason for hiding this comment

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

remove unwanted spaces

cy.wait('@shiftUpdateRequest').then((interception) => {
const responseData = interception.response.body;
expect(responseData.patient_object.name).to.eq(patientName);
expect(responseData.ambulance_phone_number).to.eq(`+91${ambulancePhoneUpdated}`);
expect(responseData.comments).to.eq(commentUpdated);
expect(responseData.reason).to.eq(shiftReasonUpdated);
expect(responseData.ambulance_driver_name).to.eq(ambulanceDriverNameUpdated);
expect(responseData.ambulance_number).to.eq(ambulanceNumberUpdated);
});

cy.get('#shift_details_name').should('be.visible').and('have.text', patientName);
cy.get('#shift_details_ambulance_phone_number').should('be.visible').and('have.text', `+91${ambulancePhoneUpdated}`);
cy.get('#shift_details_ambulance_number').should('be.visible').and('have.text', ambulanceNumberUpdated);
cy.get('#shift_details_comments').should('be.visible').and('have.text', commentUpdated);
cy.get('#shift_details_reason').should('be.visible').and('have.text', shiftReasonUpdated);
cy.get('#shift_details_ambulance_driver_name').should('be.visible').and('have.text', ambulanceDriverNameUpdated);
Comment on lines +83 to +88
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 data-testid instead of ID selectors.

For better maintainability and resilience to UI changes, use data-testid attributes instead of ID selectors.

-   cy.get('#shift_details_name')
+   cy.get('[data-testid="shift-details-name"]')
    .should('be.visible')
    .and('have.text', patientName);
    // Apply similar changes to other selectors

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

🧰 Tools
🪛 eslint

[error] 83-83: Replace '#shift_details_name').should('be.visible').and('have.text' with "#shift_details_name")⏎······.should("be.visible")⏎······.and("have.text"

(prettier/prettier)


[error] 84-84: Replace '#shift_details_ambulance_phone_number').should('be.visible').and('have.text' with "#shift_details_ambulance_phone_number")⏎······.should("be.visible")⏎······.and("have.text"

(prettier/prettier)


[error] 85-85: Replace '#shift_details_ambulance_number').should('be.visible').and('have.text' with "#shift_details_ambulance_number")⏎······.should("be.visible")⏎······.and("have.text"

(prettier/prettier)


[error] 86-86: Replace '#shift_details_comments').should('be.visible').and('have.text' with "#shift_details_comments")⏎······.should("be.visible")⏎······.and("have.text"

(prettier/prettier)


[error] 87-87: Replace '#shift_details_reason').should('be.visible').and('have.text' with "#shift_details_reason")⏎······.should("be.visible")⏎······.and("have.text"

(prettier/prettier)


[error] 88-88: Replace '#shift_details_ambulance_driver_name').should('be.visible').and('have.text' with "#shift_details_ambulance_driver_name")⏎······.should("be.visible")⏎······.and("have.text"

(prettier/prettier)


});
Comment on lines +42 to +90
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 test cases for error scenarios.

The test suite should include error scenarios such as:

  1. API failures
  2. Validation errors
  3. Network issues

Example test case:

it("Should handle API errors when creating shift request", () => {
  cy.intercept('POST', '**/api/v1/shift/**', {
    statusCode: 500,
    body: { error: 'Internal Server Error' }
  }).as('failedShiftRequest');
  
  // Perform creation steps
  shiftCreation.submitShiftForm();
  
  // Verify error handling
  cy.get('[data-testid="error-message"]').should('be.visible');
});
🧰 Tools
🪛 eslint

[error] 43-43: Replace '/patients' with "/patients"

(prettier/prettier)


[error] 46-46: Delete ····

(prettier/prettier)


[error] 56-56: Delete ····

(prettier/prettier)


[error] 61-61: Delete ··

(prettier/prettier)


[error] 73-73: Replace '@shiftUpdateRequest' with "@shiftUpdateRequest"

(prettier/prettier)


[error] 76-76: Replace +91${ambulancePhoneUpdated} with ⏎········+91${ambulancePhoneUpdated},⏎······

(prettier/prettier)


[error] 79-79: Replace ambulanceDriverNameUpdated with ⏎········ambulanceDriverNameUpdated,⏎······

(prettier/prettier)


[error] 83-83: Replace '#shift_details_name').should('be.visible').and('have.text' with "#shift_details_name")⏎······.should("be.visible")⏎······.and("have.text"

(prettier/prettier)


[error] 84-84: Replace '#shift_details_ambulance_phone_number').should('be.visible').and('have.text' with "#shift_details_ambulance_phone_number")⏎······.should("be.visible")⏎······.and("have.text"

(prettier/prettier)


[error] 85-85: Replace '#shift_details_ambulance_number').should('be.visible').and('have.text' with "#shift_details_ambulance_number")⏎······.should("be.visible")⏎······.and("have.text"

(prettier/prettier)


[error] 86-86: Replace '#shift_details_comments').should('be.visible').and('have.text' with "#shift_details_comments")⏎······.should("be.visible")⏎······.and("have.text"

(prettier/prettier)


[error] 87-87: Replace '#shift_details_reason').should('be.visible').and('have.text' with "#shift_details_reason")⏎······.should("be.visible")⏎······.and("have.text"

(prettier/prettier)


[error] 88-88: Replace '#shift_details_ambulance_driver_name').should('be.visible').and('have.text' with "#shift_details_ambulance_driver_name")⏎······.should("be.visible")⏎······.and("have.text"

(prettier/prettier)


[error] 89-90: Delete ⏎··

(prettier/prettier)

afterEach(() => {
cy.saveLocalStorage();
});
});
10 changes: 10 additions & 0 deletions cypress/pageobject/Patient/PatientConsultation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,4 +139,14 @@ export class PatientConsultationPage {
cy.get("#consultation-buttons").contains("Claims").click();
cy.wait("@policyStatus").its("response.statusCode").should("eq", 200);
}

clickShiftPatientButton() {
cy.get("#consultation-buttons").scrollIntoView();
cy.get("button").contains("Manage Patient").click();
cy.verifyAndClickElement(
"#consultation-buttons",
"Shift Patient",
);
cy.wait(3000);
Comment on lines +143 to +150
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Inconsistent selector usage needs to be fixed

Based on the evidence:

  • clickManagePatientButton() uses #show-more selector with verifyAndClickElement
  • clickShiftPatientButton() uses #consultation-buttons with a combination of scrollIntoView and direct button selection
  • Both selectors are actively used in the codebase for different purposes:
    • #show-more is used consistently for "Manage Patient" actions in both PatientConsultation and PatientDischarge
    • #consultation-buttons is used for various consultation-related actions and content verification

The implementation in clickShiftPatientButton() should be aligned with clickManagePatientButton() to use the #show-more selector for consistency, as this appears to be the standard way to access the "Manage Patient" button across the codebase.

🔗 Analysis chain

Verify and align selector usage across similar methods

There's an inconsistency in how the "Manage Patient" button is accessed across different methods:

  • clickManagePatientButton() uses #show-more
  • clickShiftPatientButton() uses #consultation-buttons

Please verify which selector should be used and align the implementation across all methods.

Also applies to: 104-106


Let me gather more information about the clickManagePatientButton method to understand the full context.


Let me try a different approach to find the method implementation.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find all usages of these selectors to determine the correct one

echo "Searching for #show-more selector..."
rg --type ts "#show-more"

echo -e "\nSearching for #consultation-buttons selector..."
rg --type ts "#consultation-buttons"

Length of output: 2081


Script:

#!/bin/bash
# Find the implementation of clickManagePatientButton
ast-grep --pattern 'clickManagePatientButton() {
  $$$
}'

Length of output: 59


Script:

#!/bin/bash
# Search for clickManagePatientButton in typescript files
rg --type ts "clickManagePatientButton" -A 5

Length of output: 1046

🧰 Tools
🪛 Biome (1.9.4)

[error] 142-142: Expected a semicolon or an implicit semicolon after a statement, but found none

An explicit or implicit semicolon is expected here...

...Which is required to end this statement

(parse)


[error] 142-150: This block statement doesn't serve any purpose and can be safely removed.

Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.

(lint/complexity/noUselessLoneBlockStatements)

}
Comment on lines +143 to +151
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

Refactor to reduce code duplication and improve reliability

The implementation duplicates logic from clickEditConsultationButton(). Consider extracting common functionality and using Cypress's built-in waiting mechanisms.

Here's a suggested refactor:

+ private clickManagePatientButton() {
+   cy.get("#consultation-buttons").scrollIntoView();
+   cy.get("button").contains("Manage Patient").click();
+ }

  clickEditConsultationButton() {
-   cy.get("#consultation-buttons").scrollIntoView();
-   cy.get("button").contains("Manage Patient").click();
+   this.clickManagePatientButton();
    cy.verifyAndClickElement(
      "#consultation-buttons",
      "Edit Consultation Details",
-   );
-   cy.wait(3000);
+   ).should('not.exist');  // Wait for menu to close
  }

  clickShiftPatientButton() {
-   cy.get("#consultation-buttons").scrollIntoView();
-   cy.get("button").contains("Manage Patient").click();
+   this.clickManagePatientButton();
    cy.verifyAndClickElement(
      "#consultation-buttons",
      "Shift Patient",
-   );
-   cy.wait(3000);
+   ).should('not.exist');  // Wait for menu to close
  }

The changes:

  1. Extract common "Manage Patient" click logic
  2. Replace fixed wait with Cypress's built-in assertions
  3. Fix formatting per static analysis
📝 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
clickShiftPatientButton() {
cy.get("#consultation-buttons").scrollIntoView();
cy.get("button").contains("Manage Patient").click();
cy.verifyAndClickElement(
"#consultation-buttons",
"Shift Patient",
);
cy.wait(3000);
}
private clickManagePatientButton() {
cy.get("#consultation-buttons").scrollIntoView();
cy.get("button").contains("Manage Patient").click();
}
clickEditConsultationButton() {
this.clickManagePatientButton();
cy.verifyAndClickElement(
"#consultation-buttons",
"Edit Consultation Details"
).should('not.exist'); // Wait for menu to close
}
clickShiftPatientButton() {
this.clickManagePatientButton();
cy.verifyAndClickElement(
"#consultation-buttons",
"Shift Patient"
).should('not.exist'); // Wait for menu to close
}
🧰 Tools
🪛 eslint

[error] 119-122: Replace ⏎······"#consultation-buttons",⏎······"Shift·Patient",⏎···· with "#consultation-buttons",·"Shift·Patient"

(prettier/prettier)

}
22 changes: 21 additions & 1 deletion cypress/pageobject/Shift/ShiftCreation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,28 @@ export class ShiftCreation {
cy.get("#reason").click().type(reason);
}

typeAmbulanceDriverName(name: string) {
cy.get("#ambulance_driver_name").click().type(name);
}

typeAmbulancePhone(number: string) {
cy.get("#ambulance_phone_number").click().type(number);
}

typeAmbulanceNumber(number: string) {
cy.get("#ambulance_number").click().type(number);
}

typeComment(comment:string) {
cy.get("#comments").click().type(comment)
}

submitShiftForm() {
cy.get("#submit").contains("Submit").click();
cy.submitButton("Submit");
}

interceptShiftCreationRequest(){
cy.intercept("POST", "api/v1/shift").as("shiftRequest");
Comment on lines +34 to +35
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

Enhance API interception with response validation.

While the interception is correctly set up, consider adding response validation for more robust testing:

 interceptShiftCreationRequest(){
-    cy.intercept("POST", "api/v1/shift").as("shiftRequest");
+    cy.intercept("POST", "api/v1/shift", (req) => {
+      req.on('response', (res) => {
+        expect(res.statusCode).to.be.oneOf([201, 200]);
+        expect(res.body).to.have.property('id');
+      });
+    }).as("shiftRequest");
 }

Also consider adding a timeout configuration if the API response might be slow:

cy.wait('@shiftRequest', { timeout: 10000 });

}
}
export default ShiftCreation;
5 changes: 5 additions & 0 deletions cypress/pageobject/Shift/ShiftDetails.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export class ShiftDetails{
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 TypeScript interface and documentation for the ShiftDetails class.

Following TypeScript best practices, let's add proper interface and documentation.

+/**
+ * Interface defining the contract for shift details operations
+ */
+interface IShiftDetails {
+  clickUpdateStatusButton(): Cypress.Chainable<void>;
+}
+
+/**
+ * Page Object Model class for handling shift details operations in Cypress tests
+ * @implements {IShiftDetails}
+ */
-export class ShiftDetails{
+export class ShiftDetails implements IShiftDetails {
📝 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
export class ShiftDetails{
/**
* Interface defining the contract for shift details operations
*/
interface IShiftDetails {
clickUpdateStatusButton(): Cypress.Chainable<void>;
}
/**
* Page Object Model class for handling shift details operations in Cypress tests
* @implements {IShiftDetails}
*/
export class ShiftDetails implements IShiftDetails {
🧰 Tools
🪛 eslint

[error] 1-1: Insert ·

(prettier/prettier)

clickUpdateStatusButton(){
cy.contains("Update Status/Details").click()
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 data-testid for more robust element selection

Using text content for element selection is fragile and can break if the text changes or with i18n. Use data-testid attributes instead.

-        cy.contains("Update Status/Details").click()
+        cy.get('[data-testid="shift-update-status-button"]').click()

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

}
Comment on lines +2 to +4
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 method implementation for better reliability and maintainability

The current implementation has several areas for improvement:

  1. Using text content for element selection is fragile
  2. Missing TypeScript types and documentation
  3. No error handling or assertions
  4. No timeout handling

Apply these improvements:

-    clickUpdateStatusButton(){
-        cy.contains("Update Status/Details").click()
-    }
+    /**
+     * Clicks the update status button in the shift details view
+     * @returns {Cypress.Chainable<void>} Cypress chainable
+     * @throws {Error} If the button is not found or not clickable
+     */
+    clickUpdateStatusButton(): Cypress.Chainable<void> {
+        return cy
+            .get('[data-testid="shift-update-status-button"]', { timeout: 10000 })
+            .should('be.visible')
+            .should('be.enabled')
+            .click();
+    }

Additionally, ensure to add the corresponding data-testid attribute in your component:

<button data-testid="shift-update-status-button">Update Status/Details</button>
🧰 Tools
🪛 eslint

[error] 2-2: Replace ····clickUpdateStatusButton() with ··clickUpdateStatusButton()·

(prettier/prettier)


[error] 3-3: Replace ········cy.contains("Update·Status/Details").click() with ····cy.contains("Update·Status/Details").click();

(prettier/prettier)


[error] 4-4: Delete ··

(prettier/prettier)

}
31 changes: 31 additions & 0 deletions cypress/pageobject/Shift/ShiftUpdate.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
export class ShiftUpdate {
typeShiftReason(reason: string) {
cy.get("#reason").click().clear().type(reason);
}
Comment on lines +2 to +4
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

Enhance form interaction methods with better practices.

The input methods could be improved with:

  1. Input validation
  2. Wait assertions
  3. Better error handling
  4. JSDoc documentation

Here's a suggested implementation pattern:

/** Constants file */
export const SELECTORS = {
  REASON: '#reason',
  DRIVER_NAME: '#ambulance_driver_name',
  // ... other selectors
} as const;

/** Example improved method */
/**
 * Types the shift reason into the reason field
 * @param reason - The reason for the shift
 * @throws {Error} If the reason field is not found
 */
typeShiftReason(reason: string): void {
  if (!reason?.trim()) {
    throw new Error('Shift reason cannot be empty');
  }
  cy.get(SELECTORS.REASON)
    .should('be.visible')
    .click()
    .clear()
    .type(reason, { delay: 50 })
    .should('have.value', reason);
}

Also applies to: 6-8, 10-12, 14-16, 18-20


typeAmbulanceDriverName(name: string) {
cy.get("#ambulance_driver_name").click().clear().type(name);
}

typeAmbulancePhone(number: string) {
Copy link
Member

Choose a reason for hiding this comment

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

  • you have created duplicate variable in two different files

Copy link
Author

Choose a reason for hiding this comment

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

But the shiftUpdate class will require the function to be slightly different than the shiftCreation one , as they will clear the previous value that was already present in the input box . That is why i created the fuctions seperately that might seem to be duplicate . If you want i can get rid of the functions but the clearing the original values wil have to be done in original functions , is that okay ?

cy.get("#ambulance_phone_number").click().clear().type(number);
}

typeAmbulanceNumber(number: string) {
cy.get("#ambulance_number").click().clear().type(number);
}

typeComment(comment: string) {
cy.get("#comments").click().clear().type(comment);
}
Comment on lines +2 to +20
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

Enhance form interaction methods while maintaining clear() functionality.

I understand the necessity of clear() calls for updating existing values. However, the methods could still benefit from:

  1. Input validation
  2. Wait assertions
  3. Error handling
  4. Type definitions for parameters

Example implementation:

interface FormSelectors {
  readonly REASON: '#reason';
  readonly DRIVER_NAME: '#ambulance_driver_name';
  // ... other selectors
}

const SELECTORS: FormSelectors = {
  REASON: '#reason',
  DRIVER_NAME: '#ambulance_driver_name',
  // ... other selectors
} as const;

typeShiftReason(reason: string): void {
  if (!reason?.trim()) {
    throw new Error('Shift reason cannot be empty');
  }
  cy.get(SELECTORS.REASON)
    .should('be.visible')
    .click()
    .clear()
    .type(reason)
    .should('have.value', reason);
}
🧰 Tools
🪛 eslint

[error] 5-5: Delete ··

(prettier/prettier)


[error] 9-9: Delete ··

(prettier/prettier)


[error] 13-13: Delete ··

(prettier/prettier)


[error] 17-17: Delete ··

(prettier/prettier)


submitShiftForm() {
cy.submitButton("Submit");
}
Comment on lines +22 to +24
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

Enhance form submission with loading and success states.

The submission method should handle loading states and verify successful submission.

submitShiftForm(): void {
  cy.submitButton('Submit')
    .should('not.be.disabled');
  
  // Wait for loading state
  cy.get('[data-testid="loading"]')
    .should('exist')
    .should('not.exist');
  
  // Verify successful submission
  cy.get('[data-testid="success-message"]')
    .should('be.visible')
    .and('contain', 'Shift updated successfully');
}


interceptShiftUpdateRequest(){
cy.intercept("PUT", "**/api/v1/shift/**").as("shiftUpdateRequest");
}
}
export default ShiftUpdate;

8 changes: 8 additions & 0 deletions cypress/pageobject/Shift/ShiftingBoard.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
export class ShiftingBoard {
openDetails(patientName) {
cy.get('#patient_name').type(patientName);
cy.intercept("GET", "**/api/v1/shift/**").as("getShiftingRequest");
cy.wait("@getShiftingRequest").its("response.statusCode").should("eq", 200);
Comment on lines +4 to +5
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Make the API intercept pattern more specific to match the actual endpoint

Based on the search results, the broad pattern **/api/v1/shift/** could intercept multiple endpoints:

  • /api/v1/shift/ (GET, POST)
  • /api/v1/shift/{id}/ (GET, PUT, DELETE)
  • /api/v1/shift/{id}/comment/ (GET, POST)
  • /api/v1/shift/{externalId}/transfer/ (POST)

Since this intercept is used in the openDetails method and followed by a status check, it should specifically target the list endpoint:

cy.intercept("GET", "/api/v1/shift/").as("getShiftingRequest");
🔗 Analysis chain

Verify API endpoint pattern and response handling.

The API interception pattern **/api/v1/shift/** is quite broad. Consider making it more specific to avoid intercepting unintended requests.

Let's verify the API endpoint pattern:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for shift-related API endpoints to ensure we're using the correct pattern
rg -g '*.{ts,js,tsx,jsx}' --no-heading "api/v1/shift" -A 2 -B 2

Length of output: 3699

cy.contains("#details").click();
}
Comment on lines +2 to +7
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

Multiple improvements needed for robustness and type safety

  1. Add TypeScript type safety
  2. Make API intercept pattern specific
  3. Add proper error handling
  4. Fix selector usage

Apply these improvements:

-    openDetails(patientName) {
+    openDetails(patientName: string): void {
         cy.get('#patient_name').type(patientName);
-        cy.intercept("GET", "**/api/v1/shift/**").as("getShiftingRequest");
+        cy.intercept("GET", "/api/v1/shift/").as("getShiftingRequest");
         cy.wait("@getShiftingRequest").its("response.statusCode").should("eq", 200);
-        cy.contains("#details").click();
+        cy.get("#details").click();
     }

Additionally, consider adding timeout and error handling:

openDetails(patientName: string): void {
    cy.get('#patient_name').type(patientName);
    cy.intercept("GET", "/api/v1/shift/").as("getShiftingRequest");
    cy.wait("@getShiftingRequest", { timeout: 10000 })
        .then((interception) => {
            if (interception.response?.statusCode !== 200) {
                throw new Error(`Failed to load shifting request: ${interception.response?.statusCode}`);
            }
        });
    cy.get("#details", { timeout: 5000 }).click();
}
🧰 Tools
🪛 eslint

[error] 2-2: Delete ··

(prettier/prettier)


[error] 3-3: Replace ····cy.get('#patient_name' with cy.get("#patient_name"

(prettier/prettier)


[error] 4-4: Replace ········ with ····

(prettier/prettier)


[error] 5-5: Delete ····

(prettier/prettier)


[error] 6-6: Replace ········ with ····

(prettier/prettier)


[error] 7-7: Delete ··

(prettier/prettier)

}
Loading
Loading