Skip to content

Removed entity delete modal#29953

Open
anuj-kumary wants to merge 3 commits into
mainfrom
entity-modal-type
Open

Removed entity delete modal#29953
anuj-kumary wants to merge 3 commits into
mainfrom
entity-modal-type

Conversation

@anuj-kumary

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

Copy link
Copy Markdown
Member

Describe your changes:

Fixes #

I worked on ... because ...

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.

Greptile Summary

This PR removes the old entity delete modal and moves the affected delete flows to the shared modal. The main changes are:

  • Deleted the legacy EntityDeleteModal component and tests.
  • Updated ingestion pipeline, sample data, and failed sample data delete flows to use DeleteModal.
  • Added delete loading state wiring for the migrated flows.
  • Updated unit and Playwright tests for the input-less delete modal.
  • Widened the shared delete modal message prop to accept rendered content.

Confidence Score: 5/5

This looks safe to merge from the follow-up changes reviewed here.

  • No blocking issues found in the changed code.
  • The deleted modal references were cleaned up.
  • The shared modal message type matches the rendered messages passed by callers.
  • The new loading state wiring disables modal actions while deletes are in progress.

Important Files Changed

Filename Overview
openmetadata-ui/src/main/resources/ui/src/components/Modals/EntityDeleteModal/EntityDeleteModal.tsx The legacy typed-confirmation delete modal was removed.
openmetadata-ui/src/main/resources/ui/src/components/common/DeleteModal/DeleteModal.interface.ts The message prop now accepts rendered content instead of only plain strings.
openmetadata-ui/src/main/resources/ui/src/components/Settings/Services/Ingestion/IngestionListTable/IngestionListTable.tsx The ingestion pipeline delete flow now uses the shared delete modal with parent-managed loading state.
openmetadata-ui/src/main/resources/ui/src/components/DataQuality/TestSuite/TestSuitePipelineTab/TestSuitePipelineTab.component.tsx The test-suite pipeline delete flow now uses the shared delete modal with parent-managed loading state.
openmetadata-ui/src/main/resources/ui/src/components/Database/SampleDataTable/SampleDataTable.component.tsx The sample data delete flow now uses the shared delete modal and tracks delete progress locally.
openmetadata-ui/src/main/resources/ui/src/components/DataQuality/IncidentManager/FailedTestCaseSampleData/FailedTestCaseSampleData.component.tsx The failed sample data delete flow now uses the shared delete modal and tracks delete progress locally.

Reviews (2): Last reviewed commit: "Merge branch 'main' into entity-modal-ty..." | Re-trigger Greptile

Context used (3)

  • Context used - CLAUDE.md (source)
  • Context used - openmetadata-ui-core-components/CLAUDE.md (source)
  • Context used - AGENTS.md (source)

@anuj-kumary anuj-kumary self-assigned this Jul 11, 2026
@anuj-kumary anuj-kumary added the safe to test Add this label to run secure Github workflows on PRs label Jul 11, 2026
@anuj-kumary anuj-kumary requested a review from a team as a code owner July 11, 2026 18:39
@anuj-kumary anuj-kumary added the skip-pr-checks Bypass PR metadata validation check label Jul 11, 2026
@github-actions

github-actions Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 64%
64.09% (74588/116371) 47.63% (43935/92239) 48.66% (13393/27521)

@github-actions

github-actions Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

🟡 Playwright Results — all passed (37 flaky)

✅ 4496 passed · ❌ 0 failed · 🟡 37 flaky · ⏭️ 95 skipped

