From e53f3032bb2cc03019dec1b5eab49911801eab3a Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Wed, 30 Aug 2017 14:09:29 -0700 Subject: [PATCH] [dashboard] adding an option to duplicate slices when "Saving AS" (#3391) * [dashboard] adding an option to duplicate slices when "Saving AS" * Fix tests --- .../javascripts/components/Checkbox.jsx | 24 ++++++++++++ .../dashboard/components/SaveModal.jsx | 23 +++++++++-- .../components/controls/CheckboxControl.jsx | 21 +++++----- .../javascripts/components/Checkbox_spec.jsx | 39 +++++++++++++++++++ .../components/CheckboxControl_spec.jsx | 3 +- superset/assets/stylesheets/superset.less | 6 +++ superset/assets/visualizations/table.css | 4 ++ superset/models/core.py | 11 ++++++ superset/views/core.py | 23 +++++++++-- tests/core_tests.py | 1 + 10 files changed, 135 insertions(+), 20 deletions(-) create mode 100644 superset/assets/javascripts/components/Checkbox.jsx create mode 100644 superset/assets/spec/javascripts/components/Checkbox_spec.jsx diff --git a/superset/assets/javascripts/components/Checkbox.jsx b/superset/assets/javascripts/components/Checkbox.jsx new file mode 100644 index 0000000000..b0564ae6b8 --- /dev/null +++ b/superset/assets/javascripts/components/Checkbox.jsx @@ -0,0 +1,24 @@ +import React from 'react'; +import PropTypes from 'prop-types'; + +const propTypes = { + checked: PropTypes.bool.isRequired, + onChange: PropTypes.func.isRequired, + style: PropTypes.object, +}; + +export default function Checkbox({ checked, onChange, style }) { + return ( + + + ); +} +Checkbox.propTypes = propTypes; diff --git a/superset/assets/javascripts/dashboard/components/SaveModal.jsx b/superset/assets/javascripts/dashboard/components/SaveModal.jsx index 82b44df0f6..ccf8f90b19 100644 --- a/superset/assets/javascripts/dashboard/components/SaveModal.jsx +++ b/superset/assets/javascripts/dashboard/components/SaveModal.jsx @@ -4,6 +4,7 @@ import PropTypes from 'prop-types'; import { Button, FormControl, FormGroup, Radio } from 'react-bootstrap'; import { getAjaxErrorMsg } from '../../modules/utils'; import ModalTrigger from '../../components/ModalTrigger'; +import Checkbox from '../../components/Checkbox'; const $ = window.$ = require('jquery'); @@ -20,13 +21,17 @@ class SaveModal extends React.PureComponent { dashboard: props.dashboard, css: props.css, saveType: 'overwrite', - newDashName: '', + newDashName: props.dashboard.dashboard_title + ' [copy]', + duplicateSlices: false, }; this.modal = null; this.handleSaveTypeChange = this.handleSaveTypeChange.bind(this); this.handleNameChange = this.handleNameChange.bind(this); this.saveDashboard = this.saveDashboard.bind(this); } + toggleDuplicateSlices() { + this.setState({ duplicateSlices: !this.state.duplicateSlices }); + } handleSaveTypeChange(event) { this.setState({ saveType: event.target.value, @@ -52,7 +57,7 @@ class SaveModal extends React.PureComponent { saveModal.close(); dashboard.onSave(); if (saveType === 'newDashboard') { - window.location = '/superset/dashboard/' + resp.id + '/'; + window.location = `/superset/dashboard/${resp.id}/`; } else { notify.success('This dashboard was saved successfully.'); } @@ -81,10 +86,11 @@ class SaveModal extends React.PureComponent { expanded_slices: expandedSlices, dashboard_title: dashboard.dashboard_title, default_filters: dashboard.readFilters(), + duplicate_slices: this.state.duplicateSlices, }; let url = null; if (saveType === 'overwrite') { - url = '/superset/save_dash/' + dashboard.id + '/'; + url = `/superset/save_dash/${dashboard.id}/`; this.saveDashboardRequest(data, url, saveType); } else if (saveType === 'newDashboard') { if (!newDashboardTitle) { @@ -95,7 +101,7 @@ class SaveModal extends React.PureComponent { }); } else { data.dashboard_title = newDashboardTitle; - url = '/superset/copy_dash/' + dashboard.id + '/'; + url = `/superset/copy_dash/${dashboard.id}/`; this.saveDashboardRequest(data, url, saveType); } } @@ -115,6 +121,7 @@ class SaveModal extends React.PureComponent { > Overwrite Dashboard [{this.props.dashboard.dashboard_title}] +
+
+ + also copy (duplicate) slices +
} modalFooter={ diff --git a/superset/assets/javascripts/explore/components/controls/CheckboxControl.jsx b/superset/assets/javascripts/explore/components/controls/CheckboxControl.jsx index fa70c18a6f..7d78646e79 100644 --- a/superset/assets/javascripts/explore/components/controls/CheckboxControl.jsx +++ b/superset/assets/javascripts/explore/components/controls/CheckboxControl.jsx @@ -1,6 +1,7 @@ import React from 'react'; import PropTypes from 'prop-types'; import ControlHeader from '../ControlHeader'; +import Checkbox from '../../../components/Checkbox'; const propTypes = { name: PropTypes.string.isRequired, @@ -15,24 +16,22 @@ const defaultProps = { onChange: () => {}, }; +const checkboxStyle = { paddingRight: '5px' }; + export default class CheckboxControl extends React.Component { - onToggle() { - this.props.onChange(!this.props.value); + onChange(checked) { + this.props.onChange(checked); } render() { return ( - -    - + } /> ); diff --git a/superset/assets/spec/javascripts/components/Checkbox_spec.jsx b/superset/assets/spec/javascripts/components/Checkbox_spec.jsx new file mode 100644 index 0000000000..72b615c690 --- /dev/null +++ b/superset/assets/spec/javascripts/components/Checkbox_spec.jsx @@ -0,0 +1,39 @@ +import React from 'react'; +import { expect } from 'chai'; +import { describe, it } from 'mocha'; +import sinon from 'sinon'; +import { shallow } from 'enzyme'; + +import Checkbox from '../../../javascripts/components/Checkbox'; + +describe('Checkbox', () => { + const defaultProps = { + checked: true, + onChange: sinon.spy(), + }; + + let wrapper; + const factory = (o) => { + const props = Object.assign({}, defaultProps, o); + return shallow(); + }; + beforeEach(() => { + wrapper = factory({}); + }); + it('is a valid element', () => { + expect(React.isValidElement()).to.equal(true); + }); + it('inits checked when checked', () => { + expect(wrapper.find('i.fa-check.text-primary')).to.have.length(1); + }); + it('inits unchecked when not checked', () => { + const el = factory({ checked: false }); + expect(el.find('i.fa-check.text-primary')).to.have.length(0); + expect(el.find('i.fa-check.text-transparent')).to.have.length(1); + }); + it('unchecks when clicked', () => { + expect(wrapper.find('i.fa-check.text-transparent')).to.have.length(0); + wrapper.find('i').first().simulate('click'); + expect(defaultProps.onChange.calledOnce).to.equal(true); + }); +}); diff --git a/superset/assets/spec/javascripts/explore/components/CheckboxControl_spec.jsx b/superset/assets/spec/javascripts/explore/components/CheckboxControl_spec.jsx index 23e58aca90..66b11a4198 100644 --- a/superset/assets/spec/javascripts/explore/components/CheckboxControl_spec.jsx +++ b/superset/assets/spec/javascripts/explore/components/CheckboxControl_spec.jsx @@ -7,6 +7,7 @@ import { shallow } from 'enzyme'; import CheckboxControl from '../../../../javascripts/explore/components/controls/CheckboxControl'; import ControlHeader from '../../../../javascripts/explore/components/ControlHeader'; +import Checkbox from '../../../../javascripts/components/Checkbox'; const defaultProps = { name: 'show_legend', @@ -27,6 +28,6 @@ describe('CheckboxControl', () => { expect(controlHeader).to.have.lengthOf(1); const headerWrapper = controlHeader.shallow(); - expect(headerWrapper.find('i.fa-check')).to.have.length(1); + expect(headerWrapper.find(Checkbox)).to.have.length(1); }); }); diff --git a/superset/assets/stylesheets/superset.less b/superset/assets/stylesheets/superset.less index 89b5cbc09e..6dca61066e 100644 --- a/superset/assets/stylesheets/superset.less +++ b/superset/assets/stylesheets/superset.less @@ -228,6 +228,12 @@ div.widget .slice_container { .m-t-5 { margin-top: 5px; } +.m-l-5 { + margin-left: 5px; +} +.m-l-25 { + margin-left: 25px; +} .Select-menu-outer { z-index: 10 !important; } diff --git a/superset/assets/visualizations/table.css b/superset/assets/visualizations/table.css index 84c7e27351..bc62227ba0 100644 --- a/superset/assets/visualizations/table.css +++ b/superset/assets/visualizations/table.css @@ -27,3 +27,7 @@ table.table thead th.sorting:after, table.table thead th.sorting_asc:after, tabl .like-pre { white-space: pre-wrap; } + +.widget.table thead tr { + height: 25px; +} diff --git a/superset/models/core.py b/superset/models/core.py index e38c8cde46..9a38b25a1f 100644 --- a/superset/models/core.py +++ b/superset/models/core.py @@ -120,6 +120,17 @@ class Slice(Model, AuditMixinNullable, ImportMixin): def datasource(self): return self.get_datasource + def clone(self): + return Slice( + slice_name=self.slice_name, + datasource_id=self.datasource_id, + datasource_type=self.datasource_type, + datasource_name=self.datasource_name, + viz_type=self.viz_type, + params=self.params, + description=self.description, + cache_timeout=self.cache_timeout) + @datasource.getter @utils.memoized def get_datasource(self): diff --git a/superset/views/core.py b/superset/views/core.py index 65d3feb500..68e82027d0 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -1295,13 +1295,28 @@ class Superset(BaseSupersetView): session = db.session() data = json.loads(request.form.get('data')) dash = models.Dashboard() - original_dash = (session - .query(models.Dashboard) - .filter_by(id=dashboard_id).first()) + original_dash = ( + session + .query(models.Dashboard) + .filter_by(id=dashboard_id).first()) dash.owners = [g.user] if g.user else [] dash.dashboard_title = data['dashboard_title'] - dash.slices = original_dash.slices + if data['duplicate_slices']: + # Duplicating slices as well, mapping old ids to new ones + old_to_new_sliceids = {} + for slc in original_dash.slices: + new_slice = slc.clone() + new_slice.owners = [g.user] if g.user else [] + session.add(new_slice) + session.flush() + new_slice.dashboards.append(dash) + old_to_new_sliceids['{}'.format(slc.id)] =\ + '{}'.format(new_slice.id) + for d in data['positions']: + d['slice_id'] = old_to_new_sliceids[d['slice_id']] + else: + dash.slices = original_dash.slices dash.params = original_dash.params self._set_dash_metadata(dash, data) diff --git a/tests/core_tests.py b/tests/core_tests.py index 245bd880ea..34f30a14f5 100644 --- a/tests/core_tests.py +++ b/tests/core_tests.py @@ -467,6 +467,7 @@ class CoreTests(SupersetTestCase): positions.append(d) data = { 'css': '', + 'duplicate_slices': False, 'expanded_slices': {}, 'positions': positions, 'dashboard_title': 'Copy Of Births',