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: custom favorite filter for dashboards, charts and saved queries #11083

Merged
merged 13 commits into from
Oct 1, 2020
8 changes: 6 additions & 2 deletions superset/charts/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
ChartUpdateFailedError,
)
from superset.charts.commands.update import UpdateChartCommand
from superset.charts.filters import ChartAllTextFilter, ChartFilter
from superset.charts.filters import ChartAllTextFilter, ChartFavoriteFilter, ChartFilter
from superset.charts.schemas import (
CHART_SCHEMAS,
ChartDataQueryContextSchema,
Expand Down Expand Up @@ -139,13 +139,17 @@ class ChartRestApi(BaseSupersetModelRestApi):
"datasource_name",
"datasource_type",
"description",
"id",
"owners",
"slice_name",
"viz_type",
]
base_order = ("changed_on", "desc")
base_filters = [["id", ChartFilter, lambda: []]]
search_filters = {"slice_name": [ChartAllTextFilter]}
search_filters = {
"id": [ChartFavoriteFilter],
"slice_name": [ChartAllTextFilter],
}

# Will just affect _info endpoint
edit_columns = ["slice_name"]
Expand Down
11 changes: 11 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.models import SqlaTable
from superset.models.slice import Slice
from superset.views.base import BaseFilter
from superset.views.base_api import BaseFavoriteFilter


class ChartAllTextFilter(BaseFilter): # pylint: disable=too-few-public-methods
Expand All @@ -44,6 +45,16 @@ def apply(self, query: Query, value: Any) -> Query:
)


class ChartFavoriteFilter(BaseFavoriteFilter): # pylint: disable=too-few-public-methods
"""
Custom filter for the GET list that filters all charts that a user has favored
"""

arg_name = "chart_is_fav"
dpgaspar marked this conversation as resolved.
Show resolved Hide resolved
class_name = "slice"
model = Slice


class ChartFilter(BaseFilter): # pylint: disable=too-few-public-methods
def apply(self, query: Query, value: Any) -> Query:
if security_manager.can_access_all_datasources():
Expand Down
20 changes: 17 additions & 3 deletions superset/dashboards/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,11 @@
DashboardUpdateFailedError,
)
from superset.dashboards.commands.update import UpdateDashboardCommand
from superset.dashboards.filters import DashboardFilter, DashboardTitleOrSlugFilter
from superset.dashboards.filters import (
DashboardFavoriteFilter,
DashboardFilter,
DashboardTitleOrSlugFilter,
)
from superset.dashboards.schemas import (
DashboardPostSchema,
DashboardPutSchema,
Expand Down Expand Up @@ -142,8 +146,18 @@ class DashboardRestApi(BaseSupersetModelRestApi):
]
edit_columns = add_columns

search_columns = ("dashboard_title", "slug", "owners", "published", "created_by")
search_filters = {"dashboard_title": [DashboardTitleOrSlugFilter]}
search_columns = (
"created_by",
"dashboard_title",
"id",
"owners",
"published",
"slug",
)
search_filters = {
"dashboard_title": [DashboardTitleOrSlugFilter],
"id": [DashboardFavoriteFilter],
}
base_order = ("changed_on", "desc")

add_model_schema = DashboardPostSchema()
Expand Down
13 changes: 13 additions & 0 deletions superset/dashboards/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
from superset.models.dashboard import Dashboard
from superset.models.slice import Slice
from superset.views.base import BaseFilter, get_user_roles
from superset.views.base_api import BaseFavoriteFilter


class DashboardTitleOrSlugFilter(BaseFilter): # pylint: disable=too-few-public-methods
Expand All @@ -43,6 +44,18 @@ def apply(self, query: Query, value: Any) -> Query:
)


class DashboardFavoriteFilter(
BaseFavoriteFilter
): # pylint: disable=too-few-public-methods
"""
Custom filter for the GET list that filters all dashboards that a user has favored
"""

arg_name = "dashboard_is_fav"
class_name = "Dashboard"
model = Dashboard


class DashboardFilter(BaseFilter): # pylint: disable=too-few-public-methods
"""
List dashboards with the following criteria:
Expand Down
3 changes: 3 additions & 0 deletions superset/models/sql_lab.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,9 @@ class SavedQuery(Model, AuditMixinNullable, ExtraJSONMixin):
backref=backref("saved_queries", cascade="all, delete-orphan"),
)

def __repr__(self) -> str:
return str(self.label)

@property
def pop_tab_link(self) -> Markup:
return Markup(
Expand Down
7 changes: 6 additions & 1 deletion superset/queries/saved_queries/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
)
from superset.queries.saved_queries.filters import (
SavedQueryAllTextFilter,
SavedQueryFavoriteFilter,
SavedQueryFilter,
)
from superset.queries.saved_queries.schemas import (
Expand Down Expand Up @@ -96,7 +97,11 @@ class SavedQueryRestApi(BaseSupersetModelRestApi):
"database.database_name",
]

search_filters = {"label": [SavedQueryAllTextFilter]}
search_columns = ["id", "label"]
search_filters = {
"id": [SavedQueryFavoriteFilter],
"label": [SavedQueryAllTextFilter],
}

apispec_parameter_schemas = {
"get_delete_ids_schema": get_delete_ids_schema,
Expand Down
14 changes: 14 additions & 0 deletions superset/queries/saved_queries/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

from superset.models.sql_lab import SavedQuery
from superset.views.base import BaseFilter
from superset.views.base_api import BaseFavoriteFilter


class SavedQueryAllTextFilter(BaseFilter): # pylint: disable=too-few-public-methods
Expand All @@ -44,6 +45,19 @@ def apply(self, query: Query, value: Any) -> Query:
)


class SavedQueryFavoriteFilter(
BaseFavoriteFilter
): # pylint: disable=too-few-public-methods
"""
Custom filter for the GET list that filters all saved queries that a user has
favored
"""

arg_name = "saved_query_is_fav"
class_name = "query"
model = SavedQuery


class SavedQueryFilter(BaseFilter): # pylint: disable=too-few-public-methods
def apply(self, query: BaseQuery, value: Any) -> BaseQuery:
"""
Expand Down
38 changes: 35 additions & 3 deletions superset/views/base_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,22 @@

from apispec import APISpec
from apispec.exceptions import DuplicateComponentNameError
from flask import Blueprint, Response
from flask import Blueprint, g, Response
from flask_appbuilder import AppBuilder, ModelRestApi
from flask_appbuilder.api import expose, protect, rison, safe
from flask_appbuilder.models.filters import BaseFilter, Filters
from flask_appbuilder.models.sqla.filters import FilterStartsWith
from flask_appbuilder.models.sqla.interface import SQLAInterface
from flask_babel import lazy_gettext as _
from marshmallow import fields, Schema
from sqlalchemy import distinct, func

from sqlalchemy import and_, distinct, func
from sqlalchemy.orm.query import Query

from superset.extensions import db, security_manager
from superset.models.core import FavStar
from superset.models.dashboard import Dashboard
from superset.models.slice import Slice
from superset.sql_lab import Query as SqllabQuery
from superset.stats_logger import BaseStatsLogger
from superset.typing import FlaskResponse
from superset.utils.core import time_function
Expand Down Expand Up @@ -84,6 +91,31 @@ def __init__(self, field_name: str, filter_class: Type[BaseFilter]):
self.filter_class = filter_class


class BaseFavoriteFilter(BaseFilter): # pylint: disable=too-few-public-methods
"""
Base Custom filter for the GET list that filters all dashboards, slices
that a user has favored or not
"""

name = _("Is favorite")
arg_name = ""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could even leverage this default and leave it blank in the subclasses :) (re: favorited)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, nice catch!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would create a BaseOuterJoinFilter with "where" and "on" arguments and then extend it

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's interesting can you add more detail or an example?

class_name = ""
""" The FavStar class_name to user """
model: Type[Union[Dashboard, Slice, SqllabQuery]] = Dashboard
""" The SQLAlchemy model """

def apply(self, query: Query, value: Any) -> Query:
# If anonymous user filter nothing
if security_manager.current_user is None:
return query
users_favorite_query = db.session.query(FavStar.obj_id).filter(
and_(FavStar.user_id == g.user.id, FavStar.class_name == self.class_name)
)
if value:
return query.filter(and_(self.model.id.in_(users_favorite_query)))
return query.filter(and_(~self.model.id.in_(users_favorite_query)))


class BaseSupersetModelRestApi(ModelRestApi):
"""
Extends FAB's ModelResApi to implement specific superset generic functionality
Expand Down
74 changes: 74 additions & 0 deletions tests/charts/api_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,14 @@
import humanize
import prison
import pytest
from sqlalchemy import and_
from sqlalchemy.sql import func

from superset.utils.core import get_example_database
from tests.test_app import app
from superset.connectors.connector_registry import ConnectorRegistry
from superset.extensions import db, security_manager
from superset.models.core import FavStar
from superset.models.dashboard import Dashboard
from superset.models.slice import Slice
from superset.utils import core as utils
Expand All @@ -38,6 +40,7 @@
from tests.fixtures.query_context import get_query_context

CHART_DATA_URI = "api/v1/chart/data"
CHARTS_FIXTURE_COUNT = 10


class TestChartApi(SupersetTestCase, ApiOwnersTestCaseMixin):
Expand Down Expand Up @@ -76,6 +79,30 @@ def insert_chart(
db.session.commit()
return slice

@pytest.fixture()
def create_charts(self):
with self.create_app().app_context():
charts = []
admin = self.get_user("admin")
for cx in range(CHARTS_FIXTURE_COUNT - 1):
charts.append(self.insert_chart(f"name{cx}", [admin.id], 1))
fav_charts = []
for cx in range(round(CHARTS_FIXTURE_COUNT / 2)):
fav_star = FavStar(
user_id=admin.id, class_name="slice", obj_id=charts[cx].id
)
db.session.add(fav_star)
db.session.commit()
fav_charts.append(fav_star)
yield charts

# rollback changes
for chart in charts:
db.session.delete(chart)
for fav_chart in fav_charts:
db.session.delete(fav_chart)
db.session.commit()

def test_delete_chart(self):
"""
Chart API: Test delete
Expand Down Expand Up @@ -656,6 +683,53 @@ def test_get_charts_custom_filter(self):
db.session.delete(chart5)
db.session.commit()

@pytest.mark.usefixtures("create_charts")
def test_get_charts_favorite_filter(self):
"""
Chart API: Test get charts favorite filter
"""
admin = self.get_user("admin")
users_favorite_query = db.session.query(FavStar.obj_id).filter(
and_(FavStar.user_id == admin.id, FavStar.class_name == "slice")
)
expected_models = (
db.session.query(Slice)
.filter(and_(Slice.id.in_(users_favorite_query)))
.order_by(Slice.slice_name.asc())
.all()
)

arguments = {
"filters": [{"col": "id", "opr": "chart_is_fav", "value": True}],
"order_column": "slice_name",
"order_direction": "asc",
"keys": ["none"],
"columns": ["slice_name"],
}
self.login(username="admin")
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"]

# Test not favorite charts
expected_models = (
db.session.query(Slice)
.filter(and_(~Slice.id.in_(users_favorite_query)))
.order_by(Slice.slice_name.asc())
.all()
)
arguments["filters"][0]["value"] = False
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"]

def test_get_charts_page(self):
"""
Chart API: Test get charts filter
Expand Down
Loading