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

chore: Migrate /superset/favstar to API v1 #23165

Merged
merged 4 commits into from
Mar 29, 2023
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 @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
4 changes: 2 additions & 2 deletions superset-frontend/src/components/FaveStar/FaveStar.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -92,5 +92,5 @@ test('Call fetchFaveStar only on the first render', async () => {
expect(props.fetchFaveStar).toBeCalledWith(props.itemId);

rerender(<FaveStar {...{ ...props, itemId: 2 }} />);
expect(props.fetchFaveStar).toBeCalledTimes(1);
expect(props.fetchFaveStar).toBeCalledTimes(2);
});
12 changes: 5 additions & 7 deletions superset-frontend/src/components/FaveStar/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
* under the License.
*/

import React, { useCallback } from 'react';
import { css, t, styled, useComponentDidMount } from '@superset-ui/core';
import React, { useCallback, useEffect } from 'react';
import { css, t, styled } from '@superset-ui/core';
import { Tooltip } from 'src/components/Tooltip';
import Icons from 'src/components/Icons';

Expand All @@ -45,11 +45,9 @@ const FaveStar = ({
saveFaveStar,
fetchFaveStar,
}: FaveStarProps) => {
useComponentDidMount(() => {
if (fetchFaveStar) {
fetchFaveStar(itemId);
}
});
useEffect(() => {
fetchFaveStar?.(itemId);
}, [fetchFaveStar, itemId]);

const onClick = useCallback(
(e: React.MouseEvent) => {
Expand Down
18 changes: 11 additions & 7 deletions superset-frontend/src/dashboard/actions/dashboardState.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/
/* eslint camelcase: 0 */
import { ActionCreators as UndoActionCreators } from 'redux-undo';
import rison from 'rison';
import {
ensureIsArray,
t,
Expand Down Expand Up @@ -82,7 +83,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 };
Expand All @@ -92,10 +92,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(
Expand All @@ -112,10 +112,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));
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ export const DashboardPage: FC<PageProps> = ({ idOrSlug }: PageProps) => {
}, [addDangerToast, datasets, datasetsApiError, dispatch]);

if (error) throw error; // caught in error boundary
if (!readyToRender) return <Loading />;
if (!readyToRender || !isDashboardHydrated.current) return <Loading />;

return (
<>
Expand Down
21 changes: 11 additions & 10 deletions superset-frontend/src/explore/actions/exploreActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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,
Expand Down Expand Up @@ -66,22 +65,24 @@ 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));
});
};
}

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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ({
Expand Down
23 changes: 10 additions & 13 deletions superset-frontend/src/views/CRUD/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -542,11 +542,6 @@ export function useImportResource(
return { state, importResource };
}

enum FavStarClassName {
CHART = 'slice',
DASHBOARD = 'Dashboard',
}

type FavoriteStatusResponse = {
result: Array<{
id: string;
Expand Down Expand Up @@ -599,15 +594,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 =>
Expand Down
91 changes: 91 additions & 0 deletions superset/charts/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -111,6 +112,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",
Expand Down Expand Up @@ -848,6 +851,94 @@ def favorite_status(self, **kwargs: Any) -> Response:
]
return self.response(200, result=res)

@expose("/<pk>/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("/<pk>/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
Expand Down
30 changes: 30 additions & 0 deletions superset/charts/dao.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
2 changes: 2 additions & 0 deletions superset/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,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",
Expand Down
Loading