-
Notifications
You must be signed in to change notification settings - Fork 626
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 encounter-notes cypress test #10538
added encounter-notes cypress test #10538
Conversation
WalkthroughThe changes introduce a new Cypress test suite for encounter notes and enhance the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (5)
cypress/e2e/patient_spec/encouter-notes.cy.ts (2)
26-26
: Fix typo in test message.The test message contains a typo: "fianl" should be "final".
- cy.get("textarea").type("This is a test message fianl!"); + cy.get("textarea").type("This is a test message final!");
14-77
: Reduce code duplication by extracting helper functions.The code for creating threads and adding messages is duplicated across test cases. Consider extracting this into a helper function for better maintainability.
function createThreadWithMessage(title: string, message: string) { cy.contains("button", "New Thread").click(); cy.get("input[placeholder='Enter discussion title...']").type(title); cy.contains("button", "Create").click(); cy.get("form").within(() => { cy.get("textarea").type(message); cy.get('button[type="submit"]').click(); }); } // Usage in tests: it("create multiple threads and add messages", () => { patientEncounter .navigateToEncounters() .openFirstEncounterDetails() .openNotesTab(); createThreadWithMessage("Test Discussion final", "This is a test message final!"); createThreadWithMessage("Test Discussion 100", "This is a test message 100!"); });🧰 Tools
🪛 ESLint
[error] 18-19: Replace
⏎······
with;
(prettier/prettier)
[error] 23-23: Replace
"Test·Discussion·final"
with⏎······"Test·Discussion·final",⏎····
(prettier/prettier)
[error] 30-32: Delete
⏎⏎····
(prettier/prettier)
[error] 34-34: Replace
"Test·Discussion·100"
with⏎······"Test·Discussion·100",⏎····
(prettier/prettier)
[error] 41-42: Replace
··cy.intercept("POST",·"https://careapi.ohc.network/api/v1/auth/logout/").as("logoutRequest");⏎
withcy.intercept("POST",·"https://careapi.ohc.network/api/v1/auth/logout/").as(⏎······"logoutRequest",
(prettier/prettier)
[error] 43-43: Insert
);
(prettier/prettier)
[error] 49-49: Replace
.navigateToEncounters().openFirstEncounterDetails()
with⏎······.navigateToEncounters()⏎······.openFirstEncounterDetails()⏎······
(prettier/prettier)
[error] 56-56: Insert
;
(prettier/prettier)
[error] 60-60: Replace
"Test·Discussion·200"
with⏎······"Test·Discussion·200",⏎····
(prettier/prettier)
[error] 65-67: Delete
⏎⏎····
(prettier/prettier)
[error] 71-71: Replace
"Test·Discussion·300"
with⏎······"Test·Discussion·300",⏎····
(prettier/prettier)
cypress/pageObject/Patients/PatientEncounter.ts (3)
35-38
: Fix formatting issues.The method has formatting issues that should be addressed.
-openNotesTab() { - cy.get('#encounter_tab_nav').contains("Notes").click(); + openNotesTab() { + cy.get("#encounter_tab_nav").contains("Notes").click();🧰 Tools
🪛 ESLint
[error] 35-35: Insert
·
(prettier/prettier)
[error] 36-36: Replace
'#encounter_tab_nav'
with"#encounter_tab_nav"
(prettier/prettier)
40-44
: Remove redundant comment.The comment "Click the submit button" is redundant as the code is self-documenting.
addMessageToThread(message: string) { cy.get('textarea[placeholder="Type your message"]').type(message); - // Click the submit button cy.get('button[type="submit"]').click(); }
46-51
: LGTM! Remove extra blank line.The method is well-implemented but has an extra blank line that should be removed.
startNewConversation() { cy.contains("button", "Start New Discussion").should("be.visible").click(); return this; } -
🧰 Tools
🪛 ESLint
[error] 49-50: Delete
⏎
(prettier/prettier)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
cypress/e2e/patient_spec/encouter-notes.cy.ts
(1 hunks)cypress/pageObject/Patients/PatientEncounter.ts
(1 hunks)
🧰 Additional context used
🪛 ESLint
cypress/pageObject/Patients/PatientEncounter.ts
[error] 35-35: Insert ·
(prettier/prettier)
[error] 36-36: Replace '#encounter_tab_nav'
with "#encounter_tab_nav"
(prettier/prettier)
[error] 49-50: Delete ⏎
(prettier/prettier)
cypress/e2e/patient_spec/encouter-notes.cy.ts
[error] 1-1: Replace FacilityCreation·}·from·"@/pageObject/facility/FacilityCreation
with PatientEncounter·}·from·"@/pageObject/Patients/PatientEncounter
(prettier/prettier)
[error] 2-2: Replace PatientEncounter·}·from·"@/pageObject/Patients/PatientEncounter
with FacilityCreation·}·from·"@/pageObject/facility/FacilityCreation
(prettier/prettier)
[error] 18-19: Replace ⏎······
with ;
(prettier/prettier)
[error] 23-23: Replace "Test·Discussion·final"
with ⏎······"Test·Discussion·final",⏎····
(prettier/prettier)
[error] 30-32: Delete ⏎⏎····
(prettier/prettier)
[error] 34-34: Replace "Test·Discussion·100"
with ⏎······"Test·Discussion·100",⏎····
(prettier/prettier)
[error] 41-42: Replace ··cy.intercept("POST",·"https://careapi.ohc.network/api/v1/auth/logout/").as("logoutRequest");⏎
with cy.intercept("POST",·"https://careapi.ohc.network/api/v1/auth/logout/").as(⏎······"logoutRequest",
(prettier/prettier)
[error] 43-43: Insert );
(prettier/prettier)
[error] 49-49: Replace .navigateToEncounters().openFirstEncounterDetails()
with ⏎······.navigateToEncounters()⏎······.openFirstEncounterDetails()⏎······
(prettier/prettier)
[error] 56-56: Insert ;
(prettier/prettier)
[error] 60-60: Replace "Test·Discussion·200"
with ⏎······"Test·Discussion·200",⏎····
(prettier/prettier)
[error] 65-67: Delete ⏎⏎····
(prettier/prettier)
[error] 71-71: Replace "Test·Discussion·300"
with ⏎······"Test·Discussion·300",⏎····
(prettier/prettier)
🔇 Additional comments (1)
cypress/pageObject/Patients/PatientEncounter.ts (1)
30-33
: LGTM!The method follows the page object pattern and correctly implements method chaining.
cy.get('button[type="submit"]').click(); | ||
}); | ||
|
||
cy.intercept("POST", "https://careapi.ohc.network/api/v1/auth/logout/").as("logoutRequest"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Separate test cases for different user roles.
The test case mixes doctor and nurse actions, which violates the single responsibility principle for tests. Consider splitting this into two separate test cases for better maintainability and clarity.
- cy.intercept("POST", "https://careapi.ohc.network/api/v1/auth/logout/").as("logoutRequest");
-
-
-
- // logging as nurse
- cy.loginByApi("devnurse");
- cy.visit("/");
- facilityCreation.selectFacility("GHC Trikaripur");
- patientEncounter.navigateToEncounters().openFirstEncounterDetails().openNotesTab();
+ });
+
+ it("create multiple threads and add messages as nurse", () => {
+ cy.loginByApi("devnurse");
+ cy.visit("/");
+ facilityCreation.selectFacility("GHC Trikaripur");
+ patientEncounter
+ .navigateToEncounters()
+ .openFirstEncounterDetails()
+ .openNotesTab();
Also applies to: 45-49
🧰 Tools
🪛 ESLint
[error] 41-42: Replace ··cy.intercept("POST",·"https://careapi.ohc.network/api/v1/auth/logout/").as("logoutRequest");⏎
with cy.intercept("POST",·"https://careapi.ohc.network/api/v1/auth/logout/").as(⏎······"logoutRequest",
(prettier/prettier)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
cypress/pageObject/Patients/PatientEncounter.ts (1)
46-50
: Use data-cy attribute for more reliable selector.The method uses text content for selection which could break if the button text changes. Consider using a data-cy attribute instead.
Apply this diff to improve the method and fix formatting:
startNewConversation() { - cy.contains("button", "Start New Discussion").should("be.visible").click(); + cy.get('[data-cy="start-discussion"]') + .should("be.visible") + .click(); return this; } -🧰 Tools
🪛 ESLint
[error] 49-50: Delete
⏎
(prettier/prettier)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cypress/pageObject/Patients/PatientEncounter.ts
(1 hunks)
🧰 Additional context used
🪛 ESLint
cypress/pageObject/Patients/PatientEncounter.ts
[error] 35-35: Insert ·
(prettier/prettier)
[error] 36-36: Replace '#encounter_tab_nav'
with "#encounter_tab_nav"
(prettier/prettier)
[error] 49-50: Delete ⏎
(prettier/prettier)
🔇 Additional comments (1)
cypress/pageObject/Patients/PatientEncounter.ts (1)
30-33
: LGTM!The method follows the page object pattern and maintains consistency with existing selectors.
addMessageToThread(message: string) { | ||
cy.get('textarea[placeholder="Type your message"]').type(message); | ||
// Click the submit button | ||
cy.get('button[type="submit"]').click(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Improve test maintainability and method chaining.
The method has several areas for improvement:
- Use data-cy attributes for more reliable selectors
- Return this to support method chaining
Apply this diff to improve the method:
addMessageToThread(message: string) {
- cy.get('textarea[placeholder="Type your message"]').type(message);
- // Click the submit button
- cy.get('button[type="submit"]').click();
+ cy.get('[data-cy="message-input"]').type(message);
+ cy.get('[data-cy="submit-message"]').click();
+ 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.
addMessageToThread(message: string) { | |
cy.get('textarea[placeholder="Type your message"]').type(message); | |
// Click the submit button | |
cy.get('button[type="submit"]').click(); | |
} | |
addMessageToThread(message: string) { | |
cy.get('[data-cy="message-input"]').type(message); | |
cy.get('[data-cy="submit-message"]').click(); | |
return this; | |
} |
openNotesTab() { | ||
cy.get('#encounter_tab_nav').contains("Notes").click(); | ||
return this; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Fix formatting issues.
The method logic is correct, but there are some formatting issues to address.
Apply this diff to fix the formatting:
- openNotesTab() {
- cy.get('#encounter_tab_nav').contains("Notes").click();
+ openNotesTab() {
+ cy.get("#encounter_tab_nav").contains("Notes").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.
openNotesTab() { | |
cy.get('#encounter_tab_nav').contains("Notes").click(); | |
return this; | |
} | |
openNotesTab() { | |
cy.get("#encounter_tab_nav").contains("Notes").click(); | |
return this; | |
} |
🧰 Tools
🪛 ESLint
[error] 35-35: Insert ·
(prettier/prettier)
[error] 36-36: Replace '#encounter_tab_nav'
with "#encounter_tab_nav"
(prettier/prettier)
@nihal467 is on vacation this week, he will review and test once he's back 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Make sure you follow the guidelines already mentioned in the issue as well as the POM structure
- the entire test need to be rewritten following the guideline
- Before writing a test, think of the workflow you are about to write test for example: A user creates a thread, then posts a note. The user logs out and logs in as a different user, then navigates to the same patient accessed by the first user. The new user verifies that they can see the message posted by the first user and adds a comment to the same thread.
beforeEach(() => { | ||
cy.loginByApi("devdoctor"); // logging in as doctor | ||
cy.visit("/"); | ||
facilityCreation.selectFacility("GHC Trikaripur"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
facilityCreation.selectFacility("GHC Trikaripur"); | |
facilityCreation.selectFacility("GHC Payyanur"); |
use the facility GHC Payyanur
|
||
describe("Encounter Notes - Threads, Messages, and Notes", () => { | ||
beforeEach(() => { | ||
cy.loginByApi("devdoctor"); // logging in as doctor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cy.loginByApi("devdoctor"); // logging in as doctor | |
cy.loginByApi("devdoctor"); // logging in as doctor |
- since we are connected to develop backend temporarily, create a new user manually in the develop, add it to the fixture file and use that in your test
|
||
|
||
// first Thread | ||
cy.contains("button", "New Thread").click(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cy.contains("button", "New Thread").click(); | |
cy.contains("button", "New Thread").click(); |
as the per given guidelines, use the command function, cy.verifyandclickelement() to click on any button with text. Make sure you uses a proper data-cy test id's or element id ,
cy.get("input[placeholder='Enter discussion title...']").type("Test Discussion final"); | ||
cy.contains("button", "Create").click(); | ||
cy.get("form").within(() => { | ||
cy.get("textarea").type("This is a test message fianl!"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cy.get("textarea").type("This is a test message fianl!"); | |
cy.get("textarea").type("This is a test message fianl!"); |
use combination of single line message as well as multi line text as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
cypress/pageObject/Patients/PatientEncounter.ts (1)
45-48
: Use data-cy attribute for more reliable selector.The text-based selector could be fragile if the button text changes. Consider using a data-cy attribute for better test maintainability.
Apply this diff to improve the selector:
- cy.contains("button", "Start New Discussion").should("be.visible").click(); + cy.get('[data-cy="start-discussion-button"]').should("be.visible").click();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cypress/pageObject/Patients/PatientEncounter.ts
(1 hunks)
🧰 Additional context used
🪛 ESLint
cypress/pageObject/Patients/PatientEncounter.ts
[error] 34-34: Insert ·
(prettier/prettier)
[error] 35-35: Replace '#encounter_tab_nav'
with "#encounter_tab_nav"
(prettier/prettier)
[error] 48-49: Delete ⏎
(prettier/prettier)
🔇 Additional comments (3)
cypress/pageObject/Patients/PatientEncounter.ts (3)
29-32
: LGTM!The method follows the established patterns in the class, using consistent selectors and proper method chaining.
34-37
: Fix formatting issues.The method logic is correct, but there are some formatting issues to address.
🧰 Tools
🪛 ESLint
[error] 34-34: Insert
·
(prettier/prettier)
[error] 35-35: Replace
'#encounter_tab_nav'
with"#encounter_tab_nav"
(prettier/prettier)
39-43
: Improve test maintainability and method chaining.The method has several areas for improvement:
- Use data-cy attributes for more reliable selectors
- Return this to support method chaining
@sanjayh-2022 what is the status of this PR |
I am working on it I will raise a pr soon |
@sanjayh-2022 Closing this PR due to inactivity and non-compliance with the guidelines. Feel free to reopen it once the code is properly refactored and ready for review. |
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit