feat(metadata-view): Add MetadataView V2 #4191
Conversation
WalkthroughThis change introduces a new metadata view feature in the Content Explorer. It adds and updates several Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ContentExplorer
participant MetadataQueryAPIHelper
participant MetadataViewContainer
participant BoxAPI
User->>ContentExplorer: Selects Metadata View
ContentExplorer->>MetadataQueryAPIHelper: fetchMetadataQueryResults()
MetadataQueryAPIHelper->>BoxAPI: Query metadata
BoxAPI-->>MetadataQueryAPIHelper: Metadata query response
MetadataQueryAPIHelper->>BoxAPI: Fetch template schema
BoxAPI-->>MetadataQueryAPIHelper: Template schema response
MetadataQueryAPIHelper-->>ContentExplorer: Collection & metadataTemplate
ContentExplorer->>MetadataViewContainer: Render with collection & metadataTemplate
MetadataViewContainer->>User: Display metadata table
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (15)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (13)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
greg-in-a-box
force-pushed
the
metadata-view
branch
4 times, most recently
from
ab8ae04 to
e92ba45
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
scripts/jest/jest.config.js (1)
29-29: LGTM! Jest configuration updated for new metadata packages.The addition of
@box/metadata-filter,@box/metadata-view, and@box/typesto thetransformIgnorePatternsis necessary to ensure Jest properly processes these packages during testing.Consider refactoring this long regex pattern into a more maintainable format, such as:
- 'node_modules/(?!(@box/react-virtualized/dist/es|@box/cldr-data|@box/blueprint-web|@box/blueprint-web-assets|@box/metadata-editor|@box/box-ai-content-answers|@box/box-ai-agent-selector|@box/item-icon|@box/combobox-with-api|@box/tree|@box/metadata-filter|@box/metadata-view|@box/types)/)', + `node_modules/(?!(${[ + '@box/react-virtualized/dist/es', + '@box/cldr-data', + '@box/blueprint-web', + '@box/blueprint-web-assets', + '@box/metadata-editor', + '@box/box-ai-content-answers', + '@box/box-ai-agent-selector', + '@box/item-icon', + '@box/combobox-with-api', + '@box/tree', + '@box/metadata-filter', + '@box/metadata-view', + '@box/types' + ].join('|')})/)`src/elements/content-explorer/__tests__/MetadataViewContainer.test.tsx (3)
83-83: Remove debug statement from production test.The
screen.debug(null, 10000)call should be removed as it's only useful during development and can clutter test output.- screen.debug(null, 10000);
15-32: Fix inconsistency in mock template field data.There's an inconsistency in the mock data: the first field has
key: ' name'with a leading space, which seems unintentional.{ id: 'field1', - key: ' name', + key: 'name', displayName: 'Name', type: 'string', },
81-89: Consider expanding test coverage.While the current test verifies basic rendering, consider adding tests for:
- Sort functionality via onSortChange callback
- Different column configurations
- Empty state handling
- Error states
src/elements/content-explorer/stories/tests/MetadataView-visual.stories.tsx (1)
38-45: Consider more flexible column widths.The fixed 150px width for all columns might not accommodate varying content lengths effectively. Consider using minWidth with flexible maxWidth or auto-sizing.
const columns = mockSchema.fields.map(field => ({ textValue: field.displayName, id: `${metadataSourceFieldName}.${field.key}`, type: field.type, allowSorting: true, - minWidth: 150, - maxWidth: 150, + minWidth: 150, + maxWidth: 300, }));src/elements/content-explorer/stories/MetadataView.stories.tsx (3)
17-22: Consider improving the documentation for the commented filter examples.The commented code provides valuable examples for filtering, but could benefit from more descriptive comments explaining when and how to use these filters.
- // // Filter items in the folder by existing metadata key - // query: 'key = :arg1', - // - // // Display items with value - // query_params: { arg1: 'value' }, + // Example: Filter items by metadata field value + // Uncomment below to show only items where a specific metadata field equals a value + // query: 'industry = :industry_value', + // query_params: { industry_value: 'Technology' },
39-50: Add defensive check for name field existence.The code assumes a field with
key === 'name'exists in the schema. Consider adding a defensive check or documenting this requirement.const columns = mockSchema.fields.map(field => { if (field.key === 'name') { return { textValue: field.displayName, id: 'name', type: 'string', allowsSorting: true, minWidth: 250, maxWidth: 250, isRowHeader: true, }; } +}).filter(Boolean); // Filter out any undefined values if name field is missing
60-63: Consider locale-aware date formatting.The date formatting uses
toLocaleDateString()without specifying a locale, which could lead to inconsistent formatting across different user environments.cellRenderer: (item, column) => { const dateValue = get(item, column.id); - return dateValue ? new Date(dateValue).toLocaleDateString() : ''; + return dateValue ? new Date(dateValue).toLocaleDateString('en-US', { + year: 'numeric', + month: 'short', + day: 'numeric' + }) : ''; },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (16)
package.json(5 hunks)scripts/i18n.config.js(1 hunks)scripts/jest/jest.config.js(1 hunks)src/common/types/core.js(2 hunks)src/elements/common/__mocks__/mockMetadata.ts(3 hunks)src/elements/content-explorer/Content.tsx(4 hunks)src/elements/content-explorer/ContentExplorer.tsx(10 hunks)src/elements/content-explorer/MetadataView.tsx(0 hunks)src/elements/content-explorer/MetadataViewContainer.tsx(1 hunks)src/elements/content-explorer/__tests__/Content.test.tsx(3 hunks)src/elements/content-explorer/__tests__/ContentExplorer.test.tsx(2 hunks)src/elements/content-explorer/__tests__/MetadataViewContainer.test.tsx(1 hunks)src/elements/content-explorer/stories/MetadataView.stories.tsx(1 hunks)src/elements/content-explorer/stories/tests/MetadataView-visual.stories.tsx(4 hunks)src/features/metadata-based-view/MetadataQueryAPIHelper.js(4 hunks)src/features/metadata-based-view/__tests__/MetadataQueryAPIHelper.test.js(1 hunks)
💤 Files with no reviewable changes (1)
- src/elements/content-explorer/MetadataView.tsx
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: rafalmaksymiuk
PR: box/box-ui-elements#4136
File: src/elements/common/types/SidebarNavigation.ts:16-26
Timestamp: 2025-06-11T16:30:10.431Z
Learning: `VersionSidebarView` intentionally uses the `versionId` field to stay consistent with current URL parameter naming; a potential rename to `fileVersionId` is deferred until React Router is removed.
scripts/i18n.config.js (3)
Learnt from: tjuanitas
PR: box/box-ui-elements#4126
File: scripts/buildTranslations.js:1-8
Timestamp: 2025-06-17T15:16:46.279Z
Learning: The buildTranslations and buildLanguageBundles functions from @box/frontend package are synchronous functions that already handle errors internally, so additional error handling wrappers and await keywords are not needed.
Learnt from: jpan-box
PR: box/box-ui-elements#4166
File: src/elements/content-sidebar/SidebarNav.js:126-126
Timestamp: 2025-07-11T14:43:02.677Z
Learning: In the box-ui-elements repository, there's a file-type-based pattern for internationalization: TypeScript files (.tsx) predominantly use the modern useIntl hook (41 vs 15 files), while JavaScript files (.js) predominantly use the legacy injectIntl HOC (64 vs 5 files). New TypeScript components should use useIntl, while existing JavaScript components typically continue using injectIntl for consistency.
Learnt from: tjuanitas
PR: box/box-ui-elements#4126
File: .storybook/reactIntl.ts:4-7
Timestamp: 2025-06-17T15:23:50.959Z
Learning: The @box/languages package exports an array of language codes as its default export, not an object. It can be used directly with array methods like .reduce().
scripts/jest/jest.config.js (2)
Learnt from: jpan-box
PR: box/box-ui-elements#4166
File: src/elements/content-sidebar/SidebarNav.js:126-126
Timestamp: 2025-07-11T14:43:02.677Z
Learning: In the box-ui-elements repository, there's a file-type-based pattern for internationalization: TypeScript files (.tsx) predominantly use the modern useIntl hook (41 vs 15 files), while JavaScript files (.js) predominantly use the legacy injectIntl HOC (64 vs 5 files). New TypeScript components should use useIntl, while existing JavaScript components typically continue using injectIntl for consistency.
Learnt from: tjuanitas
PR: box/box-ui-elements#4126
File: scripts/webpack.config.js:72-76
Timestamp: 2025-06-17T15:21:36.180Z
Learning: The Box UI Elements project does not run webpack builds on Windows machines, so Windows path separator compatibility is not a concern for their build scripts.
src/elements/content-explorer/stories/MetadataView.stories.tsx (1)
Learnt from: ahorowitz123
PR: box/box-ui-elements#4102
File: src/features/metadata-instance-editor/Instance.js:647-649
Timestamp: 2025-05-14T17:46:25.370Z
Learning: In the metadata-instance-editor component, `isExistingAIExtractionCascadePolicy` specifically checks if the cascade policy fetched from the backend has AI folder extraction enabled, using props rather than state to reflect the original server-side configuration rather than current UI state.
package.json (3)
Learnt from: jpan-box
PR: box/box-ui-elements#4166
File: src/elements/content-sidebar/SidebarNav.js:126-126
Timestamp: 2025-07-11T14:43:02.677Z
Learning: In the box-ui-elements repository, there's a file-type-based pattern for internationalization: TypeScript files (.tsx) predominantly use the modern useIntl hook (41 vs 15 files), while JavaScript files (.js) predominantly use the legacy injectIntl HOC (64 vs 5 files). New TypeScript components should use useIntl, while existing JavaScript components typically continue using injectIntl for consistency.
Learnt from: rafalmaksymiuk
PR: box/box-ui-elements#4160
File: src/elements/content-sidebar/SidebarToggle.js:21-27
Timestamp: 2025-06-25T13:09:54.538Z
Learning: The box-ui-elements project uses Flow for type annotations in JavaScript files, as indicated by @flow directives in file headers. Type annotations like `: Props` are valid Flow syntax, not TypeScript syntax.
Learnt from: tjuanitas
PR: box/box-ui-elements#4126
File: scripts/webpack.config.js:72-76
Timestamp: 2025-06-17T15:21:36.180Z
Learning: The Box UI Elements project does not run webpack builds on Windows machines, so Windows path separator compatibility is not a concern for their build scripts.
src/elements/content-explorer/Content.tsx (1)
Learnt from: ahorowitz123
PR: box/box-ui-elements#4102
File: src/features/metadata-instance-editor/Instance.js:647-649
Timestamp: 2025-05-14T17:46:25.370Z
Learning: In the metadata-instance-editor component, `isExistingAIExtractionCascadePolicy` specifically checks if the cascade policy fetched from the backend has AI folder extraction enabled, using props rather than state to reflect the original server-side configuration rather than current UI state.
src/elements/content-explorer/ContentExplorer.tsx (3)
Learnt from: ahorowitz123
PR: box/box-ui-elements#4102
File: src/features/metadata-instance-editor/Instance.js:647-649
Timestamp: 2025-05-14T17:46:25.370Z
Learning: In the metadata-instance-editor component, `isExistingAIExtractionCascadePolicy` specifically checks if the cascade policy fetched from the backend has AI folder extraction enabled, using props rather than state to reflect the original server-side configuration rather than current UI state.
Learnt from: rafalmaksymiuk
PR: box/box-ui-elements#4144
File: src/elements/content-sidebar/versions/VersionsList.js:24-33
Timestamp: 2025-06-17T13:30:02.172Z
Learning: In the box-ui-elements codebase, Flow components use .flow.js type definition files, not TypeScript .ts files. The InternalSidebarNavigation type is a union type where different variants may have different properties like versionId, and proper type safety is ensured through conditional checks in methods like getSelectedVersionId.
Learnt from: rafalmaksymiuk
PR: box/box-ui-elements#4136
File: src/elements/common/types/SidebarNavigation.ts:16-26
Timestamp: 2025-06-11T16:30:10.431Z
Learning: `VersionSidebarView` intentionally uses the `versionId` field to stay consistent with current URL parameter naming; a potential rename to `fileVersionId` is deferred until React Router is removed.
🧬 Code Graph Analysis (4)
src/elements/content-explorer/stories/tests/MetadataView-visual.stories.tsx (2)
src/elements/common/__mocks__/mockMetadata.ts (1)
mockSchema(226-226)src/elements/content-explorer/ContentExplorer.tsx (1)
ContentExplorer(1854-1854)
src/elements/content-explorer/stories/MetadataView.stories.tsx (2)
src/elements/common/__mocks__/mockMetadata.ts (2)
mockSchema(226-226)mockMetadata(226-226)src/elements/content-explorer/stories/tests/MetadataView-visual.stories.tsx (1)
metadataView(50-56)
src/elements/content-explorer/Content.tsx (2)
src/elements/content-explorer/MetadataViewContainer.tsx (1)
MetadataViewContainerProps(7-10)src/elements/content-picker/Content.js (1)
currentCollection(47-47)
src/elements/content-explorer/ContentExplorer.tsx (1)
src/elements/content-explorer/MetadataViewContainer.tsx (1)
MetadataViewContainerProps(7-10)
🪛 Biome (1.9.4)
src/features/metadata-based-view/MetadataQueryAPIHelper.js
[error] 57-57: return types can only be used in TypeScript files
remove this type annotation
(parse)
[error] 59-59: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 59-59: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 163-163: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 163-163: return type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 171-171: optional parameters are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 171-171: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 171-171: return type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 174-174: type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 164-168: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
[error] 172-172: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
[error] 247-247: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
[error] 248-254: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint_test_build
- GitHub Check: Summary
🔇 Additional comments (31)
scripts/i18n.config.js (1)
8-9: LGTM! Translation dependencies correctly added.The addition of
@box/metadata-filterand@box/metadata-viewto the translation dependencies aligns with the new metadata view functionality being introduced in this PR.src/common/types/core.js (2)
38-38: LGTM! MetadataTemplate import added correctly.The import statement properly brings in the MetadataTemplate type needed for the Collection type enhancement.
398-398: LGTM! Collection type enhanced with metadata template support.The addition of the optional
metadataTemplateproperty to the Collection type correctly supports the new metadata view functionality while maintaining backward compatibility.src/features/metadata-based-view/__tests__/MetadataQueryAPIHelper.test.js (1)
189-189: LGTM! Test fixture updated to match new API behavior.The addition of
metadataTemplate: templateto theflattenedDataWithTypestest fixture correctly reflects the updated behavior where thegetFlattenedDataWithTypesmethod now includes the metadata template in the returned collection object.src/elements/content-explorer/__tests__/ContentExplorer.test.tsx (1)
413-413: Test assertion updated correctly.The test assertion has been appropriately updated to check for the specific file name that matches the new mock data structure and V2 implementation behavior.
package.json (1)
127-140: Confirm package versions; run local security audit
- File: package.json (lines 127–140)
- All specified versions exist on the npm registry—no missing releases detected.
- Security advisories can’t be fully assessed in this sandbox. Please run
npm audit(ornpm audit --production) in your local environment to ensure no moderate-or-higher vulnerabilities were introduced by these updates.src/elements/content-explorer/__tests__/Content.test.tsx (3)
40-43: LGTM: Appropriate mocking strategy for unit tests.The mock implementation correctly replaces the MetadataViewContainer with a simple identifiable element for testing purposes.
96-102: Good refactor: Extracting shared test data.Extracting the collection object reduces duplication and improves test maintainability.
123-123: Test assertion correctly updated for new component.The test now correctly expects the mocked "MetadataViewContainer" text instead of the previous component's output.
src/elements/content-explorer/stories/tests/MetadataView-visual.stories.tsx (3)
1-1: Good practice: Explicit React import.Adding the explicit React import follows modern React best practices and ensures compatibility.
60-62: LGTM: Proper integration of metadata props.The metadataProps structure correctly passes the columns configuration to the new MetadataViewContainer component.
72-78: Good enhancement: Custom render wrapper.The padding wrapper improves the visual presentation of the story in Storybook.
src/elements/common/__mocks__/mockMetadata.ts (3)
4-4: LGTM: Addition of extension fields for enhanced metadata support.The explicit
extensionfields added to file entries align with the enhanced metadata view requirements and provide better file type identification.Also applies to: 26-26, 48-48, 69-69, 90-90
20-21: Appropriate timestamp field changes.The change from
modified_attocreated_ataligns with the updated metadata schema and API expectations for the V2 implementation.Also applies to: 42-43, 62-64, 83-85, 102-104
108-144: Excellent addition: Folder entries with metadata.Adding folder entries with metadata supports testing of the expanded item type support mentioned in the PR objectives, moving beyond files-only to all item types.
src/elements/content-explorer/Content.tsx (4)
7-7: Import change looks good.The import correctly brings in the new
MetadataViewContainercomponent and its props type, aligning with the architectural change to use the wrapper component.
39-39: Well-designed prop type definition.The use of
Omit<MetadataViewContainerProps, 'currentCollection'>is appropriate here, ascurrentCollectionis already available in the Content component's props and shouldn't be passed throughmetadataProps.
57-57: Props destructuring is correct.The
metadataPropsis properly added to the destructured props list.
84-92: Verify empty‐state support in MetadataViewThe new conditional drops the
!isViewEmptyguard, soMetadataViewContainerwill always render<MetadataView … items={[]} />when the collection is empty. We weren’t able to find any empty-state logic inMetadataViewContaineritself, nor can we inspect the external@box/metadata-viewpackage here.Please ensure that
<MetadataView>:
- Renders an appropriate empty state (e.g. “No items” message or placeholder) when its
itemsprop is an empty array- Or, if it does not handle empty collections internally, consider re-adding the
!isViewEmptycheck before renderingsrc/elements/content-explorer/stories/MetadataView.stories.tsx (1)
80-104: Story configuration is well-structured.The story properly configures the metadata view with columns, enables the V2 feature flag, and provides a clear visual container for the component demonstration.
src/features/metadata-based-view/MetadataQueryAPIHelper.js (5)
23-23: Constructor and import changes are correct.The addition of
FIELD_EXTENSIONandFIELD_CREATED_ATimports and theisV2flag with a default value maintains backward compatibility while enabling the new metadata view features.Also applies to: 57-62
155-160: Conditional metadata handling is appropriate.The V2 mode correctly returns raw metadata without flattening, which aligns with the new metadata view's capability to handle the original metadata structure.
163-168: Expanded item type support for V2.The V2 mode correctly returns all item types (files, folders, web links) instead of filtering to files only, enabling the new metadata view to display various content types.
171-180: Metadata template inclusion is necessary.Adding
metadataTemplateto the returned collection is required for theMetadataViewContainerto configure filters based on the template fields.
247-256: Additional required fields for V2 are appropriate.The V2 mode correctly requires
extensionandcreated_atfields in addition toname, providing more comprehensive file information for the enhanced metadata view.src/elements/content-explorer/MetadataViewContainer.tsx (3)
7-10: Well-designed interface.The interface correctly extends
MetadataViewPropswhile omittingitemsto avoid conflicts, as items are extracted fromcurrentCollection.
41-46: Sort handler implementation is correct.The callback properly handles the sort descriptor and forwards it to the parent component with appropriate type casting.
48-63: Component composition is well-structured.The component correctly composes props with proper precedence - spreading incoming props for
actionBarPropsandtablePropswhile adding the computedfilterGroupsandhandleSortChange.src/elements/content-explorer/ContentExplorer.tsx (3)
176-176: Type extension for flexible sorting.The
sortBy: SortBy | stringtype change appropriately supports sorting by metadata fields while maintaining compatibility with existing sort options.
403-442: Metadata query enhancements are well-implemented.The method correctly:
- Preserves existing
order_byvalues while adding new sort parameters- Conditionally instantiates
MetadataQueryAPIHelperwith V2 mode based on feature flag- Maintains backward compatibility with existing queries
854-863: Sort method correctly supports metadata view.The condition
if (id || view === VIEW_METADATA)appropriately enables sorting for both folder views (with id) and metadata views.
greg-in-a-box
force-pushed
the
metadata-view
branch
3 times, most recently
from
3c9bd8c to
b49985c
Compare
jpan-box
left a comment
jpan-box
left a comment
There was a problem hiding this comment.
Approved if you would like to move forward - otherwise, just the two comments (refactoring Collection, which can be done in a separate PR even if you think it's a good idea, and spelling)
greg-in-a-box
force-pushed
the
metadata-view
branch
from
b49985c to
8a8a0ce
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/common/types/core.js (1)
397-397: Consider using a specialized Collection type for metadata viewsBuilding on the previous discussion, adding
metadataTemplateto the coreCollectiontype does feel like mixing concerns. While CodeRabbit's suggestion of specialized Collection types (e.g.,MetadataCollection) would be architecturally cleaner, I understand the timeline constraints mentioned.For now, this optional property maintains backward compatibility. However, I'd recommend tracking this as technical debt for future refactoring when time permits.
🧹 Nitpick comments (1)
src/elements/content-explorer/MetadataQueryAPIHelper.ts (1)
177-191: Consider adding error handling for malformed metadataThe method assumes metadata follows the expected structure. Consider adding defensive checks for edge cases.
getTemplateSchemaInfo = (data: MetadataQueryResponseData): Promise<MetadataTemplateSchemaResponse | void> => { const { entries } = data; this.metadataQueryResponseData = this.filterMetdataQueryResponse(data); if (!entries || entries.length === 0) { // Don't make metadata API call to get template info return Promise.resolve(); } const metadata = getProp(entries, '[0].metadata'); + if (!metadata || typeof metadata !== 'object') { + return Promise.reject(new Error('Invalid metadata structure in query response')); + } + this.templateScope = Object.keys(metadata)[0]; + if (!this.templateScope) { + return Promise.reject(new Error('No template scope found in metadata')); + } + const instance = metadata[this.templateScope]; + if (!instance || typeof instance !== 'object') { + return Promise.reject(new Error('Invalid metadata instance structure')); + } + this.templateKey = Object.keys(instance)[0]; + if (!this.templateKey) { + return Promise.reject(new Error('No template key found in metadata instance')); + } return this.api.getMetadataAPI(true).getSchemaByTemplateKey(this.templateKey); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (16)
package.json(5 hunks)scripts/i18n.config.js(1 hunks)scripts/jest/jest.config.js(1 hunks)src/common/types/core.js(2 hunks)src/elements/common/__mocks__/mockMetadata.ts(3 hunks)src/elements/content-explorer/Content.tsx(5 hunks)src/elements/content-explorer/ContentExplorer.tsx(16 hunks)src/elements/content-explorer/MetadataQueryAPIHelper.ts(1 hunks)src/elements/content-explorer/MetadataView.tsx(0 hunks)src/elements/content-explorer/MetadataViewContainer.tsx(1 hunks)src/elements/content-explorer/__tests__/Content.test.tsx(3 hunks)src/elements/content-explorer/__tests__/ContentExplorer.test.tsx(2 hunks)src/elements/content-explorer/__tests__/MetadataQueryAPIHelper.test.ts(1 hunks)src/elements/content-explorer/__tests__/MetadataViewContainer.test.tsx(1 hunks)src/elements/content-explorer/stories/MetadataView.stories.tsx(1 hunks)src/elements/content-explorer/stories/tests/MetadataView-visual.stories.tsx(4 hunks)
💤 Files with no reviewable changes (1)
- src/elements/content-explorer/MetadataView.tsx
🚧 Files skipped from review as they are similar to previous changes (12)
- scripts/i18n.config.js
- src/elements/content-explorer/tests/ContentExplorer.test.tsx
- scripts/jest/jest.config.js
- src/elements/content-explorer/tests/MetadataViewContainer.test.tsx
- src/elements/common/mocks/mockMetadata.ts
- src/elements/content-explorer/stories/MetadataView.stories.tsx
- src/elements/content-explorer/tests/Content.test.tsx
- src/elements/content-explorer/stories/tests/MetadataView-visual.stories.tsx
- package.json
- src/elements/content-explorer/MetadataViewContainer.tsx
- src/elements/content-explorer/Content.tsx
- src/elements/content-explorer/ContentExplorer.tsx
🧰 Additional context used
🧠 Learnings (1)
src/common/types/core.js (1)
Learnt from: ahorowitz123
PR: #4102
File: src/features/metadata-instance-editor/Instance.js:647-649
Timestamp: 2025-05-14T17:46:25.370Z
Learning: In the metadata-instance-editor component, isExistingAIExtractionCascadePolicy specifically checks if the cascade policy fetched from the backend has AI folder extraction enabled, using props rather than state to reflect the original server-side configuration rather than current UI state.
🪛 Biome (2.1.2)
src/common/types/core.js
[error] 37-37: 'import type' are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint_test_build
- GitHub Check: Summary
🔇 Additional comments (1)
src/elements/content-explorer/__tests__/MetadataQueryAPIHelper.test.ts (1)
1-476: Well-structured and comprehensive test coverage!The test suite thoroughly covers all methods of MetadataQueryAPIHelper with appropriate edge cases, mocking patterns, and async handling. Great job on the test organization and coverage.
greg-in-a-box
force-pushed
the
metadata-view
branch
from
8a8a0ce to
977bd83
Compare
src/elements/content-explorer/__tests__/MetadataViewContainer.test.tsx
Outdated
Show resolved
Hide resolved
greg-in-a-box
force-pushed
the
metadata-view
branch
from
977bd83 to
ae370e0
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/elements/content-explorer/MetadataQueryAPIHelper.ts (2)
88-106: Consider adding type safety for field extraction.The method logic is sound and well-documented. However,
split('.').pop()could potentially returnundefined, which might cause issues downstream.Consider adding a filter to ensure only valid field names are returned:
- return fields.filter(field => field.includes(from)).map(field => field.split('.').pop()); + return fields.filter(field => field.includes(from)).map(field => field.split('.').pop()).filter(Boolean) as string[];
108-144: Well-implemented metadata flattening with good type enrichment.The method effectively transforms nested metadata into a flattened structure enriched with template information. The handling of different field types and the addition of display names and options for select fields is well-implemented.
Consider extracting the field mapping logic into a separate method for better readability:
private mapQueryFieldToMetadataField = (queryField: string, instance: any, templateFields: any[]): MetadataQueryInstanceTypeField => { const templateField = find(templateFields, ['key', queryField]); const type = getProp(templateField, 'type'); const displayName = getProp(templateField, 'displayName', queryField); const field: MetadataQueryInstanceTypeField = { key: `${FIELD_METADATA}.${this.templateScope}.${this.templateKey}.${queryField}`, value: instance[queryField], type, displayName, }; if (includes(SELECT_TYPES, type)) { field.options = getProp(templateField, 'options'); } return field; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (15)
package.json(5 hunks)scripts/i18n.config.js(1 hunks)scripts/jest/jest.config.js(1 hunks)src/elements/common/__mocks__/mockMetadata.ts(3 hunks)src/elements/content-explorer/Content.tsx(5 hunks)src/elements/content-explorer/ContentExplorer.tsx(16 hunks)src/elements/content-explorer/MetadataQueryAPIHelper.ts(1 hunks)src/elements/content-explorer/MetadataView.tsx(0 hunks)src/elements/content-explorer/MetadataViewContainer.tsx(1 hunks)src/elements/content-explorer/__tests__/Content.test.tsx(3 hunks)src/elements/content-explorer/__tests__/ContentExplorer.test.tsx(2 hunks)src/elements/content-explorer/__tests__/MetadataQueryAPIHelper.test.ts(1 hunks)src/elements/content-explorer/__tests__/MetadataViewContainer.test.tsx(1 hunks)src/elements/content-explorer/stories/MetadataView.stories.tsx(1 hunks)src/elements/content-explorer/stories/tests/MetadataView-visual.stories.tsx(4 hunks)
💤 Files with no reviewable changes (1)
- src/elements/content-explorer/MetadataView.tsx
🚧 Files skipped from review as they are similar to previous changes (12)
- scripts/i18n.config.js
- src/elements/content-explorer/tests/MetadataViewContainer.test.tsx
- scripts/jest/jest.config.js
- src/elements/content-explorer/tests/Content.test.tsx
- src/elements/content-explorer/tests/ContentExplorer.test.tsx
- src/elements/content-explorer/stories/MetadataView.stories.tsx
- src/elements/content-explorer/stories/tests/MetadataView-visual.stories.tsx
- src/elements/content-explorer/tests/MetadataQueryAPIHelper.test.ts
- src/elements/content-explorer/MetadataViewContainer.tsx
- src/elements/content-explorer/Content.tsx
- src/elements/content-explorer/ContentExplorer.tsx
- package.json
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: rafalmaksymiuk
PR: box/box-ui-elements#4136
File: src/elements/common/types/SidebarNavigation.ts:16-26
Timestamp: 2025-06-11T16:30:10.431Z
Learning: `VersionSidebarView` intentionally uses the `versionId` field to stay consistent with current URL parameter naming; a potential rename to `fileVersionId` is deferred until React Router is removed.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint_test_build
- GitHub Check: Summary
🔇 Additional comments (6)
src/elements/common/__mocks__/mockMetadata.ts (1)
1-146: Excellent comprehensive mock data structure!The expanded mock metadata provides a rich dataset that effectively supports the new MetadataView V2 functionality. The additions include:
- Essential file/folder properties:
extension,created_at,etag,id- Consistent metadata structure following
enterprise_0.templateNamepattern- Industry values that align with schema options (Technology, Legal, Healthcare)
- Role assignments matching the multiSelect field options
- Good mix of files and folders for comprehensive testing
This mock data will provide excellent coverage for testing the metadata view components and API helper functionality.
src/elements/content-explorer/MetadataQueryAPIHelper.ts (5)
1-37: Well-structured imports and type definitions.The imports are comprehensive and appropriate for the functionality provided. Good use of lodash utilities for data manipulation and clear type definitions for callback functions and metadata operations.
38-53: Clean constructor and property definitions.The constructor properly injects the API dependency, and the class properties are well-typed and logically organized to maintain state across method calls.
55-86: Robust JSON Patch operations implementation.The method correctly handles all JSON Patch operation types (ADD, REPLACE, REMOVE) with appropriate logic for determining which operation to use based on old/new value combinations. The inclusion of TEST operations provides good concurrency protection, and the proper handling of the value field for REMOVE operations follows RFC 6902 specifications.
146-177: Clean data processing pipeline methods.These methods form a logical progression in the metadata processing pipeline:
getDataWithTypesproperly stores the template and returns the collectiongetTemplateSchemaInfointelligently handles empty results and correctly extracts template information from the first entryqueryMetadataappropriately promisifies the callback-based API callThe implementations are clean and handle edge cases well.
179-232: Well-orchestrated main methods completing the API helper.The final methods effectively complete the functionality:
fetchMetadataQueryResultsproperly orchestrates the entire pipeline with good error handlingupdateMetadatacorrectly utilizes the JSON Patch operations for metadata updatesverifyQueryFieldsis crucial for ensuring required fields (name, extension) are present for proper UI displayThe implementation handles the complete metadata query and update lifecycle effectively. The field verification is particularly valuable for ensuring consistent UI behavior.
greg-in-a-box
force-pushed
the
metadata-view
branch
from
caf61f2 to
3e06e23
Compare
greg-in-a-box
force-pushed
the
metadata-view
branch
2 times, most recently
from
aad2fed to
f98b793
Compare
greg-in-a-box
force-pushed
the
metadata-view
branch
from
f98b793 to
eec6735
Compare
4 participants
MetadataViewmetadataPropsto provides props to theMetadataViewcomponent directlyMetadataQueryAPIHelperto switch to new logic for the v2 design such as retrieving extensions and created at date. Also, allows all item types to return instead of files onlyallowsSortingprop in thecolumndataSummary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation
Chores