Skip to content

Conversation

@SiddanthNayak
Copy link
Member

@SiddanthNayak SiddanthNayak commented Dec 26, 2025

Proposed Changes

Fixes #14304

  • Added tests for value set create
Screenshot 2025-12-26 at 11 37 03 AM

Tagging: @ohcnetwork/care-fe-code-reviewers

Merge Checklist

  • Add specs that demonstrate the bug or test the new feature.
  • Update product documentation.
  • Ensure that UI text is placed in I18n files.
  • Prepare a screenshot or demo video for the changelog entry and attach it to the issue.
  • Request peer reviews.
  • Complete QA on mobile devices.
  • Complete QA on desktop devices.
  • Add or update Playwright tests for related changes

Summary by CodeRabbit

  • Tests
    • Added a comprehensive test suite for ValueSet creation covering slug auto-generation and validation (5–25 characters), rule inclusion/exclusion, system selection (SNOMED, LOINC, UCUM), code/name verification, preview interactions, save flow, and metadata shown in filtered results.
    • Added test constants for slug length bounds.

✏️ Tip: You can customize this high-level summary in your review settings.

@SiddanthNayak SiddanthNayak requested review from a team and Copilot December 26, 2025 06:08
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 26, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Adds two slug-length constants and a new Playwright test suite that exercises ValueSet creation workflows: full-field creation, include/exclude rules, system selection, preview interactions, auto-generated slug behavior, and slug length validation (5–25 characters).

Changes

Cohort / File(s) Summary
ValueSet constants
tests/admin/valueset/valuesetConstants.ts
Added exported constants MIN_SLUG_LENGTH = 5 and MAX_SLUG_LENGTH = 25.
ValueSet creation tests
tests/admin/valueset/valuesetCreate.spec.ts
New Playwright test suite covering ValueSet creation with all fields, adding include/exclude rules and concepts, selecting systems (SNOMED/LOINC/UCUM), verifying preview dialog content and excluded rules, asserting list/filter results after save, checking auto-generated slug, and validating slug length (too-short, too-long, valid).

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description includes the linked issue reference and basic change summary, but the 'Add specs' and 'Update product documentation' checklist items are not checked, indicating incomplete fulfillment of requirements. Check the two unchecked checklist items ('Add specs that demonstrate the bug or test the new feature' and 'Update product documentation') or clarify if they are not applicable to this PR.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: adding Playwright tests for ValueSet creation functionality.
Linked Issues check ✅ Passed The PR adds comprehensive Playwright tests covering ValueSet creation with required fields, all fields, multiple rule configurations, and preview functionality, fully satisfying the linked issue #14304 requirements.
Out of Scope Changes check ✅ Passed All changes are directly scoped to the linked issue: adding test constants and a comprehensive Playwright test suite for ValueSet creation at /admin/valuesets/create as specified.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds comprehensive Playwright test coverage for the ValueSet creation functionality, addressing issue #14304. The tests validate form validation, code verification, preview functionality, and list display.

Key Changes:

  • Added end-to-end tests for ValueSet creation workflow with validation of all form fields and preview functionality
  • Added slug validation tests to ensure length constraints (5-25 characters) are enforced
  • Added constants for slug length validation boundaries

Reviewed changes

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

File Description
tests/admin/valueset/valuesetCreate.spec.ts Implements comprehensive test suite for ValueSet creation including field validation, code verification, preview dialog, and list display verification
tests/admin/valueset/valuesetConstants.ts Adds MIN_SLUG_LENGTH and MAX_SLUG_LENGTH constants to support slug validation tests

page.getByRole("textbox", { name: "Slug *" }),
);
await expect(errorMessage).toBeVisible();
await expect(errorMessage).toHaveText(/atleast \d+.*atmost 25/i);
Copy link

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The regex pattern matches 'atleast' and 'atmost', but the correct English phrases are 'at least' (two words) and 'at most' (two words). If this is matching actual error message text, the error message itself may need correction. Update the test to match the corrected error message format.

Copilot uses AI. Check for mistakes.
await page.getByRole("button", { name: /save/i }).click();

await expect(errorMessage).toBeVisible();
await expect(errorMessage).toHaveText(/atleast \d+.*atmost 25/i);
Copy link

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The regex pattern matches 'atleast' and 'atmost', but the correct English phrases are 'at least' (two words) and 'at most' (two words). If this is matching actual error message text, the error message itself may need correction. Update the test to match the corrected error message format.

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

📜 Review details

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0aed664 and 676053e.

📒 Files selected for processing (2)
  • tests/admin/valueset/valuesetConstants.ts
  • tests/admin/valueset/valuesetCreate.spec.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{ts,tsx}: Write concise, technical TypeScript code with accurate examples
Use TypeScript for all code; prefer interfaces over types
Avoid enums; use maps instead
Use TanStack Query for data fetching from the API along with query and mutate utilities for the queryFn and mutationFn
Use raviger for routing

**/*.{ts,tsx}: Use TypeScript with strict mode and ES2022 target
Use interface for defining object types in TypeScript
Avoid explicit any type in TypeScript
Use proper nullability annotations in TypeScript types
Use dedicated error handlers and TypeScript strict null checks

**/*.{ts,tsx}: Use TypeScript for all new code
Prefer interfaces over types for object definitions in TypeScript
Avoid using any type; use proper type definitions in TypeScript
Use type inference where possible in TypeScript
Use TanStack Query for API data management
Prefer React Context for global state management

**/*.{ts,tsx}: Use TanStack Query with the query and mutate utilities from @/Utils/request/
Use appropriate query keys following the resource pattern for TanStack Query
Leverage TanStack Query built-in features for pagination and debouncing
Implement proper error handling using the global error handler for TanStack Query operations
Use useQuery hook with queryKey and queryFn parameters, where queryFn wraps the API route with the query() utility
Use useMutation hook with mutationFn parameter wrapping API routes with the mutate() utility, and implement onSuccess callbacks to invalidate related queries
Support path parameters in TanStack Query using the pathParams option in query/mutate utilities
Support query parameters in TanStack Query using the queryParams option in query/mutate utilities
Use the silent: true option to suppress global error notifications when custom error handling is needed

**/*.{ts,tsx}: Use TypeScript with strict mode and ES2022 target
Use interface for defining object types
Avoid explicit any types and maint...

