feat: SIP-34 explore save modal (#10355)

* feat: SIP-34 explore save modal

* using a const for the session storage key

* backend changes

* minor tweaks

* more tweaks

* radio cosmetics

* styles

* fix tests

* CreatableSelect\!

* Fix cypress & lint

* fix unit

* lint
This commit is contained in:
Maxime Beauchemin 2020-07-23 00:26:29 -07:00 committed by GitHub
parent 9a5d812ee6
commit ea53916730
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 261 additions and 243 deletions

View File

@ -20,8 +20,14 @@
// Tests for links in the explore UI
// ***********************************************
import rison from 'rison';
import shortid from 'shortid';
import { HEALTH_POP_FORM_DATA_DEFAULTS } from './visualizations/shared.helper';
const apiURL = (endpoint, queryObject) => {
return `${endpoint}?q=${rison.encode(queryObject)}`;
};
describe('Test explore links', () => {
beforeEach(() => {
cy.login();
@ -73,98 +79,120 @@ describe('Test explore links', () => {
});
});
xit('Test chart save as', () => {
it('Test chart save as AND overwrite', () => {
const formData = {
...HEALTH_POP_FORM_DATA_DEFAULTS,
viz_type: 'table',
metrics: ['sum__SP_POP_TOTL'],
groupby: ['country_name'],
};
const newChartName = 'Test chart';
const newChartName = `Test chart [${shortid.generate()}]`;
cy.visitChartByParams(JSON.stringify(formData));
cy.verifySliceSuccess({ waitAlias: '@postJson' });
cy.url().then(url => {
cy.get('button[data-target="#save_modal"]').click();
cy.get('.modal-content').within(() => {
cy.get('#saveas-radio').check();
cy.get('input[name=new_slice_name]').type(newChartName);
cy.get('button#btn_modal_save').click();
});
cy.url().should('eq', url);
cy.visitChartByName(newChartName);
cy.verifySliceSuccess({ waitAlias: '@postJson' });
});
});
cy.visitChartByName(newChartName);
xit('Test chart save', () => {
const chartName = 'Test chart';
cy.visitChartByName(chartName);
cy.verifySliceSuccess({ waitAlias: '@postJson' });
cy.get('[data-test=groupby]').within(() => {
cy.get('.Select__clear-indicator').click();
// Overwriting!
cy.get('button[data-target="#save_modal"]').click();
cy.get('.modal-content').within(() => {
cy.get('#overwrite-radio').check();
cy.get('button#btn_modal_save').click();
});
cy.verifySliceSuccess({ waitAlias: '@postJson' });
const query = {
filters: [
{
col: 'slice_name',
opr: 'eq',
value: newChartName,
},
],
};
cy.request(apiURL('/api/v1/chart/', query)).then(response => {
expect(response.body.count).equals(1);
cy.request('DELETE', `/api/v1/chart/${response.body.ids[0]}`);
});
});
cy.get('button[data-target="#save_modal"]').click();
cy.get('.modal-content').within(() => {
cy.get('button#btn_modal_save').click();
});
cy.verifySliceSuccess({ waitAlias: '@postJson' });
cy.request(`/chart/api/read?_flt_3_slice_name=${chartName}`).then(
response => {
cy.request('DELETE', `/chart/api/delete/${response.body.pks[0]}`);
},
);
});
it('Test chart save as and add to new dashboard', () => {
cy.visitChartByName('Growth Rate');
cy.verifySliceSuccess({ waitAlias: '@postJson' });
const chartName = 'Growth Rate';
const newChartName = `${chartName} [${shortid.generate()}]`;
const dashboardTitle = `Test dashboard [${shortid.generate()}]`;
const dashboardTitle = 'Test dashboard';
cy.get('button[data-target="#save_modal"]').click();
cy.get('.modal-content').within(() => {
cy.get('input[name=new_slice_name]').type('New Growth Rate');
cy.get('input[data-test=add-to-new-dashboard]').check();
cy.get('input[placeholder="[dashboard name]"]').type(dashboardTitle);
cy.get('button#btn_modal_save').click();
});
cy.visitChartByName(chartName);
cy.verifySliceSuccess({ waitAlias: '@postJson' });
cy.request(
`/dashboard/api/read?_flt_3_dashboard_title=${dashboardTitle}`,
).then(response => {
expect(response.body.pks[0]).not.equals(null);
});
});
it('Test chart save as and add to existing dashboard', () => {
cy.visitChartByName('Most Populated Countries');
cy.verifySliceSuccess({ waitAlias: '@postJson' });
const chartName = 'New Most Populated Countries';
const dashboardTitle = 'Test dashboard';
cy.get('button[data-target="#save_modal"]').click();
cy.get('.modal-content').within(() => {
cy.get('input[name=new_slice_name]').type(chartName);
cy.get('input[data-test=add-to-existing-dashboard]').check();
cy.get('.save-modal-selector')
.click()
.within(() => {
cy.get('input').type(dashboardTitle);
cy.get('.Select__option--is-focused').trigger('mousedown');
});
cy.get('#saveas-radio').check();
cy.get('input[name=new_slice_name]').click().clear().type(newChartName);
// Add a new option using the "CreatableSelect" feature
cy.get('#dashboard-creatable-select').type(
`${dashboardTitle}{enter}{enter}`,
);
cy.get('button#btn_modal_save').click();
});
cy.verifySliceSuccess({ waitAlias: '@postJson' });
cy.request(`/chart/api/read?_flt_3_slice_name=${chartName}`).then(
response => {
cy.request('DELETE', `/chart/api/delete/${response.body.pks[0]}`);
},
);
cy.request(
`/dashboard/api/read?_flt_3_dashboard_title=${dashboardTitle}`,
).then(response => {
cy.request('DELETE', `/dashboard/api/delete/${response.body.pks[0]}`);
let query = {
filters: [
{
col: 'dashboard_title',
opr: 'eq',
value: dashboardTitle,
},
],
};
cy.request(apiURL('/api/v1/dashboard/', query)).then(response => {
expect(response.body.count).equals(1);
});
cy.visitChartByName(newChartName);
cy.verifySliceSuccess({ waitAlias: '@postJson' });
cy.get('button[data-target="#save_modal"]').click();
cy.get('.modal-content').within(() => {
cy.get('#overwrite-radio').check();
cy.get('input[name=new_slice_name]').click().clear().type(newChartName);
// This time around, typing the same dashboard name
// will select the existing one
cy.get('#dashboard-creatable-select').type(
`${dashboardTitle}{enter}{enter}`,
);
cy.get('button#btn_modal_save').click();
});
cy.verifySliceSuccess({ waitAlias: '@postJson' });
query = {
filters: [
{
col: 'slice_name',
opr: 'eq',
value: chartName,
},
],
};
cy.request(apiURL('/api/v1/chart/', query)).then(response => {
expect(response.body.count).equals(1);
});
query = {
filters: [
{
col: 'dashboard_title',
opr: 'eq',
value: dashboardTitle,
},
],
};
cy.request(apiURL('/api/v1/dashboard/', query)).then(response => {
expect(response.body.count).equals(1);
});
});
});

View File

@ -5005,6 +5005,11 @@
"inherits": "^2.0.1"
}
},
"rison": {
"version": "0.1.1",
"resolved": "https://registry.npmjs.org/rison/-/rison-0.1.1.tgz",
"integrity": "sha1-TcwFV7JBr/YOdheOd5ITVxPzMSA="
},
"run-parallel": {
"version": "1.1.9",
"resolved": "https://registry.npmjs.org/run-parallel/-/run-parallel-1.1.9.tgz",

View File

@ -9,8 +9,9 @@
"author": "Apcahe",
"license": "Apache-2.0",
"dependencies": {
"shortid": "^2.2.15",
"@cypress/code-coverage": "^3.8.1"
"@cypress/code-coverage": "^3.8.1",
"rison": "^0.1.1",
"shortid": "^2.2.15"
},
"devDependencies": {
"cypress": "4.9.0",

View File

@ -22,7 +22,7 @@ import thunk from 'redux-thunk';
import { bindActionCreators } from 'redux';
import { shallow, mount } from 'enzyme';
import { Modal, Button, Radio } from 'react-bootstrap';
import { FormControl, Modal, Button, Radio } from 'react-bootstrap';
import sinon from 'sinon';
import fetchMock from 'fetch-mock';
@ -65,7 +65,7 @@ describe('SaveModal', () => {
target: {
value: 'mock event target',
},
value: 'mock value',
value: 10,
};
const getWrapper = () =>
@ -73,19 +73,18 @@ describe('SaveModal', () => {
context: { store },
}).dive();
it('renders a Modal with 7 inputs and 2 buttons', () => {
it('renders a Modal with the right set of components', () => {
const wrapper = getWrapper();
expect(wrapper.find(Modal)).toHaveLength(1);
expect(wrapper.find('input')).toHaveLength(2);
expect(wrapper.find(Button)).toHaveLength(2);
expect(wrapper.find(Radio)).toHaveLength(5);
expect(wrapper.find(FormControl)).toHaveLength(1);
expect(wrapper.find(Button)).toHaveLength(3);
expect(wrapper.find(Radio)).toHaveLength(2);
});
it('does not show overwrite option for new slice', () => {
const wrapperNewSlice = getWrapper();
wrapperNewSlice.setProps({ slice: null });
expect(wrapperNewSlice.find('#overwrite-radio')).toHaveLength(0);
expect(wrapperNewSlice.find('#saveas-radio')).toHaveLength(1);
it('overwrite radio button is disabled for new slice', () => {
const wrapper = getWrapper();
wrapper.setProps({ slice: null });
expect(wrapper.find('#overwrite-radio').prop('disabled')).toBe(true);
});
it('disable overwrite option for non-owner', () => {
@ -128,14 +127,11 @@ describe('SaveModal', () => {
it('onChange', () => {
const wrapper = getWrapper();
wrapper.instance().onChange('newSliceName', mockEvent);
wrapper.instance().onSliceNameChange(mockEvent);
expect(wrapper.state().newSliceName).toBe(mockEvent.target.value);
wrapper.instance().onChange('saveToDashboardId', mockEvent);
wrapper.instance().onDashboardSelectChange(mockEvent);
expect(wrapper.state().saveToDashboardId).toBe(mockEvent.value);
wrapper.instance().onChange('newDashboardName', mockEvent);
expect(wrapper.state().newDashboardName).toBe(mockEvent.target.value);
});
describe('saveOrOverwrite', () => {
@ -145,7 +141,7 @@ describe('SaveModal', () => {
sinon.stub(defaultProps.actions, 'saveSlice').callsFake(() =>
Promise.resolve({
data: {
dashboard: '/mock_dashboard/',
dashboard_url: 'http://localhost/mock_dashboard/',
slice: { slice_url: '/mock_slice/' },
},
}),
@ -168,10 +164,6 @@ describe('SaveModal', () => {
const wrapper = getWrapper();
const saveToDashboardId = 100;
wrapper.setState({ addToDash: 'existing' });
wrapper.instance().saveOrOverwrite(true);
expect(wrapper.state().alert).toBe('Please select a dashboard');
wrapper.setState({ saveToDashboardId });
wrapper.instance().saveOrOverwrite(true);
const args = defaultProps.actions.saveSlice.getCall(0).args;
@ -182,10 +174,6 @@ describe('SaveModal', () => {
const wrapper = getWrapper();
const newDashboardName = 'new dashboard name';
wrapper.setState({ addToDash: 'new' });
wrapper.instance().saveOrOverwrite(true);
expect(wrapper.state().alert).toBe('Please enter a dashboard name');
wrapper.setState({ newDashboardName });
wrapper.instance().saveOrOverwrite(true);
const args = defaultProps.actions.saveSlice.getCall(0).args;

View File

@ -20,11 +20,19 @@
import React from 'react';
import PropTypes from 'prop-types';
import { connect } from 'react-redux';
import { Modal, Alert, Button, Radio } from 'react-bootstrap';
import Select from 'src/components/Select';
import {
Alert,
Button,
FormControl,
FormGroup,
ControlLabel,
Modal,
Radio,
} from 'react-bootstrap';
import { CreatableSelect } from 'src/components/Select/SupersetStyledSelect';
import { t } from '@superset-ui/translation';
import ReactMarkdown from 'react-markdown';
import { supersetURL } from '../../utils/common';
import { EXPLORE_ONLY_VIZ_TYPE } from '../constants';
const propTypes = {
@ -39,28 +47,30 @@ const propTypes = {
datasource: PropTypes.object,
};
// Session storage key for recent dashboard
const SK_DASHBOARD_ID = 'save_chart_recent_dashboard';
const SELECT_PLACEHOLDER = t('**Select** a dashboard OR **create** a new one');
class SaveModal extends React.Component {
constructor(props) {
super(props);
this.state = {
saveToDashboardId: null,
newDashboardName: '',
newSliceName: props.sliceName,
dashboards: [],
alert: null,
action: props.can_overwrite ? 'overwrite' : 'saveas',
addToDash: 'noSave',
vizType: props.form_data.viz_type,
};
this.onDashboardSelectChange = this.onDashboardSelectChange.bind(this);
this.onSliceNameChange = this.onSliceNameChange.bind(this);
}
componentDidMount() {
this.props.actions.fetchDashboards(this.props.userId).then(() => {
const dashboardIds = this.props.dashboards.map(
dashboard => dashboard.value,
);
let recentDashboard = sessionStorage.getItem(
'save_chart_recent_dashboard',
);
let recentDashboard = sessionStorage.getItem(SK_DASHBOARD_ID);
recentDashboard = recentDashboard && parseInt(recentDashboard, 10);
if (
recentDashboard !== null &&
@ -68,33 +78,22 @@ class SaveModal extends React.Component {
) {
this.setState({
saveToDashboardId: recentDashboard,
addToDash: 'existing',
});
}
});
}
onChange(name, event) {
switch (name) {
case 'newSliceName':
this.setState({ newSliceName: event.target.value });
break;
case 'saveToDashboardId':
this.setState({ saveToDashboardId: event.value });
this.changeDash('existing');
break;
case 'newDashboardName':
this.setState({ newDashboardName: event.target.value });
break;
default:
break;
}
onSliceNameChange(event) {
this.setState({ newSliceName: event.target.value });
}
onDashboardSelectChange(event) {
const newDashboardName = event ? event.label : null;
const saveToDashboardId =
event && typeof event.value === 'number' ? event.value : null;
this.setState({ saveToDashboardId, newDashboardName });
}
changeAction(action) {
this.setState({ action });
}
changeDash(dash) {
this.setState({ addToDash: dash });
}
saveOrOverwrite(gotodash) {
this.setState({ alert: null });
this.props.actions.removeSaveModalAlert();
@ -111,49 +110,20 @@ class SaveModal extends React.Component {
}
sliceParams.action = this.state.action;
sliceParams.slice_name = this.state.newSliceName;
const addToDash = this.state.addToDash;
sliceParams.add_to_dash = addToDash;
let dashboard = null;
switch (addToDash) {
case 'existing':
dashboard = this.state.saveToDashboardId;
if (!dashboard) {
this.setState({ alert: t('Please select a dashboard') });
return;
}
sliceParams.save_to_dashboard_id = dashboard;
break;
case 'new':
dashboard = this.state.newDashboardName;
if (dashboard === '') {
this.setState({ alert: t('Please enter a dashboard name') });
return;
}
sliceParams.new_dashboard_name = dashboard;
break;
default:
dashboard = null;
}
sliceParams.goto_dash = gotodash;
sliceParams.save_to_dashboard_id = this.state.saveToDashboardId;
sliceParams.new_dashboard_name = this.state.newDashboardName;
this.props.actions
.saveSlice(this.props.form_data, sliceParams)
.then(({ data }) => {
if (data.dashboard_id === null) {
sessionStorage.removeItem('save_chart_recent_dashboard');
sessionStorage.removeItem(SK_DASHBOARD_ID);
} else {
sessionStorage.setItem(
'save_chart_recent_dashboard',
data.dashboard_id,
);
sessionStorage.setItem(SK_DASHBOARD_ID, data.dashboard_id);
}
// Go to new slice url or dashboard url
if (gotodash) {
window.location.assign(supersetURL(data.dashboard));
} else {
window.location.assign(data.slice.slice_url);
}
const url = gotodash ? data.dashboard_url : data.slice.slice_url;
window.location.assign(url);
});
this.props.onHide();
}
@ -167,9 +137,9 @@ class SaveModal extends React.Component {
const canNotSaveToDash =
EXPLORE_ONLY_VIZ_TYPE.indexOf(this.state.vizType) > -1;
return (
<Modal show onHide={this.props.onHide} bsStyle="large">
<Modal show onHide={this.props.onHide}>
<Modal.Header closeButton>
<Modal.Title>{t('Save A Chart')}</Modal.Title>
<Modal.Title>{t('Save Chart')}</Modal.Title>
</Modal.Header>
<Modal.Body>
{(this.state.alert || this.props.alert) && (
@ -184,97 +154,91 @@ class SaveModal extends React.Component {
/>
</Alert>
)}
{this.props.slice && (
<FormGroup>
<Radio
id="overwrite-radio"
disabled={!this.props.can_overwrite}
inline
disabled={!(this.props.can_overwrite && this.props.slice)}
checked={this.state.action === 'overwrite'}
onChange={this.changeAction.bind(this, 'overwrite')}
>
{t('Overwrite chart %s', this.props.slice.slice_name)}
{t('Save (Overwrite)')}
</Radio>
)}
<Radio
id="saveas-radio"
inline
checked={this.state.action === 'saveas'}
onChange={this.changeAction.bind(this, 'saveas')}
>
{' '}
{t('Save as')} &nbsp;
</Radio>
<input
name="new_slice_name"
placeholder={this.state.newSliceName || t('[chart name]')}
onChange={this.onChange.bind(this, 'newSliceName')}
onFocus={this.changeAction.bind(this, 'saveas')}
/>
<br />
<Radio
id="saveas-radio"
inline
checked={this.state.action === 'saveas'}
onChange={this.changeAction.bind(this, 'saveas')}
>
{' '}
{t('Save as ...')} &nbsp;
</Radio>
</FormGroup>
<hr />
<Radio
checked={this.state.addToDash === 'noSave'}
onChange={this.changeDash.bind(this, 'noSave')}
>
{t('Do not add to a dashboard')}
</Radio>
<Radio
inline
disabled={canNotSaveToDash}
checked={this.state.addToDash === 'existing'}
onChange={this.changeDash.bind(this, 'existing')}
data-test="add-to-existing-dashboard"
>
{t('Add chart to existing dashboard')}
</Radio>
<Select
className="save-modal-selector"
disabled={canNotSaveToDash}
options={this.props.dashboards}
onChange={this.onChange.bind(this, 'saveToDashboardId')}
autoSize={false}
value={this.state.saveToDashboardId}
placeholder="Select Dashboard"
/>
<Radio
inline
checked={this.state.addToDash === 'new'}
onChange={this.changeDash.bind(this, 'new')}
disabled={canNotSaveToDash}
data-test="add-to-new-dashboard"
>
{t('Add to new dashboard')} &nbsp;
</Radio>
<input
onChange={this.onChange.bind(this, 'newDashboardName')}
disabled={canNotSaveToDash}
onFocus={this.changeDash.bind(this, 'new')}
placeholder={t('[dashboard name]')}
/>
<FormGroup>
<ControlLabel>{t('Chart name')}</ControlLabel>
<FormControl
name="new_slice_name"
type="text"
bsSize="sm"
placeholder="Name"
value={this.state.newSliceName}
onChange={this.onSliceNameChange}
/>
</FormGroup>
<FormGroup>
<ControlLabel>{t('Add to dashboard')}</ControlLabel>
<CreatableSelect
id="dashboard-creatable-select"
className="save-modal-selector"
options={this.props.dashboards}
clearable
creatable
onChange={this.onDashboardSelectChange}
autoSize={false}
value={
this.state.saveToDashboardId || this.state.newDashboardName
}
placeholder={
// Using markdown to allow for good i18n
<ReactMarkdown
source={SELECT_PLACEHOLDER}
renderers={{ paragraph: 'span' }}
/>
}
/>
</FormGroup>
</Modal.Body>
<Modal.Footer>
<Button
type="button"
id="btn_modal_save"
className="btn pull-left"
onClick={this.saveOrOverwrite.bind(this, false)}
>
{t('Save')}
</Button>
<Button
type="button"
id="btn_modal_save_goto_dash"
className="btn btn-primary pull-left gotodash"
disabled={this.state.addToDash === 'noSave' || canNotSaveToDash}
onClick={this.saveOrOverwrite.bind(this, true)}
>
{t('Save & go to dashboard')}
</Button>
<div className="float-right">
<Button
type="button"
id="btn_cancel"
bsSize="sm"
onClick={this.props.onHide}
>
{t('Cancel')}
</Button>
<Button
type="button"
id="btn_modal_save_goto_dash"
bsSize="sm"
disabled={canNotSaveToDash || !this.state.newDashboardName}
onClick={this.saveOrOverwrite.bind(this, true)}
>
{t('Save & go to dashboard')}
</Button>
<Button
type="button"
id="btn_modal_save"
bsSize="sm"
bsStyle="primary"
onClick={this.saveOrOverwrite.bind(this, false)}
>
{t('Save')}
</Button>
</div>
</Modal.Footer>
</Modal>
);

View File

@ -112,10 +112,6 @@
}
}
.save-modal-selector {
margin: 10px 0;
}
.list-group {
margin-bottom: 10px;
}

View File

@ -560,3 +560,35 @@ td.filtered {
flex-direction: column;
justify-content: center;
}
// Making native radio buttons use brand color
input[type='radio']:after {
width: 15px;
height: 15px;
border-radius: 15px;
top: -2px;
left: -1px;
position: relative;
background-color: #fff;
content: '';
display: inline-block;
visibility: visible;
border: 2px solid @gray;
}
input[type='radio']:checked:after {
width: 15px;
height: 15px;
border-radius: 15px;
top: -2px;
left: -1px;
position: relative;
background-color: #fff;
content: '';
display: inline-block;
visibility: visible;
border: 5px solid @brand-primary;
}
hr {
border-top: 1px solid @gray-light;
}

View File

@ -813,11 +813,14 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods
# Adding slice to a dashboard if requested
dash: Optional[Dashboard] = None
if request.args.get("add_to_dash") == "existing":
save_to_dashboard_id = request.args.get("save_to_dashboard_id")
new_dashboard_name = request.args.get("new_dashboard_name")
if save_to_dashboard_id:
# Adding the chart to an existing dashboard
dash = cast(
Dashboard,
db.session.query(Dashboard)
.filter_by(id=int(request.args["save_to_dashboard_id"]))
.filter_by(id=int(save_to_dashboard_id))
.one(),
)
# check edit dashboard permissions
@ -836,7 +839,8 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods
),
"info",
)
elif request.args.get("add_to_dash") == "new":
elif new_dashboard_name:
# Creating and adding to a new dashboard
# check create dashboard permissions
dash_add_perm = security_manager.can_access("can_add", "DashboardModelView")
if not dash_add_perm:
@ -868,7 +872,7 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods
"can_overwrite": is_owner(slc, g.user),
"form_data": slc.form_data,
"slice": slc.data,
"dashboard_id": dash.id if dash else None,
"dashboard_url": dash.url if dash else None,
}
if dash and request.args.get("goto_dash") == "true":