(test) O3-2107: Add patient chart allergies E2E test #1205

RandilaP · 2023-06-09T16:00:18Z

Requirements

My work conforms to the OpenMRS 3.0 Styleguide and design documentation.
This PR has a title that briefly describes the work done including the ticket number. If there is a ticket, make sure your PR title includes a conventional commit label. See existing PR titles for inspiration.
My work includes tests or is validated by existing tests.

Summary

This PR aims to write an E2E test for record allergies in the patient chart repository.

Screenshots

Related Issue

https://issues.openmrs.org/browse/O3-2107

Other

RandilaP · 2023-06-09T16:02:10Z

@jayasanka-sack @anjula-sack @Piumal1999 Could you please review this PR?

packages/esm-patient-allergies-app/src/allergies/allergies-detailed-summary.component.tsx

packages/esm-patient-allergies-app/src/allergies/allergies-overview.component.tsx

e2e/specs/record-allergies.spec.ts

anjula-sack

Shall we add the comments as well?

anjula-sack · 2023-06-09T16:36:08Z

Let's extend the test to test the food and environment allergies as well @RandilaP

anjula-sack · 2023-06-09T16:38:43Z

Good job! @RandilaP Let's test the notification as well.

RandilaP · 2023-06-10T17:57:09Z

@jayasanka-sack @anjula-sack @Piumal1999 Could you please review the changes

jayasanka-sack

Thanks @RandilaP for the PR. I've taken the time to review it and have left some comments. Could you please take a moment to review them? Let me know if you require any further clarifications.

e2e/specs/record-allergies.spec.ts

e2e/pages/PatientAllergiesPage.ts

e2e/specs/record-allergies.spec.ts

.github/workflows/e2e.yml

anjula-sack · 2023-06-19T14:06:06Z

@RandilaP Can you fix the conflicts?

anjula-sack

can you check your screenshot as well @RandilaP,

anjula-sack · 2023-06-23T06:02:01Z

e2e/pages/allergies-page.ts