Files:

  • tests/admin/valueset/valuesetConstants.ts
  • tests/admin/valueset/valuesetCreate.spec.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{ts,tsx,js,jsx}: Use functional and declarative programming patterns; avoid classes
Prefer iteration and modularization over code duplication
Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError)
Use the "function" keyword for pure functions
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements

**/*.{ts,tsx,js,jsx}: Use path aliases for imports from src (@/*)
Use double quotes for strings
Semicolons are required at end of statements
Order imports as: 3rd-party → library → CAREUI → UI → components → hooks → utils → relative
Use PascalCase for component and class names
Use camelCase for variable and function names

**/*.{ts,tsx,js,jsx}: Use path aliases @/* for imports from src directory
Use double quotes for strings
Require semicolons at end of statements
Order imports: 3rd-party → library → CAREUI → UI → components → hooks → utils → relative
Use PascalCase for component and class names
Use camelCase for variable and function names

Files:

  • tests/admin/valueset/valuesetConstants.ts
  • tests/admin/valueset/valuesetCreate.spec.ts
**/*

📄 CodeRabbit inference engine (CLAUDE.md)

Use 2-space indentation

Files:

  • tests/admin/valueset/valuesetConstants.ts
  • tests/admin/valueset/valuesetCreate.spec.ts
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/02-coding-standards.mdc)

Use camelCase for utility file names (e.g., useAuth.ts)

Files:

  • tests/admin/valueset/valuesetConstants.ts
  • tests/admin/valueset/valuesetCreate.spec.ts
**/*.{tsx,ts}

📄 CodeRabbit inference engine (.cursor/rules/04-ui-components.mdc)

**/*.{tsx,ts}: Use Shadcn UI components as the primary component library
Follow the component documentation for proper usage
Customize components using Tailwind CSS
Use navigate and useRedirect from raviger for navigation and redirects
Use Tailwind's responsive classes and follow mobile-first approach
Implement proper ARIA attributes for accessibility
Ensure keyboard navigation works in components
Define component props using TypeScript interfaces
Use translation keys from src/Locale/ for internationalization
Support RTL languages in components
Use proper date/time formatting for multiple language support

Files:

  • tests/admin/valueset/valuesetConstants.ts
  • tests/admin/valueset/valuesetCreate.spec.ts
**/*.{ts,tsx,js,jsx,json,css,scss,html}

📄 CodeRabbit inference engine (AGENTS.md)

Use 2-space indentation

Files:

  • tests/admin/valueset/valuesetConstants.ts
  • tests/admin/valueset/valuesetCreate.spec.ts
**/*.spec.ts

📄 CodeRabbit inference engine (.cursor/rules/02-coding-standards.mdc)

**/*.spec.ts: Write E2E tests in Playwright following https://playwright.dev/docs/writing-tests
Write meaningful test descriptions

Files:

  • tests/admin/valueset/valuesetCreate.spec.ts
🧠 Learnings (18)
📓 Common learnings
Learnt from: yash-learner
Repo: ohcnetwork/care_fe PR: 14108
File: tests/facility/settings/activityDefinitions/activityDefinitionCreate.spec.ts:86-147
Timestamp: 2025-11-20T09:48:26.486Z
Learning: In tests/facility/settings/activityDefinitions/activityDefinitionCreate.spec.ts, detailed verification of selected values (e.g., code from value sets) in the detail view is intentionally deferred to the list spec, as the creation test focuses on the happy path and basic verification only.
Learnt from: AdityaJ2305
Repo: ohcnetwork/care_fe PR: 13197
File: src/pages/Facility/settings/observationDefinition/ObservationDefinitionForm.tsx:470-495
Timestamp: 2025-09-16T20:46:59.537Z
Learning: In PR #13197, AdityaJ2305 indicated that aria-invalid and ref forwarding improvements for ValueSetSelect components can be deferred to a separate PR, considering them not critical for the current focus/scroll fix implementation.
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T13:50:10.786Z
Learning: Playwright E2E tests required for critical healthcare workflows and must be run before deployment
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/careui.instructions.md:0-0
Timestamp: 2025-11-25T13:50:46.407Z
Learning: Applies to src/CAREUI/**/*.{test,spec}.{ts,tsx} : Implement comprehensive testing in CAREUI components including accessibility and medical workflow scenarios
📚 Learning: 2025-11-25T13:50:54.683Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/common.instructions.md:0-0
Timestamp: 2025-11-25T13:50:54.683Z
Learning: Applies to src/common/**/constants.tsx : Define core application configuration including RESULTS_PER_PAGE_LIMIT for API pagination in constants.tsx

Applied to files:

  • tests/admin/valueset/valuesetConstants.ts
📚 Learning: 2025-09-29T14:07:50.923Z
Learnt from: GSMPRANEETH
Repo: ohcnetwork/care_fe PR: 13750
File: src/components/Questionnaire/MedicationValueSetSelect.tsx:240-254
Timestamp: 2025-09-29T14:07:50.923Z
Learning: The codebase uses src/common/constants.tsx for application-wide constants like RESULTS_PER_PAGE_LIMIT, and follows uppercase naming convention for constants.

Applied to files:

  • tests/admin/valueset/valuesetConstants.ts
📚 Learning: 2025-11-25T13:53:40.623Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/utils.instructions.md:0-0
Timestamp: 2025-11-25T13:53:40.623Z
Learning: Applies to src/Utils/**/*.{ts,tsx} : Define constants in appropriate utility files with type-safe and well-documented values

Applied to files:

  • tests/admin/valueset/valuesetConstants.ts
📚 Learning: 2025-11-25T13:51:41.208Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/lib.instructions.md:0-0
Timestamp: 2025-11-25T13:51:41.208Z
Learning: Applies to src/lib/**/*.{ts,tsx} : Handle medical coding systems with SNOMED CT and LOINC standards

Applied to files:

  • tests/admin/valueset/valuesetConstants.ts
