From f728b014c81d45de4ea86a6620abaa80a3a10385 Mon Sep 17 00:00:00 2001 From: Diego Medina Date: Wed, 22 Feb 2023 23:13:14 -0300 Subject: [PATCH 1/2] chore: Migrate /superset/favstar to API v1 --- docs/static/resources/openapi.json | 316 ++++++++++++++++-- .../integration/chart_list/list.test.ts | 8 +- .../cypress/integration/dashboard/utils.ts | 8 +- .../src/components/FaveStar/FaveStar.test.tsx | 4 +- .../src/components/FaveStar/index.tsx | 11 +- .../src/dashboard/actions/dashboardState.js | 18 +- .../dashboard/containers/DashboardPage.tsx | 2 +- .../src/explore/actions/exploreActions.ts | 21 +- .../ExploreViewContainer.test.tsx | 4 +- superset-frontend/src/views/CRUD/hooks.ts | 23 +- superset/charts/api.py | 91 +++++ superset/charts/dao.py | 30 ++ superset/dashboards/api.py | 90 +++++ superset/dashboards/dao.py | 29 ++ superset/views/core.py | 1 + tests/integration_tests/charts/api_tests.py | 69 ++++ .../integration_tests/dashboards/api_tests.py | 69 ++++ tests/unit_tests/charts/dao/dao_tests.py | 33 ++ tests/unit_tests/dashboards/dao_tests.py | 79 +++++ 19 files changed, 841 insertions(+), 65 deletions(-) create mode 100644 tests/unit_tests/dashboards/dao_tests.py diff --git a/docs/static/resources/openapi.json b/docs/static/resources/openapi.json index f303b83784026..f8d4daac5efcc 100644 --- a/docs/static/resources/openapi.json +++ b/docs/static/resources/openapi.json @@ -1787,7 +1787,7 @@ "readOnly": true }, "created_by": { - "$ref": "#/components/schemas/ChartDataRestApi.get_list.User2" + "$ref": "#/components/schemas/ChartDataRestApi.get_list.User3" }, "created_on_delta_humanized": { "readOnly": true @@ -1834,10 +1834,10 @@ "type": "string" }, "last_saved_by": { - "$ref": "#/components/schemas/ChartDataRestApi.get_list.User3" + "$ref": "#/components/schemas/ChartDataRestApi.get_list.User1" }, "owners": { - "$ref": "#/components/schemas/ChartDataRestApi.get_list.User1" + "$ref": "#/components/schemas/ChartDataRestApi.get_list.User2" }, "params": { "nullable": true, @@ -1925,16 +1925,11 @@ "last_name": { "maxLength": 64, "type": "string" - }, - "username": { - "maxLength": 64, - "type": "string" } }, "required": [ "first_name", - "last_name", - "username" + "last_name" ], "type": "object" }, @@ -1951,11 +1946,16 @@ "last_name": { "maxLength": 64, "type": "string" + }, + "username": { + "maxLength": 64, + "type": "string" } }, "required": [ "first_name", - "last_name" + "last_name", + "username" ], "type": "object" }, @@ -2580,7 +2580,7 @@ "readOnly": true }, "created_by": { - "$ref": "#/components/schemas/ChartRestApi.get_list.User2" + "$ref": "#/components/schemas/ChartRestApi.get_list.User3" }, "created_on_delta_humanized": { "readOnly": true @@ -2627,10 +2627,10 @@ "type": "string" }, "last_saved_by": { - "$ref": "#/components/schemas/ChartRestApi.get_list.User3" + "$ref": "#/components/schemas/ChartRestApi.get_list.User1" }, "owners": { - "$ref": "#/components/schemas/ChartRestApi.get_list.User1" + "$ref": "#/components/schemas/ChartRestApi.get_list.User2" }, "params": { "nullable": true, @@ -2718,16 +2718,11 @@ "last_name": { "maxLength": 64, "type": "string" - }, - "username": { - "maxLength": 64, - "type": "string" } }, "required": [ "first_name", - "last_name", - "username" + "last_name" ], "type": "object" }, @@ -2744,11 +2739,16 @@ "last_name": { "maxLength": 64, "type": "string" + }, + "username": { + "maxLength": 64, + "type": "string" } }, "required": [ "first_name", - "last_name" + "last_name", + "username" ], "type": "object" }, @@ -5972,6 +5972,33 @@ }, "type": "object" }, + "GetOrCreateDatasetSchema": { + "properties": { + "database_id": { + "description": "ID of database table belongs to", + "format": "int32", + "type": "integer" + }, + "schema": { + "description": "The schema the table belongs to", + "nullable": true, + "type": "string" + }, + "table_name": { + "description": "Name of table", + "type": "string" + }, + "template_params": { + "description": "Template params for the table", + "type": "string" + } + }, + "required": [ + "database_id", + "table_name" + ], + "type": "object" + }, "GuestTokenCreate": { "properties": { "resources": { @@ -11906,6 +11933,102 @@ ] } }, + "/api/v1/chart/{pk}/favorites/": { + "delete": { + "description": "Remove the chart from the user favorite list", + "parameters": [ + { + "in": "path", + "name": "pk", + "required": true, + "schema": { + "type": "integer" + } + } + ], + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "properties": { + "result": { + "type": "object" + } + }, + "type": "object" + } + } + }, + "description": "Chart removed from favorites" + }, + "401": { + "$ref": "#/components/responses/401" + }, + "404": { + "$ref": "#/components/responses/404" + }, + "500": { + "$ref": "#/components/responses/500" + } + }, + "security": [ + { + "jwt": [] + } + ], + "tags": [ + "Charts" + ] + }, + "post": { + "description": "Marks the chart as favorite for the current user", + "parameters": [ + { + "in": "path", + "name": "pk", + "required": true, + "schema": { + "type": "integer" + } + } + ], + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "properties": { + "result": { + "type": "object" + } + }, + "type": "object" + } + } + }, + "description": "Chart added to favorites" + }, + "401": { + "$ref": "#/components/responses/401" + }, + "404": { + "$ref": "#/components/responses/404" + }, + "500": { + "$ref": "#/components/responses/500" + } + }, + "security": [ + { + "jwt": [] + } + ], + "tags": [ + "Charts" + ] + } + }, "/api/v1/chart/{pk}/screenshot/{digest}/": { "get": { "description": "Get a computed screenshot from cache.", @@ -13962,6 +14085,102 @@ ] } }, + "/api/v1/dashboard/{pk}/favorites/": { + "delete": { + "description": "Remove the dashboard from the user favorite list", + "parameters": [ + { + "in": "path", + "name": "pk", + "required": true, + "schema": { + "type": "integer" + } + } + ], + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "properties": { + "result": { + "type": "object" + } + }, + "type": "object" + } + } + }, + "description": "Dashboard removed from favorites" + }, + "401": { + "$ref": "#/components/responses/401" + }, + "404": { + "$ref": "#/components/responses/404" + }, + "500": { + "$ref": "#/components/responses/500" + } + }, + "security": [ + { + "jwt": [] + } + ], + "tags": [ + "Dashboards" + ] + }, + "post": { + "description": "Marks the dashboard as favorite for the current user", + "parameters": [ + { + "in": "path", + "name": "pk", + "required": true, + "schema": { + "type": "integer" + } + } + ], + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "properties": { + "result": { + "type": "object" + } + }, + "type": "object" + } + } + }, + "description": "Dashboard added to favorites" + }, + "401": { + "$ref": "#/components/responses/401" + }, + "404": { + "$ref": "#/components/responses/404" + }, + "500": { + "$ref": "#/components/responses/500" + } + }, + "security": [ + { + "jwt": [] + } + ], + "tags": [ + "Dashboards" + ] + } + }, "/api/v1/dashboard/{pk}/filter_state": { "post": { "description": "Stores a new value.", @@ -16204,6 +16423,63 @@ ] } }, + "/api/v1/dataset/get_or_create/": { + "post": { + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/GetOrCreateDatasetSchema" + } + } + }, + "required": true + }, + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "properties": { + "result": { + "properties": { + "table_id": { + "type": "integer" + } + }, + "type": "object" + } + }, + "type": "object" + } + } + }, + "description": "The ID of the table" + }, + "400": { + "$ref": "#/components/responses/400" + }, + "401": { + "$ref": "#/components/responses/401" + }, + "422": { + "$ref": "#/components/responses/422" + }, + "500": { + "$ref": "#/components/responses/500" + } + }, + "security": [ + { + "jwt": [] + } + ], + "summary": "Retrieve a table by name, or create it if it does not exist", + "tags": [ + "Datasets" + ] + } + }, "/api/v1/dataset/import/": { "post": { "requestBody": { diff --git a/superset-frontend/cypress-base/cypress/integration/chart_list/list.test.ts b/superset-frontend/cypress-base/cypress/integration/chart_list/list.test.ts index 460b2cc02b468..6664281abe9b9 100644 --- a/superset-frontend/cypress-base/cypress/integration/chart_list/list.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/chart_list/list.test.ts @@ -184,8 +184,12 @@ describe('Charts list', () => { }); it('should allow to favorite/unfavorite', () => { - cy.intercept(`/superset/favstar/slice/*/select/`).as('select'); - cy.intercept(`/superset/favstar/slice/*/unselect/`).as('unselect'); + cy.intercept({ url: `/api/v1/chart/*/favorites/`, method: 'POST' }).as( + 'select', + ); + cy.intercept({ url: `/api/v1/chart/*/favorites/`, method: 'DELETE' }).as( + 'unselect', + ); setGridMode('card'); orderAlphabetical(); diff --git a/superset-frontend/cypress-base/cypress/integration/dashboard/utils.ts b/superset-frontend/cypress-base/cypress/integration/dashboard/utils.ts index 29f1e1c2645b3..84e82b796e0d5 100644 --- a/superset-frontend/cypress-base/cypress/integration/dashboard/utils.ts +++ b/superset-frontend/cypress-base/cypress/integration/dashboard/utils.ts @@ -139,11 +139,15 @@ export function interceptLog() { } export function interceptFav() { - cy.intercept(`/superset/favstar/Dashboard/*/select/`).as('select'); + cy.intercept({ url: `/api/v1/dashboard/*/favorites/`, method: 'POST' }).as( + 'select', + ); } export function interceptUnfav() { - cy.intercept(`/superset/favstar/Dashboard/*/unselect/`).as('unselect'); + cy.intercept({ url: `/api/v1/dashboard/*/favorites/`, method: 'POST' }).as( + 'unselect', + ); } export function interceptDataset() { diff --git a/superset-frontend/src/components/FaveStar/FaveStar.test.tsx b/superset-frontend/src/components/FaveStar/FaveStar.test.tsx index ab2fa9fa0ed1e..1ecb372dd06b9 100644 --- a/superset-frontend/src/components/FaveStar/FaveStar.test.tsx +++ b/superset-frontend/src/components/FaveStar/FaveStar.test.tsx @@ -75,7 +75,7 @@ test('render content on tooltip', async () => { expect(screen.getByRole('button')).toBeInTheDocument(); }); -test('Call fetchFaveStar only on the first render', async () => { +test('Call fetchFaveStar on first render and on itemId change', async () => { const props = { itemId: 3, fetchFaveStar: jest.fn(), @@ -92,5 +92,5 @@ test('Call fetchFaveStar only on the first render', async () => { expect(props.fetchFaveStar).toBeCalledWith(props.itemId); rerender(); - expect(props.fetchFaveStar).toBeCalledTimes(1); + expect(props.fetchFaveStar).toBeCalledTimes(2); }); diff --git a/superset-frontend/src/components/FaveStar/index.tsx b/superset-frontend/src/components/FaveStar/index.tsx index 8a6f4eca1f1e2..8b5d3ee89ae7d 100644 --- a/superset-frontend/src/components/FaveStar/index.tsx +++ b/superset-frontend/src/components/FaveStar/index.tsx @@ -17,10 +17,9 @@ * under the License. */ -import React, { useCallback } from 'react'; +import React, { useCallback, useEffect } from 'react'; import { css, t, styled } from '@superset-ui/core'; import { Tooltip } from 'src/components/Tooltip'; -import { useComponentDidMount } from 'src/hooks/useComponentDidMount'; import Icons from 'src/components/Icons'; export interface FaveStarProps { @@ -46,11 +45,9 @@ const FaveStar = ({ saveFaveStar, fetchFaveStar, }: FaveStarProps) => { - useComponentDidMount(() => { - if (fetchFaveStar) { - fetchFaveStar(itemId); - } - }); + useEffect(() => { + fetchFaveStar?.(itemId); + }, [fetchFaveStar, itemId]); const onClick = useCallback( (e: React.MouseEvent) => { diff --git a/superset-frontend/src/dashboard/actions/dashboardState.js b/superset-frontend/src/dashboard/actions/dashboardState.js index 8db1a500eee32..1f33562cb602b 100644 --- a/superset-frontend/src/dashboard/actions/dashboardState.js +++ b/superset-frontend/src/dashboard/actions/dashboardState.js @@ -18,6 +18,7 @@ */ /* eslint camelcase: 0 */ import { ActionCreators as UndoActionCreators } from 'redux-undo'; +import rison from 'rison'; import { ensureIsArray, t, @@ -79,7 +80,6 @@ export function removeSlice(sliceId) { return { type: REMOVE_SLICE, sliceId }; } -const FAVESTAR_BASE_URL = '/superset/favstar/Dashboard'; export const TOGGLE_FAVE_STAR = 'TOGGLE_FAVE_STAR'; export function toggleFaveStar(isStarred) { return { type: TOGGLE_FAVE_STAR, isStarred }; @@ -89,10 +89,10 @@ export const FETCH_FAVE_STAR = 'FETCH_FAVE_STAR'; export function fetchFaveStar(id) { return function fetchFaveStarThunk(dispatch) { return SupersetClient.get({ - endpoint: `${FAVESTAR_BASE_URL}/${id}/count/`, + endpoint: `/api/v1/dashboard/favorite_status/?q=${rison.encode([id])}`, }) .then(({ json }) => { - if (json.count > 0) dispatch(toggleFaveStar(true)); + dispatch(toggleFaveStar(!!json?.result?.[0]?.value)); }) .catch(() => dispatch( @@ -109,10 +109,14 @@ export function fetchFaveStar(id) { export const SAVE_FAVE_STAR = 'SAVE_FAVE_STAR'; export function saveFaveStar(id, isStarred) { return function saveFaveStarThunk(dispatch) { - const urlSuffix = isStarred ? 'unselect' : 'select'; - return SupersetClient.get({ - endpoint: `${FAVESTAR_BASE_URL}/${id}/${urlSuffix}/`, - }) + const endpoint = `/api/v1/dashboard/${id}/favorites/`; + const apiCall = isStarred + ? SupersetClient.delete({ + endpoint, + }) + : SupersetClient.post({ endpoint }); + + return apiCall .then(() => { dispatch(toggleFaveStar(!isStarred)); }) diff --git a/superset-frontend/src/dashboard/containers/DashboardPage.tsx b/superset-frontend/src/dashboard/containers/DashboardPage.tsx index 0066354f589b5..5b286157695b0 100644 --- a/superset-frontend/src/dashboard/containers/DashboardPage.tsx +++ b/superset-frontend/src/dashboard/containers/DashboardPage.tsx @@ -361,7 +361,7 @@ export const DashboardPage: FC = ({ idOrSlug }: PageProps) => { }, [addDangerToast, datasets, datasetsApiError, dispatch]); if (error) throw error; // caught in error boundary - if (!readyToRender) return ; + if (!readyToRender || !isDashboardHydrated.current) return ; return ( <> diff --git a/superset-frontend/src/explore/actions/exploreActions.ts b/superset-frontend/src/explore/actions/exploreActions.ts index 0e13499b14c43..36300b4a123a4 100644 --- a/superset-frontend/src/explore/actions/exploreActions.ts +++ b/superset-frontend/src/explore/actions/exploreActions.ts @@ -17,6 +17,7 @@ * under the License. */ /* eslint camelcase: 0 */ +import rison from 'rison'; import { Dataset } from '@superset-ui/chart-controls'; import { t, SupersetClient, QueryFormData } from '@superset-ui/core'; import { Dispatch } from 'redux'; @@ -27,8 +28,6 @@ import { import { Slice } from 'src/types/Chart'; import { SaveActionType } from 'src/explore/types'; -const FAVESTAR_BASE_URL = '/superset/favstar/slice'; - export const UPDATE_FORM_DATA_BY_DATASOURCE = 'UPDATE_FORM_DATA_BY_DATASOURCE'; export function updateFormDataByDatasource( prevDatasource: Dataset, @@ -66,11 +65,9 @@ export const FETCH_FAVE_STAR = 'FETCH_FAVE_STAR'; export function fetchFaveStar(sliceId: string) { return function (dispatch: Dispatch) { SupersetClient.get({ - endpoint: `${FAVESTAR_BASE_URL}/${sliceId}/count/`, + endpoint: `/api/v1/chart/favorite_status/?q=${rison.encode([sliceId])}`, }).then(({ json }) => { - if (json.count > 0) { - dispatch(toggleFaveStar(true)); - } + dispatch(toggleFaveStar(!!json?.result?.[0]?.value)); }); }; } @@ -78,10 +75,14 @@ export function fetchFaveStar(sliceId: string) { export const SAVE_FAVE_STAR = 'SAVE_FAVE_STAR'; export function saveFaveStar(sliceId: string, isStarred: boolean) { return function (dispatch: Dispatch) { - const urlSuffix = isStarred ? 'unselect' : 'select'; - SupersetClient.get({ - endpoint: `${FAVESTAR_BASE_URL}/${sliceId}/${urlSuffix}/`, - }) + const endpoint = `/api/v1/chart/${sliceId}/favorites/`; + const apiCall = isStarred + ? SupersetClient.delete({ + endpoint, + }) + : SupersetClient.post({ endpoint }); + + apiCall .then(() => dispatch(toggleFaveStar(!isStarred))) .catch(() => { dispatch( diff --git a/superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx b/superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx index a4ad594387433..4f3b4d8dd05ee 100644 --- a/superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx +++ b/superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx @@ -91,7 +91,9 @@ jest.mock('lodash/debounce', () => ({ fetchMock.post('glob:*/api/v1/explore/form_data*', { key: KEY }); fetchMock.put('glob:*/api/v1/explore/form_data*', { key: KEY }); fetchMock.get('glob:*/api/v1/explore/form_data*', {}); -fetchMock.get('glob:*/favstar/slice*', { count: 0 }); +fetchMock.get('glob:*/api/v1/chart/favorite_status*', { + result: [{ value: true }], +}); const defaultPath = '/explore/'; const renderWithRouter = ({ diff --git a/superset-frontend/src/views/CRUD/hooks.ts b/superset-frontend/src/views/CRUD/hooks.ts index 80a6c4793bbed..1d46855dd1733 100644 --- a/superset-frontend/src/views/CRUD/hooks.ts +++ b/superset-frontend/src/views/CRUD/hooks.ts @@ -495,11 +495,6 @@ export function useImportResource( return { state, importResource }; } -enum FavStarClassName { - CHART = 'slice', - DASHBOARD = 'Dashboard', -} - type FavoriteStatusResponse = { result: Array<{ id: string; @@ -552,15 +547,17 @@ export function useFavoriteStatus( const saveFaveStar = useCallback( (id: number, isStarred: boolean) => { - const urlSuffix = isStarred ? 'unselect' : 'select'; - SupersetClient.get({ - endpoint: `/superset/favstar/${ - type === 'chart' ? FavStarClassName.CHART : FavStarClassName.DASHBOARD - }/${id}/${urlSuffix}/`, - }).then( - ({ json }) => { + const endpoint = `/api/v1/${type}/${id}/favorites/`; + const apiCall = isStarred + ? SupersetClient.delete({ + endpoint, + }) + : SupersetClient.post({ endpoint }); + + apiCall.then( + () => { updateFavoriteStatus({ - [id]: (json as { count: number })?.count > 0, + [id]: !isStarred, }); }, createErrorHandler(errMsg => diff --git a/superset/charts/api.py b/superset/charts/api.py index 7dc6d5e1e8d93..e21b19d64d0b0 100644 --- a/superset/charts/api.py +++ b/superset/charts/api.py @@ -14,6 +14,7 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +# pylint: disable=too-many-lines import json import logging from datetime import datetime @@ -110,6 +111,8 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]: "bulk_delete", # not using RouteMethod since locally defined "viz_types", "favorite_status", + "add_favorite", + "remove_favorite", "thumbnail", "screenshot", "cache_screenshot", @@ -838,6 +841,94 @@ def favorite_status(self, **kwargs: Any) -> Response: ] return self.response(200, result=res) + @expose("//favorites/", methods=["POST"]) + @protect() + @safe + @statsd_metrics + @event_logger.log_this_with_context( + action=lambda self, *args, **kwargs: f"{self.__class__.__name__}" + f".add_favorite", + log_to_statsd=False, + ) + def add_favorite(self, pk: int) -> Response: + """Marks the chart as favorite + --- + post: + description: >- + Marks the chart as favorite for the current user + parameters: + - in: path + schema: + type: integer + name: pk + responses: + 200: + description: Chart added to favorites + content: + application/json: + schema: + type: object + properties: + result: + type: object + 401: + $ref: '#/components/responses/401' + 404: + $ref: '#/components/responses/404' + 500: + $ref: '#/components/responses/500' + """ + chart = ChartDAO.find_by_id(pk) + if not chart: + return self.response_404() + + ChartDAO.add_favorite(chart) + return self.response(200, result="OK") + + @expose("//favorites/", methods=["DELETE"]) + @protect() + @safe + @statsd_metrics + @event_logger.log_this_with_context( + action=lambda self, *args, **kwargs: f"{self.__class__.__name__}" + f".remove_favorite", + log_to_statsd=False, + ) + def remove_favorite(self, pk: int) -> Response: + """Remove the chart from the user favorite list + --- + delete: + description: >- + Remove the chart from the user favorite list + parameters: + - in: path + schema: + type: integer + name: pk + responses: + 200: + description: Chart removed from favorites + content: + application/json: + schema: + type: object + properties: + result: + type: object + 401: + $ref: '#/components/responses/401' + 404: + $ref: '#/components/responses/404' + 500: + $ref: '#/components/responses/500' + """ + chart = ChartDAO.find_by_id(pk) + if not chart: + return self.response_404() + + ChartDAO.remove_favorite(chart) + return self.response(200, result="OK") + @expose("/import/", methods=["POST"]) @protect() @statsd_metrics diff --git a/superset/charts/dao.py b/superset/charts/dao.py index 384bd9a1fe6e2..7102e6ad234bf 100644 --- a/superset/charts/dao.py +++ b/superset/charts/dao.py @@ -16,6 +16,7 @@ # under the License. # pylint: disable=arguments-renamed import logging +from datetime import datetime from typing import List, Optional, TYPE_CHECKING from sqlalchemy.exc import SQLAlchemyError @@ -82,3 +83,32 @@ def favorited_ids(charts: List[Slice]) -> List[FavStar]: ) .all() ] + + @staticmethod + def add_favorite(chart: Slice) -> None: + ids = ChartDAO.favorited_ids([chart]) + if chart.id not in ids: + db.session.add( + FavStar( + class_name=FavStarClassName.CHART, + obj_id=chart.id, + user_id=get_user_id(), + dttm=datetime.now(), + ) + ) + db.session.commit() + + @staticmethod + def remove_favorite(chart: Slice) -> None: + fav = ( + db.session.query(FavStar) + .filter( + FavStar.class_name == FavStarClassName.CHART, + FavStar.obj_id == chart.id, + FavStar.user_id == get_user_id(), + ) + .one_or_none() + ) + if fav: + db.session.delete(fav) + db.session.commit() diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py index 64ea637c663d8..b6d0694745c66 100644 --- a/superset/dashboards/api.py +++ b/superset/dashboards/api.py @@ -140,6 +140,8 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]: RouteMethod.RELATED, "bulk_delete", # not using RouteMethod since locally defined "favorite_status", + "add_favorite", + "remove_favorite", "get_charts", "get_datasets", "get_embedded", @@ -981,6 +983,94 @@ def favorite_status(self, **kwargs: Any) -> Response: ] return self.response(200, result=res) + @expose("//favorites/", methods=["POST"]) + @protect() + @safe + @statsd_metrics + @event_logger.log_this_with_context( + action=lambda self, *args, **kwargs: f"{self.__class__.__name__}" + f".add_favorite", + log_to_statsd=False, + ) + def add_favorite(self, pk: int) -> Response: + """Marks the dashboard as favorite + --- + post: + description: >- + Marks the dashboard as favorite for the current user + parameters: + - in: path + schema: + type: integer + name: pk + responses: + 200: + description: Dashboard added to favorites + content: + application/json: + schema: + type: object + properties: + result: + type: object + 401: + $ref: '#/components/responses/401' + 404: + $ref: '#/components/responses/404' + 500: + $ref: '#/components/responses/500' + """ + dashboard = DashboardDAO.find_by_id(pk) + if not dashboard: + return self.response_404() + + DashboardDAO.add_favorite(dashboard) + return self.response(200, result="OK") + + @expose("//favorites/", methods=["DELETE"]) + @protect() + @safe + @statsd_metrics + @event_logger.log_this_with_context( + action=lambda self, *args, **kwargs: f"{self.__class__.__name__}" + f".remove_favorite", + log_to_statsd=False, + ) + def remove_favorite(self, pk: int) -> Response: + """Remove the dashboard from the user favorite list + --- + delete: + description: >- + Remove the dashboard from the user favorite list + parameters: + - in: path + schema: + type: integer + name: pk + responses: + 200: + description: Dashboard removed from favorites + content: + application/json: + schema: + type: object + properties: + result: + type: object + 401: + $ref: '#/components/responses/401' + 404: + $ref: '#/components/responses/404' + 500: + $ref: '#/components/responses/500' + """ + dashboard = DashboardDAO.find_by_id(pk) + if not dashboard: + return self.response_404() + + DashboardDAO.remove_favorite(dashboard) + return self.response(200, result="OK") + @expose("/import/", methods=["POST"]) @protect() @statsd_metrics diff --git a/superset/dashboards/dao.py b/superset/dashboards/dao.py index 3f0666266f9c8..fae67bb4491dd 100644 --- a/superset/dashboards/dao.py +++ b/superset/dashboards/dao.py @@ -299,3 +299,32 @@ def favorited_ids(dashboards: List[Dashboard]) -> List[FavStar]: ) .all() ] + + @staticmethod + def add_favorite(dashboard: Dashboard) -> None: + ids = DashboardDAO.favorited_ids([dashboard]) + if dashboard.id not in ids: + db.session.add( + FavStar( + class_name=FavStarClassName.DASHBOARD, + obj_id=dashboard.id, + user_id=get_user_id(), + dttm=datetime.now(), + ) + ) + db.session.commit() + + @staticmethod + def remove_favorite(dashboard: Dashboard) -> None: + fav = ( + db.session.query(FavStar) + .filter( + FavStar.class_name == FavStarClassName.DASHBOARD, + FavStar.obj_id == dashboard.id, + FavStar.user_id == get_user_id(), + ) + .one_or_none() + ) + if fav: + db.session.delete(fav) + db.session.commit() diff --git a/superset/views/core.py b/superset/views/core.py index 7ee053e881c0e..feb02fd949bf6 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -1784,6 +1784,7 @@ def warm_up_cache( # pylint: disable=too-many-locals,no-self-use @has_access_api @event_logger.log_this @expose("/favstar////") + @deprecated() def favstar( # pylint: disable=no-self-use self, class_name: str, obj_id: int, action: str ) -> FlaskResponse: diff --git a/tests/integration_tests/charts/api_tests.py b/tests/integration_tests/charts/api_tests.py index 965a9c137ba87..f148d9a8f7ce4 100644 --- a/tests/integration_tests/charts/api_tests.py +++ b/tests/integration_tests/charts/api_tests.py @@ -1248,6 +1248,75 @@ def test_get_current_user_favorite_status(self): if res["id"] in users_favorite_ids: assert res["value"] + def test_add_favorite(self): + """ + Dataset API: Test add chart to favorites + """ + chart = Slice( + id=100, + datasource_id=1, + datasource_type="table", + datasource_name="tmp_perm_table", + slice_name="slice_name", + ) + db.session.add(chart) + db.session.commit() + + self.login(username="admin") + uri = f"api/v1/chart/favorite_status/?q={prison.dumps([chart.id])}" + rv = self.client.get(uri) + data = json.loads(rv.data.decode("utf-8")) + for res in data["result"]: + assert res["value"] is False + + uri = f"api/v1/chart/{chart.id}/favorites/" + self.client.post(uri) + + uri = f"api/v1/chart/favorite_status/?q={prison.dumps([chart.id])}" + rv = self.client.get(uri) + data = json.loads(rv.data.decode("utf-8")) + for res in data["result"]: + assert res["value"] is True + + db.session.delete(chart) + db.session.commit() + + def test_remove_favorite(self): + """ + Dataset API: Test remove chart from favorites + """ + chart = Slice( + id=100, + datasource_id=1, + datasource_type="table", + datasource_name="tmp_perm_table", + slice_name="slice_name", + ) + db.session.add(chart) + db.session.commit() + + self.login(username="admin") + uri = f"api/v1/chart/{chart.id}/favorites/" + self.client.post(uri) + + uri = f"api/v1/chart/favorite_status/?q={prison.dumps([chart.id])}" + rv = self.client.get(uri) + data = json.loads(rv.data.decode("utf-8")) + for res in data["result"]: + assert res["value"] is True + + uri = f"api/v1/chart/{chart.id}/favorites/" + self.client.delete(uri) + + uri = f"api/v1/chart/favorite_status/?q={prison.dumps([chart.id])}" + rv = self.client.get(uri) + data = json.loads(rv.data.decode("utf-8")) + for res in data["result"]: + assert res["value"] is False + + db.session.delete(chart) + db.session.commit() + def test_get_time_range(self): """ Chart API: Test get actually time range from human readable string diff --git a/tests/integration_tests/dashboards/api_tests.py b/tests/integration_tests/dashboards/api_tests.py index 725811ce5f68d..9305fdcaa3066 100644 --- a/tests/integration_tests/dashboards/api_tests.py +++ b/tests/integration_tests/dashboards/api_tests.py @@ -683,6 +683,75 @@ def test_get_current_user_favorite_status(self): if res["id"] in users_favorite_ids: assert res["value"] + def test_add_favorite(self): + """ + Dataset API: Test add dashboard to favorites + """ + dashboard = Dashboard( + id=100, + dashboard_title="test_dashboard", + slug="test_slug", + slices=[], + published=True, + ) + db.session.add(dashboard) + db.session.commit() + + self.login(username="admin") + uri = f"api/v1/dashboard/favorite_status/?q={prison.dumps([dashboard.id])}" + rv = self.client.get(uri) + data = json.loads(rv.data.decode("utf-8")) + for res in data["result"]: + assert res["value"] is False + + uri = f"api/v1/dashboard/{dashboard.id}/favorites/" + self.client.post(uri) + + uri = f"api/v1/dashboard/favorite_status/?q={prison.dumps([dashboard.id])}" + rv = self.client.get(uri) + data = json.loads(rv.data.decode("utf-8")) + for res in data["result"]: + assert res["value"] is True + + db.session.delete(dashboard) + db.session.commit() + + def test_remove_favorite(self): + """ + Dataset API: Test remove dashboard from favorites + """ + dashboard = Dashboard( + id=100, + dashboard_title="test_dashboard", + slug="test_slug", + slices=[], + published=True, + ) + db.session.add(dashboard) + db.session.commit() + + self.login(username="admin") + uri = f"api/v1/dashboard/{dashboard.id}/favorites/" + self.client.post(uri) + + uri = f"api/v1/dashboard/favorite_status/?q={prison.dumps([dashboard.id])}" + rv = self.client.get(uri) + data = json.loads(rv.data.decode("utf-8")) + for res in data["result"]: + assert res["value"] is True + + uri = f"api/v1/dashboard/{dashboard.id}/favorites/" + self.client.delete(uri) + + uri = f"api/v1/dashboard/favorite_status/?q={prison.dumps([dashboard.id])}" + rv = self.client.get(uri) + data = json.loads(rv.data.decode("utf-8")) + for res in data["result"]: + assert res["value"] is False + + db.session.delete(dashboard) + db.session.commit() + @pytest.mark.usefixtures("create_dashboards") def test_get_dashboards_not_favorite_filter(self): """ diff --git a/tests/unit_tests/charts/dao/dao_tests.py b/tests/unit_tests/charts/dao/dao_tests.py index 15310712a5f8a..faec8694db696 100644 --- a/tests/unit_tests/charts/dao/dao_tests.py +++ b/tests/unit_tests/charts/dao/dao_tests.py @@ -65,3 +65,36 @@ def test_datasource_find_by_id_skip_base_filter_not_found( 125326326, session=session_with_data, skip_base_filter=True ) assert result is None + + +def test_add_favorite(session_with_data: Session) -> None: + from superset.charts.dao import ChartDAO + + chart = ChartDAO.find_by_id(1, session=session_with_data, skip_base_filter=True) + if not chart: + return + assert len(ChartDAO.favorited_ids([chart])) == 0 + + ChartDAO.add_favorite(chart) + assert len(ChartDAO.favorited_ids([chart])) == 1 + + ChartDAO.add_favorite(chart) + assert len(ChartDAO.favorited_ids([chart])) == 1 + + +def test_remove_favorite(session_with_data: Session) -> None: + from superset.charts.dao import ChartDAO + + chart = ChartDAO.find_by_id(1, session=session_with_data, skip_base_filter=True) + if not chart: + return + assert len(ChartDAO.favorited_ids([chart])) == 0 + + ChartDAO.add_favorite(chart) + assert len(ChartDAO.favorited_ids([chart])) == 1 + + ChartDAO.remove_favorite(chart) + assert len(ChartDAO.favorited_ids([chart])) == 0 + + ChartDAO.remove_favorite(chart) + assert len(ChartDAO.favorited_ids([chart])) == 0 diff --git a/tests/unit_tests/dashboards/dao_tests.py b/tests/unit_tests/dashboards/dao_tests.py new file mode 100644 index 0000000000000..a8f93e7513906 --- /dev/null +++ b/tests/unit_tests/dashboards/dao_tests.py @@ -0,0 +1,79 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from typing import Iterator + +import pytest +from sqlalchemy.orm.session import Session + + +@pytest.fixture +def session_with_data(session: Session) -> Iterator[Session]: + from superset.models.dashboard import Dashboard + + engine = session.get_bind() + Dashboard.metadata.create_all(engine) # pylint: disable=no-member + + dashboard_obj = Dashboard( + id=100, + dashboard_title="test_dashboard", + slug="test_slug", + slices=[], + published=True, + ) + + session.add(dashboard_obj) + session.commit() + yield session + session.rollback() + + +def test_add_favorite(session_with_data: Session) -> None: + from superset.dashboards.dao import DashboardDAO + + dashboard = DashboardDAO.find_by_id( + 100, session=session_with_data, skip_base_filter=True + ) + if not dashboard: + return + assert len(DashboardDAO.favorited_ids([dashboard])) == 0 + + DashboardDAO.add_favorite(dashboard) + assert len(DashboardDAO.favorited_ids([dashboard])) == 1 + + DashboardDAO.add_favorite(dashboard) + assert len(DashboardDAO.favorited_ids([dashboard])) == 1 + + +def test_remove_favorite(session_with_data: Session) -> None: + from superset.dashboards.dao import DashboardDAO + + dashboard = DashboardDAO.find_by_id( + 100, session=session_with_data, skip_base_filter=True + ) + if not dashboard: + return + assert len(DashboardDAO.favorited_ids([dashboard])) == 0 + + DashboardDAO.add_favorite(dashboard) + assert len(DashboardDAO.favorited_ids([dashboard])) == 1 + + DashboardDAO.remove_favorite(dashboard) + assert len(DashboardDAO.favorited_ids([dashboard])) == 0 + + DashboardDAO.remove_favorite(dashboard) + assert len(DashboardDAO.favorited_ids([dashboard])) == 0 From cad33b75c3037ad8f4c4c88140e790ea6590b2f1 Mon Sep 17 00:00:00 2001 From: Diego Medina Date: Thu, 23 Feb 2023 00:04:30 -0300 Subject: [PATCH 2/2] Improvements --- superset/constants.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/superset/constants.py b/superset/constants.py index 07bbe6c07cea5..7a1187f0569b2 100644 --- a/superset/constants.py +++ b/superset/constants.py @@ -126,6 +126,8 @@ class RouteMethod: # pylint: disable=too-few-public-methods "test_connection": "read", "validate_parameters": "read", "favorite_status": "read", + "add_favorite": "read", + "remove_favorite": "read", "thumbnail": "read", "import_": "write", "refresh": "write",