Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: database delete warning #10800

Merged
merged 1 commit into from
Sep 9, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint';
Expand All @@ -36,6 +37,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 @@ -62,6 +64,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 @@ -100,6 +112,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 @@ -114,6 +130,9 @@ describe('DatabaseList', () => {

await waitForComponentToPaint(wrapper);

expect(fetchMock.calls(/database\/0\/related_objects/, 'GET')).toHaveLength(
1,
);
expect(fetchMock.calls(/database\/0/, 'DELETE')).toHaveLength(1);
});
});
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 All @@ -258,6 +273,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)