Skip to content

Fix #25457 - Nested columns not updating in real time#25624

Open
aniketkatkar97 wants to merge 18 commits intomainfrom
fix-25457
Open

Fix #25457 - Nested columns not updating in real time#25624
aniketkatkar97 wants to merge 18 commits intomainfrom
fix-25457

Conversation

@aniketkatkar97
Copy link
Member

@aniketkatkar97 aniketkatkar97 commented Jan 29, 2026

Fixes #25457

This pull request introduces comprehensive improvements for supporting and testing updates to deeply nested columns in tables, particularly focusing on immediate UI updates (without page refresh) for descriptions, tags, and display names. The changes include new Playwright end-to-end tests, enhancements to the SchemaTable component logic, and expanded unit test coverage for nested column structures.

Key changes:

End-to-end Testing for Nested Columns

  • Added a new Playwright test suite, NestedColumnUpdates.spec.ts, which verifies that updating descriptions, tags, and display names for both level 1 and deeply nested columns takes effect immediately in the UI without requiring a page refresh.

SchemaTable Component Enhancements

  • Refactored the updateColumnDetails function in SchemaTable.component.tsx to use the new updateColumnInNestedStructure utility for updating columns, ensuring correct updates for nested column structures. The function signature was simplified, and all update call sites were updated accordingly. [1] [2] [3] [4]

Nested Columns Unit Testing

  • Extended SchemaTable.test.tsx with new mock data containing nested columns and new tests to verify rendering, structure, and update logic for nested columns. Also ensured that updateColumnInNestedStructure is available and tested. [1] [2] [3]

Utilities and Test Improvements

  • Updated the Playwright utility updateDisplayNameForEntityChildren to filter out analytics requests when waiting for column update responses, making tests more robust.

Cleanup

  • Removed the obsolete TableUtils.test.ts file, as nested column logic and tests have been moved or are now covered elsewhere.

@aniketkatkar97 aniketkatkar97 self-assigned this Jan 29, 2026
@aniketkatkar97 aniketkatkar97 requested a review from a team as a code owner January 29, 2026 14:56
@aniketkatkar97 aniketkatkar97 added the To release Will cherry-pick this PR into the release branch label Jan 29, 2026
@github-actions github-actions bot added safe to test Add this label to run secure Github workflows on PRs UI UI specific issues labels Jan 29, 2026
@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 6, 2026

@shah-harshit
Copy link
Contributor

LGTM!

sonika-shah
sonika-shah previously approved these changes Feb 6, 2026
}, [permissions, fileDetails]);

const schema = pruneEmptyChildren(fileDetails?.columns ?? []);
const schema = fileDetails?.columns;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are we removing pruneEmptyChildren

columns={columns}
data-testid="file-columns-table"
dataSource={schema}
dataSource={pruneEmptyChildren(schema ?? [])}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we avoid it on render? it will run multiple times

