Skip to content

Commit

Permalink
feat(metadata-sidebar): handle success events (#3789)
Browse files Browse the repository at this point in the history
* feat(metadata-sidebar): handle success events

* feat(metadata-sidebar): fix const

* feat(metadata-sidebar): remove redundant comments

* feat(metadata-sidebar): fix failing tests

* feat(metadata-sidebar): fix failing tests

* feat(metadata-sidebar): remove redundant onSuccess call

* feat(metadata-sidebar): add waitFor

* feat(metadata-sidebar): act instead of waitFor
  • Loading branch information
kajarosz authored Dec 18, 2024
1 parent dfe1392 commit f904c9f
Show file tree
Hide file tree
Showing 9 changed files with 69 additions and 23 deletions.
6 changes: 6 additions & 0 deletions src/common/types/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,11 @@ type ErrorContextProps = {

type ElementsErrorCallback = (e: ElementsXhrError, code: string, contextInfo?: Object) => void;

type ElementsSuccess = {
code: string,
showNotification: boolean,
};

type APIOptions = {
apiHost?: string,
cache?: APICache,
Expand All @@ -92,6 +97,7 @@ export type {
ElementOrigin,
ElementsError,
ElementsErrorCallback,
ElementsSuccess,
ElementsXhrError,
ErrorContextProps,
ErrorResponseData,
Expand Down
2 changes: 2 additions & 0 deletions src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ export const METADATA_SCOPE_GLOBAL = 'global';
export const METADATA_SCOPE_ENTERPRISE = 'enterprise';
export const METADATA_TEMPLATE_FETCH_LIMIT = API_PAGE_LIMIT;
export const METADATA_SUGGESTIONS_CONFIDENCE_EXPERIMENTAL = 'experimental';
export const SUCCESS_CODE_UPDATE_METADATA_TEMPLATE_INSTANCE = 'update_metadata_template_instance_success';
export const SUCCESS_CODE_DELETE_METADATA_TEMPLATE_INSTANCE = 'delete_metadata_template_instance_success';

/* ----------------------- Fields --------------------------- */
export const FIELD_ID = 'id';
Expand Down
8 changes: 7 additions & 1 deletion src/elements/content-sidebar/MetadataSidebarRedesign.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,14 @@ export interface ErrorContextProps {
onError: (error: Error, code: string, contextInfo?: Record<string, unknown>) => void;
}

export interface SuccessContextProps {
onSuccess: (code: string, showNotification: boolean) => void;
}

export interface MetadataSidebarRedesignProps
extends PropsWithoutContext,
ErrorContextProps,
SuccessContextProps,
WithLoggerProps,
RouteComponentProps {
api: API;
Expand All @@ -75,6 +80,7 @@ function MetadataSidebarRedesign({
filteredTemplateIds = [],
history,
onError,
onSuccess,
isFeatureEnabled,
}: MetadataSidebarRedesignProps) {
const {
Expand All @@ -87,7 +93,7 @@ function MetadataSidebarRedesign({
errorMessage,
status,
templateInstances,
} = useSidebarMetadataFetcher(api, fileId, onError, isFeatureEnabled);
} = useSidebarMetadataFetcher(api, fileId, onError, onSuccess, isFeatureEnabled);

const { formatMessage } = useIntl();
const isBoxAiSuggestionsEnabled: boolean = useFeatureEnabled('metadata.aiSuggestions.enabled');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ describe('elements/content-sidebar/Metadata/MetadataSidebarRedesign', () => {
filteredTemplateIds: emptyFilteredTemplateIds,
isFeatureEnabled: true,
onError: jest.fn(),
onSuccess: jest.fn(),
...routeComponentProps,
} satisfies MetadataSidebarRedesignProps;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import {
ERROR_CODE_EMPTY_METADATA_SUGGESTIONS,
ERROR_CODE_FETCH_METADATA_SUGGESTIONS,
FIELD_PERMISSIONS_CAN_UPLOAD,
SUCCESS_CODE_DELETE_METADATA_TEMPLATE_INSTANCE,
SUCCESS_CODE_UPDATE_METADATA_TEMPLATE_INSTANCE,
} from '../../../constants';
import useSidebarMetadataFetcher, { STATUS } from '../hooks/useSidebarMetadataFetcher';

Expand Down Expand Up @@ -117,13 +119,15 @@ const api = {

describe('useSidebarMetadataFetcher', () => {
const onErrorMock = jest.fn();
const onSuccessMock = jest.fn();
const isFeatureEnabledMock = true;

const setupHook = (fileId = '123') =>
renderHook(() => useSidebarMetadataFetcher(api, fileId, onErrorMock, isFeatureEnabledMock));
renderHook(() => useSidebarMetadataFetcher(api, fileId, onErrorMock, onSuccessMock, isFeatureEnabledMock));

beforeEach(() => {
onErrorMock.mockClear();
onSuccessMock.mockClear();
mockAPI.getFile.mockClear();
mockAPI.getMetadata.mockClear();
mockAPI.deleteMetadata.mockClear();
Expand Down Expand Up @@ -152,6 +156,7 @@ describe('useSidebarMetadataFetcher', () => {

expect(result.current.file).toBeUndefined();
expect(result.current.errorMessage).toBe(messages.sidebarMetadataEditingErrorContent);
expect(onSuccessMock).not.toHaveBeenCalled();
expect(onErrorMock).toHaveBeenCalledWith(
mockError,
'file_fetch_error',
Expand All @@ -175,6 +180,7 @@ describe('useSidebarMetadataFetcher', () => {

expect(result.current.templates).toBeNull();
expect(result.current.errorMessage).toBe(messages.sidebarMetadataFetchingErrorContent);
expect(onSuccessMock).not.toHaveBeenCalled();
expect(onErrorMock).toHaveBeenCalledWith(
mockError,
'metadata_fetch_error',
Expand All @@ -201,6 +207,7 @@ describe('useSidebarMetadataFetcher', () => {
expect(result.current.templates).toEqual(mockTemplates);
expect(result.current.status).toEqual(STATUS.SUCCESS);
expect(result.current.errorMessage).toBeNull();
expect(onSuccessMock).toHaveBeenCalledWith(SUCCESS_CODE_DELETE_METADATA_TEMPLATE_INSTANCE, true);
});

test('should handle metadata instance removal error', async () => {
Expand All @@ -217,6 +224,7 @@ describe('useSidebarMetadataFetcher', () => {
await waitFor(() => result.current.handleDeleteMetadataInstance(mockTemplateInstances[0]));

expect(result.current.status).toEqual(STATUS.ERROR);
expect(onSuccessMock).not.toHaveBeenCalled();
expect(onErrorMock).toHaveBeenCalledWith(
mockError,
'metadata_remove_error',
Expand All @@ -243,6 +251,7 @@ describe('useSidebarMetadataFetcher', () => {
await waitFor(() => result.current.handleCreateMetadataInstance(newTemplateInstance, successCallback));

expect(successCallback).toHaveBeenCalled();
expect(onSuccessMock).not.toHaveBeenCalled();
});

test('should handle metadata instance creation error', async () => {
Expand All @@ -259,6 +268,7 @@ describe('useSidebarMetadataFetcher', () => {
await waitFor(() => result.current.handleCreateMetadataInstance(newTemplateInstance, jest.fn()));

expect(result.current.status).toBe(STATUS.ERROR);
expect(onSuccessMock).not.toHaveBeenCalled();
expect(onErrorMock).toHaveBeenCalledWith(
mockError,
'metadata_creation_error',
Expand Down Expand Up @@ -286,6 +296,7 @@ describe('useSidebarMetadataFetcher', () => {
result.current.handleUpdateMetadataInstance(mockTemplateInstances[0], ops, successCallback),
);
expect(successCallback).toHaveBeenCalled();
expect(onSuccessMock).toHaveBeenCalledWith(SUCCESS_CODE_UPDATE_METADATA_TEMPLATE_INSTANCE, true);
});

test('should handle metadata update error', async () => {
Expand All @@ -305,6 +316,7 @@ describe('useSidebarMetadataFetcher', () => {
);

expect(successCallback).not.toHaveBeenCalled();
expect(onSuccessMock).not.toHaveBeenCalled();

expect(result.current.status).toEqual(STATUS.ERROR);
expect(result.current.templates).toEqual(mockTemplates);
Expand Down Expand Up @@ -346,6 +358,7 @@ describe('useSidebarMetadataFetcher', () => {
const suggestions = await result.current.extractSuggestions('templateKey', 'global');

expect(suggestions).toEqual([]);
expect(onSuccessMock).not.toHaveBeenCalled();
expect(onErrorMock).toHaveBeenCalledWith(
mockError,
ERROR_CODE_FETCH_METADATA_SUGGESTIONS,
Expand All @@ -362,6 +375,7 @@ describe('useSidebarMetadataFetcher', () => {
const suggestions = await result.current.extractSuggestions('templateKey', 'global');

expect(suggestions).toEqual([]);
expect(onSuccessMock).not.toHaveBeenCalled();
expect(onErrorMock).toHaveBeenCalledWith(
new Error('No suggestions found.'),
ERROR_CODE_EMPTY_METADATA_SUGGESTIONS,
Expand Down
38 changes: 23 additions & 15 deletions src/elements/content-sidebar/hooks/useSidebarMetadataFetcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@ import {
FIELD_IS_EXTERNALLY_OWNED,
FIELD_PERMISSIONS_CAN_UPLOAD,
FIELD_PERMISSIONS,
SUCCESS_CODE_UPDATE_METADATA_TEMPLATE_INSTANCE,
SUCCESS_CODE_DELETE_METADATA_TEMPLATE_INSTANCE,
} from '../../../constants';

import messages from '../../common/messages';

import { type BoxItem } from '../../../common/types/core';
import { type ErrorContextProps, type ExternalProps } from '../MetadataSidebarRedesign';
import { type ErrorContextProps, type ExternalProps, type SuccessContextProps } from '../MetadataSidebarRedesign';

export enum STATUS {
IDLE = 'idle',
Expand Down Expand Up @@ -55,6 +57,7 @@ function useSidebarMetadataFetcher(
api: API,
fileId: string,
onError: ErrorContextProps['onError'],
onSuccess: SuccessContextProps['onSuccess'],
isFeatureEnabled: ExternalProps['isFeatureEnabled'],
): DataFetcher {
const [status, setStatus] = React.useState<STATUS>(STATUS.IDLE);
Expand Down Expand Up @@ -149,14 +152,17 @@ function useSidebarMetadataFetcher(
await api.getMetadataAPI(false).deleteMetadata(
file,
metadataInstance,
() => setStatus(STATUS.SUCCESS),
() => {
setStatus(STATUS.SUCCESS);
onSuccess(SUCCESS_CODE_DELETE_METADATA_TEMPLATE_INSTANCE, true);
},
(error: ElementsXhrError, code: string) => {
onApiError(error, code, messages.sidebarMetadataEditingErrorContent);
},
true,
);
},
[api, onApiError, file],
[api, onApiError, onSuccess, file],
);

const handleCreateMetadataInstance = React.useCallback(
Expand All @@ -180,19 +186,20 @@ function useSidebarMetadataFetcher(
JSONPatch: JSONPatchOperations,
successCallback: () => void,
) => {
await api
.getMetadataAPI(false)
.updateMetadataRedesign(
file,
metadataInstance,
JSONPatch,
successCallback,
(error: ElementsXhrError, code: string) => {
onApiError(error, code, messages.sidebarMetadataEditingErrorContent);
},
);
await api.getMetadataAPI(false).updateMetadataRedesign(
file,
metadataInstance,
JSONPatch,
() => {
successCallback();
onSuccess(SUCCESS_CODE_UPDATE_METADATA_TEMPLATE_INSTANCE, true);
},
(error: ElementsXhrError, code: string) => {
onApiError(error, code, messages.sidebarMetadataEditingErrorContent);
},
);
},
[api, file, onApiError],
[api, file, onApiError, onSuccess],
);

const [, setError] = React.useState();
Expand Down Expand Up @@ -227,6 +234,7 @@ function useSidebarMetadataFetcher(

const templateInstance = templates.find(template => template.templateKey === templateKey && template.scope);
const fields = templateInstance?.fields || [];

return fields.map(field => {
const value = answer[field.key];
// TODO: @box/metadadata-editor does not support AI suggestions, enable once supported
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ const mockLogger = {
const defaultMetadataSidebarProps: ComponentProps<typeof MetadataSidebarRedesign> = {
isBoxAiSuggestionsEnabled: true,
isFeatureEnabled: true,
onError: fn,
onError: fn(),
onSuccess: fn(),
};

export default {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ export const mockMetadataInstances = {
$typeVersion: 1,
$template: 'myTemplate',
$scope: 'enterprise_173733877',
$templateKey: 'myTemplate',
myAttribute: 'My Value',
$canEdit: true,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,13 @@ const token = global.TOKEN;
const defaultMetadataArgs = {
fileId: fileIdWithMetadata,
isFeatureEnabled: true,
onError: fn,
onError: fn(),
onSuccess: fn(),
};
const defaultMetadataSidebarProps: ComponentProps<typeof MetadataSidebarRedesign> = {
isFeatureEnabled: true,
onError: fn,
onError: fn(),
onSuccess: fn(),
};
const mockFeatures = {
'metadata.redesign.enabled': true,
Expand Down Expand Up @@ -61,7 +63,8 @@ export const AddTemplateDropdownMenuOnEmpty = {
metadataSidebarProps: {
isBoxAiSuggestionsEnabled: true,
isFeatureEnabled: true,
onError: fn,
onError: fn(),
onSuccess: fn(),
},
},
play: async ({ canvasElement }) => {
Expand Down Expand Up @@ -277,11 +280,15 @@ export const DeleteButtonIsDisabledWhenAddingNewMetadataTemplate: StoryObj<typeo

const addTemplateButton = await canvas.findByRole('button', { name: 'Add template' });
expect(addTemplateButton).toBeInTheDocument();
await userEvent.click(addTemplateButton);
await act(async () => {
await userEvent.click(addTemplateButton);
});

const customMetadataOption = canvas.getByRole('option', { name: 'Virus Scan' });
expect(customMetadataOption).toBeInTheDocument();
await userEvent.click(customMetadataOption);
await act(async () => {
await userEvent.click(customMetadataOption);
});

const deleteButton = await canvas.findByRole('button', { name: 'Delete' });
expect(deleteButton).toBeDisabled();
Expand Down

0 comments on commit f904c9f

Please sign in to comment.