Skip to content

Commit

Permalink
chore: Migrate /superset/favstar to API v1
Browse files Browse the repository at this point in the history
  • Loading branch information
diegomedina248 committed Feb 23, 2023
1 parent d4362a3 commit f728b01
Show file tree
Hide file tree
Showing 19 changed files with 841 additions and 65 deletions.
316 changes: 296 additions & 20 deletions docs/static/resources/openapi.json

Large diffs are not rendered by default.

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);
});
11 changes: 4 additions & 7 deletions superset-frontend/src/components/FaveStar/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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) => {
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 @@ -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 };
Expand All @@ -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(
Expand All @@ -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));
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,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 @@ -495,11 +495,6 @@ export function useImportResource(
return { state, importResource };
}

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

type FavoriteStatusResponse = {
result: Array<{
id: string;
Expand Down Expand Up @@ -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 =>
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 @@ -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",
Expand Down Expand Up @@ -838,6 +841,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()
Loading

0 comments on commit f728b01

Please sign in to comment.