From 8329ea2b9bbaa385ef7077d7ba8847dd69543a88 Mon Sep 17 00:00:00 2001 From: Grace Guo Date: Wed, 14 Jun 2017 12:52:12 -0700 Subject: [PATCH] Edit Dashboard title and Slice title in place (#2940) * Edit Dashboard title and Slice title in place Add EditableTitle component into Dashboard and Explore view to support edit title inline. --- .../javascripts/components/EditableTitle.jsx | 84 ++++++++++++++++++ .../javascripts/dashboard/Dashboard.jsx | 4 + .../dashboard/components/Header.jsx | 13 ++- .../dashboard/components/SaveModal.jsx | 1 + .../explore/actions/exploreActions.js | 14 ++- .../explore/components/ChartContainer.jsx | 26 +++++- .../explore/components/SaveModal.jsx | 6 +- .../javascripts/explore/exploreUtils.js | 11 ++- .../explore/reducers/exploreReducer.js | 7 ++ .../profile/EditableTitle_spec.jsx | 85 +++++++++++++++++++ superset/assets/stylesheets/superset.css | 20 ++++- superset/views/core.py | 1 + tests/core_tests.py | 45 +++++++++- 13 files changed, 302 insertions(+), 15 deletions(-) create mode 100644 superset/assets/javascripts/components/EditableTitle.jsx create mode 100644 superset/assets/spec/javascripts/profile/EditableTitle_spec.jsx diff --git a/superset/assets/javascripts/components/EditableTitle.jsx b/superset/assets/javascripts/components/EditableTitle.jsx new file mode 100644 index 0000000000000..70046f87ca1e2 --- /dev/null +++ b/superset/assets/javascripts/components/EditableTitle.jsx @@ -0,0 +1,84 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import TooltipWrapper from './TooltipWrapper'; + +const propTypes = { + title: PropTypes.string, + canEdit: PropTypes.bool, + onSaveTitle: PropTypes.func.isRequired, +}; +const defaultProps = { + title: 'Title', + canEdit: false, +}; + +class EditableTitle extends React.PureComponent { + constructor(props) { + super(props); + this.state = { + isEditing: false, + title: this.props.title, + lastTitle: this.props.title, + }; + this.handleClick = this.handleClick.bind(this); + this.handleBlur = this.handleBlur.bind(this); + this.handleChange = this.handleChange.bind(this); + } + handleClick() { + if (!this.props.canEdit) { + return; + } + + this.setState({ + isEditing: true, + }); + } + handleBlur() { + if (!this.props.canEdit) { + return; + } + + this.setState({ + isEditing: false, + }); + + if (this.state.lastTitle !== this.state.title) { + this.setState({ + lastTitle: this.state.title, + }); + this.props.onSaveTitle(this.state.title); + } + } + handleChange(ev) { + if (!this.props.canEdit) { + return; + } + + this.setState({ + title: ev.target.value, + }); + } + render() { + return ( + + + + + + ); + } +} +EditableTitle.propTypes = propTypes; +EditableTitle.defaultProps = defaultProps; + +export default EditableTitle; diff --git a/superset/assets/javascripts/dashboard/Dashboard.jsx b/superset/assets/javascripts/dashboard/Dashboard.jsx index 3707458efe439..8b0a0e1c69114 100644 --- a/superset/assets/javascripts/dashboard/Dashboard.jsx +++ b/superset/assets/javascripts/dashboard/Dashboard.jsx @@ -341,6 +341,10 @@ export function dashboardContainer(dashboard, datasources, userid) { }, }); }, + updateDashboardTitle(title) { + this.dashboard_title = title; + this.onChange(); + }, }); } diff --git a/superset/assets/javascripts/dashboard/components/Header.jsx b/superset/assets/javascripts/dashboard/components/Header.jsx index 44259b928a0e5..740b102f1df9d 100644 --- a/superset/assets/javascripts/dashboard/components/Header.jsx +++ b/superset/assets/javascripts/dashboard/components/Header.jsx @@ -2,6 +2,7 @@ import React from 'react'; import PropTypes from 'prop-types'; import Controls from './Controls'; +import EditableTitle from '../../components/EditableTitle'; const propTypes = { dashboard: PropTypes.object, @@ -14,14 +15,22 @@ class Header extends React.PureComponent { super(props); this.state = { }; + this.handleSaveTitle = this.handleSaveTitle.bind(this); + } + handleSaveTitle(title) { + this.props.dashboard.updateDashboardTitle(title); } render() { const dashboard = this.props.dashboard; return (
-

- {dashboard.dashboard_title}   +

+

diff --git a/superset/assets/javascripts/dashboard/components/SaveModal.jsx b/superset/assets/javascripts/dashboard/components/SaveModal.jsx index 8ae3b964c8fc6..a22f4ac7725e0 100644 --- a/superset/assets/javascripts/dashboard/components/SaveModal.jsx +++ b/superset/assets/javascripts/dashboard/components/SaveModal.jsx @@ -79,6 +79,7 @@ class SaveModal extends React.PureComponent { positions, css: this.state.css, expanded_slices: expandedSlices, + dashboard_title: dashboard.dashboard_title, }; let url = null; if (saveType === 'overwrite') { diff --git a/superset/assets/javascripts/explore/actions/exploreActions.js b/superset/assets/javascripts/explore/actions/exploreActions.js index 3f683243fb1a0..c8611443ddab6 100644 --- a/superset/assets/javascripts/explore/actions/exploreActions.js +++ b/superset/assets/javascripts/explore/actions/exploreActions.js @@ -212,6 +212,10 @@ export const SAVE_SLICE_FAILED = 'SAVE_SLICE_FAILED'; export function saveSliceFailed() { return { type: SAVE_SLICE_FAILED }; } +export const SAVE_SLICE_SUCCESS = 'SAVE_SLICE_SUCCESS'; +export function saveSliceSuccess(data) { + return { type: SAVE_SLICE_SUCCESS, data }; +} export const REMOVE_SAVE_MODAL_ALERT = 'REMOVE_SAVE_MODAL_ALERT'; export function removeSaveModalAlert() { @@ -220,10 +224,9 @@ export function removeSaveModalAlert() { export function saveSlice(url) { return function (dispatch) { - $.get(url, (data, status) => { + return $.get(url, (data, status) => { if (status === 'success') { - // Go to new slice url or dashboard url - window.location = data; + dispatch(saveSliceSuccess(data)); } else { dispatch(saveSliceFailed()); } @@ -231,6 +234,11 @@ export function saveSlice(url) { }; } +export const UPDATE_CHART_TITLE = 'UPDATE_CHART_TITLE'; +export function updateChartTitle(slice_name) { + return { type: UPDATE_CHART_TITLE, slice_name }; +} + export const UPDATE_CHART_STATUS = 'UPDATE_CHART_STATUS'; export function updateChartStatus(status) { return { type: UPDATE_CHART_STATUS, status }; diff --git a/superset/assets/javascripts/explore/components/ChartContainer.jsx b/superset/assets/javascripts/explore/components/ChartContainer.jsx index 2070b3db8b212..f6da538843eef 100644 --- a/superset/assets/javascripts/explore/components/ChartContainer.jsx +++ b/superset/assets/javascripts/explore/components/ChartContainer.jsx @@ -7,6 +7,7 @@ import { Alert, Collapse, Panel } from 'react-bootstrap'; import visMap from '../../../visualizations/main'; import { d3format } from '../../modules/utils'; import ExploreActionButtons from './ExploreActionButtons'; +import EditableTitle from '../../components/EditableTitle'; import FaveStar from '../../components/FaveStar'; import TooltipWrapper from '../../components/TooltipWrapper'; import Timer from '../../components/Timer'; @@ -23,6 +24,7 @@ const CHART_STATUS_MAP = { const propTypes = { actions: PropTypes.object.isRequired, alert: PropTypes.string, + can_overwrite: PropTypes.bool.isRequired, can_download: PropTypes.bool.isRequired, chartStatus: PropTypes.string, chartUpdateEndTime: PropTypes.number, @@ -39,6 +41,8 @@ const propTypes = { queryResponse: PropTypes.object, triggerRender: PropTypes.bool, standalone: PropTypes.bool, + datasourceType: PropTypes.string, + datasourceId: PropTypes.number, }; class ChartContainer extends React.PureComponent { @@ -145,6 +149,18 @@ class ChartContainer extends React.PureComponent { this.props.actions.runQuery(this.props.formData, true); } + updateChartTitle(newTitle) { + const params = { + slice_name: newTitle, + action: 'overwrite', + }; + const saveUrl = getExploreUrl(this.props.formData, 'base', false, null, params); + this.props.actions.saveSlice(saveUrl) + .then(() => { + this.props.actions.updateChartTitle(newTitle); + }); + } + renderChartTitle() { let title; if (this.props.slice) { @@ -240,7 +256,11 @@ class ChartContainer extends React.PureComponent { id="slice-header" className="clearfix panel-title-large" > - {this.renderChartTitle()} + {this.props.slice && @@ -304,6 +324,7 @@ function mapStateToProps(state) { const formData = getFormDataFromControls(state.controls); return { alert: state.chartAlert, + can_overwrite: state.can_overwrite, can_download: state.can_download, chartStatus: state.chartStatus, chartUpdateEndTime: state.chartUpdateEndTime, @@ -320,7 +341,8 @@ function mapStateToProps(state) { table_name: formData.datasource_name, viz_type: formData.viz_type, triggerRender: state.triggerRender, - datasourceType: state.datasource ? state.datasource.type : null, + datasourceType: state.datasource_type, + datasourceId: state.datasource_id, }; } diff --git a/superset/assets/javascripts/explore/components/SaveModal.jsx b/superset/assets/javascripts/explore/components/SaveModal.jsx index 35ad0c98ad519..4dbff36acfea6 100644 --- a/superset/assets/javascripts/explore/components/SaveModal.jsx +++ b/superset/assets/javascripts/explore/components/SaveModal.jsx @@ -108,7 +108,11 @@ class SaveModal extends React.Component { const saveUrl = `${baseUrl}?form_data=` + `${encodeURIComponent(JSON.stringify(this.props.form_data))}` + `&${$.param(sliceParams, true)}`; - this.props.actions.saveSlice(saveUrl); + this.props.actions.saveSlice(saveUrl) + .then((data) => { + // Go to new slice url or dashboard url + window.location = data; + }); this.props.onHide(); } removeAlert() { diff --git a/superset/assets/javascripts/explore/exploreUtils.js b/superset/assets/javascripts/explore/exploreUtils.js index 5951ff61a2e6c..2356da5eb6256 100644 --- a/superset/assets/javascripts/explore/exploreUtils.js +++ b/superset/assets/javascripts/explore/exploreUtils.js @@ -1,7 +1,8 @@ /* eslint camelcase: 0 */ import URI from 'urijs'; -export function getExploreUrl(form_data, endpointType = 'base', force = false, curUrl = null) { +export function getExploreUrl(form_data, endpointType = 'base', force = false, + curUrl = null, requestParams = {}) { if (!form_data.datasource) { return null; } @@ -38,6 +39,14 @@ export function getExploreUrl(form_data, endpointType = 'base', force = false, c if (endpointType === 'query') { search.query = 'true'; } + const paramNames = Object.keys(requestParams); + if (paramNames.length) { + paramNames.forEach((name) => { + if (requestParams.hasOwnProperty(name)) { + search[name] = requestParams[name]; + } + }); + } uri = uri.search(search).directory(directory); return uri.toString(); } diff --git a/superset/assets/javascripts/explore/reducers/exploreReducer.js b/superset/assets/javascripts/explore/reducers/exploreReducer.js index f58d21a531fe2..cc214581c6b4a 100644 --- a/superset/assets/javascripts/explore/reducers/exploreReducer.js +++ b/superset/assets/javascripts/explore/reducers/exploreReducer.js @@ -135,6 +135,10 @@ export const exploreReducer = function (state, action) { } return newState; }, + [actions.UPDATE_CHART_TITLE]() { + const updatedSlice = Object.assign({}, state.slice, { slice_name: action.slice_name }); + return Object.assign({}, state, { slice: updatedSlice }); + }, [actions.REMOVE_CHART_ALERT]() { if (state.chartAlert !== null) { return Object.assign({}, state, { chartAlert: null }); @@ -144,6 +148,9 @@ export const exploreReducer = function (state, action) { [actions.SAVE_SLICE_FAILED]() { return Object.assign({}, state, { saveModalAlert: 'Failed to save slice' }); }, + [actions.SAVE_SLICE_SUCCESS](data) { + return Object.assign({}, state, { data }); + }, [actions.REMOVE_SAVE_MODAL_ALERT]() { return Object.assign({}, state, { saveModalAlert: null }); }, diff --git a/superset/assets/spec/javascripts/profile/EditableTitle_spec.jsx b/superset/assets/spec/javascripts/profile/EditableTitle_spec.jsx new file mode 100644 index 0000000000000..edce86a0245a9 --- /dev/null +++ b/superset/assets/spec/javascripts/profile/EditableTitle_spec.jsx @@ -0,0 +1,85 @@ +import React from 'react'; +import { shallow } from 'enzyme'; +import { describe, it } from 'mocha'; +import sinon from 'sinon'; +import { expect } from 'chai'; + +import EditableTable from '../../../javascripts/components/EditableTitle'; + +describe('EditableTitle', () => { + const callback = sinon.spy(); + const mockProps = { + title: 'my title', + canEdit: true, + onSaveTitle: callback, + }; + const mockEvent = { + target: { + value: 'new title', + }, + }; + const editableWrapper = shallow(); + const notEditableWrapper = shallow(); + it('is valid', () => { + expect( + React.isValidElement(), + ).to.equal(true); + }); + it('should render title', () => { + const titleElement = editableWrapper.find('input'); + expect(titleElement.props().value).to.equal('my title'); + expect(titleElement.props().type).to.equal('button'); + }); + + describe('should handle click', () => { + it('should change title', () => { + editableWrapper.find('input').simulate('click'); + expect(editableWrapper.find('input').props().type).to.equal('text'); + }); + it('should not change title', () => { + notEditableWrapper.find('input').simulate('click'); + expect(notEditableWrapper.find('input').props().type).to.equal('button'); + }); + }); + + describe('should handle change', () => { + afterEach(() => { + editableWrapper.setState({ title: 'my title' }); + editableWrapper.setState({ lastTitle: 'my title' }); + }); + it('should change title', () => { + editableWrapper.find('input').simulate('change', mockEvent); + expect(editableWrapper.find('input').props().value).to.equal('new title'); + }); + it('should not change title', () => { + notEditableWrapper.find('input').simulate('change', mockEvent); + expect(editableWrapper.find('input').props().value).to.equal('my title'); + }); + }); + + describe('should handle blur', () => { + beforeEach(() => { + editableWrapper.find('input').simulate('click'); + expect(editableWrapper.find('input').props().type).to.equal('text'); + }); + afterEach(() => { + callback.reset(); + editableWrapper.setState({ title: 'my title' }); + editableWrapper.setState({ lastTitle: 'my title' }); + }); + + it('should trigger callback', () => { + editableWrapper.setState({ title: 'new title' }); + editableWrapper.find('input').simulate('blur'); + expect(editableWrapper.find('input').props().type).to.equal('button'); + expect(callback.callCount).to.equal(1); + expect(callback.getCall(0).args[0]).to.equal('new title'); + }); + it('should not trigger callback', () => { + editableWrapper.find('input').simulate('blur'); + expect(editableWrapper.find('input').props().type).to.equal('button'); + // no change + expect(callback.callCount).to.equal(0); + }); + }); +}); diff --git a/superset/assets/stylesheets/superset.css b/superset/assets/stylesheets/superset.css index 9db4ffde6a9e0..d0b16cad2885b 100644 --- a/superset/assets/stylesheets/superset.css +++ b/superset/assets/stylesheets/superset.css @@ -199,7 +199,7 @@ div.widget .slice_container { .navbar .alert { padding: 5px 10px; margin-top: 8px; - margin-bottom: 0px + margin-bottom: 0; } .table-condensed { @@ -209,6 +209,22 @@ div.widget .slice_container { .table-condensed input[type="checkbox"] { float: left; } -.m-r-5 { + +.editable-title input { + padding: 2px 6px 3px 6px; +} + +.editable-title input[type="button"] { + border-color: transparent; + background: inherit; +} + +.editable-title input[type="button"]:hover { + cursor: text; +} + +.editable-title input[type="button"]:focus { + outline: none; +}.m-r-5 { margin-right: 5px; } diff --git a/superset/views/core.py b/superset/views/core.py index c96308fd44558..1740cb0ae8df7 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -1320,6 +1320,7 @@ def _set_dash_metadata(dashboard, data): dashboard.position_json = json.dumps(positions, indent=4, sort_keys=True) md = dashboard.params_dict dashboard.css = data['css'] + dashboard.dashboard_title = data['dashboard_title'] if 'filter_immune_slices' not in md: md['filter_immune_slices'] = [] diff --git a/tests/core_tests.py b/tests/core_tests.py index 5155d6e1217bf..7ff48a9081a8c 100644 --- a/tests/core_tests.py +++ b/tests/core_tests.py @@ -372,10 +372,46 @@ def test_save_dash(self, username='admin'): 'css': '', 'expanded_slices': {}, 'positions': positions, + 'dashboard_title': dash.dashboard_title } url = '/superset/save_dash/{}/'.format(dash.id) - resp = self.client.post(url, data=dict(data=json.dumps(data))) - assert "SUCCESS" in resp.data.decode('utf-8') + resp = self.get_resp(url, data=dict(data=json.dumps(data))) + self.assertIn("SUCCESS", resp) + + def test_save_dash_with_dashboard_title(self, username='admin'): + self.login(username=username) + dash = ( + db.session.query(models.Dashboard) + .filter_by(slug="births") + .first() + ) + origin_title = dash.dashboard_title + positions = [] + for i, slc in enumerate(dash.slices): + d = { + 'col': 0, + 'row': i * 4, + 'size_x': 4, + 'size_y': 4, + 'slice_id': '{}'.format(slc.id)} + positions.append(d) + data = { + 'css': '', + 'expanded_slices': {}, + 'positions': positions, + 'dashboard_title': 'new title' + } + url = '/superset/save_dash/{}/'.format(dash.id) + resp = self.get_resp(url, data=dict(data=json.dumps(data))) + updatedDash = ( + db.session.query(models.Dashboard) + .filter_by(slug="births") + .first() + ) + self.assertEqual(updatedDash.dashboard_title, 'new title') + # # bring back dashboard original title + data['dashboard_title'] = origin_title + self.get_resp(url, data=dict(data=json.dumps(data))) def test_copy_dash(self, username='admin'): self.login(username=username) @@ -510,7 +546,8 @@ def test_dashboard_with_created_by_can_be_accessed_by_public_users(self): ) self.grant_public_access_to_table(table) - dash = db.session.query(models.Dashboard).filter_by(dashboard_title="Births").first() + dash = db.session.query(models.Dashboard).filter_by( + slug="births").first() dash.owners = [appbuilder.sm.find_user('admin')] dash.created_by = appbuilder.sm.find_user('admin') db.session.merge(dash) @@ -532,7 +569,7 @@ def test_only_owners_can_save(self): self.logout() self.assertRaises( - AssertionError, self.test_save_dash, 'alpha') + Exception, self.test_save_dash, 'alpha') alpha = appbuilder.sm.find_user('alpha')