From b840aec66a8421a2e71e9e51b3ac187b0fa0ee3c Mon Sep 17 00:00:00 2001 From: Jerry Jiang Date: Thu, 21 Aug 2025 18:02:02 -0500 Subject: [PATCH 1/5] feat(metadata-view): Multi value display and onSave in metadata sidepanel --- i18n/en-US.properties | 12 + src/api/Metadata.js | 32 +- src/api/__tests__/Metadata.test.js | 3 + src/elements/common/messages.js | 35 ++ .../content-explorer/ContentExplorer.tsx | 440 +++++++++++------- .../MetadataQueryAPIHelper.ts | 36 +- .../content-explorer/MetadataSidePanel.tsx | 84 +++- .../__tests__/MetadataQueryAPIHelper.test.ts | 13 +- .../__tests__/MetadataSidePanel.test.tsx | 185 +++++++- .../tests/MetadataView-visual.stories.tsx | 31 ++ src/elements/content-explorer/utils.ts | 119 ++++- 11 files changed, 766 insertions(+), 224 deletions(-) diff --git a/i18n/en-US.properties b/i18n/en-US.properties index 5a572fdd4d..fce8808092 100644 --- a/i18n/en-US.properties +++ b/i18n/en-US.properties @@ -452,10 +452,14 @@ be.emptyView.selectedState = You haven't selected any items yet. be.error = Error # Default label for signifying error in the sub header. be.errorBreadcrumb = Error +# Text shown in error notification banner +be.errorNotification = Unable to save changes. Please try again # Title when an error occurs be.errorOccured = An error occurred # Message to the user when the open with element errors be.errorOpenWithDescription = Opening this file with other services is currently unavailable +# Aria Label for type icon in the error notification banner +be.errorTypeIcon = Error # Header message to the user when an Open With integration fails to execute be.executeIntegrationOpenWithErrorHeader = We’re sorry, this integration is currently unavailable. # Sub header message to the user when an Open With integration fails to execute @@ -588,6 +592,8 @@ be.modifiedDate = Modified {date} be.modifiedDateBy = Modified {date} by {name} # Label for a button that displays more options be.moreOptions = More options +# Display text for field when there are multiple items selected and have different value +be.multipleValues = Multiple Values # Name ascending option shown in the share access drop down select. be.nameASC = Name: A → Z # Name descending option shown in the share access drop down select. @@ -606,6 +612,8 @@ be.noActivity = No activity to show be.noActivityAnnotationPrompt = Hover over the preview and use the controls at the bottom to annotate the file. # Message shown in be.noActivityCommentPrompt = Comment and @mention people to notify them. +# Aria Label for close icon in the notification banner +be.notificationCloseIcon = Close # Text shown to indicate the number of files selected be.numFilesSelected = {numSelected, plural, =0 {0 files selected} one {1 file selected} other {# files selected} } # Label for open action. @@ -834,6 +842,10 @@ be.skillUnknownError = Something went wrong with running this skill or fetching be.sort = Sort # Label for status skill card in the preview sidebar be.statusSkill = Status +# Text shown in success notification banner +be.successNotification = {numSelected, plural, =1 {1 document updated} other {# documents updated} } +# Aria Label for type icon in the success notification banner +be.successTypeIcon = Success # Shown instead of todays date. be.today = today # Label for keywords/topics skill section in the preview sidebar diff --git a/src/api/Metadata.js b/src/api/Metadata.js index 9a0ff296fb..aec57c56a8 100644 --- a/src/api/Metadata.js +++ b/src/api/Metadata.js @@ -16,7 +16,7 @@ import partition from 'lodash/partition'; import uniq from 'lodash/uniq'; import uniqueId from 'lodash/uniqueId'; import { getBadItemError, getBadPermissionsError, isUserCorrectableError } from '../utils/error'; -import { getTypedFileId } from '../utils/file'; +import { getTypedFileId, getTypedFolderId } from '../utils/file'; import { handleOnAbort, formatMetadataFieldValue } from './utils'; import File from './File'; import { @@ -115,6 +115,21 @@ class Metadata extends File { return baseUrl; } + /** + * API URL for metadata + * + * @param {string} id - a Box folder id + * @param {string} field - metadata field + * @return {string} base url for files + */ + getMetadataUrlForFolder(id: string, scope?: string, template?: string): string { + const baseUrl = `${this.getBaseApiUrl()}/folders/${id}/metadata`; + if (scope && template) { + return `${baseUrl}/${scope}/${template}`; + } + return baseUrl; + } + /** * API URL for metadata templates for a scope * @@ -810,9 +825,9 @@ class Metadata extends File { } /** - * API for patching metadata on file + * API for patching metadata on item (file/folder) * - * @param {BoxItem} file - File object for which we are changing the description + * @param {BoxItem} item - File/Folder object for which we are changing the description * @param {Object} template - Metadata template * @param {Array} operations - Array of JSON patch operations * @param {Function} successCallback - Success callback @@ -820,7 +835,7 @@ class Metadata extends File { * @return {Promise} */ async updateMetadata( - file: BoxItem, + item: BoxItem, template: MetadataTemplate, operations: JSONPatchOperations, successCallback: Function, @@ -830,7 +845,7 @@ class Metadata extends File { this.successCallback = successCallback; this.errorCallback = errorCallback; - const { id, permissions } = file; + const { id, permissions } = item; if (!id || !permissions) { this.errorHandler(getBadItemError()); return; @@ -845,11 +860,14 @@ class Metadata extends File { try { const metadata = await this.xhr.put({ - url: this.getMetadataUrl(id, template.scope, template.templateKey), + url: + item.type === 'file' + ? this.getMetadataUrl(id, template.scope, template.templateKey) + : this.getMetadataUrlForFolder(id, template.scope, template.templateKey), headers: { [HEADER_CONTENT_TYPE]: 'application/json-patch+json', }, - id: getTypedFileId(id), + id: item.type === 'file' ? getTypedFileId(id) : getTypedFolderId(id), data: operations, }); if (!this.isDestroyed()) { diff --git a/src/api/__tests__/Metadata.test.js b/src/api/__tests__/Metadata.test.js index fd90a02099..9b8609459b 100644 --- a/src/api/__tests__/Metadata.test.js +++ b/src/api/__tests__/Metadata.test.js @@ -1700,6 +1700,7 @@ describe('api/Metadata', () => { permissions: { can_upload: true, }, + type: 'file', }; const ops = [{ op: 'add' }, { op: 'test' }]; const cache = new Cache(); @@ -1767,6 +1768,7 @@ describe('api/Metadata', () => { permissions: { can_upload: true, }, + type: 'file', }; const ops = [{ op: 'add' }, { op: 'test' }]; const cache = new Cache(); @@ -1833,6 +1835,7 @@ describe('api/Metadata', () => { permissions: { can_upload: true, }, + type: 'file', }; const ops = [{ op: 'add' }, { op: 'test' }]; const cache = new Cache(); diff --git a/src/elements/common/messages.js b/src/elements/common/messages.js index 2557ab6241..f2fda7f425 100644 --- a/src/elements/common/messages.js +++ b/src/elements/common/messages.js @@ -1105,6 +1105,41 @@ const messages = defineMessages({ } `, }, + multipleValues: { + id: 'be.multipleValues', + description: 'Display text for field when there are multiple items selected and have different value', + defaultMessage: 'Multiple Values', + }, + notificationCloseIcon: { + id: 'be.notificationCloseIcon', + description: 'Aria Label for close icon in the notification banner', + defaultMessage: 'Close', + }, + errorTypeIcon: { + id: 'be.errorTypeIcon', + description: 'Aria Label for type icon in the error notification banner', + defaultMessage: 'Error', + }, + successTypeIcon: { + id: 'be.successTypeIcon', + description: 'Aria Label for type icon in the success notification banner', + defaultMessage: 'Success', + }, + errorNotification: { + id: 'be.errorNotification', + description: 'Text shown in error notification banner', + defaultMessage: 'Unable to save changes. Please try again', + }, + successNotification: { + id: 'be.successNotification', + description: 'Text shown in success notification banner', + defaultMessage: ` + {numSelected, plural, + =1 {1 document updated} + other {# documents updated} + } + `, + }, }); export default messages; diff --git a/src/elements/content-explorer/ContentExplorer.tsx b/src/elements/content-explorer/ContentExplorer.tsx index 4011268574..34987007f7 100644 --- a/src/elements/content-explorer/ContentExplorer.tsx +++ b/src/elements/content-explorer/ContentExplorer.tsx @@ -8,9 +8,11 @@ import getProp from 'lodash/get'; import noop from 'lodash/noop'; import throttle from 'lodash/throttle'; import uniqueid from 'lodash/uniqueId'; -import { TooltipProvider } from '@box/blueprint-web'; +import { Notification, TooltipProvider } from '@box/blueprint-web'; import { AxiosRequestConfig, AxiosResponse } from 'axios'; import type { Key, Selection } from 'react-aria-components'; +import type { MetadataTemplateField } from '@box/metadata-editor'; +import type { MetadataFieldType } from '@box/metadata-view'; import CreateFolderDialog from '../common/create-folder-dialog'; import UploadDialog from '../common/upload-dialog'; @@ -77,6 +79,7 @@ import { import type { ViewMode } from '../common/flowTypes'; import type { ItemAction } from '../common/item'; import type { Theme } from '../common/theming'; +import type { JSONPatchOperations } from '../../common/types/api'; import type { MetadataQuery, FieldsToShow } from '../../common/types/metadataQueries'; import type { MetadataFieldValue, MetadataTemplate } from '../../common/types/metadata'; import type { @@ -95,6 +98,7 @@ import type { BulkItemAction } from '../common/sub-header/BulkItemActionMenu'; import type { ContentPreviewProps } from '../content-preview'; import type { ContentUploaderProps } from '../content-uploader'; import type { MetadataViewContainerProps } from './MetadataViewContainer'; +import { isMultiValuesField } from './utils'; import '../common/fonts.scss'; import '../common/base.scss'; @@ -492,6 +496,80 @@ class ContentExplorer extends Component { ); } + /** + * Get operations for all fields update in the metadata sidepanel + * + * @private + * @return {JSONPatchOperations} + */ + getOperations = ( + item: BoxItem, + templateOldFields: MetadataTemplateField[], + templateNewFields: MetadataTemplateField[], + ): JSONPatchOperations => { + const { scope, templateKey } = this.state.metadataTemplate; + const itemFields = item.metadata[scope][templateKey]; + const operations = []; + templateNewFields.forEach(newField => { + let newFieldValue = newField.value; + const { key, type } = newField; + // when retrieve value from float type field, it gives a string instead + if (type === 'float' && newFieldValue !== '') { + newFieldValue = Number(newFieldValue); + } + const oldField = templateOldFields.find(f => f.key === key); + const oldFieldValue = oldField.value; + + let fieldOperations = []; + /* + Generate operations array based on all the fields' orignal value and the incoming updated value. + + Edge Case: + If there are multiple items shared different value for enum or multi-select field, the form will + return 'Multiple values' as the value. In this case, it needs to generate operation based on the + actual item's field value. + */ + if ( + isMultiValuesField(type as MetadataFieldType, oldFieldValue) && + !isMultiValuesField(type as MetadataFieldType, newFieldValue) + ) { + fieldOperations = this.metadataQueryAPIHelper.createJSONPatchOperations( + key, + itemFields[key], + newFieldValue, + ); + } else { + fieldOperations = this.metadataQueryAPIHelper.createJSONPatchOperations( + key, + oldFieldValue, + newFieldValue, + ); + } + operations.push(...fieldOperations); + }); + return operations; + }; + + /** + * Update item's metadata instance with either operations or old and new value for one specific field + * + * @private + * @return {void} + */ + updateMetadataV2 = async ( + item: BoxItem, + operations: JSONPatchOperations, + successCallback?: () => void, + errorCallback?: ErrorCallback, + ) => { + await this.metadataQueryAPIHelper.updateMetadataWithOperations( + item, + operations, + successCallback, + errorCallback, + ); + }; + /** * Resets the collection so that the loading bar starts showing * @@ -1785,188 +1863,194 @@ class ContentExplorer extends Component { /* eslint-disable jsx-a11y/no-noninteractive-tabindex */ return ( - -
- -
-
- {!isDefaultViewMetadata && ( -
+ + + +
+ +
+
+ {!isDefaultViewMetadata && ( +
+ )} + + + + + + {!isErrorView && ( +
+ +
+ )} +
+ {isDefaultViewMetadata && isMetadataViewV2Feature && isMetadataSidePanelOpen && ( + )} - - + {allowUpload && !!this.appElement ? ( + - - - - {!isErrorView && ( -
- -
- )} -
- {isDefaultViewMetadata && isMetadataViewV2Feature && isMetadataSidePanelOpen && ( - + ) : null} + {canRename && selected && !!this.appElement ? ( + + ) : null} + {canShare && selected && !!this.appElement ? ( + + ) : null} + {canPreview && selected && !!this.appElement ? ( + - )} + ) : null}
- {allowUpload && !!this.appElement ? ( - - ) : null} - {allowCreate && !!this.appElement ? ( - - ) : null} - {canDelete && selected && !!this.appElement ? ( - - ) : null} - {canRename && selected && !!this.appElement ? ( - - ) : null} - {canShare && selected && !!this.appElement ? ( - - ) : null} - {canPreview && selected && !!this.appElement ? ( - - ) : null} -
- + + ); /* eslint-enable jsx-a11y/no-static-element-interactions */ diff --git a/src/elements/content-explorer/MetadataQueryAPIHelper.ts b/src/elements/content-explorer/MetadataQueryAPIHelper.ts index ba091a856e..bf2f9e1c05 100644 --- a/src/elements/content-explorer/MetadataQueryAPIHelper.ts +++ b/src/elements/content-explorer/MetadataQueryAPIHelper.ts @@ -3,7 +3,6 @@ import find from 'lodash/find'; import getProp from 'lodash/get'; import includes from 'lodash/includes'; import isArray from 'lodash/isArray'; -import isNil from 'lodash/isNil'; import API from '../../api'; import { @@ -14,7 +13,7 @@ import { METADATA_FIELD_TYPE_ENUM, METADATA_FIELD_TYPE_MULTISELECT, } from '../../common/constants'; -import { FIELD_NAME, FIELD_METADATA, FIELD_EXTENSION } from '../../constants'; +import { FIELD_NAME, FIELD_METADATA, FIELD_EXTENSION, FIELD_PERMISSIONS } from '../../constants'; import type { MetadataQuery as MetadataQueryType, MetadataQueryResponseData } from '../../common/types/metadataQueries'; import type { @@ -26,6 +25,7 @@ import type { } from '../../common/types/metadata'; import type { ElementsXhrError, JSONPatchOperations } from '../../common/types/api'; import type { Collection, BoxItem } from '../../common/types/core'; +import { isEmptyValue } from './utils'; type SuccessCallback = (metadataQueryCollection: Collection, metadataTemplate: MetadataTemplate) => void; type ErrorCallback = (e: ElementsXhrError) => void; @@ -57,13 +57,25 @@ export default class MetadataQueryAPIHelper { oldValue: MetadataFieldValue | null, newValue: MetadataFieldValue | null, ): JSONPatchOperations => { + // check if two values are the same, return empty operations if so + if ( + (isEmptyValue(oldValue) && isEmptyValue(newValue)) || + (Array.isArray(oldValue) && + Array.isArray(newValue) && + oldValue.length === newValue.length && + oldValue.every(val => newValue.includes(val))) || + oldValue === newValue + ) { + return []; + } + let operation = JSON_PATCH_OP_REPLACE; - if (isNil(oldValue) && newValue) { + if (isEmptyValue(oldValue) && !isEmptyValue(newValue)) { operation = JSON_PATCH_OP_ADD; } - if (oldValue && isNil(newValue)) { + if (!isEmptyValue(oldValue) && isEmptyValue(newValue)) { operation = JSON_PATCH_OP_REMOVE; } @@ -205,6 +217,17 @@ export default class MetadataQueryAPIHelper { .updateMetadata(file, this.metadataTemplate, operations, successCallback, errorCallback); }; + updateMetadataWithOperations = ( + item: BoxItem, + operations: JSONPatchOperations, + successCallback: () => void, + errorCallback: ErrorCallback, + ): Promise => { + return this.api + .getMetadataAPI(true) + .updateMetadata(item, this.metadataTemplate, operations, successCallback, errorCallback); + }; + /** * Verify that the metadata query has required fields and update it if necessary * For a file item, default fields included in the response are "type", "id", "etag" @@ -225,6 +248,11 @@ export default class MetadataQueryAPIHelper { clonedFields.push(FIELD_EXTENSION); } + // This field is necessary to check if the user has permission to update metadata + if (!clonedFields.includes(FIELD_PERMISSIONS)) { + clonedFields.push(FIELD_PERMISSIONS); + } + clonedQuery.fields = clonedFields; return clonedQuery; diff --git a/src/elements/content-explorer/MetadataSidePanel.tsx b/src/elements/content-explorer/MetadataSidePanel.tsx index 707b841a8e..fcf5628fac 100644 --- a/src/elements/content-explorer/MetadataSidePanel.tsx +++ b/src/elements/content-explorer/MetadataSidePanel.tsx @@ -1,7 +1,7 @@ import React, { useState } from 'react'; import { useIntl } from 'react-intl'; -import { IconButton, SidePanel, Text } from '@box/blueprint-web'; +import { IconButton, SidePanel, Text, useNotification } from '@box/blueprint-web'; import { XMark } from '@box/blueprint-web-assets/icons/Fill/index'; import { FileDefault } from '@box/blueprint-web-assets/icons/Line/index'; import { @@ -10,12 +10,13 @@ import { JSONPatchOperations, MetadataInstance, MetadataInstanceForm, + type MetadataTemplateField, } from '@box/metadata-editor'; import type { Selection } from 'react-aria-components'; -import type { Collection } from '../../common/types/core'; +import type { BoxItem, Collection } from '../../common/types/core'; import type { MetadataTemplate } from '../../common/types/metadata'; -import { getTemplateInstance, useSelectedItemText } from './utils'; +import { useTemplateInstance, useSelectedItemText } from './utils'; import messages from '../common/messages'; @@ -23,17 +24,33 @@ import './MetadataSidePanel.scss'; export interface MetadataSidePanelProps { currentCollection: Collection; - onClose: () => void; + getOperations: ( + item: BoxItem, + templateOldFields: MetadataTemplateField[], + templateNewFields: MetadataTemplateField[], + ) => JSONPatchOperations; metadataTemplate: MetadataTemplate; + onClose: () => void; + refreshCollection: () => void; + updateMetadataV2: ( + file: BoxItem, + operations: JSONPatchOperations, + successCallback?: () => void, + errorCallback?: ErrorCallback, + ) => Promise; selectedItemIds: Selection; } const MetadataSidePanel = ({ currentCollection, + getOperations, + metadataTemplate, onClose, + refreshCollection, + updateMetadataV2, selectedItemIds, - metadataTemplate, }: MetadataSidePanelProps) => { + const { addNotification } = useNotification(); const { formatMessage } = useIntl(); const [isEditing, setIsEditing] = useState(false); const [isUnsavedChangesModalOpen, setIsUnsavedChangesModalOpen] = useState(false); @@ -43,7 +60,7 @@ const MetadataSidePanel = ({ selectedItemIds === 'all' ? currentCollection.items : currentCollection.items.filter(item => selectedItemIds.has(item.id)); - const templateInstance = getTemplateInstance(metadataTemplate, selectedItems); + const templateInstance = useTemplateInstance(metadataTemplate, selectedItems, isEditing); const handleMetadataInstanceEdit = () => { setIsEditing(true); @@ -53,19 +70,58 @@ const MetadataSidePanel = ({ setIsEditing(false); }; - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const handleMetadataInstanceFormChange = (values: FormValues) => { - // TODO: Implement on form change - }; - const handleMetadataInstanceFormDiscardUnsavedChanges = () => { setIsUnsavedChangesModalOpen(false); setIsEditing(false); }; - // eslint-disable-next-line @typescript-eslint/no-unused-vars + const handleUpdateMetadataSuccess = () => { + addNotification({ + closeButtonAriaLabel: formatMessage(messages.notificationCloseIcon), + sensitivity: 'foreground', + styledText: formatMessage(messages.successNotification, { numSelected: selectedItems.length }), + typeIconAriaLabel: formatMessage(messages.successTypeIcon), + variant: 'success', + }); + setIsEditing(false); + refreshCollection(); + }; + + const handleUpdateMetadataError = () => { + addNotification({ + closeButtonAriaLabel: formatMessage(messages.notificationCloseIcon), + sensitivity: 'foreground', + styledText: formatMessage(messages.errorNotification), + typeIconAriaLabel: formatMessage(messages.errorTypeIcon), + variant: 'error', + }); + }; + const handleMetadataInstanceFormSubmit = async (values: FormValues, operations: JSONPatchOperations) => { - // TODO: Implement onSave callback + if (selectedItems.length === 1) { + await updateMetadataV2( + selectedItems[0], + operations, + handleUpdateMetadataSuccess, + handleUpdateMetadataError, + ); + } else { + const { fields: templateNewFields } = values.metadata; + const { fields: templateOldFields } = templateInstance; + + try { + // Process items sequentially to avoid conflicts + await selectedItems.reduce(async (previousPromise, item) => { + await previousPromise; + + const ops = getOperations(item, templateOldFields, templateNewFields); + await updateMetadataV2(item, ops); + }, Promise.resolve()); + handleUpdateMetadataSuccess(); + } catch (error) { + handleUpdateMetadataError(); + } + } }; return ( @@ -99,7 +155,7 @@ const MetadataSidePanel = ({ isUnsavedChangesModalOpen={isUnsavedChangesModalOpen} selectedTemplateInstance={templateInstance} onCancel={handleMetadataInstanceFormCancel} - onChange={handleMetadataInstanceFormChange} + onChange={null} onDelete={null} onDiscardUnsavedChanges={handleMetadataInstanceFormDiscardUnsavedChanges} onSubmit={handleMetadataInstanceFormSubmit} diff --git a/src/elements/content-explorer/__tests__/MetadataQueryAPIHelper.test.ts b/src/elements/content-explorer/__tests__/MetadataQueryAPIHelper.test.ts index b996a6265d..9fa66481c6 100644 --- a/src/elements/content-explorer/__tests__/MetadataQueryAPIHelper.test.ts +++ b/src/elements/content-explorer/__tests__/MetadataQueryAPIHelper.test.ts @@ -8,7 +8,7 @@ import { JSON_PATCH_OP_REPLACE, JSON_PATCH_OP_TEST, } from '../../../common/constants'; -import { FIELD_METADATA, FIELD_NAME, FIELD_EXTENSION } from '../../../constants'; +import { FIELD_METADATA, FIELD_NAME, FIELD_EXTENSION, FIELD_PERMISSIONS } from '../../../constants'; describe('features/metadata-based-view/MetadataQueryAPIHelper', () => { let metadataQueryAPIHelper; @@ -426,27 +426,30 @@ describe('features/metadata-based-view/MetadataQueryAPIHelper', () => { expect(isArray(updatedMetadataQuery.fields)).toBe(true); expect(includes(updatedMetadataQuery.fields, FIELD_NAME)).toBe(true); expect(includes(updatedMetadataQuery.fields, FIELD_EXTENSION)).toBe(true); + expect(includes(updatedMetadataQuery.fields, FIELD_PERMISSIONS)).toBe(true); if (index === 2) { - // Verify "name" and "extension" are added to pre-existing fields + // Verify "name", "extension" and "permission" are added to pre-existing fields expect(updatedMetadataQuery.fields).toEqual([ ...mdQueryWithoutNameField.fields, FIELD_NAME, FIELD_EXTENSION, + FIELD_PERMISSIONS, ]); } if (index === 4) { - // Verify "extension" is added when "name" exists but "extension" doesn't + // Verify "extension" and "permission" are added when "name" exists but "extension" and "permission" don't expect(updatedMetadataQuery.fields).toEqual([ ...mdQueryWithoutExtensionField.fields, FIELD_EXTENSION, + FIELD_PERMISSIONS, ]); } if (index === 5) { - // No change, original query has all necessary fields - expect(updatedMetadataQuery.fields).toEqual(mdQueryWithBothFields.fields); + // Verify "permission" is added + expect(updatedMetadataQuery.fields).toEqual([...mdQueryWithBothFields.fields, FIELD_PERMISSIONS]); } }, ); diff --git a/src/elements/content-explorer/__tests__/MetadataSidePanel.test.tsx b/src/elements/content-explorer/__tests__/MetadataSidePanel.test.tsx index 7b811b38e0..dae02a8681 100644 --- a/src/elements/content-explorer/__tests__/MetadataSidePanel.test.tsx +++ b/src/elements/content-explorer/__tests__/MetadataSidePanel.test.tsx @@ -1,6 +1,7 @@ import * as React from 'react'; import userEvent from '@testing-library/user-event'; -import { render, screen } from '../../../test-utils/testing-library'; +import { Notification } from '@box/blueprint-web'; +import { render, screen, waitFor } from '../../../test-utils/testing-library'; import MetadataSidePanel, { type MetadataSidePanelProps } from '../MetadataSidePanel'; // Mock scrollTo method @@ -65,13 +66,21 @@ const mockOnClose = jest.fn(); describe('elements/content-explorer/MetadataSidePanel', () => { const defaultProps: MetadataSidePanelProps = { currentCollection: mockCollection, - onClose: mockOnClose, + getOperations: jest.fn(), metadataTemplate: mockMetadataTemplate, + onClose: mockOnClose, + refreshCollection: jest.fn(), + updateMetadataV2: jest.fn(), selectedItemIds: new Set(['1']), }; const renderComponent = (props: Partial = {}) => - render(); + render( + + + + , + ); test('renders the metadata title', () => { renderComponent(); @@ -124,4 +133,174 @@ describe('elements/content-explorer/MetadataSidePanel', () => { const submitButton = screen.getByRole('button', { name: 'Save' }); expect(submitButton).toBeInTheDocument(); }); + + test('switches back to view mode when cancel button is clicked', async () => { + renderComponent(); + const editTemplateButton = screen.getByLabelText('Edit Mock Template'); + await userEvent.click(editTemplateButton); + + const cancelButton = screen.getByRole('button', { name: 'Cancel' }); + await userEvent.click(cancelButton); + + // Should be back in view mode + expect(screen.getByLabelText('Edit Mock Template')).toBeInTheDocument(); + }); + + test('calls updateMetadataV2 when form is submitted for single item', async () => { + const mockUpdateMetadata = jest.fn().mockResolvedValue(undefined); + renderComponent({ updateMetadataV2: mockUpdateMetadata }); + + const editTemplateButton = screen.getByLabelText('Edit Mock Template'); + await userEvent.click(editTemplateButton); + + const submitButton = screen.getByRole('button', { name: 'Save' }); + await userEvent.click(submitButton); + + expect(mockUpdateMetadata).toHaveBeenCalledWith( + mockCollection.items[0], + expect.any(Array), + expect.any(Function), + expect.any(Function), + ); + }); + + test('calls getOperations and updateMetadataV2 for each item when multiple items are selected', async () => { + const mockUpdateMetadata = jest.fn().mockResolvedValue(undefined); + const mockGetOperations = jest.fn().mockReturnValue([]); + + renderComponent({ + selectedItemIds: new Set(['1', '2']), + updateMetadataV2: mockUpdateMetadata, + getOperations: mockGetOperations, + }); + + const editTemplateButton = screen.getByLabelText('Edit Mock Template'); + await userEvent.click(editTemplateButton); + + const submitButton = screen.getByRole('button', { name: 'Save' }); + await userEvent.click(submitButton); + + await waitFor(() => { + expect(mockGetOperations).toHaveBeenCalledTimes(2); + expect(mockUpdateMetadata).toHaveBeenCalledTimes(2); + }); + }); + + test('displays success notification when metadata update succeeds', async () => { + const mockUpdateMetadata = jest.fn().mockImplementation((_, __, successCallback) => { + successCallback(); + return Promise.resolve(); + }); + const mockRefreshCollection = jest.fn(); + + renderComponent({ + updateMetadataV2: mockUpdateMetadata, + refreshCollection: mockRefreshCollection, + }); + + const editTemplateButton = screen.getByLabelText('Edit Mock Template'); + await userEvent.click(editTemplateButton); + + const submitButton = screen.getByRole('button', { name: 'Save' }); + await userEvent.click(submitButton); + + await waitFor(() => { + expect(screen.getByText('1 document updated')).toBeInTheDocument(); + expect(mockRefreshCollection).toHaveBeenCalledTimes(1); + expect(screen.getByLabelText('Edit Mock Template')).toBeInTheDocument(); // Back to view mode + }); + }); + + test('displays error notification when metadata update fails', async () => { + const mockUpdateMetadata = jest.fn().mockImplementation((_, __, ___, errorCallback) => { + errorCallback(); + return Promise.resolve(); + }); + + renderComponent({ updateMetadataV2: mockUpdateMetadata }); + + const editTemplateButton = screen.getByLabelText('Edit Mock Template'); + await userEvent.click(editTemplateButton); + + const submitButton = screen.getByRole('button', { name: 'Save' }); + await userEvent.click(submitButton); + + await waitFor(() => { + expect(screen.getByText('Unable to save changes. Please try again')).toBeInTheDocument(); + }); + }); + + test('displays error notification when multiple item update fails', async () => { + const mockUpdateMetadata = jest.fn().mockRejectedValue(new Error('Update failed')); + const mockGetOperations = jest.fn().mockReturnValue([]); + + renderComponent({ + selectedItemIds: new Set(['1', '2']), + updateMetadataV2: mockUpdateMetadata, + getOperations: mockGetOperations, + }); + + const editTemplateButton = screen.getByLabelText('Edit Mock Template'); + await userEvent.click(editTemplateButton); + + const submitButton = screen.getByRole('button', { name: 'Save' }); + await userEvent.click(submitButton); + + await waitFor(() => { + expect(screen.getByText('Unable to save changes. Please try again')).toBeInTheDocument(); + }); + }); + + test('handles "all" selection correctly', () => { + renderComponent({ selectedItemIds: 'all' }); + const subtitle = screen.getByText('2 files selected'); + expect(subtitle).toBeInTheDocument(); + }); + + test('displays "Multiple Values" for items with different field values', () => { + const collectionWithDifferentValues = { + ...mockCollection, + items: [ + { + ...mockCollection.items[0], + metadata: { + enterprise_123: { + mockTemplate: { + alias: 'value-1', + }, + }, + }, + }, + { + ...mockCollection.items[1], + metadata: { + enterprise_123: { + mockTemplate: { + alias: 'value-2', + }, + }, + }, + }, + ], + }; + + renderComponent({ + currentCollection: collectionWithDifferentValues, + selectedItemIds: new Set(['1', '2']), + }); + + expect(screen.getByText('Multiple Values')).toBeInTheDocument(); + }); + + test('renders correct accessibility attributes', () => { + renderComponent(); + + // Close button should have proper aria-label + const closeButton = screen.getByLabelText('Close'); + expect(closeButton).toHaveAttribute('aria-label', 'Close'); + + // Edit button should have proper aria-label + const editButton = screen.getByLabelText('Edit Mock Template'); + expect(editButton).toHaveAttribute('aria-label', 'Edit Mock Template'); + }); }); diff --git a/src/elements/content-explorer/stories/tests/MetadataView-visual.stories.tsx b/src/elements/content-explorer/stories/tests/MetadataView-visual.stories.tsx index f3d56bd0b3..c854553462 100644 --- a/src/elements/content-explorer/stories/tests/MetadataView-visual.stories.tsx +++ b/src/elements/content-explorer/stories/tests/MetadataView-visual.stories.tsx @@ -238,6 +238,37 @@ export const metadataViewV2WithBulkItemActionMenuShowsItemActionMenu: Story = { }, }; +export const sidePanelOpenWithMultipleItemsSelected: Story = { + args: { + ...metadataViewV2ElementProps, + metadataViewProps: { + columns, + tableProps: { + isSelectAllEnabled: true, + }, + }, + }, + + play: async ({ canvas }) => { + await waitFor(() => { + expect(canvas.getByRole('row', { name: /Child 2/i })).toBeInTheDocument(); + }); + + // Select the first row by clicking its checkbox + const firstItem = canvas.getByRole('row', { name: /Child 2/i }); + const checkbox = within(firstItem).getByRole('checkbox'); + await userEvent.click(checkbox); + + // Select the second row by clicking its checkbox + const secondItem = canvas.getAllByRole('row', { name: /Child 1/i })[0]; + const secondCheckbox = within(secondItem).getByRole('checkbox'); + await userEvent.click(secondCheckbox); + + const metadataButton = canvas.getByRole('button', { name: 'Metadata' }); + await userEvent.click(metadataButton); + }, +}; + const meta: Meta = { title: 'Elements/ContentExplorer/tests/MetadataView/visual', component: ContentExplorer, diff --git a/src/elements/content-explorer/utils.ts b/src/elements/content-explorer/utils.ts index 4c3abf0c6c..bffb625cf3 100644 --- a/src/elements/content-explorer/utils.ts +++ b/src/elements/content-explorer/utils.ts @@ -1,12 +1,23 @@ -import { useMemo } from 'react'; +import React, { useMemo } from 'react'; import { useIntl } from 'react-intl'; +import isNil from 'lodash/isNil'; -import type { MetadataTemplate } from '@box/metadata-editor'; +import { + MULTI_VALUE_DEFAULT_OPTION, + MULTI_VALUE_DEFAULT_VALUE, + type MetadataTemplate, + type MetadataFormFieldValue, +} from '@box/metadata-editor'; +import type { MetadataFieldType } from '@box/metadata-view'; import type { Selection } from 'react-aria-components'; import type { BoxItem, Collection } from '../../common/types/core'; import messages from '../common/messages'; +// Specific type for metadata field value in the item +// Note: Item doesn't have field value in metadata object if that field is not set, so the value will be undefined in this case +type ItemMetadataFieldValue = string | number | Array | undefined; + // Get selected item text export function useSelectedItemText(currentCollection: Collection, selectedItemIds: Selection): string { const { formatMessage } = useIntl(); @@ -28,21 +39,103 @@ export function useSelectedItemText(currentCollection: Collection, selectedItemI }, [currentCollection.items, formatMessage, selectedItemIds]); } +// Check if the field value is empty. +// Note: 0 doesn't represent empty here because of float type field +export function isEmptyValue(value: ItemMetadataFieldValue) { + if (isNil(value)) return true; + return value === '' || (Array.isArray(value) && value.length === 0) || Number.isNaN(value); +} + +// Check if the field values are equal based on the field types +export function areFieldValuesEqual( + fieldType: MetadataFieldType, + value1: ItemMetadataFieldValue, + value2: ItemMetadataFieldValue, +) { + if (isEmptyValue(value1) && isEmptyValue(value2)) return true; + + // Handle multiSelect arrays comparison + if (fieldType === 'multiSelect' && Array.isArray(value1) && Array.isArray(value2)) { + return value1.length === value2.length && value1.every(val => value2.includes(val)); + } + + return value1 === value2; +} + +// Set the field value in Metadata Form based on the field type +function setFieldValue(fieldType: MetadataFieldType, fieldValue: ItemMetadataFieldValue) { + if (fieldValue) return fieldValue; + if (fieldType === 'multiSelect') return []; + if (fieldType === 'enum') return ''; + return fieldValue; +} + +// Check if the field value in Metadata Form is multi-values such as "Multiple values" +export function isMultiValuesField(fieldType: MetadataFieldType, fieldValue: MetadataFormFieldValue) { + if (fieldType === 'multiSelect') { + return Array.isArray(fieldValue) && fieldValue.length === 1 && fieldValue[0] === MULTI_VALUE_DEFAULT_VALUE; + } + if (fieldType === 'enum') { + return fieldValue === MULTI_VALUE_DEFAULT_VALUE; + } + return false; +} + // Get template instance based on metadata template and selected items -export function getTemplateInstance(metadataTemplate: MetadataTemplate, selectedItems: BoxItem[]) { +export function useTemplateInstance(metadataTemplate: MetadataTemplate, selectedItems: BoxItem[], isEditing: boolean) { + const { formatMessage } = useIntl(); const { displayName, fields, hidden, id, scope, templateKey, type } = metadataTemplate; const selectedItemsFields = fields.map( - ({ displayName: fieldDisplayName, hidden: fieldHidden, id: fieldId, key, options, type: fieldType }) => ({ - displayName: fieldDisplayName, - hidden: fieldHidden, - id: fieldId, - key, - options, - type: fieldType, - // TODO: Add support for multiple selected items - value: selectedItems[0].metadata[scope][templateKey][key], - }), + ({ displayName: fieldDisplayName, hidden: fieldHidden, id: fieldId, key, options, type: fieldType }) => { + const firstSelectedItem = selectedItems[0]; + let fieldValue = firstSelectedItem.metadata[scope][templateKey][key]; + + if (selectedItems.length > 1) { + const allItemsHaveSameInitialValue = selectedItems.every(selectedItem => + areFieldValuesEqual( + fieldType as MetadataFieldType, + selectedItem.metadata[scope][templateKey][key], + fieldValue, + ), + ); + + // Update field display value when selected items have different values + if (!allItemsHaveSameInitialValue) { + if (isEditing) { + // Add MultiValue Option if the field is multiSelect or enum + if (fieldType === 'multiSelect' || fieldType === 'enum') { + fieldValue = fieldType === 'enum' ? MULTI_VALUE_DEFAULT_VALUE : [MULTI_VALUE_DEFAULT_VALUE]; + const multiValueOption = options?.find(option => option.key === MULTI_VALUE_DEFAULT_VALUE); + if (!multiValueOption) { + options?.push(MULTI_VALUE_DEFAULT_OPTION); + } + } else { + fieldValue = setFieldValue(fieldType as MetadataFieldType, undefined); + } + } else { + /** + * We want to show "Multiple values" label for multiple dates across files selection. + * We use fragment here to bypass check in shared feature. + * This feature tries to parse string as date if the string is passed as value. + */ + const multipleValuesText = formatMessage(messages.multipleValues); + fieldValue = React.createElement(React.Fragment, null, multipleValuesText); + } + } else { + fieldValue = setFieldValue(fieldType as MetadataFieldType, fieldValue); + } + } + return { + displayName: fieldDisplayName, + hidden: fieldHidden, + id: fieldId, + key, + options, + type: fieldType, + value: fieldValue, + }; + }, ); return { From 0a6578a994db55c0099d358454e0cece9a535904 Mon Sep 17 00:00:00 2001 From: Jerry Jiang Date: Tue, 26 Aug 2025 16:48:45 -0500 Subject: [PATCH 2/5] feat(metadata-view): update comments --- i18n/en-US.properties | 16 ++---- src/api/Metadata.js | 5 +- src/elements/common/messages.js | 28 +++------- .../content-explorer/ContentExplorer.tsx | 46 +--------------- .../MetadataQueryAPIHelper.ts | 55 +++++++++++++++++-- .../content-explorer/MetadataSidePanel.tsx | 29 +++++----- .../__tests__/MetadataSidePanel.test.tsx | 16 +++--- src/elements/content-explorer/utils.ts | 41 ++++++++++++-- 8 files changed, 126 insertions(+), 110 deletions(-) diff --git a/i18n/en-US.properties b/i18n/en-US.properties index fce8808092..b2711211e1 100644 --- a/i18n/en-US.properties +++ b/i18n/en-US.properties @@ -452,14 +452,10 @@ be.emptyView.selectedState = You haven't selected any items yet. be.error = Error # Default label for signifying error in the sub header. be.errorBreadcrumb = Error -# Text shown in error notification banner -be.errorNotification = Unable to save changes. Please try again # Title when an error occurs be.errorOccured = An error occurred # Message to the user when the open with element errors be.errorOpenWithDescription = Opening this file with other services is currently unavailable -# Aria Label for type icon in the error notification banner -be.errorTypeIcon = Error # Header message to the user when an Open With integration fails to execute be.executeIntegrationOpenWithErrorHeader = We’re sorry, this integration is currently unavailable. # Sub header message to the user when an Open With integration fails to execute @@ -586,6 +582,10 @@ be.messageCenter.previewError = Sorry, we're having trouble showing this image. be.messageCenter.product = Product # Title for the message center modal be.messageCenter.title = What's New +# Text shown in error notification banner +be.metadataUpdateErrorNotification = Unable to save changes. Please try again +# Text shown in success notification banner +be.metadataUpdateSuccessNotification = {numSelected, plural, =1 {1 document updated} other {# documents updated} } # Text for modified date with modified prefix. be.modifiedDate = Modified {date} # Text for modified date with user with modified prefix. @@ -612,8 +612,6 @@ be.noActivity = No activity to show be.noActivityAnnotationPrompt = Hover over the preview and use the controls at the bottom to annotate the file. # Message shown in be.noActivityCommentPrompt = Comment and @mention people to notify them. -# Aria Label for close icon in the notification banner -be.notificationCloseIcon = Close # Text shown to indicate the number of files selected be.numFilesSelected = {numSelected, plural, =0 {0 files selected} one {1 file selected} other {# files selected} } # Label for open action. @@ -842,10 +840,8 @@ be.skillUnknownError = Something went wrong with running this skill or fetching be.sort = Sort # Label for status skill card in the preview sidebar be.statusSkill = Status -# Text shown in success notification banner -be.successNotification = {numSelected, plural, =1 {1 document updated} other {# documents updated} } -# Aria Label for type icon in the success notification banner -be.successTypeIcon = Success +# Generic success label. +be.success = Success # Shown instead of todays date. be.today = today # Label for keywords/topics skill section in the preview sidebar diff --git a/src/api/Metadata.js b/src/api/Metadata.js index aec57c56a8..a93aab410e 100644 --- a/src/api/Metadata.js +++ b/src/api/Metadata.js @@ -859,15 +859,16 @@ class Metadata extends File { } try { + const { type } = item; const metadata = await this.xhr.put({ url: - item.type === 'file' + type === 'file' ? this.getMetadataUrl(id, template.scope, template.templateKey) : this.getMetadataUrlForFolder(id, template.scope, template.templateKey), headers: { [HEADER_CONTENT_TYPE]: 'application/json-patch+json', }, - id: item.type === 'file' ? getTypedFileId(id) : getTypedFolderId(id), + id: type === 'file' ? getTypedFileId(id) : getTypedFolderId(id), data: operations, }); if (!this.isDestroyed()) { diff --git a/src/elements/common/messages.js b/src/elements/common/messages.js index f2fda7f425..294eecd0c1 100644 --- a/src/elements/common/messages.js +++ b/src/elements/common/messages.js @@ -27,6 +27,11 @@ const messages = defineMessages({ description: 'Generic error label.', defaultMessage: 'Error', }, + success: { + id: 'be.success', + description: 'Generic success label.', + defaultMessage: 'Success', + }, preview: { id: 'be.preview', description: 'Label for preview action.', @@ -1110,28 +1115,13 @@ const messages = defineMessages({ description: 'Display text for field when there are multiple items selected and have different value', defaultMessage: 'Multiple Values', }, - notificationCloseIcon: { - id: 'be.notificationCloseIcon', - description: 'Aria Label for close icon in the notification banner', - defaultMessage: 'Close', - }, - errorTypeIcon: { - id: 'be.errorTypeIcon', - description: 'Aria Label for type icon in the error notification banner', - defaultMessage: 'Error', - }, - successTypeIcon: { - id: 'be.successTypeIcon', - description: 'Aria Label for type icon in the success notification banner', - defaultMessage: 'Success', - }, - errorNotification: { - id: 'be.errorNotification', + metadataUpdateErrorNotification: { + id: 'be.metadataUpdateErrorNotification', description: 'Text shown in error notification banner', defaultMessage: 'Unable to save changes. Please try again', }, - successNotification: { - id: 'be.successNotification', + metadataUpdateSuccessNotification: { + id: 'be.metadataUpdateSuccessNotification', description: 'Text shown in success notification banner', defaultMessage: ` {numSelected, plural, diff --git a/src/elements/content-explorer/ContentExplorer.tsx b/src/elements/content-explorer/ContentExplorer.tsx index 34987007f7..26549ecebd 100644 --- a/src/elements/content-explorer/ContentExplorer.tsx +++ b/src/elements/content-explorer/ContentExplorer.tsx @@ -12,7 +12,6 @@ import { Notification, TooltipProvider } from '@box/blueprint-web'; import { AxiosRequestConfig, AxiosResponse } from 'axios'; import type { Key, Selection } from 'react-aria-components'; import type { MetadataTemplateField } from '@box/metadata-editor'; -import type { MetadataFieldType } from '@box/metadata-view'; import CreateFolderDialog from '../common/create-folder-dialog'; import UploadDialog from '../common/upload-dialog'; @@ -98,7 +97,6 @@ import type { BulkItemAction } from '../common/sub-header/BulkItemActionMenu'; import type { ContentPreviewProps } from '../content-preview'; import type { ContentUploaderProps } from '../content-uploader'; import type { MetadataViewContainerProps } from './MetadataViewContainer'; -import { isMultiValuesField } from './utils'; import '../common/fonts.scss'; import '../common/base.scss'; @@ -507,47 +505,7 @@ class ContentExplorer extends Component { templateOldFields: MetadataTemplateField[], templateNewFields: MetadataTemplateField[], ): JSONPatchOperations => { - const { scope, templateKey } = this.state.metadataTemplate; - const itemFields = item.metadata[scope][templateKey]; - const operations = []; - templateNewFields.forEach(newField => { - let newFieldValue = newField.value; - const { key, type } = newField; - // when retrieve value from float type field, it gives a string instead - if (type === 'float' && newFieldValue !== '') { - newFieldValue = Number(newFieldValue); - } - const oldField = templateOldFields.find(f => f.key === key); - const oldFieldValue = oldField.value; - - let fieldOperations = []; - /* - Generate operations array based on all the fields' orignal value and the incoming updated value. - - Edge Case: - If there are multiple items shared different value for enum or multi-select field, the form will - return 'Multiple values' as the value. In this case, it needs to generate operation based on the - actual item's field value. - */ - if ( - isMultiValuesField(type as MetadataFieldType, oldFieldValue) && - !isMultiValuesField(type as MetadataFieldType, newFieldValue) - ) { - fieldOperations = this.metadataQueryAPIHelper.createJSONPatchOperations( - key, - itemFields[key], - newFieldValue, - ); - } else { - fieldOperations = this.metadataQueryAPIHelper.createJSONPatchOperations( - key, - oldFieldValue, - newFieldValue, - ); - } - operations.push(...fieldOperations); - }); - return operations; + return this.metadataQueryAPIHelper.generateOperations(item, templateOldFields, templateNewFields); }; /** @@ -1952,9 +1910,9 @@ class ContentExplorer extends Component { getOperations={this.getOperations} metadataTemplate={metadataTemplate} onClose={this.closeMetadataSidePanel} + onUpdate={this.updateMetadataV2} refreshCollection={this.refreshCollection} selectedItemIds={selectedItemIds} - updateMetadataV2={this.updateMetadataV2} /> )}
diff --git a/src/elements/content-explorer/MetadataQueryAPIHelper.ts b/src/elements/content-explorer/MetadataQueryAPIHelper.ts index bf2f9e1c05..477d566a93 100644 --- a/src/elements/content-explorer/MetadataQueryAPIHelper.ts +++ b/src/elements/content-explorer/MetadataQueryAPIHelper.ts @@ -3,7 +3,11 @@ import find from 'lodash/find'; import getProp from 'lodash/get'; import includes from 'lodash/includes'; import isArray from 'lodash/isArray'; +import xor from 'lodash/xor'; +import type { MetadataTemplateField } from '@box/metadata-editor'; +import type { MetadataFieldType } from '@box/metadata-view'; import API from '../../api'; +import { isEmptyValue, isMultiValuesField } from './utils'; import { JSON_PATCH_OP_ADD, @@ -25,7 +29,6 @@ import type { } from '../../common/types/metadata'; import type { ElementsXhrError, JSONPatchOperations } from '../../common/types/api'; import type { Collection, BoxItem } from '../../common/types/core'; -import { isEmptyValue } from './utils'; type SuccessCallback = (metadataQueryCollection: Collection, metadataTemplate: MetadataTemplate) => void; type ErrorCallback = (e: ElementsXhrError) => void; @@ -60,10 +63,7 @@ export default class MetadataQueryAPIHelper { // check if two values are the same, return empty operations if so if ( (isEmptyValue(oldValue) && isEmptyValue(newValue)) || - (Array.isArray(oldValue) && - Array.isArray(newValue) && - oldValue.length === newValue.length && - oldValue.every(val => newValue.includes(val))) || + (Array.isArray(oldValue) && Array.isArray(newValue) && xor(oldValue, newValue).length === 0) || oldValue === newValue ) { return []; @@ -182,6 +182,51 @@ export default class MetadataQueryAPIHelper { return this.api.getMetadataAPI(true).getSchemaByTemplateKey(this.templateKey); }; + /** + * Generate operations for all fields update in the metadata sidepanel + * + * @private + * @return {JSONPatchOperations} + */ + generateOperations = ( + item: BoxItem, + templateOldFields: MetadataTemplateField[], + templateNewFields: MetadataTemplateField[], + ): JSONPatchOperations => { + const { scope, templateKey } = this.metadataTemplate; + const itemFields = item.metadata[scope][templateKey]; + const operations = templateNewFields.flatMap(newField => { + let newFieldValue = newField.value; + const { key, type } = newField; + // when retrieve value from float type field, it gives a string instead + if (type === 'float' && newFieldValue !== '') { + newFieldValue = Number(newFieldValue); + } + const oldField = templateOldFields.find(f => f.key === key); + const oldFieldValue = oldField.value; + + /* + Generate operations array based on all the fields' orignal value and the incoming updated value. + + Edge Case: + If there are multiple items shared different value for enum or multi-select field, the form will + return 'Multiple values' as the value. In this case, it needs to generate operation based on the + actual item's field value. + */ + const shouldUseItemFieldValue = + isMultiValuesField(type as MetadataFieldType, oldFieldValue) && + !isMultiValuesField(type as MetadataFieldType, newFieldValue); + + return this.createJSONPatchOperations( + key, + shouldUseItemFieldValue ? itemFields[key] : oldFieldValue, + newFieldValue, + ); + }); + + return operations; + }; + queryMetadata = (): Promise => { return new Promise((resolve, reject) => { this.api.getMetadataQueryAPI().queryMetadata(this.metadataQuery, resolve, reject, { forceFetch: true }); diff --git a/src/elements/content-explorer/MetadataSidePanel.tsx b/src/elements/content-explorer/MetadataSidePanel.tsx index fcf5628fac..0c3ae89eb5 100644 --- a/src/elements/content-explorer/MetadataSidePanel.tsx +++ b/src/elements/content-explorer/MetadataSidePanel.tsx @@ -31,13 +31,13 @@ export interface MetadataSidePanelProps { ) => JSONPatchOperations; metadataTemplate: MetadataTemplate; onClose: () => void; - refreshCollection: () => void; - updateMetadataV2: ( + onUpdate: ( file: BoxItem, operations: JSONPatchOperations, successCallback?: () => void, errorCallback?: ErrorCallback, ) => Promise; + refreshCollection: () => void; selectedItemIds: Selection; } @@ -46,8 +46,8 @@ const MetadataSidePanel = ({ getOperations, metadataTemplate, onClose, + onUpdate, refreshCollection, - updateMetadataV2, selectedItemIds, }: MetadataSidePanelProps) => { const { addNotification } = useNotification(); @@ -77,10 +77,12 @@ const MetadataSidePanel = ({ const handleUpdateMetadataSuccess = () => { addNotification({ - closeButtonAriaLabel: formatMessage(messages.notificationCloseIcon), + closeButtonAriaLabel: formatMessage(messages.close), sensitivity: 'foreground', - styledText: formatMessage(messages.successNotification, { numSelected: selectedItems.length }), - typeIconAriaLabel: formatMessage(messages.successTypeIcon), + styledText: formatMessage(messages.metadataUpdateSuccessNotification, { + numSelected: selectedItems.length, + }), + typeIconAriaLabel: formatMessage(messages.success), variant: 'success', }); setIsEditing(false); @@ -89,22 +91,17 @@ const MetadataSidePanel = ({ const handleUpdateMetadataError = () => { addNotification({ - closeButtonAriaLabel: formatMessage(messages.notificationCloseIcon), + closeButtonAriaLabel: formatMessage(messages.close), sensitivity: 'foreground', - styledText: formatMessage(messages.errorNotification), - typeIconAriaLabel: formatMessage(messages.errorTypeIcon), + styledText: formatMessage(messages.metadataUpdateErrorNotification), + typeIconAriaLabel: formatMessage(messages.error), variant: 'error', }); }; const handleMetadataInstanceFormSubmit = async (values: FormValues, operations: JSONPatchOperations) => { if (selectedItems.length === 1) { - await updateMetadataV2( - selectedItems[0], - operations, - handleUpdateMetadataSuccess, - handleUpdateMetadataError, - ); + await onUpdate(selectedItems[0], operations, handleUpdateMetadataSuccess, handleUpdateMetadataError); } else { const { fields: templateNewFields } = values.metadata; const { fields: templateOldFields } = templateInstance; @@ -115,7 +112,7 @@ const MetadataSidePanel = ({ await previousPromise; const ops = getOperations(item, templateOldFields, templateNewFields); - await updateMetadataV2(item, ops); + await onUpdate(item, ops); }, Promise.resolve()); handleUpdateMetadataSuccess(); } catch (error) { diff --git a/src/elements/content-explorer/__tests__/MetadataSidePanel.test.tsx b/src/elements/content-explorer/__tests__/MetadataSidePanel.test.tsx index dae02a8681..a3d630f769 100644 --- a/src/elements/content-explorer/__tests__/MetadataSidePanel.test.tsx +++ b/src/elements/content-explorer/__tests__/MetadataSidePanel.test.tsx @@ -69,8 +69,8 @@ describe('elements/content-explorer/MetadataSidePanel', () => { getOperations: jest.fn(), metadataTemplate: mockMetadataTemplate, onClose: mockOnClose, + onUpdate: jest.fn(), refreshCollection: jest.fn(), - updateMetadataV2: jest.fn(), selectedItemIds: new Set(['1']), }; @@ -146,9 +146,9 @@ describe('elements/content-explorer/MetadataSidePanel', () => { expect(screen.getByLabelText('Edit Mock Template')).toBeInTheDocument(); }); - test('calls updateMetadataV2 when form is submitted for single item', async () => { + test('calls onUpdate when form is submitted for single item', async () => { const mockUpdateMetadata = jest.fn().mockResolvedValue(undefined); - renderComponent({ updateMetadataV2: mockUpdateMetadata }); + renderComponent({ onUpdate: mockUpdateMetadata }); const editTemplateButton = screen.getByLabelText('Edit Mock Template'); await userEvent.click(editTemplateButton); @@ -164,13 +164,13 @@ describe('elements/content-explorer/MetadataSidePanel', () => { ); }); - test('calls getOperations and updateMetadataV2 for each item when multiple items are selected', async () => { + test('calls getOperations and onUpdate for each item when multiple items are selected', async () => { const mockUpdateMetadata = jest.fn().mockResolvedValue(undefined); const mockGetOperations = jest.fn().mockReturnValue([]); renderComponent({ selectedItemIds: new Set(['1', '2']), - updateMetadataV2: mockUpdateMetadata, + onUpdate: mockUpdateMetadata, getOperations: mockGetOperations, }); @@ -194,7 +194,7 @@ describe('elements/content-explorer/MetadataSidePanel', () => { const mockRefreshCollection = jest.fn(); renderComponent({ - updateMetadataV2: mockUpdateMetadata, + onUpdate: mockUpdateMetadata, refreshCollection: mockRefreshCollection, }); @@ -217,7 +217,7 @@ describe('elements/content-explorer/MetadataSidePanel', () => { return Promise.resolve(); }); - renderComponent({ updateMetadataV2: mockUpdateMetadata }); + renderComponent({ onUpdate: mockUpdateMetadata }); const editTemplateButton = screen.getByLabelText('Edit Mock Template'); await userEvent.click(editTemplateButton); @@ -236,7 +236,7 @@ describe('elements/content-explorer/MetadataSidePanel', () => { renderComponent({ selectedItemIds: new Set(['1', '2']), - updateMetadataV2: mockUpdateMetadata, + onUpdate: mockUpdateMetadata, getOperations: mockGetOperations, }); diff --git a/src/elements/content-explorer/utils.ts b/src/elements/content-explorer/utils.ts index bffb625cf3..62e54bee47 100644 --- a/src/elements/content-explorer/utils.ts +++ b/src/elements/content-explorer/utils.ts @@ -16,7 +16,7 @@ import messages from '../common/messages'; // Specific type for metadata field value in the item // Note: Item doesn't have field value in metadata object if that field is not set, so the value will be undefined in this case -type ItemMetadataFieldValue = string | number | Array | undefined; +type ItemMetadataFieldValue = string | number | Array | null | undefined; // Get selected item text export function useSelectedItemText(currentCollection: Collection, selectedItemIds: Selection): string { @@ -42,8 +42,26 @@ export function useSelectedItemText(currentCollection: Collection, selectedItemI // Check if the field value is empty. // Note: 0 doesn't represent empty here because of float type field export function isEmptyValue(value: ItemMetadataFieldValue) { - if (isNil(value)) return true; - return value === '' || (Array.isArray(value) && value.length === 0) || Number.isNaN(value); + if (isNil(value)) { + return true; + } + + // date, string, enum + if (value === '') { + return true; + } + + // multiSelect + if (Array.isArray(value) && value.length === 0) { + return true; + } + + // float + if (Number.isNaN(value)) { + return true; + } + + return false; } // Check if the field values are equal based on the field types @@ -62,11 +80,22 @@ export function areFieldValuesEqual( return value1 === value2; } +// Return default form value by field type +function getDefaultValueByFieldType(fieldType: MetadataFieldType) { + if (fieldType === 'date' || fieldType === 'enum' || fieldType === 'float' || fieldType === 'string') { + return ''; + } + if (fieldType === 'multiSelect') { + return []; + } + return undefined; +} + // Set the field value in Metadata Form based on the field type function setFieldValue(fieldType: MetadataFieldType, fieldValue: ItemMetadataFieldValue) { - if (fieldValue) return fieldValue; - if (fieldType === 'multiSelect') return []; - if (fieldType === 'enum') return ''; + if (isNil(fieldValue)) { + return getDefaultValueByFieldType(fieldType); + } return fieldValue; } From 73719dcbe51c541afc6e7bca634369cc4a9c5b2a Mon Sep 17 00:00:00 2001 From: Jerry Jiang Date: Wed, 27 Aug 2025 16:38:36 -0500 Subject: [PATCH 3/5] feat(metadata-view): add bulk metadata edit api --- src/api/Metadata.js | 69 +++++++++-- src/api/__tests__/Metadata.test.js | 114 +++++++++++++++++- .../content-explorer/ContentExplorer.tsx | 47 ++++---- .../MetadataQueryAPIHelper.ts | 17 +++ .../content-explorer/MetadataSidePanel.tsx | 44 +++---- .../__tests__/MetadataSidePanel.test.tsx | 37 ++---- 6 files changed, 232 insertions(+), 96 deletions(-) diff --git a/src/api/Metadata.js b/src/api/Metadata.js index a93aab410e..2fbc2f2f76 100644 --- a/src/api/Metadata.js +++ b/src/api/Metadata.js @@ -830,6 +830,7 @@ class Metadata extends File { * @param {BoxItem} item - File/Folder object for which we are changing the description * @param {Object} template - Metadata template * @param {Array} operations - Array of JSON patch operations + * @param {boolean} suppressCallbacks - Boolean to decide whether suppress callbacks or not * @param {Function} successCallback - Success callback * @param {Function} errorCallback - Error callback * @return {Promise} @@ -838,14 +839,19 @@ class Metadata extends File { item: BoxItem, template: MetadataTemplate, operations: JSONPatchOperations, - successCallback: Function, - errorCallback: ElementsErrorCallback, + suppressCallbacks: boolean = false, + successCallback?: Function, + errorCallback?: ElementsErrorCallback, ): Promise { this.errorCode = ERROR_CODE_UPDATE_METADATA; - this.successCallback = successCallback; - this.errorCallback = errorCallback; + if (!suppressCallbacks) { + // Only set callbacks when we intend to invoke them for this call + // so that callers performing bulk operations can suppress per-item callbacks + this.successCallback = successCallback; + this.errorCallback = errorCallback; + } - const { id, permissions } = item; + const { id, permissions, type } = item; if (!id || !permissions) { this.errorHandler(getBadItemError()); return; @@ -859,7 +865,6 @@ class Metadata extends File { } try { - const { type } = item; const metadata = await this.xhr.put({ url: type === 'file' @@ -883,13 +888,63 @@ class Metadata extends File { editor, ); } - this.successHandler(editor); + if (!suppressCallbacks) { + this.successHandler(editor); + } } } catch (e) { + if (suppressCallbacks) { + // Let the caller decide how to handle errors (e.g., aggregate for bulk operations) + throw e; + } this.errorHandler(e); } } + /** + * API for bulk patching metadata on items (file/folder) + * + * @param {BoxItem[]} items - File/Folder object for which we are changing the description + * @param {Object} template - Metadata template + * @param {Array} operations - Array of JSON patch operations for each item + * @param {Function} successCallback - Success callback + * @param {Function} errorCallback - Error callback + * @return {Promise} + */ + async bulkUpdateMetadata( + items: BoxItem[], + template: MetadataTemplate, + operations: JSONPatchOperations[], + successCallback: Function, + errorCallback: ElementsErrorCallback, + ): Promise { + this.errorCode = ERROR_CODE_UPDATE_METADATA; + this.successCallback = successCallback; + this.errorCallback = errorCallback; + + try { + const updatePromises = items.map(async (item, index) => { + try { + // Suppress per-item callbacks; aggregate outcome at the bulk level only + await this.updateMetadata(item, template, operations[index], true, null, null); + } catch (e) { + // Re-throw to be caught by Promise.all and handled once below + throw new Error(`Failed to update metadata for item "${item.name}": ${e.message || e}`); + } + }); + + await Promise.all(updatePromises); + + if (!this.isDestroyed()) { + this.successHandler(); + } + } catch (e) { + if (!this.isDestroyed()) { + this.errorHandler(e); + } + } + } + /** * API for patching metadata on file * diff --git a/src/api/__tests__/Metadata.test.js b/src/api/__tests__/Metadata.test.js index 9b8609459b..47e5f4a19b 100644 --- a/src/api/__tests__/Metadata.test.js +++ b/src/api/__tests__/Metadata.test.js @@ -1654,7 +1654,7 @@ describe('api/Metadata', () => { jest.spyOn(ErrorUtil, 'getBadItemError').mockReturnValueOnce('error'); const successCallback = jest.fn(); const errorCallback = jest.fn(); - metadata.updateMetadata({}, {}, {}, successCallback, errorCallback); + metadata.updateMetadata({}, {}, {}, false, successCallback, errorCallback); expect(errorCallback).toBeCalledWith('error', ERROR_CODE_UPDATE_METADATA); expect(successCallback).not.toBeCalled(); expect(ErrorUtil.getBadItemError).toBeCalled(); @@ -1663,7 +1663,7 @@ describe('api/Metadata', () => { jest.spyOn(ErrorUtil, 'getBadItemError').mockReturnValueOnce('error'); const successCallback = jest.fn(); const errorCallback = jest.fn(); - metadata.updateMetadata({ id: 'id' }, {}, {}, successCallback, errorCallback); + metadata.updateMetadata({ id: 'id' }, {}, {}, false, successCallback, errorCallback); expect(errorCallback).toBeCalledWith('error', ERROR_CODE_UPDATE_METADATA); expect(successCallback).not.toBeCalled(); expect(ErrorUtil.getBadItemError).toBeCalled(); @@ -1672,7 +1672,7 @@ describe('api/Metadata', () => { ErrorUtil.getBadPermissionsError = jest.fn().mockReturnValueOnce('error'); const successCallback = jest.fn(); const errorCallback = jest.fn(); - metadata.updateMetadata({ id: 'id', permissions: {} }, {}, {}, successCallback, errorCallback); + metadata.updateMetadata({ id: 'id', permissions: {} }, {}, {}, false, successCallback, errorCallback); expect(errorCallback).toBeCalledWith('error', ERROR_CODE_UPDATE_METADATA); expect(successCallback).not.toBeCalled(); expect(ErrorUtil.getBadPermissionsError).toBeCalled(); @@ -1685,6 +1685,7 @@ describe('api/Metadata', () => { { id: 'id', permissions: { can_upload: false } }, {}, {}, + false, successCallback, errorCallback, ); @@ -1739,7 +1740,7 @@ describe('api/Metadata', () => { metadata.successHandler = jest.fn(); metadata.errorHandler = jest.fn(); - await metadata.updateMetadata(file, template, ops, success, error); + await metadata.updateMetadata(file, template, ops, false, success, error); expect(metadata.successCallback).toBe(success); expect(metadata.errorCallback).toBe(error); @@ -1806,7 +1807,7 @@ describe('api/Metadata', () => { metadata.successHandler = jest.fn(); metadata.errorHandler = jest.fn(); - await metadata.updateMetadata(file, template, ops, success, error); + await metadata.updateMetadata(file, template, ops, false, success, error); expect(metadata.successCallback).toBe(success); expect(metadata.errorCallback).toBe(error); @@ -1874,7 +1875,7 @@ describe('api/Metadata', () => { metadata.successHandler = jest.fn(); metadata.errorHandler = jest.fn(); - await metadata.updateMetadata(file, template, ops, success, error); + await metadata.updateMetadata(file, template, ops, false, success, error); expect(metadata.successCallback).toBe(success); expect(metadata.errorCallback).toBe(error); @@ -1897,6 +1898,107 @@ describe('api/Metadata', () => { }); }); + describe('bulkUpdateMetadata()', () => { + test('should call updateMetadata for each item and call successHandler when all succeed', async () => { + const success = jest.fn(); + const error = jest.fn(); + const items = [ + { id: '1', name: 'file1', permissions: { can_upload: true }, type: 'file' }, + { id: '2', name: 'file2', permissions: { can_upload: true }, type: 'file' }, + ]; + const template = { scope: 'scope', templateKey: 'templateKey' }; + const ops = [[{ op: 'replace', path: '/foo', value: 'a' }], [{ op: 'replace', path: '/foo', value: 'b' }]]; + + metadata.updateMetadata = jest.fn().mockResolvedValue(undefined); + metadata.isDestroyed = jest.fn().mockReturnValue(false); + metadata.successHandler = jest.fn(); + metadata.errorHandler = jest.fn(); + + await metadata.bulkUpdateMetadata(items, template, ops, success, error); + + expect(metadata.errorCode).toBe(ERROR_CODE_UPDATE_METADATA); + expect(metadata.successCallback).toBe(success); + expect(metadata.errorCallback).toBe(error); + expect(metadata.updateMetadata).toHaveBeenCalledTimes(2); + expect(metadata.updateMetadata).toHaveBeenNthCalledWith(1, items[0], template, ops[0], true, null, null); + expect(metadata.updateMetadata).toHaveBeenNthCalledWith(2, items[1], template, ops[1], true, null, null); + expect(metadata.isDestroyed).toHaveBeenCalledTimes(1); + expect(metadata.successHandler).toHaveBeenCalledTimes(1); + expect(metadata.errorHandler).not.toHaveBeenCalled(); + }); + + test('should call errorHandler with aggregated error when any update fails', async () => { + const success = jest.fn(); + const error = jest.fn(); + const items = [ + { id: '1', name: 'file1', permissions: { can_upload: true }, type: 'file' }, + { id: '2', name: 'file2', permissions: { can_upload: true }, type: 'file' }, + ]; + const template = { scope: 'scope', templateKey: 'templateKey' }; + const ops = [[], []]; + + metadata.updateMetadata = jest + .fn() + .mockResolvedValueOnce(undefined) + .mockRejectedValueOnce(new Error('mock error')); + metadata.isDestroyed = jest.fn().mockReturnValue(false); + metadata.successHandler = jest.fn(); + metadata.errorHandler = jest.fn(); + + await metadata.bulkUpdateMetadata(items, template, ops, success, error); + + expect(metadata.updateMetadata).toHaveBeenCalledTimes(2); + expect(metadata.errorHandler).toHaveBeenCalledTimes(1); + const errArg = metadata.errorHandler.mock.calls[0][0]; + expect(errArg).toBeInstanceOf(Error); + expect(errArg.message).toContain('Failed to update metadata for item "file2"'); + expect(errArg.message).toContain('mock error'); + expect(metadata.successHandler).not.toHaveBeenCalled(); + }); + + test('should not call successHandler when destroyed after successful updates', async () => { + const success = jest.fn(); + const error = jest.fn(); + const items = [ + { id: '1', name: 'file1', permissions: { can_upload: true }, type: 'file' }, + { id: '2', name: 'file2', permissions: { can_upload: true }, type: 'file' }, + ]; + const template = { scope: 'scope', templateKey: 'templateKey' }; + const ops = [[], []]; + + metadata.updateMetadata = jest.fn().mockResolvedValue(undefined); + metadata.isDestroyed = jest.fn().mockReturnValue(true); + metadata.successHandler = jest.fn(); + metadata.errorHandler = jest.fn(); + + await metadata.bulkUpdateMetadata(items, template, ops, success, error); + + expect(metadata.successHandler).not.toHaveBeenCalled(); + expect(metadata.errorHandler).not.toHaveBeenCalled(); + }); + + test('should not call errorHandler when destroyed after failure', async () => { + const success = jest.fn(); + const error = jest.fn(); + const items = [ + { id: '1', name: 'file1', permissions: { can_upload: true }, type: 'file' }, + { id: '2', name: 'file2', permissions: { can_upload: true }, type: 'file' }, + ]; + const template = { scope: 'scope', templateKey: 'templateKey' }; + const ops = [[], []]; + + metadata.updateMetadata = jest.fn().mockRejectedValue(new Error('mock error')); + metadata.isDestroyed = jest.fn().mockReturnValue(true); + metadata.successHandler = jest.fn(); + metadata.errorHandler = jest.fn(); + + await metadata.bulkUpdateMetadata(items, template, ops, success, error); + + expect(metadata.successHandler).not.toHaveBeenCalled(); + expect(metadata.errorHandler).not.toHaveBeenCalled(); + }); + }); + describe('updateMetadataRedesign()', () => { test('should call error callback with a bad item error when no id', () => { jest.spyOn(ErrorUtil, 'getBadItemError').mockReturnValueOnce('error'); diff --git a/src/elements/content-explorer/ContentExplorer.tsx b/src/elements/content-explorer/ContentExplorer.tsx index 26549ecebd..b0f286e6a3 100644 --- a/src/elements/content-explorer/ContentExplorer.tsx +++ b/src/elements/content-explorer/ContentExplorer.tsx @@ -495,37 +495,35 @@ class ContentExplorer extends Component { } /** - * Get operations for all fields update in the metadata sidepanel - * - * @private - * @return {JSONPatchOperations} - */ - getOperations = ( - item: BoxItem, - templateOldFields: MetadataTemplateField[], - templateNewFields: MetadataTemplateField[], - ): JSONPatchOperations => { - return this.metadataQueryAPIHelper.generateOperations(item, templateOldFields, templateNewFields); - }; - - /** - * Update item's metadata instance with either operations or old and new value for one specific field + * Update selected items' metadata instances based on original and new field values in the metadata instance form * * @private * @return {void} */ updateMetadataV2 = async ( - item: BoxItem, + items: BoxItem[], operations: JSONPatchOperations, - successCallback?: () => void, - errorCallback?: ErrorCallback, + templateOldFields: MetadataTemplateField[], + templateNewFields: MetadataTemplateField[], + successCallback: () => void, + errorCallback: ErrorCallback, ) => { - await this.metadataQueryAPIHelper.updateMetadataWithOperations( - item, - operations, - successCallback, - errorCallback, - ); + if (items.length === 1) { + await this.metadataQueryAPIHelper.updateMetadataWithOperations( + items[0], + operations, + successCallback, + errorCallback, + ); + } else { + await this.metadataQueryAPIHelper.bulkUpdateMetadata( + items, + templateOldFields, + templateNewFields, + successCallback, + errorCallback, + ); + } }; /** @@ -1907,7 +1905,6 @@ class ContentExplorer extends Component { {isDefaultViewMetadata && isMetadataViewV2Feature && isMetadataSidePanelOpen && ( void, + errorCallback: ErrorCallback, + ): Promise => { + const operations: JSONPatchOperations = []; + items.forEach(item => { + const operation = this.generateOperations(item, templateOldFields, templateNewFields); + operations.push(operation); + }); + return this.api + .getMetadataAPI(true) + .bulkUpdateMetadata(items, this.metadataTemplate, operations, successCallback, errorCallback); + }; + /** * Verify that the metadata query has required fields and update it if necessary * For a file item, default fields included in the response are "type", "id", "etag" diff --git a/src/elements/content-explorer/MetadataSidePanel.tsx b/src/elements/content-explorer/MetadataSidePanel.tsx index 0c3ae89eb5..cb8d152f57 100644 --- a/src/elements/content-explorer/MetadataSidePanel.tsx +++ b/src/elements/content-explorer/MetadataSidePanel.tsx @@ -24,18 +24,15 @@ import './MetadataSidePanel.scss'; export interface MetadataSidePanelProps { currentCollection: Collection; - getOperations: ( - item: BoxItem, - templateOldFields: MetadataTemplateField[], - templateNewFields: MetadataTemplateField[], - ) => JSONPatchOperations; metadataTemplate: MetadataTemplate; onClose: () => void; onUpdate: ( - file: BoxItem, + items: BoxItem[], operations: JSONPatchOperations, - successCallback?: () => void, - errorCallback?: ErrorCallback, + templateOldFields: MetadataTemplateField[], + templateNewFields: MetadataTemplateField[], + successCallback: () => void, + errorCallback: ErrorCallback, ) => Promise; refreshCollection: () => void; selectedItemIds: Selection; @@ -43,7 +40,6 @@ export interface MetadataSidePanelProps { const MetadataSidePanel = ({ currentCollection, - getOperations, metadataTemplate, onClose, onUpdate, @@ -100,25 +96,17 @@ const MetadataSidePanel = ({ }; const handleMetadataInstanceFormSubmit = async (values: FormValues, operations: JSONPatchOperations) => { - if (selectedItems.length === 1) { - await onUpdate(selectedItems[0], operations, handleUpdateMetadataSuccess, handleUpdateMetadataError); - } else { - const { fields: templateNewFields } = values.metadata; - const { fields: templateOldFields } = templateInstance; - - try { - // Process items sequentially to avoid conflicts - await selectedItems.reduce(async (previousPromise, item) => { - await previousPromise; - - const ops = getOperations(item, templateOldFields, templateNewFields); - await onUpdate(item, ops); - }, Promise.resolve()); - handleUpdateMetadataSuccess(); - } catch (error) { - handleUpdateMetadataError(); - } - } + const { fields: templateNewFields } = values.metadata; + const { fields: templateOldFields } = templateInstance; + + await onUpdate( + selectedItems, + operations, + templateOldFields, + templateNewFields, + handleUpdateMetadataSuccess, + handleUpdateMetadataError, + ); }; return ( diff --git a/src/elements/content-explorer/__tests__/MetadataSidePanel.test.tsx b/src/elements/content-explorer/__tests__/MetadataSidePanel.test.tsx index a3d630f769..aa5175b310 100644 --- a/src/elements/content-explorer/__tests__/MetadataSidePanel.test.tsx +++ b/src/elements/content-explorer/__tests__/MetadataSidePanel.test.tsx @@ -66,7 +66,6 @@ const mockOnClose = jest.fn(); describe('elements/content-explorer/MetadataSidePanel', () => { const defaultProps: MetadataSidePanelProps = { currentCollection: mockCollection, - getOperations: jest.fn(), metadataTemplate: mockMetadataTemplate, onClose: mockOnClose, onUpdate: jest.fn(), @@ -157,21 +156,21 @@ describe('elements/content-explorer/MetadataSidePanel', () => { await userEvent.click(submitButton); expect(mockUpdateMetadata).toHaveBeenCalledWith( - mockCollection.items[0], + [mockCollection.items[0]], + expect.any(Array), + expect.any(Array), expect.any(Array), expect.any(Function), expect.any(Function), ); }); - test('calls getOperations and onUpdate for each item when multiple items are selected', async () => { + test('calls onUpdate when multiple items are selected', async () => { const mockUpdateMetadata = jest.fn().mockResolvedValue(undefined); - const mockGetOperations = jest.fn().mockReturnValue([]); renderComponent({ selectedItemIds: new Set(['1', '2']), onUpdate: mockUpdateMetadata, - getOperations: mockGetOperations, }); const editTemplateButton = screen.getByLabelText('Edit Mock Template'); @@ -181,13 +180,12 @@ describe('elements/content-explorer/MetadataSidePanel', () => { await userEvent.click(submitButton); await waitFor(() => { - expect(mockGetOperations).toHaveBeenCalledTimes(2); - expect(mockUpdateMetadata).toHaveBeenCalledTimes(2); + expect(mockUpdateMetadata).toHaveBeenCalledTimes(1); }); }); test('displays success notification when metadata update succeeds', async () => { - const mockUpdateMetadata = jest.fn().mockImplementation((_, __, successCallback) => { + const mockUpdateMetadata = jest.fn().mockImplementation((_, __, ___, ____, successCallback) => { successCallback(); return Promise.resolve(); }); @@ -212,7 +210,7 @@ describe('elements/content-explorer/MetadataSidePanel', () => { }); test('displays error notification when metadata update fails', async () => { - const mockUpdateMetadata = jest.fn().mockImplementation((_, __, ___, errorCallback) => { + const mockUpdateMetadata = jest.fn().mockImplementation((_, __, ___, ____, _____, errorCallback) => { errorCallback(); return Promise.resolve(); }); @@ -230,27 +228,6 @@ describe('elements/content-explorer/MetadataSidePanel', () => { }); }); - test('displays error notification when multiple item update fails', async () => { - const mockUpdateMetadata = jest.fn().mockRejectedValue(new Error('Update failed')); - const mockGetOperations = jest.fn().mockReturnValue([]); - - renderComponent({ - selectedItemIds: new Set(['1', '2']), - onUpdate: mockUpdateMetadata, - getOperations: mockGetOperations, - }); - - const editTemplateButton = screen.getByLabelText('Edit Mock Template'); - await userEvent.click(editTemplateButton); - - const submitButton = screen.getByRole('button', { name: 'Save' }); - await userEvent.click(submitButton); - - await waitFor(() => { - expect(screen.getByText('Unable to save changes. Please try again')).toBeInTheDocument(); - }); - }); - test('handles "all" selection correctly', () => { renderComponent({ selectedItemIds: 'all' }); const subtitle = screen.getByText('2 files selected'); From 7546c123634865129483fd48550cb4811dc2bc89 Mon Sep 17 00:00:00 2001 From: Jerry Jiang Date: Wed, 27 Aug 2025 17:51:51 -0500 Subject: [PATCH 4/5] feat(metadata-view): fix flow --- src/api/Metadata.js | 12 +++++----- src/api/__tests__/Metadata.test.js | 35 +++++++++++++++++++++--------- 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/src/api/Metadata.js b/src/api/Metadata.js index 2fbc2f2f76..ddc567aaa4 100644 --- a/src/api/Metadata.js +++ b/src/api/Metadata.js @@ -830,18 +830,18 @@ class Metadata extends File { * @param {BoxItem} item - File/Folder object for which we are changing the description * @param {Object} template - Metadata template * @param {Array} operations - Array of JSON patch operations - * @param {boolean} suppressCallbacks - Boolean to decide whether suppress callbacks or not * @param {Function} successCallback - Success callback * @param {Function} errorCallback - Error callback + * @param {boolean} suppressCallbacks - Boolean to decide whether suppress callbacks or not * @return {Promise} */ async updateMetadata( item: BoxItem, template: MetadataTemplate, operations: JSONPatchOperations, - suppressCallbacks: boolean = false, - successCallback?: Function, - errorCallback?: ElementsErrorCallback, + successCallback: Function, + errorCallback: ElementsErrorCallback, + suppressCallbacks?: boolean, ): Promise { this.errorCode = ERROR_CODE_UPDATE_METADATA; if (!suppressCallbacks) { @@ -926,10 +926,10 @@ class Metadata extends File { const updatePromises = items.map(async (item, index) => { try { // Suppress per-item callbacks; aggregate outcome at the bulk level only - await this.updateMetadata(item, template, operations[index], true, null, null); + await this.updateMetadata(item, template, operations[index], successCallback, errorCallback, true); } catch (e) { // Re-throw to be caught by Promise.all and handled once below - throw new Error(`Failed to update metadata for item "${item.name}": ${e.message || e}`); + throw new Error(`Failed to update metadata: ${e.message || e}`); } }); diff --git a/src/api/__tests__/Metadata.test.js b/src/api/__tests__/Metadata.test.js index 47e5f4a19b..00c17e5c58 100644 --- a/src/api/__tests__/Metadata.test.js +++ b/src/api/__tests__/Metadata.test.js @@ -1654,7 +1654,7 @@ describe('api/Metadata', () => { jest.spyOn(ErrorUtil, 'getBadItemError').mockReturnValueOnce('error'); const successCallback = jest.fn(); const errorCallback = jest.fn(); - metadata.updateMetadata({}, {}, {}, false, successCallback, errorCallback); + metadata.updateMetadata({}, {}, {}, successCallback, errorCallback); expect(errorCallback).toBeCalledWith('error', ERROR_CODE_UPDATE_METADATA); expect(successCallback).not.toBeCalled(); expect(ErrorUtil.getBadItemError).toBeCalled(); @@ -1663,7 +1663,7 @@ describe('api/Metadata', () => { jest.spyOn(ErrorUtil, 'getBadItemError').mockReturnValueOnce('error'); const successCallback = jest.fn(); const errorCallback = jest.fn(); - metadata.updateMetadata({ id: 'id' }, {}, {}, false, successCallback, errorCallback); + metadata.updateMetadata({ id: 'id' }, {}, {}, successCallback, errorCallback); expect(errorCallback).toBeCalledWith('error', ERROR_CODE_UPDATE_METADATA); expect(successCallback).not.toBeCalled(); expect(ErrorUtil.getBadItemError).toBeCalled(); @@ -1672,7 +1672,7 @@ describe('api/Metadata', () => { ErrorUtil.getBadPermissionsError = jest.fn().mockReturnValueOnce('error'); const successCallback = jest.fn(); const errorCallback = jest.fn(); - metadata.updateMetadata({ id: 'id', permissions: {} }, {}, {}, false, successCallback, errorCallback); + metadata.updateMetadata({ id: 'id', permissions: {} }, {}, {}, successCallback, errorCallback); expect(errorCallback).toBeCalledWith('error', ERROR_CODE_UPDATE_METADATA); expect(successCallback).not.toBeCalled(); expect(ErrorUtil.getBadPermissionsError).toBeCalled(); @@ -1685,7 +1685,6 @@ describe('api/Metadata', () => { { id: 'id', permissions: { can_upload: false } }, {}, {}, - false, successCallback, errorCallback, ); @@ -1740,7 +1739,7 @@ describe('api/Metadata', () => { metadata.successHandler = jest.fn(); metadata.errorHandler = jest.fn(); - await metadata.updateMetadata(file, template, ops, false, success, error); + await metadata.updateMetadata(file, template, ops, success, error); expect(metadata.successCallback).toBe(success); expect(metadata.errorCallback).toBe(error); @@ -1807,7 +1806,7 @@ describe('api/Metadata', () => { metadata.successHandler = jest.fn(); metadata.errorHandler = jest.fn(); - await metadata.updateMetadata(file, template, ops, false, success, error); + await metadata.updateMetadata(file, template, ops, success, error); expect(metadata.successCallback).toBe(success); expect(metadata.errorCallback).toBe(error); @@ -1875,7 +1874,7 @@ describe('api/Metadata', () => { metadata.successHandler = jest.fn(); metadata.errorHandler = jest.fn(); - await metadata.updateMetadata(file, template, ops, false, success, error); + await metadata.updateMetadata(file, template, ops, success, error); expect(metadata.successCallback).toBe(success); expect(metadata.errorCallback).toBe(error); @@ -1920,8 +1919,24 @@ describe('api/Metadata', () => { expect(metadata.successCallback).toBe(success); expect(metadata.errorCallback).toBe(error); expect(metadata.updateMetadata).toHaveBeenCalledTimes(2); - expect(metadata.updateMetadata).toHaveBeenNthCalledWith(1, items[0], template, ops[0], true, null, null); - expect(metadata.updateMetadata).toHaveBeenNthCalledWith(2, items[1], template, ops[1], true, null, null); + expect(metadata.updateMetadata).toHaveBeenNthCalledWith( + 1, + items[0], + template, + ops[0], + success, + error, + true, + ); + expect(metadata.updateMetadata).toHaveBeenNthCalledWith( + 2, + items[1], + template, + ops[1], + success, + error, + true, + ); expect(metadata.isDestroyed).toHaveBeenCalledTimes(1); expect(metadata.successHandler).toHaveBeenCalledTimes(1); expect(metadata.errorHandler).not.toHaveBeenCalled(); @@ -1951,7 +1966,7 @@ describe('api/Metadata', () => { expect(metadata.errorHandler).toHaveBeenCalledTimes(1); const errArg = metadata.errorHandler.mock.calls[0][0]; expect(errArg).toBeInstanceOf(Error); - expect(errArg.message).toContain('Failed to update metadata for item "file2"'); + expect(errArg.message).toContain('Failed to update metadata'); expect(errArg.message).toContain('mock error'); expect(metadata.successHandler).not.toHaveBeenCalled(); }); From a86ca947265ae623311894a49ec3a4a3656469d4 Mon Sep 17 00:00:00 2001 From: Jerry Jiang Date: Thu, 28 Aug 2025 15:18:09 -0500 Subject: [PATCH 5/5] feat(metadata-view): resolve comments --- i18n/en-US.properties | 2 +- src/elements/common/messages.js | 2 +- .../MetadataQueryAPIHelper.ts | 9 +- .../__tests__/MetadataSidePanel.test.tsx | 22 +--- src/elements/content-explorer/utils.ts | 113 ++++++++++-------- 5 files changed, 72 insertions(+), 76 deletions(-) diff --git a/i18n/en-US.properties b/i18n/en-US.properties index b2711211e1..0b799107a2 100644 --- a/i18n/en-US.properties +++ b/i18n/en-US.properties @@ -583,7 +583,7 @@ be.messageCenter.product = Product # Title for the message center modal be.messageCenter.title = What's New # Text shown in error notification banner -be.metadataUpdateErrorNotification = Unable to save changes. Please try again +be.metadataUpdateErrorNotification = Unable to save changes. Please try again. # Text shown in success notification banner be.metadataUpdateSuccessNotification = {numSelected, plural, =1 {1 document updated} other {# documents updated} } # Text for modified date with modified prefix. diff --git a/src/elements/common/messages.js b/src/elements/common/messages.js index 294eecd0c1..d9773595e3 100644 --- a/src/elements/common/messages.js +++ b/src/elements/common/messages.js @@ -1118,7 +1118,7 @@ const messages = defineMessages({ metadataUpdateErrorNotification: { id: 'be.metadataUpdateErrorNotification', description: 'Text shown in error notification banner', - defaultMessage: 'Unable to save changes. Please try again', + defaultMessage: 'Unable to save changes. Please try again.', }, metadataUpdateSuccessNotification: { id: 'be.metadataUpdateSuccessNotification', diff --git a/src/elements/content-explorer/MetadataQueryAPIHelper.ts b/src/elements/content-explorer/MetadataQueryAPIHelper.ts index 5c7f5ecdd3..6f4fbbb3af 100644 --- a/src/elements/content-explorer/MetadataQueryAPIHelper.ts +++ b/src/elements/content-explorer/MetadataQueryAPIHelper.ts @@ -3,11 +3,10 @@ import find from 'lodash/find'; import getProp from 'lodash/get'; import includes from 'lodash/includes'; import isArray from 'lodash/isArray'; -import xor from 'lodash/xor'; import type { MetadataTemplateField } from '@box/metadata-editor'; import type { MetadataFieldType } from '@box/metadata-view'; import API from '../../api'; -import { isEmptyValue, isMultiValuesField } from './utils'; +import { areFieldValuesEqual, isEmptyValue, isMultiValuesField } from './utils'; import { JSON_PATCH_OP_ADD, @@ -61,11 +60,7 @@ export default class MetadataQueryAPIHelper { newValue: MetadataFieldValue | null, ): JSONPatchOperations => { // check if two values are the same, return empty operations if so - if ( - (isEmptyValue(oldValue) && isEmptyValue(newValue)) || - (Array.isArray(oldValue) && Array.isArray(newValue) && xor(oldValue, newValue).length === 0) || - oldValue === newValue - ) { + if (areFieldValuesEqual(oldValue, newValue)) { return []; } diff --git a/src/elements/content-explorer/__tests__/MetadataSidePanel.test.tsx b/src/elements/content-explorer/__tests__/MetadataSidePanel.test.tsx index aa5175b310..e02b4e0a0a 100644 --- a/src/elements/content-explorer/__tests__/MetadataSidePanel.test.tsx +++ b/src/elements/content-explorer/__tests__/MetadataSidePanel.test.tsx @@ -202,11 +202,9 @@ describe('elements/content-explorer/MetadataSidePanel', () => { const submitButton = screen.getByRole('button', { name: 'Save' }); await userEvent.click(submitButton); - await waitFor(() => { - expect(screen.getByText('1 document updated')).toBeInTheDocument(); - expect(mockRefreshCollection).toHaveBeenCalledTimes(1); - expect(screen.getByLabelText('Edit Mock Template')).toBeInTheDocument(); // Back to view mode - }); + expect(screen.getByText('1 document updated')).toBeInTheDocument(); + expect(mockRefreshCollection).toHaveBeenCalledTimes(1); + expect(screen.getByLabelText('Edit Mock Template')).toBeInTheDocument(); // Back to view mode }); test('displays error notification when metadata update fails', async () => { @@ -224,7 +222,7 @@ describe('elements/content-explorer/MetadataSidePanel', () => { await userEvent.click(submitButton); await waitFor(() => { - expect(screen.getByText('Unable to save changes. Please try again')).toBeInTheDocument(); + expect(screen.getByText('Unable to save changes. Please try again.')).toBeInTheDocument(); }); }); @@ -268,16 +266,4 @@ describe('elements/content-explorer/MetadataSidePanel', () => { expect(screen.getByText('Multiple Values')).toBeInTheDocument(); }); - - test('renders correct accessibility attributes', () => { - renderComponent(); - - // Close button should have proper aria-label - const closeButton = screen.getByLabelText('Close'); - expect(closeButton).toHaveAttribute('aria-label', 'Close'); - - // Edit button should have proper aria-label - const editButton = screen.getByLabelText('Edit Mock Template'); - expect(editButton).toHaveAttribute('aria-label', 'Edit Mock Template'); - }); }); diff --git a/src/elements/content-explorer/utils.ts b/src/elements/content-explorer/utils.ts index 62e54bee47..07991feb4b 100644 --- a/src/elements/content-explorer/utils.ts +++ b/src/elements/content-explorer/utils.ts @@ -1,6 +1,7 @@ import React, { useMemo } from 'react'; import { useIntl } from 'react-intl'; import isNil from 'lodash/isNil'; +import xor from 'lodash/xor'; import { MULTI_VALUE_DEFAULT_OPTION, @@ -65,16 +66,14 @@ export function isEmptyValue(value: ItemMetadataFieldValue) { } // Check if the field values are equal based on the field types -export function areFieldValuesEqual( - fieldType: MetadataFieldType, - value1: ItemMetadataFieldValue, - value2: ItemMetadataFieldValue, -) { - if (isEmptyValue(value1) && isEmptyValue(value2)) return true; +export function areFieldValuesEqual(value1: ItemMetadataFieldValue, value2: ItemMetadataFieldValue) { + if (isEmptyValue(value1) && isEmptyValue(value2)) { + return true; + } // Handle multiSelect arrays comparison - if (fieldType === 'multiSelect' && Array.isArray(value1) && Array.isArray(value2)) { - return value1.length === value2.length && value1.every(val => value2.includes(val)); + if (Array.isArray(value1) && Array.isArray(value2)) { + return xor(value1, value2).length === 0; } return value1 === value2; @@ -92,7 +91,7 @@ function getDefaultValueByFieldType(fieldType: MetadataFieldType) { } // Set the field value in Metadata Form based on the field type -function setFieldValue(fieldType: MetadataFieldType, fieldValue: ItemMetadataFieldValue) { +function getFieldValue(fieldType: MetadataFieldType, fieldValue: ItemMetadataFieldValue) { if (isNil(fieldValue)) { return getDefaultValueByFieldType(fieldType); } @@ -117,52 +116,68 @@ export function useTemplateInstance(metadataTemplate: MetadataTemplate, selected const selectedItemsFields = fields.map( ({ displayName: fieldDisplayName, hidden: fieldHidden, id: fieldId, key, options, type: fieldType }) => { - const firstSelectedItem = selectedItems[0]; - let fieldValue = firstSelectedItem.metadata[scope][templateKey][key]; - - if (selectedItems.length > 1) { - const allItemsHaveSameInitialValue = selectedItems.every(selectedItem => - areFieldValuesEqual( - fieldType as MetadataFieldType, - selectedItem.metadata[scope][templateKey][key], - fieldValue, - ), - ); - - // Update field display value when selected items have different values - if (!allItemsHaveSameInitialValue) { - if (isEditing) { - // Add MultiValue Option if the field is multiSelect or enum - if (fieldType === 'multiSelect' || fieldType === 'enum') { - fieldValue = fieldType === 'enum' ? MULTI_VALUE_DEFAULT_VALUE : [MULTI_VALUE_DEFAULT_VALUE]; - const multiValueOption = options?.find(option => option.key === MULTI_VALUE_DEFAULT_VALUE); - if (!multiValueOption) { - options?.push(MULTI_VALUE_DEFAULT_OPTION); - } - } else { - fieldValue = setFieldValue(fieldType as MetadataFieldType, undefined); - } - } else { - /** - * We want to show "Multiple values" label for multiple dates across files selection. - * We use fragment here to bypass check in shared feature. - * This feature tries to parse string as date if the string is passed as value. - */ - const multipleValuesText = formatMessage(messages.multipleValues); - fieldValue = React.createElement(React.Fragment, null, multipleValuesText); - } - } else { - fieldValue = setFieldValue(fieldType as MetadataFieldType, fieldValue); - } - } - return { + const defaultItemField = { displayName: fieldDisplayName, hidden: fieldHidden, id: fieldId, key, options, type: fieldType, - value: fieldValue, + value: getFieldValue(fieldType as MetadataFieldType, undefined), + }; + + const firstSelectedItem = selectedItems[0]; + const firstSelectedItemFieldValue = firstSelectedItem.metadata[scope][templateKey][key]; + + // Case 1: Single selected item + if (selectedItems.length <= 1) { + return { + ...defaultItemField, + value: firstSelectedItemFieldValue, + }; + } + + // Case 2.1: Multiple selected items, but all have the same initial value + const allItemsHaveSameInitialValue = selectedItems.every(selectedItem => + areFieldValuesEqual(selectedItem.metadata[scope][templateKey][key], firstSelectedItemFieldValue), + ); + + if (allItemsHaveSameInitialValue) { + return { + ...defaultItemField, + value: getFieldValue(fieldType as MetadataFieldType, firstSelectedItemFieldValue), + }; + } + + // Case 2.2: Multiple selected items, but some have different initial values + // Case 2.2.1: Edit Mode + if (isEditing) { + let fieldValue = getFieldValue(fieldType as MetadataFieldType, undefined); + // Add MultiValue Option if the field is multiSelect or enum + if (fieldType === 'multiSelect' || fieldType === 'enum') { + fieldValue = fieldType === 'enum' ? MULTI_VALUE_DEFAULT_VALUE : [MULTI_VALUE_DEFAULT_VALUE]; + const multiValueOption = options?.find(option => option.key === MULTI_VALUE_DEFAULT_VALUE); + if (!multiValueOption) { + options?.push(MULTI_VALUE_DEFAULT_OPTION); + } + } + return { + ...defaultItemField, + value: fieldValue, + }; + } + + /** + * Case: 2.2.2 View Mode + * + * We want to show "Multiple values" label for multiple dates across files selection. + * We use fragment here to bypass check in shared feature. + * This feature tries to parse string as date if the string is passed as value. + */ + const multipleValuesText = formatMessage(messages.multipleValues); + return { + ...defaultItemField, + value: React.createElement(React.Fragment, null, multipleValuesText), }; }, );