Shard Passed Failed Flaky Skipped
🟡 Shard 1 422 0 5 16
✅ Shard 2 11 0 0 0
🟡 Shard 3 811 0 16 8
🟡 Shard 4 814 0 3 18
🟡 Shard 5 829 0 1 5
🟡 Shard 6 830 0 2 46
🟡 Shard 7 779 0 10 2
🟡 37 flaky test(s) (passed on retry)
  • Features/Glossary/GlossaryPagination.spec.ts › should check for nested glossary term search (shard 1, 1 retry)
  • Pages/AISettings.spec.ts › persists MCP Server configuration after save (shard 1, 1 retry)
  • Pages/Lineage/LineageRightPanel.spec.ts › Verify custom properties tab IS visible for supported type: metric (shard 1, 1 retry)
  • Pages/UserCreationWithPersona.spec.ts › Create user with persona and verify on profile (shard 1, 1 retry)
  • Flow/SearchRBAC.spec.ts › the browse tree only shows the asset-type categories a user can access (shard 1, 1 retry)
  • Features/BulkEditOperationBadges.spec.ts › Database service bulk edit search filters rows and clear restores them (shard 3, 1 retry)
  • Features/BulkImportWithDotInName.spec.ts › Bulk edit existing entity with dot in service name (shard 3, 1 retry)
  • Features/ContextCenterArchivePage.spec.ts › archive page lazy-loads more rows on scroll within its own scroll container (shard 3, 1 retry)
  • Features/ContextCenterArticles.spec.ts › Article listing search filters, clears, and shows empty state (shard 3, 1 retry)
  • Features/ContextCenterArticles.spec.ts › Quick link created from API can be opened and deleted from hierarchy (shard 3, 1 retry)
  • Features/ContextCenterArticles.spec.ts › Article list cards, recently viewed widget, and pagination work (shard 3, 1 retry)
  • Features/ContextCenterArticles.spec.ts › description: switching articles does not bleed unsaved content into next article (shard 3, 1 retry)
  • Features/ContextCenterMemories.spec.ts › clicking a memory row adds ?memory= param to the URL (shard 3, 1 retry)
  • Features/ContextCenterMemories.spec.ts › adding a linked asset in edit mode shows entity badge on the row (shard 3, 1 retry)
  • Features/ContextCenterMemories.spec.ts › Private memory shows "visible only to you" description (shard 3, 1 retry)
  • Features/ContextCenterPermission.spec.ts › user with view-only permission cannot see restore or delete actions on an archived document (shard 3, 1 retry)
  • Features/ContextCenterPermission.spec.ts › user with deleteAll permission can see delete action but not restore action on an archived document, and can delete it (shard 3, 1 retry)
  • Features/ContextCenterPermission.spec.ts › selecting "Updated By" actually reorders rows by updatedBy (shard 3, 1 retry)
  • Features/DataQuality/BundleSuiteBulkOperations.spec.ts › Add test case to existing Bundle Suite (shard 3, 2 retries)
  • Features/Glossary/GlossaryAdvancedOperations.spec.ts › should change domain on glossary (shard 3, 1 retry)
  • Features/Glossary/GlossaryHierarchy.spec.ts › should cancel move operation (shard 3, 1 retry)
  • Features/Glossary/GlossaryTermRelationsGraphNested.spec.ts › viewing a child term: parentOf edge is rendered between parent and child (shard 4, 1 retry)
  • Features/Glossary/GlossaryWorkflow.spec.ts › non-reviewer should not see approve/reject buttons (shard 4, 1 retry)
  • Features/SearchExport.spec.ts › Export queues a background job and downloads from the jobs tray (shard 4, 1 retry)
  • Pages/CustomProperties.spec.ts › Number (shard 5, 1 retry)
  • Pages/ExploreBrowse.spec.ts › service type drill-down disables unrelated roots and query-panel Clear resets it (shard 6, 1 retry)
  • Pages/ExplorePageRightPanel_KnowledgeCenter.spec.ts › Should remove user owner for knowledgeCenter (shard 6, 1 retry)
  • Pages/ExplorePageRightPanel.spec.ts › Should allow Data Steward to edit tags for searchIndex (shard 7, 1 retry)
  • Pages/Glossary.spec.ts › Rename Glossary Term and verify assets (shard 7, 1 retry)
  • Pages/GlossaryImportExport.spec.ts › Glossary Bulk Import Export (shard 7, 1 retry)
  • ... and 7 more

📦 Download artifacts

How to debug locally
# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

anuj-kumary and others added 2 commits July 12, 2026 11:46
… selector

The component was updated to use DeleteModal (MUI-based) instead of
EntityDeleteModal (Ant Design), which no longer renders .ant-modal-body.
Update the selector to data-testid="delete-modal".

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

gitar-bot Bot commented Jul 12, 2026

Copy link
Copy Markdown
Code Review 👍 Approved with suggestions 0 resolved / 1 findings

Refactors the codebase by removing the legacy entity delete modal in favor of a common implementation. Consider restoring the text-input safeguard requiring users to type 'DELETE' to confirm destructive actions.

💡 Edge Case: Delete confirmation no longer requires typing DELETE

📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SampleDataTableOperations.spec.ts:184-198

The migration from EntityDeleteModal to the common DeleteModal removes the "type DELETE to confirm" text-input safeguard. The new DeleteModal enables the confirm button immediately (see DeleteModal.tsx confirm-button has no gating on typed text), so destructive actions—deleting ingestion pipelines (IngestionListTable.tsx, TestSuitePipelineTab.component.tsx) and clearing sample data (SampleDataTable.component.tsx, FailedTestCaseSampleData.component.tsx)—are now a single-click operation with no typed confirmation. The Playwright test was updated accordingly (SampleDataTableOperations.spec.ts now expects the confirm button to be enabled without typing DELETE). This appears intentional for the refactor, but reduces the accidental-deletion guard for permanent/irreversible operations. Confirm this UX change is desired for these permanent-delete flows; if so no action needed.

🤖 Prompt for agents
Code Review: Refactors the codebase by removing the legacy entity delete modal in favor of a common implementation. Consider restoring the text-input safeguard requiring users to type 'DELETE' to confirm destructive actions.

1. 💡 Edge Case: Delete confirmation no longer requires typing DELETE
   Files: openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SampleDataTableOperations.spec.ts:184-198

   The migration from `EntityDeleteModal` to the common `DeleteModal` removes the "type DELETE to confirm" text-input safeguard. The new `DeleteModal` enables the confirm button immediately (see DeleteModal.tsx confirm-button has no gating on typed text), so destructive actions—deleting ingestion pipelines (IngestionListTable.tsx, TestSuitePipelineTab.component.tsx) and clearing sample data (SampleDataTable.component.tsx, FailedTestCaseSampleData.component.tsx)—are now a single-click operation with no typed confirmation. The Playwright test was updated accordingly (SampleDataTableOperations.spec.ts now expects the confirm button to be enabled without typing DELETE). This appears intentional for the refactor, but reduces the accidental-deletion guard for permanent/irreversible operations. Confirm this UX change is desired for these permanent-delete flows; if so no action needed.

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

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 skip-pr-checks Bypass PR metadata validation check

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant