Skip to content

Replaced old entity delete modal with untitles delete modal#29760

Open
anuj-kumary wants to merge 20 commits into
mainfrom
issue-29730-oss
Open

Replaced old entity delete modal with untitles delete modal#29760
anuj-kumary wants to merge 20 commits into
mainfrom
issue-29730-oss

Conversation

@anuj-kumary

@anuj-kumary anuj-kumary commented Jul 6, 2026

Copy link
Copy Markdown
Member

Describe your changes:

Fixes #29730

Replaced old entity delete modal with untitles delete modal

Screen.Recording.2026-07-07.at.3.36.41.PM.mov
Screen.Recording.2026-07-07.at.3.37.17.PM.mov

Type of change:

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

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:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • My PR is linked to a GitHub issue via Fixes #<issue-number> above.
  • 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.
  • For UI changes: I attached a screen recording and/or screenshots above.
  • I have added tests (unit / integration / Playwright as applicable) and listed them above.

Summary by Gitar

  • Refactored UI component:
    • Replaced DeleteWidgetModal with the new DeleteEntityModal component across various pages and components.
  • New component implementation:
    • Introduced DeleteEntityModal.tsx and its corresponding test file DeleteEntityModal.test.tsx to standardize entity deletion workflows.
  • Internationalization:
    • Added new locale keys for permanently-delete-common-message and soft-delete-common-message to support the new modal's messaging.
  • Testing updates:
    • Updated various unit tests (DataQualityTab.test.tsx, ManageButton.test.tsx, AlertDetailsPage.test.tsx, etc.) to reflect the component migration.
    • Updated Playwright e2e tests to align with the new deletion modal implementation.

This will update automatically on new commits.

Greptile Summary

This PR replaces DeleteWidgetModal (Ant Design-based, requires typing "DELETE" to confirm) with a new DeleteEntityModal component (from @openmetadata/ui-core-components, radio-button selection only) across the majority of entity deletion flows, and with the simpler DeleteModal for confirmation-only cases (glossaries, tags, classifications). Playwright E2E tests and unit tests are updated accordingly to remove the text-fill step.

  • New DeleteEntityModal.tsx: Radio-group for soft/hard delete, no text confirmation, try/finally cleanup on both the sync and async delete paths, and a data-testid="delete-modal" anchor for tests. DeleteWidgetModal.tsx is deleted.
  • Mixed migration pattern: ManageButton and entity list pages (Roles, Policies, Users, KPI, etc.) migrate to DeleteEntityModal; entity detail pages with custom delete logic (GlossaryHeader, GlossaryV1, TagPage, TagsPage) and DataQualityTab migrate to the simpler DeleteModal with a local onDelete handler.
  • Playwright updates: Stale Ant Design class selectors (.ant-modal, .ant-modal-title) and the old hard-delete-option data-testids are replaced with delete-modal, hard-delete, etc.; a new fillDeleteConfirmationIfPresent guard is added for flows that still use the old confirmation-input modal (ingestion pipelines).

Confidence Score: 4/5

Safe to merge with minor caveats — the deletion UX change is broad but well-structured and the async cleanup paths are correct.

The migration spans 94 files and correctly addresses the stale Ant Design selectors, the async delete try/finally cleanup, and the removal of text confirmation across all migrated paths. The mixed pattern (DeleteEntityModal vs DeleteModal + local handler) is internally consistent. Two small gaps: DataQualityTab's local handleDeleteTestCase passes undefined instead of false for the isSoftDelete callback arg, and softDeleteEntity in the Playwright utils relies entirely on the modal's default selection rather than explicitly clicking the soft-delete radio.

DataQualityTab.tsx (afterDeleteAction argument), entity.ts softDeleteEntity helper (implicit default reliance), and Tags.spec.ts (mixed DeleteModal/DeleteEntityModal paths).

Important Files Changed

