Skip to content

Feat/UI/refactor add domain form#26951

Open
siddhant1 wants to merge 55 commits intomainfrom
feat/ui/refactor-add-domain-form
Open

Feat/UI/refactor add domain form#26951
siddhant1 wants to merge 55 commits intomainfrom
feat/ui/refactor-add-domain-form

Conversation

@siddhant1
Copy link
Copy Markdown
Member

@siddhant1 siddhant1 commented Apr 2, 2026

Describe your changes:

Fixes

I worked on ... because ...

Type of change:

  • Bug fix
  • Improvement
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

Summary by Gitar

  • Test fixes:
    • Updated domain selector to target combobox role within container for reliable interaction

This will update automatically on new commits.

Siddhant and others added 2 commits April 1, 2026 17:16
Migrate AddDomainForm from Ant Design to @openmetadata/ui-core-components
using HookForm, getField, and FieldTypes. Add onFocus prop support to
AutocompleteBase, add rules prop to FormField, and export form-field
components from the core library.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
- Switch FieldTypes.SELECT to use Select with Select.Item children
  for proper icon/avatar/supportingText support
- Pass fontSize through SelectContext so items respect the fontSize prop
- Show colored-initial avatars for users and team icons via Avatar
- Show tag color dots using Dot component on tag options
- Replace custom glossary terms autocomplete with MUIGlossaryTagSuggestion

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@siddhant1 siddhant1 requested review from a team, chirag-madlani and karanh37 as code owners April 2, 2026 05:41
Copilot AI review requested due to automatic review settings April 2, 2026 05:41
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 2, 2026

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the Domain “Add” form UI to use @openmetadata/ui-core-components + react-hook-form (instead of Ant Design Form), while introducing reusable form-field building blocks in openmetadata-ui-core-components to support the new form patterns.

Changes:

  • Refactor AddDomainForm to react-hook-form + ui-core-components fields (icon picker, color picker, tag/domain/user selectors, etc.).
  • Add a new shared form-field system to openmetadata-ui-core-components (Field, getField, FormItemLabel, FieldTypes, etc.) and export it from the components entrypoint.
  • Extend core select/autocomplete plumbing (e.g., SelectContext now includes fontSize; autocomplete can invoke an onFocus callback).

Reviewed changes

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

Show a summary per file
File Description
openmetadata-ui/src/main/resources/ui/src/components/Domain/AddDomainForm/AddDomainForm.component.tsx Rebuilds the Domain/Data Product form using react-hook-form + ui-core-components fields and a custom imperative ref bridge.
openmetadata-ui/src/main/resources/ui/src/components/Domain/AddDomainForm/AddDomainForm.interface.ts Replaces AntD FormInstance with a custom DomainFormRef contract for submit/reset/validate.
openmetadata-ui/src/main/resources/ui/src/components/Domain/AddDomainForm/AddDomainForm.test.tsx Updates tests/mocks to align with the refactored form and new ref behavior.
openmetadata-ui-core-components/src/main/resources/ui/src/components/index.ts Exports the newly added form-field primitives/types.
openmetadata-ui-core-components/src/main/resources/ui/src/components/base/select/select.tsx Adds fontSize to SelectContext and provides it via SelectContext.Provider.
openmetadata-ui-core-components/src/main/resources/ui/src/components/base/select/select-item.tsx Uses fontSize from context to control item label/supporting text typography.
openmetadata-ui-core-components/src/main/resources/ui/src/components/base/form/hook-form.tsx Adds rules support to FormField props for react-hook-form validation.
openmetadata-ui-core-components/src/main/resources/ui/src/components/base/autocomplete/autocomplete.tsx Adds onFocus passthrough (used to resize popover on focus) during trigger focus.
openmetadata-ui-core-components/src/main/resources/ui/src/components/application/form-field/form-item-label.tsx New shared label component with required marker, tooltip, and beta badge.
openmetadata-ui-core-components/src/main/resources/ui/src/components/application/form-field/form-field.types.ts New shared types/enums for form fields (FieldTypes, FieldProp, etc.).
openmetadata-ui-core-components/src/main/resources/ui/src/components/application/form-field/form-field.tsx New implementation of Field/getField rendering for multiple input types.

