chore: Migrate /superset/user_slices and /superset/fave_slices to API v1 (#22964)

Co-authored-by: hughhhh <hughmil3s@gmail.com>

docs/static/resources/openapi.json

@@ -1783,12 +1783,21 @@
           "changed_on_delta_humanized": {
             "readOnly": true
           },
+          "changed_on_dttm": {
+            "readOnly": true
+          },
           "changed_on_utc": {
             "readOnly": true
           },
           "created_by": {
             "$ref": "#/components/schemas/ChartDataRestApi.get_list.User2"
           },
+          "created_by_name": {
+            "readOnly": true
+          },
+          "created_by_url": {
+            "readOnly": true
+          },
           "created_on_delta_humanized": {
             "readOnly": true
           },
@@ -1821,6 +1830,9 @@
           "edit_url": {
             "readOnly": true
           },
+          "form_data": {
+            "readOnly": true
+          },
           "id": {
             "format": "int32",
             "type": "integer"
@@ -1848,6 +1860,9 @@
             "nullable": true,
             "type": "string"
           },
+          "slice_url": {
+            "readOnly": true
+          },
           "table": {
             "$ref": "#/components/schemas/ChartDataRestApi.get_list.SqlaTable"
           },
@@ -2576,12 +2591,21 @@
           "changed_on_delta_humanized": {
             "readOnly": true
           },
+          "changed_on_dttm": {
+            "readOnly": true
+          },
           "changed_on_utc": {
             "readOnly": true
           },
           "created_by": {
             "$ref": "#/components/schemas/ChartRestApi.get_list.User2"
           },
+          "created_by_name": {
+            "readOnly": true
+          },
+          "created_by_url": {
+            "readOnly": true
+          },
           "created_on_delta_humanized": {
             "readOnly": true
           },
@@ -2614,6 +2638,9 @@
           "edit_url": {
             "readOnly": true
           },
+          "form_data": {
+            "readOnly": true
+          },
           "id": {
             "format": "int32",
             "type": "integer"
@@ -2641,6 +2668,9 @@
             "nullable": true,
             "type": "string"
           },
+          "slice_url": {
+            "readOnly": true
+          },
           "table": {
             "$ref": "#/components/schemas/ChartRestApi.get_list.SqlaTable"
           },

superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx

@@ -17,6 +17,7 @@
  * under the License.
  */
 import React from 'react';
+import rison from 'rison';
 import PropTypes from 'prop-types';
 import { CompactPicker } from 'react-color';
 import Button from 'src/components/Button';
@@ -315,34 +316,45 @@ class AnnotationLayer extends React.PureComponent {
           });
         });
       } else if (requiresQuery(sourceType)) {
-        SupersetClient.get({ endpoint: '/superset/user_slices' }).then(
+        const queryParams = rison.encode({
-          ({ json }) => {
+          filters: [
-            const registry = getChartMetadataRegistry();
+            {
-            this.setState({
+              col: 'id',
-              isLoadingOptions: false,
+              opr: 'chart_owned_created_favored_by_me',
-              valueOptions: json
+              value: true,
-                .filter(x => {
+            },
-                  const metadata = registry.get(x.viz_type);
+          ],
-                  return (
+          order_column: 'slice_name',
-                    metadata && metadata.canBeAnnotationType(annotationType)
+          order_direction: 'asc',
-                  );
+          page: 0,
-                })
+          page_size: 25,
-                .map(x => ({
+        });
-                  value: x.id,
+        SupersetClient.get({
-                  label: x.title,
+          endpoint: `/api/v1/chart/?q=${queryParams}`,
-                  slice: {
+        }).then(({ json }) => {
-                    ...x,
+          const registry = getChartMetadataRegistry();
-                    data: {
+          this.setState({
-                      ...x.data,
+            isLoadingOptions: false,
-                      groupby: x.data.groupby?.map(column =>
+            valueOptions: json.result
-                        getColumnLabel(column),
+              .filter(x => {
-                      ),
+                const metadata = registry.get(x.viz_type);
-                    },
+                return metadata && metadata.canBeAnnotationType(annotationType);
+              })
+              .map(x => ({
+                value: x.id,
+                label: x.slice_name,
+                slice: {
+                  ...x,
+                  data: {
+                    ...x.form_data,
+                    groupby: x.form_data.groupby?.map(column =>
+                      getColumnLabel(column),
+                    ),
                   },
-                })),
+                },
-            });
+              })),
-          },
+          });
-        );
+        });
       } else {
         this.setState({
           isLoadingOptions: false,

superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.test.tsx

@@ -40,9 +40,11 @@ beforeAll(() => {
     result: [{ label: 'Chart A', value: 'a' }],
   });
 
-  fetchMock.get('glob:*/superset/user_slices*', [
+  fetchMock.get('glob:*/api/v1/chart/*', {
-    { id: 'a', title: 'Chart A', viz_type: 'table', data: {} },
+    result: [
-  ]);
+      { id: 'a', slice_name: 'Chart A', viz_type: 'table', form_data: {} },
+    ],
+  });
 
   setupColors();
 

superset-frontend/src/profile/components/Favorites.tsx

@@ -22,7 +22,7 @@ import moment from 'moment';
 import { t } from '@superset-ui/core';
 import { DashboardResponse, BootstrapUser } from 'src/types/bootstrapTypes';
 import TableLoader from '../../components/TableLoader';
-import { Slice } from '../types';
+import { Chart } from '../types';
 
 interface FavoritesProps {
   user: BootstrapUser;
@@ -30,16 +30,31 @@ interface FavoritesProps {
 
 export default class Favorites extends React.PureComponent<FavoritesProps> {
   renderSliceTable() {
-    const mutator = (data: Slice[]) =>
+    const mutator = (payload: { result: Chart[] }) =>
-      data.map(slice => ({
+      payload.result.map(slice => ({
-        slice: <a href={slice.url}>{slice.title}</a>,
+        slice: <a href={slice.slice_url}>{slice.slice_name}</a>,
-        creator: <a href={slice.creator_url}>{slice.creator}</a>,
+        creator: <a href={slice.created_by_url}>{slice.created_by_name}</a>,
-        favorited: moment.utc(slice.dttm).fromNow(),
+        favorited: moment.utc(slice.changed_on_dttm).fromNow(),
-        _favorited: slice.dttm,
+        _favorited: slice.changed_on_dttm,
       }));
+
+    const query = rison.encode({
+      filters: [
+        {
+          col: 'id',
+          opr: 'chart_is_favorite',
+          value: true,
+        },
+      ],
+      order_column: 'slice_name',
+      order_direction: 'asc',
+      page: 0,
+      page_size: 25,
+    });
+
     return (
       <TableLoader
-        dataEndpoint={`/superset/fave_slices/${this.props.user?.userId}/`}
+        dataEndpoint={`/api/v1/chart/?q=${query}`}
         className="table-condensed"
         columns={['slice', 'creator', 'favorited']}
         mutator={mutator}

superset-frontend/src/profile/types.ts

@@ -26,6 +26,15 @@ export type Slice = {
   viz_type: string;
 };
 
+export type Chart = {
+  id: number;
+  slice_name: string;
+  slice_url: string;
+  created_by_name?: string;
+  created_by_url?: string;
+  changed_on_dttm: number;
+};
+
 export type Activity = {
   action: string;
   item_title: string;

superset/charts/api.py

@@ -55,6 +55,7 @@ from superset.charts.filters import (
     ChartFavoriteFilter,
     ChartFilter,
     ChartHasCreatedByFilter,
+    ChartOwnedCreatedFavoredByMeFilter,
     ChartTagFilter,
 )
 from superset.charts.schemas import (
@@ -158,10 +159,13 @@ class ChartRestApi(BaseSupersetModelRestApi):
         "changed_by_name",
         "changed_by_url",
         "changed_on_delta_humanized",
+        "changed_on_dttm",
         "changed_on_utc",
         "created_by.first_name",
         "created_by.id",
         "created_by.last_name",
+        "created_by_name",
+        "created_by_url",
         "created_on_delta_humanized",
         "datasource_id",
         "datasource_name_text",
@@ -170,6 +174,7 @@ class ChartRestApi(BaseSupersetModelRestApi):
         "description",
         "description_markeddown",
         "edit_url",
+        "form_data",
         "id",
         "last_saved_at",
         "last_saved_by.id",
@@ -183,6 +188,7 @@ class ChartRestApi(BaseSupersetModelRestApi):
         "dashboards.dashboard_title",
         "params",
         "slice_name",
+        "slice_url",
         "table.default_endpoint",
         "table.table_name",
         "thumbnail_url",
@@ -224,7 +230,11 @@ class ChartRestApi(BaseSupersetModelRestApi):
     base_order = ("changed_on", "desc")
     base_filters = [["id", ChartFilter, lambda: []]]
     search_filters = {
-        "id": [ChartFavoriteFilter, ChartCertifiedFilter],
+        "id": [
+            ChartFavoriteFilter,
+            ChartCertifiedFilter,
+            ChartOwnedCreatedFavoredByMeFilter,
+        ],
         "slice_name": [ChartAllTextFilter],
         "created_by": [ChartHasCreatedByFilter, ChartCreatedByMeFilter],
     }

superset/charts/filters.py

@@ -21,9 +21,10 @@ from sqlalchemy import and_, or_
 from sqlalchemy.orm import aliased
 from sqlalchemy.orm.query import Query
 
-from superset import security_manager
+from superset import db, security_manager
 from superset.connectors.sqla import models
 from superset.connectors.sqla.models import SqlaTable
+from superset.models.core import FavStar
 from superset.models.slice import Slice
 from superset.utils.core import get_user_id
 from superset.utils.filters import get_dataset_access_filters
@@ -127,3 +128,43 @@ class ChartCreatedByMeFilter(BaseFilter):  # pylint: disable=too-few-public-meth
                 == get_user_id(),
             )
         )
+
+
+class ChartOwnedCreatedFavoredByMeFilter(
+    BaseFilter
+):  # pylint: disable=too-few-public-methods
+    """
+    Custom filter for the GET chart that filters all charts the user
+    owns, created, changed or favored.
+    """
+
+    name = _("Owned Created or Favored")
+    arg_name = "chart_owned_created_favored_by_me"
+
+    def apply(self, query: Query, value: Any) -> Query:
+        # If anonymous user filter nothing
+        if security_manager.current_user is None:
+            return query
+
+        owner_ids_query = (
+            db.session.query(Slice.id)
+            .join(Slice.owners)
+            .filter(security_manager.user_model.id == get_user_id())
+        )
+
+        return query.join(
+            FavStar,
+            and_(
+                FavStar.user_id == get_user_id(),
+                FavStar.class_name == "slice",
+                Slice.id == FavStar.obj_id,
+            ),
+            isouter=True,
+        ).filter(  # pylint: disable=comparison-with-callable
+            or_(
+                Slice.id.in_(owner_ids_query),
+                Slice.created_by_fk == get_user_id(),
+                Slice.changed_by_fk == get_user_id(),
+                FavStar.user_id == get_user_id(),
+            )
+        )

superset/constants.py

@@ -147,6 +147,7 @@ MODEL_API_RW_METHOD_PERMISSION_MAP = {
     "delete_ssh_tunnel": "write",
     "get_updated_since": "read",
     "stop_query": "read",
+    "get_user_slices": "read",
     "schemas_access_for_file_upload": "read",
     "get_objects": "read",
     "get_all_objects": "read",

superset/models/helpers.py

@@ -89,6 +89,7 @@ from superset.superset_typing import (
 )
 from superset.utils import core as utils
 from superset.utils.core import get_user_id
+from superset.utils.dates import datetime_to_epoch
 
 if TYPE_CHECKING:
     from superset.connectors.sqla.models import SqlMetric, TableColumn
@@ -492,6 +493,12 @@ class AuditMixinNullable(AuditMixin):
             nullable=True,
         )
 
+    @property
+    def created_by_name(self) -> str:
+        if self.created_by:
+            return escape("{}".format(self.created_by))
+        return ""
+
     @property
     def changed_by_name(self) -> str:
         if self.changed_by:
@@ -514,6 +521,10 @@ class AuditMixinNullable(AuditMixin):
     def changed_on_delta_humanized(self) -> str:
         return self.changed_on_humanized
 
+    @renders("changed_on")
+    def changed_on_dttm(self) -> float:
+        return datetime_to_epoch(self.changed_on)
+
     @renders("created_on")
     def created_on_delta_humanized(self) -> str:
         return self.created_on_humanized

superset/models/slice.py

@@ -332,6 +332,12 @@ class Slice(  # pylint: disable=too-many-public-methods
         name = escape(self.chart)
         return Markup(f'<a href="{self.url}">{name}</a>')
 
+    @property
+    def created_by_url(self) -> str:
+        if not self.created_by:
+            return ""
+        return f"/superset/profile/{self.created_by.username}"
+
     @property
     def changed_by_url(self) -> str:
         return f"/superset/profile/{self.changed_by.username}"  # type: ignore

superset/views/core.py

@@ -1559,6 +1559,7 @@ class Superset(BaseSupersetView):  # pylint: disable=too-many-public-methods
     @event_logger.log_this
     @expose("/user_slices", methods=["GET"])
     @expose("/user_slices/<int:user_id>/", methods=["GET"])
+    @deprecated()
     def user_slices(self, user_id: Optional[int] = None) -> FlaskResponse:
         """List of slices a user owns, created, modified or faved"""
         if not user_id:
@@ -1644,6 +1645,7 @@ class Superset(BaseSupersetView):  # pylint: disable=too-many-public-methods
     @event_logger.log_this
     @expose("/fave_slices", methods=["GET"])
     @expose("/fave_slices/<int:user_id>/", methods=["GET"])
+    @deprecated()
     def fave_slices(self, user_id: Optional[int] = None) -> FlaskResponse:
         """Favorite slices for a user"""
         if user_id is None:

tests/integration_tests/charts/api_tests.py

@@ -28,7 +28,7 @@ from sqlalchemy import and_
 from sqlalchemy.sql import func
 
 from superset.connectors.sqla.models import SqlaTable
-from superset.extensions import cache_manager, db
+from superset.extensions import cache_manager, db, security_manager
 from superset.models.core import Database, FavStar, FavStarClassName
 from superset.models.dashboard import Dashboard
 from superset.reports.models import ReportSchedule, ReportScheduleType
@@ -1600,3 +1600,29 @@ class TestChartApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixin):
         self.assertEqual(rv.status_code, 200)
         data = json.loads(rv.data.decode("utf-8"))
         self.assertEqual(data["count"], 8)
+
+    @pytest.mark.usefixtures("create_charts")
+    def test_gets_owned_created_favorited_by_me_filter(self):
+        """
+        Chart API: Test ChartOwnedCreatedFavoredByMeFilter
+        """
+        self.login(username="admin")
+        arguments = {
+            "filters": [
+                {
+                    "col": "id",
+                    "opr": "chart_owned_created_favored_by_me",
+                    "value": True,
+                }
+            ],
+            "order_column": "slice_name",
+            "order_direction": "asc",
+            "page": 0,
+            "page_size": 25,
+        }
+        rv = self.client.get(f"api/v1/chart/?q={prison.dumps(arguments)}")
+        self.assertEqual(rv.status_code, 200)
+        data = json.loads(rv.data.decode("utf-8"))
+
+        assert data["result"][0]["slice_name"] == "name0"
+        assert data["result"][0]["datasource_id"] == 1

tests/integration_tests/charts/commands_tests.py

@@ -377,7 +377,7 @@ class TestChartsUpdateCommand(SupersetTestCase):
     def test_query_context_update_command(self, mock_sm_g, mock_g):
         """
         Test that a user can generate the chart query context
-        payloadwithout affecting owners
+        payload without affecting owners
         """
         chart = db.session.query(Slice).all()[0]
         pk = chart.id

tests/unit_tests/charts/dao/dao_tests.py

@@ -37,8 +37,8 @@ def session_with_data(session: Session) -> Iterator[Session]:
         datasource_name="tmp_perm_table",
         slice_name="slice_name",
     )
-
     session.add(slice_obj)
+
     session.commit()
     yield session
     session.rollback()