Filename Overview
openmetadata-ui/src/main/resources/ui/src/components/common/DeleteWidget/DeleteEntityModal.tsx New component replacing DeleteWidgetModal. Radio-group UX with try/finally cleanup in both sync and async paths. No text confirmation required. Minor: afterDeleteAction receives isSoftDelete=undefined (not false) for hard-delete flows that previously passed allowSoftDelete=false.
openmetadata-ui/src/main/resources/ui/playwright/utils/entity.ts Stale .ant-modal / .ant-modal-title selectors removed; hard-delete-option replaced with hard-delete. softDeleteEntity now relies on the modal's default radio selection (correct per useEffect reset). New fillDeleteConfirmationIfPresent helper guards mixed-modal flows.
openmetadata-ui/src/main/resources/ui/src/components/Database/Profiler/DataQualityTab/DataQualityTab.tsx Migrated to DeleteModal with a local handleDeleteTestCase handler that calls deleteEntity directly. Hard-delete path is preserved (hardcoded params match old allowSoftDelete=false behavior). afterDeleteAction now called with no arguments (isSoftDelete=undefined instead of false).
openmetadata-ui/src/main/resources/ui/playwright/utils/serviceIngestion.ts hard-delete-option replaced with hard-delete; redundant service-name text click removed. Simple and correct.
openmetadata-ui/src/main/resources/ui/playwright/utils/testCases.ts confirmIngestionPipelineHardDelete now sets up waitForResponse before filling the input (correct ordering); deleteTestCase no longer needs to fill text and now just waits for confirm-button.
openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/Tags.spec.ts permanentDeleteModal waits for modal-footer (present on DeleteModal) and confirm-button (present on both modals). Classification delete-button path now clicks hard-delete radio (correct since this goes through ManageButton → DeleteEntityModal).
openmetadata-ui/src/main/resources/ui/src/components/common/EntityPageInfos/ManageButton/ManageButton.tsx Simple drop-in replacement of DeleteWidgetModal with DeleteEntityModal using the same props. All callers continue to pass the same prop set.
openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/Glossary/GlossaryMiscOperations.spec.ts Uses fillDeleteConfirmationIfPresent guard; waitForResponse is set up before the helper is called ensuring no race. Correct for both async glossary/term delete via ManageButton → DeleteEntityModal.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[User clicks Delete] --> B{Which component?}

    B --> C[ManageButton / Entity list pages\nRoles, Policies, Users, KPI,\nDomainDetails, BotList, etc.]
    B --> D[Simple entity delete\nGlossaryHeader, GlossaryV1,\nTagPage, TagsPage]
    B --> E[DataQualityTab\ntest case delete]

    C --> F[DeleteEntityModal\nRadio: soft-delete / hard-delete\nNo text confirmation\ndata-testid=delete-modal]
    D --> G[DeleteModal\nSimple confirm\ndata-testid=modal-footer\ndata-testid=cancel-button]
    E --> H[DeleteModal\n+ local handleDeleteTestCase\ncalls deleteEntity directly\nhardDelete=true]

    F --> I{isAsyncDelete?}
    I -- Yes --> J[handleOnAsyncEntityDeleteConfirm\ntry/finally cleanup]
    I -- No --> K{onDelete prop?}
    K -- Yes --> L[Custom onDelete handler]
    K -- No --> M[internal deleteEntity call\ntry/finally cleanup]

    style F fill:#d4edda,stroke:#28a745
    style G fill:#d1ecf1,stroke:#17a2b8
    style H fill:#d1ecf1,stroke:#17a2b8
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[User clicks Delete] --> B{Which component?}

    B --> C[ManageButton / Entity list pages\nRoles, Policies, Users, KPI,\nDomainDetails, BotList, etc.]
    B --> D[Simple entity delete\nGlossaryHeader, GlossaryV1,\nTagPage, TagsPage]
    B --> E[DataQualityTab\ntest case delete]

    C --> F[DeleteEntityModal\nRadio: soft-delete / hard-delete\nNo text confirmation\ndata-testid=delete-modal]
    D --> G[DeleteModal\nSimple confirm\ndata-testid=modal-footer\ndata-testid=cancel-button]
    E --> H[DeleteModal\n+ local handleDeleteTestCase\ncalls deleteEntity directly\nhardDelete=true]

    F --> I{isAsyncDelete?}
    I -- Yes --> J[handleOnAsyncEntityDeleteConfirm\ntry/finally cleanup]
    I -- No --> K{onDelete prop?}
    K -- Yes --> L[Custom onDelete handler]
    K -- No --> M[internal deleteEntity call\ntry/finally cleanup]

    style F fill:#d4edda,stroke:#28a745
    style G fill:#d1ecf1,stroke:#17a2b8
    style H fill:#d1ecf1,stroke:#17a2b8
Loading

Comments Outside Diff (1)

  1. openmetadata-ui/src/main/resources/ui/playwright/utils/entity.ts, line 2222-2231 (link)

    P1 Stale Ant Design selectors and missing hard-delete-option data-testid in softDeleteEntity / hardDeleteEntity

    Both softDeleteEntity (line ~2130) and hardDeleteEntity (line 2222) still assert on [role="dialog"].ant-modal and .ant-modal-title which are Ant Design class names. The new DeleteEntityModal uses @openmetadata/ui-core-components and will not render these classes, so these waitFor / expect calls will time out every time.

    Additionally, hardDeleteEntity still clicks [data-testid="hard-delete-option"] (line 2230). In the old modal this mapped to the inner Typography.Paragraph rendered as ${option.type}-option. The new RadioButton only sets data-testid={option.type} (i.e., "hard-delete"), so "hard-delete-option" no longer exists and the click will not find any element. The same stale selector is present in serviceIngestion.ts, bot.ts, user.ts, team.ts, and several spec files.

