[QA-214] Fix flaky device service history edit flow test

[nihal467]

Proposed Changes

Fixes flaky Playwright E2E tests. Originally scoped to the device service history edit flow, this PR was expanded to fold in two additional flakiness fixes surfaced while validating the same CI shards.

1. Device service history edit flow (deviceServiceHistory.spec.ts)

Failure: The "Edit an existing service record and verify changes" test intermittently failed with a Playwright strict-mode violation:

locator.click: Error: strict mode violation:
locator('[data-slot="form-item"]').filter({ hasText: 'Service Date' })
  .locator('[data-slot="popover-trigger"]') resolved to 2 elements

Root cause: After saving a new record, the test clicked the edit button before the "Add Service Record" sheet finished its close animation/unmount. Both the Add sheet and the Edit sheet momentarily rendered a Service Date popover-trigger, so the locator matched 2 elements.

Fix: Wait for the Add sheet to fully close (its Save button to become hidden) before opening the edit sheet, scope all edit interactions to the edit dialog, and target the most recently opened dialog via .last(). Applied to both edit-flow tests in the file.

2. Prescription unit-dose highlight (prescriptionCreate.spec.ts)

Root cause: Two serial tests share one encounter/medication table, but both picked a medicine via faker.helpers.arrayElement(medicineNames). From a 5-item list they occasionally picked the same medicine, so the unit-dose test's row filter matched the highlighted (non-unit) test's row and the .bg-yellow-100 count assertion failed.

Fix: Allocate two distinct medicines at describe scope via faker.helpers.arrayElements(medicineNames, 2).

3. Patient files cross-user access (patientFiles.spec.ts)

Root cause: The "file accessible to another user" test clicked the first /view/i button on a shared patient whose first row could be an archived file, opening the ArchivedFileDialog (which has two "Close" buttons) and triggering a strict-mode violation.

Fix: Upload a uniquely named file, target that specific row by name, use an exact "View", assert the "File Preview" dialog (filtered by title), and scope the Close to that dialog. Repeated for the nurse context.

• I have gone through the CONTRIBUTING guidelines.

@ohcnetwork/care-fe-code-reviewers

Summary by CodeRabbit

• Tests
  • Improved UI test reliability for editing device service records by waiting for the Add sheet to finish closing and performing all edit actions within the “Edit Service Record” dialog.
  • Made prescription creation UI tests deterministic by selecting distinct medicine names per scenario to avoid intermittent medicine table matching conflicts consistently.

[github-actions]

⚠️ Merge Checklist Incomplete

Thank you for your contribution! To help us review your PR efficiently, please complete the merge checklist in your PR description.

Your PR will be reviewed once you have marked the appropriate checklist items.

To update the checklist:

• Change - [ ] to - [x] for completed items
• Only check items that are relevant to your PR
• Leave items unchecked if they don't apply

The checklist helps ensure code quality, testing coverage, and documentation are properly addressed.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Changes

Service record edit tests

Layer / File(s)	Summary
Edit dialog control scoping tests/facility/settings/devices/deviceServiceHistory.spec.ts	Edit scenarios wait for the add-sheet Save control to hide and scope service date, Notes, and Update interactions and assertions to the edit dialog.

Prescription creation tests

Layer / File(s)	Summary
Distinct medicine test data tests/facility/patient/encounter/medicine/prescriptionCreate.spec.ts	The tests select separate medicines for highlighted and unit-dose scenarios and use the matching medicine in form inputs and table assertions.

Suggested reviewers: amjithtitus09, copilot, amjithtitus09

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly matches the main change: fixing a flaky device service history edit flow test.
Description check	✅ Passed	The description is detailed and matches the template broadly, but it omits a Fixes #issue_number reference and most merge-checklist items.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/device-service-history-edit-flake

---

_{Comment @coderabbitai help to get the list of available commands.}

[cloudflare-workers-and-pages]

Deploying care-preview with    Cloudflare Pages

Latest commit:	221c36d
Status:	✅  Deploy successful!
Preview URL:	https://5cc89f63.care-preview-a7w.pages.dev
Branch Preview URL:	https://fix-device-service-history-e.care-preview-a7w.pages.dev

View logs

[Copilot]

Pull request overview

Fixes Playwright E2E flakiness in the device service history edit-flow tests by ensuring the “Add Service Record” sheet has fully closed before interacting with the edit sheet (preventing overlapping UI elements that trigger strict-mode locator violations).