📚 Learning: 2025-11-20T09:48:26.486Z
Learnt from: yash-learner
Repo: ohcnetwork/care_fe PR: 14108
File: tests/facility/settings/activityDefinitions/activityDefinitionCreate.spec.ts:86-147
Timestamp: 2025-11-20T09:48:26.486Z
Learning: In tests/facility/settings/activityDefinitions/activityDefinitionCreate.spec.ts, detailed verification of selected values (e.g., code from value sets) in the detail view is intentionally deferred to the list spec, as the creation test focuses on the happy path and basic verification only.

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:54:09.978Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .cursor/rules/02-coding-standards.mdc:0-0
Timestamp: 2025-11-25T13:54:09.978Z
Learning: Applies to **/*.spec.ts : Write E2E tests in Playwright following https://playwright.dev/docs/writing-tests

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:53:40.623Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/utils.instructions.md:0-0
Timestamp: 2025-11-25T13:53:40.623Z
Learning: Applies to src/Utils/**/*.{ts,tsx} : Create test helpers and fixtures as separate utility files; keep test utilities separate from production code

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:52:33.081Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/providers.instructions.md:0-0
Timestamp: 2025-11-25T13:52:33.081Z
Learning: Applies to src/Providers/**/*.{test,spec}.{ts,tsx} : Test provider state management and updates with comprehensive unit tests

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:53:40.623Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/utils.instructions.md:0-0
Timestamp: 2025-11-25T13:53:40.623Z
Learning: Applies to src/Utils/**/*.{ts,tsx} : Provide utilities for common testing scenarios in test utility files

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:50:46.407Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/careui.instructions.md:0-0
Timestamp: 2025-11-25T13:50:46.407Z
Learning: Applies to src/CAREUI/**/*.{test,spec}.{ts,tsx} : Implement comprehensive testing in CAREUI components including accessibility and medical workflow scenarios

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:51:07.680Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/config-files.instructions.md:0-0
Timestamp: 2025-11-25T13:51:07.680Z
Learning: Applies to playwright.config.ts : Maintain `playwright.config.ts` as the Playwright testing configuration

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:54:09.978Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .cursor/rules/02-coding-standards.mdc:0-0
Timestamp: 2025-11-25T13:54:09.978Z
Learning: Applies to **/*.spec.ts : Write meaningful test descriptions

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:52:33.081Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/providers.instructions.md:0-0
Timestamp: 2025-11-25T13:52:33.081Z
Learning: Applies to src/Providers/**/*.{test,spec}.{ts,tsx} : Validate audit logging and compliance features in provider tests

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-20T09:26:50.157Z
Learnt from: yash-learner
Repo: ohcnetwork/care_fe PR: 14108
File: tests/facility/settings/activityDefinitions/activityDefinitionCreate.spec.ts:91-94
Timestamp: 2025-11-20T09:26:50.157Z
Learning: In tests/facility/settings/activityDefinitions/activityDefinitionCreate.spec.ts, the hard-coded bounds for faker.number.int (e.g., 0-4, 0-2) when selecting itemIndex are intentionally based on stable backend seed data that is guaranteed to be available in the test environment, making the approach valid and not flaky.

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2024-12-04T18:58:47.241Z
Learnt from: kihan2518B
Repo: ohcnetwork/care_fe PR: 8956
File: cypress/e2e/facility_spec/FacilityCreation.cy.ts:261-261
Timestamp: 2024-12-04T18:58:47.241Z
Learning: In `cypress/e2e/facility_spec/FacilityCreation.cy.ts`, the test case titled "Create a new facility with no bed and doctor capacity" includes steps to select bed types and specializations before validation. This is done intentionally to verify that error messages are shown properly when no capacity is specified.

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-01-15T11:05:16.663Z
Learnt from: nihal467
Repo: ohcnetwork/care_fe PR: 9986
File: cypress/e2e/users_spec/user_creation.cy.ts:43-55
Timestamp: 2025-01-15T11:05:16.663Z
Learning: UI verification for user creation in the users list should be implemented only after search functionality is available in the UI to avoid flaky tests. Currently, the test focuses on API verification only.

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:51:23.408Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/context.instructions.md:0-0
Timestamp: 2025-11-25T13:51:23.408Z
Learning: Applies to src/context/**/*.{test,spec}.{ts,tsx} : Validate emergency override scenarios in context tests

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
🧬 Code graph analysis (1)
tests/admin/valueset/valuesetCreate.spec.ts (4)
tests/helper/utils.ts (1)
  • expectedSlug (6-8)
tests/admin/valueset/valuesetConstants.ts (10)
  • STATUS_OPTIONS (93-93)
  • SYSTEM_OPTIONS (91-91)
  • VALID_SNOMED_CODES (1-12)
  • SNOMED_CODE_NAME (14-25)
  • VALID_LOINC_CODES (27-38)
  • LOINC_CODE_NAME (40-51)
  • VALID_UCUM_CODES (53-63)
  • UCUM_CODE_NAME (65-75)
  • MIN_SLUG_LENGTH (95-95)
  • MAX_SLUG_LENGTH (96-96)
tests/helper/ui.ts (2)
  • closeAnyOpenPopovers (47-61)
  • expectToast (345-352)
tests/helper/error.ts (1)
  • getFieldErrorMessage (16-18)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: build
  • GitHub Check: Test
  • GitHub Check: CodeQL-Build
🔇 Additional comments (2)
tests/admin/valueset/valuesetConstants.ts (1)

95-96: LGTM! Well-defined constants for slug validation.

The constants are appropriately named, exported, and will support consistent slug validation testing across the test suite.

tests/admin/valueset/valuesetCreate.spec.ts (1)

118-130: No changes needed. The Status field has a default value of ValueSetStatus.ACTIVE (line 380 of ValueSetForm.tsx), so tests that don't explicitly select a status will use this default when submitting the form. Tests 2 and 3 don't attempt to save, making Status selection irrelevant. Test 4 successfully saves with the default Status value.

Comment on lines +78 to +81
const displayValue = await page
.getByRole("textbox", { name: "Unverified" })
.inputValue();
expect(displayValue).toContain(codeName);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Potentially fragile selector using "Unverified" after code verification.

After clicking "Verify code" and confirming success, the textbox label might change from "Unverified" to something else (e.g., "Verified" or a code-specific label), making this selector brittle.

