maxmustermann/kibana
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

remove double modal when cloning dashboard with duplicate title (#19049)

Browse source 
* remove double modal when cloning dashboard with duplicate title

* reset duplicate state when input is updated

* update functional test for new clone workflow

* change warning message to EuiCallout

* updates from Stacey-Gammon review
This commit is contained in:
Nathan Reese 2018-05-22 10:29:19 -06:00 committed by GitHub
parent 7c27a53d2a
commit 760f849ac5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 133 additions and 43 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

27
src/core_plugins/kibana/public/dashboard/dashboard_app.js
View file

@ -269,8 +269,20 @@ app.directive('dashboardApp', function ($injector) {
);
};
$scope.save = function () {
return saveDashboard(angular.toJson, timefilter, dashboardStateManager)
/**
* Saves the dashboard.
*
* @param {object} [saveOptions={}]
* @property {boolean} [saveOptions.confirmOverwrite=false] - If true, attempts to create the source so it
* can confirm an overwrite if a document with the id already exists.
* @property {boolean} [saveOptions.isTitleDuplicateConfirmed=false] - If true, save allowed with duplicate title
* @property {func} [saveOptions.onTitleDuplicate] - function called if duplicate title exists.
* When not provided, confirm modal will be displayed asking user to confirm or cancel save.
* @return {Promise}
* @resolved {String} - The id of the doc
*/
$scope.save = function (saveOptions) {
return saveDashboard(angular.toJson, timefilter, dashboardStateManager, saveOptions)
.then(function (id) {
$scope.kbnTopNav.close('save');
if (id) {
@ -307,10 +319,15 @@ app.directive('dashboardApp', function ($injector) {
navActions[TopNavIds.ENTER_EDIT_MODE] = () => onChangeViewMode(DashboardViewMode.EDIT);
navActions[TopNavIds.CLONE] = () => {
const currentTitle = $scope.model.title;
const onClone = (newTitle) => {
const onClone = (newTitle, isTitleDuplicateConfirmed, onTitleDuplicate) => {
dashboardStateManager.savedDashboard.copyOnSave = true;
dashboardStateManager.setTitle(newTitle);
return $scope.save().then(id => {
const saveOptions = {
confirmOverwrite: false,
isTitleDuplicateConfirmed,
onTitleDuplicate,
};
return $scope.save(saveOptions).then(id => {
// If the save wasn't successful, put the original title back.
if (!id) {
$scope.model.title = currentTitle;
@ -322,7 +339,7 @@ app.directive('dashboardApp', function ($injector) {
});
};
showCloneModal(onClone, currentTitle, $rootScope, $compile);
showCloneModal(onClone, currentTitle);
};
updateViewMode(dashboardStateManager.getViewMode());

10
src/core_plugins/kibana/public/dashboard/lib/save_dashboard.js
View file

@ -7,10 +7,16 @@ import { updateSavedDashboard } from './update_saved_dashboard';
* JSON.stringify
* @param timeFilter
* @param dashboardStateManager {DashboardStateManager}
* @param {object} [saveOptions={}]
* @property {boolean} [saveOptions.confirmOverwrite=false] - If true, attempts to create the source so it
* can confirm an overwrite if a document with the id already exists.
* @property {boolean} [saveOptions.isTitleDuplicateConfirmed=false] - If true, save allowed with duplicate title
* @property {func} [saveOptions.onTitleDuplicate] - function called if duplicate title exists.
* When not provided, confirm modal will be displayed asking user to confirm or cancel save.
* @returns {Promise<string>} A promise that if resolved, will contain the id of the newly saved
* dashboard.
*/
export function saveDashboard(toJson, timeFilter, dashboardStateManager) {
export function saveDashboard(toJson, timeFilter, dashboardStateManager, saveOptions) {
dashboardStateManager.saveState();
const savedDashboard = dashboardStateManager.savedDashboard;
@ -18,7 +24,7 @@ export function saveDashboard(toJson, timeFilter, dashboardStateManager) {
updateSavedDashboard(savedDashboard, appState, timeFilter, toJson);
return savedDashboard.save()
return savedDashboard.save(saveOptions)
.then((id) => {
dashboardStateManager.lastSavedDashboardFilters = dashboardStateManager.getFilterState();
dashboardStateManager.resetState();

4
src/core_plugins/kibana/public/dashboard/top_nav/__snapshots__/clone_modal.test.js.snap
View file

@ -4,7 +4,7 @@ exports[`renders DashboardCloneModal 1`] = `
<EuiOverlayMask>
<EuiModal
className="dashboardCloneModal"
data-tests-subj="dashboardCloneModal"
data-test-subj="dashboardCloneModal"
onClose={[Function]}
>
<EuiModalHeader>
@ -28,6 +28,7 @@ exports[`renders DashboardCloneModal 1`] = `
compressed={false}
data-test-subj="clonedDashboardTitle"
fullWidth={false}
isInvalid={false}
isLoading={false}
onChange={[Function]}
value="dash title"
@ -49,6 +50,7 @@ exports[`renders DashboardCloneModal 1`] = `
data-test-subj="cloneConfirmButton"
fill={true}
iconSide="left"
isLoading={false}
onClick={[Function]}
type="button"
>

70
src/core_plugins/kibana/public/dashboard/top_nav/clone_modal.js
View file

@ -1,4 +1,4 @@
import React from 'react';
import React, { Fragment } from 'react';
import PropTypes from 'prop-types';
import {
@ -12,6 +12,7 @@ import {
EuiOverlayMask,
EuiSpacer,
EuiText,
EuiCallOut,
} from '@elastic/eui';
export class DashboardCloneModal extends React.Component {
@ -19,23 +20,75 @@ export class DashboardCloneModal extends React.Component {
super(props);
this.state = {
newDashboardName: props.title
newDashboardName: props.title,
isTitleDuplicateConfirmed: false,
hasTitleDuplicate: false,
isLoading: false,
};
}
componentDidMount() {
this._isMounted = true;
}
cloneDashboard = () => {
this.props.onClone(this.state.newDashboardName);
componentWillUnmount() {
this._isMounted = false;
}
onTitleDuplicate = () => {
this.setState({
isTitleDuplicateConfirmed: true,
hasTitleDuplicate: true,
});
}
cloneDashboard = async () => {
this.setState({
isLoading: true,
});
await this.props.onClone(this.state.newDashboardName, this.state.isTitleDuplicateConfirmed, this.onTitleDuplicate);
if (this._isMounted) {
this.setState({
isLoading: false,
});
}
};
onInputChange = (event) => {
this.setState({ newDashboardName: event.target.value });
this.setState({
newDashboardName: event.target.value,
isTitleDuplicateConfirmed: false,
hasTitleDuplicate: false,
});
};
renderDuplicateTitleCallout = () => {
if (!this.state.hasTitleDuplicate) {
return;
}
return (
<Fragment>
<EuiCallOut
title={`A Dashboard with the title '${this.state.newDashboardName}' already exists.`}
color="warning"
data-test-subj="cloneModalTitleDupicateWarnMsg"
>
<p>
Click <strong>Confirm Clone</strong> to clone the dashboard with the duplicate title.
</p>
</EuiCallOut>
<EuiSpacer />
</Fragment>
);
}
render() {
return (
<EuiOverlayMask>
<EuiModal
data-tests-subj="dashboardCloneModal"
data-test-subj="dashboardCloneModal"
className="dashboardCloneModal"
onClose={this.props.onClose}
>
@ -54,12 +107,16 @@ export class DashboardCloneModal extends React.Component {
<EuiSpacer />
{this.renderDuplicateTitleCallout()}
<EuiFieldText
autoFocus
data-test-subj="clonedDashboardTitle"
value={this.state.newDashboardName}
onChange={this.onInputChange}
isInvalid={this.state.hasTitleDuplicate}
/>
</EuiModalBody>
<EuiModalFooter>
@ -74,6 +131,7 @@ export class DashboardCloneModal extends React.Component {
fill
data-test-subj="cloneConfirmButton"
onClick={this.cloneDashboard}
isLoading={this.state.isLoading}
>
Confirm Clone
</EuiButton>

4
src/core_plugins/kibana/public/dashboard/top_nav/show_clone_modal.js
View file

@ -9,8 +9,8 @@ export function showCloneModal(onClone, title) {
document.body.removeChild(container);
};
const onCloneConfirmed = (newTitle) => {
onClone(newTitle).then(id => {
const onCloneConfirmed = (newTitle, isTitleDuplicateConfirmed, onTitleDuplicate) => {
onClone(newTitle, isTitleDuplicateConfirmed, onTitleDuplicate).then(id => {
if (id) {
closeModal();
}

39
src/ui/public/courier/saved_object/saved_object.js
View file

@ -283,12 +283,21 @@ export function SavedObjectProvider(Promise, Private, Notifier, confirmModalProm
});
};
/**
* Returns a promise that resolves to true if either the title is unique, or if the user confirmed they
* wished to save the duplicate title. Promise is rejected if the user rejects the confirmation.
*/
const warnIfDuplicateTitle = () => {
// Don't warn if the user isn't updating the title, otherwise that would become very annoying to have
const displayDuplicateTitleConfirmModal = () => {
const confirmMessage =
`A ${this.getDisplayName()} with the title '${this.title}' already exists. Would you like to save anyway?`;
return confirmModalPromise(confirmMessage, { confirmButtonText: `Save ${this.getDisplayName()}` })
.catch(() => Promise.reject(new Error(SAVE_DUPLICATE_REJECTED)));
};
const checkForDuplicateTitle = (isTitleDuplicateConfirmed, onTitleDuplicate) => {
// Don't check for duplicates if user has already confirmed save with duplicate title
if (isTitleDuplicateConfirmed) {
return Promise.resolve();
}
// Don't check if the user isn't updating the title, otherwise that would become very annoying to have
// to confirm the save every time, except when copyOnSave is true, then we do want to check.
if (this.title === this.lastSavedTitle && !this.copyOnSave) {
return Promise.resolve();
@ -299,11 +308,14 @@ export function SavedObjectProvider(Promise, Private, Notifier, confirmModalProm
if (!duplicate) return true;
if (duplicate.id === this.id) return true;
const confirmMessage =
`A ${this.getDisplayName()} with the title '${this.title}' already exists. Would you like to save anyway?`;
if (onTitleDuplicate) {
onTitleDuplicate();
return Promise.reject(new Error(SAVE_DUPLICATE_REJECTED));
}
return confirmModalPromise(confirmMessage, { confirmButtonText: `Save ${this.getDisplayName()}` })
.catch(() => Promise.reject(new Error(SAVE_DUPLICATE_REJECTED)));
// TODO: make onTitleDuplicate a required prop and remove UI components from this class
// Need to leave here until all users pass onTitleDuplicate.
return displayDuplicateTitleConfirmModal();
});
};
@ -313,10 +325,13 @@ export function SavedObjectProvider(Promise, Private, Notifier, confirmModalProm
* @param {object} [options={}]
* @property {boolean} [options.confirmOverwrite=false] - If true, attempts to create the source so it
* can confirm an overwrite if a document with the id already exists.
* @property {boolean} [options.isTitleDuplicateConfirmed=false] - If true, save allowed with duplicate title
* @property {func} [options.onTitleDuplicate] - function called if duplicate title exists.
* When not provided, confirm modal will be displayed asking user to confirm or cancel save.
* @return {Promise}
* @resolved {String} - The id of the doc
*/
this.save = ({ confirmOverwrite } = {}) => {
this.save = ({ confirmOverwrite = false, isTitleDuplicateConfirmed = false, onTitleDuplicate } = {}) => {
// Save the original id in case the save fails.
const originalId = this.id;
// Read https://github.com/elastic/kibana/issues/9056 and
@ -333,7 +348,7 @@ export function SavedObjectProvider(Promise, Private, Notifier, confirmModalProm
this.isSaving = true;
return warnIfDuplicateTitle()
return checkForDuplicateTitle(isTitleDuplicateConfirmed, onTitleDuplicate)
.then(() => {
if (confirmOverwrite) {
return createSource(source);

18
test/functional/apps/dashboard/_dashboard_clone.js
View file

@ -42,23 +42,11 @@ export default function ({ getService, getPageObjects }) {
it('and warns on duplicate name', async function () {
await PageObjects.dashboard.confirmClone();
const isConfirmOpen = await PageObjects.common.isConfirmModalOpen();
expect(isConfirmOpen).to.equal(true);
});
it('preserves the original title on cancel', async function () {
await PageObjects.common.clickCancelOnModal();
await PageObjects.dashboard.confirmClone();
await retry.try(async () => {
// Should see the same confirmation if the title is the same.
const isConfirmOpen = await PageObjects.common.isConfirmModalOpen();
expect(isConfirmOpen).to.equal(true);
});
const isWarningDisplayed = await PageObjects.dashboard.isCloneDuplicateTitleWarningDisplayed();
expect(isWarningDisplayed).to.equal(true);
});
it('and doesn\'t save', async () => {
await PageObjects.common.clickCancelOnModal();
await PageObjects.dashboard.cancelClone();
const countOfDashboards = await PageObjects.dashboard.getDashboardCountWithName(dashboardName);
@ -70,7 +58,7 @@ export default function ({ getService, getPageObjects }) {
await PageObjects.dashboard.clickClone();
await PageObjects.dashboard.confirmClone();
await PageObjects.common.clickConfirmOnModal();
await PageObjects.dashboard.confirmClone();
// This is important since saving a new dashboard will cause a refresh of the page. We have to
// wait till it finishes reloading or it might reload the url after simulating the

4
test/functional/page_objects/dashboard_page.js
View file

@ -162,6 +162,10 @@ export function DashboardPageProvider({ getService, getPageObjects }) {
await testSubjects.setValue('clonedDashboardTitle', title);
}
async isCloneDuplicateTitleWarningDisplayed() {
return await testSubjects.exists('cloneModalTitleDupicateWarnMsg');
}
async clickEdit() {
log.debug('Clicking edit');
return await testSubjects.click('dashboardEditMode');