Changes:

• Added an assertion to wait for the “Save” button (from the Add sheet) to become hidden before opening the edit sheet.
• Applied the same wait to both edit-flow tests in deviceServiceHistory.spec.ts.

[greptile-apps]

Greptile Summary

This PR fixes three distinct sources of intermittent Playwright E2E test flakiness, each caused by insufficient isolation between overlapping UI state, shared test data, or shared browser resources.

• deviceServiceHistory.spec.ts: Waits for the Add sheet's Save button to become hidden before interacting with the Edit sheet, and scopes all edit-sheet interactions to page.getByRole("dialog").last() to eliminate strict-mode violations from overlapping close animations.
• prescriptionCreate.spec.ts: Pre-allocates two distinct medicines at describe scope with faker.helpers.arrayElements(medicineNames, 2) so serial tests against a shared table never accidentally target the same row.
• patientFiles.spec.ts: Extracts an openFilePreview helper that scopes to an exact-named file row, uses a unique display name per upload, and adds try/finally cleanup for the nurse browser context to prevent resource leaks when assertions fail.

Confidence Score: 5/5

All three changes are scoped exclusively to E2E test files with no effect on production code; the fixes are targeted and well-reasoned.

The changes address real, reproducible race conditions and shared-state collisions in Playwright tests. Each fix is minimal and directly tied to the described failure mode. The try/finally cleanup in patientFiles.spec.ts is a strict improvement over the original. No production logic is touched.

No files require special attention.

Important Files Changed

Filename	Overview
tests/facility/patient/encounter/medicine/prescriptionCreate.spec.ts	Allocates two distinct medicines at describe scope via arrayElements to prevent serial tests from accidentally picking the same medicine and causing assertion mismatches on shared table rows.
tests/facility/patient/patientDetails/files/patientFiles.spec.ts	Extracts a reusable openFilePreview helper that scopes interactions to a uniquely-named file row, adds try/finally cleanup for the nurse browser context, and avoids strict-mode collisions with archived-file dialogs.
tests/facility/settings/devices/deviceServiceHistory.spec.ts	Adds a wait for the Add sheet's Save button to become hidden before opening the Edit sheet, then scopes all edit interactions to page.getByRole("dialog").last() to prevent strict-mode violations from overlapping sheet animations.

_{Reviews (9): Last reviewed commit: "test: drop hardcoded timeouts, rely on g..." | Re-trigger Greptile}

[github-actions]

🎭 Playwright Test Results

Status: ✅ Passed
Test Shards: 3

Metric	Count
Total Tests	332
✅ Passed	332
❌ Failed	0
⏭️ Skipped	0

📊 Detailed results are available in the playwright-final-report artifact.

_{Run: #10281}

[nihal467]

@copilot fix the playwright test failure in the ci/cd and make sure its fully passes

[Copilot]

@copilot fix the playwright test failure in the ci/cd and make sure its fully passes

The previous toBeHidden approach wasn't sufficient — even when the Save button became hidden (opacity 0 during close animation), the Add sheet's form items remained in the DOM, still causing a strict-mode violation.

Fixed in commit test: scope edit-sheet interactions to dialog by scoping all edit-sheet interactions to page.getByRole("dialog", { name: "Edit Service Record" }). Since the Add and Edit sheets have distinct accessible names ("Add Service Record" vs "Edit Service Record"), the locator is now unambiguous regardless of whether the Add sheet is still animating out. Applied to both edit-flow tests.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/facility/settings/devices/deviceServiceHistory.spec.ts`:
- Around line 82-87: In both affected tests, wait for the Add sheet’s Save
button to become hidden immediately after saving and before clicking the Edit
trigger. Keep the existing editSheet dialog scoping and visibility assertion
unchanged, and apply this synchronization to both occurrences identified around
the edit flow.
- Around line 84-87: Update both dialog locators around editSheet and the
corresponding locator to avoid hard-coded "Edit Service Record"; resolve the
dialog name through the existing test i18n helper or use the dialog’s stable
data-slot selector so the tests work under every active locale.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 43785f93-6908-41f4-a49b-371bdf22e29c

📥 Commits

Reviewing files that changed from the base of the PR and between 6456c18 and 7dedb17.

📒 Files selected for processing (1)

• tests/facility/settings/devices/deviceServiceHistory.spec.ts

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

[Jacobjeevan]

Would this still be present since you have added the check on line 78? Not a big deal either ways, but still.