Consider using a more stable selector such as a test-id or a selector that doesn't depend on verification state, or verify that the label remains "Unverified" in the UI even after successful verification.

🤖 Prompt for AI Agents
In tests/admin/valueset/valuesetCreate.spec.ts around lines 78 to 81, the test
uses getByRole("textbox", { name: "Unverified" }) which is brittle because the
textbox label can change after verification; replace this with a stable selector
(preferably a data-testid on the input) or select the textbox by its
position/aria-label that does not depend on verification state, then read its
value and assert it contains codeName; if adding a test-id is not possible,
locate the textbox via a surrounding fieldset/label text that is stable or by
input[name|id] and use that instead.

Comment on lines +95 to +97
await previewDialog.getByRole("combobox").click();

await expect(page.getByText(codeName)).toBeVisible();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Use dialog-scoped selectors for more robust assertions.

Lines 95 and 97 use selectors that could be fragile:

  • Line 95: previewDialog.getByRole("combobox") lacks a specific name, which could fail if multiple comboboxes exist in the dialog.
  • Line 97: page.getByText(codeName) checks the entire page instead of the dialog, which could produce false positives if codeName appears elsewhere.
🔎 Recommended improvements
-    await previewDialog.getByRole("combobox").click();
+    await previewDialog.getByRole("combobox", { name: "Select code" }).click(); // or appropriate accessible name

-    await expect(page.getByText(codeName)).toBeVisible();
+    await expect(previewDialog.getByText(codeName)).toBeVisible();

Note: Adjust the combobox name to match the actual accessible name in the UI.

📝 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.

Suggested change
await previewDialog.getByRole("combobox").click();
await expect(page.getByText(codeName)).toBeVisible();
await previewDialog.getByRole("combobox", { name: "Select code" }).click(); // or appropriate accessible name
await expect(previewDialog.getByText(codeName)).toBeVisible();
🤖 Prompt for AI Agents
In tests/admin/valueset/valuesetCreate.spec.ts around lines 95 to 97, the test
uses non-specific selectors (previewDialog.getByRole("combobox") without a name
and page.getByText(codeName)), which can be fragile and cause false positives;
update the combobox selection to use the dialog-scoped selector with its
accessible name (e.g., previewDialog.getByRole("combobox", { name:
"COMBOBOX_NAME" })) and change the page-wide text assertion to a dialog-scoped
assertion (e.g., previewDialog.getByText(codeName)) so both selectors are scoped
to the dialog and unambiguous.

Comment on lines +124 to +125
expect(slugValue).toBeTruthy();
expect(slugValue.length).toBeGreaterThan(0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Remove redundant slug value check.

Checking both toBeTruthy() and length > 0 is redundant since any string with length > 0 is already truthy.

🔎 Simplified check
     const slugValue = await slugField.inputValue();
-    expect(slugValue).toBeTruthy();
-    expect(slugValue.length).toBeGreaterThan(0);
+    expect(slugValue.length).toBeGreaterThan(0);
📝 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.

Suggested change
expect(slugValue).toBeTruthy();
expect(slugValue.length).toBeGreaterThan(0);
const slugValue = await slugField.inputValue();
expect(slugValue.length).toBeGreaterThan(0);
🤖 Prompt for AI Agents
In tests/admin/valueset/valuesetCreate.spec.ts around lines 124 to 125, the test
redundantly asserts both expect(slugValue).toBeTruthy() and
expect(slugValue.length).toBeGreaterThan(0); remove one of these redundant
checks — keep the explicit length assertion
(expect(slugValue.length).toBeGreaterThan(0)) or, if you prefer truthiness, keep
expect(slugValue).toBeTruthy() — and delete the other assertion so the test only
verifies the slug once.


const expectedSlugValue = expectedSlug(name);

expect(slugValue).toContain(expectedSlugValue);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider using toBe for precise slug matching.

Using toContain instead of toBe could miss cases where the slug has extra characters or formatting. If the slug should exactly match expectedSlug(name), use toBe for a stricter assertion.

🔎 More precise assertion
-    expect(slugValue).toContain(expectedSlugValue);
+    expect(slugValue).toBe(expectedSlugValue);

Use toContain only if additional slug processing (e.g., uniqueness suffixes) is expected.

📝 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.

Suggested change
expect(slugValue).toContain(expectedSlugValue);
expect(slugValue).toBe(expectedSlugValue);
🤖 Prompt for AI Agents
In tests/admin/valueset/valuesetCreate.spec.ts around line 129, the test
currently uses expect(slugValue).toContain(expectedSlugValue); which allows
extra characters in the slug; replace this with a strict equality assertion
using expect(slugValue).toBe(expectedSlugValue) so the test fails if the slug
differs or contains unexpected suffixes, unless the code intentionally appends
uniqueness or formatting — in that case leave toContain and document the
expectation.

Comment on lines +152 to +155
const displayValue = await page
.getByRole("textbox", { name: "Unverified" })
.inputValue();
expect(displayValue).toContain(codeName);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Potentially fragile selector using "Unverified" after code verification.

Same issue as in the first test: after code verification succeeds, the textbox label might change, making this selector unreliable.

Consider using a selector that doesn't depend on verification state.

🤖 Prompt for AI Agents
In tests/admin/valueset/valuesetCreate.spec.ts around lines 152-155, the test is
using page.getByRole("textbox", { name: "Unverified" }) which is brittle because
the label changes after verification; replace this locator with one that does
not depend on the verification state — e.g. target the textbox by a stable
attribute (data-testid or input id/aria-label/placeholder that is invariant), or
add a data-testid to the input in the component and use
page.getByTestId("...").inputValue() so the assertion no longer relies on the
"Unverified" label.

Comment on lines +169 to +171
await previewDialog.getByRole("combobox").click();

await expect(page.getByText(codeName)).not.toBeVisible();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Scope selectors to the preview dialog for accurate assertions.

Lines 169-171 have the same selector issues as the first test:

  • Line 169: Unspecified combobox could be fragile.
  • Line 171: page.getByText(codeName).not.toBeVisible() checks the entire page, which could produce false negatives if codeName is hidden in the dialog but visible elsewhere on the page.
🔎 Recommended improvements
-    await previewDialog.getByRole("combobox").click();
+    await previewDialog.getByRole("combobox", { name: "Select code" }).click(); // use appropriate accessible name

-    await expect(page.getByText(codeName)).not.toBeVisible();
+    await expect(previewDialog.getByText(codeName)).not.toBeVisible();
📝 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.

Suggested change
await previewDialog.getByRole("combobox").click();
await expect(page.getByText(codeName)).not.toBeVisible();
await previewDialog.getByRole("combobox", { name: "Select code" }).click();
await expect(previewDialog.getByText(codeName)).not.toBeVisible();
🤖 Prompt for AI Agents
tests/admin/valueset/valuesetCreate.spec.ts lines 169-171: the selectors are too
global — scope them to the previewDialog to avoid false negatives; replace the
unspecified combobox selector with a scoped one (e.g.,
previewDialog.getByRole("combobox", { name: /.../ }) or at minimum
previewDialog.getByRole("combobox").click()) and change
page.getByText(codeName).not.toBeVisible() to
previewDialog.getByText(codeName).not.toBeVisible() so the assertion only checks
the dialog contents.

page.getByRole("textbox", { name: "Slug *" }),
);
await expect(errorMessage).toBeVisible();
await expect(errorMessage).toHaveText(/atleast \d+.*atmost 25/i);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider whether hard-coded "25" in error message check needs updating.

