feat(metadata-editor): Parse autofill values correctly for taxonomy fields #4398
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThis PR updates three Box package dependencies ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/elements/content-sidebar/__tests__/useSidebarMetadataFetcher.test.tsx (1)
374-420: Good test coverage for the taxonomy mapping feature.The test correctly verifies that taxonomy field suggestions are mapped to the expected
{value, displayValue}structure. The test setup is clean and the assertions are appropriate.Consider adding edge case tests for robustness:
- Empty taxonomy array
- Taxonomy items with missing
idordisplayName- Mixed field types (taxonomy and non-taxonomy) in the same template
📜 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 (3)
package.json(2 hunks)src/elements/content-sidebar/__tests__/useSidebarMetadataFetcher.test.tsx(1 hunks)src/elements/content-sidebar/hooks/useSidebarMetadataFetcher.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: tjuanitas
Repo: box/box-ui-elements PR: 4224
File: package.json:296-297
Timestamp: 2025-08-12T18:04:17.698Z
Learning: In the box-ui-elements project, the team is comfortable with raising peerDependency minimum versions when upgrading blueprint-web packages, even if it's a breaking change for consumers.
Learnt from: jpan-box
Repo: box/box-ui-elements PR: 4248
File: src/elements/content-explorer/MetadataViewContainer.tsx:30-44
Timestamp: 2025-08-27T15:25:53.253Z
Learning: In the Box API, there is an inconsistency where some endpoints require trimmed metadata field names (e.g., "industry") while others require the full field path (e.g., "metadata.enterprise_1515946.templateName.industry"). The trimMetadataFieldPrefix helper function in MetadataViewContainer.tsx correctly trims the field names for API endpoints that expect the shorter format.
📚 Learning: 2025-08-25T16:19:22.007Z
Learnt from: greg-in-a-box
Repo: box/box-ui-elements PR: 4235
File: src/elements/content-explorer/MetadataQueryBuilder.ts:103-110
Timestamp: 2025-08-25T16:19:22.007Z
Learning: In src/elements/content-explorer/MetadataQueryBuilder.ts, the getSelectFilter function has a special case that maps 'mimetype-filter' to 'item.extension' because mimetype-filter is a multi-select field but the actual database field is 'item.extension'. This mapping is necessary and should not be removed.
Applied to files:
src/elements/content-sidebar/__tests__/useSidebarMetadataFetcher.test.tsxsrc/elements/content-sidebar/hooks/useSidebarMetadataFetcher.ts
📚 Learning: 2025-08-12T18:04:17.698Z
Learnt from: tjuanitas
Repo: box/box-ui-elements PR: 4224
File: package.json:296-297
Timestamp: 2025-08-12T18:04:17.698Z
Learning: In the box-ui-elements project, the team is comfortable with raising peerDependency minimum versions when upgrading blueprint-web packages, even if it's a breaking change for consumers.
Applied to files:
package.json
📚 Learning: 2025-08-21T19:19:44.584Z
Learnt from: jpan-box
Repo: box/box-ui-elements PR: 4237
File: src/api/ZipDownload.ts:6-8
Timestamp: 2025-08-21T19:19:44.584Z
Learning: For Box TypeScript SDK integrations, jpan-box prefers deep imports (e.g., from box-typescript-sdk-gen/lib/schemas/...generated.d.ts.js) over defining local types, citing that Box v2 APIs don't typically have breaking changes, making the deep import approach more viable despite potential brittleness.
Applied to files:
package.json
⏰ 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). (3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Summary
🔇 Additional comments (2)
package.json (1)
299-305: Confirm that the dependency updates are intentional and tested.Multiple Box package dependencies have been updated:
- @box/blueprint-web to ^12.104.1
- @box/combobox-with-api to ^1.18.0
These appear to be significant minor version bumps. Please confirm that these updates have been tested and are compatible with the current codebase, especially given the changes to taxonomy field handling in this PR.
src/elements/content-sidebar/hooks/useSidebarMetadataFetcher.ts (1)
268-284: Add defensive checks for taxonomy field values to handle API response variability.The code assumes taxonomy field
valueis always an array of objects withidanddisplayNameproperties without runtime validation. Given that Box API response structures have been known to change frequently, consider adding guards to safely access these properties:if (!value) { return field; } if (field.type === 'taxonomy') { return { ...field, aiSuggestion: Array.isArray(value) ? value.map(item => ({ value: item?.id, displayValue: item?.displayName, })) : value, }; }Alternatively, verify the TypeScript types guarantee this structure is always enforced.
|
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/components/sidebar-toggle-button/SidebarToggleButton.js (1)
66-79: Add inline comments to clarify the icon selection logic.The
renderModernizedIconfunction has complex branching based on breakpoint, direction, and open state. The icon behavior is unintuitive becauseRightSidebarChevron*icons are used for both sidebar directions with reversed semantics. Consider adding comments explaining:
- Why
ChevronUpandChevronDownare used below the medium breakpoint- Why the
RightSidebarChevron*icons have inverted meanings between left and right directions (e.g.,RightSidebarChevronOpenmeans "opened left sidebar" but "closed right sidebar")
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
i18n/ja-JP.propertiesis excluded by!i18n/**yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (5)
flow-typed/npm/@box/blueprint-web-assets_vx.x.x.js(1 hunks)package.json(2 hunks)src/components/sidebar-toggle-button/SidebarToggleButton.js(4 hunks)src/components/sidebar-toggle-button/SidebarToggleButton.scss(1 hunks)src/components/sidebar-toggle-button/__tests__/SidebarToggleButton.test.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: tjuanitas
Repo: box/box-ui-elements PR: 4224
File: package.json:296-297
Timestamp: 2025-08-12T18:04:17.698Z
Learning: In the box-ui-elements project, the team is comfortable with raising peerDependency minimum versions when upgrading blueprint-web packages, even if it's a breaking change for consumers.
📚 Learning: 2025-09-03T18:10:42.760Z
Learnt from: fpan225
Repo: box/box-ui-elements PR: 4239
File: src/elements/content-sidebar/SidebarNav.js:195-206
Timestamp: 2025-09-03T18:10:42.760Z
Learning: In src/elements/content-sidebar/SidebarNav.js, the maintainer wants users to be able to override data-* attributes (data-resin-target, data-target-id, data-testid) through navButtonProps for custom tabs. The spread of {...navButtonProps} should come after the data-* attributes to allow overriding, not before them.
Applied to files:
src/components/sidebar-toggle-button/SidebarToggleButton.scsssrc/components/sidebar-toggle-button/SidebarToggleButton.jssrc/components/sidebar-toggle-button/__tests__/SidebarToggleButton.test.js
📚 Learning: 2025-09-03T18:10:29.467Z
Learnt from: fpan225
Repo: box/box-ui-elements PR: 4239
File: src/elements/content-sidebar/SidebarNav.js:94-106
Timestamp: 2025-09-03T18:10:29.467Z
Learning: In the ContentSidebar CustomSidebarPanel API, the navButtonProps spread should come after explicit data-* attributes to allow users to override analytics tracking attributes like data-resin-target, data-target-id, and data-testid when needed. This is intentional API design for flexibility.
Applied to files:
src/components/sidebar-toggle-button/SidebarToggleButton.scsssrc/components/sidebar-toggle-button/SidebarToggleButton.js
📚 Learning: 2025-06-25T13:09:54.538Z
Learnt from: rafalmaksymiuk
Repo: box/box-ui-elements PR: 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.
Applied to files:
flow-typed/npm/@box/blueprint-web-assets_vx.x.x.js
📚 Learning: 2025-06-17T13:30:02.172Z
Learnt from: rafalmaksymiuk
Repo: box/box-ui-elements PR: 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.
Applied to files:
flow-typed/npm/@box/blueprint-web-assets_vx.x.x.js
📚 Learning: 2025-08-12T18:04:17.698Z
Learnt from: tjuanitas
Repo: box/box-ui-elements PR: 4224
File: package.json:296-297
Timestamp: 2025-08-12T18:04:17.698Z
Learning: In the box-ui-elements project, the team is comfortable with raising peerDependency minimum versions when upgrading blueprint-web packages, even if it's a breaking change for consumers.
Applied to files:
flow-typed/npm/@box/blueprint-web-assets_vx.x.x.js
📚 Learning: 2025-06-25T13:10:19.128Z
Learnt from: rafalmaksymiuk
Repo: box/box-ui-elements PR: 4160
File: src/elements/content-sidebar/SidebarToggle.js:11-11
Timestamp: 2025-06-25T13:10:19.128Z
Learning: In the box-ui-elements codebase, Flow files support `import type` syntax for type-only imports. The `import type` statement is valid Flow syntax, not just TypeScript, and should not be flagged as an error when used in Flow files (indicated by flow comments).
Applied to files:
flow-typed/npm/@box/blueprint-web-assets_vx.x.x.js
📚 Learning: 2025-09-03T18:30:44.447Z
Learnt from: fpan225
Repo: box/box-ui-elements PR: 4239
File: src/elements/content-sidebar/SidebarPanels.js:0-0
Timestamp: 2025-09-03T18:30:44.447Z
Learning: In the CustomSidebarPanel type, the component field is required (React.ComponentType<any>), so runtime checks for component existence are unnecessary since Flow will catch missing components at compile time. User fpan225 prefers to rely on the type system rather than adding redundant runtime checks.
Applied to files:
flow-typed/npm/@box/blueprint-web-assets_vx.x.x.js
📚 Learning: 2025-07-11T14:43:02.677Z
Learnt from: jpan-box
Repo: box/box-ui-elements PR: 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.
Applied to files:
src/components/sidebar-toggle-button/SidebarToggleButton.js
📚 Learning: 2025-08-27T17:03:48.322Z
Learnt from: bxie-box
Repo: box/box-ui-elements PR: 4250
File: src/features/classification/applied-by-ai-classification-reason/__tests__/AppliedByAiClassificationReason.test.tsx:44-51
Timestamp: 2025-08-27T17:03:48.322Z
Learning: In test files for bxie-box, prefer using queryByTestId over getByTestId when testing for expected elements that should always exist, as null access serves as validation for regressions or unexpected changes rather than masking issues with defensive assertions.
Applied to files:
src/components/sidebar-toggle-button/__tests__/SidebarToggleButton.test.js
🧬 Code graph analysis (2)
src/components/sidebar-toggle-button/SidebarToggleButton.js (2)
src/elements/content-sidebar/additional-tabs/AdditionalTabTooltip.js (1)
isPreviewModernizationEnabled(23-23)src/elements/common/feature-checking/hooks.js (1)
useFeatureConfig(6-9)
src/components/sidebar-toggle-button/__tests__/SidebarToggleButton.test.js (1)
src/components/sidebar-toggle-button/SidebarToggleButton.js (1)
isPreviewModernizationEnabled(44-44)
⏰ 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). (4)
- GitHub Check: lint_test_build
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Summary
🔇 Additional comments (9)
src/components/sidebar-toggle-button/SidebarToggleButton.scss (2)
33-38: LGTM: Well-documented stacking context fix.The comment clearly explains why
position: relativeis needed for z-index layering with ContentSidebar.
40-46: LGTM: Modernized hover interaction.The hover transform is applied only to the modernized variant, while the legacy styles remain intact for backward compatibility.
flow-typed/npm/@box/blueprint-web-assets_vx.x.x.js (1)
12-15: LGTM: Icon type declarations added.The Flow type declarations for the new chevron icons follow the existing pattern and support the modernized SidebarToggleButton rendering path.
src/components/sidebar-toggle-button/__tests__/SidebarToggleButton.test.js (1)
68-75: LGTM: Test assertions updated for modernization flag.The conditional assertions correctly verify that the modernized class is present when the feature flag is enabled and absent when disabled, ensuring proper feature flag behavior.
src/components/sidebar-toggle-button/SidebarToggleButton.js (5)
43-56: LGTM: Feature flag and direction logic.The
useBreakpointhook is called unconditionally (following React's rules of hooks), and the direction/tooltip position logic is clear and well-structured.
96-101: LGTM: Legacy icon rendering extracted.Extracting the icon selection logic into
renderIconimproves code organization while preserving the original behavior.
103-116: LGTM: Legacy rendering path preserved.The non-modernized path maintains backward compatibility while benefiting from the
renderIconrefactoring. This is a solid feature flag implementation pattern.
82-93: Verify BPTooltipsideprop API.The modernized path uses
side={tooltipPositionModernized}with values 'right' or 'left', while the legacy path usesposition={tooltipPosition}with values 'middle-right' or 'middle-left'. Confirm that BPTooltip'ssideprop accepts these values and that the positioning behavior is equivalent.
7-13: Verify library API compatibility.The component now uses
IconButton,useBreakpoint,Breakpoint, and new icon imports from upgraded package versions. Ensure these APIs are available and stable in the specified versions.#!/bin/bash # Verify the package versions and exports exist # Check @box/blueprint-web version in package.json echo "=== Checking @box/blueprint-web version ===" cat package.json | jq '.dependencies["@box/blueprint-web"], .peerDependencies["@box/blueprint-web"]' # Check @box/blueprint-web-assets version echo "=== Checking @box/blueprint-web-assets version ===" cat package.json | jq '.dependencies["@box/blueprint-web-assets"], .peerDependencies["@box/blueprint-web-assets"]' # Search for other usages of IconButton, useBreakpoint, Breakpoint in the codebase echo "=== Checking IconButton usage ===" rg -n --type=js --type=tsx -C2 "import.*IconButton.*from.*blueprint-web" echo "=== Checking useBreakpoint usage ===" rg -n --type=js --type=tsx -C2 "import.*useBreakpoint.*from.*blueprint-web"
AnnaStepchenko
force-pushed
the
metadata-editor-taxonomy-ai-autofill
branch
from
7c93883 to
f65ec63
Compare
AnnaStepchenko
force-pushed
the
metadata-editor-taxonomy-ai-autofill
branch
from
f65ec63 to
4f03993
Compare
Merge Queue Status✅ The pull request has been merged at adfb61b This pull request spent 7 seconds in the queue, with no time running CI. Required conditions to merge
|
Description
Adds support for taxonomy field suggestions in the metadata sidebar fetcher. Taxonomy suggestions are now properly mapped with
valueanddisplayValueproperties for AI-generated suggestions.Related issues
N/A
Changes made
useSidebarMetadataFetcherto map taxonomy field suggestions withvalueanddisplayValuepropertiesType of change
Testing done
Screenshots/Videos
Dependencies
None
Deployment notes
None
How to test
N/A
How to review
Review the implementation in
useSidebarMetadataFetcher.tsto ensure taxonomy suggestions are properly mapped. Verify the test case covers the expected behavior.Self-review checklist
Additional notes
None
Summary by CodeRabbit
Chores
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.