Skip to content

Commit

Permalink
feat: database delete warning (apache#10800)
Browse files Browse the repository at this point in the history
  • Loading branch information
nytai authored and auxten committed Nov 20, 2020
1 parent cd5fa76 commit 642aac3
Show file tree
Hide file tree
Showing 6 changed files with 234 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { styledMount as mount } from 'spec/helpers/theming';

import DatabaseList from 'src/views/CRUD/data/database/DatabaseList';
import DatabaseModal from 'src/views/CRUD/data/database/DatabaseModal';
import DeleteModal from 'src/components/DeleteModal';
import SubMenu from 'src/components/Menu/SubMenu';
import ListView from 'src/components/ListView';
import Filters from 'src/components/ListView/Filters';
Expand All @@ -37,6 +38,7 @@ const store = mockStore({});
const databasesInfoEndpoint = 'glob:*/api/v1/database/_info*';
const databasesEndpoint = 'glob:*/api/v1/database/?*';
const databaseEndpoint = 'glob:*/api/v1/database/*';
const databaseRelatedEndpoint = 'glob:*/api/v1/database/*/related_objects*';

const mockdatabases = [...new Array(3)].map((_, i) => ({
changed_by: {
Expand All @@ -63,6 +65,16 @@ fetchMock.get(databasesEndpoint, {
});

fetchMock.delete(databaseEndpoint, {});
fetchMock.get(databaseRelatedEndpoint, {
charts: {
count: 0,
result: [],
},
dashboards: {
count: 0,
result: [],
},
});

describe('DatabaseList', () => {
const wrapper = mount(<DatabaseList />, { context: { store } });
Expand Down Expand Up @@ -101,6 +113,10 @@ describe('DatabaseList', () => {
});
await waitForComponentToPaint(wrapper);

expect(wrapper.find(DeleteModal).props().description).toMatchInlineSnapshot(
`"The database db 0 is linked to 0 charts that appear on 0 dashboards. Are you sure you want to continue? Deleting the database will break those objects."`,
);

act(() => {
wrapper
.find('#delete')
Expand All @@ -115,6 +131,9 @@ describe('DatabaseList', () => {

await waitForComponentToPaint(wrapper);

expect(fetchMock.calls(/database\/0\/related_objects/, 'GET')).toHaveLength(
1,
);
expect(fetchMock.calls(/database\/0/, 'DELETE')).toHaveLength(1);
});

Expand Down
91 changes: 62 additions & 29 deletions superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ import React, { useState, useMemo } from 'react';
import { useListViewResource } from 'src/views/CRUD/hooks';
import { createErrorHandler } from 'src/views/CRUD/utils';
import withToasts from 'src/messageToasts/enhancers/withToasts';
import ConfirmStatusChange from 'src/components/ConfirmStatusChange';
import SubMenu, { SubMenuProps } from 'src/components/Menu/SubMenu';
import DeleteModal from 'src/components/DeleteModal';
import TooltipWrapper from 'src/components/TooltipWrapper';
import Icon from 'src/components/Icon';
import ListView, { Filters } from 'src/components/ListView';
Expand All @@ -34,6 +34,10 @@ import { DatabaseObject } from './types';

const PAGE_SIZE = 25;

interface DatabaseDeleteObject extends DatabaseObject {
chart_count: number;
dashboard_count: number;
}
interface DatabaseListProps {
addDangerToast: (msg: string) => void;
addSuccessToast: (msg: string) => void;
Expand Down Expand Up @@ -63,10 +67,34 @@ function DatabaseList({ addDangerToast, addSuccessToast }: DatabaseListProps) {
addDangerToast,
);
const [databaseModalOpen, setDatabaseModalOpen] = useState<boolean>(false);
const [
databaseCurrentlyDeleting,
setDatabaseCurrentlyDeleting,
] = useState<DatabaseDeleteObject | null>(null);
const [currentDatabase, setCurrentDatabase] = useState<DatabaseObject | null>(
null,
);

const openDatabaseDeleteModal = (database: DatabaseObject) =>
SupersetClient.get({
endpoint: `/api/v1/database/${database.id}/related_objects/`,
})
.then(({ json = {} }) => {
setDatabaseCurrentlyDeleting({
...database,
chart_count: json.charts.count,
dashboard_count: json.dashboards.count,
});
})
.catch(
createErrorHandler(errMsg =>
t(
'An error occurred while fetching database related data: %s',
errMsg,
),
),
);

function handleDatabaseDelete({ id, database_name: dbName }: DatabaseObject) {
SupersetClient.delete({
endpoint: `/api/v1/database/${id}`,
Expand Down Expand Up @@ -198,41 +226,28 @@ function DatabaseList({ addDangerToast, addSuccessToast }: DatabaseListProps) {
},
{
Cell: ({ row: { original } }: any) => {
const handleDelete = () => handleDatabaseDelete(original);
const handleDelete = () => openDatabaseDeleteModal(original);
if (!canDelete) {
return null;
}
return (
<span className="actions">
{canDelete && (
<ConfirmStatusChange
title={t('Please Confirm')}
description={
<>
{t('Are you sure you want to delete')}{' '}
<b>{original.database_name}</b>?
</>
}
onConfirm={handleDelete}
<span
role="button"
tabIndex={0}
className="action-button"
data-test="database-delete"
onClick={handleDelete}
>
{confirmDelete => (
<span
role="button"
tabIndex={0}
className="action-button"
data-test="database-delete"
onClick={confirmDelete}
>
<TooltipWrapper
label="delete-action"
tooltip={t('Delete database')}
placement="bottom"
>
<Icon name="trash" />
</TooltipWrapper>
</span>
)}
</ConfirmStatusChange>
<TooltipWrapper
label="delete-action"
tooltip={t('Delete database')}
placement="bottom"
>
<Icon name="trash" />
</TooltipWrapper>
</span>
)}
</span>
);
Expand Down Expand Up @@ -298,6 +313,24 @@ function DatabaseList({ addDangerToast, addSuccessToast }: DatabaseListProps) {
/* TODO: add database logic here */
}}
/>
{databaseCurrentlyDeleting && (
<DeleteModal
description={t(
'The database %s is linked to %s charts that appear on %s dashboards. Are you sure you want to continue? Deleting the database will break those objects.',
databaseCurrentlyDeleting.database_name,
databaseCurrentlyDeleting.chart_count,
databaseCurrentlyDeleting.dashboard_count,
)}
onConfirm={() => {
if (databaseCurrentlyDeleting) {
handleDatabaseDelete(databaseCurrentlyDeleting);
}
}}
onHide={() => setDatabaseCurrentlyDeleting(null)}
open
title={t('Delete Database?')}
/>
)}

<ListView<DatabaseObject>
className="database-list-view"
Expand Down
61 changes: 61 additions & 0 deletions superset/databases/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,14 @@
DatabaseUpdateFailedError,
)
from superset.databases.commands.update import UpdateDatabaseCommand
from superset.databases.dao import DatabaseDAO
from superset.databases.decorators import check_datasource_access
from superset.databases.filters import DatabaseFilter
from superset.databases.schemas import (
database_schemas_query_schema,
DatabasePostSchema,
DatabasePutSchema,
DatabaseRelatedObjectsResponse,
SchemasResponseSchema,
SelectStarResponseSchema,
TableMetadataResponseSchema,
Expand All @@ -63,6 +65,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
"table_metadata",
"select_star",
"schemas",
"related_objects",
}
class_permission_name = "DatabaseView"
resource_name = "database"
Expand Down Expand Up @@ -148,6 +151,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
}
openapi_spec_tag = "Database"
openapi_spec_component_schemas = (
DatabaseRelatedObjectsResponse,
TableMetadataResponseSchema,
SelectStarResponseSchema,
SchemasResponseSchema,
Expand Down Expand Up @@ -501,3 +505,60 @@ def select_star(
return self.response(404, message="Table not found on the database")
self.incr_stats("success", self.select_star.__name__)
return self.response(200, result=result)

@expose("/<int:pk>/related_objects/", methods=["GET"])
@protect()
@safe
@statsd_metrics
def related_objects(self, pk: int) -> Response:
"""Get charts and dashboards count associated to a database
---
get:
description:
Get charts and dashboards count associated to a database
parameters:
- in: path
name: pk
schema:
type: integer
responses:
200:
200:
description: Query result
content:
application/json:
schema:
$ref: "#/components/schemas/DatabaseRelatedObjectsResponse"
401:
$ref: '#/components/responses/401'
404:
$ref: '#/components/responses/404'
500:
$ref: '#/components/responses/500'
"""
dataset = DatabaseDAO.find_by_id(pk)
if not dataset:
return self.response_404()
data = DatabaseDAO.get_related_objects(pk)
charts = [
{
"id": chart.id,
"slice_name": chart.slice_name,
"viz_type": chart.viz_type,
}
for chart in data["charts"]
]
dashboards = [
{
"id": dashboard.id,
"json_metadata": dashboard.json_metadata,
"slug": dashboard.slug,
"title": dashboard.dashboard_title,
}
for dashboard in data["dashboards"]
]
return self.response(
200,
charts={"count": len(charts), "result": charts},
dashboards={"count": len(dashboards), "result": dashboards},
)
28 changes: 28 additions & 0 deletions superset/databases/dao.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,14 @@
# specific language governing permissions and limitations
# under the License.
import logging
from typing import Any, Dict

from superset.dao.base import BaseDAO
from superset.databases.filters import DatabaseFilter
from superset.extensions import db
from superset.models.core import Database
from superset.models.dashboard import Dashboard
from superset.models.slice import Slice

logger = logging.getLogger(__name__)

Expand All @@ -41,3 +44,28 @@ def validate_update_uniqueness(database_id: int, database_name: str) -> bool:
Database.database_name == database_name, Database.id != database_id,
)
return not db.session.query(database_query.exists()).scalar()

@classmethod
def get_related_objects(cls, database_id: int) -> Dict[str, Any]:
datasets = cls.find_by_id(database_id).tables
dataset_ids = [dataset.id for dataset in datasets]

charts = (
db.session.query(Slice)
.filter(
Slice.datasource_id.in_(dataset_ids), Slice.datasource_type == "table"
)
.all()
)
chart_ids = [chart.id for chart in charts]

dashboards = (
(
db.session.query(Dashboard)
.join(Dashboard.slices)
.filter(Slice.id.in_(chart_ids))
)
.distinct()
.all()
)
return dict(charts=charts, dashboards=dashboards)
32 changes: 32 additions & 0 deletions superset/databases/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -339,3 +339,35 @@ class SelectStarResponseSchema(Schema):

class SchemasResponseSchema(Schema):
result = fields.List(fields.String(description="A database schema name"))


class DatabaseRelatedChart(Schema):
id = fields.Integer()
slice_name = fields.String()
viz_type = fields.String()


class DatabaseRelatedDashboard(Schema):
id = fields.Integer()
json_metadata = fields.Dict()
slug = fields.String()
title = fields.String()


class DatabaseRelatedCharts(Schema):
count = fields.Integer(description="Chart count")
result = fields.List(
fields.Nested(DatabaseRelatedChart), description="A list of dashboards"
)


class DatabaseRelatedDashboards(Schema):
count = fields.Integer(description="Dashboard count")
result = fields.List(
fields.Nested(DatabaseRelatedDashboard), description="A list of dashboards"
)


class DatabaseRelatedObjectsResponse(Schema):
charts = fields.Nested(DatabaseRelatedCharts)
dashboards = fields.Nested(DatabaseRelatedDashboards)
32 changes: 32 additions & 0 deletions tests/databases/api_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -651,3 +651,35 @@ def test_database_schemas_invalid_query(self):
f"api/v1/database/{database.id}/schemas/?q={prison.dumps({'force': 'nop'})}"
)
self.assertEqual(rv.status_code, 400)

def test_get_database_related_objects(self):
"""
Database API: Test get chart and dashboard count related to a database
:return:
"""
self.login(username="admin")
database = get_example_database()
uri = f"api/v1/database/{database.id}/related_objects/"
rv = self.get_assert_metric(uri, "related_objects")
self.assertEqual(rv.status_code, 200)
response = json.loads(rv.data.decode("utf-8"))
self.assertEqual(response["charts"]["count"], 33)
self.assertEqual(response["dashboards"]["count"], 6)

def test_get_database_related_objects_not_found(self):
"""
Database API: Test related objects not found
"""
max_id = db.session.query(func.max(Database.id)).scalar()
# id does not exist and we get 404
invalid_id = max_id + 1
uri = f"api/v1/database/{invalid_id}/related_objects/"
self.login(username="admin")
rv = self.client.get(uri)
self.assertEqual(rv.status_code, 404)
self.logout()
self.login(username="gamma")
database = get_example_database()
uri = f"api/v1/database/{database.id}/related_objects/"
rv = self.client.get(uri)
self.assertEqual(rv.status_code, 404)

0 comments on commit 642aac3

Please sign in to comment.