aminghadersohi commented on code in PR #45071:
URL: https://github.com/apache/superset/pull/45071#discussion_r4225251979
##########
superset/commands/semantic_layer/delete.py:
##########
@@ -32,11 +39,214 @@
)
from superset.commands.utils import current_user_can_modify_object
from superset.daos.semantic_layer import SemanticLayerDAO, SemanticViewDAO
+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.utils import json
from superset.utils.decorators import on_error, transaction
logger = logging.getLogger(__name__)
+_DEPENDENT_LIMIT: int = 20
+
+
+def _semantic_target_matches(target: object, view_ids: set[int]) -> bool:
+ """A missing type denotes a table, even when its integer ID collides."""
+ if not isinstance(target, dict) or target.get("datasourceType") !=
"semantic_view":
+ return False
+ raw_id: object = target.get("datasetId")
+ if isinstance(raw_id, bool) or not (
+ isinstance(raw_id, int) or (isinstance(raw_id, str) and
raw_id.isdecimal())
+ ):
+ return False
+ try:
+ return int(raw_id) in view_ids
+ except ValueError:
+ return False
+
+
+def _dashboard_targets_view(
+ metadata: str, view_ids: set[int], dashboard_id: int
+) -> bool:
+ """Match typed semantic targets in native filters or display controls."""
+ try:
+ decoded: object = json.loads(metadata)
+ except (TypeError, ValueError, RecursionError):
+ logger.debug("Skipping malformed targets on dashboard %s",
dashboard_id)
+ return False
+ if not isinstance(decoded, dict):
+ logger.debug("Skipping non-object targets on dashboard %s",
dashboard_id)
+ return False
+ for key in ("native_filter_configuration", "chart_customization_config"):
+ controls: object = decoded.get(key)
+ if not isinstance(controls, list):
+ continue
+ for control in controls:
+ if not isinstance(control, dict):
+ continue
+ targets: object = control.get("targets")
+ if isinstance(targets, list) and any(
+ _semantic_target_matches(target, view_ids) for target in
targets
+ ):
+ return True
+ return False
+
+
+def _dashboard_target_ids(view_ids: sa.Select) -> set[int]:
+ """Find live 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%"),
+ )
+ matched_ids: set[int] = set()
+ for dashboard_id, metadata in db.session.execute(
+ candidates.execution_options(yield_per=1000)
+ ):
+ if _dashboard_targets_view(metadata, selected_ids, dashboard_id):
+ matched_ids.add(dashboard_id)
+ return matched_ids
+
+
+def _dependent_assets(
+ view_ids: sa.Select,
+) -> tuple[int, list[dict[str, str | 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_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)
+ .where(
+ Slice.id.in_(chart_ids),
+ Dashboard.deleted_at.is_(None),
+ )
+ .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),
+ sa.or_(
+ Dashboard.id.in_(chart_dashboard_ids),
+ Dashboard.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(
+ "name"
+ ),
+ ).where(Dashboard.id.in_(dashboard_ids)),
+ sa.select(
+ sa.func.lower(ReportSchedule.type).label("type"),
+ ReportSchedule.id.label("id"),
+ ReportSchedule.name.label("name"),
+ ).where(
+ ReportSchedule.active.is_(True),
+ sa.or_(
+ ReportSchedule.chart_id.in_(chart_ids),
+ ReportSchedule.dashboard_id.in_(dashboard_ids),
+ ),
+ ),
+ ).subquery()
+ visible_chart_ids: sa.Select = (
+ cast(
+ sa.Select,
+ ChartFilter("id", SQLAInterface(Slice, db.session))
+ .apply(db.session.query(Slice.id), None)
+ .statement,
+ )
+ if security_manager.can_access("can_read", "Chart")
+ else sa.select(Slice.id).where(sa.false())
+ )
+ visible_dashboard_ids: sa.Select = (
+ cast(
+ sa.Select,
+ DashboardAccessFilter("id", SQLAInterface(Dashboard, db.session))
+ .apply(db.session.query(Dashboard.id), None)
+ .statement,
+ )
+ if security_manager.can_access("can_read", "Dashboard")
+ else sa.select(Dashboard.id).where(sa.false())
Review Comment:
Still open at `7abd4ca6dd`: setting the Dashboard (line 187) or
ReportSchedule (line 197) else-arm to `sa.true()` still leaves all 158 tests
green.
##########
superset-frontend/src/pages/DatasetList/DatasetList.behavior.test.tsx:
##########
@@ -73,6 +73,106 @@ afterEach(async () => {
jest.restoreAllMocks();
});
+test('single semantic view delete displays the counted dependency conflict',
async () => {
+ const view = {
+ ...mockDatasets[0],
+ id: 99,
+ table_name: 'orders_semantic',
+ kind: 'semantic_view',
+ };
+ const reason =
+ 'Semantic source is used by 25 dependent assets and cannot be deleted.';
+ const addDangerToast = jest.fn();
+ mockDatasetListEndpoints({ result: [view], count: 1 });
+ fetchMock.delete('glob:*/api/v1/semantic_view/99', {
+ status: 409,
+ body: { message: reason },
+ });
+ renderDatasetList(mockAdminUser, {
+ addDangerToast,
+ 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' }));
+ await waitFor(() =>
+ expect(addDangerToast).toHaveBeenCalledWith(
+ `There was an issue deleting orders_semantic: ${reason}`,
+ ),
+ );
+});
+
+test('mixed bulk delete names refused semantic views and their conflict
reason', async () => {
+ const dataset = mockDatasets[0];
+ const semanticView = {
+ ...mockDatasets[1],
+ id: 99,
+ table_name: 'orders_semantic',
+ kind: 'semantic_view',
+ };
+ const addDangerToast = jest.fn();
+ const reason =
+ 'Semantic source is used by 25 dependent assets and cannot be deleted.';
+ mockDatasetListEndpoints({ result: [dataset, semanticView], count: 2 });
+ fetchMock.get(API_ENDPOINTS.DATASET_BULK_RELATED_OBJECTS, {
+ charts: { count: 0, result: [] },
+ dashboards: { count: 0, result: [] },
+ });
+ fetchMock.delete(API_ENDPOINTS.DATASET_BULK_DELETE, { message: 'OK' });
+ fetchMock.delete('glob:*/api/v1/semantic_view/?q=*', {
+ status: 409,
+ body: {
+ message: reason,
+ total: 25,
+ dependents: [],
+ inaccessible_count: 25,
+ },
+ });
+ renderDatasetList(mockAdminUser, {
+ addDangerToast,
+ addSuccessToast: jest.fn(),
+ });
+ await screen.findByText(semanticView.table_name);
+ await userEvent.click(screen.getByRole('button', { name: /bulk select/i }));
+ const table = screen.getByTestId('listview-table');
+ for (const name of [dataset.table_name, semanticView.table_name]) {
+ const cell = await within(table).findByText(name);
+ await userEvent.click(within(cell.closest('tr')!).getByRole('checkbox'));
+ }
+ await waitFor(() =>
+ expect(screen.getByTestId('bulk-select-copy')).toHaveTextContent(
+ '2 Selected',
+ ),
+ );
+ await userEvent.click(
+ within(screen.getByTestId('bulk-select-controls')).getByRole('button', {
+ name: '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' }));
+ await waitFor(() =>
+ expect(addDangerToast).toHaveBeenCalledWith(
+ `Could not delete semantic views orders_semantic: ${reason}`,
+ ),
+ );
Review Comment:
When datasets succeed and the views are refused, nothing checks that the
generic dataset toast stays suppressed: dropping `&& results[0].status ===
'rejected'` keeps this test green.
```suggestion
);
expect(addDangerToast).toHaveBeenCalledTimes(1);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
