From 726f212d0c4bc262d00aea455538ee9c7006ba5e Mon Sep 17 00:00:00 2001 From: Vadim Kibana <82822460+vadimkibana@users.noreply.github.com> Date: Thu, 28 Sep 2023 18:53:31 +0200 Subject: [PATCH] [Tags] Prevent duplicates (#167072) --- .buildkite/ftr_configs.yml | 1 + .../saved_objects_tagging_oss/common/types.ts | 1 + .../public/api.mock.ts | 1 + .../components/edition_modal/create_modal.tsx | 39 +++-- .../edition_modal/create_or_edit_modal.tsx | 24 ++- .../components/edition_modal/edit_modal.tsx | 45 ++++-- .../components/edition_modal/open_modal.tsx | 14 +- .../edition_modal/use_validation.ts | 143 ++++++++++++++++++ .../public/components/edition_modal/utils.ts | 9 ++ .../public/management/actions/edit.ts | 2 +- .../public/management/tag_management_page.tsx | 4 +- .../saved_objects_tagging/public/plugin.ts | 3 +- .../public/services/tags/tags_client.mock.ts | 1 + .../public/services/tags/tags_client.ts | 9 ++ .../public/ui_api/components.ts | 6 +- .../public/ui_api/index.ts | 13 +- .../server/routes/internal/find_tags.ts | 2 +- .../server/routes/tags/create_tag.ts | 8 + .../server/routes/tags/update_tag.ts | 8 + .../server/services/tags/tags_client.mock.ts | 1 + .../server/services/tags/tags_client.ts | 24 ++- .../security_and_spaces/apis/test_utils.ts | 26 +++- .../tagging_api/apis/create.ts | 21 +++ .../api_integration/tagging_api/apis/index.ts | 1 - .../tagging_api/apis/update.ts | 33 ++++ .../tagging_usage_collection/config.ts | 36 +++++ .../tagging_usage_collection/services.ts | 15 ++ .../tests.ts} | 3 +- 28 files changed, 451 insertions(+), 42 deletions(-) create mode 100644 x-pack/plugins/saved_objects_tagging/public/components/edition_modal/use_validation.ts create mode 100644 x-pack/test/saved_object_tagging/api_integration/tagging_usage_collection/config.ts create mode 100644 x-pack/test/saved_object_tagging/api_integration/tagging_usage_collection/services.ts rename x-pack/test/saved_object_tagging/api_integration/{tagging_api/apis/usage_collection.ts => tagging_usage_collection/tests.ts} (94%) diff --git a/.buildkite/ftr_configs.yml b/.buildkite/ftr_configs.yml index b25f6f1bf44ac..309bf005084c9 100644 --- a/.buildkite/ftr_configs.yml +++ b/.buildkite/ftr_configs.yml @@ -357,6 +357,7 @@ enabled: - x-pack/test/saved_object_api_integration/spaces_only/config.ts - x-pack/test/saved_object_tagging/api_integration/security_and_spaces/config.ts - x-pack/test/saved_object_tagging/api_integration/tagging_api/config.ts + - x-pack/test/saved_object_tagging/api_integration/tagging_usage_collection/config.ts - x-pack/test/saved_object_tagging/functional/config.ts - x-pack/test/saved_objects_field_count/config.ts - x-pack/test/search_sessions_integration/config.ts diff --git a/src/plugins/saved_objects_tagging_oss/common/types.ts b/src/plugins/saved_objects_tagging_oss/common/types.ts index e62639659b5f4..046e187a6b9d7 100644 --- a/src/plugins/saved_objects_tagging_oss/common/types.ts +++ b/src/plugins/saved_objects_tagging_oss/common/types.ts @@ -35,6 +35,7 @@ export interface ITagsClient { create(attributes: TagAttributes, options?: CreateTagOptions): Promise; get(id: string): Promise; getAll(options?: GetAllTagsOptions): Promise; + findByName(name: string, options?: { exact?: boolean }): Promise; delete(id: string): Promise; update(id: string, attributes: TagAttributes): Promise; } diff --git a/src/plugins/saved_objects_tagging_oss/public/api.mock.ts b/src/plugins/saved_objects_tagging_oss/public/api.mock.ts index e96aa2277b0a5..f481319ec7fb0 100644 --- a/src/plugins/saved_objects_tagging_oss/public/api.mock.ts +++ b/src/plugins/saved_objects_tagging_oss/public/api.mock.ts @@ -16,6 +16,7 @@ const createClientMock = () => { getAll: jest.fn(), delete: jest.fn(), update: jest.fn(), + findByName: jest.fn(), }; return mock; diff --git a/x-pack/plugins/saved_objects_tagging/public/components/edition_modal/create_modal.tsx b/x-pack/plugins/saved_objects_tagging/public/components/edition_modal/create_modal.tsx index ed4b73c3ea0d3..9f270771b3cc7 100644 --- a/x-pack/plugins/saved_objects_tagging/public/components/edition_modal/create_modal.tsx +++ b/x-pack/plugins/saved_objects_tagging/public/components/edition_modal/create_modal.tsx @@ -6,17 +6,22 @@ */ import React, { FC, useState, useCallback } from 'react'; +import { first, lastValueFrom } from 'rxjs'; +import { i18n } from '@kbn/i18n'; +import type { NotificationsStart } from '@kbn/core/public'; + import { ITagsClient, Tag, TagAttributes } from '../../../common/types'; -import { TagValidation } from '../../../common/validation'; import { isServerValidationError } from '../../services/tags'; import { getRandomColor, validateTag } from './utils'; import { CreateOrEditModal } from './create_or_edit_modal'; +import { useValidation } from './use_validation'; interface CreateTagModalProps { defaultValues?: Partial; onClose: () => void; onSave: (tag: Tag) => void; tagClient: ITagsClient; + notifications: NotificationsStart; } const getDefaultAttributes = (providedDefaults?: Partial): TagAttributes => ({ @@ -26,22 +31,21 @@ const getDefaultAttributes = (providedDefaults?: Partial): TagAtt ...providedDefaults, }); -const initialValidation: TagValidation = { - valid: true, - warnings: [], - errors: {}, -}; - export const CreateTagModal: FC = ({ defaultValues, tagClient, + notifications, onClose, onSave, }) => { - const [validation, setValidation] = useState(initialValidation); const [tagAttributes, setTagAttributes] = useState( getDefaultAttributes(defaultValues) ); + const { validation, setValidation, onNameChange, validation$, isValidating } = useValidation({ + tagAttributes, + tagClient, + validateDuplicateNameOnMount: true, + }); const setField = useCallback( (field: T) => @@ -55,6 +59,14 @@ export const CreateTagModal: FC = ({ ); const onSubmit = useCallback(async () => { + const { hasDuplicateNameError } = await lastValueFrom( + validation$.pipe(first((v) => v.isValidating === false)) + ); + + if (hasDuplicateNameError) { + return; + } + const clientValidation = validateTag(tagAttributes); setValidation(clientValidation); if (!clientValidation.valid) { @@ -68,18 +80,27 @@ export const CreateTagModal: FC = ({ // if e is IHttpFetchError, actual server error payload is in e.body if (isServerValidationError(e.body)) { setValidation(e.body.attributes); + } else { + notifications.toasts.addDanger({ + title: i18n.translate('xpack.savedObjectsTagging.saveTagErrorTitle', { + defaultMessage: 'An error occurred creating tag', + }), + text: e.body.message, + }); } } - }, [tagAttributes, tagClient, onSave]); + }, [validation$, tagAttributes, setValidation, tagClient, onSave, notifications.toasts]); return ( ); }; diff --git a/x-pack/plugins/saved_objects_tagging/public/components/edition_modal/create_or_edit_modal.tsx b/x-pack/plugins/saved_objects_tagging/public/components/edition_modal/create_or_edit_modal.tsx index f972220843b47..0b5e838213959 100644 --- a/x-pack/plugins/saved_objects_tagging/public/components/edition_modal/create_or_edit_modal.tsx +++ b/x-pack/plugins/saved_objects_tagging/public/components/edition_modal/create_or_edit_modal.tsx @@ -5,7 +5,7 @@ * 2.0. */ -import React, { FC, useState, useCallback, useMemo } from 'react'; +import React, { FC, useState, useCallback, useMemo, useRef } from 'react'; import { EuiButtonEmpty, EuiButton, @@ -27,6 +27,7 @@ import { } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n-react'; +import useDebounce from 'react-use/lib/useDebounce'; import { TagAttributes, TagValidation, @@ -40,16 +41,23 @@ import { getRandomColor, useIfMounted } from './utils'; interface CreateOrEditModalProps { onClose: () => void; onSubmit: () => Promise; + onNameChange: ( + name: string, + options?: { debounced?: boolean; hasBeenModified?: boolean } + ) => Promise; mode: 'create' | 'edit'; tag: TagAttributes; validation: TagValidation; + isValidating: boolean; setField: (field: T) => (value: TagAttributes[T]) => void; } export const CreateOrEditModal: FC = ({ onClose, onSubmit, + onNameChange, validation, + isValidating, setField, tag, mode, @@ -57,6 +65,7 @@ export const CreateOrEditModal: FC = ({ const optionalMessageId = htmlIdGenerator()(); const ifMounted = useIfMounted(); const [submitting, setSubmitting] = useState(false); + const lastNameValue = useRef(tag.name); // we don't want this value to change when the user edits the tag // eslint-disable-next-line react-hooks/exhaustive-deps @@ -68,6 +77,8 @@ export const CreateOrEditModal: FC = ({ tag.description !== initialTag.description, [initialTag, tag] ); + const nameHasBeenModified = tag.name !== lastNameValue.current; + const setName = useMemo(() => setField('name'), [setField]); const setColor = useMemo(() => setField('color'), [setField]); const setDescription = useMemo(() => setField('description'), [setField]); @@ -91,6 +102,15 @@ export const CreateOrEditModal: FC = ({ }); }, [ifMounted, onSubmit]); + useDebounce( + () => { + onNameChange(tag.name, { debounced: true, hasBeenModified: nameHasBeenModified }); + lastNameValue.current = tag.name; + }, + 300, + [tag.name, nameHasBeenModified] + ); + return ( @@ -130,6 +150,7 @@ export const CreateOrEditModal: FC = ({ maxLength={tagNameMaxLength} value={tag.name} onChange={(e) => setName(e.target.value)} + isLoading={isValidating} data-test-subj="createModalField-name" /> @@ -238,6 +259,7 @@ export const CreateOrEditModal: FC = ({ fill data-test-subj="createModalConfirmButton" onClick={onFormSubmit} + isLoading={submitting} isDisabled={submitting || (isEdit && !tagHasBeenModified)} > {isEdit ? ( diff --git a/x-pack/plugins/saved_objects_tagging/public/components/edition_modal/edit_modal.tsx b/x-pack/plugins/saved_objects_tagging/public/components/edition_modal/edit_modal.tsx index 4f0e956e6561c..d3419e831c18a 100644 --- a/x-pack/plugins/saved_objects_tagging/public/components/edition_modal/edit_modal.tsx +++ b/x-pack/plugins/saved_objects_tagging/public/components/edition_modal/edit_modal.tsx @@ -6,33 +6,40 @@ */ import React, { FC, useState, useCallback } from 'react'; +import { i18n } from '@kbn/i18n'; +import type { NotificationsStart } from '@kbn/core/public'; +import { first, lastValueFrom } from 'rxjs'; import { ITagsClient, Tag, TagAttributes } from '../../../common/types'; -import { TagValidation } from '../../../common/validation'; import { isServerValidationError } from '../../services/tags'; import { CreateOrEditModal } from './create_or_edit_modal'; import { validateTag } from './utils'; +import { useValidation } from './use_validation'; interface EditTagModalProps { tag: Tag; onClose: () => void; onSave: (tag: Tag) => void; tagClient: ITagsClient; + notifications: NotificationsStart; } -const initialValidation: TagValidation = { - valid: true, - warnings: [], - errors: {}, -}; - const getAttributes = (tag: Tag): TagAttributes => { const { id, ...attributes } = tag; return attributes; }; -export const EditTagModal: FC = ({ tag, onSave, onClose, tagClient }) => { - const [validation, setValidation] = useState(initialValidation); +export const EditTagModal: FC = ({ + tag, + onSave, + onClose, + tagClient, + notifications, +}) => { const [tagAttributes, setTagAttributes] = useState(getAttributes(tag)); + const { validation, setValidation, onNameChange, isValidating, validation$ } = useValidation({ + tagAttributes, + tagClient, + }); const setField = useCallback( (field: T) => @@ -46,8 +53,17 @@ export const EditTagModal: FC = ({ tag, onSave, onClose, tagC ); const onSubmit = useCallback(async () => { + const { hasDuplicateNameError } = await lastValueFrom( + validation$.pipe(first((v) => v.isValidating === false)) + ); + + if (hasDuplicateNameError) { + return; + } + const clientValidation = validateTag(tagAttributes); setValidation(clientValidation); + if (!clientValidation.valid) { return; } @@ -59,18 +75,27 @@ export const EditTagModal: FC = ({ tag, onSave, onClose, tagC // if e is IHttpFetchError, actual server error payload is in e.body if (isServerValidationError(e.body)) { setValidation(e.body.attributes); + } else { + notifications.toasts.addDanger({ + title: i18n.translate('xpack.savedObjectsTagging.editTagErrorTitle', { + defaultMessage: 'An error occurred editing tag', + }), + text: e.body.message, + }); } } - }, [tagAttributes, tagClient, onSave, tag]); + }, [validation$, tagAttributes, setValidation, tagClient, tag.id, onSave, notifications.toasts]); return ( ); }; diff --git a/x-pack/plugins/saved_objects_tagging/public/components/edition_modal/open_modal.tsx b/x-pack/plugins/saved_objects_tagging/public/components/edition_modal/open_modal.tsx index 66ec3139a89f9..9fb8dc28c466e 100644 --- a/x-pack/plugins/saved_objects_tagging/public/components/edition_modal/open_modal.tsx +++ b/x-pack/plugins/saved_objects_tagging/public/components/edition_modal/open_modal.tsx @@ -7,13 +7,19 @@ import React from 'react'; import { EuiDelayRender, EuiLoadingSpinner } from '@elastic/eui'; -import { OverlayStart, OverlayRef, ThemeServiceStart } from '@kbn/core/public'; +import type { + OverlayStart, + OverlayRef, + ThemeServiceStart, + NotificationsStart, +} from '@kbn/core/public'; import { toMountPoint } from '@kbn/kibana-react-plugin/public'; import { Tag, TagAttributes } from '../../../common/types'; import { ITagInternalClient } from '../../services'; interface GetModalOpenerOptions { overlays: OverlayStart; + notifications: NotificationsStart; theme: ThemeServiceStart; tagClient: ITagInternalClient; } @@ -40,7 +46,7 @@ const LazyEditTagModal = React.lazy(() => ); export const getCreateModalOpener = - ({ overlays, theme, tagClient }: GetModalOpenerOptions): CreateModalOpener => + ({ overlays, theme, tagClient, notifications }: GetModalOpenerOptions): CreateModalOpener => async ({ onCreate, defaultValues }: OpenCreateModalOptions) => { const modal = overlays.openModal( toMountPoint( @@ -55,6 +61,7 @@ export const getCreateModalOpener = onCreate(tag); }} tagClient={tagClient} + notifications={notifications} /> , { theme$: theme.theme$ } @@ -69,7 +76,7 @@ interface OpenEditModalOptions { } export const getEditModalOpener = - ({ overlays, theme, tagClient }: GetModalOpenerOptions) => + ({ overlays, theme, tagClient, notifications }: GetModalOpenerOptions) => async ({ tagId, onUpdate }: OpenEditModalOptions) => { const tag = await tagClient.get(tagId); @@ -86,6 +93,7 @@ export const getEditModalOpener = onUpdate(saved); }} tagClient={tagClient} + notifications={notifications} /> , { theme$: theme.theme$ } diff --git a/x-pack/plugins/saved_objects_tagging/public/components/edition_modal/use_validation.ts b/x-pack/plugins/saved_objects_tagging/public/components/edition_modal/use_validation.ts new file mode 100644 index 0000000000000..0d386e7aaa5a0 --- /dev/null +++ b/x-pack/plugins/saved_objects_tagging/public/components/edition_modal/use_validation.ts @@ -0,0 +1,143 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import { BehaviorSubject } from 'rxjs'; +import useObservable from 'react-use/lib/useObservable'; + +import { type TagValidation, validateTagName } from '../../../common'; +import type { ITagsClient, TagAttributes } from '../../../common/types'; +import { duplicateTagNameErrorMessage, validateTag } from './utils'; + +const initialValidation: TagValidation = { + valid: true, + warnings: [], + errors: {}, +}; + +export const useValidation = ({ + tagAttributes, + tagClient, + validateDuplicateNameOnMount = false, +}: { + tagAttributes: TagAttributes; + tagClient: ITagsClient; + validateDuplicateNameOnMount?: boolean; +}) => { + const isMounted = useRef(false); + const [validation, setValidation] = useState(initialValidation); + const { + errors: { name: nameError }, + } = validation; + + const validation$ = useMemo( + () => + new BehaviorSubject({ + isValidating: false, + hasDuplicateNameError: false, + }), + [] + ); + + const { isValidating = false } = useObservable(validation$) ?? {}; + + const setIsValidating = useCallback( + (value: boolean) => { + validation$.next({ + ...validation$.value, + isValidating: value, + }); + }, + [validation$] + ); + + const validateDuplicateTagName = useCallback( + async (name: string) => { + const error = validateTagName(name); + if (error) { + return; + } + + const existingTag = await tagClient.findByName(name, { exact: true }); + + if (existingTag) { + setValidation((prev) => ({ + ...prev, + valid: false, + errors: { + ...prev.errors, + name: duplicateTagNameErrorMessage, + }, + })); + } + + setIsValidating(false); + }, + [tagClient, setIsValidating] + ); + + const onNameChange = useCallback( + async ( + name: string, + { + debounced = false, + hasBeenModified = true, + }: { debounced?: boolean; hasBeenModified?: boolean } = {} + ) => { + setIsValidating(true); + + if (debounced) { + if (hasBeenModified) { + await validateDuplicateTagName(name); + } + setIsValidating(false); + } + }, + [setIsValidating, validateDuplicateTagName] + ); + + useEffect(() => { + if (isMounted.current) { + onNameChange(tagAttributes.name); + } + }, [onNameChange, tagAttributes.name]); + + useEffect(() => { + if (isMounted.current) { + setValidation(validateTag(tagAttributes)); + } + }, [tagAttributes]); + + useEffect(() => { + if (validateDuplicateNameOnMount && tagAttributes.name && !isMounted.current) { + setIsValidating(true); + validateDuplicateTagName(tagAttributes.name); + } + isMounted.current = true; + }, [ + validateDuplicateNameOnMount, + tagAttributes.name, + validateDuplicateTagName, + validation$, + setIsValidating, + ]); + + useEffect(() => { + validation$.next({ + ...validation$.value, + hasDuplicateNameError: nameError === duplicateTagNameErrorMessage, + }); + }, [nameError, validation$]); + + return { + validation, + setValidation, + isValidating, + validation$, + onNameChange, + }; +}; diff --git a/x-pack/plugins/saved_objects_tagging/public/components/edition_modal/utils.ts b/x-pack/plugins/saved_objects_tagging/public/components/edition_modal/utils.ts index 52258be0600f3..62ae1023c5b84 100644 --- a/x-pack/plugins/saved_objects_tagging/public/components/edition_modal/utils.ts +++ b/x-pack/plugins/saved_objects_tagging/public/components/edition_modal/utils.ts @@ -6,6 +6,8 @@ */ import { useCallback, useEffect, useRef } from 'react'; +import { i18n } from '@kbn/i18n'; + import { TagAttributes, TagValidation, @@ -21,6 +23,13 @@ export const getRandomColor = (): string => { return '#' + String(Math.floor(Math.random() * 16777215).toString(16)).padStart(6, '0'); }; +export const duplicateTagNameErrorMessage = i18n.translate( + 'xpack.savedObjectsTagging.validation.name.duplicateError', + { + defaultMessage: 'Name has already been taken.', + } +); + export const validateTag = (tag: TagAttributes): TagValidation => { const validation: TagValidation = { valid: true, diff --git a/x-pack/plugins/saved_objects_tagging/public/management/actions/edit.ts b/x-pack/plugins/saved_objects_tagging/public/management/actions/edit.ts index 57955c05a2ac4..f0b8e1ed5ec0d 100644 --- a/x-pack/plugins/saved_objects_tagging/public/management/actions/edit.ts +++ b/x-pack/plugins/saved_objects_tagging/public/management/actions/edit.ts @@ -27,7 +27,7 @@ export const getEditAction = ({ tagClient, fetchTags, }: GetEditActionOptions): TagAction => { - const editModalOpener = getEditModalOpener({ overlays, theme, tagClient }); + const editModalOpener = getEditModalOpener({ overlays, theme, tagClient, notifications }); return { id: 'edit', name: ({ name }) => diff --git a/x-pack/plugins/saved_objects_tagging/public/management/tag_management_page.tsx b/x-pack/plugins/saved_objects_tagging/public/management/tag_management_page.tsx index 3bad8d82b664c..8093d59a29bd2 100644 --- a/x-pack/plugins/saved_objects_tagging/public/management/tag_management_page.tsx +++ b/x-pack/plugins/saved_objects_tagging/public/management/tag_management_page.tsx @@ -75,8 +75,8 @@ export const TagManagementPage: FC = ({ }); const createModalOpener = useMemo( - () => getCreateModalOpener({ overlays, theme, tagClient }), - [overlays, theme, tagClient] + () => getCreateModalOpener({ overlays, theme, tagClient, notifications }), + [overlays, theme, tagClient, notifications] ); const tableActions = useMemo(() => { diff --git a/x-pack/plugins/saved_objects_tagging/public/plugin.ts b/x-pack/plugins/saved_objects_tagging/public/plugin.ts index 5c93917f476c5..e5ad1dfa5095f 100644 --- a/x-pack/plugins/saved_objects_tagging/public/plugin.ts +++ b/x-pack/plugins/saved_objects_tagging/public/plugin.ts @@ -67,7 +67,7 @@ export class SavedObjectTaggingPlugin return {}; } - public start({ http, application, overlays, theme, analytics }: CoreStart) { + public start({ http, application, overlays, theme, analytics, notifications }: CoreStart) { this.tagCache = new TagsCache({ refreshHandler: () => this.tagClient!.getAll({ asSystemRequest: true }), refreshInterval: this.config.cacheRefreshInterval, @@ -92,6 +92,7 @@ export class SavedObjectTaggingPlugin capabilities: getTagsCapabilities(application.capabilities), overlays, theme, + notifications, }), }; } diff --git a/x-pack/plugins/saved_objects_tagging/public/services/tags/tags_client.mock.ts b/x-pack/plugins/saved_objects_tagging/public/services/tags/tags_client.mock.ts index d7cc52b857dc0..0183be7fc9d21 100644 --- a/x-pack/plugins/saved_objects_tagging/public/services/tags/tags_client.mock.ts +++ b/x-pack/plugins/saved_objects_tagging/public/services/tags/tags_client.mock.ts @@ -15,6 +15,7 @@ const createInternalClientMock = () => { delete: jest.fn(), update: jest.fn(), find: jest.fn(), + findByName: jest.fn(), bulkDelete: jest.fn(), }; diff --git a/x-pack/plugins/saved_objects_tagging/public/services/tags/tags_client.ts b/x-pack/plugins/saved_objects_tagging/public/services/tags/tags_client.ts index 46aa43df7d075..224c658632523 100644 --- a/x-pack/plugins/saved_objects_tagging/public/services/tags/tags_client.ts +++ b/x-pack/plugins/saved_objects_tagging/public/services/tags/tags_client.ts @@ -173,6 +173,15 @@ export class TagsClient implements ITagInternalClient { return response; } + public async findByName(name: string, { exact }: { exact?: boolean } = { exact: false }) { + const { tags = [] } = await this.find({ page: 1, perPage: 10000, search: name }); + if (exact) { + const tag = tags.find((t) => t.name.toLocaleLowerCase() === name.toLocaleLowerCase()); + return tag ?? null; + } + return tags.length > 0 ? tags[0] : null; + } + public async bulkDelete(tagIds: string[]) { const startTime = window.performance.now(); await this.http.post<{}>('/internal/saved_objects_tagging/tags/_bulk_delete', { diff --git a/x-pack/plugins/saved_objects_tagging/public/ui_api/components.ts b/x-pack/plugins/saved_objects_tagging/public/ui_api/components.ts index 36f4f61f4d582..e6a504c49e87d 100644 --- a/x-pack/plugins/saved_objects_tagging/public/ui_api/components.ts +++ b/x-pack/plugins/saved_objects_tagging/public/ui_api/components.ts @@ -5,7 +5,7 @@ * 2.0. */ -import { OverlayStart, ThemeServiceStart } from '@kbn/core/public'; +import { NotificationsStart, OverlayStart, ThemeServiceStart } from '@kbn/core/public'; import { SavedObjectsTaggingApiUiComponent } from '@kbn/saved-objects-tagging-oss-plugin/public'; import { TagsCapabilities } from '../../common'; import { ITagInternalClient, ITagsCache } from '../services'; @@ -22,6 +22,7 @@ export interface GetComponentsOptions { overlays: OverlayStart; theme: ThemeServiceStart; tagClient: ITagInternalClient; + notifications: NotificationsStart; } export const getComponents = ({ @@ -30,8 +31,9 @@ export const getComponents = ({ overlays, theme, tagClient, + notifications, }: GetComponentsOptions): SavedObjectsTaggingApiUiComponent => { - const openCreateModal = getCreateModalOpener({ overlays, theme, tagClient }); + const openCreateModal = getCreateModalOpener({ overlays, theme, tagClient, notifications }); return { TagList: getConnectedTagListComponent({ cache }), TagSelector: getConnectedTagSelectorComponent({ cache, capabilities, openCreateModal }), diff --git a/x-pack/plugins/saved_objects_tagging/public/ui_api/index.ts b/x-pack/plugins/saved_objects_tagging/public/ui_api/index.ts index 8d53135f3f55a..b2dca68d5cc95 100644 --- a/x-pack/plugins/saved_objects_tagging/public/ui_api/index.ts +++ b/x-pack/plugins/saved_objects_tagging/public/ui_api/index.ts @@ -5,7 +5,7 @@ * 2.0. */ -import { OverlayStart, ThemeServiceStart } from '@kbn/core/public'; +import type { NotificationsStart, OverlayStart, ThemeServiceStart } from '@kbn/core/public'; import { SavedObjectsTaggingApiUi } from '@kbn/saved-objects-tagging-oss-plugin/public'; import { TagsCapabilities } from '../../common'; import { ITagsCache, ITagInternalClient } from '../services'; @@ -29,6 +29,7 @@ interface GetUiApiOptions { capabilities: TagsCapabilities; cache: ITagsCache; client: ITagInternalClient; + notifications: NotificationsStart; } export const getUiApi = ({ @@ -37,8 +38,16 @@ export const getUiApi = ({ client, overlays, theme, + notifications, }: GetUiApiOptions): SavedObjectsTaggingApiUi => { - const components = getComponents({ cache, capabilities, overlays, theme, tagClient: client }); + const components = getComponents({ + cache, + capabilities, + overlays, + theme, + tagClient: client, + notifications, + }); const getTagList = buildGetTagList(cache); diff --git a/x-pack/plugins/saved_objects_tagging/server/routes/internal/find_tags.ts b/x-pack/plugins/saved_objects_tagging/server/routes/internal/find_tags.ts index 32318f39b5a0c..8da060eda0cca 100644 --- a/x-pack/plugins/saved_objects_tagging/server/routes/internal/find_tags.ts +++ b/x-pack/plugins/saved_objects_tagging/server/routes/internal/find_tags.ts @@ -33,7 +33,7 @@ export const registerInternalFindTagsRoute = (router: TagsPluginRouter) => { perPage: query.perPage, search: query.search, type: [tagSavedObjectTypeName], - searchFields: ['title', 'description'], + searchFields: ['name', 'description'], }); const tags = findResponse.saved_objects.map(savedObjectToTag); diff --git a/x-pack/plugins/saved_objects_tagging/server/routes/tags/create_tag.ts b/x-pack/plugins/saved_objects_tagging/server/routes/tags/create_tag.ts index 7b9e6a32d3aea..0c48168eed281 100644 --- a/x-pack/plugins/saved_objects_tagging/server/routes/tags/create_tag.ts +++ b/x-pack/plugins/saved_objects_tagging/server/routes/tags/create_tag.ts @@ -24,6 +24,14 @@ export const registerCreateTagRoute = (router: TagsPluginRouter) => { router.handleLegacyErrors(async (ctx, req, res) => { try { const { tagsClient } = await ctx.tags; + + const existingTag = await tagsClient.findByName(req.body.name, { exact: true }); + if (existingTag) { + return res.conflict({ + body: `A tag with the name "${req.body.name}" already exists.`, + }); + } + const tag = await tagsClient.create(req.body); return res.ok({ body: { diff --git a/x-pack/plugins/saved_objects_tagging/server/routes/tags/update_tag.ts b/x-pack/plugins/saved_objects_tagging/server/routes/tags/update_tag.ts index 67b7d7a6acbda..62f8c73dddc78 100644 --- a/x-pack/plugins/saved_objects_tagging/server/routes/tags/update_tag.ts +++ b/x-pack/plugins/saved_objects_tagging/server/routes/tags/update_tag.ts @@ -28,6 +28,14 @@ export const registerUpdateTagRoute = (router: TagsPluginRouter) => { const { id } = req.params; try { const { tagsClient } = await ctx.tags; + + const existingTag = await tagsClient.findByName(req.body.name, { exact: true }); + if (existingTag && existingTag.id !== id) { + return res.conflict({ + body: `A tag with the name "${req.body.name}" already exists.`, + }); + } + const tag = await tagsClient.update(id, req.body); return res.ok({ body: { diff --git a/x-pack/plugins/saved_objects_tagging/server/services/tags/tags_client.mock.ts b/x-pack/plugins/saved_objects_tagging/server/services/tags/tags_client.mock.ts index e56bf2b963112..9507208971d84 100644 --- a/x-pack/plugins/saved_objects_tagging/server/services/tags/tags_client.mock.ts +++ b/x-pack/plugins/saved_objects_tagging/server/services/tags/tags_client.mock.ts @@ -14,6 +14,7 @@ const createClientMock = () => { getAll: jest.fn(), delete: jest.fn(), update: jest.fn(), + findByName: jest.fn(), }; return mock; diff --git a/x-pack/plugins/saved_objects_tagging/server/services/tags/tags_client.ts b/x-pack/plugins/saved_objects_tagging/server/services/tags/tags_client.ts index 23e1da56d6143..f213f279975a3 100644 --- a/x-pack/plugins/saved_objects_tagging/server/services/tags/tags_client.ts +++ b/x-pack/plugins/saved_objects_tagging/server/services/tags/tags_client.ts @@ -6,7 +6,7 @@ */ import { SavedObjectsClientContract } from '@kbn/core/server'; -import { CreateTagOptions } from '@kbn/saved-objects-tagging-oss-plugin/common/types'; +import { CreateTagOptions, Tag } from '@kbn/saved-objects-tagging-oss-plugin/common/types'; import { TagSavedObject, TagAttributes, ITagsClient } from '../../../common/types'; import { tagSavedObjectTypeName } from '../../../common/constants'; import { TagValidationError } from './errors'; @@ -63,6 +63,28 @@ export class TagsClient implements ITagsClient { return results.map(savedObjectToTag); } + public async findByName( + name: string, + { exact = false }: { exact?: boolean | undefined } = {} + ): Promise { + const response = await this.soClient.find({ + type: this.type, + search: name, + searchFields: ['name'], + perPage: 1000, + }); + + if (response.total === 0) { + return null; + } + + const tag = exact + ? response.saved_objects.find((t) => t.attributes.name.toLowerCase() === name.toLowerCase()) + : response.saved_objects[0]; + + return tag ? savedObjectToTag(tag) : null; + } + public async delete(id: string) { // `removeReferencesTo` security check is the same as a `delete` operation's, so we can use the scoped client here. // If that was to change, we would need to use the internal client instead. A FTR test is ensuring diff --git a/x-pack/test/saved_object_tagging/api_integration/security_and_spaces/apis/test_utils.ts b/x-pack/test/saved_object_tagging/api_integration/security_and_spaces/apis/test_utils.ts index ca821c491c9c4..39c9a8bb22b0b 100644 --- a/x-pack/test/saved_object_tagging/api_integration/security_and_spaces/apis/test_utils.ts +++ b/x-pack/test/saved_object_tagging/api_integration/security_and_spaces/apis/test_utils.ts @@ -40,11 +40,23 @@ export const createTags = async ({ getService }: FtrProviderContext) => { export const deleteTags = async ({ getService }: FtrProviderContext) => { const kibanaServer = getService('kibanaServer'); - await kibanaServer.importExport.unload( - 'x-pack/test/saved_object_tagging/common/fixtures/es_archiver/rbac_tags/default_space.json' - ); - await kibanaServer.importExport.unload( - 'x-pack/test/saved_object_tagging/common/fixtures/es_archiver/rbac_tags/space_1.json', - { space: 'space_1' } - ); + while (true) { + const defaultTags = await kibanaServer.savedObjects.find({ type: 'tag', space: 'default' }); + const spaceTags = await kibanaServer.savedObjects.find({ type: 'tag', space: 'space_1' }); + if (defaultTags.saved_objects.length === 0 && spaceTags.saved_objects.length === 0) { + await new Promise((resolve) => setTimeout(resolve, 1000)); + break; + } + if (defaultTags.saved_objects.length !== 0) { + await kibanaServer.savedObjects.bulkDelete({ + objects: defaultTags.saved_objects.map(({ type, id }) => ({ type, id })), + }); + } + if (spaceTags.saved_objects.length !== 0) { + await kibanaServer.savedObjects.bulkDelete({ + objects: spaceTags.saved_objects.map(({ type, id }) => ({ type, id })), + space: 'space_1', + }); + } + } }; diff --git a/x-pack/test/saved_object_tagging/api_integration/tagging_api/apis/create.ts b/x-pack/test/saved_object_tagging/api_integration/tagging_api/apis/create.ts index 4c867ca50b11e..60cbc51364813 100644 --- a/x-pack/test/saved_object_tagging/api_integration/tagging_api/apis/create.ts +++ b/x-pack/test/saved_object_tagging/api_integration/tagging_api/apis/create.ts @@ -59,6 +59,8 @@ export default function ({ getService }: FtrProviderContext) { }, }); }); + + await supertest.delete(`/api/saved_objects_tagging/tags/${newTagId}`); }); it('should return an error with details when validation failed', async () => { @@ -86,5 +88,24 @@ export default function ({ getService }: FtrProviderContext) { }); }); }); + + it('cannot create a new tag with existing name', async () => { + const existingName = 'tag-1'; + await supertest + .post(`/api/saved_objects_tagging/tags/create`) + .send({ + name: existingName, + description: 'some desc', + color: '#000000', + }) + .expect(409) + .then(({ body }) => { + expect(body).to.eql({ + statusCode: 409, + error: 'Conflict', + message: `A tag with the name "${existingName}" already exists.`, + }); + }); + }); }); } diff --git a/x-pack/test/saved_object_tagging/api_integration/tagging_api/apis/index.ts b/x-pack/test/saved_object_tagging/api_integration/tagging_api/apis/index.ts index f291d2537ed02..e8b16d5878f2d 100644 --- a/x-pack/test/saved_object_tagging/api_integration/tagging_api/apis/index.ts +++ b/x-pack/test/saved_object_tagging/api_integration/tagging_api/apis/index.ts @@ -14,6 +14,5 @@ export default function ({ loadTestFile }: FtrProviderContext) { loadTestFile(require.resolve('./create')); loadTestFile(require.resolve('./update')); loadTestFile(require.resolve('./bulk_assign')); - loadTestFile(require.resolve('./usage_collection')); }); } diff --git a/x-pack/test/saved_object_tagging/api_integration/tagging_api/apis/update.ts b/x-pack/test/saved_object_tagging/api_integration/tagging_api/apis/update.ts index 2efe8184ed7c9..b75513bb001da 100644 --- a/x-pack/test/saved_object_tagging/api_integration/tagging_api/apis/update.ts +++ b/x-pack/test/saved_object_tagging/api_integration/tagging_api/apis/update.ts @@ -61,6 +61,39 @@ export default function ({ getService }: FtrProviderContext) { }); }); + it('should not allow updating tag name to an existing name', async () => { + const existingName = 'tag-3'; + await supertest + .post(`/api/saved_objects_tagging/tags/tag-2`) + .send({ + name: existingName, + description: 'updated desc', + color: '#123456', + }) + .expect(409) + .then(({ body }) => { + expect(body).to.eql({ + statusCode: 409, + error: 'Conflict', + message: `A tag with the name "${existingName}" already exists.`, + }); + }); + + await supertest + .get(`/api/saved_objects_tagging/tags/tag-3`) + .expect(200) + .then(({ body }) => { + expect(body).to.eql({ + tag: { + id: 'tag-3', + name: 'tag-3', + description: 'Last but not least', + color: '#000000', + }, + }); + }); + }); + it('should return a 404 when trying to update a non existing tag', async () => { await supertest .post(`/api/saved_objects_tagging/tags/unknown-tag-id`) diff --git a/x-pack/test/saved_object_tagging/api_integration/tagging_usage_collection/config.ts b/x-pack/test/saved_object_tagging/api_integration/tagging_usage_collection/config.ts new file mode 100644 index 0000000000000..22edd56fffdbc --- /dev/null +++ b/x-pack/test/saved_object_tagging/api_integration/tagging_usage_collection/config.ts @@ -0,0 +1,36 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { FtrConfigProviderContext } from '@kbn/test'; +import { services } from './services'; + +// eslint-disable-next-line import/no-default-export +export default async function ({ readConfigFile }: FtrConfigProviderContext) { + const apiIntegrationConfig = await readConfigFile( + require.resolve('../../../api_integration/config.ts') + ); + + return { + testFiles: [require.resolve('./tests')], + servers: apiIntegrationConfig.get('servers'), + services, + junit: { + reportName: 'X-Pack Saved Object Tagging Usage Collection', + }, + esTestCluster: { + ...apiIntegrationConfig.get('esTestCluster'), + license: 'trial', + }, + kbnTestServer: { + ...apiIntegrationConfig.get('kbnTestServer'), + serverArgs: [ + ...apiIntegrationConfig.get('kbnTestServer.serverArgs'), + '--server.xsrf.disableProtection=true', + ], + }, + }; +} diff --git a/x-pack/test/saved_object_tagging/api_integration/tagging_usage_collection/services.ts b/x-pack/test/saved_object_tagging/api_integration/tagging_usage_collection/services.ts new file mode 100644 index 0000000000000..194d6ec533066 --- /dev/null +++ b/x-pack/test/saved_object_tagging/api_integration/tagging_usage_collection/services.ts @@ -0,0 +1,15 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { GenericFtrProviderContext } from '@kbn/test'; +import { services as apiIntegrationServices } from '../../../api_integration/services'; + +export const services = { + ...apiIntegrationServices, +}; + +export type FtrProviderContext = GenericFtrProviderContext; diff --git a/x-pack/test/saved_object_tagging/api_integration/tagging_api/apis/usage_collection.ts b/x-pack/test/saved_object_tagging/api_integration/tagging_usage_collection/tests.ts similarity index 94% rename from x-pack/test/saved_object_tagging/api_integration/tagging_api/apis/usage_collection.ts rename to x-pack/test/saved_object_tagging/api_integration/tagging_usage_collection/tests.ts index d5d042ad9f956..b8dab8d648333 100644 --- a/x-pack/test/saved_object_tagging/api_integration/tagging_api/apis/usage_collection.ts +++ b/x-pack/test/saved_object_tagging/api_integration/tagging_usage_collection/tests.ts @@ -6,7 +6,7 @@ */ import expect from '@kbn/expect'; -import { FtrProviderContext } from '../services'; +import { FtrProviderContext } from './services'; // eslint-disable-next-line import/no-default-export export default function ({ getService }: FtrProviderContext) { @@ -15,6 +15,7 @@ export default function ({ getService }: FtrProviderContext) { describe('saved_object_tagging usage collector data', () => { beforeEach(async () => { + await kibanaServer.savedObjects.cleanStandardList(); await kibanaServer.importExport.load( 'x-pack/test/saved_object_tagging/common/fixtures/es_archiver/usage_collection/data.json' );