From 97c8f5436caf67af1f1e0fe789cf749f3f16df1f Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Tue, 20 Oct 2026 16:22:32 -0600 Subject: [PATCH 01/12] fix(semantic): preserve archived dependencies and clarify delete dialogs --- .../docs/using-superset/recently-archived.mdx | 2 + docs/static/resources/openapi.json | 18 ++- .../DatabaseList.semanticLayerDelete.test.tsx | 12 +- .../src/pages/DatabaseList/index.tsx | 11 +- .../DatasetList/DatasetList.behavior.test.tsx | 6 +- .../DatasetList.integration.test.tsx | 2 +- .../src/pages/DatasetList/index.tsx | 26 ++-- superset/commands/semantic_layer/delete.py | 78 ++++++---- .../commands/semantic_layer/exceptions.py | 35 +++-- superset/semantic_layers/api.py | 30 +++- superset/translations/messages.pot | 74 +++++++-- .../commands/semantic_layer/delete_test.py | 141 ++++++++++++++---- tests/unit_tests/semantic_layers/api_test.py | 10 +- 13 files changed, 329 insertions(+), 116 deletions(-) diff --git a/docs/docs/using-superset/recently-archived.mdx b/docs/docs/using-superset/recently-archived.mdx index b2f9f9683635..c217d2febe66 100644 --- a/docs/docs/using-superset/recently-archived.mdx +++ b/docs/docs/using-superset/recently-archived.mdx @@ -392,3 +392,5 @@ exit status 0 alone as proof of erasure. The purge removes the entity together with its [version history](./version-history.mdx). The retention window for version history itself is configured separately and is described on that page. + +Archived charts and dashboards still block deletion of semantic views and layers they depend on. The refusal includes the number of archived dependents. Use **Recently Archived** to restore and repoint them, or permanently delete them, before deleting the semantic source. diff --git a/docs/static/resources/openapi.json b/docs/static/resources/openapi.json index f1bedd1da143..1be46e8d1146 100644 --- a/docs/static/resources/openapi.json +++ b/docs/static/resources/openapi.json @@ -34263,6 +34263,9 @@ "application/json": { "schema": { "properties": { + "archived_count": { + "type": "integer" + }, "dependents": { "items": { "properties": { @@ -34306,7 +34309,8 @@ "message", "total", "dependents", - "inaccessible_count" + "inaccessible_count", + "archived_count" ], "type": "object" } @@ -34648,6 +34652,9 @@ "application/json": { "schema": { "properties": { + "archived_count": { + "type": "integer" + }, "dependents": { "items": { "properties": { @@ -34691,7 +34698,8 @@ "message", "total", "dependents", - "inaccessible_count" + "inaccessible_count", + "archived_count" ], "type": "object" } @@ -34810,6 +34818,9 @@ "application/json": { "schema": { "properties": { + "archived_count": { + "type": "integer" + }, "dependents": { "items": { "properties": { @@ -34853,7 +34864,8 @@ "message", "total", "dependents", - "inaccessible_count" + "inaccessible_count", + "archived_count" ], "type": "object" } diff --git a/superset-frontend/src/pages/DatabaseList/DatabaseList.semanticLayerDelete.test.tsx b/superset-frontend/src/pages/DatabaseList/DatabaseList.semanticLayerDelete.test.tsx index cf4afdffa157..57d61b402ff6 100644 --- a/superset-frontend/src/pages/DatabaseList/DatabaseList.semanticLayerDelete.test.tsx +++ b/superset-frontend/src/pages/DatabaseList/DatabaseList.semanticLayerDelete.test.tsx @@ -205,7 +205,7 @@ test('delete confirmation warns about cascade-deleting dependent views by count expect( within(dialog).getByText( - 'This will also permanently delete its 3 semantic views. Charts built on those views will stop working.', + 'This will also permanently delete its 3 semantic views. This cannot be undone.', ), ).toBeInTheDocument(); expect( @@ -232,7 +232,7 @@ test('a single dependent view is announced in the singular', async () => { expect( within(dialog).getByText( - 'This will also permanently delete its 1 semantic view. Charts built on that view will stop working.', + 'This will also permanently delete its 1 semantic view. This cannot be undone.', ), ).toBeInTheDocument(); }); @@ -275,7 +275,7 @@ test('a counted response with an empty name page keeps the count but omits the l expect( within(dialog).getByText( - 'This will also permanently delete its 3 semantic views. Charts built on those views will stop working.', + 'This will also permanently delete its 3 semantic views. This cannot be undone.', ), ).toBeInTheDocument(); expect( @@ -291,7 +291,7 @@ test('a failed dependent lookup still opens the modal with an uncounted warning' expect( within(dialog).getByText( - 'Deleting this semantic layer also permanently deletes any semantic views it contains, and charts built on those views will stop working. The affected views could not be listed.', + 'Deleting this semantic layer also permanently deletes any semantic views it contains. This cannot be undone. The affected views could not be listed.', ), ).toBeInTheDocument(); }); @@ -316,7 +316,7 @@ test('the overflow footer reports dependent views beyond the listed page', async expect( within(dialog).getByText( - 'This will also permanently delete its 12 semantic views. Charts built on those views will stop working.', + 'This will also permanently delete its 12 semantic views. This cannot be undone.', ), ).toBeInTheDocument(); expect(within(dialog).getByText('view_10')).toBeInTheDocument(); @@ -443,7 +443,7 @@ test("a stale lookup resolving late cannot replace a newer row's modal", async ( test('layer delete displays the counted dependency conflict', async () => { const reason = - 'Semantic source is used by 25 dependent assets and cannot be deleted.'; + 'Semantic source is used by 25 dependent assets and cannot be deleted. 3 dependent assets are archived. Go to Recently Archived to restore and repoint archived assets, or delete them permanently.'; setupMocks({ dependents: [], deleteError: reason }); const addDangerToast = jest.fn(); renderDatabaseList(mockUser, addDangerToast); diff --git a/superset-frontend/src/pages/DatabaseList/index.tsx b/superset-frontend/src/pages/DatabaseList/index.tsx index c0fdb53dcdd7..591cdb3537c8 100644 --- a/superset-frontend/src/pages/DatabaseList/index.tsx +++ b/superset-frontend/src/pages/DatabaseList/index.tsx @@ -137,7 +137,7 @@ function SemanticLayerCascadeWarning({ return (

{t( - 'Deleting this semantic layer also permanently deletes any semantic views it contains, and charts built on those views will stop working. The affected views could not be listed.', + 'Deleting this semantic layer also permanently deletes any semantic views it contains. This cannot be undone. The affected views could not be listed.', )}

); @@ -158,8 +158,8 @@ function SemanticLayerCascadeWarning({ <>

{tn( - 'This will also permanently delete its %s semantic view. Charts built on that view will stop working.', - 'This will also permanently delete its %s semantic views. Charts built on those views will stop working.', + 'This will also permanently delete its %s semantic view. This cannot be undone.', + 'This will also permanently delete its %s semantic views. This cannot be undone.', preview.dependentViewCount, preview.dependentViewCount, )} @@ -1181,6 +1181,11 @@ function DatabaseList({ {slDeletePreview.item.database_name}?

+

+ {t( + 'Deletion is blocked while any assets depend on these views, including archived assets. Remove or repoint them first. Use Recently Archived to restore and repoint archived assets, or delete them permanently.', + )} +

} onConfirm={() => { diff --git a/superset-frontend/src/pages/DatasetList/DatasetList.behavior.test.tsx b/superset-frontend/src/pages/DatasetList/DatasetList.behavior.test.tsx index 097e8c02102b..cf38acb1b6b2 100644 --- a/superset-frontend/src/pages/DatasetList/DatasetList.behavior.test.tsx +++ b/superset-frontend/src/pages/DatasetList/DatasetList.behavior.test.tsx @@ -81,7 +81,7 @@ test('single semantic view delete displays the counted dependency conflict', asy kind: 'semantic_view', }; const reason = - 'Semantic source is used by 25 dependent assets and cannot be deleted.'; + 'Semantic source is used by 25 dependent assets and cannot be deleted. 3 dependent assets are archived. Go to Recently Archived to restore and repoint archived assets, or delete them permanently.'; const addDangerToast = jest.fn(); mockDatasetListEndpoints({ result: [view], count: 1 }); fetchMock.delete('glob:*/api/v1/semantic_view/99', { @@ -95,6 +95,8 @@ test('single semantic view delete displays the counted dependency conflict', asy await screen.findByText(view.table_name); await userEvent.click(screen.getByTestId('dataset-row-delete')); const modal = await screen.findByRole('dialog'); + expect(modal).toHaveTextContent('This cannot be undone.'); + expect(modal).toHaveTextContent('including archived assets'); await userEvent.type( within(modal).getByTestId('delete-modal-input'), 'DELETE', @@ -117,7 +119,7 @@ test('mixed bulk delete names refused semantic views and their conflict reason', }; const addDangerToast = jest.fn(); const reason = - 'Semantic source is used by 25 dependent assets and cannot be deleted.'; + 'Semantic source is used by 25 dependent assets and cannot be deleted. 3 dependent assets are archived. Go to Recently Archived to restore and repoint archived assets, or delete them permanently.'; mockDatasetListEndpoints({ result: [dataset, semanticView], count: 2 }); fetchMock.get(API_ENDPOINTS.DATASET_BULK_RELATED_OBJECTS, { charts: { count: 0, result: [] }, diff --git a/superset-frontend/src/pages/DatasetList/DatasetList.integration.test.tsx b/superset-frontend/src/pages/DatasetList/DatasetList.integration.test.tsx index fa620ccabd5c..0c73dade7b75 100644 --- a/superset-frontend/src/pages/DatasetList/DatasetList.integration.test.tsx +++ b/superset-frontend/src/pages/DatasetList/DatasetList.integration.test.tsx @@ -394,7 +394,7 @@ test('bulk delete confirm never claims a semantic view has no dependents', async expect( await within(modal).findByText( - /charts built on the selected semantic view/i, + /deletion is blocked while any assets depend on the selected semantic views/i, ), ).toBeInTheDocument(); expect(modal).not.toHaveTextContent(/no charts or dashboards depend on/i); diff --git a/superset-frontend/src/pages/DatasetList/index.tsx b/superset-frontend/src/pages/DatasetList/index.tsx index 43b82ec3fca6..398ed8678602 100644 --- a/superset-frontend/src/pages/DatasetList/index.tsx +++ b/superset-frontend/src/pages/DatasetList/index.tsx @@ -1592,10 +1592,21 @@ const DatasetList: FunctionComponent = ({ )} {svCurrentlyDeleting && ( +

+ {t( + 'Are you sure you want to delete %s? This cannot be undone.', + svCurrentlyDeleting.table_name, + )} +

+

+ {t( + 'Deletion is blocked while any assets depend on this view, including archived assets. Remove or repoint them first. Use Recently Archived to restore and repoint archived assets, or delete them permanently.', + )} +

+ + } onConfirm={handleSemanticViewDeleteConfirm} onHide={() => setSvCurrentlyDeleting(null)} open @@ -1718,11 +1729,8 @@ const DatasetList: FunctionComponent = ({ ))} {pendingBulkSemanticCount > 0 && (

- {tn( - 'Charts built on the selected semantic view are not checked and will break if it is deleted.', - 'Charts built on the %s selected semantic views are not checked and will break if they are deleted.', - pendingBulkSemanticCount, - pendingBulkSemanticCount, + {t( + 'Deletion is blocked while any assets depend on the selected semantic views, including archived assets. Remove or repoint them first. Use Recently Archived to restore and repoint archived assets, or delete them permanently.', )}

)} diff --git a/superset/commands/semantic_layer/delete.py b/superset/commands/semantic_layer/delete.py index 1416819c6727..2f4ea894cc65 100644 --- a/superset/commands/semantic_layer/delete.py +++ b/superset/commands/semantic_layer/delete.py @@ -97,13 +97,14 @@ def _dashboard_targets_view( def _dashboard_target_ids(view_ids: sa.Select) -> set[int]: - """Find live dashboards with semantic targets after a coarse SQL prefilter.""" + """Find dashboards with semantic targets after a coarse SQL prefilter.""" selected_ids: set[int] = set(db.session.execute(view_ids).scalars()) if not selected_ids: return set() - candidates: sa.Select = sa.select(Dashboard.id, Dashboard.json_metadata).where( - Dashboard.deleted_at.is_(None), - Dashboard.json_metadata.like("%semantic_view%"), + candidates: sa.Select = sa.select( + Dashboard.__table__.c.id, Dashboard.__table__.c.json_metadata + ).where( + Dashboard.__table__.c.json_metadata.like("%semantic_view%"), ) matched_ids: set[int] = set() for dashboard_id, metadata in db.session.execute( @@ -116,47 +117,52 @@ def _dashboard_target_ids(view_ids: sa.Select) -> set[int]: def _dependent_assets( view_ids: sa.Select, -) -> tuple[int, list[dict[str, str | int]], int]: +) -> tuple[int, list[dict[str, str | int]], int, int]: """Count dependents, naming only assets visible through their list APIs.""" - chart_ids: sa.Select = sa.select(Slice.id).where( - Slice.datasource_type == "semantic_view", - Slice.datasource_id.in_(view_ids), - Slice.deleted_at.is_(None), + chart_ids: sa.Select = sa.select(Slice.__table__.c.id).where( + Slice.__table__.c.datasource_type == "semantic_view", + Slice.__table__.c.datasource_id.in_(view_ids), ) chart_dashboard_ids: sa.Select = ( sa.select(dashboard_slices.c.dashboard_id) - .join(Slice, dashboard_slices.c.slice_id == Slice.id) - .join(Dashboard, dashboard_slices.c.dashboard_id == Dashboard.id) + .join(Slice.__table__, dashboard_slices.c.slice_id == Slice.__table__.c.id) + .join( + Dashboard.__table__, + dashboard_slices.c.dashboard_id == Dashboard.__table__.c.id, + ) .where( - Slice.id.in_(chart_ids), - Dashboard.deleted_at.is_(None), + Slice.__table__.c.id.in_(chart_ids), ) .distinct() ) dashboard_target_ids: set[int] = _dashboard_target_ids(view_ids) - dashboard_ids: sa.Select = sa.select(Dashboard.id).where( - Dashboard.deleted_at.is_(None), + dashboard_ids: sa.Select = sa.select(Dashboard.__table__.c.id).where( sa.or_( - Dashboard.id.in_(chart_dashboard_ids), - Dashboard.id.in_(dashboard_target_ids), + Dashboard.__table__.c.id.in_(chart_dashboard_ids), + Dashboard.__table__.c.id.in_(dashboard_target_ids), ), ) dependents: sa.Subquery = sa.union_all( sa.select( sa.literal("chart").label("type"), - Slice.id.label("id"), - sa.func.coalesce(Slice.slice_name, str(_("Untitled"))).label("name"), - ).where(Slice.id.in_(chart_ids)), - sa.select( - sa.literal("dashboard").label("type"), - Dashboard.id.label("id"), - sa.func.coalesce(Dashboard.dashboard_title, str(_("Untitled"))).label( + Slice.__table__.c.id.label("id"), + Slice.__table__.c.deleted_at.is_not(None).label("archived"), + sa.func.coalesce(Slice.__table__.c.slice_name, str(_("Untitled"))).label( "name" ), - ).where(Dashboard.id.in_(dashboard_ids)), + ).where(Slice.__table__.c.id.in_(chart_ids)), + sa.select( + sa.literal("dashboard").label("type"), + Dashboard.__table__.c.id.label("id"), + Dashboard.__table__.c.deleted_at.is_not(None).label("archived"), + sa.func.coalesce( + Dashboard.__table__.c.dashboard_title, str(_("Untitled")) + ).label("name"), + ).where(Dashboard.__table__.c.id.in_(dashboard_ids)), sa.select( sa.func.lower(ReportSchedule.type).label("type"), ReportSchedule.id.label("id"), + sa.literal(False).label("archived"), ReportSchedule.name.label("name"), ).where( ReportSchedule.active.is_(True), @@ -170,7 +176,7 @@ def _dependent_assets( cast( sa.Select, ChartFilter("id", SQLAInterface(Slice, db.session)) - .apply(db.session.query(Slice.id), None) + .apply(db.session.query(Slice.id).filter(Slice.deleted_at.is_(None)), None) .statement, ) if security_manager.can_access("can_read", "Chart") @@ -180,7 +186,10 @@ def _dependent_assets( cast( sa.Select, DashboardAccessFilter("id", SQLAInterface(Dashboard, db.session)) - .apply(db.session.query(Dashboard.id), None) + .apply( + db.session.query(Dashboard.id).filter(Dashboard.deleted_at.is_(None)), + None, + ) .statement, ) if security_manager.can_access("can_read", "Dashboard") @@ -211,18 +220,19 @@ def _dependent_assets( ), ) # Both counts come from one statement so READ COMMITTED cannot mix snapshots. - counts: tuple[int, int | None] = cast( - tuple[int, int | None], + counts: tuple[int, int | None, int | None] = cast( + tuple[int, int | None, int | None], db.session.execute( sa.select( sa.func.count(), sa.func.sum(sa.case((visibility, 1), else_=0)), + sa.func.sum(sa.case((dependents.c.archived, 1), else_=0)), ).select_from(dependents) ).one(), ) total: int = int(counts[0]) if not total: - return 0, [], 0 + return 0, [], 0, 0 visible_total: int = int(counts[1] or 0) visible: sa.Subquery = sa.select(dependents).where(visibility).subquery() rows: list[tuple[str, int, str]] = cast( @@ -248,6 +258,7 @@ def _dependent_assets( for asset_type, asset_id, name in rows ], total - visible_total, + int(counts[2] or 0), ) @@ -256,9 +267,12 @@ def _raise_for_dependents(view_ids: sa.Select) -> None: total: int dependents: list[dict[str, str | int]] inaccessible_count: int - total, dependents, inaccessible_count = _dependent_assets(view_ids) + archived_count: int + total, dependents, inaccessible_count, archived_count = _dependent_assets(view_ids) if total: - raise SemanticDeleteDependentsError(total, dependents, inaccessible_count) + raise SemanticDeleteDependentsError( + total, dependents, inaccessible_count, archived_count + ) class DeleteSemanticLayerCommand(BaseCommand): diff --git a/superset/commands/semantic_layer/exceptions.py b/superset/commands/semantic_layer/exceptions.py index 95f329ddc02a..40d0bd3939f5 100644 --- a/superset/commands/semantic_layer/exceptions.py +++ b/superset/commands/semantic_layer/exceptions.py @@ -77,7 +77,7 @@ class SemanticViewDeleteFailedError(DeleteFailedError): class SemanticDeleteDependentsError(CommandException): - """A semantic view or layer still has live dependent assets.""" + """A semantic view or layer still has live or archived dependent assets.""" status: int = 409 @@ -86,17 +86,32 @@ def __init__( total: int, dependents: list[dict[str, str | int]], inaccessible_count: int = 0, + archived_count: int = 0, ) -> None: + self.archived_count: int = archived_count self.total: int = total self.dependents: list[dict[str, str | int]] = dependents self.inaccessible_count: int = inaccessible_count - super().__init__( - ngettext( - "Semantic source is used by %(count)s dependent asset " - "and cannot be deleted.", - "Semantic source is used by %(count)s dependent assets " - "and cannot be deleted.", - total, - count=total, - ) + message: str = ngettext( + "Semantic source is used by %(count)s dependent asset " + "and cannot be deleted.", + "Semantic source is used by %(count)s dependent assets " + "and cannot be deleted.", + total, + count=total, + ) + message += " " + _( + "Remove or repoint these assets before deleting the semantic source." ) + if archived_count: + message += " " + ngettext( + "%(count)s dependent asset is archived.", + "%(count)s dependent assets are archived.", + archived_count, + count=archived_count, + ) + message += " " + _( + "Go to Recently Archived to restore and repoint archived assets, " + "or delete them permanently." + ) + super().__init__(message) diff --git a/superset/semantic_layers/api.py b/superset/semantic_layers/api.py index 791cf3289636..1559fd9e86ba 100644 --- a/superset/semantic_layers/api.py +++ b/superset/semantic_layers/api.py @@ -519,7 +519,12 @@ def delete(self, pk: int) -> Response: application/json: schema: type: object - required: [message, total, dependents, inaccessible_count] + required: + - message + - total + - dependents + - inaccessible_count + - archived_count properties: message: type: string @@ -541,6 +546,8 @@ def delete(self, pk: int) -> Response: type: string inaccessible_count: type: integer + archived_count: + type: integer 422: $ref: '#/components/responses/422' """ @@ -561,6 +568,7 @@ def delete(self, pk: int) -> Response: total=ex.total, dependents=ex.dependents, inaccessible_count=ex.inaccessible_count, + archived_count=ex.archived_count, ) except SemanticViewDeleteFailedError as ex: logger.error( @@ -612,7 +620,12 @@ def bulk_delete(self, **kwargs: Any) -> Response: application/json: schema: type: object - required: [message, total, dependents, inaccessible_count] + required: + - message + - total + - dependents + - inaccessible_count + - archived_count properties: message: type: string @@ -634,6 +647,8 @@ def bulk_delete(self, **kwargs: Any) -> Response: type: string inaccessible_count: type: integer + archived_count: + type: integer 422: $ref: '#/components/responses/422' """ @@ -662,6 +677,7 @@ def bulk_delete(self, **kwargs: Any) -> Response: total=ex.total, dependents=ex.dependents, inaccessible_count=ex.inaccessible_count, + archived_count=ex.archived_count, ) except SemanticViewDeleteFailedError as ex: logger.error( @@ -1135,7 +1151,12 @@ def delete(self, uuid: str) -> FlaskResponse: application/json: schema: type: object - required: [message, total, dependents, inaccessible_count] + required: + - message + - total + - dependents + - inaccessible_count + - archived_count properties: message: type: string @@ -1157,6 +1178,8 @@ def delete(self, uuid: str) -> FlaskResponse: type: string inaccessible_count: type: integer + archived_count: + type: integer 422: $ref: '#/components/responses/422' """ @@ -1174,6 +1197,7 @@ def delete(self, uuid: str) -> FlaskResponse: total=ex.total, dependents=ex.dependents, inaccessible_count=ex.inaccessible_count, + archived_count=ex.archived_count, ) except SemanticLayerDeleteFailedError as ex: logger.error( diff --git a/superset/translations/messages.pot b/superset/translations/messages.pot index ffd822fc4a17..a897b4b5afda 100644 --- a/superset/translations/messages.pot +++ b/superset/translations/messages.pot @@ -3247,12 +3247,6 @@ msgstr "" msgid "Charts" msgstr "" -#, python-format -msgid "Charts built on the selected semantic view are not checked and will break if it is deleted." -msgid_plural "Charts built on the %s selected semantic views are not checked and will break if they are deleted." -msgstr[0] "" -msgstr[1] "" - msgid "Charts could not be deleted." msgstr "" @@ -5262,9 +5256,6 @@ msgstr "" msgid "Deleting this dataset is permanent. The affected charts and dashboards will remain, but they may no longer work." msgstr "" -msgid "Deleting this semantic layer also permanently deletes any semantic views it contains, and charts built on those views will stop working. The affected views could not be listed." -msgstr "" - msgid "Delimited long & lat single column" msgstr "" @@ -15206,12 +15197,6 @@ msgstr[1] "" msgid "This will abort (stop) the task for all %s subscriber(s)." msgstr "" -#, python-format -msgid "This will also permanently delete its %s semantic view. Charts built on that view will stop working." -msgid_plural "This will also permanently delete its %s semantic views. Charts built on those views will stop working." -msgstr[0] "" -msgstr[1] "" - msgid "This will be applied to the whole table. Arrows (↑ and ↓) will be added to main columns for increase and decrease. Basic conditional formatting can be overwritten by conditional formatting below." msgstr "" @@ -18261,3 +18246,62 @@ msgstr "" msgid "The view configuration must be a JSON object." msgstr "" + +#, python-format +msgid "%(count)s dependent asset is archived." +msgid_plural "%(count)s dependent assets are archived." +msgstr[0] "" +msgstr[1] "" + + +#, python-format +msgid "Are you sure you want to delete %s? This cannot be undone." +msgstr "" + + +msgid "" +"Deleting this semantic layer also permanently deletes any semantic views it " +"contains. This cannot be undone. The affected views could not be listed." +msgstr "" + + +msgid "" +"Deletion is blocked while any assets depend on the selected semantic views, " +"including archived assets. Remove or repoint them first. Use Recently " +"Archived to restore and repoint archived assets, or delete them permanently." +msgstr "" + + +msgid "" +"Deletion is blocked while any assets depend on these views, including " +"archived assets. Remove or repoint them first. Use Recently Archived to " +"restore and repoint archived assets, or delete them permanently." +msgstr "" + + +msgid "" +"Deletion is blocked while any assets depend on this view, including archived" +" assets. Remove or repoint them first. Use Recently Archived to restore and " +"repoint archived assets, or delete them permanently." +msgstr "" + + +msgid "" +"Go to Recently Archived to restore and repoint archived assets, or delete " +"them permanently." +msgstr "" + + +msgid "Remove or repoint these assets before deleting the semantic source." +msgstr "" + + +#, python-format +msgid "" +"This will also permanently delete its %s semantic view. This cannot be " +"undone." +msgid_plural "" +"This will also permanently delete its %s semantic views. This cannot be " +"undone." +msgstr[0] "" +msgstr[1] "" diff --git a/tests/unit_tests/commands/semantic_layer/delete_test.py b/tests/unit_tests/commands/semantic_layer/delete_test.py index cbae58162162..fa8f7048c9eb 100644 --- a/tests/unit_tests/commands/semantic_layer/delete_test.py +++ b/tests/unit_tests/commands/semantic_layer/delete_test.py @@ -41,7 +41,7 @@ def test_delete_semantic_layer_success(mocker: MockerFixture) -> None: mock_model.uuid = "00000000-0000-0000-0000-000000000001" mocker.patch( "superset.commands.semantic_layer.delete._dependent_assets", - return_value=(0, [], 0), + return_value=(0, [], 0, 0), ) dao = mocker.patch( @@ -74,7 +74,7 @@ def test_delete_semantic_layer_refuses_dependents(mocker: MockerFixture) -> None ) mocker.patch( "superset.commands.semantic_layer.delete._dependent_assets", - return_value=(1, [{"type": "chart", "id": 7, "name": "Revenue"}], 0), + return_value=(1, [{"type": "chart", "id": 7, "name": "Revenue"}], 0, 0), ) exc_info: pytest.ExceptionInfo[SemanticDeleteDependentsError] @@ -138,7 +138,7 @@ def test_delete_semantic_layer_creator_allowed(mocker: MockerFixture) -> None: mock_model.uuid = "00000000-0000-0000-0000-000000000001" mocker.patch( "superset.commands.semantic_layer.delete._dependent_assets", - return_value=(0, [], 0), + return_value=(0, [], 0, 0), ) dao = mocker.patch( @@ -187,7 +187,7 @@ def test_delete_semantic_view_success(mocker: MockerFixture) -> None: mock_model.id = 42 mocker.patch( "superset.commands.semantic_layer.delete._dependent_assets", - return_value=(0, [], 0), + return_value=(0, [], 0, 0), ) dao = mocker.patch( @@ -225,7 +225,7 @@ def test_delete_semantic_view_refuses_dependents(mocker: MockerFixture) -> None: ) mocker.patch( "superset.commands.semantic_layer.delete._dependent_assets", - return_value=(1, [{"type": "chart", "id": 7, "name": "Revenue"}], 0), + return_value=(1, [{"type": "chart", "id": 7, "name": "Revenue"}], 0, 0), ) exc_info: pytest.ExceptionInfo[SemanticDeleteDependentsError] @@ -254,7 +254,7 @@ def test_bulk_delete_semantic_views_refuses_all_on_one_dependency( ) mocker.patch( "superset.commands.semantic_layer.delete._dependent_assets", - return_value=(1, [{"type": "chart", "id": 7, "name": "Revenue"}], 0), + return_value=(1, [{"type": "chart", "id": 7, "name": "Revenue"}], 0, 0), ) with pytest.raises(SemanticDeleteDependentsError): @@ -264,7 +264,7 @@ def test_bulk_delete_semantic_views_refuses_all_on_one_dependency( @pytest.mark.parametrize("orm_execution", [False, True]) -def test_semantic_delete_lists_live_dependents_only( +def test_semantic_delete_counts_archived_dependents_without_listing_them( session: Session, mocker: MockerFixture, orm_execution: bool ) -> None: """Charts, dashboard membership and active schedules form the dependency set.""" @@ -356,10 +356,14 @@ def test_semantic_delete_lists_live_dependents_only( total: int dependents: list[dict[str, str | int]] inaccessible_count: int - total, dependents, inaccessible_count = _dependent_assets(sa.select(sa.literal(42))) + archived_count: int + total, dependents, inaccessible_count, archived_count = _dependent_assets( + sa.select(sa.literal(42)) + ) - assert total == 5 - assert inaccessible_count == 0 + assert total == 6 + assert archived_count == 1 + assert inaccessible_count == 1 assert {item["name"] for item in dependents} == { "Live chart", "Live dashboard", @@ -377,13 +381,13 @@ def test_semantic_delete_lists_live_dependents_only( capped_total: int capped_dependents: list[dict[str, str | int]] capped_inaccessible_count: int - capped_total, capped_dependents, capped_inaccessible_count = _dependent_assets( - sa.select(sa.literal(42)) + capped_total, capped_dependents, capped_inaccessible_count, archived_count = ( + _dependent_assets(sa.select(sa.literal(42))) ) - assert capped_total == 5 + assert capped_total == 6 assert len(capped_dependents) == 2 assert [item["type"] for item in capped_dependents] == ["chart", "chart"] - assert capped_inaccessible_count == 0 + assert capped_inaccessible_count == 1 mocker.patch( "superset.commands.semantic_layer.delete.ChartFilter.apply", return_value=session.query(Slice.id).filter(sa.false()), @@ -391,11 +395,11 @@ def test_semantic_delete_lists_live_dependents_only( hidden_total: int hidden_dependents: list[dict[str, str | int]] hidden_count: int - hidden_total, hidden_dependents, hidden_count = _dependent_assets( + hidden_total, hidden_dependents, hidden_count, archived_count = _dependent_assets( sa.select(sa.literal(42)) ) - assert hidden_total == 5 - assert hidden_count == 2 + assert hidden_total == 6 + assert hidden_count == 3 assert all(dependent["type"] != "chart" for dependent in hidden_dependents) if not orm_execution: connection.close() @@ -404,7 +408,7 @@ def test_semantic_delete_lists_live_dependents_only( def test_semantic_delete_finds_native_filter_dashboards( session: Session, mocker: MockerFixture ) -> None: - """Only live, typed semantic targets block deletion; membership is deduped.""" + """Live and archived typed targets block deletion; membership is deduped.""" import uuid from datetime import datetime @@ -537,11 +541,13 @@ def metadata(source_type: str | None, dataset_id: int | str) -> str: total: int dependents: list[dict[str, str | int]] inaccessible_count: int - total, dependents, inaccessible_count = _dependent_assets( + archived_count: int + total, dependents, inaccessible_count, archived_count = _dependent_assets( sa.select(SemanticView.id).where(SemanticView.id == 42) ) - assert total == 5 - assert inaccessible_count == 0 + assert total == 6 + assert archived_count == 1 + assert inaccessible_count == 1 assert {item["name"] for item in dependents} == { "Typed integer", "Typed string", @@ -568,11 +574,11 @@ def metadata(source_type: str | None, dataset_id: int | str) -> str: hidden_total: int hidden_dependents: list[dict[str, str | int]] hidden_count: int - hidden_total, hidden_dependents, hidden_count = _dependent_assets( - sa.select(SemanticView.id).where(SemanticView.id == 42) + hidden_total, hidden_dependents, hidden_count, archived_count = ( + _dependent_assets(sa.select(SemanticView.id).where(SemanticView.id == 42)) ) - assert hidden_total == 5 - assert hidden_count == 3 + assert hidden_total == 6 + assert hidden_count == 4 assert {item["name"] for item in hidden_dependents} == { "Member chart", "Native report", @@ -767,14 +773,17 @@ def test_semantic_delete_counts_use_one_snapshot( "superset.db.session.scalar", side_effect=[1, 2] ) rows: MagicMock = mocker.patch("superset.db.session.execute") - rows.return_value.one.return_value = (1, 0) + rows.return_value.one.return_value = (1, 0, 0) rows.return_value.all.return_value = [] mocker.patch("superset.security_manager.can_access", return_value=False) total: int dependents: list[dict[str, str | int]] inaccessible_count: int - total, dependents, inaccessible_count = _dependent_assets(sa.select(sa.literal(42))) + archived_count: int + total, dependents, inaccessible_count, archived_count = _dependent_assets( + sa.select(sa.literal(42)) + ) assert (total, dependents, inaccessible_count) == (1, [], 1) stale_scalar.assert_not_called() @@ -810,7 +819,7 @@ def test_delete_semantic_view_creator_allowed(mocker: MockerFixture) -> None: mock_model.id = 42 mocker.patch( "superset.commands.semantic_layer.delete._dependent_assets", - return_value=(0, [], 0), + return_value=(0, [], 0, 0), ) dao = mocker.patch( @@ -877,7 +886,7 @@ def test_bulk_delete_semantic_view_success(mocker: MockerFixture) -> None: mock_models = [MagicMock(), MagicMock()] mocker.patch( "superset.commands.semantic_layer.delete._dependent_assets", - return_value=(0, [], 0), + return_value=(0, [], 0, 0), ) dao = mocker.patch( @@ -925,7 +934,7 @@ def test_bulk_delete_semantic_view_creator_allowed(mocker: MockerFixture) -> Non mock_models = [MagicMock(), MagicMock()] mocker.patch( "superset.commands.semantic_layer.delete._dependent_assets", - return_value=(0, [], 0), + return_value=(0, [], 0, 0), ) dao = mocker.patch( @@ -987,3 +996,75 @@ def test_bulk_delete_semantic_view_not_found(mocker: MockerFixture) -> None: with pytest.raises(SemanticViewNotFoundError): BulkDeleteSemanticViewCommand([1, 2]).run() + + +@pytest.mark.parametrize("kind", ["layer", "view", "bulk"]) +def test_archived_chart_blocks_semantic_delete( + session: Session, mocker: MockerFixture, kind: str +) -> None: + """A recoverable chart must retain its semantic source on every delete path.""" + import uuid + from datetime import datetime + + from superset.commands.semantic_layer.delete import ( + BulkDeleteSemanticViewCommand, + DeleteSemanticViewCommand, + ) + from superset.models.slice import Slice + from superset.semantic_layers.models import SemanticLayer, SemanticView + + Slice.metadata.create_all(session.get_bind()) + layer_uuid: uuid.UUID = uuid.uuid4() + session.execute( + SemanticLayer.__table__.insert().values( + uuid=layer_uuid, name="source", type="test" + ) + ) + session.execute( + SemanticView.__table__.insert().values( + id=42, name="view", semantic_layer_uuid=layer_uuid + ) + ) + session.execute( + Slice.__table__.insert().values( + id=901, + slice_name="archived", + datasource_type="semantic_view", + datasource_id=42, + deleted_at=datetime(2026, 1, 1), + ) + ) + mocker.patch("superset.security_manager.can_access", return_value=False) + mocker.patch( + "superset.commands.semantic_layer.delete.current_user_can_modify_object", + return_value=True, + ) + layer_dao: MagicMock = mocker.patch( + "superset.commands.semantic_layer.delete.SemanticLayerDAO" + ) + view_dao: MagicMock = mocker.patch( + "superset.commands.semantic_layer.delete.SemanticViewDAO" + ) + layer_dao.find_by_uuid.return_value = MagicMock(uuid=layer_uuid) + view_dao.find_by_id.return_value = MagicMock(id=42) + view_dao.find_by_ids.return_value = [MagicMock(id=42)] + command: ( + DeleteSemanticLayerCommand + | DeleteSemanticViewCommand + | BulkDeleteSemanticViewCommand + ) = ( + DeleteSemanticLayerCommand(str(layer_uuid)) + if kind == "layer" + else DeleteSemanticViewCommand(42) + if kind == "view" + else BulkDeleteSemanticViewCommand([42]) + ) + exc_info: pytest.ExceptionInfo[SemanticDeleteDependentsError] + with pytest.raises(SemanticDeleteDependentsError) as exc_info: + command.run() + assert exc_info.value.total == 1 + assert exc_info.value.archived_count == 1 + assert exc_info.value.dependents == [] + assert "Recently Archived" in str(exc_info.value) + layer_dao.delete.assert_not_called() + view_dao.delete.assert_not_called() diff --git a/tests/unit_tests/semantic_layers/api_test.py b/tests/unit_tests/semantic_layers/api_test.py index f86058071d76..cb6bd20bcead 100644 --- a/tests/unit_tests/semantic_layers/api_test.py +++ b/tests/unit_tests/semantic_layers/api_test.py @@ -1036,7 +1036,7 @@ def test_semantic_delete_reports_dependents_as_conflict( } mock_command: MagicMock = mocker.patch(f"superset.semantic_layers.api.{command}") mock_command.return_value.run.side_effect = SemanticDeleteDependentsError( - 25, [dependent], 24 + 25, [dependent], 24, 3 ) request_path: str = ( @@ -1047,11 +1047,16 @@ def test_semantic_delete_reports_dependents_as_conflict( response: TestResponse = client.delete(request_path) assert response.status_code == 409 + assert response.json["archived_count"] == 3 assert response.json["total"] == 25 assert response.json["dependents"] == [dependent] assert response.json["inaccessible_count"] == 24 assert response.json["message"] == ( - "Semantic source is used by 25 dependent assets and cannot be deleted." + "Semantic source is used by 25 dependent assets and cannot be deleted. " + "Remove or repoint these assets before deleting the semantic source. " + "3 dependent assets are archived. " + "Go to Recently Archived to restore and repoint archived assets, " + "or delete them permanently." ) @@ -1083,6 +1088,7 @@ def test_semantic_delete_openapi_conflict_schema( "total", "dependents", "inaccessible_count", + "archived_count", } assert schema["properties"]["dependents"]["maxItems"] == 20 assert schema["properties"]["dependents"]["items"]["properties"]["type"][ From 119a57d32755a8a31d9c464c426db121833f58b7 Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Tue, 20 Oct 2026 16:37:50 -0600 Subject: [PATCH 02/12] fix(semantic): clarify deletion guidance and guard archived dashboards --- .../docs/using-superset/recently-archived.mdx | 8 ++- docs/static/resources/openapi.json | 3 + .../DatabaseList.semanticLayerDelete.test.tsx | 6 +- .../src/pages/DatabaseList/index.tsx | 21 +++++-- .../DatasetList/DatasetList.behavior.test.tsx | 4 +- .../src/pages/DatasetList/index.tsx | 4 +- superset/commands/semantic_layer/delete.py | 4 ++ .../commands/semantic_layer/exceptions.py | 2 +- superset/semantic_layers/api.py | 9 +++ superset/translations/messages.pot | 46 +++++++------- .../commands/semantic_layer/delete_test.py | 61 +++++++++++++++++++ tests/unit_tests/semantic_layers/api_test.py | 2 +- 12 files changed, 131 insertions(+), 39 deletions(-) diff --git a/docs/docs/using-superset/recently-archived.mdx b/docs/docs/using-superset/recently-archived.mdx index c217d2febe66..5b08cd893d5b 100644 --- a/docs/docs/using-superset/recently-archived.mdx +++ b/docs/docs/using-superset/recently-archived.mdx @@ -85,6 +85,12 @@ filter cleanup; the response warns that the dashboard's filters need review. ## Deleting an object permanently +Archived charts and dashboards still block deletion of semantic views and layers +they depend on. The refusal includes the number of archived dependents. Use +**Recently Archived** to restore and repoint them, or permanently delete them, +before deleting the semantic source. If you cannot access those assets, ask an +admin for help. + Use the **Delete permanently** action on a row to remove an object for good. You will be asked to confirm. @@ -392,5 +398,3 @@ exit status 0 alone as proof of erasure. The purge removes the entity together with its [version history](./version-history.mdx). The retention window for version history itself is configured separately and is described on that page. - -Archived charts and dashboards still block deletion of semantic views and layers they depend on. The refusal includes the number of archived dependents. Use **Recently Archived** to restore and repoint them, or permanently delete them, before deleting the semantic source. diff --git a/docs/static/resources/openapi.json b/docs/static/resources/openapi.json index 1be46e8d1146..e4e1d91dd171 100644 --- a/docs/static/resources/openapi.json +++ b/docs/static/resources/openapi.json @@ -34264,6 +34264,7 @@ "schema": { "properties": { "archived_count": { + "description": "Archived dependents are included in inaccessible_count; these counts overlap and must not be added together.", "type": "integer" }, "dependents": { @@ -34653,6 +34654,7 @@ "schema": { "properties": { "archived_count": { + "description": "Archived dependents are included in inaccessible_count; these counts overlap and must not be added together.", "type": "integer" }, "dependents": { @@ -34819,6 +34821,7 @@ "schema": { "properties": { "archived_count": { + "description": "Archived dependents are included in inaccessible_count; these counts overlap and must not be added together.", "type": "integer" }, "dependents": { diff --git a/superset-frontend/src/pages/DatabaseList/DatabaseList.semanticLayerDelete.test.tsx b/superset-frontend/src/pages/DatabaseList/DatabaseList.semanticLayerDelete.test.tsx index 57d61b402ff6..d7793fec08dc 100644 --- a/superset-frontend/src/pages/DatabaseList/DatabaseList.semanticLayerDelete.test.tsx +++ b/superset-frontend/src/pages/DatabaseList/DatabaseList.semanticLayerDelete.test.tsx @@ -245,10 +245,12 @@ test('a genuinely empty layer says so instead of warning about nonexistent views renderDatabaseList(); const dialog = await openDeleteModal(); + expect(dialog).toHaveTextContent('This cannot be undone.'); + expect(dialog).not.toHaveTextContent('Deletion is blocked'); expect( within(dialog).getByText( - 'This semantic layer has no dependent semantic views.', + 'This semantic layer has no dependent semantic views. This cannot be undone.', ), ).toBeInTheDocument(); expect( @@ -443,7 +445,7 @@ test("a stale lookup resolving late cannot replace a newer row's modal", async ( test('layer delete displays the counted dependency conflict', async () => { const reason = - 'Semantic source is used by 25 dependent assets and cannot be deleted. 3 dependent assets are archived. Go to Recently Archived to restore and repoint archived assets, or delete them permanently.'; + 'Semantic source is used by 25 dependent assets and cannot be deleted. 3 dependent assets are archived. Go to Recently Archived to restore and repoint archived assets, or delete them permanently, or ask an admin for help.'; setupMocks({ dependents: [], deleteError: reason }); const addDangerToast = jest.fn(); renderDatabaseList(mockUser, addDangerToast); diff --git a/superset-frontend/src/pages/DatabaseList/index.tsx b/superset-frontend/src/pages/DatabaseList/index.tsx index 591cdb3537c8..0dcc3bad7243 100644 --- a/superset-frontend/src/pages/DatabaseList/index.tsx +++ b/superset-frontend/src/pages/DatabaseList/index.tsx @@ -148,7 +148,13 @@ function SemanticLayerCascadeWarning({ // never access-filtered. An honest empty message keeps the destructive // warning credible for the layers where it matters. if (preview.dependentViewCount === 0) { - return

{t('This semantic layer has no dependent semantic views.')}

; + return ( +

+ {t( + 'This semantic layer has no dependent semantic views. This cannot be undone.', + )} +

+ ); } const listedViewCount = preview.dependentViewNames.length; @@ -1181,11 +1187,14 @@ function DatabaseList({ {slDeletePreview.item.database_name}?

-

- {t( - 'Deletion is blocked while any assets depend on these views, including archived assets. Remove or repoint them first. Use Recently Archived to restore and repoint archived assets, or delete them permanently.', - )} -

+ {(slDeletePreview.status === 'failed' || + slDeletePreview.dependentViewCount > 0) && ( +

+ {t( + 'Deletion is blocked while any assets depend on these views, including archived assets. Remove or repoint them first. Use Recently Archived to restore and repoint archived assets, or delete them permanently, or ask an admin for help.', + )} +

+ )} } onConfirm={() => { diff --git a/superset-frontend/src/pages/DatasetList/DatasetList.behavior.test.tsx b/superset-frontend/src/pages/DatasetList/DatasetList.behavior.test.tsx index cf38acb1b6b2..de1db7bd4dc8 100644 --- a/superset-frontend/src/pages/DatasetList/DatasetList.behavior.test.tsx +++ b/superset-frontend/src/pages/DatasetList/DatasetList.behavior.test.tsx @@ -81,7 +81,7 @@ test('single semantic view delete displays the counted dependency conflict', asy kind: 'semantic_view', }; const reason = - 'Semantic source is used by 25 dependent assets and cannot be deleted. 3 dependent assets are archived. Go to Recently Archived to restore and repoint archived assets, or delete them permanently.'; + 'Semantic source is used by 25 dependent assets and cannot be deleted. 3 dependent assets are archived. Go to Recently Archived to restore and repoint archived assets, or delete them permanently, or ask an admin for help.'; const addDangerToast = jest.fn(); mockDatasetListEndpoints({ result: [view], count: 1 }); fetchMock.delete('glob:*/api/v1/semantic_view/99', { @@ -119,7 +119,7 @@ test('mixed bulk delete names refused semantic views and their conflict reason', }; const addDangerToast = jest.fn(); const reason = - 'Semantic source is used by 25 dependent assets and cannot be deleted. 3 dependent assets are archived. Go to Recently Archived to restore and repoint archived assets, or delete them permanently.'; + 'Semantic source is used by 25 dependent assets and cannot be deleted. 3 dependent assets are archived. Go to Recently Archived to restore and repoint archived assets, or delete them permanently, or ask an admin for help.'; mockDatasetListEndpoints({ result: [dataset, semanticView], count: 2 }); fetchMock.get(API_ENDPOINTS.DATASET_BULK_RELATED_OBJECTS, { charts: { count: 0, result: [] }, diff --git a/superset-frontend/src/pages/DatasetList/index.tsx b/superset-frontend/src/pages/DatasetList/index.tsx index 398ed8678602..8b0aeacb3f25 100644 --- a/superset-frontend/src/pages/DatasetList/index.tsx +++ b/superset-frontend/src/pages/DatasetList/index.tsx @@ -1602,7 +1602,7 @@ const DatasetList: FunctionComponent = ({

{t( - 'Deletion is blocked while any assets depend on this view, including archived assets. Remove or repoint them first. Use Recently Archived to restore and repoint archived assets, or delete them permanently.', + 'Deletion is blocked while any assets depend on this view, including archived assets. Remove or repoint them first. Use Recently Archived to restore and repoint archived assets, or delete them permanently, or ask an admin for help.', )}

@@ -1730,7 +1730,7 @@ const DatasetList: FunctionComponent = ({ {pendingBulkSemanticCount > 0 && (

{t( - 'Deletion is blocked while any assets depend on the selected semantic views, including archived assets. Remove or repoint them first. Use Recently Archived to restore and repoint archived assets, or delete them permanently.', + 'Deletion is blocked while any assets depend on the selected semantic views, including archived assets. Remove or repoint them first. Use Recently Archived to restore and repoint archived assets, or delete them permanently, or ask an admin for help.', )}

)} diff --git a/superset/commands/semantic_layer/delete.py b/superset/commands/semantic_layer/delete.py index 2f4ea894cc65..17f150d52365 100644 --- a/superset/commands/semantic_layer/delete.py +++ b/superset/commands/semantic_layer/delete.py @@ -101,6 +101,8 @@ def _dashboard_target_ids(view_ids: sa.Select) -> set[int]: selected_ids: set[int] = set(db.session.execute(view_ids).scalars()) if not selected_ids: return set() + # Core columns bypass the ORM do_orm_execute soft-delete filter so archived + # dashboards remain deletion blockers, including metadata-only targets. candidates: sa.Select = sa.select( Dashboard.__table__.c.id, Dashboard.__table__.c.json_metadata ).where( @@ -119,6 +121,8 @@ def _dependent_assets( view_ids: sa.Select, ) -> tuple[int, list[dict[str, str | int]], int, int]: """Count dependents, naming only assets visible through their list APIs.""" + # Core table selects bypass the ORM do_orm_execute soft-delete filter: + # archived charts and dashboards must still block semantic-source deletion. chart_ids: sa.Select = sa.select(Slice.__table__.c.id).where( Slice.__table__.c.datasource_type == "semantic_view", Slice.__table__.c.datasource_id.in_(view_ids), diff --git a/superset/commands/semantic_layer/exceptions.py b/superset/commands/semantic_layer/exceptions.py index 40d0bd3939f5..3aa2690d87b0 100644 --- a/superset/commands/semantic_layer/exceptions.py +++ b/superset/commands/semantic_layer/exceptions.py @@ -112,6 +112,6 @@ def __init__( ) message += " " + _( "Go to Recently Archived to restore and repoint archived assets, " - "or delete them permanently." + "or delete them permanently, or ask an admin for help." ) super().__init__(message) diff --git a/superset/semantic_layers/api.py b/superset/semantic_layers/api.py index 1559fd9e86ba..8346631d8675 100644 --- a/superset/semantic_layers/api.py +++ b/superset/semantic_layers/api.py @@ -548,6 +548,9 @@ def delete(self, pk: int) -> Response: type: integer archived_count: type: integer + description: >- + Archived dependents are included in inaccessible_count; + these counts overlap and must not be added together. 422: $ref: '#/components/responses/422' """ @@ -649,6 +652,9 @@ def bulk_delete(self, **kwargs: Any) -> Response: type: integer archived_count: type: integer + description: >- + Archived dependents are included in inaccessible_count; + these counts overlap and must not be added together. 422: $ref: '#/components/responses/422' """ @@ -1180,6 +1186,9 @@ def delete(self, uuid: str) -> FlaskResponse: type: integer archived_count: type: integer + description: >- + Archived dependents are included in inaccessible_count; + these counts overlap and must not be added together. 422: $ref: '#/components/responses/422' """ diff --git a/superset/translations/messages.pot b/superset/translations/messages.pot index a897b4b5afda..67c57ac440df 100644 --- a/superset/translations/messages.pot +++ b/superset/translations/messages.pot @@ -15128,9 +15128,6 @@ msgstr "" msgid "This section contains validation errors" msgstr "" -msgid "This semantic layer has no dependent semantic views." -msgstr "" - msgid "This semantic provider cannot apply the requested offset. Turn off pagination or use a supported limit and offset." msgstr "" @@ -18265,43 +18262,46 @@ msgid "" msgstr "" +msgid "Remove or repoint these assets before deleting the semantic source." +msgstr "" + + +#, python-format +msgid "" +"This will also permanently delete its %s semantic view. This cannot be " +"undone." +msgid_plural "" +"This will also permanently delete its %s semantic views. This cannot be " +"undone." +msgstr[0] "" +msgstr[1] "" + msgid "" "Deletion is blocked while any assets depend on the selected semantic views, " "including archived assets. Remove or repoint them first. Use Recently " -"Archived to restore and repoint archived assets, or delete them permanently." +"Archived to restore and repoint archived assets, or delete them permanently," +" or ask an admin for help." msgstr "" - msgid "" "Deletion is blocked while any assets depend on these views, including " "archived assets. Remove or repoint them first. Use Recently Archived to " -"restore and repoint archived assets, or delete them permanently." +"restore and repoint archived assets, or delete them permanently, or ask an " +"admin for help." msgstr "" - msgid "" "Deletion is blocked while any assets depend on this view, including archived" " assets. Remove or repoint them first. Use Recently Archived to restore and " -"repoint archived assets, or delete them permanently." +"repoint archived assets, or delete them permanently, or ask an admin for " +"help." msgstr "" - msgid "" "Go to Recently Archived to restore and repoint archived assets, or delete " -"them permanently." -msgstr "" - - -msgid "Remove or repoint these assets before deleting the semantic source." +"them permanently, or ask an admin for help." msgstr "" - -#, python-format msgid "" -"This will also permanently delete its %s semantic view. This cannot be " -"undone." -msgid_plural "" -"This will also permanently delete its %s semantic views. This cannot be " -"undone." -msgstr[0] "" -msgstr[1] "" +"This semantic layer has no dependent semantic views. This cannot be undone." +msgstr "" diff --git a/tests/unit_tests/commands/semantic_layer/delete_test.py b/tests/unit_tests/commands/semantic_layer/delete_test.py index fa8f7048c9eb..8c094a62f98d 100644 --- a/tests/unit_tests/commands/semantic_layer/delete_test.py +++ b/tests/unit_tests/commands/semantic_layer/delete_test.py @@ -1068,3 +1068,64 @@ def test_archived_chart_blocks_semantic_delete( assert "Recently Archived" in str(exc_info.value) layer_dao.delete.assert_not_called() view_dao.delete.assert_not_called() + + +@pytest.mark.parametrize( + "path", ["membership", "native_filter_configuration", "chart_customization_config"] +) +def test_archived_dashboard_blocks_delete_with_orm_listener( + session: Session, mocker: MockerFixture, path: str +) -> None: + """Core dependency queries must retain dashboards hidden by the ORM listener.""" + from datetime import datetime + + from superset.commands.semantic_layer.delete import _raise_for_dependents + from superset.models.dashboard import Dashboard + from superset.models.slice import Slice + from superset.utils import json + + Dashboard.metadata.create_all(session.get_bind()) + session.execute( + Dashboard.__table__.insert().values( + id=950, + dashboard_title="Archived dashboard", + deleted_at=datetime(2026, 1, 1), + json_metadata=json.dumps( + { + path: [ + { + "targets": [ + {"datasourceType": "semantic_view", "datasetId": 42} + ] + } + ] + } + ) + if path != "membership" + else "{}", + ) + ) + if path == "membership": + session.execute( + Slice.__table__.insert().values( + id=951, + slice_name="Member", + datasource_type="semantic_view", + datasource_id=42, + ) + ) + session.execute( + sa.text( + "INSERT INTO dashboard_slices (dashboard_id, slice_id) " + "VALUES (950, 951)" + ) + ) + # Prove the session listener is active; do not substitute Connection.execute. + assert session.scalar(sa.select(Dashboard.id).where(Dashboard.id == 950)) is None + mocker.patch("superset.security_manager.can_access", return_value=False) + exc_info: pytest.ExceptionInfo[SemanticDeleteDependentsError] + with pytest.raises(SemanticDeleteDependentsError) as exc_info: + _raise_for_dependents(sa.select(sa.literal(42))) + assert exc_info.value.total == (2 if path == "membership" else 1) + assert exc_info.value.archived_count == 1 + assert exc_info.value.dependents == [] diff --git a/tests/unit_tests/semantic_layers/api_test.py b/tests/unit_tests/semantic_layers/api_test.py index cb6bd20bcead..6115f2a607c8 100644 --- a/tests/unit_tests/semantic_layers/api_test.py +++ b/tests/unit_tests/semantic_layers/api_test.py @@ -1056,7 +1056,7 @@ def test_semantic_delete_reports_dependents_as_conflict( "Remove or repoint these assets before deleting the semantic source. " "3 dependent assets are archived. " "Go to Recently Archived to restore and repoint archived assets, " - "or delete them permanently." + "or delete them permanently, or ask an admin for help." ) From da0f8246475774750dc97c39eea3c79367bf9e4e Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Wed, 21 Oct 2026 17:00:15 -0600 Subject: [PATCH 03/12] fix(semantic): keep deletion conflicts in confirmation dialogs --- .../docs/using-superset/recently-archived.mdx | 6 ++ .../DeleteModal/DeleteModal.test.tsx | 26 ++++++ .../src/components/DeleteModal/index.tsx | 7 +- .../src/components/DeleteModal/types.ts | 2 + .../DatabaseList.semanticLayerDelete.test.tsx | 29 +++++-- .../src/pages/DatabaseList/index.tsx | 87 ++++++++++++++----- .../DatasetList/DatasetList.behavior.test.tsx | 26 ++++-- .../src/pages/DatasetList/index.tsx | 80 ++++++++++++----- superset/translations/messages.pot | 20 ++--- 9 files changed, 213 insertions(+), 70 deletions(-) diff --git a/docs/docs/using-superset/recently-archived.mdx b/docs/docs/using-superset/recently-archived.mdx index 5b08cd893d5b..da5c0b1935ac 100644 --- a/docs/docs/using-superset/recently-archived.mdx +++ b/docs/docs/using-superset/recently-archived.mdx @@ -91,6 +91,12 @@ they depend on. The refusal includes the number of archived dependents. Use before deleting the semantic source. If you cannot access those assets, ask an admin for help. +When deleting a semantic layer or view, a dependency refusal stays in the +confirmation dialog with the total and archived counts and a link to Recently +Archived. The typed confirmation is preserved so you can retry after removing +or repointing the dependents. Dependencies are checked when you confirm deletion, +not before you type the confirmation. + Use the **Delete permanently** action on a row to remove an object for good. You will be asked to confirm. diff --git a/superset-frontend/packages/superset-ui-core/src/components/DeleteModal/DeleteModal.test.tsx b/superset-frontend/packages/superset-ui-core/src/components/DeleteModal/DeleteModal.test.tsx index 07e329d22ecd..30f28b7ca3c1 100644 --- a/superset-frontend/packages/superset-ui-core/src/components/DeleteModal/DeleteModal.test.tsx +++ b/superset-frontend/packages/superset-ui-core/src/components/DeleteModal/DeleteModal.test.tsx @@ -299,3 +299,29 @@ test('confirmation reset key clears and re-arms type-to-confirm', async () => { expect(screen.getByTestId('delete-modal-input')).toHaveValue(''); expect(screen.getByRole('button', { name: 'Delete' })).toBeDisabled(); }); + +test.each([false, true])( + 'confirmation preservation is opt-in: %s', + async preserveConfirmationOnConfirm => { + const onConfirm = jest.fn(); + render( + , + ); + const input = screen.getByTestId('delete-modal-input'); + await userEvent.type(input, 'DELETE'); + await userEvent.click(screen.getByTestId('modal-confirm-button')); + expect(onConfirm).toHaveBeenCalledTimes(1); + expect(input).toHaveValue(preserveConfirmationOnConfirm ? 'DELETE' : ''); + await userEvent.click(screen.getByTestId('close-modal-btn')); + expect(input).toHaveValue(''); + }, +); diff --git a/superset-frontend/packages/superset-ui-core/src/components/DeleteModal/index.tsx b/superset-frontend/packages/superset-ui-core/src/components/DeleteModal/index.tsx index 6acb1104a8ec..f2350ee8d2d1 100644 --- a/superset-frontend/packages/superset-ui-core/src/components/DeleteModal/index.tsx +++ b/superset-frontend/packages/superset-ui-core/src/components/DeleteModal/index.tsx @@ -46,6 +46,7 @@ export function DeleteModal({ disableConfirmationInput = false, loading = false, confirmationResetKey, + preserveConfirmationOnConfirm = false, }: DeleteModalProps) { // Recoverable (archive) deletes drop the "type DELETE to confirm" step; // a permanent delete keeps it. @@ -77,8 +78,10 @@ export function DeleteModal({ }; const confirm = () => { - setConfirmation(''); - setDisableChange(true); + if (!preserveConfirmationOnConfirm) { + setConfirmation(''); + setDisableChange(true); + } onConfirm(); }; diff --git a/superset-frontend/packages/superset-ui-core/src/components/DeleteModal/types.ts b/superset-frontend/packages/superset-ui-core/src/components/DeleteModal/types.ts index a62917e66547..d3c337d502da 100644 --- a/superset-frontend/packages/superset-ui-core/src/components/DeleteModal/types.ts +++ b/superset-frontend/packages/superset-ui-core/src/components/DeleteModal/types.ts @@ -55,4 +55,6 @@ export interface DeleteModalProps { loading?: boolean; /** Clear and re-arm the typed-text gate when the reviewed data changes. */ confirmationResetKey?: string | number; + /** Keep typed confirmation until dismissal or an explicit reset. */ + preserveConfirmationOnConfirm?: boolean; } diff --git a/superset-frontend/src/pages/DatabaseList/DatabaseList.semanticLayerDelete.test.tsx b/superset-frontend/src/pages/DatabaseList/DatabaseList.semanticLayerDelete.test.tsx index d7793fec08dc..540f9c7b3706 100644 --- a/superset-frontend/src/pages/DatabaseList/DatabaseList.semanticLayerDelete.test.tsx +++ b/superset-frontend/src/pages/DatabaseList/DatabaseList.semanticLayerDelete.test.tsx @@ -137,7 +137,12 @@ const setupMocks = ({ } fetchMock.delete( DELETE_ROUTE, - deleteError ? { status: 409, body: { message: deleteError } } : {}, + deleteError + ? { + status: 409, + body: { message: deleteError, total: 25, archived_count: 3 }, + } + : {}, ); }; @@ -202,6 +207,7 @@ test('delete confirmation warns about cascade-deleting dependent views by count renderDatabaseList(); const dialog = await openDeleteModal(); + expect(dialog).not.toHaveTextContent('Deletion is blocked'); expect( within(dialog).getByText( @@ -455,11 +461,24 @@ test('layer delete displays the counted dependency conflict', async () => { 'DELETE', ); await userEvent.click(within(dialog).getByRole('button', { name: 'Delete' })); - await waitFor(() => - expect(addDangerToast).toHaveBeenCalledWith( - `There was an issue deleting Demo Semantic Layer: ${reason}`, - ), + const alert = await within(dialog).findByRole('alert'); + expect(alert).toHaveTextContent('25 dependent assets'); + expect(alert).toHaveTextContent('3 archived'); + expect( + within(alert).getByRole('link', { name: 'Recently Archived' }), + ).toHaveAttribute('href', '/archived/'); + expect(within(dialog).getByTestId('delete-modal-input')).toHaveValue( + 'DELETE', ); + expect(addDangerToast).not.toHaveBeenCalled(); + await userEvent.click(within(dialog).getByRole('button', { name: 'Cancel' })); + await openDeleteModal(); + expect( + within(screen.getByRole('dialog')).queryByRole('alert'), + ).not.toBeInTheDocument(); + expect( + within(screen.getByRole('dialog')).getByTestId('delete-modal-input'), + ).toHaveValue(''); }); test('confirming the modal deletes the semantic layer', async () => { diff --git a/superset-frontend/src/pages/DatabaseList/index.tsx b/superset-frontend/src/pages/DatabaseList/index.tsx index 0dcc3bad7243..27a9c2715376 100644 --- a/superset-frontend/src/pages/DatabaseList/index.tsx +++ b/superset-frontend/src/pages/DatabaseList/index.tsx @@ -16,10 +16,12 @@ * specific language governing permissions and limitations * under the License. */ +import { Alert } from '@apache-superset/core/components'; import { t, tn } from '@apache-superset/core/translation'; import { getExtensionsRegistry, SupersetClient, + getClientErrorObject, isFeatureEnabled, FeatureFlag, } from '@superset-ui/core'; @@ -422,6 +424,13 @@ function DatabaseList({ const [slCurrentlyEditing, setSlCurrentlyEditing] = useState( null, ); + const [slDeleteError, setSLDeleteError] = useState<{ + error: string; + total?: number; + archived_count?: number; + } | null>(null); + const [slDeleting, setSLDeleting] = useState(false); + const slDeleteRequest = useRef(0); const [slDeletePreview, setSlDeletePreview] = useState(null); @@ -481,6 +490,9 @@ function DatabaseList({ const slDeleteLookupRef = useRef(0); const openSemanticLayerDeleteModal = useCallback((item: ConnectionItem) => { slDeleteLookupRef.current += 1; + slDeleteRequest.current += 1; + setSLDeleteError(null); + setSLDeleting(false); const lookupId = slDeleteLookupRef.current; setSlDeletePreview({ status: 'loading', item }); return SupersetClient.get({ @@ -786,21 +798,34 @@ function DatabaseList({ const initialSort = [{ id: 'changed_on_delta_humanized', desc: true }]; - function handleSemanticLayerDelete(item: ConnectionItem) { - SupersetClient.delete({ - endpoint: `/api/v1/semantic_layer/${item.uuid}`, - }).then( - () => { - refreshData(); - addSuccessToast(t('Deleted: %s', item.database_name)); - setSlDeletePreview(null); - }, - createErrorHandler(errMsg => + async function handleSemanticLayerDelete(item: ConnectionItem) { + slDeleteRequest.current += 1; + const request = slDeleteRequest.current; + setSLDeleting(true); + try { + await SupersetClient.delete({ + endpoint: `/api/v1/semantic_layer/${item.uuid}`, + }); + refreshData(); + addSuccessToast(t('Deleted: %s', item.database_name)); + if (request === slDeleteRequest.current) setSlDeletePreview(null); + } catch (error) { + const parsed = await getClientErrorObject(error); + if (request !== slDeleteRequest.current) return; + if (parsed.status === 409) { + setSLDeleteError(parsed); + } else { addDangerToast( - t('There was an issue deleting %s: %s', item.database_name, errMsg), - ), - ), - ); + t( + 'There was an issue deleting %s: %s', + item.database_name, + parsed.error, + ), + ); + } + } finally { + if (request === slDeleteRequest.current) setSLDeleting(false); + } } const columns = useMemo( @@ -1180,6 +1205,8 @@ function DatabaseList({ /> {slDeletePreview && slDeletePreview.status !== 'loading' && (

@@ -1187,20 +1214,36 @@ function DatabaseList({ {slDeletePreview.item.database_name}?

- {(slDeletePreview.status === 'failed' || - slDeletePreview.dependentViewCount > 0) && ( -

- {t( - 'Deletion is blocked while any assets depend on these views, including archived assets. Remove or repoint them first. Use Recently Archived to restore and repoint archived assets, or delete them permanently, or ask an admin for help.', - )} -

+ {slDeleteError && ( + + {typeof slDeleteError.total === 'number' && + typeof slDeleteError.archived_count === 'number' + ? t( + 'Deletion is blocked by %s dependent assets (%s archived). Remove or repoint them before deleting.', + slDeleteError.total, + slDeleteError.archived_count, + ) + : slDeleteError.error}{' '} + + {t('Recently Archived')} + + + } + /> )} } onConfirm={() => { handleSemanticLayerDelete(slDeletePreview.item); }} - onHide={() => setSlDeletePreview(null)} + onHide={() => { + slDeleteRequest.current += 1; + setSlDeletePreview(null); + }} open title={ - expect(addDangerToast).toHaveBeenCalledWith( - `There was an issue deleting orders_semantic: ${reason}`, - ), - ); + const alert = await within(modal).findByRole('alert'); + expect(alert).toHaveTextContent('25 dependent assets'); + expect(alert).toHaveTextContent('3 archived'); + expect( + within(alert).getByRole('link', { name: 'Recently Archived' }), + ).toHaveAttribute('href', '/archived/'); + expect(within(modal).getByTestId('delete-modal-input')).toHaveValue('DELETE'); + expect(addDangerToast).not.toHaveBeenCalled(); + await userEvent.click(within(modal).getByRole('button', { name: 'Cancel' })); + await userEvent.click(screen.getByTestId('dataset-row-delete')); + await screen.findByRole('dialog'); + expect( + within(screen.getByRole('dialog')).queryByRole('alert'), + ).not.toBeInTheDocument(); + expect( + within(screen.getByRole('dialog')).getByTestId('delete-modal-input'), + ).toHaveValue(''); }); test('mixed bulk delete names refused semantic views and their conflict reason', async () => { diff --git a/superset-frontend/src/pages/DatasetList/index.tsx b/superset-frontend/src/pages/DatasetList/index.tsx index 8b0aeacb3f25..e206e46bdac5 100644 --- a/superset-frontend/src/pages/DatasetList/index.tsx +++ b/superset-frontend/src/pages/DatasetList/index.tsx @@ -16,10 +16,12 @@ * specific language governing permissions and limitations * under the License. */ +import { Alert } from '@apache-superset/core/components'; import { t, tn } from '@apache-superset/core/translation'; import { getExtensionsRegistry, SupersetClient, + getClientErrorObject, isFeatureEnabled, FeatureFlag, } from '@superset-ui/core'; @@ -628,6 +630,13 @@ const DatasetList: FunctionComponent = ({ null, ); + const [svDeleteError, setSVDeleteError] = useState<{ + error: string; + total?: number; + archived_count?: number; + } | null>(null); + const [svDeleting, setSVDeleting] = useState(false); + const svDeleteRequest = useRef(0); const [svCurrentlyDeleting, setSvCurrentlyDeleting] = useState(null); @@ -817,26 +826,36 @@ const DatasetList: FunctionComponent = ({ ); const handleSemanticViewDelete = (sv: Dataset) => { + svDeleteRequest.current += 1; + setSVDeleteError(null); + setSVDeleting(false); setSvCurrentlyDeleting(sv); }; - const handleSemanticViewDeleteConfirm = () => { + const handleSemanticViewDeleteConfirm = async () => { if (!svCurrentlyDeleting) return; const { id, table_name: tableName } = svCurrentlyDeleting; - SupersetClient.delete({ - endpoint: `/api/v1/semantic_view/${id}`, - }).then( - () => { - setSvCurrentlyDeleting(null); - refreshData(); - addSuccessToast(t('Deleted: %s', tableName)); - }, - createErrorHandler(errMsg => + svDeleteRequest.current += 1; + const request = svDeleteRequest.current; + setSVDeleting(true); + try { + await SupersetClient.delete({ endpoint: `/api/v1/semantic_view/${id}` }); + refreshData(); + addSuccessToast(t('Deleted: %s', tableName)); + if (request === svDeleteRequest.current) setSvCurrentlyDeleting(null); + } catch (error) { + const parsed = await getClientErrorObject(error); + if (request !== svDeleteRequest.current) return; + if (parsed.status === 409) { + setSVDeleteError(parsed); + } else { addDangerToast( - t('There was an issue deleting %s: %s', tableName, errMsg), - ), - ), - ); + t('There was an issue deleting %s: %s', tableName, parsed.error), + ); + } + } finally { + if (request === svDeleteRequest.current) setSVDeleting(false); + } }; const columns = useMemo( @@ -1592,6 +1611,8 @@ const DatasetList: FunctionComponent = ({ )} {svCurrentlyDeleting && (

@@ -1600,15 +1621,34 @@ const DatasetList: FunctionComponent = ({ svCurrentlyDeleting.table_name, )}

-

- {t( - 'Deletion is blocked while any assets depend on this view, including archived assets. Remove or repoint them first. Use Recently Archived to restore and repoint archived assets, or delete them permanently, or ask an admin for help.', - )} -

+ {svDeleteError && ( + + {typeof svDeleteError.total === 'number' && + typeof svDeleteError.archived_count === 'number' + ? t( + 'Deletion is blocked by %s dependent assets (%s archived). Remove or repoint them before deleting.', + svDeleteError.total, + svDeleteError.archived_count, + ) + : svDeleteError.error}{' '} + + {t('Recently Archived')} + + + } + /> + )} } onConfirm={handleSemanticViewDeleteConfirm} - onHide={() => setSvCurrentlyDeleting(null)} + onHide={() => { + svDeleteRequest.current += 1; + setSvCurrentlyDeleting(null); + }} open title={t('Delete Semantic View?')} /> diff --git a/superset/translations/messages.pot b/superset/translations/messages.pot index 67c57ac440df..4cdd2b3f1d23 100644 --- a/superset/translations/messages.pot +++ b/superset/translations/messages.pot @@ -18283,20 +18283,6 @@ msgid "" " or ask an admin for help." msgstr "" -msgid "" -"Deletion is blocked while any assets depend on these views, including " -"archived assets. Remove or repoint them first. Use Recently Archived to " -"restore and repoint archived assets, or delete them permanently, or ask an " -"admin for help." -msgstr "" - -msgid "" -"Deletion is blocked while any assets depend on this view, including archived" -" assets. Remove or repoint them first. Use Recently Archived to restore and " -"repoint archived assets, or delete them permanently, or ask an admin for " -"help." -msgstr "" - msgid "" "Go to Recently Archived to restore and repoint archived assets, or delete " "them permanently, or ask an admin for help." @@ -18305,3 +18291,9 @@ msgstr "" msgid "" "This semantic layer has no dependent semantic views. This cannot be undone." msgstr "" + +#, python-format +msgid "" +"Deletion is blocked by %s dependent assets (%s archived). Remove or repoint " +"them before deleting." +msgstr "" From 2746774f67217b97f8ad3e56ac8cb2dbe3b3d44f Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Thu, 22 Oct 2026 12:02:15 -0600 Subject: [PATCH 04/12] fix(semantic): name the assets that block a semantic layer or view delete The deletion refusal already returns up to 20 visible dependents, but the dialogs showed counts only, and archived dependents were never named, even to admins, because visibility was computed over live rows only. Name archived charts and dashboards to the audience Recently Archived shows them to (admins and the asset's editors), flag each dependent as archived, and list the dependents in the layer and view delete dialogs with links for live assets, an "and N more" overflow, and an "ask an admin" hint when some dependents are hidden. Also replace "Semantic source" in the refusal, bold the view name and add the trash icon to the view dialog, and align "Recently Archived" capitalisation. Co-Authored-By: Claude Opus 5.5 --- .../docs/using-superset/recently-archived.mdx | 11 +- .../semanticLayers/SemanticDeleteBlockers.tsx | 115 ++++++++++++++++++ .../DatabaseList.semanticLayerDelete.test.tsx | 2 +- .../src/pages/DatabaseList/index.tsx | 37 ++---- .../DatasetList/DatasetList.behavior.test.tsx | 47 +++++++ .../src/pages/DatasetList/index.tsx | 46 +++---- superset/commands/semantic_layer/delete.py | 53 +++++--- .../commands/semantic_layer/exceptions.py | 8 +- superset/semantic_layers/api.py | 30 +++-- superset/translations/messages.pot | 33 +++-- .../commands/semantic_layer/delete_test.py | 89 +++++++++++++- tests/unit_tests/semantic_layers/api_test.py | 4 +- 12 files changed, 365 insertions(+), 110 deletions(-) create mode 100644 superset-frontend/src/features/semanticLayers/SemanticDeleteBlockers.tsx diff --git a/docs/docs/using-superset/recently-archived.mdx b/docs/docs/using-superset/recently-archived.mdx index da5c0b1935ac..755ec34c34d2 100644 --- a/docs/docs/using-superset/recently-archived.mdx +++ b/docs/docs/using-superset/recently-archived.mdx @@ -88,12 +88,15 @@ filter cleanup; the response warns that the dashboard's filters need review. Archived charts and dashboards still block deletion of semantic views and layers they depend on. The refusal includes the number of archived dependents. Use **Recently Archived** to restore and repoint them, or permanently delete them, -before deleting the semantic source. If you cannot access those assets, ask an -admin for help. +before deleting the semantic layer or view. If you cannot access those assets, +ask an admin for help. When deleting a semantic layer or view, a dependency refusal stays in the -confirmation dialog with the total and archived counts and a link to Recently -Archived. The typed confirmation is preserved so you can retry after removing +confirmation dialog with the total and archived counts, the names of up to 20 +blocking assets you can see, and a link to Recently Archived. Archived blockers +are named only to users who could restore them from Recently Archived (admins +and the asset's editors); the dialog counts the others and suggests asking an +admin. The typed confirmation is preserved so you can retry after removing or repointing the dependents. Dependencies are checked when you confirm deletion, not before you type the confirmation. diff --git a/superset-frontend/src/features/semanticLayers/SemanticDeleteBlockers.tsx b/superset-frontend/src/features/semanticLayers/SemanticDeleteBlockers.tsx new file mode 100644 index 000000000000..ccea4b03ef50 --- /dev/null +++ b/superset-frontend/src/features/semanticLayers/SemanticDeleteBlockers.tsx @@ -0,0 +1,115 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { t } from '@apache-superset/core/translation'; +import { Alert } from '@apache-superset/core/components'; +import { ensureAppRoot } from 'src/utils/navigationUtils'; + +interface Dependent { + type: string; + id: number; + name: string; + archived?: boolean; +} + +export interface SemanticDeleteError { + error: string; + total?: number; + archived_count?: number; + inaccessible_count?: number; + dependents?: Dependent[]; +} + +const typeLabel = (type: string): string => { + const labels: Record = { + chart: t('chart'), + dashboard: t('dashboard'), + alert: t('alert'), + report: t('report'), + }; + return labels[type] ?? type; +}; + +const LIVE_LINKS: Record string> = { + chart: id => `/explore/?slice_id=${id}`, + dashboard: id => `/superset/dashboard/${id}/`, +}; + +/** The refusal shown inside a semantic layer or view delete dialog. */ +export default function SemanticDeleteBlockers({ + error, +}: { + error: SemanticDeleteError; +}) { + const dependents = error.dependents ?? []; + const hidden = error.inaccessible_count ?? 0; + const unlisted = (error.total ?? 0) - hidden - dependents.length; + const counted = + typeof error.total === 'number' && typeof error.archived_count === 'number'; + const summary = counted + ? t( + 'Deletion is blocked by %s dependent assets (%s archived). Remove or repoint them before deleting.', + error.total, + error.archived_count, + ) + : error.error; + // Archived assets have no page to open; Recently Archived lists them. + const href = (item: Dependent): string | undefined => + item.archived ? undefined : LIVE_LINKS[item.type]?.(item.id); + return ( + + {dependents.length > 0 && ( +
    + {dependents.map(item => ( +
  • + {href(item) ? ( + + {item.name} + + ) : ( + item.name + )}{' '} + ( + {item.archived + ? t('archived %s', typeLabel(item.type)) + : typeLabel(item.type)} + ) +
    • + ))} +
+ )} + {unlisted > 0 &&

{t('and %s more', unlisted)}

} + {hidden > 0 && ( +

+ {t( + "%s dependent asset(s) you can't see. Ask an admin for help.", + hidden, + )} +

+ )} + {t('Recently Archived')} + + } + /> + ); +} diff --git a/superset-frontend/src/pages/DatabaseList/DatabaseList.semanticLayerDelete.test.tsx b/superset-frontend/src/pages/DatabaseList/DatabaseList.semanticLayerDelete.test.tsx index 540f9c7b3706..5ab00a8e131c 100644 --- a/superset-frontend/src/pages/DatabaseList/DatabaseList.semanticLayerDelete.test.tsx +++ b/superset-frontend/src/pages/DatabaseList/DatabaseList.semanticLayerDelete.test.tsx @@ -256,7 +256,7 @@ test('a genuinely empty layer says so instead of warning about nonexistent views expect( within(dialog).getByText( - 'This semantic layer has no dependent semantic views. This cannot be undone.', + 'This semantic layer has no semantic views. This cannot be undone.', ), ).toBeInTheDocument(); expect( diff --git a/superset-frontend/src/pages/DatabaseList/index.tsx b/superset-frontend/src/pages/DatabaseList/index.tsx index 27a9c2715376..30208978babe 100644 --- a/superset-frontend/src/pages/DatabaseList/index.tsx +++ b/superset-frontend/src/pages/DatabaseList/index.tsx @@ -16,7 +16,6 @@ * specific language governing permissions and limitations * under the License. */ -import { Alert } from '@apache-superset/core/components'; import { t, tn } from '@apache-superset/core/translation'; import { getExtensionsRegistry, @@ -74,6 +73,9 @@ import { DatabaseObject } from 'src/features/databases/types'; import { QueryObjectColumns } from 'src/views/CRUD/types'; import { WIDER_DROPDOWN_WIDTH } from 'src/components/ListView/utils'; import { ModalTitleWithIcon } from 'src/components/ModalTitleWithIcon'; +import SemanticDeleteBlockers, { + SemanticDeleteError, +} from 'src/features/semanticLayers/SemanticDeleteBlockers'; import type User from 'src/types/User'; import { databaseLabel, @@ -152,9 +154,7 @@ function SemanticLayerCascadeWarning({ if (preview.dependentViewCount === 0) { return (

- {t( - 'This semantic layer has no dependent semantic views. This cannot be undone.', - )} + {t('This semantic layer has no semantic views. This cannot be undone.')}

); } @@ -424,11 +424,8 @@ function DatabaseList({ const [slCurrentlyEditing, setSlCurrentlyEditing] = useState( null, ); - const [slDeleteError, setSLDeleteError] = useState<{ - error: string; - total?: number; - archived_count?: number; - } | null>(null); + const [slDeleteError, setSLDeleteError] = + useState(null); const [slDeleting, setSLDeleting] = useState(false); const slDeleteRequest = useRef(0); const [slDeletePreview, setSlDeletePreview] = @@ -1215,25 +1212,7 @@ function DatabaseList({

{slDeleteError && ( - - {typeof slDeleteError.total === 'number' && - typeof slDeleteError.archived_count === 'number' - ? t( - 'Deletion is blocked by %s dependent assets (%s archived). Remove or repoint them before deleting.', - slDeleteError.total, - slDeleteError.archived_count, - ) - : slDeleteError.error}{' '} - - {t('Recently Archived')} - - - } - /> + )} } @@ -1273,7 +1252,7 @@ function DatabaseList({ databaseCurrentlyDeleting.datasets.count, )}{' '} {t( - 'Delete or move active datasets. To remove archived datasets, open Recently archived and select Delete permanently.', + 'Delete or move active datasets. To remove archived datasets, open Recently Archived and select Delete permanently.', )}

) : ( diff --git a/superset-frontend/src/pages/DatasetList/DatasetList.behavior.test.tsx b/superset-frontend/src/pages/DatasetList/DatasetList.behavior.test.tsx index 9634be0b6e76..33d8f33e4532 100644 --- a/superset-frontend/src/pages/DatasetList/DatasetList.behavior.test.tsx +++ b/superset-frontend/src/pages/DatasetList/DatasetList.behavior.test.tsx @@ -121,6 +121,53 @@ test('single semantic view delete displays the counted dependency conflict', asy ).toHaveValue(''); }); +test('single semantic view delete names its blockers, including archived ones', async () => { + const view = { + ...mockDatasets[0], + id: 99, + table_name: 'orders_semantic', + kind: 'semantic_view', + }; + mockDatasetListEndpoints({ result: [view], count: 1 }); + fetchMock.delete('glob:*/api/v1/semantic_view/99', { + status: 409, + body: { + message: 'blocked', + total: 5, + archived_count: 1, + inaccessible_count: 2, + dependents: [ + { type: 'chart', id: 7, name: 'Revenue chart', archived: false }, + { type: 'dashboard', id: 8, name: 'Old board', archived: true }, + ], + }, + }); + renderDatasetList(mockAdminUser, { + addDangerToast: jest.fn(), + addSuccessToast: jest.fn(), + }); + await screen.findByText(view.table_name); + await userEvent.click(screen.getByTestId('dataset-row-delete')); + const modal = await screen.findByRole('dialog'); + await userEvent.type( + within(modal).getByTestId('delete-modal-input'), + 'DELETE', + ); + await userEvent.click(within(modal).getByRole('button', { name: 'Delete' })); + const alert = await within(modal).findByRole('alert'); + expect( + within(alert).getByRole('link', { name: 'Revenue chart' }), + ).toHaveAttribute('href', '/explore/?slice_id=7'); + expect(alert).toHaveTextContent('Old board (archived dashboard)'); + expect( + within(alert).queryByRole('link', { name: 'Old board' }), + ).not.toBeInTheDocument(); + expect(alert).toHaveTextContent('and 1 more'); + expect(alert).toHaveTextContent( + "2 dependent asset(s) you can't see. Ask an admin for help.", + ); +}); + test('mixed bulk delete names refused semantic views and their conflict reason', async () => { const dataset = mockDatasets[0]; const semanticView = { diff --git a/superset-frontend/src/pages/DatasetList/index.tsx b/superset-frontend/src/pages/DatasetList/index.tsx index e206e46bdac5..565368090946 100644 --- a/superset-frontend/src/pages/DatasetList/index.tsx +++ b/superset-frontend/src/pages/DatasetList/index.tsx @@ -16,7 +16,6 @@ * specific language governing permissions and limitations * under the License. */ -import { Alert } from '@apache-superset/core/components'; import { t, tn } from '@apache-superset/core/translation'; import { getExtensionsRegistry, @@ -110,6 +109,10 @@ import { import { useSelector } from 'react-redux'; import { QueryObjectColumns } from 'src/views/CRUD/types'; import { WIDER_DROPDOWN_WIDTH } from 'src/components/ListView/utils'; +import { ModalTitleWithIcon } from 'src/components/ModalTitleWithIcon'; +import SemanticDeleteBlockers, { + SemanticDeleteError, +} from 'src/features/semanticLayers/SemanticDeleteBlockers'; import type { BootstrapData, UserWithPermissionsAndRoles, @@ -630,11 +633,8 @@ const DatasetList: FunctionComponent = ({ null, ); - const [svDeleteError, setSVDeleteError] = useState<{ - error: string; - total?: number; - archived_count?: number; - } | null>(null); + const [svDeleteError, setSVDeleteError] = + useState(null); const [svDeleting, setSVDeleting] = useState(false); const svDeleteRequest = useRef(0); const [svCurrentlyDeleting, setSvCurrentlyDeleting] = @@ -1616,31 +1616,12 @@ const DatasetList: FunctionComponent = ({ description={ <>

- {t( - 'Are you sure you want to delete %s? This cannot be undone.', - svCurrentlyDeleting.table_name, - )} + {t('Are you sure you want to delete')}{' '} + {svCurrentlyDeleting.table_name}?{' '} + {t('This cannot be undone.')}

{svDeleteError && ( - - {typeof svDeleteError.total === 'number' && - typeof svDeleteError.archived_count === 'number' - ? t( - 'Deletion is blocked by %s dependent assets (%s archived). Remove or repoint them before deleting.', - svDeleteError.total, - svDeleteError.archived_count, - ) - : svDeleteError.error}{' '} - - {t('Recently Archived')} - - - } - /> + )} } @@ -1650,7 +1631,12 @@ const DatasetList: FunctionComponent = ({ setSvCurrentlyDeleting(null); }} open - title={t('Delete Semantic View?')} + title={ + } + title={t('Delete Semantic View?')} + /> + } /> )} {datasetCurrentlyEditing && ( diff --git a/superset/commands/semantic_layer/delete.py b/superset/commands/semantic_layer/delete.py index 17f150d52365..0e34c30711ee 100644 --- a/superset/commands/semantic_layer/delete.py +++ b/superset/commands/semantic_layer/delete.py @@ -26,7 +26,7 @@ from sqlalchemy.exc import SQLAlchemyError from superset import db, security_manager -from superset.charts.filters import ChartFilter +from superset.charts.filters import ChartDeletedStateFilter, ChartFilter from superset.commands.base import BaseCommand from superset.commands.semantic_layer.exceptions import ( SemanticDeleteDependentsError, @@ -38,8 +38,12 @@ SemanticViewNotFoundError, ) from superset.commands.utils import current_user_can_modify_object +from superset.constants import SKIP_VISIBILITY_FILTER_CLASSES from superset.daos.semantic_layer import SemanticLayerDAO, SemanticViewDAO -from superset.dashboards.filters import DashboardAccessFilter +from superset.dashboards.filters import ( + DashboardAccessFilter, + DashboardDeletedStateFilter, +) from superset.exceptions import SupersetSecurityException from superset.models.dashboard import Dashboard, dashboard_slices from superset.models.slice import Slice @@ -51,6 +55,11 @@ logger = logging.getLogger(__name__) _DEPENDENT_LIMIT: int = 20 +# Visibility subqueries see archived charts and dashboards; the restore-audience +# filter then decides which of them may be named. +_INCLUDE_ARCHIVED: dict[str, set[type]] = { + SKIP_VISIBILITY_FILTER_CLASSES: {Slice, Dashboard} +} def _semantic_target_matches(target: object, view_ids: set[int]) -> bool: @@ -179,8 +188,16 @@ def _dependent_assets( visible_chart_ids: sa.Select = ( cast( sa.Select, - ChartFilter("id", SQLAInterface(Slice, db.session)) - .apply(db.session.query(Slice.id).filter(Slice.deleted_at.is_(None)), None) + # Archived rows are named only to their Recently Archived audience. + ChartDeletedStateFilter( # pylint: disable=protected-access + "id", SQLAInterface(Slice, db.session) + ) + ._scope_to_restore_audience( + ChartFilter("id", SQLAInterface(Slice, db.session)).apply( + db.session.query(Slice.id), None + ), + "include", + ) .statement, ) if security_manager.can_access("can_read", "Chart") @@ -189,10 +206,14 @@ def _dependent_assets( visible_dashboard_ids: sa.Select = ( cast( sa.Select, - DashboardAccessFilter("id", SQLAInterface(Dashboard, db.session)) - .apply( - db.session.query(Dashboard.id).filter(Dashboard.deleted_at.is_(None)), - None, + DashboardDeletedStateFilter( # pylint: disable=protected-access + "id", SQLAInterface(Dashboard, db.session) + ) + ._scope_to_restore_audience( + DashboardAccessFilter("id", SQLAInterface(Dashboard, db.session)).apply( + db.session.query(Dashboard.id), None + ), + "include", ) .statement, ) @@ -231,7 +252,8 @@ def _dependent_assets( sa.func.count(), sa.func.sum(sa.case((visibility, 1), else_=0)), sa.func.sum(sa.case((dependents.c.archived, 1), else_=0)), - ).select_from(dependents) + ).select_from(dependents), + execution_options=_INCLUDE_ARCHIVED, ).one(), ) total: int = int(counts[0]) @@ -239,10 +261,10 @@ def _dependent_assets( return 0, [], 0, 0 visible_total: int = int(counts[1] or 0) visible: sa.Subquery = sa.select(dependents).where(visibility).subquery() - rows: list[tuple[str, int, str]] = cast( - list[tuple[str, int, str]], + rows: list[tuple[str, int, str, bool]] = cast( + list[tuple[str, int, str, bool]], db.session.execute( - sa.select(visible.c.type, visible.c.id, visible.c.name) + sa.select(visible.c.type, visible.c.id, visible.c.name, visible.c.archived) .order_by( sa.case( (visible.c.type == "chart", 0), @@ -252,14 +274,15 @@ def _dependent_assets( visible.c.type, visible.c.id, ) - .limit(_DEPENDENT_LIMIT) + .limit(_DEPENDENT_LIMIT), + execution_options=_INCLUDE_ARCHIVED, ).all(), ) return ( total, [ - {"type": asset_type, "id": asset_id, "name": name} - for asset_type, asset_id, name in rows + {"type": asset_type, "id": asset_id, "name": name, "archived": archived} + for asset_type, asset_id, name, archived in rows ], total - visible_total, int(counts[2] or 0), diff --git a/superset/commands/semantic_layer/exceptions.py b/superset/commands/semantic_layer/exceptions.py index 3aa2690d87b0..f54e121cd182 100644 --- a/superset/commands/semantic_layer/exceptions.py +++ b/superset/commands/semantic_layer/exceptions.py @@ -93,16 +93,14 @@ def __init__( self.dependents: list[dict[str, str | int]] = dependents self.inaccessible_count: int = inaccessible_count message: str = ngettext( - "Semantic source is used by %(count)s dependent asset " + "This semantic layer or view is used by %(count)s dependent asset " "and cannot be deleted.", - "Semantic source is used by %(count)s dependent assets " + "This semantic layer or view is used by %(count)s dependent assets " "and cannot be deleted.", total, count=total, ) - message += " " + _( - "Remove or repoint these assets before deleting the semantic source." - ) + message += " " + _("Remove or repoint these assets before deleting it.") if archived_count: message += " " + ngettext( "%(count)s dependent asset is archived.", diff --git a/superset/semantic_layers/api.py b/superset/semantic_layers/api.py index 8346631d8675..0b6e7d07de9a 100644 --- a/superset/semantic_layers/api.py +++ b/superset/semantic_layers/api.py @@ -535,7 +535,7 @@ def delete(self, pk: int) -> Response: maxItems: 20 items: type: object - required: [type, id, name] + required: [type, id, name, archived] properties: type: type: string @@ -544,13 +544,17 @@ def delete(self, pk: int) -> Response: type: integer name: type: string + archived: + type: boolean inaccessible_count: type: integer archived_count: type: integer description: >- - Archived dependents are included in inaccessible_count; - these counts overlap and must not be added together. + Archived dependents are named only to users who could + restore them from Recently Archived; the others are + included in inaccessible_count. The counts overlap and + must not be added together. 422: $ref: '#/components/responses/422' """ @@ -639,7 +643,7 @@ def bulk_delete(self, **kwargs: Any) -> Response: maxItems: 20 items: type: object - required: [type, id, name] + required: [type, id, name, archived] properties: type: type: string @@ -648,13 +652,17 @@ def bulk_delete(self, **kwargs: Any) -> Response: type: integer name: type: string + archived: + type: boolean inaccessible_count: type: integer archived_count: type: integer description: >- - Archived dependents are included in inaccessible_count; - these counts overlap and must not be added together. + Archived dependents are named only to users who could + restore them from Recently Archived; the others are + included in inaccessible_count. The counts overlap and + must not be added together. 422: $ref: '#/components/responses/422' """ @@ -1173,7 +1181,7 @@ def delete(self, uuid: str) -> FlaskResponse: maxItems: 20 items: type: object - required: [type, id, name] + required: [type, id, name, archived] properties: type: type: string @@ -1182,13 +1190,17 @@ def delete(self, uuid: str) -> FlaskResponse: type: integer name: type: string + archived: + type: boolean inaccessible_count: type: integer archived_count: type: integer description: >- - Archived dependents are included in inaccessible_count; - these counts overlap and must not be added together. + Archived dependents are named only to users who could + restore them from Recently Archived; the others are + included in inaccessible_count. The counts overlap and + must not be added together. 422: $ref: '#/components/responses/422' """ diff --git a/superset/translations/messages.pot b/superset/translations/messages.pot index 4cdd2b3f1d23..d086d7150af2 100644 --- a/superset/translations/messages.pot +++ b/superset/translations/messages.pot @@ -5107,7 +5107,7 @@ msgstr "" msgid "Delete item" msgstr "" -msgid "Delete or move active datasets. To remove archived datasets, open Recently archived and select Delete permanently." +msgid "Delete or move active datasets. To remove archived datasets, open Recently Archived and select Delete permanently." msgstr "" msgid "Delete permanently" @@ -12393,8 +12393,8 @@ msgid "Semantic layer updated" msgstr "" #, python-format -msgid "Semantic source is used by %(count)s dependent asset and cannot be deleted." -msgid_plural "Semantic source is used by %(count)s dependent assets and cannot be deleted." +msgid "This semantic layer or view is used by %(count)s dependent asset and cannot be deleted." +msgid_plural "This semantic layer or view is used by %(count)s dependent assets and cannot be deleted." msgstr[0] "" msgstr[1] "" @@ -18251,10 +18251,6 @@ msgstr[0] "" msgstr[1] "" -#, python-format -msgid "Are you sure you want to delete %s? This cannot be undone." -msgstr "" - msgid "" "Deleting this semantic layer also permanently deletes any semantic views it " @@ -18262,7 +18258,7 @@ msgid "" msgstr "" -msgid "Remove or repoint these assets before deleting the semantic source." +msgid "Remove or repoint these assets before deleting it." msgstr "" @@ -18288,8 +18284,7 @@ msgid "" "them permanently, or ask an admin for help." msgstr "" -msgid "" -"This semantic layer has no dependent semantic views. This cannot be undone." +msgid "This semantic layer has no semantic views. This cannot be undone." msgstr "" #, python-format @@ -18297,3 +18292,21 @@ msgid "" "Deletion is blocked by %s dependent assets (%s archived). Remove or repoint " "them before deleting." msgstr "" + +msgid "This cannot be undone." +msgstr "" + +msgid "alert" +msgstr "" + +#, python-format +msgid "archived %s" +msgstr "" + +#, python-format +msgid "and %s more" +msgstr "" + +#, python-format +msgid "%s dependent asset(s) you can't see. Ask an admin for help." +msgstr "" diff --git a/tests/unit_tests/commands/semantic_layer/delete_test.py b/tests/unit_tests/commands/semantic_layer/delete_test.py index 8c094a62f98d..3469133553bd 100644 --- a/tests/unit_tests/commands/semantic_layer/delete_test.py +++ b/tests/unit_tests/commands/semantic_layer/delete_test.py @@ -264,7 +264,7 @@ def test_bulk_delete_semantic_views_refuses_all_on_one_dependency( @pytest.mark.parametrize("orm_execution", [False, True]) -def test_semantic_delete_counts_archived_dependents_without_listing_them( +def test_semantic_delete_counts_and_names_archived_dependents( session: Session, mocker: MockerFixture, orm_execution: bool ) -> None: """Charts, dashboard membership and active schedules form the dependency set.""" @@ -363,9 +363,13 @@ def test_semantic_delete_counts_archived_dependents_without_listing_them( assert total == 6 assert archived_count == 1 - assert inaccessible_count == 1 + assert inaccessible_count == 0 + assert {item["name"] for item in dependents if item["archived"]} == { + "Deleted chart" + } assert {item["name"] for item in dependents} == { "Live chart", + "Deleted chart", "Live dashboard", "Live alert", "Live report", @@ -387,7 +391,7 @@ def test_semantic_delete_counts_archived_dependents_without_listing_them( assert capped_total == 6 assert len(capped_dependents) == 2 assert [item["type"] for item in capped_dependents] == ["chart", "chart"] - assert capped_inaccessible_count == 1 + assert capped_inaccessible_count == 0 mocker.patch( "superset.commands.semantic_layer.delete.ChartFilter.apply", return_value=session.query(Slice.id).filter(sa.false()), @@ -547,13 +551,14 @@ def metadata(source_type: str | None, dataset_id: int | str) -> str: ) assert total == 6 assert archived_count == 1 - assert inaccessible_count == 1 + assert inaccessible_count == 0 assert {item["name"] for item in dependents} == { "Typed integer", "Typed string", "Filter and chart", "Member chart", "Native report", + "Deleted", } dao: MagicMock = mocker.patch( "superset.commands.semantic_layer.delete.SemanticViewDAO" @@ -654,7 +659,7 @@ def test_semantic_layer_delete_finds_dashboard_target( DeleteSemanticLayerCommand(str(layer_uuid)).run() assert exc_info.value.total == 1 assert exc_info.value.dependents == [ - {"type": "dashboard", "id": 741, "name": "Filter-only"} + {"type": "dashboard", "id": 741, "name": "Filter-only", "archived": False} ] dao.delete.assert_not_called() finally: @@ -1129,3 +1134,77 @@ def test_archived_dashboard_blocks_delete_with_orm_listener( assert exc_info.value.total == (2 if path == "membership" else 1) assert exc_info.value.archived_count == 1 assert exc_info.value.dependents == [] + + +@pytest.mark.parametrize( + ("viewer", "named"), + [ + ("admin", {"Live chart": False, "Archived chart": True}), + ("editor", {"Live chart": False, "Archived chart": True}), + ("reader", {"Live chart": False}), + ], +) +def test_semantic_delete_names_archived_dependents_to_restore_audience( + session: Session, mocker: MockerFixture, viewer: str, named: dict[str, bool] +) -> None: + """Archived blockers are named to whoever Recently Archived would show them.""" + from datetime import datetime + + from superset.commands.semantic_layer.delete import _dependent_assets + from superset.models.slice import Slice + from superset.subjects.models import chart_editors, Subject + + Slice.metadata.create_all(session.get_bind()) + session.execute(Subject.__table__.insert().values(id=1, label="editor", type=1)) + session.execute( + Slice.__table__.insert(), + [ + { + "id": 961, + "slice_name": "Live chart", + "datasource_type": "semantic_view", + "datasource_id": 42, + "deleted_at": None, + }, + { + "id": 962, + "slice_name": "Archived chart", + "datasource_type": "semantic_view", + "datasource_id": 42, + "deleted_at": datetime(2026, 1, 1), + }, + ], + ) + session.execute(chart_editors.insert().values(chart_id=962, subject_id=1)) + # Prove the session listener is active; do not substitute Connection.execute. + assert session.scalar(sa.select(Slice.id).where(Slice.id == 962)) is None + mocker.patch("superset.security_manager.can_access", return_value=True) + for access_filter in ( + "ChartFilter", + "DashboardAccessFilter", + "ReportScheduleFilter", + ): + mocker.patch( + f"superset.commands.semantic_layer.delete.{access_filter}.apply", + side_effect=lambda query, value: query, + ) + mocker.patch("superset.security_manager.is_admin", return_value=viewer == "admin") + mocker.patch("superset.views.filters.get_user_id", return_value=7) + mocker.patch( + "superset.subjects.utils.get_user_subject_ids_subquery", + return_value=sa.select(sa.literal(1 if viewer == "editor" else 2)), + ) + + total: int + dependents: list[dict[str, str | int | bool]] + inaccessible_count: int + archived_count: int + total, dependents, inaccessible_count, archived_count = _dependent_assets( + sa.select(sa.literal(42)) + ) + + assert (total, archived_count) == (2, 1) + assert {str(item["name"]): item["archived"] for item in dependents} == named + assert inaccessible_count == 2 - len(named) + # The bypass is per statement: later queries still hide archived charts. + assert session.scalar(sa.select(Slice.id).where(Slice.id == 962)) is None diff --git a/tests/unit_tests/semantic_layers/api_test.py b/tests/unit_tests/semantic_layers/api_test.py index 6115f2a607c8..ebe0987a8bf0 100644 --- a/tests/unit_tests/semantic_layers/api_test.py +++ b/tests/unit_tests/semantic_layers/api_test.py @@ -1052,8 +1052,8 @@ def test_semantic_delete_reports_dependents_as_conflict( assert response.json["dependents"] == [dependent] assert response.json["inaccessible_count"] == 24 assert response.json["message"] == ( - "Semantic source is used by 25 dependent assets and cannot be deleted. " - "Remove or repoint these assets before deleting the semantic source. " + "This semantic layer or view is used by 25 dependent assets and cannot " + "be deleted. Remove or repoint these assets before deleting it. " "3 dependent assets are archived. " "Go to Recently Archived to restore and repoint archived assets, " "or delete them permanently, or ask an admin for help." From 546d456b6d0e1ae66fdf1ecda1dffd64cb61328a Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Thu, 22 Oct 2026 12:02:42 -0600 Subject: [PATCH 05/12] docs(semantic): regenerate OpenAPI for named archived dependents Co-Authored-By: Claude Opus 5.5 --- docs/static/resources/openapi.json | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/docs/static/resources/openapi.json b/docs/static/resources/openapi.json index e4e1d91dd171..4c96f13c0bec 100644 --- a/docs/static/resources/openapi.json +++ b/docs/static/resources/openapi.json @@ -34264,12 +34264,15 @@ "schema": { "properties": { "archived_count": { - "description": "Archived dependents are included in inaccessible_count; these counts overlap and must not be added together.", + "description": "Archived dependents are named only to users who could restore them from Recently Archived; the others are included in inaccessible_count. The counts overlap and must not be added together.", "type": "integer" }, "dependents": { "items": { "properties": { + "archived": { + "type": "boolean" + }, "id": { "type": "integer" }, @@ -34289,7 +34292,8 @@ "required": [ "type", "id", - "name" + "name", + "archived" ], "type": "object" }, @@ -34654,12 +34658,15 @@ "schema": { "properties": { "archived_count": { - "description": "Archived dependents are included in inaccessible_count; these counts overlap and must not be added together.", + "description": "Archived dependents are named only to users who could restore them from Recently Archived; the others are included in inaccessible_count. The counts overlap and must not be added together.", "type": "integer" }, "dependents": { "items": { "properties": { + "archived": { + "type": "boolean" + }, "id": { "type": "integer" }, @@ -34679,7 +34686,8 @@ "required": [ "type", "id", - "name" + "name", + "archived" ], "type": "object" }, @@ -34821,12 +34829,15 @@ "schema": { "properties": { "archived_count": { - "description": "Archived dependents are included in inaccessible_count; these counts overlap and must not be added together.", + "description": "Archived dependents are named only to users who could restore them from Recently Archived; the others are included in inaccessible_count. The counts overlap and must not be added together.", "type": "integer" }, "dependents": { "items": { "properties": { + "archived": { + "type": "boolean" + }, "id": { "type": "integer" }, @@ -34846,7 +34857,8 @@ "required": [ "type", "id", - "name" + "name", + "archived" ], "type": "object" }, From 5bb9d45f7e24fd419b1c92d01faad6d29219b423 Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Thu, 22 Oct 2026 13:36:25 -0600 Subject: [PATCH 06/12] fix(semantic): name archived blockers without widening live visibility Bypassing the soft-delete listener for charts and dashboards across the whole dependents statement also changed the live access filters: the dashboard filter's chart join started counting archived charts. Keep the live filters as they were and add archived rows through a separate audience read from the editor tables, which is the Recently Archived audience (admins see every archived row, others the rows they edit). Co-Authored-By: Claude Opus 5.5 --- superset/commands/semantic_layer/delete.py | 82 +++++++++++-------- .../commands/semantic_layer/delete_test.py | 20 ++++- 2 files changed, 63 insertions(+), 39 deletions(-) diff --git a/superset/commands/semantic_layer/delete.py b/superset/commands/semantic_layer/delete.py index 0e34c30711ee..c17508bda3c5 100644 --- a/superset/commands/semantic_layer/delete.py +++ b/superset/commands/semantic_layer/delete.py @@ -26,7 +26,7 @@ from sqlalchemy.exc import SQLAlchemyError from superset import db, security_manager -from superset.charts.filters import ChartDeletedStateFilter, ChartFilter +from superset.charts.filters import ChartFilter from superset.commands.base import BaseCommand from superset.commands.semantic_layer.exceptions import ( SemanticDeleteDependentsError, @@ -38,28 +38,22 @@ SemanticViewNotFoundError, ) from superset.commands.utils import current_user_can_modify_object -from superset.constants import SKIP_VISIBILITY_FILTER_CLASSES from superset.daos.semantic_layer import SemanticLayerDAO, SemanticViewDAO -from superset.dashboards.filters import ( - DashboardAccessFilter, - DashboardDeletedStateFilter, -) +from superset.dashboards.filters import DashboardAccessFilter from superset.exceptions import SupersetSecurityException from superset.models.dashboard import Dashboard, dashboard_slices from superset.models.slice import Slice from superset.reports.filters import ReportScheduleFilter from superset.reports.models import ReportSchedule from superset.semantic_layers.models import SemanticLayer, SemanticView +from superset.subjects.models import chart_editors, dashboard_editors +from superset.subjects.utils import get_user_subject_ids_subquery from superset.utils import json +from superset.utils.core import get_user_id from superset.utils.decorators import on_error, transaction logger = logging.getLogger(__name__) _DEPENDENT_LIMIT: int = 20 -# Visibility subqueries see archived charts and dashboards; the restore-audience -# filter then decides which of them may be named. -_INCLUDE_ARCHIVED: dict[str, set[type]] = { - SKIP_VISIBILITY_FILTER_CLASSES: {Slice, Dashboard} -} def _semantic_target_matches(target: object, view_ids: set[int]) -> bool: @@ -126,6 +120,22 @@ def _dashboard_target_ids(view_ids: sa.Select) -> set[int]: return matched_ids +def _archived_audience_ids(table: sa.Table, editors: sa.Table, fk: str) -> sa.Select: + """Archived ids the user could restore from Recently Archived. + + Mirrors ``BaseDeletedStateFilter``: admins see every archived row, others + only rows they edit. Core tables keep the soft-delete listener out of it. + """ + if security_manager.is_admin(): + return sa.select(table.c.id) + user_id: int | None = get_user_id() + if not user_id: + return sa.select(table.c.id).where(sa.false()) + return sa.select(editors.c[fk]).where( + editors.c.subject_id.in_(get_user_subject_ids_subquery(user_id)) + ) + + def _dependent_assets( view_ids: sa.Select, ) -> tuple[int, list[dict[str, str | int]], int, int]: @@ -188,16 +198,8 @@ def _dependent_assets( visible_chart_ids: sa.Select = ( cast( sa.Select, - # Archived rows are named only to their Recently Archived audience. - ChartDeletedStateFilter( # pylint: disable=protected-access - "id", SQLAInterface(Slice, db.session) - ) - ._scope_to_restore_audience( - ChartFilter("id", SQLAInterface(Slice, db.session)).apply( - db.session.query(Slice.id), None - ), - "include", - ) + ChartFilter("id", SQLAInterface(Slice, db.session)) + .apply(db.session.query(Slice.id).filter(Slice.deleted_at.is_(None)), None) .statement, ) if security_manager.can_access("can_read", "Chart") @@ -206,14 +208,10 @@ def _dependent_assets( visible_dashboard_ids: sa.Select = ( cast( sa.Select, - DashboardDeletedStateFilter( # pylint: disable=protected-access - "id", SQLAInterface(Dashboard, db.session) - ) - ._scope_to_restore_audience( - DashboardAccessFilter("id", SQLAInterface(Dashboard, db.session)).apply( - db.session.query(Dashboard.id), None - ), - "include", + DashboardAccessFilter("id", SQLAInterface(Dashboard, db.session)) + .apply( + db.session.query(Dashboard.id).filter(Dashboard.deleted_at.is_(None)), + None, ) .statement, ) @@ -230,14 +228,30 @@ def _dependent_assets( if security_manager.can_access("can_read", "ReportSchedule") else sa.select(ReportSchedule.id).where(sa.false()) ) + archived_chart_ids: sa.Select = ( + _archived_audience_ids(Slice.__table__, chart_editors, "chart_id") + if security_manager.can_access("can_read", "Chart") + else sa.select(Slice.__table__.c.id).where(sa.false()) + ) + archived_dashboard_ids: sa.Select = ( + _archived_audience_ids(Dashboard.__table__, dashboard_editors, "dashboard_id") + if security_manager.can_access("can_read", "Dashboard") + else sa.select(Dashboard.__table__.c.id).where(sa.false()) + ) visibility: sa.ColumnElement[bool] = sa.or_( sa.and_( dependents.c.type == "chart", - dependents.c.id.in_(visible_chart_ids), + sa.or_( + dependents.c.id.in_(visible_chart_ids), + dependents.c.archived & dependents.c.id.in_(archived_chart_ids), + ), ), sa.and_( dependents.c.type == "dashboard", - dependents.c.id.in_(visible_dashboard_ids), + sa.or_( + dependents.c.id.in_(visible_dashboard_ids), + dependents.c.archived & dependents.c.id.in_(archived_dashboard_ids), + ), ), sa.and_( dependents.c.type.notin_(("chart", "dashboard")), @@ -252,8 +266,7 @@ def _dependent_assets( sa.func.count(), sa.func.sum(sa.case((visibility, 1), else_=0)), sa.func.sum(sa.case((dependents.c.archived, 1), else_=0)), - ).select_from(dependents), - execution_options=_INCLUDE_ARCHIVED, + ).select_from(dependents) ).one(), ) total: int = int(counts[0]) @@ -274,8 +287,7 @@ def _dependent_assets( visible.c.type, visible.c.id, ) - .limit(_DEPENDENT_LIMIT), - execution_options=_INCLUDE_ARCHIVED, + .limit(_DEPENDENT_LIMIT) ).all(), ) return ( diff --git a/tests/unit_tests/commands/semantic_layer/delete_test.py b/tests/unit_tests/commands/semantic_layer/delete_test.py index 3469133553bd..31d463ba77c3 100644 --- a/tests/unit_tests/commands/semantic_layer/delete_test.py +++ b/tests/unit_tests/commands/semantic_layer/delete_test.py @@ -396,6 +396,16 @@ def test_semantic_delete_counts_and_names_archived_dependents( "superset.commands.semantic_layer.delete.ChartFilter.apply", return_value=session.query(Slice.id).filter(sa.false()), ) + # A user who cannot see the charts is not an admin or their editor either. + mocker.patch("superset.security_manager.is_admin", return_value=False) + for access_filter in ("DashboardAccessFilter", "ReportScheduleFilter"): + mocker.patch( + f"superset.commands.semantic_layer.delete.{access_filter}.apply", + side_effect=lambda query, value: query, + ) + mocker.patch( + "superset.commands.semantic_layer.delete.get_user_id", return_value=None + ) hidden_total: int hidden_dependents: list[dict[str, str | int]] hidden_count: int @@ -576,6 +586,10 @@ def metadata(source_type: str | None, dataset_id: int | str) -> str: "superset.commands.semantic_layer.delete.DashboardAccessFilter.apply", return_value=session.query(Dashboard.id).filter(sa.false()), ) + mocker.patch("superset.security_manager.is_admin", return_value=False) + mocker.patch( + "superset.commands.semantic_layer.delete.get_user_id", return_value=None + ) hidden_total: int hidden_dependents: list[dict[str, str | int]] hidden_count: int @@ -1189,9 +1203,9 @@ def test_semantic_delete_names_archived_dependents_to_restore_audience( side_effect=lambda query, value: query, ) mocker.patch("superset.security_manager.is_admin", return_value=viewer == "admin") - mocker.patch("superset.views.filters.get_user_id", return_value=7) + mocker.patch("superset.commands.semantic_layer.delete.get_user_id", return_value=7) mocker.patch( - "superset.subjects.utils.get_user_subject_ids_subquery", + "superset.commands.semantic_layer.delete.get_user_subject_ids_subquery", return_value=sa.select(sa.literal(1 if viewer == "editor" else 2)), ) @@ -1206,5 +1220,3 @@ def test_semantic_delete_names_archived_dependents_to_restore_audience( assert (total, archived_count) == (2, 1) assert {str(item["name"]): item["archived"] for item in dependents} == named assert inaccessible_count == 2 - len(named) - # The bypass is per statement: later queries still hide archived charts. - assert session.scalar(sa.select(Slice.id).where(Slice.id == 962)) is None From 2e689b1afc45d4e3199ecebbc12284c189b38923 Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Thu, 22 Oct 2026 13:37:49 -0600 Subject: [PATCH 07/12] fix(semantic): pluralise the hidden-blocker hint Co-Authored-By: Claude Opus 5.5 --- .../features/semanticLayers/SemanticDeleteBlockers.tsx | 8 +++++--- .../src/pages/DatasetList/DatasetList.behavior.test.tsx | 2 +- superset/translations/messages.pot | 6 ++++-- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/superset-frontend/src/features/semanticLayers/SemanticDeleteBlockers.tsx b/superset-frontend/src/features/semanticLayers/SemanticDeleteBlockers.tsx index ccea4b03ef50..60b25b82adcc 100644 --- a/superset-frontend/src/features/semanticLayers/SemanticDeleteBlockers.tsx +++ b/superset-frontend/src/features/semanticLayers/SemanticDeleteBlockers.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { t } from '@apache-superset/core/translation'; +import { t, tn } from '@apache-superset/core/translation'; import { Alert } from '@apache-superset/core/components'; import { ensureAppRoot } from 'src/utils/navigationUtils'; @@ -101,8 +101,10 @@ export default function SemanticDeleteBlockers({ {unlisted > 0 &&

{t('and %s more', unlisted)}

} {hidden > 0 && (

- {t( - "%s dependent asset(s) you can't see. Ask an admin for help.", + {tn( + "%s dependent asset is one you can't see. Ask an admin for help.", + "%s dependent assets are ones you can't see. Ask an admin for help.", + hidden, hidden, )}

diff --git a/superset-frontend/src/pages/DatasetList/DatasetList.behavior.test.tsx b/superset-frontend/src/pages/DatasetList/DatasetList.behavior.test.tsx index 33d8f33e4532..51d7ed4f4953 100644 --- a/superset-frontend/src/pages/DatasetList/DatasetList.behavior.test.tsx +++ b/superset-frontend/src/pages/DatasetList/DatasetList.behavior.test.tsx @@ -164,7 +164,7 @@ test('single semantic view delete names its blockers, including archived ones', ).not.toBeInTheDocument(); expect(alert).toHaveTextContent('and 1 more'); expect(alert).toHaveTextContent( - "2 dependent asset(s) you can't see. Ask an admin for help.", + "2 dependent assets are ones you can't see. Ask an admin for help.", ); }); diff --git a/superset/translations/messages.pot b/superset/translations/messages.pot index d086d7150af2..c65f35c658fa 100644 --- a/superset/translations/messages.pot +++ b/superset/translations/messages.pot @@ -18308,5 +18308,7 @@ msgid "and %s more" msgstr "" #, python-format -msgid "%s dependent asset(s) you can't see. Ask an admin for help." -msgstr "" +msgid "%s dependent asset is one you can't see. Ask an admin for help." +msgid_plural "%s dependent assets are ones you can't see. Ask an admin for help." +msgstr[0] "" +msgstr[1] "" From d5cdb99175590e2e3de3763c71515c69ae3ba157 Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Thu, 22 Oct 2026 14:02:50 -0600 Subject: [PATCH 08/12] fix(semantic): never name archived blockers to guest tokens An Admin-role guest token skipped straight to the admin branch of the archived audience, naming more than Recently Archived would show it. Fail closed for guests first, as the restore gate does, and pin that editorship names archived rows only. Co-Authored-By: Claude Opus 5.5 --- superset/commands/semantic_layer/delete.py | 3 +++ .../commands/semantic_layer/delete_test.py | 22 +++++++++++++++++-- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/superset/commands/semantic_layer/delete.py b/superset/commands/semantic_layer/delete.py index c17508bda3c5..21211d354b84 100644 --- a/superset/commands/semantic_layer/delete.py +++ b/superset/commands/semantic_layer/delete.py @@ -126,6 +126,9 @@ def _archived_audience_ids(table: sa.Table, editors: sa.Table, fk: str) -> sa.Se Mirrors ``BaseDeletedStateFilter``: admins see every archived row, others only rows they edit. Core tables keep the soft-delete listener out of it. """ + # A guest token never restores archived assets, whatever its role. + if security_manager.is_guest_user(): + return sa.select(table.c.id).where(sa.false()) if security_manager.is_admin(): return sa.select(table.c.id) user_id: int | None = get_user_id() diff --git a/tests/unit_tests/commands/semantic_layer/delete_test.py b/tests/unit_tests/commands/semantic_layer/delete_test.py index 31d463ba77c3..95c1e73a4aa6 100644 --- a/tests/unit_tests/commands/semantic_layer/delete_test.py +++ b/tests/unit_tests/commands/semantic_layer/delete_test.py @@ -1156,6 +1156,8 @@ def test_archived_dashboard_blocks_delete_with_orm_listener( ("admin", {"Live chart": False, "Archived chart": True}), ("editor", {"Live chart": False, "Archived chart": True}), ("reader", {"Live chart": False}), + ("guest", {"Live chart": False}), + ("hidden-editor", {"Archived chart": True}), ], ) def test_semantic_delete_names_archived_dependents_to_restore_audience( @@ -1202,11 +1204,27 @@ def test_semantic_delete_names_archived_dependents_to_restore_audience( f"superset.commands.semantic_layer.delete.{access_filter}.apply", side_effect=lambda query, value: query, ) - mocker.patch("superset.security_manager.is_admin", return_value=viewer == "admin") + if viewer == "hidden-editor": + # Editorship names archived rows only: a live row the base filter hides + # stays hidden even from its editor. + session.execute(chart_editors.insert().values(chart_id=961, subject_id=1)) + mocker.patch( + "superset.commands.semantic_layer.delete.ChartFilter.apply", + side_effect=lambda query, value: query.filter(sa.false()), + ) + mocker.patch( + "superset.security_manager.is_admin", + return_value=viewer in ("admin", "guest"), + ) + mocker.patch( + "superset.security_manager.is_guest_user", return_value=viewer == "guest" + ) mocker.patch("superset.commands.semantic_layer.delete.get_user_id", return_value=7) mocker.patch( "superset.commands.semantic_layer.delete.get_user_subject_ids_subquery", - return_value=sa.select(sa.literal(1 if viewer == "editor" else 2)), + return_value=sa.select( + sa.literal(1 if viewer in ("editor", "hidden-editor") else 2) + ), ) total: int From 8a97757a4956d739b6f781e53bcb5fd343801c1f Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Thu, 22 Oct 2026 15:04:12 -0600 Subject: [PATCH 09/12] fix(semantic): link blocking dashboards through the app-root route Use /dashboard// rather than a hard-coded /superset/ path, as the navigation invariant requires; ensureAppRoot applies the deployment root. Co-Authored-By: Claude Opus 5.5 --- .../src/features/semanticLayers/SemanticDeleteBlockers.tsx | 2 +- .../src/pages/DatasetList/DatasetList.behavior.test.tsx | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/superset-frontend/src/features/semanticLayers/SemanticDeleteBlockers.tsx b/superset-frontend/src/features/semanticLayers/SemanticDeleteBlockers.tsx index 60b25b82adcc..13fbab920678 100644 --- a/superset-frontend/src/features/semanticLayers/SemanticDeleteBlockers.tsx +++ b/superset-frontend/src/features/semanticLayers/SemanticDeleteBlockers.tsx @@ -47,7 +47,7 @@ const typeLabel = (type: string): string => { const LIVE_LINKS: Record string> = { chart: id => `/explore/?slice_id=${id}`, - dashboard: id => `/superset/dashboard/${id}/`, + dashboard: id => `/dashboard/${id}/`, }; /** The refusal shown inside a semantic layer or view delete dialog. */ diff --git a/superset-frontend/src/pages/DatasetList/DatasetList.behavior.test.tsx b/superset-frontend/src/pages/DatasetList/DatasetList.behavior.test.tsx index 51d7ed4f4953..dbce49bee833 100644 --- a/superset-frontend/src/pages/DatasetList/DatasetList.behavior.test.tsx +++ b/superset-frontend/src/pages/DatasetList/DatasetList.behavior.test.tsx @@ -133,12 +133,13 @@ test('single semantic view delete names its blockers, including archived ones', status: 409, body: { message: 'blocked', - total: 5, + total: 6, archived_count: 1, inaccessible_count: 2, dependents: [ { type: 'chart', id: 7, name: 'Revenue chart', archived: false }, { type: 'dashboard', id: 8, name: 'Old board', archived: true }, + { type: 'dashboard', id: 9, name: 'Live board', archived: false }, ], }, }); @@ -159,6 +160,9 @@ test('single semantic view delete names its blockers, including archived ones', within(alert).getByRole('link', { name: 'Revenue chart' }), ).toHaveAttribute('href', '/explore/?slice_id=7'); expect(alert).toHaveTextContent('Old board (archived dashboard)'); + expect( + within(alert).getByRole('link', { name: 'Live board' }), + ).toHaveAttribute('href', '/dashboard/9/'); expect( within(alert).queryByRole('link', { name: 'Old board' }), ).not.toBeInTheDocument(); From a3d85f6906bfb19b56560fc742fa3bff1ace70dd Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Thu, 22 Oct 2026 16:24:20 -0600 Subject: [PATCH 10/12] fix(semantic-layer): link delete blockers only by numeric id The blocker ids come from the error response, so a non-numeric id could steer the link off the app. Build links only from whole-number ids and keep names as plain text. Co-Authored-By: Claude Opus 5.5 --- .../SemanticDeleteBlockers.test.tsx | 51 +++++++++++++++++++ .../semanticLayers/SemanticDeleteBlockers.tsx | 38 +++++++------- 2 files changed, 71 insertions(+), 18 deletions(-) create mode 100644 superset-frontend/src/features/semanticLayers/SemanticDeleteBlockers.test.tsx diff --git a/superset-frontend/src/features/semanticLayers/SemanticDeleteBlockers.test.tsx b/superset-frontend/src/features/semanticLayers/SemanticDeleteBlockers.test.tsx new file mode 100644 index 000000000000..c16b4e8d89b1 --- /dev/null +++ b/superset-frontend/src/features/semanticLayers/SemanticDeleteBlockers.test.tsx @@ -0,0 +1,51 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { render, screen } from 'spec/helpers/testing-library'; +import SemanticDeleteBlockers, { + SemanticDeleteError, +} from './SemanticDeleteBlockers'; + +test('markup in the refusal is shown as text, not rendered as HTML', () => { + const error = { + error: 'Blocked', + dependents: [ + { type: 'chart', id: 7, name: 'Revenue', archived: false }, + { + type: 'dashboard', + id: '/evil.example' as unknown as number, + name: 'Odd board', + archived: false, + }, + ], + } satisfies SemanticDeleteError; + const { container } = render(); + expect( + screen.getByText('Blocked'), + ).toBeInTheDocument(); + expect(screen.getByRole('link', { name: 'Revenue' })).toHaveAttribute( + 'href', + '/explore/?slice_id=7', + ); + expect(container.querySelector('img, b')).toBeNull(); + // An id that is not a whole number never becomes a link. + expect(screen.getByText(/Odd board/)).toBeInTheDocument(); + expect( + screen.queryByRole('link', { name: 'Odd board' }), + ).not.toBeInTheDocument(); +}); diff --git a/superset-frontend/src/features/semanticLayers/SemanticDeleteBlockers.tsx b/superset-frontend/src/features/semanticLayers/SemanticDeleteBlockers.tsx index 13fbab920678..d51960e6b5ea 100644 --- a/superset-frontend/src/features/semanticLayers/SemanticDeleteBlockers.tsx +++ b/superset-frontend/src/features/semanticLayers/SemanticDeleteBlockers.tsx @@ -69,8 +69,13 @@ export default function SemanticDeleteBlockers({ ) : error.error; // Archived assets have no page to open; Recently Archived lists them. - const href = (item: Dependent): string | undefined => - item.archived ? undefined : LIVE_LINKS[item.type]?.(item.id); + // The id comes from the error body, so only a whole number becomes a link. + const href = (item: Dependent): string | undefined => { + const id = Number(item.id); + return item.archived || !Number.isSafeInteger(id) + ? undefined + : LIVE_LINKS[item.type]?.(id); + }; return ( {dependents.length > 0 && (
    - {dependents.map(item => ( -
  • - {href(item) ? ( - - {item.name} - - ) : ( - item.name - )}{' '} - ( - {item.archived - ? t('archived %s', typeLabel(item.type)) - : typeLabel(item.type)} - ) -
    • - ))} + {dependents.map(item => { + const link = href(item); + const name = String(item.name); + return ( +
  • + {link ? {name} : name} ( + {item.archived + ? t('archived %s', typeLabel(item.type)) + : typeLabel(item.type)} + ) +
    • + ); + })}
)} {unlisted > 0 &&

{t('and %s more', unlisted)}

} From bbaa8464830d4f8eb98f8206f379ef8a8b0b79c1 Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Thu, 22 Oct 2026 23:13:47 -0600 Subject: [PATCH 11/12] chore(i18n): align semantic deletion template after stack merge --- superset/translations/messages.pot | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/superset/translations/messages.pot b/superset/translations/messages.pot index 3649793f16e8..36da400218bf 100644 --- a/superset/translations/messages.pot +++ b/superset/translations/messages.pot @@ -2266,10 +2266,6 @@ msgstr "" msgid "Are you sure you want to delete %s?" msgstr "" -#, python-format -msgid "Are you sure you want to delete %s? This cannot be undone." -msgstr "" - #, python-format msgid "Are you sure you want to delete the selected %s?" msgstr "" @@ -11017,9 +11013,6 @@ msgstr "" msgid "Remove or repoint these assets before deleting it." msgstr "" -msgid "Remove or repoint these assets before deleting the semantic source." -msgstr "" - msgid "Remove query from log" msgstr "" @@ -15173,9 +15166,6 @@ msgstr "" msgid "This section contains validation errors" msgstr "" -msgid "This semantic layer has no dependent semantic views. This cannot be undone." -msgstr "" - msgid "This semantic layer has no semantic views. This cannot be undone." msgstr "" From 64f1e3e9f546041a550044a7159063d5edb7f7cd Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Mon, 26 Oct 2026 16:40:35 -0600 Subject: [PATCH 12/12] fix(semantic-layer): require restore capability for archived blocker names --- docs/docs/using-superset/recently-archived.mdx | 2 +- superset/commands/semantic_layer/delete.py | 4 ++-- .../commands/semantic_layer/delete_test.py | 13 ++++++++++--- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/docs/docs/using-superset/recently-archived.mdx b/docs/docs/using-superset/recently-archived.mdx index 755ec34c34d2..f2be235f9bab 100644 --- a/docs/docs/using-superset/recently-archived.mdx +++ b/docs/docs/using-superset/recently-archived.mdx @@ -95,7 +95,7 @@ When deleting a semantic layer or view, a dependency refusal stays in the confirmation dialog with the total and archived counts, the names of up to 20 blocking assets you can see, and a link to Recently Archived. Archived blockers are named only to users who could restore them from Recently Archived (admins -and the asset's editors); the dialog counts the others and suggests asking an +and the asset's editors with write permission on its resource type); the dialog counts the others and suggests asking an admin. The typed confirmation is preserved so you can retry after removing or repointing the dependents. Dependencies are checked when you confirm deletion, not before you type the confirmation. diff --git a/superset/commands/semantic_layer/delete.py b/superset/commands/semantic_layer/delete.py index 21211d354b84..74a866ff3bf3 100644 --- a/superset/commands/semantic_layer/delete.py +++ b/superset/commands/semantic_layer/delete.py @@ -233,12 +233,12 @@ def _dependent_assets( ) archived_chart_ids: sa.Select = ( _archived_audience_ids(Slice.__table__, chart_editors, "chart_id") - if security_manager.can_access("can_read", "Chart") + if security_manager.can_access("can_write", "Chart") else sa.select(Slice.__table__.c.id).where(sa.false()) ) archived_dashboard_ids: sa.Select = ( _archived_audience_ids(Dashboard.__table__, dashboard_editors, "dashboard_id") - if security_manager.can_access("can_read", "Dashboard") + if security_manager.can_access("can_write", "Dashboard") else sa.select(Dashboard.__table__.c.id).where(sa.false()) ) visibility: sa.ColumnElement[bool] = sa.or_( diff --git a/tests/unit_tests/commands/semantic_layer/delete_test.py b/tests/unit_tests/commands/semantic_layer/delete_test.py index 95c1e73a4aa6..f9071ca602e4 100644 --- a/tests/unit_tests/commands/semantic_layer/delete_test.py +++ b/tests/unit_tests/commands/semantic_layer/delete_test.py @@ -1156,6 +1156,7 @@ def test_archived_dashboard_blocks_delete_with_orm_listener( ("admin", {"Live chart": False, "Archived chart": True}), ("editor", {"Live chart": False, "Archived chart": True}), ("reader", {"Live chart": False}), + ("read-only-editor", {"Live chart": False}), ("guest", {"Live chart": False}), ("hidden-editor", {"Archived chart": True}), ], @@ -1163,7 +1164,7 @@ def test_archived_dashboard_blocks_delete_with_orm_listener( def test_semantic_delete_names_archived_dependents_to_restore_audience( session: Session, mocker: MockerFixture, viewer: str, named: dict[str, bool] ) -> None: - """Archived blockers are named to whoever Recently Archived would show them.""" + """Archived blockers are named only to callers with restore capability.""" from datetime import datetime from superset.commands.semantic_layer.delete import _dependent_assets @@ -1194,7 +1195,11 @@ def test_semantic_delete_names_archived_dependents_to_restore_audience( session.execute(chart_editors.insert().values(chart_id=962, subject_id=1)) # Prove the session listener is active; do not substitute Connection.execute. assert session.scalar(sa.select(Slice.id).where(Slice.id == 962)) is None - mocker.patch("superset.security_manager.can_access", return_value=True) + mocker.patch( + "superset.security_manager.can_access", + side_effect=lambda permission, resource: viewer != "read-only-editor" + or permission != "can_write", + ) for access_filter in ( "ChartFilter", "DashboardAccessFilter", @@ -1223,7 +1228,9 @@ def test_semantic_delete_names_archived_dependents_to_restore_audience( mocker.patch( "superset.commands.semantic_layer.delete.get_user_subject_ids_subquery", return_value=sa.select( - sa.literal(1 if viewer in ("editor", "hidden-editor") else 2) + sa.literal( + 1 if viewer in ("editor", "hidden-editor", "read-only-editor") else 2 + ) ), )