Skip to content

Commit

Permalink
[Tags] Prevent duplicates (#167072)
Browse files Browse the repository at this point in the history
  • Loading branch information
vadimkibana authored Sep 28, 2023
1 parent 6ac03d7 commit 726f212
Show file tree
Hide file tree
Showing 28 changed files with 451 additions and 42 deletions.
1 change: 1 addition & 0 deletions .buildkite/ftr_configs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions src/plugins/saved_objects_tagging_oss/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export interface ITagsClient {
create(attributes: TagAttributes, options?: CreateTagOptions): Promise<Tag>;
get(id: string): Promise<Tag>;
getAll(options?: GetAllTagsOptions): Promise<Tag[]>;
findByName(name: string, options?: { exact?: boolean }): Promise<Tag | null>;
delete(id: string): Promise<void>;
update(id: string, attributes: TagAttributes): Promise<Tag>;
}
1 change: 1 addition & 0 deletions src/plugins/saved_objects_tagging_oss/public/api.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const createClientMock = () => {
getAll: jest.fn(),
delete: jest.fn(),
update: jest.fn(),
findByName: jest.fn(),
};

return mock;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<TagAttributes>;
onClose: () => void;
onSave: (tag: Tag) => void;
tagClient: ITagsClient;
notifications: NotificationsStart;
}

const getDefaultAttributes = (providedDefaults?: Partial<TagAttributes>): TagAttributes => ({
Expand All @@ -26,22 +31,21 @@ const getDefaultAttributes = (providedDefaults?: Partial<TagAttributes>): TagAtt
...providedDefaults,
});

const initialValidation: TagValidation = {
valid: true,
warnings: [],
errors: {},
};

export const CreateTagModal: FC<CreateTagModalProps> = ({
defaultValues,
tagClient,
notifications,
onClose,
onSave,
}) => {
const [validation, setValidation] = useState<TagValidation>(initialValidation);
const [tagAttributes, setTagAttributes] = useState<TagAttributes>(
getDefaultAttributes(defaultValues)
);
const { validation, setValidation, onNameChange, validation$, isValidating } = useValidation({
tagAttributes,
tagClient,
validateDuplicateNameOnMount: true,
});

const setField = useCallback(
<T extends keyof TagAttributes>(field: T) =>
Expand All @@ -55,6 +59,14 @@ export const CreateTagModal: FC<CreateTagModalProps> = ({
);

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) {
Expand All @@ -68,18 +80,27 @@ export const CreateTagModal: FC<CreateTagModalProps> = ({
// 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 (
<CreateOrEditModal
onClose={onClose}
onSubmit={onSubmit}
onNameChange={onNameChange}
mode={'create'}
tag={tagAttributes}
setField={setField}
validation={validation}
isValidating={isValidating}
/>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -40,23 +41,31 @@ import { getRandomColor, useIfMounted } from './utils';
interface CreateOrEditModalProps {
onClose: () => void;
onSubmit: () => Promise<void>;
onNameChange: (
name: string,
options?: { debounced?: boolean; hasBeenModified?: boolean }
) => Promise<void>;
mode: 'create' | 'edit';
tag: TagAttributes;
validation: TagValidation;
isValidating: boolean;
setField: <T extends keyof TagAttributes>(field: T) => (value: TagAttributes[T]) => void;
}

export const CreateOrEditModal: FC<CreateOrEditModalProps> = ({
onClose,
onSubmit,
onNameChange,
validation,
isValidating,
setField,
tag,
mode,
}) => {
const optionalMessageId = htmlIdGenerator()();
const ifMounted = useIfMounted();
const [submitting, setSubmitting] = useState<boolean>(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
Expand All @@ -68,6 +77,8 @@ export const CreateOrEditModal: FC<CreateOrEditModalProps> = ({
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]);
Expand All @@ -91,6 +102,15 @@ export const CreateOrEditModal: FC<CreateOrEditModalProps> = ({
});
}, [ifMounted, onSubmit]);

useDebounce(
() => {
onNameChange(tag.name, { debounced: true, hasBeenModified: nameHasBeenModified });
lastNameValue.current = tag.name;
},
300,
[tag.name, nameHasBeenModified]
);

return (
<EuiModal onClose={onClose} initialFocus="[name=name]" style={{ minWidth: '600px' }}>
<EuiModalHeader>
Expand Down Expand Up @@ -130,6 +150,7 @@ export const CreateOrEditModal: FC<CreateOrEditModalProps> = ({
maxLength={tagNameMaxLength}
value={tag.name}
onChange={(e) => setName(e.target.value)}
isLoading={isValidating}
data-test-subj="createModalField-name"
/>
</EuiFormRow>
Expand Down Expand Up @@ -238,6 +259,7 @@ export const CreateOrEditModal: FC<CreateOrEditModalProps> = ({
fill
data-test-subj="createModalConfirmButton"
onClick={onFormSubmit}
isLoading={submitting}
isDisabled={submitting || (isEdit && !tagHasBeenModified)}
>
{isEdit ? (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<EditTagModalProps> = ({ tag, onSave, onClose, tagClient }) => {
const [validation, setValidation] = useState<TagValidation>(initialValidation);
export const EditTagModal: FC<EditTagModalProps> = ({
tag,
onSave,
onClose,
tagClient,
notifications,
}) => {
const [tagAttributes, setTagAttributes] = useState<TagAttributes>(getAttributes(tag));
const { validation, setValidation, onNameChange, isValidating, validation$ } = useValidation({
tagAttributes,
tagClient,
});

const setField = useCallback(
<T extends keyof TagAttributes>(field: T) =>
Expand All @@ -46,8 +53,17 @@ export const EditTagModal: FC<EditTagModalProps> = ({ 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;
}
Expand All @@ -59,18 +75,27 @@ export const EditTagModal: FC<EditTagModalProps> = ({ 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 (
<CreateOrEditModal
onClose={onClose}
onSubmit={onSubmit}
onNameChange={onNameChange}
mode={'edit'}
tag={tagAttributes}
setField={setField}
validation={validation}
isValidating={isValidating}
/>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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(
Expand All @@ -55,6 +61,7 @@ export const getCreateModalOpener =
onCreate(tag);
}}
tagClient={tagClient}
notifications={notifications}
/>
</React.Suspense>,
{ theme$: theme.theme$ }
Expand All @@ -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);

Expand All @@ -86,6 +93,7 @@ export const getEditModalOpener =
onUpdate(saved);
}}
tagClient={tagClient}
notifications={notifications}
/>
</React.Suspense>,
{ theme$: theme.theme$ }
Expand Down
Loading

0 comments on commit 726f212

Please sign in to comment.