[QA-223] Add per-file E2E config override seam for patient registration - #16585
[QA-223] Add per-file E2E config override seam for patient registration#16585nihal467 wants to merge 5 commits into
Conversation
Adds a build-flag-gated (REACT_ENABLE_E2E_CONFIG_OVERRIDES) window seam in care.config.ts so Playwright specs can flip minimalPatientRegistration and minGeoOrganizationLevelsRequired per test file via page.addInitScript, without editing .env.local. Includes a build:e2e script, the applyCareConfig test helper, and example specs.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughChangesAdds a build-gated Patient registration E2E overrides
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Deploying care-preview with
|
| Latest commit: |
7a2b1c1
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://bc03779b.care-preview-a7w.pages.dev |
| Branch Preview URL: | https://e2e-patient-registration-con.care-preview-a7w.pages.dev |
nihal467
changed the title
There was a problem hiding this comment.
Pull request overview
Adds a build-flag-gated runtime config override seam to let Playwright E2E specs control select patient-registration flags per test file (via page.addInitScript), without editing .env.local, addressing the fact that import.meta.env.REACT_* values are inlined into the preview bundle.
Changes:
- Introduces
window.__CARE_E2E_CONFIG__override support incare.config.ts, gated byREACT_ENABLE_E2E_CONFIG_OVERRIDES. - Adds a
build:e2escript and env typing to enable the seam for E2E builds. - Adds a Playwright helper (
applyCareConfig) and example specs demonstrating per-file overrides.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| care.config.ts | Adds gated runtime override seam for patient-registration config used by E2E. |
| package.json | Adds build:e2e script to build with overrides enabled. |
| src/vite-env.d.ts | Types the new REACT_ENABLE_E2E_CONFIG_OVERRIDES env var. |
| tests/helper/careConfig.ts | Adds applyCareConfig(page, config) helper using page.addInitScript. |
| tests/facility/patient/patientRegistration.spec.ts | Demonstrates per-file config overrides with new helper and assertions. |
| const e2eConfigOverrides: E2EConfigOverrides = | ||
| booleanFromString(env.REACT_ENABLE_E2E_CONFIG_OVERRIDES, false) && | ||
| typeof window !== "undefined" | ||
| ? ((window as unknown as { __CARE_E2E_CONFIG__?: E2EConfigOverrides }) | ||
| .__CARE_E2E_CONFIG__ ?? {}) | ||
| : {}; |
Greptile SummaryThis PR introduces a build-flag-gated runtime seam (
Confidence Score: 5/5Safe to merge — the override seam is completely inert in normal production builds and the type/bounds validation in readE2EConfigOverrides is solid. All changes are either test-only or gated behind a build flag that is off by default. The production bundle is unaffected. The runtime parsing in care.config.ts carefully validates types and enforces minGeoOrganizationLevelsRequired >= 1 before applying any override. The test helpers document their ordering constraints clearly, and the new specs degrade gracefully via test.skip when run against a non-e2e build. No files require special attention. Important Files Changed
Reviews (4): Last reviewed commit: "test(e2e): scope patient card age/gender..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const additionalDetailsSectionText = | ||
| await additionalDetailsSection.textContent(); | ||
|
|
||
| if (additionalDetailsSectionText?.toLowerCase().includes("optional")) { | ||
| await additionalDetailsSection.click(); | ||
| } | ||
|
|
||
| await page | ||
| .getByRole("textbox", { name: "Address" }) | ||
| .fill(patientData.address); | ||
| await page | ||
| .getByRole("spinbutton", { name: "PIN Code" }) |
There was a problem hiding this comment.
The test checks additionalDetailsSectionText?.toLowerCase().includes("optional") to decide whether to click and expand the "Additional Details" section, mirroring the same pattern already in fillAdditionalDetails. If the section is collapsed but its label text doesn't contain "optional" (e.g. the label changes or the section is required in a different locale), the if block is skipped, the section remains closed, and the subsequent getByRole("textbox", { name: "Address" }) fill will fail silently or throw. Checking aria-expanded on the trigger button would be a more robust signal of the section's open/closed state.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@care.config.ts`:
- Around line 53-78: Export the E2EConfigOverrides type used by the window
override in care.config.ts. In tests/helper/careConfig.ts, remove the duplicate
CareE2EConfig declaration and import the exported type from care.config.ts,
confirming the test TypeScript/build configuration supports that src import;
update both affected sites so the app and tests share one override contract.
In `@tests/helper/careConfig.ts`:
- Around line 10-13: Remove the duplicate CareE2EConfig shape and reuse the
existing E2EConfigOverrides type from care.config.ts for the same configuration
contract. Update dependent references in the careConfig test helper to import
and use E2EConfigOverrides, keeping the existing optional fields and behavior
unchanged.
🪄 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: 4bebee06-cd95-47b2-b00b-5c2a7bdb3e59
📒 Files selected for processing (5)
care.config.tspackage.jsonsrc/vite-env.d.tstests/facility/patient/patientRegistration.spec.tstests/helper/careConfig.ts
| /** | ||
| * E2E test-only runtime overrides for patient-registration config. | ||
| * | ||
| * Build-time `import.meta.env.REACT_*` values are inlined and frozen into the | ||
| * bundle, so Playwright specs (which run against the production `preview` | ||
| * build) cannot flip them per test file. To make per-file control possible, a | ||
| * spec sets `window.__CARE_E2E_CONFIG__` via `page.addInitScript(...)` — which | ||
| * runs before this module loads — and the values below are consulted first. | ||
| * | ||
| * This seam is gated behind the `REACT_ENABLE_E2E_CONFIG_OVERRIDES` build flag | ||
| * (default off), so it is completely inert in normal production builds. The | ||
| * affected flags only relax client-side form validation; the backend | ||
| * independently enforces required fields, so this is not a security boundary. | ||
| */ | ||
| interface E2EConfigOverrides { | ||
| minGeoOrganizationLevelsRequired?: number; | ||
| minimalPatientRegistration?: boolean; | ||
| } | ||
|
|
||
| const e2eConfigOverrides: E2EConfigOverrides = | ||
| booleanFromString(env.REACT_ENABLE_E2E_CONFIG_OVERRIDES, false) && | ||
| typeof window !== "undefined" | ||
| ? ((window as unknown as { __CARE_E2E_CONFIG__?: E2EConfigOverrides }) | ||
| .__CARE_E2E_CONFIG__ ?? {}) | ||
| : {}; | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
E2EConfigOverrides and CareE2EConfig duplicate the same override contract across app and test code. Both types describe the identical window.__CARE_E2E_CONFIG__ shape by hand, with no import relationship, so the two can silently drift out of sync as fields are added or changed.
care.config.ts#L53-L78: exportE2EConfigOverrides(or an equivalent shared type) so it can be reused, instead of keeping it private to this module.tests/helper/careConfig.ts#L10-L13: import the exported type fromcare.config.ts(or a shared location both files can reach) instead of redeclaringCareE2EConfigindependently; verify the tests' tsconfig/build setup permits importing fromsrcbefore doing so.
📍 Affects 2 files
care.config.ts#L53-L78(this comment)tests/helper/careConfig.ts#L10-L13
🤖 Prompt for 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.
In `@care.config.ts` around lines 53 - 78, Export the E2EConfigOverrides type used
by the window override in care.config.ts. In tests/helper/careConfig.ts, remove
the duplicate CareE2EConfig declaration and import the exported type from
care.config.ts, confirming the test TypeScript/build configuration supports that
src import; update both affected sites so the app and tests share one override
contract.
| export type CareE2EConfig = { | ||
| minimalPatientRegistration?: boolean; | ||
| minGeoOrganizationLevelsRequired?: number; | ||
| }; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Shape duplicates E2EConfigOverrides in care.config.ts.
Both types describe the same override contract but are maintained independently. See the consolidated comment for the shared fix across both files.
🤖 Prompt for 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.
In `@tests/helper/careConfig.ts` around lines 10 - 13, Remove the duplicate
CareE2EConfig shape and reuse the existing E2EConfigOverrides type from
care.config.ts for the same configuration contract. Update dependent references
in the careConfig test helper to import and use E2EConfigOverrides, keeping the
existing optional fields and behavior unchanged.
🎭 Playwright Test ResultsStatus: ❌ Failed
📊 Detailed results are available in the playwright-final-report artifact. Run: #10332 |
|
@copilot fix all the AI review comments and the ci/cd failure, make sure it passes fully |
Co-authored-by: nihal467 <57055998+nihal467@users.noreply.github.com>
Co-authored-by: nihal467 <57055998+nihal467@users.noreply.github.com>
Addressed all review feedback and the CI failure:
Done in the latest commits. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
care.config.ts:77
- The comment says fields are "type-checked and coerced", but the implementation only validates and conditionally applies values (no coercion is performed). Updating the wording avoids misleading future readers about the safety guarantees here.
* or out of range. Each field is therefore type-checked and coerced, and
| await page.waitForURL("**/patients/home**"); | ||
| await expect(page.getByRole("heading", { name: data.name })).toBeVisible(); | ||
| await expect( | ||
| page.getByText(new RegExp(`Y,\\s*${data.gender}`, "i")).first(), | ||
| ).toBeVisible(); |
|
@copilot resolve all the AI comment |
Co-authored-by: nihal467 <57055998+nihal467@users.noreply.github.com>
Resolved the remaining AI review comment: |
| if ( | ||
| !booleanFromString(env.REACT_ENABLE_E2E_CONFIG_OVERRIDES, false) || | ||
| typeof window === "undefined" | ||
| ) { |
Proposed Changes
Adds a small, build-flag-gated seam so Playwright specs can control patient-registration config flags per test file without editing
.env.local.Why
Build-time
import.meta.env.REACT_*values are inlined and frozen into the production bundle, so specs running against thepreviewbuild cannot flip them per file. This adds a runtimewindow.__CARE_E2E_CONFIG__seam thatcare.config.tsconsults first.What
care.config.ts— newE2EConfigOverridesseam that readswindow.__CARE_E2E_CONFIG__, gated behind the newREACT_ENABLE_E2E_CONFIG_OVERRIDESbuild flag (default off, completely inert in normal production builds). Currently supportsminimalPatientRegistrationandminGeoOrganizationLevelsRequired.package.json— newbuild:e2escript that builds with the flag enabled.src/vite-env.d.ts— type for the new env var.tests/helper/careConfig.ts—applyCareConfig(page, config)helper usingpage.addInitScript(runs before app scripts load).tests/facility/patient/patientRegistration.spec.ts— example specs demonstrating per-file overrides for minimal registration and reduced geo-org levels.Notes
addInitScript, so it applies to that test only and is torn down automatically.@ohcnetwork/care-fe-code-reviewers
Summary by CodeRabbit
build:e2escript to run builds with overrides turned on.