Comment on lines +1388 to +1392
field?: string
): Column[] => {
return columns.map((column: Column) => {
if (column.fullyQualifiedName === targetFqn) {
const newCol = omit(column, field ?? '');
Copy link

Choose a reason for hiding this comment

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

⚠️ Bug: omit(column, '') is a no-op but masks missing field argument

When field is undefined, the code calls omit(column, ''), which is a no-op since no Column has a key of empty string. This works accidentally but doesn't match the intent expressed by the original code comment: "Have to omit the field which is being updated to avoid persisted old value."

The field parameter is typed as string | undefined, but some callers (e.g., SchemaTable.component.tsx) changed its signature to field: keyof Column (required). However, the utility function's own signature still has field?: string (optional).

If any caller omits field, the old stale value won't be omitted from the column before spreading the update, potentially causing stale data to persist. Consider either:

  1. Making field required in updateColumnInNestedStructure to match callers
  2. Or skipping the omit call entirely when field is undefined instead of passing an empty string, making the intent clearer:
const newCol = field ? omit(column, field) : column;

Was this helpful? React with 👍 / 👎

@gitar-bot
Copy link

gitar-bot bot commented Feb 6, 2026

🔍 CI failure analysis for 0627999: NEW FAILURE: maven-sonarcloud-ci failed with 4 test failures (1 GlossaryTermResourceTest, 3 AwsCredentialsUtilTest), causing Test Report to fail. Previous findings remain: ALL Playwright PASSED, PostgreSQL+OpenSearch PASSED. All failures unrelated to nested column functionality.

Issue

NEW: maven-sonarcloud-ci failed with 4 test failures (job 62787464470), causing Test Report to fail (job 62806392378).

Previous: Multiple unrelated failures from earlier analysis:

  1. maven-postgresql-ci: 5 test failures (2 FeedResourceTest, 3 AwsCredentialsUtilTest)
  2. py-run-tests (3.10): 7 Trino classifier setup errors (98.7% success)
  3. integration-tests-mysql-elasticsearch: Disk space exhausted

Root Cause

All failures are unrelated to this PR's nested column functionality.


NEW FAILURE: Maven SonarCloud CI

Test Failures Summary

maven-sonarcloud-ci (job 62787464470)

  • Tests Run: 7,918 tests
  • Failed: 1 test failure + 3 test errors = 4 total
  • Skipped: 701 tests
  • Success Rate: 99.95%
  • Duration: 3 hours 11 minutes
  • SonarCloud Analysis: NOT EXECUTED (build failed before analysis)

Failed Tests Details

1. GlossaryTermResourceTest (1 failure) - NEW

File: openmetadata-service/src/test/java/org/openmetadata/service/resources/glossary/GlossaryTermResourceTest.java:2290

Test: test_GlossaryTermWorkflow_TermHasReviewers_UpdatedByNotReviewer_WorkflowTriggered

Error:

org.opentest4j.AssertionFailedError: Term should be approved after task resolution 
==> expected: <Approved> but was: <In Review>

Root Cause:

  • Glossary term workflow state transition issue
  • Test expects term to transition to "Approved" after task resolution
  • Term remains stuck in "In Review" status
  • Indicates approval process not completing when task resolved by non-reviewer

2. AwsCredentialsUtilTest (3 errors) - SAME AS POSTGRESQL CI

File: openmetadata-service/src/test/java/org/openmetadata/service/util/AwsCredentialsUtilTest.java

All 3 tests failing at AwsCredentialsUtil.buildBaseProvider():48 with:

java.lang.IllegalArgumentException: AWS credentials not configured. 
Either provide accessKeyId/secretAccessKey or set enabled=true to use 
IAM authentication via the default credential provider chain.

Failed Tests:

  1. testBuildCredentialsProviderWithNoCredentials at line 52
  2. testBuildCredentialsProviderWithOnlyAccessKey at line 64
  3. testBuildCredentialsProviderWithEmptyCredentials at line 77

Root Cause:

  • AwsCredentialsUtil.buildBaseProvider() throws IllegalArgumentException for missing/incomplete credentials
  • Tests expect graceful handling but implementation changed to throw exceptions
  • SAME 3 failures as maven-postgresql-ci

Why Unrelated to PR #25624

This PR modifies:

  • FileRepository.java (File entity PATCH operations for nested columns ONLY)
  • Frontend components (SchemaTable, FileColumnsTable, ModelTab)
  • Playwright test files for nested column updates
  • Frontend utilities (TableUtils)

Does NOT touch:

  • Glossary term workflow logic (GlossaryTermResource.java, approval workflows)
  • Task resolution functionality
  • Reviewer/approval state transitions
  • AWS credentials utilities (AwsCredentialsUtil.java)
  • Any glossary or workflow code

Evidence:

  • PR modifies only 1 Java file: FileRepository.java (File entity operations)
  • GlossaryTermResourceTest.java NOT in PR diff
  • AwsCredentialsUtilTest.java NOT in PR diff
  • Glossary term workflow completely separate from File entity nested columns
  • AWS credentials logic unrelated to File entity PATCH operations
  • Commit 0627999 is merge from main - failures likely from main branch changes

Pattern Across CI Jobs:

  • Maven MySQL CI: PASSED (no test failures)
  • Maven PostgreSQL CI: 5 test failures (FeedResource + AwsCredentials)
  • Maven SonarCloud CI: 4 test failures (GlossaryTermResource + AwsCredentials)
  • AwsCredentialsUtilTest failures consistent across PostgreSQL and SonarCloud CI
  • Confirms failures from environment/main branch, not PR code

Test Report Failure (job 62806392378)

Status: FAILED (dependent job)

Root Cause:

  • Test Report job aggregates results from all test jobs
  • Failed because maven-sonarcloud-ci reported 4 test failures
  • No independent failure - purely a reporting job
  • Duration: <1 second (immediate failure)

EXCELLENT NEWS: Major CI Successes (From Previous Analysis)

Playwright CI - ALL SHARDS PASSED

  • Shard 1/6: PASSED ✅
  • Shard 2/6: PASSED ✅
  • Shard 3/6: PASSED ✅
  • Shard 4/6: PASSED ✅
  • Shard 5/6: PASSED ✅
  • Shard 6/6: PASSED ✅

Impact:

  • ALL Playwright tests passing
  • Configuration fixes from merge successful
  • No nested column test failures

Other Successes

  • Maven MySQL CI: PASSED (all tests) ✅
  • Maven Collate CI: PASSED ✅
  • PostgreSQL+OpenSearch integration: PASSED (9,017+ tests) ✅
  • UI coverage tests: PASSED ✅
  • Python tests 3.11: PASSED ✅
  • Python build tests: PASSED ✅
  • All code analysis: PASSED ✅
  • CodeQL: PASSED ✅
  • Python checkstyle: PASSED ✅

Complete CI Status Summary

Playwright CI:

  • ALL 6 SHARDS: PASSED ✅

Integration Tests:

  • PostgreSQL+OpenSearch: PASSED (9,017+ tests) ✅
  • MySQL+Elasticsearch: Disk space exhausted (transient) ⚠️

Backend CI:

  • Maven MySQL: PASSED ✅
  • Maven Collate: PASSED ✅
  • Maven PostgreSQL: FAILED (5 test failures: 2 FeedResource, 3 AwsCredentials, 99.94% success) ❌
  • Maven SonarCloud: FAILED (4 test failures: 1 GlossaryTerm, 3 AwsCredentials, 99.95% success) ❌
  • Test Report: FAILED (dependent on SonarCloud CI) ❌

Python Tests:

  • py-run-tests (3.11): PASSED ✅
  • py-run-tests (3.10): 7 Trino classifier setup errors (98.7% success, unrelated) ⚠️
  • py-run-build-tests: PASSED ✅
  • py-checkstyle: PASSED ✅

Code Quality:

  • All analysis jobs: PASSED ✅
  • CodeQL: PASSED ✅
  • UI coverage: PASSED ✅

Failure Pattern Analysis

Common Failures Across CI Jobs

AwsCredentialsUtilTest (3 errors):

  • Appears in BOTH maven-postgresql-ci AND maven-sonarcloud-ci
  • Same 3 tests failing with identical error
  • Pattern confirms issue from main branch merge, not PR-specific

Database/Environment-Specific Failures:

  • Maven MySQL CI: ZERO test failures ✅
  • Maven PostgreSQL CI: 5 failures (FeedResource + AwsCredentials)
  • Maven SonarCloud CI: 4 failures (GlossaryTerm + AwsCredentials)
  • Different test failures across environments suggest environmental/config issues

Summary

PR Status: EXCELLENT - Nested Column Functionality Fully Working

Critical Success:

  • ALL Playwright shards (1-6): PASSED ✅
  • PostgreSQL+OpenSearch integration: PASSED (9,017+ tests) ✅
  • Maven MySQL CI: PASSED (all tests) ✅
  • NO nested column test failures across ANY test suite

Unrelated Failures (from main branch merge):

  1. Maven SonarCloud CI: 4 test failures (99.95% success)

    • GlossaryTermResourceTest: Workflow state transition issue
    • AwsCredentialsUtilTest: 3 credential validation errors
    • NOT in PR diff, from main branch
  2. Maven PostgreSQL CI: 5 test failures (99.94% success)

    • FeedResourceTest: Feed/mention filtering issues
    • AwsCredentialsUtilTest: 3 credential validation errors (same as above)
    • NOT in PR diff, from main branch
  3. Test Report: Failed (dependent on SonarCloud CI)

  4. py-run-tests (3.10): 7 Trino classifier setup errors (98.7% success, no Python files in PR)

  5. MySQL+Elasticsearch integration: Disk space exhausted (transient runner issue)

Commit 0627999 Impact:

  • Playwright configuration fixes: SUCCESSFUL ✅
  • Flaky test fixes from main: Working ✅
  • Merge from main introduced multiple test failures (GlossaryTerm, FeedResource, AwsCredentials) - not from PR code
  • AwsCredentialsUtilTest failures consistent across multiple CI jobs confirms main branch origin

Conclusion:
PR #25624 successfully implements nested column real-time updates with zero failures in nested column functionality. The test failures across maven-postgresql-ci (5 failures) and maven-sonarcloud-ci (4 failures) are unrelated to this PR and originated from the main branch merge. AwsCredentialsUtilTest failures appearing in multiple CI jobs confirm they're from main branch changes, not PR code. Maven MySQL CI passes completely with zero failures. All nested column tests pass across all platforms.

Code Review 👍 Approved with suggestions 2 resolved / 5 findings

Solid implementation of nested column real-time updates. The SearchIndexFieldsTable data-change-during-search issue has been properly resolved by splitting the useEffect. The omit(column, '') no-op concern remains valid at the utility function level despite callers now always providing the field parameter. Test cleanup and optional chaining concerns from the previous review are still present but minor.

⚠️ Bug: omit(column, '') is a no-op but masks missing field argument

📄 openmetadata-ui/src/main/resources/ui/src/utils/TableUtils.tsx:1388-1392

When field is undefined, the code calls omit(column, ''), which is a no-op since no Column has a key of empty string. This works accidentally but doesn't match the intent expressed by the original code comment: "Have to omit the field which is being updated to avoid persisted old value."

The field parameter is typed as string | undefined, but some callers (e.g., SchemaTable.component.tsx) changed its signature to field: keyof Column (required). However, the utility function's own signature still has field?: string (optional).

If any caller omits field, the old stale value won't be omitted from the column before spreading the update, potentially causing stale data to persist. Consider either:

  1. Making field required in updateColumnInNestedStructure to match callers
  2. Or skipping the omit call entirely when field is undefined instead of passing an empty string, making the intent clearer:
const newCol = field ? omit(column, field) : column;
💡 Quality: Missing test cleanup for entities created in beforeAll

📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Flow/NestedChildrenUpdates.spec.ts:33

The test suite creates entities in test.beforeAll (line 33-38) using entity.create(apiContext), but there's no corresponding test.afterAll to clean up these created entities. This can lead to test data accumulation over time and potential test pollution.

Impact:

  • Leftover test data in the database after test runs
  • Potential flakiness if entity names collide across test runs

Suggested fix:
Add a test.afterAll block to delete the created entities:

test.afterAll(async ({ browser }) => {
  const { apiContext, afterAction } = await createNewPage(browser);
  await entity.delete(apiContext);
  await afterAction();
});

Note: This is the same issue as the previous finding but in the renamed file (previously NestedColumnUpdates.spec.ts, now NestedChildrenUpdates.spec.ts).

💡 Edge Case: Unsafe optional chaining in getNestedColumnDetails may cause test failures

📄 openmetadata-ui/src/main/resources/ui/playwright/utils/nestedColumnUpdatesUtils.ts:31

In nestedColumnUpdatesUtils.ts, the getNestedColumnDetails function chains multiple optional accesses to build FQN keys (e.g., entity.children?.[0].children?.[0].children?.[0].name). If any intermediate level is undefined (e.g., the entity's children structure doesn't match expectations), the key will contain the literal string "undefined", leading to cryptic test failures where selectors silently don't match any element.

For example:

const level2Key = `${level1Key}.${entity.children?.[0].children?.[0].children?.[0].name}`;
// If children?.[0].children?.[0].children is undefined, level2Key becomes "...undefined"

Consider adding a guard/assertion at the top of each case to fail fast with a clear message if the expected nested structure is missing, e.g.:

if (!entity.children?.[0]?.children?.[0]?.children?.[0]) {
  throw new Error(`Expected 3-level nested structure for ${type}`);
}
✅ 2 resolved
Edge Case: SearchIndexFieldsTable ignores data changes during active search

📄 openmetadata-ui/src/main/resources/ui/src/pages/SearchIndexDetailsPage/SearchIndexFieldsTable/SearchIndexFieldsTable.tsx:398
The useEffect split in SearchIndexFieldsTable.tsx introduces a behavioral regression: when searchText is non-empty and searchIndexFields changes (e.g., a nested column update is made while searching), the searched results won't reflect the updated data.

Before (old code):

useEffect(() => {
  if (searchText) {
    // re-runs search against updated data
    const searchFields = searchInFields(sortByOrdinalPosition, searchText);
    setSearchedFields(searchFields);
    setExpandedRowKeys(fieldAllRowKeys);
  } else {
    setSearchedFields(sortByOrdinalPosition);
    setExpandedRowKeys([]);
  }
}, [searchText, searchIndexFields]);

After (new code):
The first effect only triggers on [searchText] changes, and the second effect only syncs when !searchText. So if a user edits a nested column while a search filter is active, the table won't update until the search text is cleared or changed.

Suggested fix: Add searchIndexFields / sortByOrdinalPosition back to the first effect's dependency array for the searchText truthy branch, or add a third effect that re-runs the search when fields change and searchText is present:

useEffect(() => {
  if (searchText) {
    const searchFields = searchInFields(sortByOrdinalPosition, searchText);
    setSearchedFields(searchFields);
  }
}, [sortByOrdinalPosition]);
Quality: Missing test cleanup in NestedColumnUpdates.spec.ts

📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Flow/NestedColumnUpdates.spec.ts:35
The Playwright E2E test file NestedColumnUpdates.spec.ts creates a test table entity in test.beforeAll but doesn't have a corresponding test.afterAll to clean up the created entity after the tests complete.

This can lead to:

  1. Test data accumulation over time
  2. Potential test pollution if entity names are not sufficiently unique
  3. Inconsistent test environment state

Suggested fix:
Add a test.afterAll block to delete the created table:

test.afterAll('Cleanup entities', async ({ browser }) => {
  const { apiContext, afterAction } = await createNewPage(browser);
  await tableWithNestedColumns.delete(apiContext);
  await afterAction();
});

Looking at other similar test files in the OpenMetadata codebase, this is a common pattern for proper test cleanup.

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 6, 2026

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 To release Will cherry-pick this PR into the release branch UI UI specific issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Nested column description does not refresh after edit until page reload

5 participants