From 7fb95cb025515c26a29cfe8af047203dde6ecc83 Mon Sep 17 00:00:00 2001 From: Rajdeep Singh Date: Fri, 3 Apr 2026 14:11:43 +0530 Subject: [PATCH 1/5] Fixes #25434: Preserve empty string for Kafka schemaRegistryTopicSuffixName --- .../src/utils/MessagingServiceUtils.test.ts | 78 +++++++++++++++++++ .../ui/src/utils/MessagingServiceUtils.ts | 8 ++ 2 files changed, 86 insertions(+) create mode 100644 openmetadata-ui/src/main/resources/ui/src/utils/MessagingServiceUtils.test.ts diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/MessagingServiceUtils.test.ts b/openmetadata-ui/src/main/resources/ui/src/utils/MessagingServiceUtils.test.ts new file mode 100644 index 000000000000..dfae6083c331 --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/src/utils/MessagingServiceUtils.test.ts @@ -0,0 +1,78 @@ +/* + * Copyright 2025 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. + */ + +jest.mock( + '../jsons/connectionSchemas/connections/messaging/kafkaConnection.json', + () => ({}), + { virtual: true } +); +jest.mock( + '../jsons/connectionSchemas/connections/messaging/redpandaConnection.json', + () => ({}), + { virtual: true } +); +jest.mock( + '../jsons/connectionSchemas/connections/messaging/customMessagingConnection.json', + () => ({}), + { virtual: true } +); +jest.mock( + '../jsons/connectionSchemas/connections/messaging/kinesisConnection.json', + () => ({}), + { virtual: true } +); +jest.mock( + '../jsons/connectionSchemas/connections/messaging/pubSubConnection.json', + () => ({}), + { virtual: true } +); + +import { COMMON_UI_SCHEMA } from '../constants/Services.constant'; +import { MessagingServiceType } from '../generated/entity/services/messagingService'; +import { getMessagingConfig } from './MessagingServiceUtils'; + +describe('MessagingServiceUtils', () => { + it('Kafka uiSchema should include ui:emptyValue for schemaRegistryTopicSuffixName', () => { + const config = getMessagingConfig(MessagingServiceType.Kafka); + + expect(config.uiSchema).toMatchObject({ + ...COMMON_UI_SCHEMA, + schemaRegistryTopicSuffixName: { + 'ui:emptyValue': '', + }, + }); + }); + + it('Redpanda uiSchema should include ui:emptyValue for schemaRegistryTopicSuffixName', () => { + const config = getMessagingConfig(MessagingServiceType.Redpanda); + + expect(config.uiSchema).toMatchObject({ + ...COMMON_UI_SCHEMA, + schemaRegistryTopicSuffixName: { + 'ui:emptyValue': '', + }, + }); + }); + + it('non-broker services should not include schemaRegistryTopicSuffixName uiSchema', () => { + const config = getMessagingConfig(MessagingServiceType.Kinesis); + + expect(config.uiSchema).not.toHaveProperty('schemaRegistryTopicSuffixName'); + }); + + it('getMessagingConfig should return only common UI schema for invalid types', () => { + const config = getMessagingConfig('' as MessagingServiceType); + + expect(config.uiSchema).toEqual({ ...COMMON_UI_SCHEMA }); + }); +}); diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/MessagingServiceUtils.ts b/openmetadata-ui/src/main/resources/ui/src/utils/MessagingServiceUtils.ts index c435fa95110a..e46e3283033d 100644 --- a/openmetadata-ui/src/main/resources/ui/src/utils/MessagingServiceUtils.ts +++ b/openmetadata-ui/src/main/resources/ui/src/utils/MessagingServiceUtils.ts @@ -34,6 +34,12 @@ export const getBrokers = (config: MessagingConnection['config']) => { return !isUndefined(retVal) ? retVal : '--'; }; +const SCHEMA_REGISTRY_SUFFIX_UI_SCHEMA = { + schemaRegistryTopicSuffixName: { + 'ui:emptyValue': '', + }, +}; + export const getMessagingConfig = (type: MessagingServiceType) => { let schema = {}; const uiSchema = { ...COMMON_UI_SCHEMA }; @@ -41,11 +47,13 @@ export const getMessagingConfig = (type: MessagingServiceType) => { switch (type) { case MessagingServiceType.Kafka: schema = kafkaConnection; + Object.assign(uiSchema, SCHEMA_REGISTRY_SUFFIX_UI_SCHEMA); break; case MessagingServiceType.Redpanda: schema = redpandaConnection; + Object.assign(uiSchema, SCHEMA_REGISTRY_SUFFIX_UI_SCHEMA); break; From 7a63739b72dc59fae2034a8490de636c9639007e Mon Sep 17 00:00:00 2001 From: Rajdeep Singh Date: Fri, 17 Apr 2026 12:38:25 +0530 Subject: [PATCH 2/5] test(e2e): add Playwright coverage for empty schemaRegistryTopicSuffixName (#25434) Adds a regression test in ServiceForm.spec.ts that creates a Kafka messaging service with schemaRegistryTopicSuffixName='-value', clears the field in the edit-connection form, and asserts the PATCH payload carries an explicit empty string and that the cleared value persists on reload. Addresses review feedback on PR #27015 requesting Playwright coverage. --- .../playwright/e2e/Flow/ServiceForm.spec.ts | 101 ++++++++++++++++++ 1 file changed, 101 insertions(+) diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Flow/ServiceForm.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Flow/ServiceForm.spec.ts index 88d1746ad0a9..6182928635a7 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Flow/ServiceForm.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Flow/ServiceForm.spec.ts @@ -15,6 +15,7 @@ import { expect, test } from '@playwright/test'; import * as fs from 'fs'; import * as path from 'path'; import { PLAYWRIGHT_INGESTION_TAG_OBJ } from '../../constant/config'; +import { SERVICE_TYPE } from '../../constant/service'; import { CERT_FILE, lookerFormDetails, @@ -23,9 +24,11 @@ import { supersetFormDetails3, supersetFormDetails4, } from '../../constant/serviceForm'; +import { MessagingServiceClass } from '../../support/entity/service/MessagingServiceClass'; import { UserClass } from '../../support/user/UserClass'; import { createNewPage, redirectToHomePage, uuid } from '../../utils/common'; import { waitForAllLoadersToDisappear } from '../../utils/entity'; +import { visitServiceDetailsPage } from '../../utils/service'; import { fillSupersetFormDetails } from '../../utils/serviceFormUtils'; const SERVICE_NAMES = { @@ -453,5 +456,103 @@ test.describe( ); }); }); + + // Regression coverage for issue #25434 (PR #27015): + // Clearing `schemaRegistryTopicSuffixName` on a Kafka connection must + // send an empty string to the backend instead of dropping the field, + // so the cleared value is persisted on reload. + test.describe('Kafka', () => { + const kafkaService = new MessagingServiceClass( + `pw-kafka-empty-suffix-${uuid()}` + ); + + test.beforeAll('Create Kafka service with suffix set', async ({ + browser, + }) => { + const { apiContext, afterAction } = await createNewPage(browser); + await kafkaService.create(apiContext); + await kafkaService.patch(apiContext, [ + { + op: 'add', + path: '/connection/config/schemaRegistryURL', + value: 'http://localhost:8081', + }, + { + op: 'add', + path: '/connection/config/schemaRegistryTopicSuffixName', + value: '-value', + }, + ]); + await afterAction(); + }); + + test.afterAll('Cleanup Kafka service', async ({ browser }) => { + const { apiContext, afterAction } = await createNewPage(browser); + await kafkaService.delete(apiContext); + await afterAction(); + }); + + test('should persist empty schemaRegistryTopicSuffixName when the field is cleared', async ({ + page, + }) => { + await visitServiceDetailsPage( + page, + { + name: kafkaService.entity.name, + type: SERVICE_TYPE.Messaging, + }, + false, + false + ); + + await page.getByRole('tab', { name: 'Connection' }).click(); + await page.getByTestId('edit-connection-button').click(); + await waitForAllLoadersToDisappear(page); + + const suffixInput = page.locator( + String.raw`#root\/schemaRegistryTopicSuffixName` + ); + + await expect(suffixInput).toHaveValue('-value'); + await suffixInput.clear(); + await expect(suffixInput).toHaveValue(''); + + const patchResponse = page.waitForResponse( + (response) => + response.url().includes('/api/v1/services/messagingServices') && + response.request().method() === 'PATCH' + ); + + await page.getByTestId('submit-btn').click(); + await page.getByTestId('submit-btn').click(); + + const patch = await patchResponse; + const patchBody = patch.request().postDataJSON() as Array<{ + op: string; + path: string; + value?: unknown; + }>; + + const suffixOp = patchBody.find( + (op) => + op.path === '/connection/config/schemaRegistryTopicSuffixName' + ); + + // The fix must send an explicit empty string, not drop the field + // (which would leave the stale "-value" on the server). + expect(suffixOp).toBeDefined(); + expect(suffixOp?.value).toBe(''); + + await waitForAllLoadersToDisappear(page); + + // Reopen the edit form and verify the cleared value persisted. + await page.getByTestId('edit-connection-button').click(); + await waitForAllLoadersToDisappear(page); + + await expect( + page.locator(String.raw`#root\/schemaRegistryTopicSuffixName`) + ).toHaveValue(''); + }); + }); } ); From a01fc78fc1a5772d1c501c97023d5edc22fb0d63 Mon Sep 17 00:00:00 2001 From: Rajdeep Singh Date: Fri, 17 Apr 2026 13:04:21 +0530 Subject: [PATCH 3/5] style: apply prettier formatting to ServiceForm.spec.ts --- .../playwright/e2e/Flow/ServiceForm.spec.ts | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Flow/ServiceForm.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Flow/ServiceForm.spec.ts index 6182928635a7..61f3c1c500d7 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Flow/ServiceForm.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Flow/ServiceForm.spec.ts @@ -466,25 +466,26 @@ test.describe( `pw-kafka-empty-suffix-${uuid()}` ); - test.beforeAll('Create Kafka service with suffix set', async ({ - browser, - }) => { - const { apiContext, afterAction } = await createNewPage(browser); - await kafkaService.create(apiContext); - await kafkaService.patch(apiContext, [ - { - op: 'add', - path: '/connection/config/schemaRegistryURL', - value: 'http://localhost:8081', - }, - { - op: 'add', - path: '/connection/config/schemaRegistryTopicSuffixName', - value: '-value', - }, - ]); - await afterAction(); - }); + test.beforeAll( + 'Create Kafka service with suffix set', + async ({ browser }) => { + const { apiContext, afterAction } = await createNewPage(browser); + await kafkaService.create(apiContext); + await kafkaService.patch(apiContext, [ + { + op: 'add', + path: '/connection/config/schemaRegistryURL', + value: 'http://localhost:8081', + }, + { + op: 'add', + path: '/connection/config/schemaRegistryTopicSuffixName', + value: '-value', + }, + ]); + await afterAction(); + } + ); test.afterAll('Cleanup Kafka service', async ({ browser }) => { const { apiContext, afterAction } = await createNewPage(browser); @@ -534,8 +535,7 @@ test.describe( }>; const suffixOp = patchBody.find( - (op) => - op.path === '/connection/config/schemaRegistryTopicSuffixName' + (op) => op.path === '/connection/config/schemaRegistryTopicSuffixName' ); // The fix must send an explicit empty string, not drop the field From 5d07160170b4362bb25fc841de623d8c4c845cae Mon Sep 17 00:00:00 2001 From: Rajdeep Singh Date: Mon, 20 Apr 2026 01:59:54 +0530 Subject: [PATCH 4/5] style: drop PR number from regression comment per Copilot review --- .../main/resources/ui/playwright/e2e/Flow/ServiceForm.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Flow/ServiceForm.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Flow/ServiceForm.spec.ts index 61f3c1c500d7..3a3140fc9aa6 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Flow/ServiceForm.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Flow/ServiceForm.spec.ts @@ -457,7 +457,7 @@ test.describe( }); }); - // Regression coverage for issue #25434 (PR #27015): + // Regression coverage for issue #25434: // Clearing `schemaRegistryTopicSuffixName` on a Kafka connection must // send an empty string to the backend instead of dropping the field, // so the cleared value is persisted on reload. From c45cee47c12e65dc2f16d26236acf22ab4e224f2 Mon Sep 17 00:00:00 2001 From: Rajdeep Singh Date: Mon, 20 Apr 2026 15:36:17 +0530 Subject: [PATCH 5/5] fix(test): import COMMON_UI_SCHEMA from non-deprecated ServiceUISchema.constant per Copilot review --- .../main/resources/ui/src/utils/MessagingServiceUtils.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/MessagingServiceUtils.test.ts b/openmetadata-ui/src/main/resources/ui/src/utils/MessagingServiceUtils.test.ts index dfae6083c331..9cf1fa9b5ac3 100644 --- a/openmetadata-ui/src/main/resources/ui/src/utils/MessagingServiceUtils.test.ts +++ b/openmetadata-ui/src/main/resources/ui/src/utils/MessagingServiceUtils.test.ts @@ -37,7 +37,7 @@ jest.mock( { virtual: true } ); -import { COMMON_UI_SCHEMA } from '../constants/Services.constant'; +import { COMMON_UI_SCHEMA } from '../constants/ServiceUISchema.constant'; import { MessagingServiceType } from '../generated/entity/services/messagingService'; import { getMessagingConfig } from './MessagingServiceUtils';