mirror of
https://github.com/elastic/kibana.git
synced 2025-04-24 01:38:56 -04:00
* 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:
parent
742f9b58c6
commit
470004d66b
8 changed files with 133 additions and 43 deletions
|
@ -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());
|
||||
|
||||
|
|
|
@ -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,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"
|
||||
>
|
||||
|
|
|
@ -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>
|
||||
|
|
|
@ -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();
|
||||
}
|
||||
|
|
|
@ -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);
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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');
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue