fix(ui): handle malformed JSON in version diff payloads (#28154) - #33580
hadimobarra wants to merge 1 commit into
Conversation
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
Code Review ✅ Approved🟡 Medium risk Adds defensive handling for malformed JSON in version diff payloads to prevent parsing failures. No issues found. OptionsDisplay: compact → Counting what did not apply, without listing it. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |
|
Hi @hadimobarra Can you please share issue details in the Github Issue and link it with PR? |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| : {}; | ||
|
|
||
| const value = newValueJSON ?? oldValueJSON; | ||
| const value = fieldChange.newValue ?? fieldChange.oldValue; |
There was a problem hiding this comment.
When a deleted field has an empty or malformed newValue but a valid oldValue, newValue ?? oldValue selects the unusable value before parsing. The parser then returns an empty array or object, so column, tag, and owner deletion summaries omit the deleted names even though valid data is available in oldValue. Select the first value that can be decoded successfully instead.
| const oldDiff = parseVersionDiffArray<MlFeature>( | ||
| getChangedEntityOldValue(featureDiff), | ||
| isMlFeatureDiff | ||
| ); | ||
| const newDiff = parseVersionDiffArray<MlFeature>( | ||
| getChangedEntityNewValue(featureDiff), | ||
| isMlFeatureDiff | ||
| ); |
There was a problem hiding this comment.
The old and new ML-feature arrays are filtered independently, but the comparison helpers still pair their entries by array index. If only one side contains a malformed feature among valid entries, later entries shift and a valid feature is compared with another feature's old description or tags. This makes the version history show incorrect changes. Pair features by their stable name instead of their post-filter index.
2 participants
Describe your changes:
Fixes #
I worked on ... because ...
Type of change:
High-level design:
N/A — small change.
Tests:
Use cases covered
Unit tests
Backend integration tests
Ingestion integration tests
Playwright (UI) tests
Manual testing performed
UI screen recording / screenshots:
Not applicable.
Checklist:
Fixes <issue-number>: <short explanation>Fixes #<issue-number>above.The PR is not yet safe to merge because malformed version data can suppress valid deletion summaries and misattribute ML-feature changes.
Findings
Summary
This PR adds validation-backed JSON parsing throughout the entity-version UI so malformed persisted diff payloads no longer crash version pages. It also corrects search-index field lookup and permits numeric version values.
Diagram
%%{init: {'theme': 'neutral'}}%% flowchart LR A[Persisted version diff JSON] --> B[Safe JSON parser] B --> C{Expected shape?} C -->|No| D[Discard malformed payload or item] C -->|Yes| E[Typed diff data] E --> F[Entity-specific version formatter] F --> G[Version timeline and detail UI]Reviews (1) · Last reviewed commit: "fix(ui): handle malformed JSON in versio..."