Add export support for classification and tag level - #31938
Conversation
anuj-kumary
added
the
safe to test
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
| Count | Rule |
|---|---|
| 4 | react-hooks/exhaustive-deps |
| 2 | sonarjs/cyclomatic-complexity |
| 1 | sonarjs/expression-complexity |
| 1 | openmetadata-imports/no-rest-ui-imports |
| 1 | openmetadata-imports/no-internal-barrel-imports |
All findings
| Location | Rule | Message | |
|---|---|---|---|
| 🟡 | src/components/Classifications/ClassificationDetails/ClassificationDetails.tsx:115:5 |
sonarjs/cyclomatic-complexity |
{"message":"Function has a complexity of 15 which is greater than 10 authorized.","cost":5,"secondaryLocations":[{"line":115,"column":4,"endLine":115,"endColumn |
| 🟡 | src/components/Classifications/ClassificationDetails/ClassificationDetails.tsx:210:8 |
react-hooks/exhaustive-deps |
React Hook useCallback has missing dependencies: 'isVersionView' and 'navigate'. Either include them or remove the dependency array. |
| 🟡 | src/components/Classifications/ClassificationDetails/ClassificationDetails.tsx:219:20 |
sonarjs/cyclomatic-complexity |
{"message":"Function has a complexity of 11 which is greater than 10 authorized.","cost":1,"secondaryLocations":[{"line":219,"column":19,"endLine":219,"endColum |
| 🟡 | src/components/Classifications/ClassificationDetails/ClassificationDetails.tsx:306:9 |
sonarjs/expression-complexity |
Reduce the number of conditional operators (5) used in the expression (maximum allowed 3). |
| 🟡 | src/components/Classifications/ClassificationDetails/ClassificationDetails.tsx:374:8 |
react-hooks/exhaustive-deps |
React Hook useMemo has a missing dependency: 't'. Either include it or remove the dependency array. |
| 🟡 | src/components/Classifications/ClassificationDetails/ClassificationDetails.tsx:446:7 |
react-hooks/exhaustive-deps |
React Hook useMemo has a missing dependency: 't'. Either include it or remove the dependency array. |
| 🟡 | src/components/Classifications/ClassificationDetails/ClassificationDetails.tsx:496:8 |
react-hooks/exhaustive-deps |
React Hook useEffect has missing dependencies: 'fetchClassificationChildren', 'handlePageChange', and 'isAddingTag'. Either include them or remove the dependenc |
| 🟡 | src/rest/tagAPI.ts:17:1 |
openmetadata-imports/no-rest-ui-imports |
REST clients must not import UI or state-layer modules. Move shared request/response types to the interface layer. |
| 🟡 | src/rest/tagAPI.ts:27:1 |
openmetadata-imports/no-internal-barrel-imports |
Import the internal module directly instead of its index barrel so unrelated siblings do not enter the bundle graph. |
Fix locally (fast - only checks files changed in this branch):
make ui-checkstyle-changed
…tag-level export test
- Use the required-field marker 'name*' in ClassificationResourceIT import
CSV headers so validateHeaders accepts the file; previously the header
mismatch aborted every import (ApiStatus.ABORTED, 0 rows, tag never
created), failing the three import tests.
- Remove the tag-level export test from TagResourceIT; export/import is
supported at the classification level only, so /v1/tags/name/{fqn}/export
does not exist and the test 404'd.
- Fix classificationCsvDocumentation.json owner and reviewers examples to
the runtime 'type:name' colon format (e.g. team:marketing, user:john),
matching addOwners/addReviewers export output.
… import
The CSV import override looked up the existing tag with Include.NON_DELETED to
preserve mutuallyExclusive. On a Redis-cached backend that lookup marks the
(not-yet-created) tag FQN in the not-found cache. The batch create path
(createManyEntitiesForImport) writes the entity through to Redis but, unlike
the batch update path, never invalidates the not-found markers, so the stale
negative marker survives and the immediate GET /tags/name/{fqn} returns 404.
Use Include.ALL, matching EntityRepository.findMatchForImport, which never
touches the NON_DELETED negative cache. Verified locally on the
postgres+elasticsearch+redis lane: ClassificationResourceIT 215 tests pass
(0 errors) with the fix; reverting to NON_DELETED reproduces 3 import-test
failures with 'tag ... not found' 404s.
The CSV import built a fresh Tag from the 11 CSV columns, so any tag field not represented in the CSV (recognizers, autoClassificationEnabled/Priority, deprecated, ...) was reset to its schema default on re-import - e.g. a tag's auto-classification config was silently wiped by an export/edit/import cycle. On update, start from the stored tag and overlay only the CSV columns, so every non-CSV field is retained automatically - including any field added to the tag schema later, with no per-field handling. Verified on the postgres+elasticsearch+redis lane: autoClassificationEnabled/Priority now survive a round-trip (were true/7 -> false/50 before); full ClassificationResourceIT green. Adds a regression test.
Code Review ✅ Approved 4 resolved / 4 findingsAdds classification and tag level CSV export and import support across the service and UI, resolving tag lookup issues and preserving non-CSV fields. No issues found. ✅ 4 resolved✅ Quality: No tests for new classification/tag CSV export logic
✅ Edge Case: Backend import lacks system-classification guard enforced only in UI
✅ Edge Case: Import silently defaults mutuallyExclusive to false on empty cell
✅ Quality: ClassificationCsv stores classification but never uses it
OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |
|
|
4 participants
Describe your changes:
Fixes #23053
Add export support for classification and tag level
Screen.Recording.2026-08-24.at.1.17.32.PM.mov
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.Greptile Summary
This PR adds classification-level CSV import and export across the service API and classification UI.
Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains.
Important Files Changed
Sequence Diagram
sequenceDiagram actor User participant UI as Classification UI participant API as ClassificationResource participant Repo as ClassificationRepository participant CSV as ClassificationCsv participant Tags as TagRepository alt Export User->>UI: Select Export UI->>API: "GET /classifications/name/{name}/exportAsync" API->>Repo: exportToCsv(name, user) Repo->>Tags: listAllForCSV(...) Tags-->>Repo: Classification tags Repo->>CSV: exportCsv(tags) CSV-->>User: CSV export job/result else Import User->>UI: Upload CSV UI->>API: "PUT /classifications/name/{name}/import" API->>Repo: importFromCsv(name, csv, dryRun, user) Repo->>CSV: importCsv(...) CSV->>Tags: Create or update tags Tags-->>CSV: Import results CSV-->>User: Validation/import result endReviews (16): Last reviewed commit: "Merge branch 'main' into issue-23053" | Re-trigger Greptile