135 better editing popup#138
Merged
Merged
Conversation
Closed
|
Warning Review limit reached
More reviews will be available in 12 minutes and 48 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds a method-picker modal and stylesheet; implements a shared schema editor controller (row/text modes) with enum parsing/normalization; wires the picker into grid editors, app generator, and app test-data grid; adds Playwright test abstractions, CSS/markup updates, docs, and many unit/interaction tests. ChangesMethod Picker Modal & Enum Support
Shared Schema Editor Controller & Integration
Browser Test Abstractions
Styling & Markup
Tests & Documentation
🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs:
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements a shared “method picker” modal to replace schema type/command dropdown editing across both the Generator and the embedded App Test Data schema editor, while also improving enum parsing/mapping (notably handling datatype.enum and shorthand enum formats) and aligning docs/help metadata URLs with AnyWayData documentation.
Changes:
- Added a reusable method-picker modal (UI + styles + tests) and integrated it into generator schema editing and app test-data schema editing (including grid editors).
- Introduced a shared schema editor controller to unify row/text mode behavior and sample schema insertion across screens.
- Improved enum parsing/mapping to support shorthand formats and consistent
datatype.enumhandling; updated/added coverage tests accordingly.
Reviewed changes
Copilot reviewed 55 out of 55 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/tests/data_generation/unit/enum/enumTestDataRuleValidator.test.js | Adds unit coverage for shorthand/parentheses/quoted enum parsing cases. |
| packages/core/js/data_generation/utils/enumParser.js | Extends enum parsing to support shorthand enum ..., optional parens, and quoted CSV items. |
| packages/core-ui/src/tests/utils/method-picker-modal.test.js | Adds unit tests for modal filtering, selection, tabs, and detail rendering. |
| packages/core-ui/src/tests/utils/domain-command-help-metadata.test.js | Tests domain docs URL resolution and synthetic datatype.enum help. |
| packages/core-ui/src/tests/shared/schema-row-mapper.test.js | Verifies schema/grid row mapping for enum/domain datatype.enum scenarios. |
| packages/core-ui/src/tests/shared/help-model-builder.test.js | Expands help model expectations (params/examples/docs links) for types and faker. |
| packages/core-ui/src/tests/shared/generation-controller.test.js | Ensures domain datatype.enum rows are treated as enum rules during generator config. |
| packages/core-ui/src/tests/interaction/support/focused-app-test-data-harness.js | Updates interaction harness to the new row-based schema UI and controls. |
| packages/core-ui/src/tests/interaction/matrix/support/schema-interaction-scenario-builder.js | Adds domain param defaults for datatype.enum in interaction scenarios. |
| packages/core-ui/src/tests/interaction/matrix/support/generator-interaction-harness.js | Updates expected faker docs link resolution through new resolver. |
| packages/core-ui/src/tests/interaction/matrix/support/app-test-data-interaction-harness.js | Migrates harness to new schema rows UI and updated syncing behavior. |
| packages/core-ui/src/tests/interaction/matrix/schema-interaction-scenario-builder.test.js | Asserts datatype.enum visibility and metadata availability. |
| packages/core-ui/src/tests/interaction/generator-focused-schema-editing.test.js | Updates expectations around help links and sample insertion behavior. |
| packages/core-ui/src/tests/interaction/generator-focused-generation.test.js | Adds focused generation coverage for datatype.enum without text-mode roundtrip. |
| packages/core-ui/src/tests/interaction/app-test-data-focused-schema-sync.test.js | Updates sync tests to new row DOM and field mappings. |
| packages/core-ui/src/tests/interaction/app-test-data-focused-generation.test.js | Adds generation coverage for datatype.enum in the app test-data panel. |
| packages/core-ui/src/tests/grid/schema/test-data-schema-grid-engine-compat.test.js | Reworks compatibility tests for new row-based schema editor (less engine-specific). |
| packages/core-ui/src/tests/grid/schema/test-data-command-catalog.test.js | Adds method-picker option assertions (help metadata present). |
| packages/core-ui/src/tests/generator/schema-row-rule-mapper.test.js | Extends enum build/extract cases including datatype.enum and shorthand formats. |
| packages/core-ui/src/tests/generator/data-generator-schema-ui.test.js | Updates UI test to expect the picker button instead of a command select. |
| packages/core-ui/src/tests/generator/data-generator-page.test.js | Updates expected faker docs URL to AnyWayData docs. |
| packages/core-ui/js/gui_components/shared/test-data/ui/method-picker-modal.js | New modal implementation (tabs/search/details/recent storage + CSS injection). |
| packages/core-ui/js/gui_components/shared/test-data/ui/method-picker-modal.css | New standalone stylesheet for the modal with light/dark theme variables. |
| packages/core-ui/js/gui_components/shared/test-data/ui/index.js | Exports the new modal API. |
| packages/core-ui/js/gui_components/shared/test-data/schema/shared-schema-editor-controller.js | New shared controller orchestrating row/text modes and picker integration. |
| packages/core-ui/js/gui_components/shared/test-data/schema/schema-row-mapper.js | Ensures enum rows extract enum values correctly when mapping from rules. |
| packages/core-ui/js/gui_components/shared/test-data/schema/index.js | Exposes the shared schema editor controller. |
| packages/core-ui/js/gui_components/shared/test-data/help/help-model-builder.js | Enhances help models (params/examples) and resolves faker helper docs URLs. |
| packages/core-ui/js/gui_components/shared/test-data/generation/generation-controller.js | Treats domain datatype.enum as enum rule type when configuring generator. |
| packages/core-ui/js/gui_components/shared/schema-row-rule-mapper.js | Adds enum ruleSpec builder + enum value extraction helper and datatype.enum mapping. |
| packages/core-ui/js/gui_components/shared/domain-commands.js | Adds datatype.enum as a synthetic domain command for visibility in catalogs. |
| packages/core-ui/js/gui_components/shared/domain-command-help-metadata.js | Adds synthetic help for datatype.enum and normalizes domain docs URLs. |
| packages/core-ui/js/gui_components/generator/schema/data-generator-schema-ui.js | Replaces visible command select with a picker button + hidden select for compatibility. |
| packages/core-ui/js/gui_components/generator/controller/data-generator-page-controller.js | Integrates modal picker into generator row button handling. |
| packages/core-ui/js/gui_components/data-grid-editor/ag-grid/select-filter-editor.js | Replaces list-input editor with a modal-driven picker button for AG Grid. |
| packages/core-ui/js/gui_components/app/test-data-grid/schema/test-data-grid-schema-grid-controller.js | Switches to shared schema editor controller for app test-data schema editing. |
| packages/core-ui/js/gui_components/app/test-data-grid/schema/test-data-command-catalog.js | Adds method-picker options builder including help metadata. |
| packages/core-ui/js/gui_components/app/test-data-grid/schema/index.js | Exports method-picker option builder from schema module. |
| packages/core-ui/js/gui_components/app/test-data-grid/host/test-data-grid-panel-html.js | Updates app panel HTML to new schema editor layout/controls. |
| packages/core-ui/js/gui_components/app/test-data-grid/grid-engines/test-data-grid-tabulator-editor.js | Swaps Tabulator command editor to modal picker-driven button. |
| packages/core-ui/js/gui_components/app/test-data-grid/grid-engines/test-data-grid-engine-setup.js | Plumbs method-picker options into engine setup. |
| packages/core-ui/js/gui_components/app/test-data-grid/grid-engines/test-data-grid-ag-grid-editor.js | Passes picker option provider through to AG Grid cell editor params. |
| packages/core-ui/js/gui_components/app/test-data-grid/controller/test-data-grid-controller.js | Wires shared schema editor flows + picker options into app test-data controller. |
| docs-src/src/pages/contact.mdx | Adds Contact page content. |
| docs-src/src/pages/about.md | Adds About page content. |
| docs-src/docusaurus.config.js | Adds About/Contact to footer links. |
| docs-src/docs/040-test-data/035-method-picker-ui-spec.md | Documents the method picker UI spec, behavior, and test expectations. |
| apps/web/styles.css | Updates generator schema layout and adds styling for picker controls + modal. |
| apps/web/src/tests/browser/shared/abstractions/components/schema-editor.component.js | Adds reusable Playwright schema editor component for row/text + picker flows. |
| apps/web/src/tests/browser/shared/abstractions/components/method-picker-dialog.component.js | Adds Playwright abstraction for interacting with the modal picker dialog. |
| apps/web/src/tests/browser/generator/functional/schema-edit.spec.js | Updates enum value expectations (unwrap enum wrapper in UI). |
| apps/web/src/tests/browser/generator/abstractions/components/generator-schema.component.js | Refactors to reuse shared SchemaEditorComponent abstraction. |
| apps/web/src/tests/browser/app/functional/test-data/text-schema.spec.js | Relaxes row count assertion to accommodate new syncing behavior. |
| apps/web/src/tests/browser/app/functional/test-data/schema-grid-row-controls.spec.js | Updates assertions and row index handling for new schema editor behavior. |
| apps/web/src/tests/browser/app/abstractions/components/test-data-panel.component.js | Refactors app test-data panel Playwright abstraction for the new schema editor + picker. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core-ui/src/tests/interaction/support/focused-app-test-data-harness.js (1)
17-137:⚠️ Potential issue | 🟠 MajorUI test command didn’t complete for this harness refactor
pnpm run test:browserfailed (exit code 1): Timed out waiting 30000ms fromconfig.webServer.pnpm test -- packages/core-ui/src/testspassed (89 suites passed, 1 skipped).pnpm test -- apps/web/src/tests/jestpassed (4 suites passed).🤖 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 `@packages/core-ui/src/tests/interaction/support/focused-app-test-data-harness.js` around lines 17 - 137, The harness never initializes the DOM/control automatically which causes the browser UI test runner to hang; update createFocusedAppTestDataHarness to call reset() once the helper functions are defined (so the DOM, latestDataTable, importer/exporter, textPreviewRenderer, gridExtras and control are set up) before returning the harness object so tests using createFocusedAppTestDataHarness get an initialized state; reference createFocusedAppTestDataHarness, reset, and control.enableTestDataGenerationInterface when making the change.
🧹 Nitpick comments (1)
packages/core-ui/src/tests/interaction/app-test-data-focused-generation.test.js (1)
129-133: ⚡ Quick winAssert generated
Statusvalues, not just absence of errors.This test should also validate the produced values are within
active|inactive|pending; otherwise enum-mapping regressions can slip through whilegetSchemaErrorText()stays empty.🤖 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 `@packages/core-ui/src/tests/interaction/app-test-data-focused-generation.test.js` around lines 129 - 133, After generating, add an assertion that the produced enum values for "Status" are one of the allowed set ("active","inactive","pending") in addition to the existing error check; use the existing test harness methods (after harness.setGenerateCount(3); harness.clickGenerate(); harness.assertSuccessfulGeneration('app datatype.enum generation'); harness.getSchemaErrorText()) then call the harness method that returns generated output (e.g., harness.getGeneratedData() or harness.getGeneratedValues('Status')) and assert each generated Status value is in ["active","inactive","pending"] with expect(...) assertions so enum-mapping regressions fail the test.
🤖 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
`@apps/web/src/tests/browser/app/abstractions/components/test-data-panel.component.js`:
- Around line 46-48: The row count is undercounting because domCount subtracts 1
from editor.rows.count() even though the locator targets only real rows; update
the logic in test-data-panel.component.js to stop subtracting one so domCount is
based on the actual count (use domCount = Math.max(0, await editor.rows.count())
or equivalent), leaving the rest of the comparison with textCount unchanged;
locate the domCount assignment that references editor.rows.count() and remove
the "- 1" adjustment to fix row-editor mode counts.
In
`@apps/web/src/tests/browser/shared/abstractions/components/method-picker-dialog.component.js`:
- Around line 29-31: The code currently masks lookup failures by assigning
target = this.commandLabels.first() when (await target.count()) === 0; instead,
fail fast: when target.count() is 0 throw a descriptive Error (or assertion)
that the requested command tile was not found (include the selector/command name
in the message) rather than falling back to this.commandLabels.first(); apply
the same change to the other occurrence (the similar block referencing target
and this.commandLabels.first()) so tests report missing elements instead of
producing false positives.
In
`@apps/web/src/tests/browser/shared/abstractions/components/schema-editor.component.js`:
- Around line 40-42: The current isRowEditorMode uses this.rows.count() which
counts hidden rows too; update isRowEditorMode to check visibility instead by
iterating visible rows: loop over this.rows (use this.rows.count() to get total)
and return true if any this.rows.nth(i).isVisible() is true, otherwise return
false; keep the function name isRowEditorMode and the this.rows locator
reference so callers remain unchanged.
In `@apps/web/styles.css`:
- Line 921: The Stylelint rule fails because the declarations "width: min(94vw,
980px);" (occurring at the two reported spots) lack the required empty line
before them; fix by inserting a single blank line immediately above each of
these width declarations in apps/web/styles.css so they conform to the
configured declaration-empty-line-before rule (ensure you add the blank line in
both locations where "width: min(94vw, 980px);" appears).
In `@docs-src/src/pages/about.md`:
- Line 8: Fix the user-facing typos and capitalization in the about page: change
"Synthentic" to "Synthetic" (the heading sentence), "github" to "GitHub", and
"richardson" to "Richardson" wherever those exact strings appear so the copy is
properly spelled and capitalized.
In
`@packages/core-ui/js/gui_components/app/test-data-grid/grid-engines/test-data-grid-tabulator-editor.js`:
- Around line 38-52: The click handler on editor that calls
openMethodPickerModal should guard against promise rejection: wrap the await
openMethodPickerModal(...) inside a try/catch within the
editor.addEventListener('click', ...) callback and in the catch set completed =
true and call cancel() to ensure the editor completion/cancel flow still runs on
errors; only call finishWithValue(selected.command) when the call succeeds and
selected?.command exists, and optionally log or surface the error inside the
catch for debugging.
In
`@packages/core-ui/js/gui_components/data-grid-editor/ag-grid/select-filter-editor.js`:
- Around line 26-50: The click handler attached in this.input.addEventListener
must guard against a rejected openMethodPickerModal; wrap the await
openMethodPickerModal(...) call in a try/catch, and in the catch set
this.completed = true and call this.params?.stopEditing?.(true) (or the safe
cancel path) to ensure the editor is closed when the modal errors; keep the
existing success behavior otherwise and reference openMethodPickerModal,
this.completed and this.params.stopEditing in the fix.
In
`@packages/core-ui/js/gui_components/generator/controller/data-generator-page-controller.js`:
- Around line 465-514: In handleRowButtonClick: wrap the await
openMethodPickerModal call in try/catch to handle modal rejects, and after the
await re-resolve the target row/index (recompute index =
this.schemaRows.findIndex(...)) and verify index >= 0 before calling
this.schemaSession.updateRowAtIndex and this.renderSchemaRows; if the row no
longer exists, bail gracefully. Use the same row id retrieval (from
pickerButton.getAttribute('data-row-id')) to re-find the row, and preserve the
existing logic to update sourceType/command only when selected?.command is
present.
In
`@packages/core-ui/js/gui_components/shared/test-data/schema/shared-schema-editor-controller.js`:
- Around line 278-297: Wrap the await of openMethodPickerModal in a try/catch to
prevent unhandled promise rejections; inside try keep the existing logic that
checks selected?.command and calls session.updateRowAtIndex, renderRows, and
syncTextFromRows, and in catch log or surface the error (e.g., using
console.error or the repo logger) and ensure UI cleanup by calling renderRows()
and syncTextFromRows() so state stays consistent after a failed picker;
reference openMethodPickerModal, session.updateRowAtIndex, renderRows,
syncTextFromRows, normaliseDomainCommand, and normaliseFakerCommand when making
the change.
In
`@packages/core-ui/js/gui_components/shared/test-data/ui/method-picker-modal.css`:
- Line 163: Replace all deprecated occurrences of the CSS declaration
"word-break: break-word" with the modern equivalent "overflow-wrap: anywhere;"
(and remove the deprecated property) across the file; search for the exact token
"word-break: break-word" and update each rule block (the four occurrences
flagged) to use "overflow-wrap: anywhere;" and, if needed for behavior parity,
keep or add "word-break: normal;" alongside it, then re-run the provided ripgrep
command to confirm no deprecated usages remain.
In
`@packages/core-ui/src/tests/grid/schema/test-data-schema-grid-engine-compat.test.js`:
- Line 83: Replace the hard-coded await new Promise((resolve) =>
setTimeout(resolve, 1100)) with a condition-based wait: remove the setTimeout
sleep and instead use a testing-library wait helper (e.g., waitFor or findBy* /
waitForElementToBeRemoved) to wait until the UI state your test needs (for
example the "Pairwise Generate" button becomes enabled/visible, or a specific
loading indicator disappears) before performing the click on the pairwise
generate control; alternatively, switch the test to use jest.useFakeTimers() and
advanceTimersByTime(...) combined with flushing effects if you prefer
deterministic time control—locate the sleep in the test around the pairwise
generate click and replace it with one of these approaches.
In
`@packages/core-ui/src/tests/interaction/matrix/support/app-test-data-interaction-harness.js`:
- Around line 112-114: The unbounded while loop waiting for '`#testDataSchemaRows`
.generator-schema-row' to reach a given index can hang CI; modify the loop in
app-test-data-interaction-harness.js to add a retry guard (e.g., maxRetries or
timeout) around the
clickElement(document.getElementById('testDataAddSchemaRowButton')) calls,
incrementing a counter each iteration and breaking with a clear failure/throw if
the limit is exceeded so the test fails fast instead of spinning forever.
---
Outside diff comments:
In
`@packages/core-ui/src/tests/interaction/support/focused-app-test-data-harness.js`:
- Around line 17-137: The harness never initializes the DOM/control
automatically which causes the browser UI test runner to hang; update
createFocusedAppTestDataHarness to call reset() once the helper functions are
defined (so the DOM, latestDataTable, importer/exporter, textPreviewRenderer,
gridExtras and control are set up) before returning the harness object so tests
using createFocusedAppTestDataHarness get an initialized state; reference
createFocusedAppTestDataHarness, reset, and
control.enableTestDataGenerationInterface when making the change.
---
Nitpick comments:
In
`@packages/core-ui/src/tests/interaction/app-test-data-focused-generation.test.js`:
- Around line 129-133: After generating, add an assertion that the produced enum
values for "Status" are one of the allowed set ("active","inactive","pending")
in addition to the existing error check; use the existing test harness methods
(after harness.setGenerateCount(3); harness.clickGenerate();
harness.assertSuccessfulGeneration('app datatype.enum generation');
harness.getSchemaErrorText()) then call the harness method that returns
generated output (e.g., harness.getGeneratedData() or
harness.getGeneratedValues('Status')) and assert each generated Status value is
in ["active","inactive","pending"] with expect(...) assertions so enum-mapping
regressions fail the test.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 316cca5d-bca2-4400-ae19-8554869bc134
📒 Files selected for processing (55)
apps/web/src/tests/browser/app/abstractions/components/test-data-panel.component.jsapps/web/src/tests/browser/app/functional/test-data/schema-grid-row-controls.spec.jsapps/web/src/tests/browser/app/functional/test-data/text-schema.spec.jsapps/web/src/tests/browser/generator/abstractions/components/generator-schema.component.jsapps/web/src/tests/browser/generator/functional/schema-edit.spec.jsapps/web/src/tests/browser/shared/abstractions/components/method-picker-dialog.component.jsapps/web/src/tests/browser/shared/abstractions/components/schema-editor.component.jsapps/web/styles.cssdocs-src/docs/040-test-data/035-method-picker-ui-spec.mddocs-src/docusaurus.config.jsdocs-src/src/pages/about.mddocs-src/src/pages/contact.mdxpackages/core-ui/js/gui_components/app/test-data-grid/controller/test-data-grid-controller.jspackages/core-ui/js/gui_components/app/test-data-grid/grid-engines/test-data-grid-ag-grid-editor.jspackages/core-ui/js/gui_components/app/test-data-grid/grid-engines/test-data-grid-engine-setup.jspackages/core-ui/js/gui_components/app/test-data-grid/grid-engines/test-data-grid-tabulator-editor.jspackages/core-ui/js/gui_components/app/test-data-grid/host/test-data-grid-panel-html.jspackages/core-ui/js/gui_components/app/test-data-grid/schema/index.jspackages/core-ui/js/gui_components/app/test-data-grid/schema/test-data-command-catalog.jspackages/core-ui/js/gui_components/app/test-data-grid/schema/test-data-grid-schema-grid-controller.jspackages/core-ui/js/gui_components/data-grid-editor/ag-grid/select-filter-editor.jspackages/core-ui/js/gui_components/generator/controller/data-generator-page-controller.jspackages/core-ui/js/gui_components/generator/schema/data-generator-schema-ui.jspackages/core-ui/js/gui_components/shared/domain-command-help-metadata.jspackages/core-ui/js/gui_components/shared/domain-commands.jspackages/core-ui/js/gui_components/shared/schema-row-rule-mapper.jspackages/core-ui/js/gui_components/shared/test-data/generation/generation-controller.jspackages/core-ui/js/gui_components/shared/test-data/help/help-model-builder.jspackages/core-ui/js/gui_components/shared/test-data/schema/index.jspackages/core-ui/js/gui_components/shared/test-data/schema/schema-row-mapper.jspackages/core-ui/js/gui_components/shared/test-data/schema/shared-schema-editor-controller.jspackages/core-ui/js/gui_components/shared/test-data/ui/index.jspackages/core-ui/js/gui_components/shared/test-data/ui/method-picker-modal.csspackages/core-ui/js/gui_components/shared/test-data/ui/method-picker-modal.jspackages/core-ui/src/tests/generator/data-generator-page.test.jspackages/core-ui/src/tests/generator/data-generator-schema-ui.test.jspackages/core-ui/src/tests/generator/schema-row-rule-mapper.test.jspackages/core-ui/src/tests/grid/schema/test-data-command-catalog.test.jspackages/core-ui/src/tests/grid/schema/test-data-schema-grid-engine-compat.test.jspackages/core-ui/src/tests/interaction/app-test-data-focused-generation.test.jspackages/core-ui/src/tests/interaction/app-test-data-focused-schema-sync.test.jspackages/core-ui/src/tests/interaction/generator-focused-generation.test.jspackages/core-ui/src/tests/interaction/generator-focused-schema-editing.test.jspackages/core-ui/src/tests/interaction/matrix/schema-interaction-scenario-builder.test.jspackages/core-ui/src/tests/interaction/matrix/support/app-test-data-interaction-harness.jspackages/core-ui/src/tests/interaction/matrix/support/generator-interaction-harness.jspackages/core-ui/src/tests/interaction/matrix/support/schema-interaction-scenario-builder.jspackages/core-ui/src/tests/interaction/support/focused-app-test-data-harness.jspackages/core-ui/src/tests/shared/generation-controller.test.jspackages/core-ui/src/tests/shared/help-model-builder.test.jspackages/core-ui/src/tests/shared/schema-row-mapper.test.jspackages/core-ui/src/tests/utils/domain-command-help-metadata.test.jspackages/core-ui/src/tests/utils/method-picker-modal.test.jspackages/core/js/data_generation/utils/enumParser.jspackages/core/src/tests/data_generation/unit/enum/enumTestDataRuleValidator.test.js
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/tests/browser/shared/abstractions/components/method-picker-dialog.component.js (1)
3-37:⚠️ Potential issue | 🟠 MajorRun required UI test suites for this test abstraction (browser suite isn’t completing)
pnpm run test:browserexited with code 1: timed out waiting 30000ms forconfig.webServer.- The guideline also requires
pnpm testto complete; rerun once the browser/webServer startup issue is resolved.🤖 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 `@apps/web/src/tests/browser/shared/abstractions/components/method-picker-dialog.component.js` around lines 3 - 37, The browser test run is failing because Playwright's config.webServer isn't starting (timeout waiting 30000ms); ensure the web server for UI tests is started before running the MethodPickerDialogComponent browser tests: update your Playwright/web test setup to provide a valid webServer.command or webServer.url (and increase webServer.timeout if slow), or start the dev server (pnpm start) in a background step before running pnpm run test:browser; then re-run pnpm run test:browser and full pnpm test to verify MethodPickerDialogComponent.chooseCommand and expectOpen tests complete.
🧹 Nitpick comments (1)
apps/web/src/tests/browser/shared/abstractions/components/method-picker-dialog.component.js (1)
1-1:@playwright/test1.59.1 is valid, but not the latest (latest: 1.60.0); npm audit reports no vulnerabilities.🤖 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 `@apps/web/src/tests/browser/shared/abstractions/components/method-picker-dialog.component.js` at line 1, The test imports '`@playwright/test`' which is pinned to 1.59.1; update the Playwright dependency to the latest 1.60.0 (or a caret range like ^1.60.0) in package.json's devDependencies, run npm install to update package-lock.json/yarn.lock, and re-run tests/CI to ensure compatibility; if any API breaks occur, adjust usages in tests (e.g., files like apps/web/src/tests/browser/shared/abstractions/components/method-picker-dialog.component.js) to match the 1.60.0 API.
🤖 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.
Outside diff comments:
In
`@apps/web/src/tests/browser/shared/abstractions/components/method-picker-dialog.component.js`:
- Around line 3-37: The browser test run is failing because Playwright's
config.webServer isn't starting (timeout waiting 30000ms); ensure the web server
for UI tests is started before running the MethodPickerDialogComponent browser
tests: update your Playwright/web test setup to provide a valid
webServer.command or webServer.url (and increase webServer.timeout if slow), or
start the dev server (pnpm start) in a background step before running pnpm run
test:browser; then re-run pnpm run test:browser and full pnpm test to verify
MethodPickerDialogComponent.chooseCommand and expectOpen tests complete.
---
Nitpick comments:
In
`@apps/web/src/tests/browser/shared/abstractions/components/method-picker-dialog.component.js`:
- Line 1: The test imports '`@playwright/test`' which is pinned to 1.59.1; update
the Playwright dependency to the latest 1.60.0 (or a caret range like ^1.60.0)
in package.json's devDependencies, run npm install to update
package-lock.json/yarn.lock, and re-run tests/CI to ensure compatibility; if any
API breaks occur, adjust usages in tests (e.g., files like
apps/web/src/tests/browser/shared/abstractions/components/method-picker-dialog.component.js)
to match the 1.60.0 API.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 30cf43c5-9962-4b7c-9b66-a683dc4e23d2
📒 Files selected for processing (15)
apps/web/src/tests/browser/app/abstractions/components/test-data-panel.component.jsapps/web/src/tests/browser/shared/abstractions/components/method-picker-dialog.component.jsapps/web/src/tests/browser/shared/abstractions/components/schema-editor.component.jsapps/web/styles.cssdocs-src/src/pages/about.mdpackages/core-ui/js/gui_components/app/test-data-grid/grid-engines/test-data-grid-tabulator-editor.jspackages/core-ui/js/gui_components/data-grid-editor/ag-grid/select-filter-editor.jspackages/core-ui/js/gui_components/generator/controller/data-generator-page-controller.jspackages/core-ui/js/gui_components/shared/test-data/schema/shared-schema-editor-controller.jspackages/core-ui/js/gui_components/shared/test-data/ui/method-picker-modal.csspackages/core-ui/src/tests/generator/schema-row-rule-mapper.test.jspackages/core-ui/src/tests/grid/schema/test-data-schema-grid-engine-compat.test.jspackages/core-ui/src/tests/interaction/app-test-data-focused-generation.test.jspackages/core-ui/src/tests/interaction/matrix/support/app-test-data-interaction-harness.jspackages/core/src/tests/data_generation/unit/enum/enumTestDataRuleValidator.test.js
💤 Files with no reviewable changes (1)
- apps/web/styles.css
🚧 Files skipped from review as they are similar to previous changes (13)
- docs-src/src/pages/about.md
- packages/core-ui/src/tests/interaction/app-test-data-focused-generation.test.js
- packages/core-ui/src/tests/generator/schema-row-rule-mapper.test.js
- packages/core/src/tests/data_generation/unit/enum/enumTestDataRuleValidator.test.js
- packages/core-ui/src/tests/grid/schema/test-data-schema-grid-engine-compat.test.js
- packages/core-ui/js/gui_components/data-grid-editor/ag-grid/select-filter-editor.js
- packages/core-ui/js/gui_components/app/test-data-grid/grid-engines/test-data-grid-tabulator-editor.js
- packages/core-ui/js/gui_components/shared/test-data/schema/shared-schema-editor-controller.js
- packages/core-ui/js/gui_components/shared/test-data/ui/method-picker-modal.css
- packages/core-ui/src/tests/interaction/matrix/support/app-test-data-interaction-harness.js
- apps/web/src/tests/browser/shared/abstractions/components/schema-editor.component.js
- apps/web/src/tests/browser/app/abstractions/components/test-data-panel.component.js
- packages/core-ui/js/gui_components/generator/controller/data-generator-page-controller.js
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/tests/browser/app/abstractions/components/test-data-panel.component.js (1)
145-205:⚠️ Potential issue | 🟠 MajorRe-run the browser + UI Jest suites for this abstraction change (webServer timeout prevented coverage).
pnpm run test:browserfailed withTimed out waiting 30000ms from config.webServer, so the required browser coverage didn’t complete.- Re-run
pnpm run test:browserafter the webServer issue is addressed, then runpnpm test(or targeted UI Jest suites) coveringpackages/core-ui/src/testsandapps/web/src/tests/jest.🤖 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 `@apps/web/src/tests/browser/app/abstractions/components/test-data-panel.component.js` around lines 145 - 205, The browser/UI test run failed due to the webServer timeout; after fixing the webServer issue, re-run the browser suite and Jest UI tests by executing `pnpm run test:browser` to regain browser coverage and then run `pnpm test` (or the targeted UI Jest suites covering packages/core-ui/src/tests and apps/web/src/tests/jest`) to validate changes in components like the abstractions in test-data-panel.component.js (notably methods getSchemaCell, setSchemaCell, selectSchemaRow); ensure the webServer is healthy so config.webServer no longer times out and re-run until the browser coverage completes successfully.
🤖 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
`@apps/web/src/tests/browser/app/abstractions/components/test-data-panel.component.js`:
- Around line 198-205: When mapped === 'value' the current branch returns '' if
no params input exists, which breaks legacy grid reads; update the branch in the
function that handles getSchemaCell (the mapped === 'value' block) to first try
the params input (paramsInput from
schemaEditor.row(rowIndex).locator('[data-field="params"]')) as it does now, but
if that locator.count() === 0 then fall back to reading the actual value cell
using the existing schemaRenderer.getCellTextByField('value', rowIndex) (or by
locating '[data-field="value"]' on schemaEditor.row(rowIndex)) instead of
returning an empty string; preserve the current behavior of returning '' only if
neither params input nor a value cell is present.
---
Outside diff comments:
In
`@apps/web/src/tests/browser/app/abstractions/components/test-data-panel.component.js`:
- Around line 145-205: The browser/UI test run failed due to the webServer
timeout; after fixing the webServer issue, re-run the browser suite and Jest UI
tests by executing `pnpm run test:browser` to regain browser coverage and then
run `pnpm test` (or the targeted UI Jest suites covering
packages/core-ui/src/tests and apps/web/src/tests/jest`) to validate changes in
components like the abstractions in test-data-panel.component.js (notably
methods getSchemaCell, setSchemaCell, selectSchemaRow); ensure the webServer is
healthy so config.webServer no longer times out and re-run until the browser
coverage completes successfully.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b4f74cb4-0f43-40a3-840e-d4d41becb8cb
📒 Files selected for processing (1)
apps/web/src/tests/browser/app/abstractions/components/test-data-panel.component.js
This was referenced May 28, 2026
This was referenced Jun 17, 2026
This was referenced Jun 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #135
also closes #136
Summary by CodeRabbit
New Features
Documentation
Bug Fixes & Improvements