Cypress: Patient Prescription Management #10740
Conversation
WalkthroughThis update introduces a new Cypress test suite for patient prescription management along with enhancements to supporting page objects and React components. The test file automates the addition and deletion of a medication using the new methods added to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🪧 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. |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
cypress/e2e/patient_spec/patient_prescription.cy.ts (1)
13-38: Consider enhancing test coverage and data management.While the test successfully verifies adding a medication, consider:
- Moving test data to fixtures for better maintainability
- Adding validation for the added medication
- Testing error scenarios (e.g., invalid inputs)
+import testData from '../../fixtures/medication.json'; + it("should add a new medicine for the patient", () => { facilityCreation.selectFacility("GHC payyanur"); - const medicineName = "Senna 15 mg oral tablet"; - const dosage = 6; - const frequency = "BID (1-0-1)"; - const instructions = "Until symptoms improve"; - const route = "Sublabial route"; - const site = "Structure of left deltoid muscle"; - const method = "Bathe"; - const notes = "testing notes"; + const { medicineName, dosage, frequency, instructions, route, site, method, notes } = testData.validMedication; patientEncounter .navigateToEncounters() .openFirstEncounterDetails() .clickMedicinesTab() .clickEditPrescription() .addMedication( medicineName, dosage, frequency, instructions, route, site, method, notes, - ); + ) + .verifyMedicationAdded(medicineName);cypress/pageObject/Patients/PatientEncounter.ts (1)
23-58: Consider breaking down the addMedication method for better maintainability.The method is quite long and handles multiple responsibilities. Consider splitting it into smaller, more focused methods.
+ private fillMedicationDetails(medicineName: string) { + cy.get('[data-cy="question-medication-request"]').click(); + cy.get('[role="listbox"]') + .find('[role="option"]') + .contains(medicineName) + .click(); + return this; + } + + private fillDosageDetails(dosage: number, frequency: string) { + cy.get('input[inputmode="numeric"]').should("exist").type(dosage); + cy.get('[data-cy="frequency"]').click(); + cy.get('[role="option"]').contains(frequency).click(); + return this; + } + addMedication( medicineName, dosage, frequency, instructions, route, site, method, notes, ) { - cy.get('[data-cy="question-medication-request"]').click(); - cy.get('[role="listbox"]') - .find('[role="option"]') - .contains(medicineName) - .click(); - cy.get('input[inputmode="numeric"]').should("exist").type(dosage); - cy.get('[data-cy="frequency"]').click(); - cy.get('[role="option"]').contains(frequency).click(); + this.fillMedicationDetails(medicineName) + .fillDosageDetails(dosage, frequency); // ... rest of the methodsrc/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx (2)
84-376: Consider breaking down the MedicationRequestQuestion component.The component is quite large and handles multiple responsibilities. Consider splitting it into smaller, more focused components for better maintainability.
+const MedicationList = ({ medications, onUpdate, onRemove }) => { + // Extract medication list rendering logic +}; + +const MedicationForm = ({ onAdd }) => { + // Extract medication form logic +}; + export function MedicationRequestQuestion({ questionnaireResponse, updateQuestionnaireResponseCB, disabled, patientId, encounterId, }: MedicationRequestQuestionProps) { // ... state and handlers return ( <div className="space-y-4"> - {/* Current implementation */} + <MedicationList + medications={medications} + onUpdate={handleUpdateMedication} + onRemove={handleRemoveMedication} + /> + <MedicationForm onAdd={handleAddMedication} /> </div> ); }
624-624: Consider using a more descriptive data-cy value.The current data-cy value "frequency" could be more specific to better indicate its purpose.
- <SelectTrigger className="h-9 text-sm" data-cy="frequency"> + <SelectTrigger className="h-9 text-sm" data-cy="medication-frequency-select">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
cypress/e2e/patient_spec/patient_prescription.cy.ts(1 hunks)cypress/pageObject/Patients/PatientEncounter.ts(1 hunks)src/components/Medicine/MedicationRequestTable/index.tsx(1 hunks)src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx(4 hunks)src/components/Questionnaire/QuestionTypes/NotesInput.tsx(2 hunks)src/pages/Encounters/EncounterShow.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/components/Medicine/MedicationRequestTable/index.tsx
- src/components/Questionnaire/QuestionTypes/NotesInput.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: cypress-run (1)
🔇 Additional comments (4)
cypress/e2e/patient_spec/patient_prescription.cy.ts (1)
7-11: LGTM! Good test setup.The beforeEach hook properly sets up the test environment by handling login and navigation.
cypress/pageObject/Patients/PatientEncounter.ts (1)
15-22: LGTM! Good use of data-cy attributes.The methods use data-cy attributes for reliable element selection and follow the builder pattern.
src/pages/Encounters/EncounterShow.tsx (1)
173-173: LGTM! Good test attribute addition.The data-cy attribute addition improves test reliability and follows the established pattern.
src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx (1)
205-205: LGTM! Consistent test attribute implementation.The data-cy attributes are consistently applied across the component for reliable test selection.
Also applies to: 321-321, 884-884
| describe("Patient Prescription Management", () => { | ||
| beforeEach(() => { | ||
| cy.loginByApi("devnurse"); | ||
| cy.visit("/"); | ||
| }); | ||
|
|
||
| it("should add a new medicine for the patient", () => { | ||
| facilityCreation.selectFacility("GHC payyanur"); | ||
| const medicineName = "Senna 15 mg oral tablet"; | ||
| const dosage = 6; | ||
| const frequency = "BID (1-0-1)"; | ||
| const instructions = "Until symptoms improve"; | ||
| const route = "Sublabial route"; | ||
| const site = "Structure of left deltoid muscle"; | ||
| const method = "Bathe"; | ||
| const notes = "testing notes"; | ||
| patientEncounter | ||
| .navigateToEncounters() | ||
| .openFirstEncounterDetails() | ||
| .clickMedicinesTab() | ||
| .clickEditPrescription() | ||
| .addMedication( | ||
| medicineName, | ||
| dosage, | ||
| frequency, | ||
| instructions, | ||
| route, | ||
| site, | ||
| method, | ||
| notes, | ||
| ); | ||
| }); | ||
| it("should delete prescription", () => { | ||
| facilityCreation.selectFacility("GHC payyanur"); | ||
|
|
||
| patientEncounter | ||
| .navigateToEncounters() | ||
| .openFirstEncounterDetails() | ||
| .clickMedicinesTab() | ||
| .clickEditPrescription() | ||
| .removeMedication(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add more test scenarios for comprehensive coverage.
The test suite should include additional scenarios:
- Editing an existing medication
- Adding multiple medications
- Error handling for invalid inputs
- Validation of required fields
it("should edit an existing medication", () => {
// Add test for editing medication
});
it("should validate required fields", () => {
// Add test for field validation
});
it("should handle invalid inputs gracefully", () => {
// Add test for error scenarios
});| it("should delete prescription", () => { | ||
| facilityCreation.selectFacility("GHC payyanur"); | ||
|
|
||
| patientEncounter | ||
| .navigateToEncounters() | ||
| .openFirstEncounterDetails() | ||
| .clickMedicinesTab() | ||
| .clickEditPrescription() | ||
| .removeMedication(); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for medication removal.
The deletion test should verify that the medication was actually removed.
it("should delete prescription", () => {
facilityCreation.selectFacility("GHC payyanur");
+ const medicineName = "Senna 15 mg oral tablet";
+
+ // First add a medication to ensure we have something to delete
+ patientEncounter
+ .navigateToEncounters()
+ .openFirstEncounterDetails()
+ .clickMedicinesTab()
+ .clickEditPrescription()
+ .addMedication(medicineName, 6, "BID (1-0-1)", "", "", "", "", "");
patientEncounter
.navigateToEncounters()
.openFirstEncounterDetails()
.clickMedicinesTab()
.clickEditPrescription()
- .removeMedication();
+ .removeMedication()
+ .verifyMedicationRemoved(medicineName);
});📝 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.
| it("should delete prescription", () => { | |
| facilityCreation.selectFacility("GHC payyanur"); | |
| patientEncounter | |
| .navigateToEncounters() | |
| .openFirstEncounterDetails() | |
| .clickMedicinesTab() | |
| .clickEditPrescription() | |
| .removeMedication(); | |
| }); | |
| it("should delete prescription", () => { | |
| facilityCreation.selectFacility("GHC payyanur"); | |
| const medicineName = "Senna 15 mg oral tablet"; | |
| // First add a medication to ensure we have something to delete | |
| patientEncounter | |
| .navigateToEncounters() | |
| .openFirstEncounterDetails() | |
| .clickMedicinesTab() | |
| .clickEditPrescription() | |
| .addMedication(medicineName, 6, "BID (1-0-1)", "", "", "", "", ""); | |
| patientEncounter | |
| .navigateToEncounters() | |
| .openFirstEncounterDetails() | |
| .clickMedicinesTab() | |
| .clickEditPrescription() | |
| .removeMedication() | |
| .verifyMedicationRemoved(medicineName); | |
| }); |
| removeMedication() { | ||
| cy.get('[data-cy="medication-remove"]').first().click(); | ||
| cy.verifyAndClickElement('[data-cy="confirm-remove-medication"]', "Remove"); | ||
| this.clickSubmitQuestionnaire(); | ||
| this.verifyQuestionnaireSubmission(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add verification after medication removal.
The removeMedication method should include verification that the medication was actually removed.
removeMedication() {
cy.get('[data-cy="medication-remove"]').first().click();
cy.verifyAndClickElement('[data-cy="confirm-remove-medication"]', "Remove");
this.clickSubmitQuestionnaire();
this.verifyQuestionnaireSubmission();
+ // Add verification that medication was removed
+ cy.get('[data-cy="medication-list"]').should('not.contain', this.lastRemovedMedication);
+ return this;
}📝 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.
| removeMedication() { | |
| cy.get('[data-cy="medication-remove"]').first().click(); | |
| cy.verifyAndClickElement('[data-cy="confirm-remove-medication"]', "Remove"); | |
| this.clickSubmitQuestionnaire(); | |
| this.verifyQuestionnaireSubmission(); | |
| } | |
| removeMedication() { | |
| cy.get('[data-cy="medication-remove"]').first().click(); | |
| cy.verifyAndClickElement('[data-cy="confirm-remove-medication"]', "Remove"); | |
| this.clickSubmitQuestionnaire(); | |
| this.verifyQuestionnaireSubmission(); | |
| // Add verification that medication was removed | |
| cy.get('[data-cy="medication-list"]').should('not.contain', this.lastRemovedMedication); | |
| return this; | |
| } |
|
@DonXavierdev is it ready for review ? |
@nihal467 Yes i want to know the changes required in this to further implement this |
|
|
||
| describe("Patient Prescription Management", () => { | ||
| beforeEach(() => { | ||
| cy.loginByApi("devnurse"); |
There was a problem hiding this comment.
| cy.loginByApi("devnurse"); | |
| cy.loginByApi("devnurse"); |
- create a new user and update it in fixtures then use that id here
| route, | ||
| site, | ||
| method, | ||
| notes, |
There was a problem hiding this comment.
| notes, | |
| notes, |
- verify that the added medicine is reflected properly in the medicine module,
- Verify the success notification and API request
| notes, | ||
| ); | ||
| }); | ||
| it("should delete prescription", () => { |
There was a problem hiding this comment.
- don't create a separate test, combine this with the above test itself
| .openFirstEncounterDetails() | ||
| .clickMedicinesTab() | ||
| .clickEditPrescription() | ||
| .removeMedication(); |
There was a problem hiding this comment.
| .removeMedication(); | |
| .removeMedication(); |
once the medicine is removed, verify that the changes in the relevant module is verified
| notes, | ||
| ) { | ||
| cy.get('[data-cy="question-medication-request"]').click(); | ||
| cy.get('[role="listbox"]') |
There was a problem hiding this comment.
| cy.get('[role="listbox"]') | |
| cy.get('[role="listbox"]') |
check already existing command function to interact with dropdown, replace every usage with command function
| .find('[role="option"]') | ||
| .contains(medicineName) | ||
| .click(); | ||
| cy.get('input[inputmode="numeric"]').should("exist").type(dosage); |
There was a problem hiding this comment.
- use proper id
- use existing command function for this
…o issues/10387/Cypress-Medicine-Test
|
Conflicts have been detected against the base branch. Please merge the base branch into your branch.
|
…o issues/10387/Cypress-Medicine-Test
@nihal467 I dont understand this |
|
@DonXavierdev SORRY FOR THE TYPO MISTAKE. verify that the added medicine is properly visible in the medicine dashboard. for reference attached the screenshot |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
cypress/pageObject/Patients/PatientEncounter.ts (2)
23-47: Consider simplifying or verifying typed input
There is a slight redundancy in typing the dosage and then callingclickAndSelectOptionon the same selector ([data-cy="dosage"]). If this is intentional (e.g., typing triggers dropdown suggestions), consider clarifying through comments. Otherwise, consolidating these steps could reduce confusion.
75-96: Clarify function naming
The method is namedverifyDeletedMedication, but we validate the presence of the medication in the "stopped medications" table. Renaming this function to something likeverifyStoppedMedicationmight reduce confusion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
cypress/e2e/patient_spec/patient_prescription.cy.ts(1 hunks)cypress/fixtures/users.json(1 hunks)cypress/pageObject/Patients/PatientEncounter.ts(1 hunks)src/components/Medicine/MedicationRequestTable/index.tsx(2 hunks)src/components/Medicine/MedicationsTable.tsx(1 hunks)src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx(9 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/components/Medicine/MedicationsTable.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- cypress/e2e/patient_spec/patient_prescription.cy.ts
- src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx
- src/components/Medicine/MedicationRequestTable/index.tsx
🔇 Additional comments (4)
cypress/fixtures/users.json (1)
29-33: New Test User Addition: "testnurse4"The new user entry is formatted consistently with the existing fixture data. The addition of the
"testnurse4"user, with a username matching its key and a designated test password, appears appropriate for supporting the new prescription management tests. Please ensure that all test suites that require nurse authentication are updated to include or reference this user as needed. Additionally, verify that these test credentials remain strictly within your testing environment and are never used in production deployments.cypress/pageObject/Patients/PatientEncounter.ts (3)
15-18: Method is succinct and chainable
TheclickMedicinesTabmethod follows a clear naming convention and returnsthisfor chainability, which is consistent with the rest of the class.
19-22: Implementation is consistent
TheclickEditPrescriptionmethod is aligned with the pattern used elsewhere in this file. It correctly verifies and clicks the edit button, then returnsthisfor continued chaining.
70-74: Add verification after medication removal
This repeats a prior suggestion: confirm that the removed medication no longer appears in the active list or has moved to the "stopped" list. This helps detect potential failures if the UI or request fails.
| verifyMedication( | ||
| medicineName, | ||
| dosage, | ||
| frequency, | ||
| instructions, | ||
| route, | ||
| site, | ||
| method, | ||
| notes, | ||
| ) { | ||
| cy.get('[data-cy="medications-table"]').within(() => { | ||
| cy.contains("td", medicineName).should("exist"); | ||
| cy.contains("td", dosage).should("exist"); | ||
| cy.contains("td", frequency).should("exist"); | ||
| cy.contains("td", instructions).should("exist"); | ||
| cy.contains("td", route).should("exist"); | ||
| cy.contains("td", site).should("exist"); | ||
| cy.contains("td", method).should("exist"); | ||
| cy.contains("td", notes).should("exist"); | ||
| }); | ||
| return this; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid potential false positives by verifying each medication field on the same row
Currently, verifying each field individually can pass if multiple rows share the same values. To ensure correctness, locate the row containing the medicine name and verify the other details on that row, for instance:
- cy.contains("td", medicineName).should("exist");
- cy.contains("td", dosage).should("exist");
...
+ cy.get('[data-cy="medications-table"]')
+ .contains('tr', medicineName)
+ .should('contain', dosage)
+ .should('contain', frequency)
+ .should('contain', instructions);📝 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.
| verifyMedication( | |
| medicineName, | |
| dosage, | |
| frequency, | |
| instructions, | |
| route, | |
| site, | |
| method, | |
| notes, | |
| ) { | |
| cy.get('[data-cy="medications-table"]').within(() => { | |
| cy.contains("td", medicineName).should("exist"); | |
| cy.contains("td", dosage).should("exist"); | |
| cy.contains("td", frequency).should("exist"); | |
| cy.contains("td", instructions).should("exist"); | |
| cy.contains("td", route).should("exist"); | |
| cy.contains("td", site).should("exist"); | |
| cy.contains("td", method).should("exist"); | |
| cy.contains("td", notes).should("exist"); | |
| }); | |
| return this; | |
| } | |
| verifyMedication( | |
| medicineName, | |
| dosage, | |
| frequency, | |
| instructions, | |
| route, | |
| site, | |
| method, | |
| notes, | |
| ) { | |
| - cy.get('[data-cy="medications-table"]').within(() => { | |
| - cy.contains("td", medicineName).should("exist"); | |
| - cy.contains("td", dosage).should("exist"); | |
| - cy.contains("td", frequency).should("exist"); | |
| - cy.contains("td", instructions).should("exist"); | |
| - cy.contains("td", route).should("exist"); | |
| - cy.contains("td", site).should("exist"); | |
| - cy.contains("td", method).should("exist"); | |
| - cy.contains("td", notes).should("exist"); | |
| - }); | |
| + cy.get('[data-cy="medications-table"]') | |
| + .contains('tr', medicineName) | |
| + .should('contain', dosage) | |
| + .should('contain', frequency) | |
| + .should('contain', instructions) | |
| + .should('contain', route) | |
| + .should('contain', site) | |
| + .should('contain', method) | |
| + .should('contain', notes); | |
| return this; | |
| } |
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
Improvements