Reviews (14): Last reviewed commit: "fix lint checks" | Re-trigger Greptile

@anuj-kumary anuj-kumary self-assigned this Jul 6, 2026
@anuj-kumary anuj-kumary added the UI UI specific issues label Jul 6, 2026
@anuj-kumary anuj-kumary requested a review from a team as a code owner July 6, 2026 07:56
@anuj-kumary anuj-kumary added the safe to test Add this label to run secure Github workflows on PRs label Jul 6, 2026
Comment thread openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/Roles.spec.ts Outdated
Comment thread openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/Policies.spec.ts Outdated
The migration from EntityDeleteModal to DeleteEntityModal introduced a
dependency on Dialog, Modal, ModalOverlay, RadioButton, RadioGroup from
@openmetadata/ui-core-components. These sub-components were not covered
by the existing @openmetadata/ui-core-components mock in the test, causing
all tests to fail with "Cannot read properties of undefined (reading 'Header')".

Mock DeleteEntityModal directly (same pattern used in ManageButton.test.tsx,
CustomMetricGraphs.test.tsx, DataQualityTab.test.tsx) rather than expanding
the @openmetadata/ui-core-components mock with Dialog sub-components.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 63%
63.76% (72748/114080) 46.9% (42343/90268) 48.2% (13026/27020)