@@ -3,6 +3,7 @@ import { Page } from '@playwright/test';
 export class AllergiesPage {
  constructor(readonly page: Page) {}

+  readonly tableRow = () => this.page.locator('tr');


Let's add a space here.

Suggested change

readonly tableRow = () => this.page.locator('tr');

readonly tableRow = () => this.page.locator('tr');

There can be other tables on the page. To be specific, we can use the extension id attribute.
data-extension-id="allergies-details-widget".

Also, instead of locating the row, it would be better to locate only the table here. Check my other comment on the spec. Using a variable name something like allergyTable would also work.

I wonder why this doesn't fail because the heading row of the table itself a tr

RandilaP · 2023-06-23T06:31:04Z

@anjula-sack @Piumal1999 @jayasanka-sack Could you please review this

jayasanka-sack · 2023-06-25T07:43:19Z

@anjula-sack @Piumal1999 @jayasanka-sack Could you please review this

Hey, @RandilaP! Pro tip: It might be a cool idea to resolve any ongoing conversations before throwing in another review request. 😁 Happy resolving and reviewing!

jayasanka-sack

Keep up the great work! Cheers! 🎉

jayasanka-sack · 2023-06-25T07:56:55Z

e2e/pages/allergies-page.ts

@@ -3,6 +3,7 @@ import { Page } from '@playwright/test';
 export class AllergiesPage {
  constructor(readonly page: Page) {}

+  readonly tableRow = () => this.page.locator('tr');


There can be other tables on the page. To be specific, we can use the extension id attribute.
data-extension-id="allergies-details-widget".

Also, instead of locating the row, it would be better to locate only the table here. Check my other comment on the spec. Using a variable name something like allergyTable would also work.

jayasanka-sack · 2023-06-25T07:59:02Z

e2e/pages/allergies-page.ts

@@ -3,6 +3,7 @@ import { Page } from '@playwright/test';
 export class AllergiesPage {
  constructor(readonly page: Page) {}

+  readonly tableRow = () => this.page.locator('tr');


I wonder why this doesn't fail because the heading row of the table itself a tr

e2e/specs/record-food-allergy.spec.ts

packages/esm-patient-chart-app/translations/fr.json

e2e/specs/record-drug-allergies.spec.ts

e2e/specs/record-environmental-allergy.spec.ts

e2e/specs/record-drug-allergies.spec.ts

e2e/specs/record-food-allergy.spec.ts

anjula-sack · 2023-08-08T03:12:08Z

@RandilaP can you fix the conflicts here?

.github/workflows/e2e.yml

e2e/pages/allergies-page.ts

anjula-sack

Good job! @RandilaP

Requested change is done

RandilaP marked this pull request as draft June 9, 2023 16:00

anjula-sack requested changes Jun 9, 2023

View reviewed changes

e2e/specs/record-allergies.spec.ts Outdated Show resolved Hide resolved

anjula-sack requested changes Jun 9, 2023

View reviewed changes

RandilaP marked this pull request as ready for review June 11, 2023 03:15

jayasanka-sack requested changes Jun 11, 2023

View reviewed changes

jayasanka-sack changed the title ~~(test) O3:2107 - Add patient chart allergies E2E test~~ (test) O3-2107: Add patient chart allergies E2E test Jun 11, 2023

anjula-sack requested changes Jun 13, 2023

View reviewed changes

.github/workflows/e2e.yml Outdated Show resolved Hide resolved

.github/workflows/e2e.yml Outdated Show resolved Hide resolved

RandilaP force-pushed the O3-2107 branch from a12cdc6 to c26028f Compare June 23, 2023 05:33

anjula-sack reviewed Jun 23, 2023

View reviewed changes

jayasanka-sack previously requested changes Jun 25, 2023

View reviewed changes

RandilaP force-pushed the O3-2107 branch from c26028f to 5c4e5c7 Compare July 15, 2023 08:57

anjula-sack reviewed Jul 15, 2023

View reviewed changes

packages/esm-patient-chart-app/translations/fr.json Outdated Show resolved Hide resolved

anjula-sack reviewed Jul 17, 2023

View reviewed changes

e2e/specs/record-drug-allergies.spec.ts Outdated Show resolved Hide resolved

RandilaP force-pushed the O3-2107 branch from 5c4e5c7 to 53a6e29 Compare July 24, 2023 15:03

Piumal1999 reviewed Jul 24, 2023

View reviewed changes

e2e/specs/record-drug-allergies.spec.ts Outdated Show resolved Hide resolved

Piumal1999 reviewed Jul 24, 2023

View reviewed changes

e2e/specs/record-environmental-allergy.spec.ts Outdated Show resolved Hide resolved

Piumal1999 reviewed Jul 24, 2023

View reviewed changes

e2e/specs/record-environmental-allergy.spec.ts Outdated Show resolved Hide resolved

anjula-sack requested changes Jul 24, 2023

View reviewed changes

e2e/specs/record-drug-allergies.spec.ts Outdated Show resolved Hide resolved

anjula-sack requested changes Jul 24, 2023

View reviewed changes

e2e/specs/record-food-allergy.spec.ts Outdated Show resolved Hide resolved

e2e/specs/record-food-allergy.spec.ts Outdated Show resolved Hide resolved

RandilaP marked this pull request as draft July 31, 2023 03:57

Add patient allergies e2e test

23ecdf6

RandilaP added 11 commits August 10, 2023 23:18

Adjust requested changes

85613b2

Made sepereate spec files

534b970

Change the node version

57073b6

Restructure the e2e tests

255d2e4

Add changes to patientAllergiesPage

7a7fd8d

Add a locator to table row

bac96fc

Add data test id to table

d8d9b44

Add data test ids

eee340c

Adjust new requirements

32c8f07

Adjust some changes

e8b4494

Do the requested changes

6acfda4

RandilaP force-pushed the O3-2107 branch from 0bacc28 to 6acfda4 Compare August 10, 2023 18:01

RandilaP marked this pull request as ready for review August 11, 2023 07:32

RandilaP requested review from jayasanka-sack and anjula-sack August 11, 2023 07:34

anjula-sack requested changes Aug 13, 2023

View reviewed changes

.github/workflows/e2e.yml Outdated Show resolved Hide resolved

.github/workflows/e2e.yml Outdated Show resolved Hide resolved

anjula-sack requested changes Aug 13, 2023

View reviewed changes

e2e/pages/allergies-page.ts Outdated Show resolved Hide resolved

RandilaP and others added 2 commits August 13, 2023 23:52

Adjust required changes

828fca5

Merge branch 'main' into O3-2107

c67a638

anjula-sack approved these changes Aug 15, 2023

View reviewed changes

anjula-sack merged commit 06696d7 into openmrs:main Aug 15, 2023
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(test) O3-2107: Add patient chart allergies E2E test #1205

(test) O3-2107: Add patient chart allergies E2E test #1205

RandilaP commented Jun 9, 2023 •

edited

RandilaP commented Jun 9, 2023

anjula-sack left a comment

anjula-sack commented Jun 9, 2023

anjula-sack commented Jun 9, 2023

RandilaP commented Jun 10, 2023

jayasanka-sack left a comment

anjula-sack commented Jun 19, 2023

anjula-sack left a comment

anjula-sack Jun 23, 2023

jayasanka-sack Jun 25, 2023

jayasanka-sack Jun 25, 2023

RandilaP commented Jun 23, 2023

jayasanka-sack commented Jun 25, 2023

jayasanka-sack left a comment

jayasanka-sack Jun 25, 2023

jayasanka-sack Jun 25, 2023

anjula-sack commented Aug 8, 2023

anjula-sack left a comment

	readonly tableRow = () => this.page.locator('tr');
	readonly tableRow = () => this.page.locator('tr');

(test) O3-2107: Add patient chart allergies E2E test #1205

(test) O3-2107: Add patient chart allergies E2E test #1205

Conversation

RandilaP commented Jun 9, 2023 • edited

Requirements

Summary

Screenshots

Related Issue

Other

RandilaP commented Jun 9, 2023

anjula-sack left a comment

Choose a reason for hiding this comment

anjula-sack commented Jun 9, 2023

anjula-sack commented Jun 9, 2023

RandilaP commented Jun 10, 2023

jayasanka-sack left a comment

Choose a reason for hiding this comment

anjula-sack commented Jun 19, 2023

anjula-sack left a comment

Choose a reason for hiding this comment

anjula-sack Jun 23, 2023

Choose a reason for hiding this comment

jayasanka-sack Jun 25, 2023

Choose a reason for hiding this comment

jayasanka-sack Jun 25, 2023

Choose a reason for hiding this comment

RandilaP commented Jun 23, 2023

jayasanka-sack commented Jun 25, 2023

jayasanka-sack left a comment

Choose a reason for hiding this comment

jayasanka-sack Jun 25, 2023

Choose a reason for hiding this comment

jayasanka-sack Jun 25, 2023

Choose a reason for hiding this comment

anjula-sack commented Aug 8, 2023

anjula-sack left a comment

Choose a reason for hiding this comment

RandilaP commented Jun 9, 2023 •

edited