Fix #25457 - Nested columns not updating in real time#25624
Fix #25457 - Nested columns not updating in real time#25624aniketkatkar97 wants to merge 18 commits intomainfrom
Conversation
…and added test coverage for all entities
|
|
LGTM! |
| }, [permissions, fileDetails]); | ||
|
|
||
| const schema = pruneEmptyChildren(fileDetails?.columns ?? []); | ||
| const schema = fileDetails?.columns; |
There was a problem hiding this comment.
Why are we removing pruneEmptyChildren
| columns={columns} | ||
| data-testid="file-columns-table" | ||
| dataSource={schema} | ||
| dataSource={pruneEmptyChildren(schema ?? [])} |
There was a problem hiding this comment.
Can we avoid it on render? it will run multiple times
| field?: string | ||
| ): Column[] => { | ||
| return columns.map((column: Column) => { | ||
| if (column.fullyQualifiedName === targetFqn) { | ||
| const newCol = omit(column, field ?? ''); |
There was a problem hiding this comment.
⚠️ 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:
- Making
fieldrequired inupdateColumnInNestedStructureto match callers - Or skipping the
omitcall entirely whenfieldis undefined instead of passing an empty string, making the intent clearer:
const newCol = field ? omit(column, field) : column;Was this helpful? React with 👍 / 👎
🔍 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.IssueNEW: maven-sonarcloud-ci failed with 4 test failures (job 62787464470), causing Test Report to fail (job 62806392378). Previous: Multiple unrelated failures from earlier analysis:
Root CauseAll failures are unrelated to this PR's nested column functionality. NEW FAILURE: Maven SonarCloud CITest Failures Summarymaven-sonarcloud-ci (job 62787464470)
Failed Tests Details1. GlossaryTermResourceTest (1 failure) - NEWFile: Test: Error: Root Cause:
2. AwsCredentialsUtilTest (3 errors) - SAME AS POSTGRESQL CIFile: All 3 tests failing at 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:
Root Cause:
Why Unrelated to PR #25624This PR modifies:
Does NOT touch:
Evidence:
Pattern Across CI Jobs:
Test Report Failure (job 62806392378)Status: FAILED (dependent job) Root Cause:
EXCELLENT NEWS: Major CI Successes (From Previous Analysis)Playwright CI - ALL SHARDS PASSED
Impact:
Other Successes
Complete CI Status SummaryPlaywright CI:
Integration Tests:
Backend CI:
Python Tests:
Code Quality:
Failure Pattern AnalysisCommon Failures Across CI JobsAwsCredentialsUtilTest (3 errors):
Database/Environment-Specific Failures:
SummaryPR Status: EXCELLENT - Nested Column Functionality Fully Working Critical Success:
Unrelated Failures (from main branch merge):
Commit 0627999 Impact:
Conclusion: Code Review 👍 Approved with suggestions 2 resolved / 5 findingsSolid implementation of nested column real-time updates. The
|
| Auto-apply | Compact |
|
|
Was this helpful? React with 👍 / 👎 | Gitar
|



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
SchemaTablecomponent logic, and expanded unit test coverage for nested column structures.Key changes:
End-to-end Testing for Nested Columns
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
updateColumnDetailsfunction inSchemaTable.component.tsxto use the newupdateColumnInNestedStructureutility 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
SchemaTable.test.tsxwith new mock data containing nested columns and new tests to verify rendering, structure, and update logic for nested columns. Also ensured thatupdateColumnInNestedStructureis available and tested. [1] [2] [3]Utilities and Test Improvements
updateDisplayNameForEntityChildrento filter out analytics requests when waiting for column update responses, making tests more robust.Cleanup
TableUtils.test.tsfile, as nested column logic and tests have been moved or are now covered elsewhere.