The error message regex hard-codes "25" instead of referencing MAX_SLUG_LENGTH. While this is checking UI text (not validation logic), if MAX_SLUG_LENGTH changes, this regex would need manual updating.

This is acceptable as-is since error messages are typically fixed strings, but consider documenting that this test expects the UI to display "atmost 25" in the error message.

Note: The spelling "atleast" (vs. "at least") should match the actual UI text. Verify this is intentional.

🤖 Prompt for AI Agents
In tests/admin/valueset/valuesetCreate.spec.ts around line 192, the regex
asserts "atmost 25" with a hard-coded 25; replace the literal with the project
constant (MAX_SLUG_LENGTH) so the expectation follows any future changes, e.g.,
build the regex/string using that constant imported from the module that defines
it; alternatively, if you intend this test to lock UI wording, add a brief
comment above the assertion stating the test intentionally expects the fixed
text "atmost 25" and verify the spelling "atleast"/"at least" matches the UI
before leaving it hard-coded.

@github-actions
Copy link

github-actions bot commented Dec 26, 2025

🎭 Playwright Test Results

Status: ✅ Passed
Test Shards: 3

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

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

Run: #4178

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings January 2, 2026 10:41
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment on lines +21 to +29
let name: string;
let slug: string;
let status: string;
let description: string;
let code: string;
let system: string;
let codeName: string;

