From f4a76df890e02a0e7e6e319399705a3869b3b0b1 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Mon, 5 Jun 2023 14:52:36 -0500 Subject: [PATCH 01/12] remove custom empty button label for annotations --- .../config_panel/buttons/empty_dimension_button.tsx | 5 ++--- x-pack/plugins/lens/public/types.ts | 1 - .../public/visualizations/xy/annotations/helpers.tsx | 11 ----------- 3 files changed, 2 insertions(+), 15 deletions(-) diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/buttons/empty_dimension_button.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/buttons/empty_dimension_button.tsx index f0ef1508c5076..79c2db0d01268 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/buttons/empty_dimension_button.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/buttons/empty_dimension_button.tsx @@ -54,7 +54,6 @@ const defaultButtonLabels = { }; const DefaultEmptyButton = ({ columnId, group, onClick }: EmptyButtonProps) => { - const { buttonAriaLabel, buttonLabel } = group.labels || {}; return ( { contentProps={{ className: 'lnsLayerPanel__triggerTextContent', }} - aria-label={buttonAriaLabel || defaultButtonLabels.ariaLabel(group.groupLabel)} + aria-label={defaultButtonLabels.ariaLabel(group.groupLabel)} data-test-subj="lns-empty-dimension" onClick={() => { onClick(columnId); }} > - {buttonLabel || defaultButtonLabels.label} + {defaultButtonLabels.label} ); }; diff --git a/x-pack/plugins/lens/public/types.ts b/x-pack/plugins/lens/public/types.ts index 70efe28d0852d..ce5a3f5b7a736 100644 --- a/x-pack/plugins/lens/public/types.ts +++ b/x-pack/plugins/lens/public/types.ts @@ -845,7 +845,6 @@ export type VisualizationDimensionGroupConfig = SharedDimensionProps & { paramEditorCustomProps?: ParamEditorCustomProps; enableFormatSelector?: boolean; formatSelectorOptions?: FormatSelectorOptions; // only relevant if supportFieldFormat is true - labels?: { buttonAriaLabel: string; buttonLabel: string }; }; export interface VisualizationDimensionChangeProps { diff --git a/x-pack/plugins/lens/public/visualizations/xy/annotations/helpers.tsx b/x-pack/plugins/lens/public/visualizations/xy/annotations/helpers.tsx index 6174017f3054b..630428568535a 100644 --- a/x-pack/plugins/lens/public/visualizations/xy/annotations/helpers.tsx +++ b/x-pack/plugins/lens/public/visualizations/xy/annotations/helpers.tsx @@ -478,16 +478,6 @@ export const getAnnotationsConfiguration = ({ }) => { const groupLabel = getAxisName('x', { isHorizontal: isHorizontalChart(state.layers) }); - const emptyButtonLabels = { - buttonAriaLabel: i18n.translate('xpack.lens.indexPattern.addColumnAriaLabelClick', { - defaultMessage: 'Add an annotation to {groupLabel}', - values: { groupLabel }, - }), - buttonLabel: i18n.translate('xpack.lens.configure.emptyConfigClick', { - defaultMessage: 'Add an annotation', - }), - }; - return { groups: [ { @@ -507,7 +497,6 @@ export const getAnnotationsConfiguration = ({ supportFieldFormat: false, enableDimensionEditor: true, filterOperations: () => false, - labels: emptyButtonLabels, }, ], }; From 1cebef8dc28d929edde18ee53145734236c70962 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Mon, 5 Jun 2023 15:04:00 -0500 Subject: [PATCH 02/12] vertically position annotation saved object finder --- ...t_annotation_group_saved_object_finder.tsx | 56 ++++++++++--------- .../xy/load_annotation_library_flyout.tsx | 1 + 2 files changed, 30 insertions(+), 27 deletions(-) diff --git a/src/plugins/event_annotation/public/components/event_annotation_group_saved_object_finder.tsx b/src/plugins/event_annotation/public/components/event_annotation_group_saved_object_finder.tsx index 34b1ce7eb0694..72f931797d3c4 100644 --- a/src/plugins/event_annotation/public/components/event_annotation_group_saved_object_finder.tsx +++ b/src/plugins/event_annotation/public/components/event_annotation_group_saved_object_finder.tsx @@ -67,35 +67,37 @@ export const EventAnnotationGroupSavedObjectFinder = ({ direction="column" justifyContent="center" > - - - - } - body={ - -

+ + -

- - } - actions={ - onCreateNew()} size="s"> - - - } - /> + + } + body={ + +

+ +

+ + } + actions={ + onCreateNew()} size="s"> + + + } + /> + ) : ( Date: Mon, 5 Jun 2023 15:07:51 -0500 Subject: [PATCH 03/12] fix icon size --- .../editor_frame/config_panel/layer_actions/layer_actions.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_actions/layer_actions.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_actions/layer_actions.tsx index 97a7740211b13..31cc3843eb199 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_actions/layer_actions.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_actions/layer_actions.tsx @@ -188,7 +188,7 @@ export const LayerActions = (props: LayerActionsProps) => { Date: Mon, 5 Jun 2023 15:12:17 -0500 Subject: [PATCH 04/12] string changes --- .../visualizations/xy/annotations/actions/save_action.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/lens/public/visualizations/xy/annotations/actions/save_action.tsx b/x-pack/plugins/lens/public/visualizations/xy/annotations/actions/save_action.tsx index 0e66654af48c1..358bdcc321b8d 100644 --- a/x-pack/plugins/lens/public/visualizations/xy/annotations/actions/save_action.tsx +++ b/x-pack/plugins/lens/public/visualizations/xy/annotations/actions/save_action.tsx @@ -203,7 +203,7 @@ export const onSave = async ({

annotation library, }} @@ -236,7 +236,7 @@ export const getSaveLayerAction = ({ const displayName = i18n.translate( 'xpack.lens.xyChart.annotations.saveAnnotationGroupToLibrary', { - defaultMessage: 'Save annotation group', + defaultMessage: 'Save to library', } ); From 05da4d5ca423119bb47e5a803e2d2b1681d74032 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Mon, 5 Jun 2023 15:21:55 -0500 Subject: [PATCH 05/12] improve saved object save modal flex structure --- .../save_modal/saved_object_save_modal.tsx | 32 ++++++++----------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/src/plugins/saved_objects/public/save_modal/saved_object_save_modal.tsx b/src/plugins/saved_objects/public/save_modal/saved_object_save_modal.tsx index c471271ead7ed..781c413cc765f 100644 --- a/src/plugins/saved_objects/public/save_modal/saved_object_save_modal.tsx +++ b/src/plugins/saved_objects/public/save_modal/saved_object_save_modal.tsx @@ -30,7 +30,6 @@ import { FormattedMessage } from '@kbn/i18n-react'; import React from 'react'; import { EuiText } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; -import { css } from '@emotion/react'; export interface OnSaveProps { newTitle: string; @@ -167,20 +166,19 @@ export class SavedObjectSaveModal extends React.Component - - {this.renderCopyOnSave()} - - - - - {this.renderConfirmButton()} + + + {this.props.showCopyOnSave && {this.renderCopyOnSave()}} + + + + + + {this.renderConfirmButton()} + ); @@ -351,10 +349,6 @@ export class SavedObjectSaveModal extends React.Component }; private renderCopyOnSave = () => { - if (!this.props.showCopyOnSave) { - return; - } - return ( Date: Mon, 5 Jun 2023 15:42:52 -0500 Subject: [PATCH 06/12] add more padding to unsaved indicator --- .../shared_components/static_header.tsx | 10 +-------- .../xy/xy_config_panel/layer_header.tsx | 21 ++++++++++++------- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/x-pack/plugins/lens/public/shared_components/static_header.tsx b/x-pack/plugins/lens/public/shared_components/static_header.tsx index 8069a2d2c9849..b4c5d8931065e 100644 --- a/x-pack/plugins/lens/public/shared_components/static_header.tsx +++ b/x-pack/plugins/lens/public/shared_components/static_header.tsx @@ -40,15 +40,7 @@ export const StaticHeader = ({

{label}
- {indicator && ( -
- {indicator} -
- )} + {indicator} ); diff --git a/x-pack/plugins/lens/public/visualizations/xy/xy_config_panel/layer_header.tsx b/x-pack/plugins/lens/public/visualizations/xy/xy_config_panel/layer_header.tsx index 3f6c0a6f81754..fd3532d4a161c 100644 --- a/x-pack/plugins/lens/public/visualizations/xy/xy_config_panel/layer_header.tsx +++ b/x-pack/plugins/lens/public/visualizations/xy/xy_config_panel/layer_header.tsx @@ -96,13 +96,20 @@ function AnnotationsLayerHeader({ } indicator={ hasUnsavedChanges && ( - +
+ +
) } /> From 847367a0463beb751dccef2da054e194d2df73b0 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Mon, 5 Jun 2023 15:59:34 -0500 Subject: [PATCH 07/12] better title validation --- .../public/save_modal/saved_object_save_modal.tsx | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/plugins/saved_objects/public/save_modal/saved_object_save_modal.tsx b/src/plugins/saved_objects/public/save_modal/saved_object_save_modal.tsx index 781c413cc765f..b9bd5e5d7fe36 100644 --- a/src/plugins/saved_objects/public/save_modal/saved_object_save_modal.tsx +++ b/src/plugins/saved_objects/public/save_modal/saved_object_save_modal.tsx @@ -92,11 +92,19 @@ export class SavedObjectSaveModal extends React.Component const hasColumns = !!this.props.rightOptions; + const titleInputValid = + hasAttemptedSubmit && + ((!isTitleDuplicateConfirmed && hasTitleDuplicate) || title.length === 0); + const formBodyContent = ( <> } + isInvalid={titleInputValid} + error={i18n.translate('savedObjects.saveModal.titleRequired', { + defaultMessage: 'A title is required', + })} > data-test-subj="savedObjectTitle" value={title} onChange={this.onTitleChange} - isInvalid={ - hasAttemptedSubmit && - ((!isTitleDuplicateConfirmed && hasTitleDuplicate) || title.length === 0) - } + isInvalid={titleInputValid} aria-describedby={this.state.hasTitleDuplicate ? duplicateWarningId : undefined} /> From 9b9d92cca2b977695c02f7eb6cca12ec30aa2867 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Thu, 29 Jun 2023 15:57:19 -0600 Subject: [PATCH 08/12] update remove layer modal text --- .../layer_actions/layer_actions.tsx | 4 +- .../layer_actions/remove_layer_action.tsx | 55 ++++++++++++------- .../editor_frame/config_panel/layer_panel.tsx | 5 +- x-pack/plugins/lens/public/types.ts | 9 +++ .../visualizations/xy/visualization.test.tsx | 50 ++++++++++++++++- .../visualizations/xy/visualization.tsx | 9 +++ .../xy/visualization_helpers.tsx | 2 +- 7 files changed, 110 insertions(+), 24 deletions(-) diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_actions/layer_actions.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_actions/layer_actions.tsx index 31cc3843eb199..875d35c2b758e 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_actions/layer_actions.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_actions/layer_actions.tsx @@ -47,6 +47,7 @@ export const getSharedActions = ({ openLayerSettings, onCloneLayer, onRemoveLayer, + customRemoveModalText, }: { onRemoveLayer: () => void; onCloneLayer: () => void; @@ -54,12 +55,12 @@ export const getSharedActions = ({ layerId: string; isOnlyLayer: boolean; activeVisualization: Visualization; - visualizationState: unknown; layerType?: LayerType; isTextBasedLanguage?: boolean; hasLayerSettings: boolean; openLayerSettings: () => void; core: Pick; + customRemoveModalText?: { title?: string; description?: string }; }) => [ getOpenLayerSettingsAction({ hasLayerSettings, @@ -77,6 +78,7 @@ export const getSharedActions = ({ layerType, isOnlyLayer, core, + customModalText: customRemoveModalText, }), ]; diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_actions/remove_layer_action.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_actions/remove_layer_action.tsx index 29333dcfba06b..4a138273c6118 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_actions/remove_layer_action.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_actions/remove_layer_action.tsx @@ -34,13 +34,15 @@ interface RemoveLayerAction { layerType?: LayerType; isOnlyLayer: boolean; core: Pick; + customModalText?: { title?: string; description?: string }; } const SKIP_DELETE_MODAL_KEY = 'skipDeleteModal'; const getCopy = ( layerType: LayerType, - isOnlyLayer?: boolean + isOnlyLayer?: boolean, + customModalText: { title?: string; description?: string } | undefined = undefined ): { buttonLabel: string; modalTitle: string; modalBody: string } => { if (isOnlyLayer && layerType === 'data') { return { @@ -64,34 +66,46 @@ const getCopy = ( case 'data': return { buttonLabel, - modalTitle: i18n.translate('xpack.lens.modalTitle.title.deleteVis', { - defaultMessage: 'Delete visualization layer?', - }), - modalBody: i18n.translate('xpack.lens.layer.confirmModal.deleteVis', { - defaultMessage: `Deleting this layer removes the visualization and its configurations. `, - }), + modalTitle: + customModalText?.title ?? + i18n.translate('xpack.lens.modalTitle.title.deleteVis', { + defaultMessage: 'Delete visualization layer?', + }), + modalBody: + customModalText?.description ?? + i18n.translate('xpack.lens.layer.confirmModal.deleteVis', { + defaultMessage: `Deleting this layer removes the visualization and its configurations. `, + }), }; case 'annotations': return { buttonLabel, - modalTitle: i18n.translate('xpack.lens.modalTitle.title.deleteAnnotations', { - defaultMessage: 'Delete annotations layer?', - }), - modalBody: i18n.translate('xpack.lens.layer.confirmModal.deleteAnnotation', { - defaultMessage: `Deleting this layer removes the annotations and their configurations. `, - }), + modalTitle: + customModalText?.title ?? + i18n.translate('xpack.lens.modalTitle.title.deleteAnnotations', { + defaultMessage: 'Delete annotation group?', + }), + modalBody: + customModalText?.description ?? + i18n.translate('xpack.lens.layer.confirmModal.deleteAnnotation', { + defaultMessage: `Deleting this layer removes the annotations and their configurations. `, + }), }; case 'referenceLine': return { buttonLabel, - modalTitle: i18n.translate('xpack.lens.modalTitle.title.deleteReferenceLines', { - defaultMessage: 'Delete reference lines layer?', - }), - modalBody: i18n.translate('xpack.lens.layer.confirmModal.deleteRefLine', { - defaultMessage: `Deleting this layer removes the reference lines and their configurations. `, - }), + modalTitle: + customModalText?.title ?? + i18n.translate('xpack.lens.modalTitle.title.deleteReferenceLines', { + defaultMessage: 'Delete reference lines layer?', + }), + modalBody: + customModalText?.description ?? + i18n.translate('xpack.lens.layer.confirmModal.deleteRefLine', { + defaultMessage: `Deleting this layer removes the reference lines and their configurations. `, + }), }; default: @@ -193,7 +207,8 @@ const RemoveConfirmModal = ({ export const getRemoveLayerAction = (props: RemoveLayerAction): LayerAction => { const { buttonLabel, modalTitle, modalBody } = getCopy( props.layerType || LayerTypes.DATA, - props.isOnlyLayer + props.isOnlyLayer, + props.customModalText ); return { diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx index cad30547f71ac..c502965fef900 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx @@ -351,7 +351,6 @@ export function LayerPanel( ...getSharedActions({ layerId, activeVisualization, - visualizationState, core, layerIndex, layerType: activeVisualization.getLayerType(layerId, visualizationState), @@ -365,6 +364,10 @@ export function LayerPanel( openLayerSettings: () => setPanelSettingsOpen(true), onCloneLayer, onRemoveLayer: () => onRemoveLayer(layerId), + customRemoveModalText: activeVisualization.getCustomRemoveLayerText?.( + layerId, + visualizationState + ), }), ].filter((i) => i.isCompatible), [ diff --git a/x-pack/plugins/lens/public/types.ts b/x-pack/plugins/lens/public/types.ts index ce5a3f5b7a736..e20b2a7f615bf 100644 --- a/x-pack/plugins/lens/public/types.ts +++ b/x-pack/plugins/lens/public/types.ts @@ -1116,6 +1116,15 @@ export interface Visualization LayerAction[]; + /** + * This method is a clunky solution to the problem, but I'm banking on the confirm modal being removed + * with undo/redo anyways + */ + getCustomRemoveLayerText?: ( + layerId: string, + state: T + ) => { title?: string; description?: string } | undefined; + /** returns the type string of the given layer */ getLayerType: (layerId: string, state?: T) => LayerType | undefined; diff --git a/x-pack/plugins/lens/public/visualizations/xy/visualization.test.tsx b/x-pack/plugins/lens/public/visualizations/xy/visualization.test.tsx index 6f720f0e21f2f..f279d0b46da6c 100644 --- a/x-pack/plugins/lens/public/visualizations/xy/visualization.test.tsx +++ b/x-pack/plugins/lens/public/visualizations/xy/visualization.test.tsx @@ -3636,7 +3636,7 @@ describe('xy_visualization', () => { Object { "data-test-subj": "lnsXY_annotationLayer_saveToLibrary", "description": "Saves annotation group as separate saved object", - "displayName": "Save annotation group", + "displayName": "Save to library", "execute": [Function], "icon": "save", "isCompatible": true, @@ -3897,4 +3897,52 @@ describe('xy_visualization', () => { ).not.toThrowError(); }); }); + + describe('#getCustomRemoveLayerText', () => { + it('should NOT return custom text for the remove layer button if not by-reference', () => { + expect(xyVisualization.getCustomRemoveLayerText!('first', exampleState())).toBeUndefined(); + }); + + it('should return custom text for the remove layer button if by-reference', () => { + const layerId = 'layer-id'; + + const commonProps = { + layerId, + layerType: 'annotations' as const, + indexPatternId: 'some-index-pattern', + ignoreGlobalFilters: false, + annotations: [ + { + id: 'some-annotation-id', + type: 'manual', + key: { + type: 'point_in_time', + timestamp: 'timestamp', + }, + } as PointInTimeEventAnnotationConfig, + ], + }; + + const layer: XYByReferenceAnnotationLayerConfig = { + ...commonProps, + annotationGroupId: 'some-group-id', + __lastSaved: { + ...commonProps, + title: 'My saved object title', + description: 'some description', + tags: [], + }, + }; + expect( + xyVisualization.getCustomRemoveLayerText!(layerId, { + ...exampleState(), + layers: [layer], + }) + ).toMatchInlineSnapshot(` + Object { + "title": "Delete \\"My saved object title\\"", + } + `); + }); + }); }); diff --git a/x-pack/plugins/lens/public/visualizations/xy/visualization.tsx b/x-pack/plugins/lens/public/visualizations/xy/visualization.tsx index cc8897143a63d..f360b0dd6ba54 100644 --- a/x-pack/plugins/lens/public/visualizations/xy/visualization.tsx +++ b/x-pack/plugins/lens/public/visualizations/xy/visualization.tsx @@ -94,6 +94,7 @@ import { getVisualizationType, isAnnotationsLayer, isBucketed, + isByReferenceAnnotationsLayer, isDataLayer, isNumericDynamicMetric, isReferenceLayer, @@ -299,6 +300,14 @@ export const getXyVisualization = ({ return actions; }, + getCustomRemoveLayerText(layerId, state) { + const layerIndex = state.layers.findIndex((l) => l.layerId === layerId); + const layer = state.layers[layerIndex]; + if (layer && isByReferenceAnnotationsLayer(layer)) { + return { title: `Delete "${layer.__lastSaved.title}"` }; + } + }, + hasLayerSettings({ state, layerId: currentLayerId }) { const layer = state.layers?.find(({ layerId }) => layerId === currentLayerId); return { data: Boolean(layer && isAnnotationsLayer(layer)), appearance: false }; diff --git a/x-pack/plugins/lens/public/visualizations/xy/visualization_helpers.tsx b/x-pack/plugins/lens/public/visualizations/xy/visualization_helpers.tsx index a549ac42d9448..c9bd442301596 100644 --- a/x-pack/plugins/lens/public/visualizations/xy/visualization_helpers.tsx +++ b/x-pack/plugins/lens/public/visualizations/xy/visualization_helpers.tsx @@ -161,7 +161,7 @@ export const isPersistedByValueAnnotationsLayer = ( (layer.persistanceType === 'byValue' || !layer.persistanceType); export const isByReferenceAnnotationsLayer = ( - layer: XYAnnotationLayerConfig + layer: XYLayerConfig ): layer is XYByReferenceAnnotationLayerConfig => 'annotationGroupId' in layer && '__lastSaved' in layer; From d2fc144f9c4ac23fdd715cc12139b52e1f9e5d02 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Fri, 30 Jun 2023 14:18:44 -0600 Subject: [PATCH 09/12] duplicate title checking --- .../common/content_management/v1/types.ts | 5 +- .../event_annotation_service/service.tsx | 39 +++++++-- .../public/event_annotation_service/types.ts | 1 + .../event_annotation_group_storage.ts | 2 + .../annotations/actions/save_action.test.tsx | 80 ++++++++++++++++++- .../xy/annotations/actions/save_action.tsx | 43 +++++++++- 6 files changed, 158 insertions(+), 12 deletions(-) diff --git a/src/plugins/event_annotation/common/content_management/v1/types.ts b/src/plugins/event_annotation/common/content_management/v1/types.ts index a5c2306100821..d3370b9ff28bf 100644 --- a/src/plugins/event_annotation/common/content_management/v1/types.ts +++ b/src/plugins/event_annotation/common/content_management/v1/types.ts @@ -120,6 +120,9 @@ export interface EventAnnotationGroupSearchQuery { searchFields?: string[]; } -export type EventAnnotationGroupSearchIn = SearchIn; +export type EventAnnotationGroupSearchIn = SearchIn< + EventAnnotationGroupContentType, + EventAnnotationGroupSearchQuery +>; export type EventAnnotationGroupSearchOut = SearchResult; diff --git a/src/plugins/event_annotation/public/event_annotation_service/service.tsx b/src/plugins/event_annotation/public/event_annotation_service/service.tsx index 724b1093145fc..a9a02d7930958 100644 --- a/src/plugins/event_annotation/public/event_annotation_service/service.tsx +++ b/src/plugins/event_annotation/public/event_annotation_service/service.tsx @@ -34,7 +34,8 @@ import { isQueryAnnotationConfig, } from './helpers'; import { EventAnnotationGroupSavedObjectFinder } from '../components/event_annotation_group_saved_object_finder'; -import { +import { CONTENT_ID } from '../../common/content_management'; +import type { EventAnnotationGroupCreateIn, EventAnnotationGroupCreateOut, EventAnnotationGroupDeleteIn, @@ -106,7 +107,7 @@ export function getEventAnnotationService( savedObjectId: string ): Promise => { const savedObject = await client.get({ - contentTypeId: EVENT_ANNOTATION_GROUP_TYPE, + contentTypeId: CONTENT_ID, id: savedObjectId, }); @@ -117,6 +118,29 @@ export function getEventAnnotationService( return mapSavedObjectToGroupConfig(savedObject.item); }; + const groupExistsWithTitle = async (title: string): Promise => { + const { hits } = await client.search< + EventAnnotationGroupSearchIn, + EventAnnotationGroupSearchOut + >({ + contentTypeId: CONTENT_ID, + query: { + text: title, + }, + options: { + searchFields: ['title'], + }, + }); + + for (const hit of hits) { + if (hit.attributes.title.toLowerCase() === title.toLowerCase()) { + return true; + } + } + + return false; + }; + const findAnnotationGroupContent = async ( searchTerm: string, pageSize: number, @@ -138,7 +162,7 @@ export function getEventAnnotationService( EventAnnotationGroupSearchIn, EventAnnotationGroupSearchOut >({ - contentTypeId: EVENT_ANNOTATION_GROUP_TYPE, + contentTypeId: CONTENT_ID, query: { text: searchOptions.search, }, @@ -153,7 +177,7 @@ export function getEventAnnotationService( const deleteAnnotationGroups = async (ids: string[]): Promise => { for (const id of ids) { await client.delete({ - contentTypeId: EVENT_ANNOTATION_GROUP_TYPE, + contentTypeId: CONTENT_ID, id, }); } @@ -223,7 +247,7 @@ export function getEventAnnotationService( const groupSavedObjectId = ( await client.create({ - contentTypeId: EVENT_ANNOTATION_GROUP_TYPE, + contentTypeId: CONTENT_ID, data: { ...attributes, }, @@ -243,7 +267,7 @@ export function getEventAnnotationService( const { attributes, references } = getAnnotationGroupAttributesAndReferences(group); await client.update({ - contentTypeId: EVENT_ANNOTATION_GROUP_TYPE, + contentTypeId: CONTENT_ID, id: annotationGroupId, data: { ...attributes, @@ -259,7 +283,7 @@ export function getEventAnnotationService( EventAnnotationGroupSearchIn, EventAnnotationGroupSearchOut >({ - contentTypeId: EVENT_ANNOTATION_GROUP_TYPE, + contentTypeId: CONTENT_ID, query: { text: '*', }, @@ -270,6 +294,7 @@ export function getEventAnnotationService( return { loadAnnotationGroup, + groupExistsWithTitle, updateAnnotationGroup, createAnnotationGroup, deleteAnnotationGroups, diff --git a/src/plugins/event_annotation/public/event_annotation_service/types.ts b/src/plugins/event_annotation/public/event_annotation_service/types.ts index 8742ea86afff8..8837792954b51 100644 --- a/src/plugins/event_annotation/public/event_annotation_service/types.ts +++ b/src/plugins/event_annotation/public/event_annotation_service/types.ts @@ -14,6 +14,7 @@ import { EventAnnotationConfig, EventAnnotationGroupConfig } from '../../common' export interface EventAnnotationServiceType { loadAnnotationGroup: (savedObjectId: string) => Promise; + groupExistsWithTitle: (title: string) => Promise; findAnnotationGroupContent: ( searchTerm: string, pageSize: number, diff --git a/src/plugins/event_annotation/server/content_management/event_annotation_group_storage.ts b/src/plugins/event_annotation/server/content_management/event_annotation_group_storage.ts index ce032374e7b53..5755a7efe998f 100644 --- a/src/plugins/event_annotation/server/content_management/event_annotation_group_storage.ts +++ b/src/plugins/event_annotation/server/content_management/event_annotation_group_storage.ts @@ -268,9 +268,11 @@ export class EventAnnotationGroupStorage EventAnnotationGroupSearchQuery, EventAnnotationGroupSearchQuery >(options); + if (optionsError) { throw Boom.badRequest(`Invalid payload. ${optionsError.message}`); } + const { searchFields = ['title^3', 'description'], types = [SO_TYPE] } = optionsToLatest; const { included, excluded } = query.tags ?? {}; diff --git a/x-pack/plugins/lens/public/visualizations/xy/annotations/actions/save_action.test.tsx b/x-pack/plugins/lens/public/visualizations/xy/annotations/actions/save_action.test.tsx index 8505f9811749a..cf389614a50b4 100644 --- a/x-pack/plugins/lens/public/visualizations/xy/annotations/actions/save_action.test.tsx +++ b/x-pack/plugins/lens/public/visualizations/xy/annotations/actions/save_action.test.tsx @@ -113,7 +113,7 @@ describe('annotation group save action', () => { describe('save routine', () => { const layerId = 'mylayerid'; - const layer: XYByValueAnnotationLayerConfig = { + const byValueLayer: XYByValueAnnotationLayerConfig = { layerId, layerType: 'annotations', indexPatternId: 'some-index-pattern', @@ -144,10 +144,11 @@ describe('annotation group save action', () => { legend: { isVisible: true, position: 'bottom' }, layers: [{ layerId } as XYAnnotationLayerConfig], } as XYState, - layer, + layer: byValueLayer, setState: jest.fn(), eventAnnotationService: { createAnnotationGroup: jest.fn(() => Promise.resolve({ id: savedId })), + groupExistsWithTitle: jest.fn(() => Promise.resolve(false)), updateAnnotationGroup: jest.fn(), loadAnnotationGroup: jest.fn(), toExpression: jest.fn(), @@ -162,7 +163,7 @@ describe('annotation group save action', () => { newTags: ['my-tag'], newCopyOnSave: false, isTitleDuplicateConfirmed: false, - onTitleDuplicate: () => {}, + onTitleDuplicate: jest.fn(), }, dataViews, goToAnnotationLibrary: () => Promise.resolve(), @@ -321,5 +322,78 @@ describe('annotation group save action', () => { expect(props.toasts.addSuccess).toHaveBeenCalledTimes(1); }); + + it.each` + existingGroup | newCopyOnSave | titleChanged | isTitleDuplicateConfirmed | expectPreventSave + ${false} | ${false} | ${false} | ${false} | ${true} + ${false} | ${false} | ${false} | ${true} | ${false} + ${true} | ${false} | ${false} | ${false} | ${false} + ${true} | ${true} | ${false} | ${false} | ${true} + ${true} | ${true} | ${false} | ${true} | ${false} + `( + 'checks duplicate title when saving group', + async ({ + existingGroup, + newCopyOnSave, + titleChanged, + isTitleDuplicateConfirmed, + expectPreventSave, + }) => { + (props.eventAnnotationService.groupExistsWithTitle as jest.Mock).mockResolvedValueOnce( + true + ); + + const oldTitle = 'old title'; + let layer: XYAnnotationLayerConfig = byValueLayer; + if (existingGroup) { + const byReferenceLayer: XYByReferenceAnnotationLayerConfig = { + ...props.layer, + annotationGroupId: 'my-group-id', + __lastSaved: { + ...props.layer, + title: oldTitle, + description: 'description', + tags: [], + }, + }; + layer = byReferenceLayer; + } + + const newTitle = titleChanged ? 'my changed title' : oldTitle; + + await onSave({ + ...props, + layer, + modalOnSaveProps: { + ...props.modalOnSaveProps, + newTitle, + isTitleDuplicateConfirmed, + newCopyOnSave, + }, + }); + + if (expectPreventSave) { + expect(props.eventAnnotationService.updateAnnotationGroup).not.toHaveBeenCalled(); + + expect(props.eventAnnotationService.createAnnotationGroup).not.toHaveBeenCalled(); + + expect(props.modalOnSaveProps.closeModal).not.toHaveBeenCalled(); + + expect(props.setState).not.toHaveBeenCalled(); + + expect(props.toasts.addSuccess).not.toHaveBeenCalled(); + + expect(props.modalOnSaveProps.onTitleDuplicate).toHaveBeenCalled(); + } else { + expect(props.modalOnSaveProps.onTitleDuplicate).not.toHaveBeenCalled(); + + expect(props.modalOnSaveProps.closeModal).toHaveBeenCalled(); + + expect(props.setState).toHaveBeenCalled(); + + expect(props.toasts.addSuccess).toHaveBeenCalledTimes(1); + } + } + ); }); }); diff --git a/x-pack/plugins/lens/public/visualizations/xy/annotations/actions/save_action.tsx b/x-pack/plugins/lens/public/visualizations/xy/annotations/actions/save_action.tsx index 19bd4403ee8d1..2d21109ca0e39 100644 --- a/x-pack/plugins/lens/public/visualizations/xy/annotations/actions/save_action.tsx +++ b/x-pack/plugins/lens/public/visualizations/xy/annotations/actions/save_action.tsx @@ -133,6 +133,28 @@ const saveAnnotationGroupToLibrary = async ( return { id: savedId, config: groupConfig }; }; +const shouldStopBecauseDuplicateTitle = async ( + newTitle: string, + existingTitle: string, + newCopyOnSave: ModalOnSaveProps['newCopyOnSave'], + onTitleDuplicate: ModalOnSaveProps['onTitleDuplicate'], + isTitleDuplicateConfirmed: ModalOnSaveProps['isTitleDuplicateConfirmed'], + eventAnnotationService: EventAnnotationServiceType +) => { + if (isTitleDuplicateConfirmed || (newTitle === existingTitle && !newCopyOnSave)) { + return false; + } + + const duplicateExists = await eventAnnotationService.groupExistsWithTitle(newTitle); + + if (duplicateExists) { + onTitleDuplicate(); + return true; + } else { + return false; + } +}; + /** @internal exported for testing only */ export const onSave = async ({ state, @@ -140,7 +162,15 @@ export const onSave = async ({ setState, eventAnnotationService, toasts, - modalOnSaveProps: { newTitle, newDescription, newTags, closeModal, newCopyOnSave }, + modalOnSaveProps: { + newTitle, + newDescription, + newTags, + closeModal, + newCopyOnSave, + onTitleDuplicate, + isTitleDuplicateConfirmed, + }, dataViews, goToAnnotationLibrary, }: { @@ -153,6 +183,17 @@ export const onSave = async ({ dataViews: DataViewsContract; goToAnnotationLibrary: () => Promise; }) => { + const shouldStop = await shouldStopBecauseDuplicateTitle( + newTitle, + isByReferenceAnnotationsLayer(layer) ? layer.__lastSaved.title : '', + newCopyOnSave, + onTitleDuplicate, + isTitleDuplicateConfirmed, + eventAnnotationService + ); + + if (shouldStop) return; + let savedInfo: Awaited>; try { savedInfo = await saveAnnotationGroupToLibrary( From 2168720bec84b488d74af49765590b250d3b0464 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Fri, 30 Jun 2023 14:25:58 -0600 Subject: [PATCH 10/12] fix CI --- .../saved_object_save_modal.test.tsx.snap | 288 +++++++++--------- .../xy/xy_config_panel/layer_header.tsx | 1 + .../translations/translations/fr-FR.json | 4 +- .../translations/translations/ja-JP.json | 4 +- .../translations/translations/zh-CN.json | 4 +- 5 files changed, 148 insertions(+), 153 deletions(-) diff --git a/src/plugins/saved_objects/public/save_modal/__snapshots__/saved_object_save_modal.test.tsx.snap b/src/plugins/saved_objects/public/save_modal/__snapshots__/saved_object_save_modal.test.tsx.snap index 70ba2d5619d03..e42ac60f9ba89 100644 --- a/src/plugins/saved_objects/public/save_modal/__snapshots__/saved_object_save_modal.test.tsx.snap +++ b/src/plugins/saved_objects/public/save_modal/__snapshots__/saved_object_save_modal.test.tsx.snap @@ -28,9 +28,11 @@ exports[`SavedObjectSaveModal should render matching snapshot 1`] = ` - - - + - - - - Save - + + + + + + + + Save + + + `; @@ -154,9 +154,11 @@ exports[`SavedObjectSaveModal should render matching snapshot when custom isVali - - - + - - - - Save - + + + + + + + + Save + + + `; @@ -280,9 +280,11 @@ exports[`SavedObjectSaveModal should render matching snapshot when custom isVali - - - + - - - - Save - + + + + + + + + Save + + + `; @@ -410,9 +410,11 @@ exports[`SavedObjectSaveModal should render matching snapshot when given options - - - + - - - - Save - + + + + + + + + Save + + + `; diff --git a/x-pack/plugins/lens/public/visualizations/xy/xy_config_panel/layer_header.tsx b/x-pack/plugins/lens/public/visualizations/xy/xy_config_panel/layer_header.tsx index 5f230f0b5798d..cb37fcbf5edfc 100644 --- a/x-pack/plugins/lens/public/visualizations/xy/xy_config_panel/layer_header.tsx +++ b/x-pack/plugins/lens/public/visualizations/xy/xy_config_panel/layer_header.tsx @@ -19,6 +19,7 @@ import { import { ToolbarButton } from '@kbn/kibana-react-plugin/public'; import { IconChartBarReferenceLine, IconChartBarAnnotations } from '@kbn/chart-icons'; import { euiThemeVars } from '@kbn/ui-theme'; +import { css } from '@emotion/react'; import { getIgnoreGlobalFilterIcon } from '../../../shared_components/ignore_global_filter/data_view_picker_icon'; import type { VisualizationLayerHeaderContentProps, diff --git a/x-pack/plugins/translations/translations/fr-FR.json b/x-pack/plugins/translations/translations/fr-FR.json index d9e041ec102d9..e9f1e92c19ae7 100644 --- a/x-pack/plugins/translations/translations/fr-FR.json +++ b/x-pack/plugins/translations/translations/fr-FR.json @@ -20080,7 +20080,6 @@ "xpack.lens.functions.timeScale.dateColumnMissingMessage": "L'ID de colonne de date spécifié {columnId} n'existe pas.", "xpack.lens.heatmapVisualization.arrayValuesWarningMessage": "{label} contient des valeurs de tableau. Le rendu de votre visualisation peut ne pas se présenter comme attendu.", "xpack.lens.indexPattern.addColumnAriaLabel": "Ajouter ou faire glisser un champ vers {groupLabel}", - "xpack.lens.indexPattern.addColumnAriaLabelClick": "Ajouter une annotation à {groupLabel}", "xpack.lens.indexPattern.annotationsDimensionEditorLabel": "Annotation {groupLabel}", "xpack.lens.indexPattern.ascendingCountPrecisionErrorWarning": "{name} pour cette visualisation peut être approximatif en raison de la manière dont les données sont indexées. Essayez de trier par rareté plutôt que par nombre ascendant d’enregistrements. Pour en savoir plus sur cette limitation, {link}.", "xpack.lens.indexPattern.autoIntervalLabel": "Auto ({interval})", @@ -20315,7 +20314,6 @@ "xpack.lens.configPanel.selectVisualization": "Sélectionner une visualisation", "xpack.lens.configPanel.visualizationType": "Type de visualisation", "xpack.lens.configure.emptyConfig": "Ajouter ou glisser-déposer un champ", - "xpack.lens.configure.emptyConfigClick": "Ajouter une annotation", "xpack.lens.configure.invalidBottomReferenceLineDimension": "La ligne de référence est affectée à un axe qui n’existe plus ou qui n’est plus valide. Vous pouvez déplacer cette ligne de référence vers un autre axe disponible ou la supprimer.", "xpack.lens.configure.invalidConfigTooltip": "Configuration non valide.", "xpack.lens.configure.invalidConfigTooltipClick": "Cliquez pour en savoir plus.", @@ -39440,4 +39438,4 @@ "xpack.painlessLab.title": "Painless Lab", "xpack.painlessLab.walkthroughButtonLabel": "Présentation" } -} \ No newline at end of file +} diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json index 499805b310a36..4a5b66909d950 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -20079,7 +20079,6 @@ "xpack.lens.functions.timeScale.dateColumnMissingMessage": "指定したdateColumnId {columnId}は存在しません。", "xpack.lens.heatmapVisualization.arrayValuesWarningMessage": "{label}には配列値が含まれます。可視化が想定通りに表示されない場合があります。", "xpack.lens.indexPattern.addColumnAriaLabel": "フィールドを追加するか、{groupLabel}にドラッグアンドドロップします", - "xpack.lens.indexPattern.addColumnAriaLabelClick": "注釈を{groupLabel}に追加", "xpack.lens.indexPattern.annotationsDimensionEditorLabel": "{groupLabel}注釈", "xpack.lens.indexPattern.ascendingCountPrecisionErrorWarning": "データのインデックス方法のため、このビジュアライゼーションの{name}は近似される場合があります。レコード数の昇順ではなく希少性で並べ替えてください。この制限の詳細については、{link}。", "xpack.lens.indexPattern.autoIntervalLabel": "自動（{interval}）", @@ -20315,7 +20314,6 @@ "xpack.lens.configPanel.selectVisualization": "ビジュアライゼーションを選択してください", "xpack.lens.configPanel.visualizationType": "ビジュアライゼーションタイプ", "xpack.lens.configure.emptyConfig": "フィールドを追加するか、ドラッグアンドドロップします", - "xpack.lens.configure.emptyConfigClick": "注釈の追加", "xpack.lens.configure.invalidBottomReferenceLineDimension": "この基準線は存在しないか有効ではない軸に割り当てられています。この基準線を別の使用可能な軸に移動するか、削除することができます。", "xpack.lens.configure.invalidConfigTooltip": "無効な構成です。", "xpack.lens.configure.invalidConfigTooltipClick": "詳細はクリックしてください。", @@ -39410,4 +39408,4 @@ "xpack.painlessLab.title": "Painless Lab", "xpack.painlessLab.walkthroughButtonLabel": "実地検証" } -} \ No newline at end of file +} diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index 08567d2070538..42ba9293e7b08 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -20079,7 +20079,6 @@ "xpack.lens.functions.timeScale.dateColumnMissingMessage": "指定的 dateColumnId {columnId} 不存在。", "xpack.lens.heatmapVisualization.arrayValuesWarningMessage": "{label} 包含数组值。您的可视化可能无法正常渲染。", "xpack.lens.indexPattern.addColumnAriaLabel": "将字段添加或拖放到 {groupLabel}", - "xpack.lens.indexPattern.addColumnAriaLabelClick": "添加标注到 {groupLabel}", "xpack.lens.indexPattern.annotationsDimensionEditorLabel": "{groupLabel} 标注", "xpack.lens.indexPattern.ascendingCountPrecisionErrorWarning": "由于数据的索引方式，此可视化的 {name} 可能为近似值。尝试按稀有度排序，而不是采用升序记录计数。有关此限制的详情，{link}。", "xpack.lens.indexPattern.autoIntervalLabel": "自动 ({interval})", @@ -20315,7 +20314,6 @@ "xpack.lens.configPanel.selectVisualization": "选择可视化", "xpack.lens.configPanel.visualizationType": "可视化类型", "xpack.lens.configure.emptyConfig": "添加或拖放字段", - "xpack.lens.configure.emptyConfigClick": "添加标注", "xpack.lens.configure.invalidBottomReferenceLineDimension": "此参考线分配给了不再存在或不再有效的轴。您可以将此参考线移到其他可用的轴，或将其移除。", "xpack.lens.configure.invalidConfigTooltip": "配置无效。", "xpack.lens.configure.invalidConfigTooltipClick": "单击了解更多详情。", @@ -39404,4 +39402,4 @@ "xpack.painlessLab.title": "Painless 实验室", "xpack.painlessLab.walkthroughButtonLabel": "指导" } -} \ No newline at end of file +} From 184f8bc8d593520393f0a9c1b74fe49ff6b29c52 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Fri, 30 Jun 2023 15:37:43 -0600 Subject: [PATCH 11/12] remove padding --- .../xy/annotations/actions/save_action.tsx | 41 +++++++++---------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/x-pack/plugins/lens/public/visualizations/xy/annotations/actions/save_action.tsx b/x-pack/plugins/lens/public/visualizations/xy/annotations/actions/save_action.tsx index 2d21109ca0e39..c00a8643be38d 100644 --- a/x-pack/plugins/lens/public/visualizations/xy/annotations/actions/save_action.tsx +++ b/x-pack/plugins/lens/public/visualizations/xy/annotations/actions/save_action.tsx @@ -243,30 +243,29 @@ export const onSave = async ({ }, } ), + toastLifeTimeMs: 10000000, text: ((element) => render( -

- goToAnnotationLibrary()} - > - {i18n.translate( - 'xpack.lens.xyChart.annotations.saveAnnotationGroupToLibrary.annotationLibrary', - { - defaultMessage: 'annotation library', - } - )} - - ), - }} - /> -

+ goToAnnotationLibrary()} + > + {i18n.translate( + 'xpack.lens.xyChart.annotations.saveAnnotationGroupToLibrary.annotationLibrary', + { + defaultMessage: 'annotation library', + } + )} + + ), + }} + /> , element )) as MountPoint, From 9f49d43011d40c55562d466af1084f666a7150f9 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Fri, 30 Jun 2023 17:16:42 -0500 Subject: [PATCH 12/12] Remove toast TTL --- .../public/visualizations/xy/annotations/actions/save_action.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugins/lens/public/visualizations/xy/annotations/actions/save_action.tsx b/x-pack/plugins/lens/public/visualizations/xy/annotations/actions/save_action.tsx index c00a8643be38d..803dc3e41e87d 100644 --- a/x-pack/plugins/lens/public/visualizations/xy/annotations/actions/save_action.tsx +++ b/x-pack/plugins/lens/public/visualizations/xy/annotations/actions/save_action.tsx @@ -243,7 +243,6 @@ export const onSave = async ({ }, } ), - toastLifeTimeMs: 10000000, text: ((element) => render(