From 7a08eda11c4d0b0bda42e76196b9998c6bbd880a Mon Sep 17 00:00:00 2001 From: Aniket Katkar Date: Thu, 16 Jul 2026 17:47:24 +0530 Subject: [PATCH 1/6] fix(ui): resolve metadata agent form crash and manifest JSON editor UX The add/edit metadata agent form crashed with "Unsupported widget definition: object" for Storage (S3) services. IngestionWorkflowForm registered its RJSF widgets as raw React.lazy objects; @rjsf/utils getWidget accepts only function/forwardRef/memo, so resolving the manifestJson widget threw. Wrap the lazy widgets in the existing withSuspenseFallback (forwardRef) so getWidget accepts them while keeping code-splitting. Also fixes the Default Manifest JSON editor: - Full width: isWide() now treats uiFieldType 'code' fields as wide. - No caret jump: SchemaEditor gains an autoFormat prop (default true); the manifest widget disables it so the buffer is not re-indented on every keystroke. - No stale default: the sample is shown as a CodeMirror placeholder (muted --tw-color-text-placeholder, multi-line, editor flex-fills its wrapper) instead of being written as the field value. Adds a Playwright spec (render + edit/save round-trip, width, clears cleanly, caret stability) and unit tests for getSchemaEditorValue; updates ManifestJsonWidget tests to the placeholder contract. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Features/StorageMetadataAgentForm.spec.ts | 214 ++++++++++++++++++ .../SchemaEditor/SchemaEditor.interface.ts | 1 + .../Database/SchemaEditor/SchemaEditor.tsx | 12 +- .../IngestionObjectFieldTemplate.tsx | 4 + .../IngestionWorkflowForm.tsx | 35 +-- .../ManifestJsonWidget.test.tsx | 33 ++- .../ManifestJsonWidget/ManifestJsonWidget.tsx | 24 +- .../manifest-json-widget.less | 28 ++- .../ui/src/utils/SchemaEditor.utils.test.ts | 63 ++++++ 9 files changed, 375 insertions(+), 39 deletions(-) create mode 100644 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/StorageMetadataAgentForm.spec.ts create mode 100644 openmetadata-ui/src/main/resources/ui/src/utils/SchemaEditor.utils.test.ts diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/StorageMetadataAgentForm.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/StorageMetadataAgentForm.spec.ts new file mode 100644 index 000000000000..48a3cef2e6cb --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/StorageMetadataAgentForm.spec.ts @@ -0,0 +1,214 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { expect, Page, test } from '@playwright/test'; +import { PLAYWRIGHT_INGESTION_TAG_OBJ } from '../../constant/config'; +import { StorageServiceClass } from '../../support/entity/service/StorageServiceClass'; +import { createNewPage, redirectToHomePage, uuid } from '../../utils/common'; + +// use the admin user to login +test.use({ storageState: 'playwright/.auth/admin.json' }); + +const service = new StorageServiceClass(); +const manifestTag = `pw-manifest-${uuid()}`; +const initialBucket = `${manifestTag}-initial`; +const updatedBucket = `${manifestTag}-updated`; +const initialManifest = `{"entries":[{"containerName":"${initialBucket}","dataPath":"data/*.parquet","structureFormat":"parquet"}]}`; +const updatedManifest = `{"entries":[{"containerName":"${updatedBucket}","dataPath":"logs/*.json","structureFormat":"json"}]}`; + +let pipelineName = ''; + +const openMetadataAgentEditForm = async (page: Page) => { + await redirectToHomePage(page); + await service.visitEntityPage(page); + await page.getByTestId('data-assets-header').waitFor(); + await page.getByTestId('agents').click(); + + const metadataSubTab = page.getByTestId('metadata-sub-tab'); + if (await metadataSubTab.isVisible()) { + await metadataSubTab.click(); + } + + await page.getByTestId('more-actions').first().click(); + await page.getByTestId('edit-button').click(); + + // The add/edit ingestion form renders here. On the buggy build this throws + // "Unsupported widget definition: object" from RJSF getWidget because the + // manifest widget was registered as a raw React.lazy object. + await page.getByTestId('add-ingestion-container').waitFor(); +}; + +test.describe( + 'Storage metadata agent manifest widget', + PLAYWRIGHT_INGESTION_TAG_OBJ, + () => { + test.beforeAll(async ({ browser }) => { + const { apiContext, afterAction } = await createNewPage(browser); + + await service.create(apiContext); + + pipelineName = `pw-storage-metadata-${uuid()}`; + const pipelineResponse = await apiContext.post( + '/api/v1/services/ingestionPipelines', + { + data: { + airflowConfig: { scheduleInterval: '0 0 * * *' }, + loggerLevel: 'INFO', + name: pipelineName, + pipelineType: 'metadata', + service: { + id: service.entityResponseData.id, + type: 'storageService', + }, + sourceConfig: { + config: { + type: 'StorageMetadata', + defaultManifest: initialManifest, + }, + }, + }, + } + ); + + expect(pipelineResponse.status()).toBe(201); + + await afterAction(); + }); + + test.afterAll(async ({ browser }) => { + const { apiContext, afterAction } = await createNewPage(browser); + + await service.delete(apiContext); + await afterAction(); + }); + + test('edit form renders and manifest edits are saved', async ({ page }) => { + test.slow(); + + await openMetadataAgentEditForm(page); + + const manifestWidget = page.locator('.manifest-json-widget'); + + await test.step('Manifest widget renders without crashing', async () => { + await expect(manifestWidget).toBeVisible(); + }); + + await test.step('Saved manifest is loaded into the widget', async () => { + await expect(manifestWidget).toContainText(initialBucket); + }); + + await test.step('Manifest value can be edited in the widget', async () => { + const editor = manifestWidget.locator('.CodeMirror'); + await editor.click(); + await page.keyboard.press('ControlOrMeta+A'); + await page.keyboard.press('Delete'); + // insertText bypasses keydown so CodeMirror autoCloseBrackets does not + // duplicate the JSON braces/quotes we type. + await page.keyboard.insertText(updatedManifest); + + await expect(manifestWidget).toContainText(updatedBucket); + }); + + await test.step('Edited manifest is persisted on save', async () => { + await page.getByTestId('next-button').click(); + + const updateResponse = page.waitForResponse( + (response) => + response.request().method() === 'PATCH' && + response.url().includes('/services/ingestionPipelines/') && + response.status() === 200 + ); + + await page.getByTestId('next-button').click(); + + const updatedPipeline = await (await updateResponse).json(); + + expect(updatedPipeline.sourceConfig.config.defaultManifest).toContain( + updatedBucket + ); + }); + + await test.step('Reopening the agent shows the persisted manifest', async () => { + await openMetadataAgentEditForm(page); + + await expect(page.locator('.manifest-json-widget')).toContainText( + updatedBucket + ); + }); + }); + + test('manifest editor is full width, clears cleanly and keeps caret stable', async ({ + page, + }) => { + test.slow(); + + await openMetadataAgentEditForm(page); + + const manifestWidget = page.locator('.manifest-json-widget'); + await expect(manifestWidget).toBeVisible(); + + await test.step('Editor spans the full form width', async () => { + const widgetBox = await manifestWidget.boundingBox(); + const containerBox = await page + .getByTestId('add-ingestion-container') + .boundingBox(); + + // A half-width grid cell sits near ~0.5; a full-row field is well above. + expect(widgetBox && containerBox).toBeTruthy(); + expect(widgetBox!.width / containerBox!.width).toBeGreaterThan(0.7); + }); + + const editor = manifestWidget.locator('.CodeMirror'); + + await test.step('Clearing the editor leaves it empty, not the sample', async () => { + await editor.click(); + await page.keyboard.press('ControlOrMeta+A'); + await page.keyboard.press('Delete'); + + // CodeMirror inserts the placeholder element only while the document is + // empty; its presence proves the value cleared instead of reverting to + // the sample (the stale-default bug). + const placeholder = manifestWidget.locator('.CodeMirror-placeholder'); + await expect(placeholder).toBeAttached(); + + // It must read as a hint: muted global placeholder colour, and + // white-space preserved so the multi-line sample keeps its line breaks. + const style = await placeholder.evaluate((el) => { + const computed = getComputedStyle(el); + + return { color: computed.color, whiteSpace: computed.whiteSpace }; + }); + expect(style.whiteSpace).toBe('pre-wrap'); + expect(style.color).toBe('rgb(113, 118, 128)'); + + // The full multi-line sample is present (not truncated to one line). + const placeholderText = await placeholder.textContent(); + expect(placeholderText).toContain('containerName'); + expect(placeholderText?.split('\n').length).toBeGreaterThan(5); + + // The editor keeps its full height when empty so the sample is visible + // rather than collapsing to ~1 line. + const editorBox = await editor.boundingBox(); + expect(editorBox!.height).toBeGreaterThan(150); + }); + + await test.step('Typing valid JSON is not live-reformatted', async () => { + // insertText bypasses keydown so autoCloseBrackets does not interfere. + await page.keyboard.insertText('{"x":1,"y":2}'); + + // With live auto-format the buffer would re-indent to 4 lines on each + // keystroke (the cause of the caret jump); disabled, it stays one line. + await expect(manifestWidget.locator('.CodeMirror-line')).toHaveCount(1); + }); + }); + } +); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaEditor/SchemaEditor.interface.ts b/openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaEditor/SchemaEditor.interface.ts index 0d2fbb28d69f..61561fbdd7c4 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaEditor/SchemaEditor.interface.ts +++ b/openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaEditor/SchemaEditor.interface.ts @@ -21,6 +21,7 @@ export type Mode = { export interface SchemaEditorProps { value?: string; + autoFormat?: boolean; refreshEditor?: boolean; className?: string; mode?: Mode; diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaEditor/SchemaEditor.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaEditor/SchemaEditor.tsx index 9d1b3c4e8a1f..b27d93c53d86 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaEditor/SchemaEditor.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaEditor/SchemaEditor.tsx @@ -15,6 +15,7 @@ import Icon from '@ant-design/icons'; import { Button, Tooltip } from 'antd'; import classNames from 'classnames'; import { Editor, EditorChange } from 'codemirror'; +import 'codemirror/addon/display/placeholder'; import 'codemirror/addon/edit/closebrackets.js'; import 'codemirror/addon/edit/matchbrackets.js'; import 'codemirror/addon/fold/brace-fold'; @@ -40,6 +41,7 @@ import { SchemaEditorProps } from './SchemaEditor.interface'; const SchemaEditor = ({ value = '', + autoFormat = true, className = '', mode = { name: CSMode.JAVASCRIPT, @@ -71,7 +73,7 @@ const SchemaEditor = ({ ...options, }; const [internalValue, setInternalValue] = useState( - getSchemaEditorValue(value) + getSchemaEditorValue(value, autoFormat) ); const editorInstance = useRef(null); const wasHiddenRef = useRef(false); @@ -82,7 +84,7 @@ const SchemaEditor = ({ _data: EditorChange, value: string ): void => { - setInternalValue(getSchemaEditorValue(value)); + setInternalValue(getSchemaEditorValue(value, autoFormat)); }; const handleEditorInputChange = ( _editor: Editor, @@ -90,7 +92,7 @@ const SchemaEditor = ({ value: string ): void => { if (!isUndefined(onChange)) { - onChange(getSchemaEditorValue(value)); + onChange(getSchemaEditorValue(value, autoFormat)); } }; @@ -118,8 +120,8 @@ const SchemaEditor = ({ }, [editorInstance, wrapperRef]); useEffect(() => { - setInternalValue(getSchemaEditorValue(value)); - }, [value]); + setInternalValue(getSchemaEditorValue(value, autoFormat)); + }, [value, autoFormat]); // Auto-detect display:none → visible transitions (e.g. Ant Design tab switches). // When a parent sets display:none, boundingClientRect collapses to 0. diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Settings/Services/AddIngestion/IngestionObjectFieldTemplate/IngestionObjectFieldTemplate.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Settings/Services/AddIngestion/IngestionObjectFieldTemplate/IngestionObjectFieldTemplate.tsx index dc3d0b6a76fd..3650a5d4a5e2 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Settings/Services/AddIngestion/IngestionObjectFieldTemplate/IngestionObjectFieldTemplate.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/Settings/Services/AddIngestion/IngestionObjectFieldTemplate/IngestionObjectFieldTemplate.tsx @@ -44,6 +44,7 @@ interface SchemaProperty { anyOf?: unknown[]; oneOf?: unknown[]; type?: string | string[]; + uiFieldType?: string; } interface IngestionSectionConfig { @@ -72,6 +73,9 @@ const isWide = ( if (!prop) { return false; } + if (prop.uiFieldType === 'code') { + return true; + } if (prop.type === 'object' || prop.type === 'array') { return true; } diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Settings/Services/Ingestion/IngestionWorkflowForm/IngestionWorkflowForm.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Settings/Services/Ingestion/IngestionWorkflowForm/IngestionWorkflowForm.tsx index 564262307742..c7b3fa34fa86 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Settings/Services/Ingestion/IngestionWorkflowForm/IngestionWorkflowForm.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/Settings/Services/Ingestion/IngestionWorkflowForm/IngestionWorkflowForm.tsx @@ -43,6 +43,7 @@ import { import ProfilerConfigurationClassBase from '../../../../../pages/ProfilerConfigurationPage/ProfilerConfigurationClassBase'; import { transformErrors } from '../../../../../utils/formPureUtils'; import { getSchemaByWorkflowType } from '../../../../../utils/IngestionWorkflowUtils'; +import { withSuspenseFallback } from '../../../../AppRouter/withSuspenseFallback'; import CoreInputWidget from '../../../../common/FormBuilderV1/widgets/CoreInputWidget'; import CoreSelectWidget from '../../../../common/FormBuilderV1/widgets/CoreSelectWidget'; import Loader from '../../../../common/Loader/Loader'; @@ -249,6 +250,26 @@ const IngestionWorkflowForm = forwardRef< return fields; }, [pipeLineType]); + // RJSF getWidget only accepts function/forwardRef/memo widgets; React.lazy + // widgets are objects and throw "Unsupported widget definition". Wrap each in a + // forwardRef Suspense boundary so they resolve while keeping code-splitting. + const widgets = useMemo( + () => ({ + CheckboxWidget: withSuspenseFallback(CoreCheckboxWidget), + EmailWidget: CoreInputWidget, + PasswordWidget: withSuspenseFallback(CorePasswordWidget), + RadioWidget: withSuspenseFallback(CoreRadioWidget), + SelectWidget: CoreSelectWidget, + TextWidget: CoreInputWidget, + TextareaWidget: withSuspenseFallback(CoreTextAreaWidget), + URLWidget: CoreInputWidget, + UpDownWidget: CoreInputWidget, + code: withSuspenseFallback(CodeWidget), + manifestJson: withSuspenseFallback(ManifestJsonWidget), + }), + [] + ); + // Exposes submit to the parent card footer, which triggers the form when hideFooter is true. useImperativeHandle( ref, @@ -309,19 +330,7 @@ const IngestionWorkflowForm = forwardRef< transformErrors={transformErrors} uiSchema={uiSchema} validator={validator} - widgets={{ - CheckboxWidget: CoreCheckboxWidget, - EmailWidget: CoreInputWidget, - PasswordWidget: CorePasswordWidget, - RadioWidget: CoreRadioWidget, - SelectWidget: CoreSelectWidget, - TextWidget: CoreInputWidget, - TextareaWidget: CoreTextAreaWidget, - URLWidget: CoreInputWidget, - UpDownWidget: CoreInputWidget, - code: CodeWidget, - manifestJson: ManifestJsonWidget, - }} + widgets={widgets} onChange={handleOnChange} onFocus={onFocus} onSubmit={handleSubmit}> diff --git a/openmetadata-ui/src/main/resources/ui/src/components/common/Form/JSONSchema/JsonSchemaWidgets/ManifestJsonWidget/ManifestJsonWidget.test.tsx b/openmetadata-ui/src/main/resources/ui/src/components/common/Form/JSONSchema/JsonSchemaWidgets/ManifestJsonWidget/ManifestJsonWidget.test.tsx index 45a70c711a73..36b304843e1d 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/common/Form/JSONSchema/JsonSchemaWidgets/ManifestJsonWidget/ManifestJsonWidget.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/common/Form/JSONSchema/JsonSchemaWidgets/ManifestJsonWidget/ManifestJsonWidget.test.tsx @@ -30,14 +30,20 @@ const mockSchemaEditor = jest value, mode, readOnly, + autoFormat, + options, }: { value: string; mode?: { name?: string; json?: boolean }; readOnly?: boolean; + autoFormat?: boolean; + options?: { placeholder?: string }; }) => (
{value || ''} @@ -360,19 +366,24 @@ describe('ManifestJsonWidget', () => { jest.clearAllMocks(); }); - it('displays the sample JSON as a placeholder when value is empty (without writing it to form state)', async () => { + it('shows the sample only as a placeholder when value is empty, keeping the editor value empty', async () => { const onChange = jest.fn(); render(); - expect(await screen.findByTestId('schema-editor')).toBeInTheDocument(); - // Sample is displayed in the editor... - expect(screen.getByTestId('schema-editor')).toHaveTextContent(/"entries"/); - // ...but we do NOT call onChange on mount — the field may be - // populated asynchronously from the saved pipeline config. + const editor = await screen.findByTestId('schema-editor'); + + // The editor value stays empty — the sample must not leak into form state... + expect(editor).toHaveTextContent(''); + // ...it is passed only as a placeholder, and typing is not auto-formatted. + expect(editor).toHaveAttribute( + 'data-placeholder', + expect.stringContaining('"entries"') + ); + expect(editor).toHaveAttribute('data-autoformat', 'false'); expect(onChange).not.toHaveBeenCalled(); }); - it('displays the sample when value is null without mutating form state', () => { + it('keeps the editor empty with a placeholder when value is null', () => { const onChange = jest.fn(); render( { /> ); - expect(screen.getByTestId('schema-editor')).toHaveTextContent(/"entries"/); + const editor = screen.getByTestId('schema-editor'); + + expect(editor).toHaveTextContent(''); + expect(editor).toHaveAttribute( + 'data-placeholder', + expect.stringContaining('"entries"') + ); expect(onChange).not.toHaveBeenCalled(); }); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/common/Form/JSONSchema/JsonSchemaWidgets/ManifestJsonWidget/ManifestJsonWidget.tsx b/openmetadata-ui/src/main/resources/ui/src/components/common/Form/JSONSchema/JsonSchemaWidgets/ManifestJsonWidget/ManifestJsonWidget.tsx index 1fd08377bb5c..73061954ec15 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/common/Form/JSONSchema/JsonSchemaWidgets/ManifestJsonWidget/ManifestJsonWidget.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/common/Form/JSONSchema/JsonSchemaWidgets/ManifestJsonWidget/ManifestJsonWidget.tsx @@ -502,13 +502,15 @@ const ManifestJsonWidget = ({ onFocus?.(props.id, props.value); }, [onFocus, props.id, props.value]); - // Display the sample JSON as a placeholder when the field is empty so - // users have a ready template. We purposely do NOT write it into form - // state on mount — the field may be populated asynchronously after a - // saved pipeline config loads, and writing our sample into form data - // would overwrite the real value. We also skip when disabled. - const hasUserValue = typeof value === 'string' && value.trim().length > 0; - const effectiveValue = hasUserValue ? value : SAMPLE_MANIFEST_JSON; + // Show the sample JSON only as a greyed CodeMirror placeholder — never as the + // field value — so clearing the editor leaves it truly empty and typing is not + // reformatted mid-edit (autoFormat is disabled). + const editorValue = typeof value === 'string' ? value : ''; + + const editorOptions = useMemo( + () => ({ placeholder: SAMPLE_MANIFEST_JSON }), + [] + ); const handleChange = useCallback( (next: string) => { @@ -521,19 +523,21 @@ const ManifestJsonWidget = ({ ); const validation = useMemo( - () => validateManifestJson(effectiveValue), - [effectiveValue] + () => validateManifestJson(editorValue), + [editorValue] ); return (
diff --git a/openmetadata-ui/src/main/resources/ui/src/components/common/Form/JSONSchema/JsonSchemaWidgets/ManifestJsonWidget/manifest-json-widget.less b/openmetadata-ui/src/main/resources/ui/src/components/common/Form/JSONSchema/JsonSchemaWidgets/ManifestJsonWidget/manifest-json-widget.less index 06ecccdeca69..aa3f2dfd37ba 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/common/Form/JSONSchema/JsonSchemaWidgets/ManifestJsonWidget/manifest-json-widget.less +++ b/openmetadata-ui/src/main/resources/ui/src/components/common/Form/JSONSchema/JsonSchemaWidgets/ManifestJsonWidget/manifest-json-widget.less @@ -15,6 +15,8 @@ .manifest-json-widget { .manifest-json-widget-resize-wrapper { position: relative; + display: flex; + flex-direction: column; resize: vertical; overflow: auto; min-height: 180px; @@ -23,10 +25,30 @@ border: 1px solid @grey-22; border-radius: 4px; - .manifest-json-widget-editor, - .manifest-json-widget-editor > div, + // Flex-fill the wrapper so the editor container keeps its full height even + // when empty. `.manifest-json-widget-editor` (schema-editor-container) is + // position:relative, so the CodeMirror below can absolutely fill it. + .manifest-json-widget-editor { + flex: 1 1 auto; + min-height: 0; + } + + // CodeMirror defaults to 300px and a `height: 100%` won't resolve against a + // flex item, so it collapses to its content (~1 line) when empty. Absolutely + // filling the relative container pins it to the real box height instead. .CodeMirror { - height: 100% !important; + position: absolute; + inset: 0; + height: auto !important; + } + + // The sample manifest is shown only as a placeholder: keep its line breaks + // so it fills the editor, and mute the colour with the global placeholder + // token so it reads as a hint. !important overrides CodeMirror's white-space. + .CodeMirror-placeholder, + .CodeMirror-placeholder.CodeMirror-line-like { + color: var(--tw-color-text-placeholder); + white-space: pre-wrap !important; } } diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/SchemaEditor.utils.test.ts b/openmetadata-ui/src/main/resources/ui/src/utils/SchemaEditor.utils.test.ts new file mode 100644 index 000000000000..7a706cf26bcd --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/src/utils/SchemaEditor.utils.test.ts @@ -0,0 +1,63 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { getSchemaEditorValue } from './SchemaEditor.utils'; + +describe('getSchemaEditorValue', () => { + describe('with autoFormat enabled (default)', () => { + it('should pretty-print compact valid JSON with a 2-space indent', () => { + expect(getSchemaEditorValue('{"a":1,"b":[2]}')).toBe( + '{\n "a": 1,\n "b": [\n 2\n ]\n}' + ); + }); + + it('should return invalid JSON unchanged', () => { + expect(getSchemaEditorValue('{ not: valid')).toBe('{ not: valid'); + }); + + it('should return plain non-JSON text unchanged', () => { + expect(getSchemaEditorValue('hello world')).toBe('hello world'); + }); + + it('should return the raw value when the JSON parses to a falsy value', () => { + // getJSONFromString returns 0/false/null which the util treats as "not + // parseable" and falls back to the original string. + expect(getSchemaEditorValue('0')).toBe('0'); + expect(getSchemaEditorValue('false')).toBe('false'); + }); + }); + + describe('with autoFormat disabled', () => { + it('should return compact valid JSON untouched (no live re-indent)', () => { + expect(getSchemaEditorValue('{"a":1,"b":[2]}', false)).toBe( + '{"a":1,"b":[2]}' + ); + }); + + it('should preserve a partially typed JSON string verbatim', () => { + expect(getSchemaEditorValue('{"a":', false)).toBe('{"a":'); + }); + }); + + describe('with non-string input', () => { + it('should stringify an object with a 2-space indent', () => { + expect(getSchemaEditorValue({ a: 1 } as unknown as string)).toBe( + '{\n "a": 1\n}' + ); + }); + + it('should return an empty string for a number or undefined', () => { + expect(getSchemaEditorValue(42 as unknown as string)).toBe(''); + expect(getSchemaEditorValue(undefined as unknown as string)).toBe(''); + }); + }); +}); From 40e744050c2c156b5b22098ed45ff77a4c194e99 Mon Sep 17 00:00:00 2001 From: Aniket Katkar Date: Thu, 16 Jul 2026 17:55:20 +0530 Subject: [PATCH 2/6] fix(ui): add space after // in manifest autoFormat comment Satisfies the space-after-comment lint rule flagged in review. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../JsonSchemaWidgets/ManifestJsonWidget/ManifestJsonWidget.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openmetadata-ui/src/main/resources/ui/src/components/common/Form/JSONSchema/JsonSchemaWidgets/ManifestJsonWidget/ManifestJsonWidget.tsx b/openmetadata-ui/src/main/resources/ui/src/components/common/Form/JSONSchema/JsonSchemaWidgets/ManifestJsonWidget/ManifestJsonWidget.tsx index 73061954ec15..f3fede171590 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/common/Form/JSONSchema/JsonSchemaWidgets/ManifestJsonWidget/ManifestJsonWidget.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/common/Form/JSONSchema/JsonSchemaWidgets/ManifestJsonWidget/ManifestJsonWidget.tsx @@ -531,7 +531,7 @@ const ManifestJsonWidget = ({
Date: Thu, 16 Jul 2026 18:22:40 +0530 Subject: [PATCH 3/6] fix(ui): keep caret stable in manifest editor after auto-close react-codemirror2's Controlled mode cancels CodeMirror's native change and re-applies the value, which drops the caret that autoCloseBrackets places between the inserted pair (typing `{` produced `{}` with the caret after `}`, so the next character landed outside). Add an opt-in `uncontrolled` mode to SchemaEditor that renders the UnControlled CodeMirror so the editor owns its buffer and caret. The value prop is used only as initial content; external updates (e.g. an async-loaded saved config) are synced imperatively while the editor is blurred so active typing is never disturbed. ManifestJsonWidget opts in. Adds a Playwright assertion that typing `{` then `x` yields `{x}`. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Features/StorageMetadataAgentForm.spec.ts | 24 ++++++ .../SchemaEditor/SchemaEditor.interface.ts | 3 + .../Database/SchemaEditor/SchemaEditor.tsx | 73 +++++++++++++++---- .../ManifestJsonWidget/ManifestJsonWidget.tsx | 1 + 4 files changed, 86 insertions(+), 15 deletions(-) diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/StorageMetadataAgentForm.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/StorageMetadataAgentForm.spec.ts index 48a3cef2e6cb..f8950694f37d 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/StorageMetadataAgentForm.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/StorageMetadataAgentForm.spec.ts @@ -210,5 +210,29 @@ test.describe( await expect(manifestWidget.locator('.CodeMirror-line')).toHaveCount(1); }); }); + + test('auto-close keeps the caret between the inserted pair', async ({ + page, + }) => { + test.slow(); + await openMetadataAgentEditForm(page); + + const editor = page.locator('.manifest-json-widget .CodeMirror'); + await editor.click(); + await page.keyboard.press('ControlOrMeta+A'); + await page.keyboard.press('Delete'); + + // Typing `{` auto-inserts `}` with the caret between them; the next char + // must land inside. Before the fix the caret jumped to the end (`{}x`). + await page.keyboard.type('{'); + await page.keyboard.type('x'); + + const value = await editor.evaluate((el) => + ( + el as unknown as { CodeMirror: { getValue(): string } } + ).CodeMirror.getValue() + ); + expect(value).toBe('{x}'); + }); } ); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaEditor/SchemaEditor.interface.ts b/openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaEditor/SchemaEditor.interface.ts index 61561fbdd7c4..eb1adb19cfa3 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaEditor/SchemaEditor.interface.ts +++ b/openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaEditor/SchemaEditor.interface.ts @@ -22,6 +22,9 @@ export type Mode = { export interface SchemaEditorProps { value?: string; autoFormat?: boolean; + // Render an uncontrolled CodeMirror so it owns the caret (fixes cursor jumps + // after autoCloseBrackets). The value prop is used only as initial content. + uncontrolled?: boolean; refreshEditor?: boolean; className?: string; mode?: Mode; diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaEditor/SchemaEditor.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaEditor/SchemaEditor.tsx index b27d93c53d86..212810753ea5 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaEditor/SchemaEditor.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaEditor/SchemaEditor.tsx @@ -29,7 +29,10 @@ import 'codemirror/mode/python/python'; import 'codemirror/mode/sql/sql'; import { isUndefined } from 'lodash'; import { useCallback, useEffect, useRef, useState } from 'react'; -import { Controlled as CodeMirror } from 'react-codemirror2'; +import { + Controlled as CodeMirror, + UnControlled as UnControlledCodeMirror, +} from 'react-codemirror2'; import { useTranslation } from 'react-i18next'; import { ReactComponent as CopyIcon } from '../../../assets/svg/ic-duplicate.svg'; import { JSON_TAB_SIZE } from '../../../constants/constants'; @@ -42,6 +45,7 @@ import { SchemaEditorProps } from './SchemaEditor.interface'; const SchemaEditor = ({ value = '', autoFormat = true, + uncontrolled = false, className = '', mode = { name: CSMode.JAVASCRIPT, @@ -79,6 +83,13 @@ const SchemaEditor = ({ const wasHiddenRef = useRef(false); const { onCopyToClipBoard, hasCopied } = useClipboard(internalValue); + // In uncontrolled mode CodeMirror owns the buffer, so the value prop is only + // the initial content. Freezing it prevents the parent's onChange echo from + // re-hydrating the editor and losing the caret (e.g. after autoCloseBrackets). + const initialValueRef = useRef( + getSchemaEditorValue(value, autoFormat) + ); + const handleEditorInputBeforeChange = ( _editor: Editor, _data: EditorChange, @@ -91,8 +102,12 @@ const SchemaEditor = ({ _data: EditorChange, value: string ): void => { + const nextValue = getSchemaEditorValue(value, autoFormat); + if (uncontrolled) { + setInternalValue(nextValue); + } if (!isUndefined(onChange)) { - onChange(getSchemaEditorValue(value, autoFormat)); + onChange(nextValue); } }; @@ -123,6 +138,20 @@ const SchemaEditor = ({ setInternalValue(getSchemaEditorValue(value, autoFormat)); }, [value, autoFormat]); + // Uncontrolled editors own their buffer, so react-codemirror2 does not push + // later value changes in. Sync external updates (e.g. an async-loaded saved + // config) ourselves, but only while the editor is blurred so we never disturb + // the caret or an in-progress edit. + useEffect(() => { + const editor = editorInstance.current; + if (uncontrolled && editor) { + const nextValue = getSchemaEditorValue(value, autoFormat); + if (!editor.hasFocus() && editor.getValue() !== nextValue) { + editor.setValue(nextValue); + } + } + }, [value, autoFormat, uncontrolled]); + // Auto-detect display:none → visible transitions (e.g. Ant Design tab switches). // When a parent sets display:none, boundingClientRect collapses to 0. // When it becomes visible again, we refresh CodeMirror and reset scroll. @@ -185,19 +214,33 @@ const SchemaEditor = ({
)} - { - editorInstance.current = editor; - }} - editorWillUnmount={editorWillUnmount} - options={defaultOptions} - ref={wrapperRef} - value={internalValue} - onBeforeChange={handleEditorInputBeforeChange} - onChange={handleEditorInputChange} - {...(onFocus && { onFocus })} - /> + {uncontrolled ? ( + { + editorInstance.current = editor; + }} + editorWillUnmount={editorWillUnmount} + options={defaultOptions} + value={initialValueRef.current} + onChange={handleEditorInputChange} + {...(onFocus && { onFocus })} + /> + ) : ( + { + editorInstance.current = editor; + }} + editorWillUnmount={editorWillUnmount} + options={defaultOptions} + ref={wrapperRef} + value={internalValue} + onBeforeChange={handleEditorInputBeforeChange} + onChange={handleEditorInputChange} + {...(onFocus && { onFocus })} + /> + )}
); }; diff --git a/openmetadata-ui/src/main/resources/ui/src/components/common/Form/JSONSchema/JsonSchemaWidgets/ManifestJsonWidget/ManifestJsonWidget.tsx b/openmetadata-ui/src/main/resources/ui/src/components/common/Form/JSONSchema/JsonSchemaWidgets/ManifestJsonWidget/ManifestJsonWidget.tsx index f3fede171590..f59f1e00fdf2 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/common/Form/JSONSchema/JsonSchemaWidgets/ManifestJsonWidget/ManifestJsonWidget.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/common/Form/JSONSchema/JsonSchemaWidgets/ManifestJsonWidget/ManifestJsonWidget.tsx @@ -531,6 +531,7 @@ const ManifestJsonWidget = ({
Date: Thu, 16 Jul 2026 18:27:49 +0530 Subject: [PATCH 4/6] fix(ui): flush pending uncontrolled editor value on blur Address review edge case: an external value arriving while the uncontrolled editor is focused was skipped by the blurred-only sync and never re-applied. Track the latest value and apply it on the next blur. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Database/SchemaEditor/SchemaEditor.tsx | 37 ++++++++++++++++--- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaEditor/SchemaEditor.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaEditor/SchemaEditor.tsx index 212810753ea5..b8d50a9cca92 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaEditor/SchemaEditor.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaEditor/SchemaEditor.tsx @@ -89,6 +89,11 @@ const SchemaEditor = ({ const initialValueRef = useRef( getSchemaEditorValue(value, autoFormat) ); + // Latest external value; used to flush a pending update on blur when the + // sync effect had to skip it because the editor was focused. + const latestValueRef = useRef( + getSchemaEditorValue(value, autoFormat) + ); const handleEditorInputBeforeChange = ( _editor: Editor, @@ -143,15 +148,37 @@ const SchemaEditor = ({ // config) ourselves, but only while the editor is blurred so we never disturb // the caret or an in-progress edit. useEffect(() => { + const nextValue = getSchemaEditorValue(value, autoFormat); + latestValueRef.current = nextValue; const editor = editorInstance.current; - if (uncontrolled && editor) { - const nextValue = getSchemaEditorValue(value, autoFormat); - if (!editor.hasFocus() && editor.getValue() !== nextValue) { - editor.setValue(nextValue); - } + if ( + uncontrolled && + editor && + !editor.hasFocus() && + editor.getValue() !== nextValue + ) { + editor.setValue(nextValue); } }, [value, autoFormat, uncontrolled]); + // A value that arrived while the editor was focused is skipped above; apply + // any such pending change once the editor loses focus. + useEffect(() => { + const editor = editorInstance.current; + let detach = () => undefined as void; + if (uncontrolled && editor) { + const flushOnBlur = () => { + if (editor.getValue() !== latestValueRef.current) { + editor.setValue(latestValueRef.current); + } + }; + editor.on('blur', flushOnBlur); + detach = () => editor.off('blur', flushOnBlur); + } + + return detach; + }, [uncontrolled]); + // Auto-detect display:none → visible transitions (e.g. Ant Design tab switches). // When a parent sets display:none, boundingClientRect collapses to 0. // When it becomes visible again, we refresh CodeMirror and reset scroll. From 0b9ea275c097c37b6492aa655b113680ad8c198c Mon Sep 17 00:00:00 2001 From: Aniket Katkar Date: Fri, 17 Jul 2026 18:12:46 +0530 Subject: [PATCH 5/6] Fix form container border issue --- .../src/pages/AddIngestionPage/AddIngestionPage.component.tsx | 2 +- .../ui/src/pages/AddServicePage/AddServicePage.component.tsx | 2 +- .../EditConnectionFormPage/EditConnectionFormPage.component.tsx | 2 +- .../src/pages/EditIngestionPage/EditIngestionPage.component.tsx | 2 +- .../EmbeddedAddServicePage/EmbeddedAddServicePage.component.tsx | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/AddIngestionPage/AddIngestionPage.component.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/AddIngestionPage/AddIngestionPage.component.tsx index 66b169c804bf..84ffa236ac38 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/AddIngestionPage/AddIngestionPage.component.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/AddIngestionPage/AddIngestionPage.component.tsx @@ -257,7 +257,7 @@ const AddIngestionPage = () => { }; const firstPanelChildren = ( -
+
diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/AddServicePage/AddServicePage.component.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/AddServicePage/AddServicePage.component.tsx index c1ff4899235b..5124dea01cd1 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/AddServicePage/AddServicePage.component.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/AddServicePage/AddServicePage.component.tsx @@ -376,7 +376,7 @@ const AddServicePage = () => { // flex-col layout bounds the scroll area so the footer stays anchored at the card bottom, // keeping the card's rounded corners visible at all times during scroll. const firstPanelChildren = ( -
+
+
{ }; const firstPanelChildren = ( -
+
diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/EmbeddedAddServicePage/EmbeddedAddServicePage.component.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/EmbeddedAddServicePage/EmbeddedAddServicePage.component.tsx index 5c9aa86b7248..c30c5b1b2019 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/EmbeddedAddServicePage/EmbeddedAddServicePage.component.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/EmbeddedAddServicePage/EmbeddedAddServicePage.component.tsx @@ -430,7 +430,7 @@ const EmbeddedAddServicePage = () => { // flex-col layout bounds the scroll area so the footer stays anchored at the card bottom, // keeping the card's rounded corners visible at all times during scroll. const firstPanelChildren = ( -
+
Date: Fri, 17 Jul 2026 19:17:21 +0530 Subject: [PATCH 6/6] Remove the box shadow for test connection card --- .../src/components/common/TestConnection/TestConnection.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/openmetadata-ui/src/main/resources/ui/src/components/common/TestConnection/TestConnection.tsx b/openmetadata-ui/src/main/resources/ui/src/components/common/TestConnection/TestConnection.tsx index 7c06795cf1f2..1fccc27b6bef 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/common/TestConnection/TestConnection.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/common/TestConnection/TestConnection.tsx @@ -693,9 +693,7 @@ const TestConnection: FC = ({ {showDetails ? (