test.describe("ValueSet Create", () => {
Copy link

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test variables are declared at module scope but only initialized in beforeEach. This pattern can lead to test interdependencies if tests run in parallel or if a test fails before beforeEach completes. Consider moving these declarations inside the describe block or using a factory function to generate fresh test data for each test.

Suggested change
let name: string;
let slug: string;
let status: string;
let description: string;
let code: string;
let system: string;
let codeName: string;
test.describe("ValueSet Create", () => {
test.describe("ValueSet Create", () => {
let name: string;
let slug: string;
let status: string;
let description: string;
let code: string;
let system: string;
let codeName: string;

Copilot uses AI. Check for mistakes.
case "UCUM":
code = faker.helpers.arrayElement(VALID_UCUM_CODES);
codeName = UCUM_CODE_NAME[code];
break;
Copy link

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The switch statement lacks a default case, which means if system is randomly selected to a value outside ['SNOMED', 'LOINC', 'UCUM'], the variables code and codeName will remain undefined. This will cause test failures in tests that use these variables. Add a default case that throws an error or ensure the random selection is properly constrained.

Suggested change
break;
break;
default:
throw new Error(`Unsupported system: ${system}`);

Copilot uses AI. Check for mistakes.
page.getByRole("textbox", { name: "Slug *" }),
);
await expect(errorMessage).toBeVisible();
await expect(errorMessage).toHaveText(/atleast \d+.*atmost 25/i);
Copy link

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected spelling of 'atleast' to 'at least' in the error message pattern.

Copilot uses AI. Check for mistakes.
await page.getByRole("button", { name: /save/i }).click();

await expect(errorMessage).toBeVisible();
await expect(errorMessage).toHaveText(/atleast \d+.*atmost 25/i);
Copy link

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected spelling of 'atleast' to 'at least' in the error message pattern.

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

♻️ Duplicate comments (7)
tests/admin/valueset/valuesetCreate.spec.ts (7)

78-81: Potentially fragile selector using "Unverified" after code verification.

After clicking "Verify code" and confirming success, the textbox label might change from "Unverified" to something else, making this selector unreliable. Consider using a more stable selector such as a data-testid or an input-specific attribute that doesn't depend on verification state.


95-97: Use dialog-scoped selectors for more robust assertions.

Line 95 uses an unspecified combobox selector that could fail if multiple comboboxes exist. Line 97 checks the entire page instead of the dialog, which could produce false positives if codeName appears elsewhere.

🔎 Recommended improvements
-    await previewDialog.getByRole("combobox").click();
+    await previewDialog.getByRole("combobox", { name: "Select code" }).click();

-    await expect(page.getByText(codeName)).toBeVisible();
+    await expect(previewDialog.getByText(codeName)).toBeVisible();

Note: Adjust the combobox name to match the actual accessible name in the UI.


124-125: Remove redundant slug value check.

Checking both toBeTruthy() and length > 0 is redundant since any string with length > 0 is already truthy.

🔎 Simplified check
     const slugValue = await slugField.inputValue();
-    expect(slugValue).toBeTruthy();
-    expect(slugValue.length).toBeGreaterThan(0);
+    expect(slugValue.length).toBeGreaterThan(0);

129-129: Consider using toBe for precise slug matching.

Using toContain instead of toBe could miss cases where the slug has extra characters. If the slug should exactly match expectedSlug(name), use toBe for stricter validation.

🔎 More precise assertion
-    expect(slugValue).toContain(expectedSlugValue);
+    expect(slugValue).toBe(expectedSlugValue);

Use toContain only if additional slug processing (e.g., uniqueness suffixes) is expected.


152-155: Potentially fragile selector using "Unverified" after code verification.

Same issue as in the first test: after code verification succeeds, the textbox label might change, making this selector unreliable. Consider using a selector that doesn't depend on verification state.


169-171: Scope selectors to the preview dialog for accurate assertions.

Line 169 uses an unspecified combobox that could be fragile. Line 171 checks the entire page instead of the dialog, which could produce false negatives if codeName is hidden in the dialog but visible elsewhere.

🔎 Recommended improvements
-    await previewDialog.getByRole("combobox").click();
+    await previewDialog.getByRole("combobox", { name: "Select code" }).click();

-    await expect(page.getByText(codeName)).not.toBeVisible();
+    await expect(previewDialog.getByText(codeName)).not.toBeVisible();

192-202: Hard-coded "25" and potential spelling issue in error message regex.

Lines 192 and 202 hard-code "25" in the regex instead of using MAX_SLUG_LENGTH. Additionally, the regex matches "atleast" and "atmost" (one word), but the correct English phrases are "at least" and "at most" (two words). If this matches actual error message text, the error message itself may need correction.

📜 Review details

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 676053e and 3ae5d19.

📒 Files selected for processing (1)
  • tests/admin/valueset/valuesetCreate.spec.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{ts,tsx}: Write concise, technical TypeScript code with accurate examples
Use TypeScript for all code; prefer interfaces over types
Avoid enums; use maps instead
Use TanStack Query for data fetching from the API along with query and mutate utilities for the queryFn and mutationFn
Use raviger for routing

**/*.{ts,tsx}: Use TypeScript with strict mode and ES2022 target
Use interface for defining object types in TypeScript
Avoid explicit any type in TypeScript
Use proper nullability annotations in TypeScript types
Use dedicated error handlers and TypeScript strict null checks

**/*.{ts,tsx}: Use TypeScript for all new code
Prefer interfaces over types for object definitions in TypeScript
Avoid using any type; use proper type definitions in TypeScript
Use type inference where possible in TypeScript
Use TanStack Query for API data management
Prefer React Context for global state management

**/*.{ts,tsx}: Use TanStack Query with the query and mutate utilities from @/Utils/request/
Use appropriate query keys following the resource pattern for TanStack Query
Leverage TanStack Query built-in features for pagination and debouncing
Implement proper error handling using the global error handler for TanStack Query operations
Use useQuery hook with queryKey and queryFn parameters, where queryFn wraps the API route with the query() utility
Use useMutation hook with mutationFn parameter wrapping API routes with the mutate() utility, and implement onSuccess callbacks to invalidate related queries
Support path parameters in TanStack Query using the pathParams option in query/mutate utilities
Support query parameters in TanStack Query using the queryParams option in query/mutate utilities
Use the silent: true option to suppress global error notifications when custom error handling is needed

**/*.{ts,tsx}: Use TypeScript with strict mode and ES2022 target
Use interface for defining object types
Avoid explicit any types and maint...

Files:

  • tests/admin/valueset/valuesetCreate.spec.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{ts,tsx,js,jsx}: Use functional and declarative programming patterns; avoid classes
Prefer iteration and modularization over code duplication
Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError)
Use the "function" keyword for pure functions
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements

**/*.{ts,tsx,js,jsx}: Use path aliases for imports from src (@/*)
Use double quotes for strings
Semicolons are required at end of statements
Order imports as: 3rd-party → library → CAREUI → UI → components → hooks → utils → relative
Use PascalCase for component and class names
Use camelCase for variable and function names

**/*.{ts,tsx,js,jsx}: Use path aliases @/* for imports from src directory
Use double quotes for strings
Require semicolons at end of statements
Order imports: 3rd-party → library → CAREUI → UI → components → hooks → utils → relative
Use PascalCase for component and class names
Use camelCase for variable and function names

Files:

  • tests/admin/valueset/valuesetCreate.spec.ts
**/*

📄 CodeRabbit inference engine (CLAUDE.md)

Use 2-space indentation

Files:

  • tests/admin/valueset/valuesetCreate.spec.ts
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/02-coding-standards.mdc)

Use camelCase for utility file names (e.g., useAuth.ts)

Files:

  • tests/admin/valueset/valuesetCreate.spec.ts
**/*.spec.ts

📄 CodeRabbit inference engine (.cursor/rules/02-coding-standards.mdc)

**/*.spec.ts: Write E2E tests in Playwright following https://playwright.dev/docs/writing-tests
Write meaningful test descriptions

Files:

  • tests/admin/valueset/valuesetCreate.spec.ts
**/*.{tsx,ts}

📄 CodeRabbit inference engine (.cursor/rules/04-ui-components.mdc)

**/*.{tsx,ts}: Use Shadcn UI components as the primary component library
Follow the component documentation for proper usage
Customize components using Tailwind CSS
Use navigate and useRedirect from raviger for navigation and redirects
Use Tailwind's responsive classes and follow mobile-first approach
Implement proper ARIA attributes for accessibility
Ensure keyboard navigation works in components
Define component props using TypeScript interfaces
Use translation keys from src/Locale/ for internationalization
Support RTL languages in components
Use proper date/time formatting for multiple language support

Files:

  • tests/admin/valueset/valuesetCreate.spec.ts
**/*.{ts,tsx,js,jsx,json,css,scss,html}

📄 CodeRabbit inference engine (AGENTS.md)

Use 2-space indentation

Files:

  • tests/admin/valueset/valuesetCreate.spec.ts
🧠 Learnings (26)
📓 Common learnings
Learnt from: yash-learner
Repo: ohcnetwork/care_fe PR: 14108
File: tests/facility/settings/activityDefinitions/activityDefinitionCreate.spec.ts:86-147
Timestamp: 2025-11-20T09:48:26.486Z
Learning: In tests/facility/settings/activityDefinitions/activityDefinitionCreate.spec.ts, detailed verification of selected values (e.g., code from value sets) in the detail view is intentionally deferred to the list spec, as the creation test focuses on the happy path and basic verification only.
Learnt from: AdityaJ2305
Repo: ohcnetwork/care_fe PR: 13197
File: src/pages/Facility/settings/observationDefinition/ObservationDefinitionForm.tsx:470-495
Timestamp: 2025-09-16T20:46:59.537Z
Learning: In PR #13197, AdityaJ2305 indicated that aria-invalid and ref forwarding improvements for ValueSetSelect components can be deferred to a separate PR, considering them not critical for the current focus/scroll fix implementation.
📚 Learning: 2025-11-20T09:48:26.486Z
Learnt from: yash-learner
Repo: ohcnetwork/care_fe PR: 14108
File: tests/facility/settings/activityDefinitions/activityDefinitionCreate.spec.ts:86-147
Timestamp: 2025-11-20T09:48:26.486Z
Learning: In tests/facility/settings/activityDefinitions/activityDefinitionCreate.spec.ts, detailed verification of selected values (e.g., code from value sets) in the detail view is intentionally deferred to the list spec, as the creation test focuses on the happy path and basic verification only.

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:54:09.978Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .cursor/rules/02-coding-standards.mdc:0-0
Timestamp: 2025-11-25T13:54:09.978Z
Learning: Applies to **/*.spec.ts : Write E2E tests in Playwright following https://playwright.dev/docs/writing-tests

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:50:46.407Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/careui.instructions.md:0-0
Timestamp: 2025-11-25T13:50:46.407Z
Learning: Applies to src/CAREUI/**/*.{test,spec}.{ts,tsx} : Implement comprehensive testing in CAREUI components including accessibility and medical workflow scenarios

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:53:40.623Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/utils.instructions.md:0-0
Timestamp: 2025-11-25T13:53:40.623Z
Learning: Applies to src/Utils/**/*.{ts,tsx} : Create test helpers and fixtures as separate utility files; keep test utilities separate from production code

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:54:09.978Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .cursor/rules/02-coding-standards.mdc:0-0
Timestamp: 2025-11-25T13:54:09.978Z
Learning: Applies to **/*.spec.ts : Write meaningful test descriptions

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:52:33.081Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/providers.instructions.md:0-0
Timestamp: 2025-11-25T13:52:33.081Z
Learning: Applies to src/Providers/**/*.{test,spec}.{ts,tsx} : Test provider state management and updates with comprehensive unit tests

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-20T09:26:50.157Z
Learnt from: yash-learner
Repo: ohcnetwork/care_fe PR: 14108
File: tests/facility/settings/activityDefinitions/activityDefinitionCreate.spec.ts:91-94
Timestamp: 2025-11-20T09:26:50.157Z
Learning: In tests/facility/settings/activityDefinitions/activityDefinitionCreate.spec.ts, the hard-coded bounds for faker.number.int (e.g., 0-4, 0-2) when selecting itemIndex are intentionally based on stable backend seed data that is guaranteed to be available in the test environment, making the approach valid and not flaky.

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:53:40.623Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/utils.instructions.md:0-0
Timestamp: 2025-11-25T13:53:40.623Z
Learning: Applies to src/Utils/**/*.{ts,tsx} : Provide utilities for common testing scenarios in test utility files

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2024-12-04T18:58:47.241Z
Learnt from: kihan2518B
Repo: ohcnetwork/care_fe PR: 8956
File: cypress/e2e/facility_spec/FacilityCreation.cy.ts:261-261
Timestamp: 2024-12-04T18:58:47.241Z
Learning: In `cypress/e2e/facility_spec/FacilityCreation.cy.ts`, the test case titled "Create a new facility with no bed and doctor capacity" includes steps to select bed types and specializations before validation. This is done intentionally to verify that error messages are shown properly when no capacity is specified.

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:51:23.408Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/context.instructions.md:0-0
Timestamp: 2025-11-25T13:51:23.408Z
Learning: Applies to src/context/**/*.{test,spec}.{ts,tsx} : Verify audit trail completeness in context tests

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2024-12-05T04:20:20.231Z
Learnt from: nihal467
Repo: ohcnetwork/care_fe PR: 9301
File: cypress/e2e/facility_spec/FacilityLocation.cy.ts:220-220
Timestamp: 2024-12-05T04:20:20.231Z
Learning: In `FacilityLocation.cy.ts`, ensure not to suggest adding success message verifications if they are already present in the test steps.

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2024-11-14T10:29:07.447Z
Learnt from: Jacobjeevan
Repo: ohcnetwork/care_fe PR: 9080
File: cypress/e2e/users_spec/UsersManage.cy.ts:129-131
Timestamp: 2024-11-14T10:29:07.447Z
Learning: In the Cypress test files (e.g., `cypress/e2e/users_spec/UsersManage.cy.ts`), it is acceptable to use hardcoded English error messages, as testing is conducted in English.

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-01-14T09:22:13.878Z
Learnt from: rajku-dev
Repo: ohcnetwork/care_fe PR: 9887
File: src/components/Users/CreateUserForm.tsx:93-93
Timestamp: 2025-01-14T09:22:13.878Z
Learning: The suggestion to use GENDER_TYPES.map(g => g.id) for gender enum validation in CreateUserForm.tsx was incorrect and caused an error.

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2024-11-15T05:22:27.912Z
Learnt from: AnveshNalimela
Repo: ohcnetwork/care_fe PR: 8930
File: cypress/pageobject/Users/UserCreation.ts:3-5
Timestamp: 2024-11-15T05:22:27.912Z
Learning: In the `clickProfileName()` method of the `UserCreationPage` class, the profile name is dynamic and cannot be verified using a static text. Therefore, `cy.get("#user-profile-name").click();` should be used instead of `cy.verifyAndClickElement`.

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-01-15T11:05:16.663Z
Learnt from: nihal467
Repo: ohcnetwork/care_fe PR: 9986
File: cypress/e2e/users_spec/user_creation.cy.ts:43-55
Timestamp: 2025-01-15T11:05:16.663Z
Learning: UI verification for user creation in the users list should be implemented only after search functionality is available in the UI to avoid flaky tests. Currently, the test focuses on API verification only.

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2024-11-14T10:28:32.680Z
Learnt from: Jacobjeevan
Repo: ohcnetwork/care_fe PR: 9080
File: cypress/pageobject/Users/ManageUserPage.ts:181-188
Timestamp: 2024-11-14T10:28:32.680Z
Learning: In the Cypress tests for user management (`cypress/pageobject/Users/ManageUserPage.ts`), the method `verifyDoctorQualification()` is intended to verify qualifications specifically for the Doctor role.

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-01-19T14:12:10.781Z
Learnt from: AdityaJ2305
Repo: ohcnetwork/care_fe PR: 10054
File: src/components/Users/CreateUserForm.tsx:278-297
Timestamp: 2025-01-19T14:12:10.781Z
Learning: When using Cypress with React forms that have real-time validation, use `should('be.visible')` before typing and add `{ force: true }` option or break down type commands into smaller chunks with intermediate assertions to handle DOM detachment issues.

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2024-11-18T10:46:56.270Z
Learnt from: Jacobjeevan
Repo: ohcnetwork/care_fe PR: 9145
File: cypress/e2e/facility_spec/FacilityHomepage.cy.ts:229-317
Timestamp: 2024-11-18T10:46:56.270Z
Learning: In `cypress/e2e/facility_spec/FacilityHomepage.cy.ts`, when verifying the bed capacity badge reflection, the badge remains the same during multiple actions and only updates after a bed is attached to a patient during consultation. Therefore, it's appropriate to keep the test case consolidated rather than splitting it into smaller tests.

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2024-11-14T08:09:58.453Z
Learnt from: Jacobjeevan
Repo: ohcnetwork/care_fe PR: 9080
File: cypress/pageobject/Users/ManageUserPage.ts:67-97
Timestamp: 2024-11-14T08:09:58.453Z
Learning: In Cypress test files (`cypress/pageobject/Users/ManageUserPage.ts`), when methods are called to check input provided earlier in the test, adding TypeScript interfaces, input validation, and custom error messages is unnecessary.

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:52:19.758Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/pages.instructions.md:0-0
Timestamp: 2025-11-25T13:52:19.758Z
Learning: Applies to src/pages/**/*.{ts,tsx} : Use proper ARIA labels and roles in page components

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:52:19.758Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/pages.instructions.md:0-0
Timestamp: 2025-11-25T13:52:19.758Z
Learning: Applies to src/pages/**/*.{ts,tsx} : Add appropriate `data-cy` attributes for E2E tests to page components

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:51:23.408Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/context.instructions.md:0-0
Timestamp: 2025-11-25T13:51:23.408Z
Learning: Applies to src/context/**/*.{test,spec}.{ts,tsx} : Validate emergency override scenarios in context tests

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2024-11-18T10:48:08.500Z
Learnt from: Jacobjeevan
Repo: ohcnetwork/care_fe PR: 9145
File: cypress/e2e/patient_spec/PatientConsultationCreation.cy.ts:93-94
Timestamp: 2024-11-18T10:48:08.500Z
Learning: In `cypress/e2e/patient_spec/PatientConsultationCreation.cy.ts`, bed capacity verification assertions after patient admission are already being performed elsewhere, so adding them here is unnecessary.

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2024-11-20T23:25:06.602Z
Learnt from: Jacobjeevan
Repo: ohcnetwork/care_fe PR: 9145
File: src/common/validation.tsx:41-41
Timestamp: 2024-11-20T23:25:06.602Z
Learning: When updating the name validation pattern in `src/common/validation.tsx`, avoid partial internationalization by not adding specific non-Latin Unicode ranges unless we're prepared to support all non-Latin scripts.

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:52:33.081Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/providers.instructions.md:0-0
Timestamp: 2025-11-25T13:52:33.081Z
Learning: Applies to src/Providers/**/*.{test,spec}.{ts,tsx} : Validate audit logging and compliance features in provider tests

Applied to files:

  • tests/admin/valueset/valuesetCreate.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: build
  • GitHub Check: Test
  • GitHub Check: CodeQL-Build
🔇 Additional comments (1)
tests/admin/valueset/valuesetCreate.spec.ts (1)

29-212: Verify test coverage against PR objectives.

The PR objectives mention three test cases:

  1. Include rules with concept-only entries ✓ (covered in test 1)
  2. Include rules with filters ✗ (not covered)
  3. Both exclude and include rules together ✗ (test 3 only covers exclude rules)

Consider whether additional tests are needed to cover filter-based rules and the combination of both include and exclude rules in a single ValueSet.

Based on learnings, creation tests may focus on the happy path with detailed verification deferred to list specs. Please confirm if the current coverage is sufficient or if additional test cases should be added in a follow-up.

Comment on lines +37 to +50
switch (system) {
case "SNOMED":
code = faker.helpers.arrayElement(VALID_SNOMED_CODES);
codeName = SNOMED_CODE_NAME[code];
break;
case "LOINC":
code = faker.helpers.arrayElement(VALID_LOINC_CODES);
codeName = LOINC_CODE_NAME[code];
break;
case "UCUM":
code = faker.helpers.arrayElement(VALID_UCUM_CODES);
codeName = UCUM_CODE_NAME[code];
break;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add a default case to handle unexpected system values.

The switch statement lacks a default case. If SYSTEM_OPTIONS contains a system value not handled by the existing cases, code and codeName will remain undefined, leading to confusing test failures.

🔎 Proposed fix
     switch (system) {
       case "SNOMED":
         code = faker.helpers.arrayElement(VALID_SNOMED_CODES);
         codeName = SNOMED_CODE_NAME[code];
         break;
       case "LOINC":
         code = faker.helpers.arrayElement(VALID_LOINC_CODES);
         codeName = LOINC_CODE_NAME[code];
         break;
       case "UCUM":
         code = faker.helpers.arrayElement(VALID_UCUM_CODES);
         codeName = UCUM_CODE_NAME[code];
         break;
+      default:
+        throw new Error(`Unsupported system: ${system}`);
     }
🤖 Prompt for AI Agents
In tests/admin/valueset/valuesetCreate.spec.ts around lines 37 to 50, the switch
on system misses a default branch so code and codeName can remain undefined for
unexpected SYSTEM_OPTIONS values; add a default case that assigns safe fallback
values (e.g., pick from a general valid codes array or set code/codeName to a
known placeholder) or throw a clear error to fail fast, ensuring the test either
recovers with valid data or reports a descriptive failure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add tests for ValueSet Creation

2 participants