From 785f26ecb37d2cafb1f51ec7d7ed5658e70e8f1c Mon Sep 17 00:00:00 2001 From: "Michael S. molina" Date: Fri, 31 Mar 2023 16:26:32 -0300 Subject: [PATCH 1/7] feat: Shows user charts by default when editing a dashboard --- .../src/dashboard/actions/sliceEntities.js | 173 ----------------- .../dashboard/actions/sliceEntities.test.js | 102 ---------- .../src/dashboard/actions/sliceEntities.ts | 178 ++++++++++++++++++ .../src/dashboard/components/SliceAdder.jsx | 108 +++++++++-- .../src/dashboard/containers/SliceAdder.jsx | 14 +- .../src/dashboard/reducers/sliceEntities.js | 12 +- .../dashboard/reducers/sliceEntities.test.js | 4 +- superset-frontend/src/dashboard/types.ts | 21 +++ 8 files changed, 306 insertions(+), 306 deletions(-) delete mode 100644 superset-frontend/src/dashboard/actions/sliceEntities.js delete mode 100644 superset-frontend/src/dashboard/actions/sliceEntities.test.js create mode 100644 superset-frontend/src/dashboard/actions/sliceEntities.ts diff --git a/superset-frontend/src/dashboard/actions/sliceEntities.js b/superset-frontend/src/dashboard/actions/sliceEntities.js deleted file mode 100644 index 99a0627211180..0000000000000 --- a/superset-frontend/src/dashboard/actions/sliceEntities.js +++ /dev/null @@ -1,173 +0,0 @@ -/** - * 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. - */ -/* eslint camelcase: 0 */ -import { FeatureFlag, SupersetClient, t } from '@superset-ui/core'; -import rison from 'rison'; - -import { addDangerToast } from 'src/components/MessageToasts/actions'; -import { getClientErrorObject } from 'src/utils/getClientErrorObject'; -import { isFeatureEnabled } from 'src/featureFlags'; - -export const SET_ALL_SLICES = 'SET_ALL_SLICES'; -const FETCH_SLICES_PAGE_SIZE = 200; - -export function getDatasourceParameter(datasourceId, datasourceType) { - return `${datasourceId}__${datasourceType}`; -} - -export function setAllSlices(slices) { - return { type: SET_ALL_SLICES, payload: { slices } }; -} - -export const FETCH_ALL_SLICES_STARTED = 'FETCH_ALL_SLICES_STARTED'; -export function fetchAllSlicesStarted() { - return { type: FETCH_ALL_SLICES_STARTED }; -} - -export const FETCH_ALL_SLICES_FAILED = 'FETCH_ALL_SLICES_FAILED'; -export function fetchAllSlicesFailed(error) { - return { type: FETCH_ALL_SLICES_FAILED, payload: { error } }; -} - -export function fetchSlices( - userId, - dispatch, - filter_value, - sortColumn = 'changed_on', - slices = {}, -) { - const additional_filters = filter_value - ? [{ col: 'slice_name', opr: 'chart_all_text', value: filter_value }] - : []; - - if (isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS)) { - additional_filters.push({ - col: 'viz_type', - opr: 'neq', - value: 'filter_box', - }); - } - - const cloneSlices = { ...slices }; - - return SupersetClient.get({ - endpoint: `/api/v1/chart/?q=${rison.encode({ - columns: [ - 'changed_on_delta_humanized', - 'changed_on_utc', - 'datasource_id', - 'datasource_type', - 'datasource_url', - 'datasource_name_text', - 'description_markeddown', - 'description', - 'id', - 'params', - 'slice_name', - 'thumbnail_url', - 'url', - 'viz_type', - ], - filters: [...additional_filters], - page_size: FETCH_SLICES_PAGE_SIZE, - order_column: - sortColumn === 'changed_on' ? 'changed_on_delta_humanized' : sortColumn, - order_direction: sortColumn === 'changed_on' ? 'desc' : 'asc', - })}`, - }) - .then(({ json }) => { - const { result } = json; - result.forEach(slice => { - let form_data = JSON.parse(slice.params); - form_data = { - ...form_data, - // force using datasource stored in relational table prop - datasource: - getDatasourceParameter( - slice.datasource_id, - slice.datasource_type, - ) || form_data.datasource, - }; - cloneSlices[slice.id] = { - slice_id: slice.id, - slice_url: slice.url, - slice_name: slice.slice_name, - form_data, - datasource_name: slice.datasource_name_text, - datasource_url: slice.datasource_url, - datasource_id: slice.datasource_id, - datasource_type: slice.datasource_type, - changed_on: new Date(slice.changed_on_utc).getTime(), - description: slice.description, - description_markdown: slice.description_markeddown, - viz_type: slice.viz_type, - modified: slice.changed_on_delta_humanized, - changed_on_humanized: slice.changed_on_delta_humanized, - thumbnail_url: slice.thumbnail_url, - }; - }); - - return dispatch(setAllSlices(cloneSlices)); - }) - .catch(errorResponse => - getClientErrorObject(errorResponse).then(({ error }) => { - dispatch( - fetchAllSlicesFailed(error || t('Could not fetch all saved charts')), - ); - dispatch( - addDangerToast( - t('Sorry there was an error fetching saved charts: ') + error, - ), - ); - }), - ); -} - -export function fetchAllSlices(userId) { - return (dispatch, getState) => { - const { sliceEntities } = getState(); - if (sliceEntities.lastUpdated === 0) { - dispatch(fetchAllSlicesStarted()); - return fetchSlices(userId, dispatch, undefined); - } - - return dispatch(setAllSlices(sliceEntities.slices)); - }; -} - -export function fetchSortedSlices(userId, order_column) { - return dispatch => { - dispatch(fetchAllSlicesStarted()); - return fetchSlices(userId, dispatch, undefined, order_column); - }; -} - -export function fetchFilteredSlices(userId, filter_value) { - return (dispatch, getState) => { - dispatch(fetchAllSlicesStarted()); - const { sliceEntities } = getState(); - return fetchSlices( - userId, - dispatch, - filter_value, - undefined, - sliceEntities.slices, - ); - }; -} diff --git a/superset-frontend/src/dashboard/actions/sliceEntities.test.js b/superset-frontend/src/dashboard/actions/sliceEntities.test.js deleted file mode 100644 index 1f3d0cb440bed..0000000000000 --- a/superset-frontend/src/dashboard/actions/sliceEntities.test.js +++ /dev/null @@ -1,102 +0,0 @@ -/** - * 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. - */ -import sinon from 'sinon'; -import { SupersetClient } from '@superset-ui/core'; - -import { - FETCH_ALL_SLICES_STARTED, - fetchSortedSlices, - fetchFilteredSlices, - fetchAllSlices, -} from './sliceEntities'; - -describe('slice entity actions', () => { - const mockState = { - sliceEntities: { slices: {} }, - isLoading: true, - errorMessage: null, - lastUpdated: 0, - }; - - function setup(stateOverrides) { - const state = { ...mockState, ...stateOverrides }; - const getState = sinon.spy(() => state); - const dispatch = sinon.spy(); - - return { getState, dispatch, state }; - } - - let spy; - - beforeEach(() => { - spy = sinon.spy(SupersetClient); - }); - - afterEach(() => { - sinon.restore(); - }); - - describe('fetchSortedSlices', () => { - it('should dispatch an fetchAllSlicesStarted action', async () => { - const { dispatch } = setup(); - const thunk1 = fetchSortedSlices('userId', false, 'orderColumn'); - await thunk1(dispatch); - expect(dispatch.getCall(0).args[0]).toEqual({ - type: FETCH_ALL_SLICES_STARTED, - }); - expect(spy.get.callCount).toBe(1); - }); - }); - - describe('fetchFilteredSlices', () => { - it('should dispatch an fetchAllSlicesStarted action', async () => { - const { dispatch, getState } = setup(); - const thunk1 = fetchFilteredSlices('userId', 'filter_value'); - await thunk1(dispatch, getState); - expect(dispatch.getCall(0).args[0]).toEqual({ - type: FETCH_ALL_SLICES_STARTED, - }); - expect(spy.get.callCount).toBe(1); - }); - }); - - describe('fetchAllSlices', () => { - it('should not trigger fetchSlices when sliceEntities lastUpdate is not 0', async () => { - const { dispatch, getState } = setup({ - sliceEntities: { slices: {}, lastUpdated: 1 }, - }); - - const thunk1 = fetchAllSlices('userId', 'filter_value'); - await thunk1(dispatch, getState); - - expect(spy.get.callCount).toBe(0); - }); - - it('should trigger fetchSlices when sliceEntities lastUpdate is 0', async () => { - const { dispatch, getState } = setup({ - sliceEntities: { slices: {}, lastUpdated: 0 }, - }); - - const thunk1 = fetchAllSlices('userId', false, 'filter_value'); - await thunk1(dispatch, getState); - - expect(spy.get.callCount).toBe(1); - }); - }); -}); diff --git a/superset-frontend/src/dashboard/actions/sliceEntities.ts b/superset-frontend/src/dashboard/actions/sliceEntities.ts new file mode 100644 index 0000000000000..18a7a55561011 --- /dev/null +++ b/superset-frontend/src/dashboard/actions/sliceEntities.ts @@ -0,0 +1,178 @@ +/** + * 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. + */ +import rison from 'rison'; +import { + DatasourceType, + FeatureFlag, + SupersetClient, + t, +} from '@superset-ui/core'; +import { addDangerToast } from 'src/components/MessageToasts/actions'; +import { getClientErrorObject } from 'src/utils/getClientErrorObject'; +import { isFeatureEnabled } from 'src/featureFlags'; +import { Dispatch } from 'redux'; +import { Slice } from '../types'; + +const FETCH_SLICES_PAGE_SIZE = 200; + +export function getDatasourceParameter( + datasourceId: number, + datasourceType: DatasourceType, +) { + return `${datasourceId}__${datasourceType}`; +} + +export const ADD_SLICES = 'ADD_SLICES'; +function addSlices(slices: { [id: number]: Slice }) { + return { type: ADD_SLICES, payload: { slices } }; +} + +export const SET_SLICES = 'SET_SLICES'; +function setSlices(slices: { [id: number]: Slice }) { + return { type: SET_SLICES, payload: { slices } }; +} + +export const FETCH_ALL_SLICES_STARTED = 'FETCH_ALL_SLICES_STARTED'; +function fetchAllSlicesStarted() { + return { type: FETCH_ALL_SLICES_STARTED }; +} + +export const FETCH_ALL_SLICES_FAILED = 'FETCH_ALL_SLICES_FAILED'; +function fetchAllSlicesFailed(error: string) { + return { type: FETCH_ALL_SLICES_FAILED, payload: { error } }; +} + +const parseResult = (result: any[]) => { + const slices: { [id: number]: Slice } = {}; + result.forEach((slice: any) => { + let form_data = JSON.parse(slice.params); + form_data = { + ...form_data, + // force using datasource stored in relational table prop + datasource: + getDatasourceParameter(slice.datasource_id, slice.datasource_type) || + form_data.datasource, + }; + slices[slice.id] = { + slice_id: slice.id, + slice_url: slice.url, + slice_name: slice.slice_name, + form_data, + datasource_name: slice.datasource_name_text, + datasource_url: slice.datasource_url, + datasource_id: slice.datasource_id, + datasource_type: slice.datasource_type, + changed_on: new Date(slice.changed_on_utc).getTime(), + description: slice.description, + description_markdown: slice.description_markeddown, + viz_type: slice.viz_type, + modified: slice.changed_on_delta_humanized, + changed_on_humanized: slice.changed_on_delta_humanized, + thumbnail_url: slice.thumbnail_url, + owners: slice.owners, + created_by: slice.created_by, + }; + }); + return slices; +}; + +export function updateSlices(slices: { [id: number]: Slice }) { + return (dispatch: Dispatch) => { + dispatch(setSlices(slices)); + }; +} + +export function fetchSlices( + userId?: number, + filter_value?: string, + sortColumn = 'changed_on', +) { + return (dispatch: Dispatch) => { + dispatch(fetchAllSlicesStarted()); + + const filters: { + col: string; + opr: string; + value: string | number; + }[] = filter_value + ? [{ col: 'slice_name', opr: 'chart_all_text', value: filter_value }] + : []; + + if (isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS)) { + filters.push({ + col: 'viz_type', + opr: 'neq', + value: 'filter_box', + }); + } + + if (userId) { + filters.push({ col: 'owners', opr: 'rel_m_m', value: userId }); + } + + return SupersetClient.get({ + endpoint: `/api/v1/chart/?q=${rison.encode({ + columns: [ + 'changed_on_delta_humanized', + 'changed_on_utc', + 'datasource_id', + 'datasource_type', + 'datasource_url', + 'datasource_name_text', + 'description_markeddown', + 'description', + 'id', + 'params', + 'slice_name', + 'thumbnail_url', + 'url', + 'viz_type', + 'owners.id', + 'created_by.id', + ], + filters, + page_size: FETCH_SLICES_PAGE_SIZE, + order_column: + sortColumn === 'changed_on' + ? 'changed_on_delta_humanized' + : sortColumn, + order_direction: sortColumn === 'changed_on' ? 'desc' : 'asc', + })}`, + }) + .then(({ json }) => { + const { result } = json; + const slices = parseResult(result); + return dispatch(addSlices(slices)); + }) + .catch(errorResponse => + getClientErrorObject(errorResponse).then(({ error }) => { + dispatch( + fetchAllSlicesFailed( + error || t('Could not fetch all saved charts'), + ), + ); + dispatch( + addDangerToast( + t('Sorry there was an error fetching saved charts: ') + error, + ), + ); + }), + ); + }; +} diff --git a/superset-frontend/src/dashboard/components/SliceAdder.jsx b/superset-frontend/src/dashboard/components/SliceAdder.jsx index 95f6c930ab358..25dd50f431eae 100644 --- a/superset-frontend/src/dashboard/components/SliceAdder.jsx +++ b/superset-frontend/src/dashboard/components/SliceAdder.jsx @@ -37,18 +37,20 @@ import { NEW_COMPONENTS_SOURCE_ID, } from 'src/dashboard/util/constants'; import { slicePropShape } from 'src/dashboard/util/propShapes'; -import _ from 'lodash'; +import { debounce, pickBy } from 'lodash'; +import Checkbox from 'src/components/Checkbox'; import AddSliceCard from './AddSliceCard'; import AddSliceDragPreview from './dnd/AddSliceDragPreview'; import DragDroppable from './dnd/DragDroppable'; const propTypes = { - fetchAllSlices: PropTypes.func.isRequired, + fetchSlices: PropTypes.func.isRequired, + updateSlices: PropTypes.func.isRequired, isLoading: PropTypes.bool.isRequired, slices: PropTypes.objectOf(slicePropShape).isRequired, lastUpdated: PropTypes.number.isRequired, errorMessage: PropTypes.string, - userId: PropTypes.string.isRequired, + userId: PropTypes.number.isRequired, selectedSliceIds: PropTypes.arrayOf(PropTypes.number), editMode: PropTypes.bool, dashboardId: PropTypes.number, @@ -133,23 +135,33 @@ class SliceAdder extends React.Component { searchTerm: '', sortBy: DEFAULT_SORT_KEY, selectedSliceIdsSet: new Set(props.selectedSliceIds), + showAccessibleCharts: false, }; this.rowRenderer = this.rowRenderer.bind(this); this.searchUpdated = this.searchUpdated.bind(this); this.handleKeyPress = this.handleKeyPress.bind(this); this.handleSelect = this.handleSelect.bind(this); + this.userIdForFetch = this.userIdForFetch.bind(this); + this.onAccessibleCharts = this.onAccessibleCharts.bind(this); + } + + userIdForFetch() { + return this.state.showAccessibleCharts ? undefined : this.props.userId; } componentDidMount() { - this.slicesRequest = this.props.fetchAllSlices(this.props.userId); + this.slicesRequest = this.props.fetchSlices(this.userIdForFetch()); } UNSAFE_componentWillReceiveProps(nextProps) { const nextState = {}; if (nextProps.lastUpdated !== this.props.lastUpdated) { - nextState.filteredSlices = Object.values(nextProps.slices) - .filter(createFilter(this.state.searchTerm, KEYS_TO_FILTERS)) - .sort(SliceAdder.sortByComparator(this.state.sortBy)); + nextState.filteredSlices = this.getFilteredSortedSlices( + nextProps.slices, + this.state.searchTerm, + this.state.sortBy, + this.state.showAccessibleCharts, + ); } if (nextProps.selectedSliceIds !== this.props.selectedSliceIds) { @@ -162,13 +174,25 @@ class SliceAdder extends React.Component { } componentWillUnmount() { + // Clears the redux store keeping only selected items + const selectedSlices = pickBy(this.props.slices, value => + this.state.selectedSliceIdsSet.has(value.slice_id), + ); + this.props.updateSlices(selectedSlices); if (this.slicesRequest && this.slicesRequest.abort) { this.slicesRequest.abort(); } } - getFilteredSortedSlices(searchTerm, sortBy) { - return Object.values(this.props.slices) + getFilteredSortedSlices(slices, searchTerm, sortBy, showAccessibleCharts) { + return Object.values(slices) + .filter(slice => + showAccessibleCharts + ? true + : (slice.owners && + slice.owners.find(owner => owner.id === this.props.userId)) || + (slice.created_by && slice.created_by.id === this.props.userId), + ) .filter(createFilter(searchTerm, KEYS_TO_FILTERS)) .sort(SliceAdder.sortByComparator(sortBy)); } @@ -181,19 +205,23 @@ class SliceAdder extends React.Component { } } - handleChange = _.debounce(value => { + handleChange = debounce(value => { this.searchUpdated(value); - - const { userId } = this.props; - this.slicesRequest = this.props.fetchFilteredSlices(userId, value); + this.slicesRequest = this.props.fetchSlices( + this.userIdForFetch(), + value, + this.state.sortBy, + ); }, 300); searchUpdated(searchTerm) { this.setState(prevState => ({ searchTerm, filteredSlices: this.getFilteredSortedSlices( + this.props.slices, searchTerm, prevState.sortBy, + prevState.showAccessibleCharts, ), })); } @@ -202,13 +230,17 @@ class SliceAdder extends React.Component { this.setState(prevState => ({ sortBy, filteredSlices: this.getFilteredSortedSlices( + this.props.slices, prevState.searchTerm, sortBy, + prevState.showAccessibleCharts, ), })); - - const { userId } = this.props; - this.slicesRequest = this.props.fetchSortedSlices(userId, sortBy); + this.slicesRequest = this.props.fetchSlices( + this.userIdForFetch(), + this.state.searchTerm, + sortBy, + ); } rowRenderer({ key, index, style }) { @@ -258,6 +290,25 @@ class SliceAdder extends React.Component { ); } + onAccessibleCharts(showAccessibleCharts) { + if (showAccessibleCharts) { + this.slicesRequest = this.props.fetchSlices( + undefined, + this.state.searchTerm, + this.state.sortBy, + ); + } + this.setState(prevState => ({ + showAccessibleCharts, + filteredSlices: this.getFilteredSortedSlices( + this.props.slices, + prevState.searchTerm, + prevState.sortBy, + showAccessibleCharts, + ), + })); + } + render() { return (
this.handleChange(ev.target.value)} onKeyPress={this.handleKeyPress} @@ -302,6 +357,25 @@ class SliceAdder extends React.Component { placeholder={t('Sort by')} /> +
css` + display: flex; + flex-direction: row; + justify-content: center; + padding: 0 ${theme.gridUnit * 3}px ${theme.gridUnit * 3}px + ${theme.gridUnit * 3}px; + + & > span { + margin-right: ${theme.gridUnit}px; + } + `} + > + + {t('Show other charts I have access to')} +
{this.props.isLoading && } {!this.props.isLoading && this.state.filteredSlices.length > 0 && ( diff --git a/superset-frontend/src/dashboard/containers/SliceAdder.jsx b/superset-frontend/src/dashboard/containers/SliceAdder.jsx index e3c8e1db87a2c..992098a0da2a4 100644 --- a/superset-frontend/src/dashboard/containers/SliceAdder.jsx +++ b/superset-frontend/src/dashboard/containers/SliceAdder.jsx @@ -18,12 +18,7 @@ */ import { bindActionCreators } from 'redux'; import { connect } from 'react-redux'; - -import { - fetchAllSlices, - fetchSortedSlices, - fetchFilteredSlices, -} from '../actions/sliceEntities'; +import { fetchSlices, updateSlices } from '../actions/sliceEntities'; import SliceAdder from '../components/SliceAdder'; function mapStateToProps( @@ -32,7 +27,7 @@ function mapStateToProps( ) { return { height: ownProps.height, - userId: dashboardInfo.userId, + userId: +dashboardInfo.userId, dashboardId: dashboardInfo.id, selectedSliceIds: dashboardState.sliceIds, slices: sliceEntities.slices, @@ -46,9 +41,8 @@ function mapStateToProps( function mapDispatchToProps(dispatch) { return bindActionCreators( { - fetchAllSlices, - fetchSortedSlices, - fetchFilteredSlices, + fetchSlices, + updateSlices, }, dispatch, ); diff --git a/superset-frontend/src/dashboard/reducers/sliceEntities.js b/superset-frontend/src/dashboard/reducers/sliceEntities.js index 70b66db72475d..1a065b11fab0b 100644 --- a/superset-frontend/src/dashboard/reducers/sliceEntities.js +++ b/superset-frontend/src/dashboard/reducers/sliceEntities.js @@ -21,7 +21,8 @@ import { t } from '@superset-ui/core'; import { FETCH_ALL_SLICES_FAILED, FETCH_ALL_SLICES_STARTED, - SET_ALL_SLICES, + ADD_SLICES, + SET_SLICES, } from '../actions/sliceEntities'; import { HYDRATE_DASHBOARD } from '../actions/hydrate'; @@ -48,7 +49,7 @@ export default function sliceEntitiesReducer( isLoading: true, }; }, - [SET_ALL_SLICES]() { + [ADD_SLICES]() { return { ...state, isLoading: false, @@ -56,6 +57,13 @@ export default function sliceEntitiesReducer( lastUpdated: new Date().getTime(), }; }, + [SET_SLICES]() { + return { + isLoading: false, + slices: { ...action.payload.slices }, + lastUpdated: new Date().getTime(), + }; + }, [FETCH_ALL_SLICES_FAILED]() { return { ...state, diff --git a/superset-frontend/src/dashboard/reducers/sliceEntities.test.js b/superset-frontend/src/dashboard/reducers/sliceEntities.test.js index 73b5aebee5c69..444535d2767b3 100644 --- a/superset-frontend/src/dashboard/reducers/sliceEntities.test.js +++ b/superset-frontend/src/dashboard/reducers/sliceEntities.test.js @@ -19,7 +19,7 @@ import { FETCH_ALL_SLICES_FAILED, FETCH_ALL_SLICES_STARTED, - SET_ALL_SLICES, + ADD_SLICES, } from 'src/dashboard/actions/sliceEntities'; import sliceEntitiesReducer from 'src/dashboard/reducers/sliceEntities'; @@ -41,7 +41,7 @@ describe('sliceEntities reducer', () => { it('should set slices', () => { const result = sliceEntitiesReducer( { slices: { a: {} } }, - { type: SET_ALL_SLICES, payload: { slices: { 1: {}, 2: {} } } }, + { type: ADD_SLICES, payload: { slices: { 1: {}, 2: {} } } }, ); expect(result.slices).toEqual({ diff --git a/superset-frontend/src/dashboard/types.ts b/superset-frontend/src/dashboard/types.ts index 56bccf900e84a..7345f0b1aff19 100644 --- a/superset-frontend/src/dashboard/types.ts +++ b/superset-frontend/src/dashboard/types.ts @@ -19,6 +19,7 @@ import { ChartProps, DataMaskStateWithId, + DatasourceType, ExtraFormData, GenericDataType, JsonObject, @@ -193,3 +194,23 @@ export type EmbeddedDashboard = { dashboard_id: string; allowed_domains: string[]; }; + +export type Slice = { + slice_id: number; + slice_name: string; + description: string; + description_markdown: string; + form_data: any; + slice_url: string; + viz_type: string; + thumbnail_url: string; + changed_on: number; + changed_on_humanized: string; + modified: string; + datasource_id: number; + datasource_type: DatasourceType; + datasource_url: string; + datasource_name: string; + owners: { id: number }[]; + created_by: { id: number }; +}; From 59f75c14b7fcf8e6ce9d998c6ab8757e010bfa7f Mon Sep 17 00:00:00 2001 From: "Michael S. molina" Date: Mon, 3 Apr 2023 10:33:41 -0300 Subject: [PATCH 2/7] Fixes tests --- .../integration/dashboard/editmode.test.ts | 4 +- .../src/dashboard/components/SliceAdder.jsx | 12 +--- .../dashboard/components/SliceAdder.test.jsx | 56 +++++++++---------- 3 files changed, 31 insertions(+), 41 deletions(-) diff --git a/superset-frontend/cypress-base/cypress/integration/dashboard/editmode.test.ts b/superset-frontend/cypress-base/cypress/integration/dashboard/editmode.test.ts index 4251b6a7ae502..6201a582aae85 100644 --- a/superset-frontend/cypress-base/cypress/integration/dashboard/editmode.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/dashboard/editmode.test.ts @@ -751,8 +751,8 @@ describe('Dashboard edit', () => { cy.getBySel('dashboard-component-chart-holder').should('have.length', 1); }); - it('should remove added charts', () => { - dragComponent('Pivot Table'); + it.only('should remove added charts', () => { + dragComponent('Unicode Cloud'); cy.getBySel('dashboard-component-chart-holder').should('have.length', 1); cy.getBySel('dashboard-delete-component-button').click(); cy.getBySel('dashboard-component-chart-holder').should('have.length', 0); diff --git a/superset-frontend/src/dashboard/components/SliceAdder.jsx b/superset-frontend/src/dashboard/components/SliceAdder.jsx index 25dd50f431eae..e282dd4f414fb 100644 --- a/superset-frontend/src/dashboard/components/SliceAdder.jsx +++ b/superset-frontend/src/dashboard/components/SliceAdder.jsx @@ -70,7 +70,7 @@ const KEYS_TO_SORT = { changed_on: t('recent'), }; -const DEFAULT_SORT_KEY = 'changed_on'; +export const DEFAULT_SORT_KEY = 'changed_on'; const DEFAULT_CELL_HEIGHT = 128; @@ -139,7 +139,6 @@ class SliceAdder extends React.Component { }; this.rowRenderer = this.rowRenderer.bind(this); this.searchUpdated = this.searchUpdated.bind(this); - this.handleKeyPress = this.handleKeyPress.bind(this); this.handleSelect = this.handleSelect.bind(this); this.userIdForFetch = this.userIdForFetch.bind(this); this.onAccessibleCharts = this.onAccessibleCharts.bind(this); @@ -197,14 +196,6 @@ class SliceAdder extends React.Component { .sort(SliceAdder.sortByComparator(sortBy)); } - handleKeyPress(ev) { - if (ev.key === 'Enter') { - ev.preventDefault(); - - this.searchUpdated(ev.target.value); - } - } - handleChange = debounce(value => { this.searchUpdated(value); this.slicesRequest = this.props.fetchSlices( @@ -343,7 +334,6 @@ class SliceAdder extends React.Component { } className="search-input" onChange={ev => this.handleChange(ev.target.value)} - onKeyPress={this.handleKeyPress} data-test="dashboard-charts-filter-search-input" /> fn => { + // eslint-disable-next-line no-param-reassign + fn.throttle = jest.fn(); + return fn; +}); + describe('SliceAdder', () => { - const mockEvent = { - key: 'Enter', - target: { - value: 'mock event target', - }, - preventDefault: () => {}, - }; const props = { ...mockSliceEntities, - fetchAllSlices: () => {}, - fetchSortedSlices: () => {}, + fetchSlices: jest.fn(), + updateSlices: jest.fn(), selectedSliceIds: [127, 128], - userId: '1', + userId: 1, }; const errorProps = { ...props, @@ -84,16 +86,16 @@ describe('SliceAdder', () => { it('componentDidMount', () => { sinon.spy(SliceAdder.prototype, 'componentDidMount'); - sinon.spy(props, 'fetchAllSlices'); + sinon.spy(props, 'fetchSlices'); shallow(, { lifecycleExperimental: true, }); expect(SliceAdder.prototype.componentDidMount.calledOnce).toBe(true); - expect(props.fetchAllSlices.calledOnce).toBe(true); + expect(props.fetchSlices.calledOnce).toBe(true); SliceAdder.prototype.componentDidMount.restore(); - props.fetchAllSlices.restore(); + props.fetchSlices.restore(); }); describe('UNSAFE_componentWillReceiveProps', () => { @@ -138,32 +140,30 @@ describe('SliceAdder', () => { let wrapper; let spy; beforeEach(() => { - wrapper = shallow(); + spy = props.fetchSlices; + wrapper = shallow(); wrapper.setState({ filteredSlices: Object.values(props.slices) }); - spy = sinon.spy(wrapper.instance(), 'getFilteredSortedSlices'); }); afterEach(() => { - spy.restore(); + spy.mockReset(); }); it('searchUpdated', () => { const newSearchTerm = 'new search term'; - wrapper.instance().searchUpdated(newSearchTerm); - expect(spy.calledOnce).toBe(true); - expect(spy.lastCall.args[0]).toBe(newSearchTerm); + wrapper.instance().handleChange(newSearchTerm); + expect(spy).toHaveBeenCalled(); + expect(spy).toHaveBeenCalledWith( + props.userId, + newSearchTerm, + DEFAULT_SORT_KEY, + ); }); it('handleSelect', () => { const newSortBy = 'viz_type'; wrapper.instance().handleSelect(newSortBy); - expect(spy.calledOnce).toBe(true); - expect(spy.lastCall.args[1]).toBe(newSortBy); - }); - - it('handleKeyPress', () => { - wrapper.instance().handleKeyPress(mockEvent); - expect(spy.calledOnce).toBe(true); - expect(spy.lastCall.args[0]).toBe(mockEvent.target.value); + expect(spy).toHaveBeenCalled(); + expect(spy).toHaveBeenCalledWith(props.userId, '', newSortBy); }); }); }); From 7b53fcc77e00ec375325f400f8c4328a3d153762 Mon Sep 17 00:00:00 2001 From: "Michael S. molina" Date: Mon, 3 Apr 2023 11:22:34 -0300 Subject: [PATCH 3/7] Addresses comments --- .../src/dashboard/components/SliceAdder.jsx | 81 ++++++++++++------- .../src/utils/localStorageHelpers.ts | 2 + 2 files changed, 54 insertions(+), 29 deletions(-) diff --git a/superset-frontend/src/dashboard/components/SliceAdder.jsx b/superset-frontend/src/dashboard/components/SliceAdder.jsx index e282dd4f414fb..150d63ffccab0 100644 --- a/superset-frontend/src/dashboard/components/SliceAdder.jsx +++ b/superset-frontend/src/dashboard/components/SliceAdder.jsx @@ -28,6 +28,11 @@ import { Select } from 'src/components'; import Loading from 'src/components/Loading'; import Button from 'src/components/Button'; import Icons from 'src/components/Icons'; +import { + LocalStorageKeys, + getItem, + setItem, +} from 'src/utils/localStorageHelpers'; import { CHART_TYPE, NEW_COMPONENT_SOURCE_TYPE, @@ -39,6 +44,7 @@ import { import { slicePropShape } from 'src/dashboard/util/propShapes'; import { debounce, pickBy } from 'lodash'; import Checkbox from 'src/components/Checkbox'; +import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls'; import AddSliceCard from './AddSliceCard'; import AddSliceDragPreview from './dnd/AddSliceDragPreview'; import DragDroppable from './dnd/DragDroppable'; @@ -75,10 +81,15 @@ export const DEFAULT_SORT_KEY = 'changed_on'; const DEFAULT_CELL_HEIGHT = 128; const Controls = styled.div` - display: flex; - flex-direction: row; - padding: ${({ theme }) => theme.gridUnit * 3}px; - padding-top: ${({ theme }) => theme.gridUnit * 4}px; + ${({ theme }) => ` + display: flex; + flex-direction: row; + padding: + ${theme.gridUnit * 4}px + ${theme.gridUnit * 3}px + ${theme.gridUnit * 4}px + ${theme.gridUnit * 3}px; + `} `; const StyledSelect = styled(Select)` @@ -135,17 +146,20 @@ class SliceAdder extends React.Component { searchTerm: '', sortBy: DEFAULT_SORT_KEY, selectedSliceIdsSet: new Set(props.selectedSliceIds), - showAccessibleCharts: false, + showOnlyMyCharts: getItem( + LocalStorageKeys.dashboard__editor_show_only_my_charts, + true, + ), }; this.rowRenderer = this.rowRenderer.bind(this); this.searchUpdated = this.searchUpdated.bind(this); this.handleSelect = this.handleSelect.bind(this); this.userIdForFetch = this.userIdForFetch.bind(this); - this.onAccessibleCharts = this.onAccessibleCharts.bind(this); + this.onShowOnlyMyCharts = this.onShowOnlyMyCharts.bind(this); } userIdForFetch() { - return this.state.showAccessibleCharts ? undefined : this.props.userId; + return this.state.showOnlyMyCharts ? this.props.userId : undefined; } componentDidMount() { @@ -159,7 +173,7 @@ class SliceAdder extends React.Component { nextProps.slices, this.state.searchTerm, this.state.sortBy, - this.state.showAccessibleCharts, + this.state.showOnlyMyCharts, ); } @@ -183,14 +197,14 @@ class SliceAdder extends React.Component { } } - getFilteredSortedSlices(slices, searchTerm, sortBy, showAccessibleCharts) { + getFilteredSortedSlices(slices, searchTerm, sortBy, showOnlyMyCharts) { return Object.values(slices) .filter(slice => - showAccessibleCharts - ? true - : (slice.owners && + showOnlyMyCharts + ? (slice.owners && slice.owners.find(owner => owner.id === this.props.userId)) || - (slice.created_by && slice.created_by.id === this.props.userId), + (slice.created_by && slice.created_by.id === this.props.userId) + : true, ) .filter(createFilter(searchTerm, KEYS_TO_FILTERS)) .sort(SliceAdder.sortByComparator(sortBy)); @@ -212,7 +226,7 @@ class SliceAdder extends React.Component { this.props.slices, searchTerm, prevState.sortBy, - prevState.showAccessibleCharts, + prevState.showOnlyMyCharts, ), })); } @@ -224,7 +238,7 @@ class SliceAdder extends React.Component { this.props.slices, prevState.searchTerm, sortBy, - prevState.showAccessibleCharts, + prevState.showOnlyMyCharts, ), })); this.slicesRequest = this.props.fetchSlices( @@ -281,8 +295,8 @@ class SliceAdder extends React.Component { ); } - onAccessibleCharts(showAccessibleCharts) { - if (showAccessibleCharts) { + onShowOnlyMyCharts(showOnlyMyCharts) { + if (!showOnlyMyCharts) { this.slicesRequest = this.props.fetchSlices( undefined, this.state.searchTerm, @@ -290,14 +304,18 @@ class SliceAdder extends React.Component { ); } this.setState(prevState => ({ - showAccessibleCharts, + showOnlyMyCharts, filteredSlices: this.getFilteredSortedSlices( this.props.slices, prevState.searchTerm, prevState.sortBy, - showAccessibleCharts, + showOnlyMyCharts, ), })); + setItem( + LocalStorageKeys.dashboard__editor_show_only_my_charts, + showOnlyMyCharts, + ); } render() { @@ -328,7 +346,7 @@ class SliceAdder extends React.Component { css` display: flex; flex-direction: row; - justify-content: center; - padding: 0 ${theme.gridUnit * 3}px ${theme.gridUnit * 3}px + justify-content: flex-start; + align-items: center; + gap: ${theme.gridUnit}px; + padding: 0 ${theme.gridUnit * 3}px ${theme.gridUnit * 4}px ${theme.gridUnit * 3}px; - - & > span { - margin-right: ${theme.gridUnit}px; - } `} > + {t('Show only my charts')} + - {t('Show other charts I have access to')}
{this.props.isLoading && } {!this.props.isLoading && this.state.filteredSlices.length > 0 && ( diff --git a/superset-frontend/src/utils/localStorageHelpers.ts b/superset-frontend/src/utils/localStorageHelpers.ts index 3dceaba86ece6..beb5ed10147ab 100644 --- a/superset-frontend/src/utils/localStorageHelpers.ts +++ b/superset-frontend/src/utils/localStorageHelpers.ts @@ -54,6 +54,7 @@ export enum LocalStorageKeys { explore__data_table_original_formatted_time_columns = 'explore__data_table_original_formatted_time_columns', dashboard__custom_filter_bar_widths = 'dashboard__custom_filter_bar_widths', dashboard__explore_context = 'dashboard__explore_context', + dashboard__editor_show_only_my_charts = 'dashboard__editor_show_only_my_charts', common__resizable_sidebar_widths = 'common__resizable_sidebar_widths', } @@ -73,6 +74,7 @@ export type LocalStorageValues = { explore__data_table_original_formatted_time_columns: Record; dashboard__custom_filter_bar_widths: Record; dashboard__explore_context: Record; + dashboard__editor_show_only_my_charts: boolean; common__resizable_sidebar_widths: Record; }; From d22c8f939b57e80aaf0c09c3301ac61c18493292 Mon Sep 17 00:00:00 2001 From: "Michael S. molina" Date: Tue, 4 Apr 2023 14:38:09 -0300 Subject: [PATCH 4/7] Inverts input message --- superset-frontend/src/dashboard/components/SliceAdder.jsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/superset-frontend/src/dashboard/components/SliceAdder.jsx b/superset-frontend/src/dashboard/components/SliceAdder.jsx index 150d63ffccab0..6d05800ac701b 100644 --- a/superset-frontend/src/dashboard/components/SliceAdder.jsx +++ b/superset-frontend/src/dashboard/components/SliceAdder.jsx @@ -347,8 +347,8 @@ class SliceAdder extends React.Component { this.handleChange(ev.target.value)} From fe4f17f57ec5f44bd640d24f49e3b641f0601850 Mon Sep 17 00:00:00 2001 From: "Michael S. molina" Date: Tue, 4 Apr 2023 14:49:28 -0300 Subject: [PATCH 5/7] Removes only --- .../cypress-base/cypress/integration/dashboard/editmode.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/cypress-base/cypress/integration/dashboard/editmode.test.ts b/superset-frontend/cypress-base/cypress/integration/dashboard/editmode.test.ts index 6201a582aae85..50fd153146e8e 100644 --- a/superset-frontend/cypress-base/cypress/integration/dashboard/editmode.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/dashboard/editmode.test.ts @@ -751,7 +751,7 @@ describe('Dashboard edit', () => { cy.getBySel('dashboard-component-chart-holder').should('have.length', 1); }); - it.only('should remove added charts', () => { + it('should remove added charts', () => { dragComponent('Unicode Cloud'); cy.getBySel('dashboard-component-chart-holder').should('have.length', 1); cy.getBySel('dashboard-delete-component-button').click(); From 92a5600f7328547565e3bc11f9fd617bd30a0a46 Mon Sep 17 00:00:00 2001 From: "Michael S. molina" Date: Tue, 4 Apr 2023 16:47:06 -0300 Subject: [PATCH 6/7] Uses reduce --- .../src/dashboard/actions/sliceEntities.ts | 48 +++++++++---------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/superset-frontend/src/dashboard/actions/sliceEntities.ts b/superset-frontend/src/dashboard/actions/sliceEntities.ts index 18a7a55561011..a30470aad12a4 100644 --- a/superset-frontend/src/dashboard/actions/sliceEntities.ts +++ b/superset-frontend/src/dashboard/actions/sliceEntities.ts @@ -58,9 +58,8 @@ function fetchAllSlicesFailed(error: string) { return { type: FETCH_ALL_SLICES_FAILED, payload: { error } }; } -const parseResult = (result: any[]) => { - const slices: { [id: number]: Slice } = {}; - result.forEach((slice: any) => { +const parseResult = (result: any[]) => + result.reduce((slices, slice: any) => { let form_data = JSON.parse(slice.params); form_data = { ...form_data, @@ -69,28 +68,29 @@ const parseResult = (result: any[]) => { getDatasourceParameter(slice.datasource_id, slice.datasource_type) || form_data.datasource, }; - slices[slice.id] = { - slice_id: slice.id, - slice_url: slice.url, - slice_name: slice.slice_name, - form_data, - datasource_name: slice.datasource_name_text, - datasource_url: slice.datasource_url, - datasource_id: slice.datasource_id, - datasource_type: slice.datasource_type, - changed_on: new Date(slice.changed_on_utc).getTime(), - description: slice.description, - description_markdown: slice.description_markeddown, - viz_type: slice.viz_type, - modified: slice.changed_on_delta_humanized, - changed_on_humanized: slice.changed_on_delta_humanized, - thumbnail_url: slice.thumbnail_url, - owners: slice.owners, - created_by: slice.created_by, + return { + ...slices, + [slice.id]: { + slice_id: slice.id, + slice_url: slice.url, + slice_name: slice.slice_name, + form_data, + datasource_name: slice.datasource_name_text, + datasource_url: slice.datasource_url, + datasource_id: slice.datasource_id, + datasource_type: slice.datasource_type, + changed_on: new Date(slice.changed_on_utc).getTime(), + description: slice.description, + description_markdown: slice.description_markeddown, + viz_type: slice.viz_type, + modified: slice.changed_on_delta_humanized, + changed_on_humanized: slice.changed_on_delta_humanized, + thumbnail_url: slice.thumbnail_url, + owners: slice.owners, + created_by: slice.created_by, + }, }; - }); - return slices; -}; + }, {}); export function updateSlices(slices: { [id: number]: Slice }) { return (dispatch: Dispatch) => { From bd7741d105e963aedf4c8d8dc8e7695ce1003213 Mon Sep 17 00:00:00 2001 From: "Michael S. molina" Date: Thu, 6 Apr 2023 08:45:20 -0300 Subject: [PATCH 7/7] Changes copy --- superset-frontend/src/dashboard/components/SliceAdder.jsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/superset-frontend/src/dashboard/components/SliceAdder.jsx b/superset-frontend/src/dashboard/components/SliceAdder.jsx index 6d05800ac701b..617fd32e8fc8f 100644 --- a/superset-frontend/src/dashboard/components/SliceAdder.jsx +++ b/superset-frontend/src/dashboard/components/SliceAdder.jsx @@ -384,8 +384,8 @@ class SliceAdder extends React.Component {