Comment on lines +114 to +121
if ('current' in targetRef) {
targetRef.current = value;

return () => {
if (targetRef.current === value) {
targetRef.current = null;
}
: null;

const iconField: FieldProp = {
name: 'iconURL',
id: 'root/iconURL',
label: t('label.icon'),
muiLabel: t('label.icon'),
required: false,
type: FieldTypes.ICON_PICKER_MUI,
helperText: iconTooltipDataRender(),
props: {
'data-testid': 'icon-url',
allowUrl: true,
placeholder: t('label.icon-url'),
backgroundColor: selectedColor,
defaultIcon:
};
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

syncFormRef assigns to targetRef.current, but formRef is typed as Ref<DomainFormRef> where RefObject.current is readonly under @types/react v18. This will cause a TS error and/or require unsafe mutation of a readonly ref.

Consider changing DomainFormRefProp to accept MutableRefObject<DomainFormRef | null> (and/or RefObject separately) and casting inside this branch, or using useImperativeHandle with a forwarded ref instead of mutating .current directly.

Copilot uses AI. Check for mistakes.
Comment on lines +296 to +307
const fetchDomainOptions = useCallback(async (searchText = '') => {
try {
const domains = await searchDomains(searchText, 1);
const nextOptions = domains.map((domain: any) =>
mapEntityReferenceToOption({
displayName: domain.displayName,
fullyQualifiedName: domain.fullyQualifiedName,
id: domain.id,
name: domain.name,
type: EntityType.DOMAIN,
})
);
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

domains.map((domain: any) => ...) introduces an any in UI code. This breaks the repo’s zero-tolerance type-safety requirement and also hides the shape expected from searchDomains().

Prefer typing the response from searchDomains() (e.g., Domain[] or a minimal interface with id/name/displayName/fullyQualifiedName) and removing the any cast.

Copilot uses AI. Check for mistakes.
Comment on lines 677 to 693
const expertsField: FieldProp = {
id: 'root/experts',
label: t('label.expert-plural'),
},
formItemProps: {
valuePropName: 'value',
trigger: 'onChange',
initialValue: [],
},
};

const createPermission = useMemo(
() =>
checkPermission(Operation.Create, ResourceEntity.GLOSSARY, permissions),
[permissions]
);

const selectedOwners =
Form.useWatch<EntityReference | EntityReference[]>('owners', form) ?? [];

const ownersList = Array.isArray(selectedOwners)
? selectedOwners
: [selectedOwners];

const expertsList = Form.useWatch<EntityReference[]>('experts', form) ?? [];

const handleFormSubmit: FormProps['onFinish'] = (formData) => {
const updatedData = omit(
formData,
'color',
'iconURL',
'glossaryTerms'
// Keep 'coverImage' - parent will extract and remove it before API call
// Don't exclude 'domains' - we need it for DataProducts
);
const style = {
color: formData.color,
iconURL: formData.iconURL,
// Don't include coverImage here - it's not uploaded yet
// Parent will add it to style after upload
name: 'experts',
placeholder: t('label.select-field', {
field: t('label.expert-plural'),
}),
props: {
filterOption: () => true,
multiple: true,
onFocus: handleUserTeamFocus,
onSearchChange: (searchText: string) =>
debouncedUserTeamSearch(searchText),
options: userTeamOptions,
},
type: FieldTypes.USER_TEAM_SELECT,
};
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

experts in CreateDomain/CreateDataProduct is defined as a list of user/login names (string[]), but the options backing expertsField (userTeamOptions) include teams. Selecting a team here will submit a team name as an expert, which is likely invalid and can break creation.

Consider fetching users-only options for the experts field (or filtering userTeamOptions by type === EntityType.USER) so only users can be selected as experts.

Copilot uses AI. Check for mistakes.
Comment on lines +155 to 178
export const SelectContext = createContext<{
size: 'sm' | 'md';
fontSize: 'xs' | 'sm' | 'md' | 'lg' | 'xl';
}>({
size: 'sm',
fontSize: 'md',
});

const Select = ({
placeholder = 'Select',
placeholderIcon,
size = 'sm',
fontSize = 'md',
children,
items,
label,
hint,
tooltip,
className,
...rest
}: SelectProps) => {
return (
<SelectContext.Provider value={{ size }}>
<SelectContext.Provider value={{ size, fontSize }}>
<AriaSelect
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

SelectContext now requires fontSize in its value shape. This is a breaking change for existing SelectContext.Provider usages that only provide { size } (e.g., Select ComboBox / Autocomplete providers), and will either fail type-checking or make fontSize undefined downstream.

To avoid regressions, update all SelectContext.Provider values to include fontSize (or change the context type to make fontSize optional with a default).

Copilot uses AI. Check for mistakes.
@siddhant1 siddhant1 added the safe to test Add this label to run secure Github workflows on PRs label Apr 2, 2026
…rm PR

Fix jsx-sort-props errors in form-field.tsx, replace `any` with `Domain`
type, use MutableRefObject to fix readonly ref assignment, add missing
fontSize to SelectContext providers, and restrict experts field to
user-only options matching original behavior.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…y modules

Split the monolithic form-field.tsx (~1290 lines) into focused files:
- fields/color-picker-field.tsx and fields/icon-picker-field.tsx for standalone field components
- render-field-element.tsx for the central field type dispatch
- form-field.tsx reduced to thin Field/FormFields wrapper (~95 lines)

Replaced Record<string, unknown> FieldPropsMap with a properly typed interface,
eliminated all `as` casts and `unknown` types, and removed the unnecessary
select normalization layer (consumers pass correctly shaped SelectItemType data).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 2, 2026 06:49
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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

Comments suppressed due to low confidence (1)

openmetadata-ui/src/main/resources/ui/src/components/Domain/AddDomainForm/AddDomainForm.test.tsx:253

  • The refactor changes submission/validation behavior substantially, but the “should handle form submission” test doesn’t assert that onSubmit is invoked or that the payload is correctly shaped (e.g., tags/owners/experts/domains mapping, coverImage file value). Strengthen this test to populate key fields, trigger submit, and verify the exact onSubmit arguments (and formRef.validateFields() behavior if that’s part of the external contract).
  it('should handle form submission', () => {
    render(<AddDomainForm {...defaultProps} />);

    const saveButton = screen.getByTestId('save-domain');
    fireEvent.click(saveButton);

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated 1 comment.

@github-actions
Copy link
Copy Markdown
Contributor

✅ TypeScript Types Auto-Updated

The generated TypeScript types have been automatically updated based on JSON schema changes in this PR.

Siddhant and others added 23 commits April 17, 2026 11:01
The new Autocomplete.Item in @openmetadata/ui-core-components computes
the option's accessible name as `label + ' ' + supportingText`, so
`getByRole('option', { name: tagFqn, exact: true })` never matches when
a tag has a displayName. Revert to `getByTestId('tag-option-<fqn>')`,
which is still emitted by TagSuggestion on every Autocomplete.Item and
is unaffected by the supporting-text concatenation.

Fixes three TagSuggestion tests that timed out on all retries in
playwright-ci-postgresql (4, 6):
  - DataProducts.spec.ts:449 Create data product with tags using TagSuggestion
  - Domains.spec.ts:1196 Create domain with tags using TagSuggestion
  - Domains.spec.ts:1243 Create subdomain with tags using TagSuggestion

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…dDomainForm/AddDomainForm.component.tsx

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
The refactored AddDomainForm renders the tag input via the generic
Autocomplete renderer (getDefaultAutocompleteItems), which never passes
data-testid to Autocomplete.Item. As a result, the playwright helper
selectTagInTagSuggestion() could not find tag-option-<fqn> elements in
the dropdown, causing three TagSuggestion specs to time out:
  - DataProducts.spec.ts:449 Create data product with tags using TagSuggestion
  - Domains.spec.ts:1196 Create domain with tags using TagSuggestion
  - Domains.spec.ts:1243 Create subdomain with tags using TagSuggestion

Provide a custom renderItem on the tags field that mirrors the default
item rendering and forwards data-testid="tag-option-${item.id}" so the
existing test selector works against the new component path.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…from dropdown after classification re-enable (#27415)

* tests: fix flaky SystemCertificationTags test where Gold tag missing from dropdown after classification re-enable

* nit
* increase data contracts test timeout

* fix failure in restroreEntityInheritedFields

* fix restrore entity inheritance specs

* remove unnecessary multiple domains check

---------

Co-authored-by: Shrabanti Paul <shrabantipaul@Shrabantis-MacBook-Pro.local>
* increase data contracts test timeout

* fix flaky customizeDetailPage specs

---------

Co-authored-by: Shrabanti Paul <shrabantipaul@Shrabantis-MacBook-Pro.local>
* fix(test): assign role to team in P-11 team-based permissions test

The "Team-based permissions work correctly" test created a team and added
testUser to it, but never attached the new role to the team — so the user
inherited no permissions from team membership. The test relied on default
Organization conditional rules, which the frontend treats as no-permission
(only Access.Allow becomes truthy in PermissionsUtils). The glossary page
then rendered the no-permission placeholder and never fired
/api/v1/glossaries?fields=*, hanging visitGlossaryPage's waitForResponse
for the full 30s toPass budget.

Patch the team's defaultRoles after initializePermissions so the user
actually inherits Allow via team membership, matching the test's intent.

Local repro (full file × 10 repeats × 4 workers, retries=0):
- Before: 10/10 P-11 failures
- After:  10/10 P-11 pass, 93/93 overall pass

* fix(test): isolate P-11 to a dedicated user/team

Avoid mutating the file-shared testUser. Previously the fix added testUser
to a temporary team and attached an Allow role; if cleanup failed, the
elevated permissions would leak into subsequent tests sharing the same
worker.

Now P-11 creates its own UserClass + page, runs the team-permission
verification in isolation, and tears down the user, team, role, and policy
at the end. Other tests in the file see no state change from this test
beyond what initializePermissions/cleanupPermissions already do.

Local verification (full file × 10 repeats × 4 workers, retries=0):
93/93 pass, P-11 10/10.

---------

Co-authored-by: Siddhant <siddhant@MacBook-Pro-621.local>
…able for dbt test entity links (#27366)

* fix: use test_metadata.kwargs['model'] to identify primary table for entity links (issue #24636)

For dbt relationship tests with multiple upstream dependencies, the order of
tables in depends_on.nodes varies by database engine (Snowflake vs Unity Catalog).
The primary table being tested is explicitly specified in test_metadata.kwargs['model']
for generic tests, making this a reliable order-independent way to identify the correct
table for entity link generation. This fixes validation failures when columns exist in
the primary table but not in the referenced table.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Run formatter

* Apply Gitar-bot comments

---------

Co-authored-by: Claude <noreply@anthropic.com>
…w comment (#27402)

* feat: add consolidated UI checkstyle commands for all and changed files

* update prt to pr

* test commit to fail ui-checkstyle

* update the comment

* Revert "test commit to fail ui-checkstyle"

This reverts commit ed056f0.

* Revert "update prt to pr"

This reverts commit 0666fa5.

* Worked on comments

* pull request target remove

* Revert "pull request target remove"

This reverts commit b61e98c.

* Worked on comments
…d remove broken v1125 migration (#27427)

* fix(migration): add v1126 reverse migration to revert webhook authType back to secretKey

* fix(migration): remove migrateWebhookSecretKeyToAuthType from v1125 migration

* fix(test): remove migrateWebhookSecretKeyToAuthType references from v1125 migration tests

* fix(migration): address copilot review comments on v1126 migration

* fix(migration): case-insensitive bearer check and verify JSON content in v1126 tests

* fix(migration): remove unused constants from v1125 and add postgres path + SQL verification to v1126 tests
…s with accurate counts (#27354)

* feat(explore): redesign search export scope to export full tab results with accurate counts

* fix UI checkstyle and explore support

* fix checkstyle

* fix failing spec & code refactor

* add backend support for handling totalVotes export in explore page

* fix specs

* add skeleton loader
* Fix omjob pod/label naming length constraints

- Fix SHA-256 hash byte formatting with & 0xff mask for proper
  2-hex-digit encoding per byte
- Enforce Kubernetes 63-character label limit via hash-based truncation
- Extract shared hash utility to HashUtils
- Add comprehensive tests for truncation, uniqueness, and edge cases

Fixes #27004

* Address review: fix hash bounds, add edge case tests, remove redundant substring

- Guard ensureValidLabelValue fallback against StringIndexOutOfBounds
- Add tests for separator-only inputs exercising the fallback path
- Remove redundant .substring() since HashUtils.hash() already returns 6 chars
- Use 253 (K8s DNS subdomain limit) instead of 260 in PodManagerTest
- Fix wrong assertion in LabelBuilderTest (podSelector has 2 entries)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: complete all code review issues for PR merge

- Add HTTP 409 idempotency handling in TableClass.create() for sharded Playwright tests
- Apply Java Spotless formatting to fix checkstyle violations
- Apply Playwright formatting: organize-imports, eslint --fix, prettier --write
- Resolve all 10 code review findings:
  ✅ StringIndexOutOfBoundsException in hash truncation (already fixed)
  ✅ Redundant substring operation (already fixed)
  ✅ Duplicate hash code extraction (already done)
  ✅ Playwright 409 conflict handling (now added)
  ✅ Java formatting compliance (now applied)
  ✅ TypeScript formatting compliance (now applied)

PR is now ready for merge with all CI checks expected to pass.

---------

Co-authored-by: Pere Miquel Brull <peremiquelbrull@gmail.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* test:Added missing test for ontology

* Added other missing part of test

* fix lint issue
…artial migrations (#27234)

* fix(workflows): make Flowable schema upgrades idempotent to survive partial migrations

Fixes #26048.

When the server crashed mid-startup during a Flowable schema upgrade, the DB
was left in a partially-migrated state. On restart, Flowable re-ran the same
DDL and failed on already-existing objects (indexes, tables, columns), permanently
wedging both the server and migrate --force.

Changes:
1. WorkflowHandler: webserver now uses DB_SCHEMA_UPDATE_FALSE — it validates the
   schema but never runs DDL. Only migrate CLI uses DB_SCHEMA_UPDATE_TRUE.
2. OpenMetadataOperations: explicit WorkflowHandler.initialize(config, true) inside
   the migrate command so Flowable DDL always runs during migration.
3. WorkflowHandler: catches FlowableWrongDbException on webserver startup and
   rethrows with an actionable message directing the operator to run migrate.
4. IdempotentDdlDataSource + IdempotentDdlStatement: JDBC DataSource wrapper used
   exclusively in migration context. Intercepts execute(sql) for CREATE INDEX,
   CREATE TABLE, and ALTER TABLE ADD COLUMN and pre-checks existence via standard
   DatabaseMetaData (getIndexInfo, getTables, getColumns) before executing. If the
   object already exists it logs a skip and returns — no SQL state codes, no string
   matching, works on MySQL and PostgreSQL.

Unit tests cover schema-update mode selection in both contexts.

* fix(workflows): address review comments on idempotent DDL wrapper

- Extract shouldSkip() helper; apply idempotency checks to all execute()
  and executeUpdate() overloads, not just execute(String)
- Tighten ALTER TABLE regex with negative lookahead to exclude SQL keywords
  (CONSTRAINT, PRIMARY, UNIQUE, FOREIGN, CHECK, INDEX, KEY) from being
  matched as column names
- IdempotentDdlDataSource now wraps a DataSource delegate instead of calling
  DriverManager directly; uses migrationDataSource() helper in WorkflowHandler
  to resolve from existing DataSource or JDBC params
- Fix InvocationTargetException wrapping in Connection proxy — unwrap cause
  so callers receive the original SQLException
- Wrap all createStatement() variants in the proxy, not just the no-arg form
- Contextual error message in WorkflowHandler — distinguish between server
  startup and migration context
- Add IdempotentDdlStatementTest: 11 tests covering skip/execute for
  CREATE INDEX, CREATE UNIQUE INDEX, CREATE TABLE, ALTER TABLE ADD COLUMN,
  keyword-guarded ALTER TABLE, executeUpdate overload, and pass-through

* fix(workflows): include DB/library versions in FlowableWrongDbException message

* test(workflows): add IdempotentDdlDataSourceTest for proxy wrapping and exception surfacing

* test(workflows): assert exception identity in proxy exception-surfacing tests

* fix(workflows): catalog-aware identifier normalization in IdempotentDdlStatement

On MySQL with lower_case_table_names=0 (default on Linux), table names are
stored as-is and catalog=null metadata lookups can miss existing objects.

- Use connection.getCatalog() for all getIndexInfo/getTables/getColumns calls
- Normalize identifiers via DatabaseMetaData.storesLowerCaseIdentifiers() /
  storesUpperCaseIdentifiers() instead of unconditional toLowerCase()
- stripIdentifierQuotes() handles backtick, double-quote and bracket quoting
- extractObjectName() handles schema-qualified names (schema.table)
- columnExists now iterates and normalizes COLUMN_NAME from ResultSet
- Test: added MySQL uppercase storage case to IdempotentDdlStatementTest

* fix(workflows): null guard in shouldSkip, drop-create Flowable init, robust test indexing

- shouldSkip() returns false immediately for null SQL, preserving JDBC contract
  (delegate handles null and throws the driver's own error)
- drop-create command now calls WorkflowHandler.initialize(config, true) after
  native migrations so it produces a fully startable DB including Flowable tables
- WorkflowHandlerSchemaUpdateTest: replace brittle get(1) with getLast() so the
  test is not sensitive to how many StandaloneProcessEngineConfiguration instances
  are constructed before initializeNewProcessEngine runs
…27431)

* Move ontology/glossary relation migration from 1.14.0 back to 1.13.0

Ontology feature will ship in 1.13.0, not 1.14.0. Move the glossary term
relation migrations (relationType backfill, settings insert, stale
relatedTerms strip, conceptMappings backfill) back to the 1.13.0
postDataMigrationSQLScript for both MySQL and PostgreSQL.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Restore empty 1.14.0 SQL migration files for Java migration framework

The V114 MigrationUtil.java package requires the 1.14.0 migration
directory to exist with SQL files for the migration to be picked up.
Keep them as empty files (matching convention of other versions with
no post-data SQL).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Add schemaChanges.sql and comment all 1.14.0 SQL migration files

Add both schemaChanges.sql and postDataMigrationSQLScript.sql for
mysql and postgres with a comment explaining the directory is required
for the V114 Java migrations to be picked up by the migration framework.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Fix missing trailing newline in postgres postDataMigrationSQLScript

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* address feedback

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Karan Hotchandani <33024356+karanh37@users.noreply.github.com>
…#27438)

* feat(migration): add webhook secretKey to authType migration in v1130

* test(migration): add idempotency test for already-migrated webhook rows in v1130
… Redshift AUTO-distribution MVs (#27016)

Co-authored-by: Teddy <teddy.crepineau@gmail.com>
…ce (#27372)

Bumps org.eclipse.jetty:jetty-http from 12.1.6 to 12.1.7.

---
updated-dependencies:
- dependency-name: org.eclipse.jetty:jetty-http
  dependency-version: 12.1.7
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: sonika-shah <58761340+sonika-shah@users.noreply.github.com>
…ith regression tests (#27413)

* deps(ingestion): upgrade collate-sqllineage to >=2.1.1 with expanded lineage test coverage

* address copilot comments
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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

Comment thread openmetadata-ui/src/main/resources/ui/src/utils/IconUtils.test.tsx
@gitar-bot
Copy link
Copy Markdown

gitar-bot bot commented Apr 17, 2026

Code Review ✅ Approved 9 resolved / 9 findings

Refactors the domain creation form and updates type safety, addressing issues with data validation, callback handling, and submission logic. No issues remain.

✅ 9 resolved
Quality: Untyped any used for domain search response

📄 openmetadata-ui/src/main/resources/ui/src/components/Domain/AddDomainForm/AddDomainForm.component.tsx:299
Line 299 uses (domain: any) in the fetchDomainOptions callback, bypassing TypeScript checks on the domain object shape. This should use the proper domain type from the API response.

Bug: validateFields returns raw form data, not CreateDomain shape

📄 openmetadata-ui/src/main/resources/ui/src/components/Domain/AddDomainForm/AddDomainForm.component.tsx:493-501 📄 openmetadata-ui/src/main/resources/ui/src/components/Domain/AddDomainForm/AddDomainForm.component.tsx:436-450
The validateFields method (line 493-501) is typed to return Promise<CreateDomain | CreateDataProduct> but returns form.getValues() — the raw form data with separate color, iconURL, glossaryTerms, and unprocessed experts/owners arrays. The actual transformation (combining tags + glossaryTerms, building the style object, mapping experts to name strings, etc.) only happens inside handleFormSubmit.

Currently the only caller (useFormDrawerWithRef) discards the return value when formRef.submit is present, so it works by accident. However, the type contract is violated and any future caller relying on the return value will receive malformed API payloads.

Bug: onChange callback silently dropped for Switch/Checkbox/Slider

📄 openmetadata-ui-core-components/src/main/resources/ui/src/components/application/form-field/render-field-element.tsx:342-345 📄 openmetadata-ui-core-components/src/main/resources/ui/src/components/application/form-field/render-field-element.tsx:361-364 📄 openmetadata-ui-core-components/src/main/resources/ui/src/components/application/form-field/render-field-element.tsx:373-376
The onChange prop callback was removed from SWITCH, CHECKBOX, and SLIDER field type handlers in renderFieldElement. While no current consumers appear to pass onChange for these field types, this is a shared component library — removing the callback without deprecation silently breaks the contract for any consumer (including external ones) that may rely on it. The other field types (e.g., text, select) still forward onChange.

Bug: DataProduct submitted with empty domains array when no domain selected

📄 openmetadata-ui/src/main/resources/ui/src/components/Domain/AddDomainForm/AddDomainForm.component.tsx:460-468
When creating a DataProduct, if neither formData.domains nor parentDomain has a fullyQualifiedName, the code now submits domains: []. Previously, the domains field was only set when a value existed. While this is type-safe (domains: string[]), the API likely requires at least one domain for a DataProduct. This could result in a server-side validation error or, worse, creation of an orphaned DataProduct with no domain association.

Bug: Submit handler expects objects but autocomplete stores string IDs

📄 openmetadata-ui/src/main/resources/ui/src/components/Domain/AddDomainForm/AddDomainForm.component.tsx:442-456 📄 openmetadata-ui-core-components/src/main/resources/ui/src/components/application/form-field/render-field-element.tsx:166-180
The new autocomplete fields store only string IDs via field.onChange(selectedItem.id) (render-field-element.tsx:178), but handleFormSubmit casts form values as full objects:

  • domains (line 464): formData.domains is a UUID string, cast as EntityReference. Accessing .fullyQualifiedName returns undefined, so CreateDataProduct.domains becomes [undefined].
  • tags (line 444): formData.tags is string[] of FQNs, cast as TagLabel[]. The submitted tags will be plain strings, not TagLabel objects.
  • owners (line 447): formData.owners is string[] of UUIDs, cast as EntityReference[]. Submitted as raw strings instead of entity references.
  • experts (line 446): Same issue — item.name on a string yields undefined, so all experts become ''.

This is a fundamental mismatch introduced by switching from Ant Design's form (which stored full objects in field values) to the new autocomplete (which stores only ID strings). The submit handler needs to map IDs back to full objects using the option lists.

...and 4 more resolved from earlier reviews

Options

Display: compact → Showing less information.

Comment with these commands to change:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@sonarqubecloud
Copy link
Copy Markdown

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

Labels

safe to test Add this label to run secure Github workflows on PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.