Skip to content

Commit

Permalink
feat: deprecate created_slices API endpoint (#21664)
Browse files Browse the repository at this point in the history
Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com>
  • Loading branch information
dpgaspar and villebro authored Oct 4, 2022
1 parent 5da20f4 commit 3057e42
Show file tree
Hide file tree
Showing 9 changed files with 132 additions and 28 deletions.
41 changes: 29 additions & 12 deletions superset-frontend/src/profile/components/CreatedContent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,30 +18,45 @@
*/
import rison from 'rison';
import React from 'react';
import moment from 'moment';
import { t } from '@superset-ui/core';

import TableLoader from '../../components/TableLoader';
import { Slice } from '../types';
import { User, DashboardResponse } from '../../types/bootstrapTypes';
import TableLoader from 'src/components/TableLoader';
import {
User,
DashboardResponse,
ChartResponse,
} from 'src/types/bootstrapTypes';

interface CreatedContentProps {
user: User;
}

class CreatedContent extends React.PureComponent<CreatedContentProps> {
renderSliceTable() {
const mutator = (data: Slice[]) =>
data.map(slice => ({
slice: <a href={slice.url}>{slice.title}</a>,
created: moment.utc(slice.dttm).fromNow(),
_created: slice.dttm,
const search = [
{ col: 'created_by', opr: 'chart_created_by_me', value: 'me' },
];
const query = rison.encode({
keys: ['none'],
columns: ['created_on_delta_humanized', 'slice_name', 'url'],
filters: search,
order_column: 'changed_on_delta_humanized',
order_direction: 'desc',
page: 0,
page_size: 100,
});

const mutator = (data: ChartResponse) =>
data.result.map(chart => ({
chart: <a href={chart.url}>{chart.slice_name}</a>,
created: chart.created_on_delta_humanized,
_created: chart.created_on_delta_humanized,
}));
return (
<TableLoader
dataEndpoint={`/superset/created_slices/${this.props.user.userId}/`}
dataEndpoint={`/api/v1/chart/?q=${query}`}
className="table-condensed"
columns={['slice', 'created']}
columns={['chart', 'created']}
mutator={mutator}
noDataText={t('No charts')}
sortable
Expand All @@ -50,7 +65,9 @@ class CreatedContent extends React.PureComponent<CreatedContentProps> {
}

renderDashboardTable() {
const search = [{ col: 'created_by', opr: 'created_by_me', value: 'me' }];
const search = [
{ col: 'created_by', opr: 'dashboard_created_by_me', value: 'me' },
];
const query = rison.encode({
keys: ['none'],
columns: ['created_on_delta_humanized', 'dashboard_title', 'url'],
Expand Down
10 changes: 10 additions & 0 deletions superset-frontend/src/types/bootstrapTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,16 @@ export type DashboardResponse = {
result: DashboardData[];
};

export type ChartData = {
slice_name: string;
created_on_delta_humanized?: string;
url: string;
};

export type ChartResponse = {
result: ChartData[];
};

export interface CommonBootstrapData {
flash_messages: string[][];
conf: JsonObject;
Expand Down
4 changes: 3 additions & 1 deletion superset/charts/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
from superset.charts.filters import (
ChartAllTextFilter,
ChartCertifiedFilter,
ChartCreatedByMeFilter,
ChartFavoriteFilter,
ChartFilter,
ChartHasCreatedByFilter,
Expand Down Expand Up @@ -150,6 +151,7 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]:
"created_by.first_name",
"created_by.id",
"created_by.last_name",
"created_on_delta_humanized",
"datasource_id",
"datasource_name_text",
"datasource_type",
Expand Down Expand Up @@ -211,7 +213,7 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]:
search_filters = {
"id": [ChartFavoriteFilter, ChartCertifiedFilter],
"slice_name": [ChartAllTextFilter],
"created_by": [ChartHasCreatedByFilter],
"created_by": [ChartHasCreatedByFilter, ChartCreatedByMeFilter],
}

# Will just affect _info endpoint
Expand Down
16 changes: 16 additions & 0 deletions superset/charts/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
from superset.connectors.sqla import models
from superset.connectors.sqla.models import SqlaTable
from superset.models.slice import Slice
from superset.utils.core import get_user_id
from superset.views.base import BaseFilter
from superset.views.base_api import BaseFavoriteFilter

Expand Down Expand Up @@ -109,3 +110,18 @@ def apply(self, query: Query, value: Any) -> Query:
if value is False:
return query.filter(and_(Slice.created_by_fk.is_(None)))
return query


class ChartCreatedByMeFilter(BaseFilter): # pylint: disable=too-few-public-methods
name = _("Created by me")
arg_name = "chart_created_by_me"

def apply(self, query: Query, value: Any) -> Query:
return query.filter(
or_(
Slice.created_by_fk # pylint: disable=comparison-with-callable
== get_user_id(),
Slice.changed_by_fk # pylint: disable=comparison-with-callable
== get_user_id(),
)
)
2 changes: 1 addition & 1 deletion superset/dashboards/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def apply(self, query: Query, value: Any) -> Query:

class DashboardCreatedByMeFilter(BaseFilter): # pylint: disable=too-few-public-methods
name = _("Created by me")
arg_name = "created_by_me"
arg_name = "dashboard_created_by_me"

def apply(self, query: Query, value: Any) -> Query:
return query.filter(
Expand Down
5 changes: 5 additions & 0 deletions superset/views/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -1708,6 +1708,11 @@ def user_slices(self, user_id: Optional[int] = None) -> FlaskResponse:
@expose("/created_slices/<int:user_id>/", methods=["GET"])
def created_slices(self, user_id: Optional[int] = None) -> FlaskResponse:
"""List of slices created by this user"""
logger.warning(
"%s.created_slices "
"This API endpoint is deprecated and will be removed in version 3.0.0",
self.__class__.__name__,
)
if not user_id:
user_id = cast(int, get_user_id())
error_obj = self.get_user_activity_access_error(user_id)
Expand Down
40 changes: 40 additions & 0 deletions tests/integration_tests/charts/api_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,19 @@ def create_charts(self):
db.session.delete(fav_chart)
db.session.commit()

@pytest.fixture()
def create_charts_created_by_gamma(self):
with self.create_app().app_context():
charts = []
user = self.get_user("gamma")
for cx in range(CHARTS_FIXTURE_COUNT - 1):
charts.append(self.insert_chart(f"gamma{cx}", [user.id], 1))
yield charts
# rollback changes
for chart in charts:
db.session.delete(chart)
db.session.commit()

@pytest.fixture()
def create_certified_charts(self):
with self.create_app().app_context():
Expand Down Expand Up @@ -1124,6 +1137,33 @@ def test_get_charts_favorite_filter(self):
assert rv.status_code == 200
assert len(expected_models) == data["count"]

@pytest.mark.usefixtures("create_charts_created_by_gamma")
def test_get_charts_created_by_me_filter(self):
"""
Chart API: Test get charts with created by me special filter
"""
gamma_user = self.get_user("gamma")
expected_models = (
db.session.query(Slice).filter(Slice.created_by_fk == gamma_user.id).all()
)
arguments = {
"filters": [
{"col": "created_by", "opr": "chart_created_by_me", "value": "me"}
],
"order_column": "slice_name",
"order_direction": "asc",
"keys": ["none"],
"columns": ["slice_name"],
}
self.login(username="gamma")
uri = f"api/v1/chart/?q={prison.dumps(arguments)}"
rv = self.client.get(uri)
data = json.loads(rv.data.decode("utf-8"))
assert rv.status_code == 200
assert len(expected_models) == data["count"]
for i, expected_model in enumerate(expected_models):
assert expected_model.slice_name == data["result"][i]["slice_name"]

@pytest.mark.usefixtures("create_charts")
def test_get_current_user_favorite_status(self):
"""
Expand Down
38 changes: 25 additions & 13 deletions tests/integration_tests/dashboards/api_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,16 +161,16 @@ def create_dashboards(self):
db.session.commit()

@pytest.fixture()
def create_created_by_admin_dashboards(self):
def create_created_by_gamma_dashboards(self):
with self.create_app().app_context():
dashboards = []
admin = self.get_user("admin")
gamma = self.get_user("gamma")
for cx in range(2):
dashboard = self.insert_dashboard(
f"create_title{cx}",
f"create_slug{cx}",
[admin.id],
created_by=admin,
[gamma.id],
created_by=gamma,
)
sleep(1)
dashboards.append(dashboard)
Expand Down Expand Up @@ -697,21 +697,23 @@ def test_gets_not_certified_dashboards_filter(self):
data = json.loads(rv.data.decode("utf-8"))
self.assertEqual(data["count"], 5)

@pytest.mark.usefixtures("create_created_by_admin_dashboards")
@pytest.mark.usefixtures("create_created_by_gamma_dashboards")
def test_get_dashboards_created_by_me(self):
"""
Dashboard API: Test get dashboards created by current user
"""
query = {
"columns": ["created_on_delta_humanized", "dashboard_title", "url"],
"filters": [{"col": "created_by", "opr": "created_by_me", "value": "me"}],
"filters": [
{"col": "created_by", "opr": "dashboard_created_by_me", "value": "me"}
],
"order_column": "changed_on",
"order_direction": "desc",
"page": 0,
"page_size": 100,
}
uri = f"api/v1/dashboard/?q={prison.dumps(query)}"
self.login(username="admin")
self.login(username="gamma")
rv = self.client.get(uri)
data = json.loads(rv.data.decode("utf-8"))
assert rv.status_code == 200
Expand Down Expand Up @@ -1837,11 +1839,17 @@ def test_embedded_dashboards(self):
resp = self.get_assert_metric(uri, "get_embedded")
self.assertEqual(resp.status_code, 404)

@pytest.mark.usefixtures("create_created_by_admin_dashboards")
@pytest.mark.usefixtures("create_created_by_gamma_dashboards")
def test_gets_created_by_user_dashboards_filter(self):
expected_models = (
db.session.query(Dashboard)
.filter(Dashboard.created_by_fk.isnot(None))
.all()
)

arguments = {
"filters": [
{"col": "id", "opr": "dashboard_has_created_by", "value": True}
{"col": "created_by", "opr": "dashboard_has_created_by", "value": True}
],
"keys": ["none"],
"columns": ["dashboard_title"],
Expand All @@ -1852,23 +1860,27 @@ def test_gets_created_by_user_dashboards_filter(self):
rv = self.get_assert_metric(uri, "get_list")
self.assertEqual(rv.status_code, 200)
data = json.loads(rv.data.decode("utf-8"))
self.assertEqual(data["count"], 7)
self.assertEqual(data["count"], len(expected_models))

def test_gets_not_created_by_user_dashboards_filter(self):
dashboard = self.insert_dashboard(f"title", f"slug", [])
expected_models = (
db.session.query(Dashboard).filter(Dashboard.created_by_fk.is_(None)).all()
)

arguments = {
"filters": [
{"col": "id", "opr": "dashboard_has_created_by", "value": False}
{"col": "created_by", "opr": "dashboard_has_created_by", "value": False}
],
"keys": ["none"],
"columns": ["dashboard_title"],
}
dashboard = self.insert_dashboard(f"title", f"slug", [])
self.login(username="admin")

uri = f"api/v1/dashboard/?q={prison.dumps(arguments)}"
rv = self.get_assert_metric(uri, "get_list")
self.assertEqual(rv.status_code, 200)
data = json.loads(rv.data.decode("utf-8"))
self.assertEqual(data["count"], 6)
self.assertEqual(data["count"], len(expected_models))
db.session.delete(dashboard)
db.session.commit()
4 changes: 3 additions & 1 deletion tests/integration_tests/model_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -592,7 +592,9 @@ def test_data_for_slices_with_query_context(self):
assert len(data_for_slices["metrics"]) == 1
assert len(data_for_slices["columns"]) == 2
assert data_for_slices["metrics"][0]["metric_name"] == "sum__num"
assert data_for_slices["columns"][0]["column_name"] == "name"
column_names = [col["column_name"] for col in data_for_slices["columns"]]
assert "name" in column_names
assert "state" in column_names
assert set(data_for_slices["verbose_map"].keys()) == {
"__timestamp",
"sum__num",
Expand Down

0 comments on commit 3057e42

Please sign in to comment.