Resolves modify/delete conflict on KnowledgeCenter.spec.ts (deleted in
main as part of the KnowledgeCenter→ContextCenter rename; our branch had
a minor modification — accepted main's deletion).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
anuj-kumary and others added 2 commits July 7, 2026 10:41
…o DeleteEntityModal

Replace EntityDeleteModal (AntD type-in-DELETE modal) with DeleteEntityModal
(MUI radio-button modal) in TagPage, TagsPage, and DataProductsDetailsPage so
Playwright tests no longer time out waiting for the removed #deleteTextInput.

Also remove stale #deleteTextInput fill from testCases.ts deleteTestCase and
replace .ant-modal-title assertion in ExploreDiscovery.spec.ts with delete-modal
waitFor.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment thread openmetadata-ui/src/main/resources/ui/src/pages/TagPage/TagPage.tsx Outdated
anuj-kumary and others added 3 commits July 7, 2026 11:18
…lete flows

Replace DeleteEntityModal (radio-button modal) with the simpler DeleteModal
(single confirm button) wherever only permanent delete is offered, matching
the pattern already used by Workflow delete:

- GlossaryHeader: glossary delete
- GlossaryV1: glossary / glossary-term delete
- TagPage: tag delete
- TagsPage: classification delete
- DataProductsDetailsPage: data product delete
- DataQualityTab: test case delete (adds explicit API handler)

Also updates testCases.ts to wait for confirm-button instead of delete-modal
since DeleteModal uses modal-footer/confirm-button testids.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tion

Align tests with the DeleteModal (single-confirm) and DeleteEntityModal
(radio-button) patterns after migrating glossary, tag, and classification
delete flows:

- glossary.ts deleteGlossary: replace delete-confirmation-modal waitFor
  with delete-modal, and replace confirmation-text-input fill with
  hard-delete radio click (ManageButton → DeleteEntityModal flow)
- glossary.ts deleteGlossaryOrTerm: add hard-delete click before confirm
  so the async hard-delete endpoint is triggered correctly
- Glossary.spec.ts: remove stale body-text/confirmation-text-input checks,
  add delete-modal waitFor + hard-delete click
- Tags.spec.ts permanentDeleteModal: replace .ant-modal-content with
  modal-footer waitFor (DeleteModal uses MUI, no ant classes)
- Tags.spec.ts: replace .ant-modal-content waitFor with confirm-button
  waitFor on tag delete row action

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@gitar-bot

gitar-bot Bot commented Jul 7, 2026

Copy link
Copy Markdown
Code Review ⚠️ Changes requested 2 resolved / 3 findings

Replaces the legacy entity delete modal with a new standardized component, but the async delete path lacks error handling, causing the modal to lock up on failure. Additionally, numerous Playwright helpers still reference stale Ant Design selectors and missing data-testids.

⚠️ Bug: Merge dropped used i18n key intake-form-plural-description

📄 openmetadata-ui/src/main/resources/ui/src/locale/languages/en-us.json:3287

The most recent commit (the main merge) removes the message.intake-form-plural-description key from all 20 locale files (e.g. en-us.json line ~3287). However, that key is still referenced in src/utils/GlobalSettingsClassBase.ts (description: t('message.intake-form-plural-description')) and src/pages/IntakeForms/IntakeFormsPage.tsx (subHeader: t('message.intake-form-plural-description')). On the PR branch prior to the merge (commit 7649465) the key was present and resolved correctly; after the merge it is gone from every locale file while the two call sites remain. As a result i18next will render the raw key string message.intake-form-plural-description in the Intake Forms settings page/subheader instead of the intended text.

This looks like a merge-conflict misresolution that accidentally deleted the key. Restore the intake-form-plural-description entry in all 20 locale files (or, if the Intake Forms feature is being intentionally dropped, remove the two t(...) call sites as well) so the key and its usages stay in sync.

Re-add the accidentally-removed key to all 20 locale files (translated per language) so the two t('message.intake-form-plural-description') call sites resolve.
"intake-form-plural-description": "Define governance-policy required fields for Data Products, Domains, and Glossary Terms. Enforced at both the API and UI layers.",
✅ 2 resolved
Bug: Policies delete test still asserts on stale .ant-modal selector

📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/Policies.spec.ts
The policy delete test still asserts await expect(page.locator('[role="dialog"].ant-modal')).toBeVisible(); after clicking the delete action. This PR migrated PoliciesListPage.tsx to render the new radio-based DeleteEntityModal (imported from components/common/DeleteWidget/DeleteEntityModal), which is built on @openmetadata/ui-core-components — not Ant Design — so it does not render the .ant-modal class. The toBeVisible() assertion will time out and fail the test. Since this PR removed the accompanying page.fill('DELETE') line (that flow no longer needs typed confirmation), the stale Ant Design dialog assertion should be updated to target the new modal's DOM (e.g. page.getByTestId('confirm-button') visibility) or removed.

Bug: page.locator() called with no selector will throw

📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/Glossary/GlossaryCRUDOperations.spec.ts:411-420
In GlossaryCRUDOperations.spec.ts, the edit that removed the old confirmation-text-input reference left behind const confirmInput = page.locator(); with no arguments. Playwright's Page.locator() requires a selector string argument; calling it with none throws a runtime error (locator: Expected a selector) as soon as this line executes. Because the new DeleteEntityModal no longer renders any confirmation-text-input (it uses radio buttons + a confirm button instead), the entire confirmInput block (lines 411-420) is now dead and broken. It should be removed, and the confirm button should be clicked directly after the modal becomes visible.

🤖 Prompt for agents
Code Review: Replaces the legacy entity delete modal with a new standardized component, but the async delete path lacks error handling, causing the modal to lock up on failure. Additionally, numerous Playwright helpers still reference stale Ant Design selectors and missing data-testids.

1. ⚠️ Bug: Merge dropped used i18n key intake-form-plural-description
   Files: openmetadata-ui/src/main/resources/ui/src/locale/languages/en-us.json:3287

   The most recent commit (the `main` merge) removes the `message.intake-form-plural-description` key from all 20 locale files (e.g. `en-us.json` line ~3287). However, that key is still referenced in `src/utils/GlobalSettingsClassBase.ts` (`description: t('message.intake-form-plural-description')`) and `src/pages/IntakeForms/IntakeFormsPage.tsx` (`subHeader: t('message.intake-form-plural-description')`). On the PR branch prior to the merge (commit 76494659) the key was present and resolved correctly; after the merge it is gone from every locale file while the two call sites remain. As a result i18next will render the raw key string `message.intake-form-plural-description` in the Intake Forms settings page/subheader instead of the intended text.
   
   This looks like a merge-conflict misresolution that accidentally deleted the key. Restore the `intake-form-plural-description` entry in all 20 locale files (or, if the Intake Forms feature is being intentionally dropped, remove the two `t(...)` call sites as well) so the key and its usages stay in sync.

   Fix (Re-add the accidentally-removed key to all 20 locale files (translated per language) so the two t('message.intake-form-plural-description') call sites resolve.):
   "intake-form-plural-description": "Define governance-policy required fields for Data Products, Domains, and Glossary Terms. Enforced at both the API and UI layers.",

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@sonarqubecloud

sonarqubecloud Bot commented Jul 7, 2026

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 UI UI specific issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UI - update delete modal design to match new design framework

1 participant