-
Notifications
You must be signed in to change notification settings - Fork 344
feat(metadata-editor): Parse autofill values correctly for taxonomy fields (2) #4401
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(metadata-editor): Parse autofill values correctly for taxonomy fields (2) #4401
Conversation
|
Anna Stepchenko seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
WalkthroughThis PR updates Box UI package dependencies (blueprint-web, combobox-with-api, metadata-editor) and adds support for AI suggestions in taxonomy-type fields, mapping suggestions to value/displayValue pairs when available. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/elements/content-sidebar/hooks/useSidebarMetadataFetcher.ts (1)
267-279: Consider adding defensive checks for taxonomy field value structure.The code assumes that for taxonomy fields,
valueis always an array and each item hasidanddisplayNameproperties. If the API returns unexpected data, this could cause a runtime error.🔎 Apply this diff to add defensive checks:
if (field.type === 'taxonomy') { + const taxonomyValue = Array.isArray(value) ? value : []; return { ...field, - aiSuggestion: value.map(item => ({ - value: item.id, - displayValue: item.displayName, + aiSuggestion: taxonomyValue.map(item => ({ + value: item?.id ?? '', + displayValue: item?.displayName ?? '', })), }; }src/elements/content-sidebar/__tests__/useSidebarMetadataFetcher.test.tsx (1)
374-420: LGTM! Test validates taxonomy field mapping correctly.The test comprehensively validates that taxonomy field suggestions are mapped to the expected
{value, displayValue}structure. The setup, mocking, and assertions are all correct.Consider adding edge case tests for taxonomy fields, such as:
- Empty taxonomy suggestions array
- Taxonomy items missing
idordisplayNameproperties- Non-array value for taxonomy field type
These would help ensure robustness if the API returns unexpected data.
📜 Review details
Configuration used: Organization 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 (5)
📓 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-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/elements/content-sidebar/__tests__/useSidebarMetadataFetcher.test.tsx
📚 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
🧬 Code graph analysis (1)
src/elements/content-sidebar/__tests__/useSidebarMetadataFetcher.test.tsx (1)
src/features/unified-share-modal/__tests__/ContactsField.test.js (1)
suggestions(68-87)
⏰ 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: lint_test_build
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Summary
🔇 Additional comments (1)
package.json (1)
127-127: Verify package versions and security advisories for the updated Box UI dependencies.The dependency updates include version bumps for
@box/blueprint-web(^12.104.1),@box/combobox-with-api(^1.18.0), and@box/metadata-editor(^1.18.0). Ensure these versions exist in the npm registry and check for any security advisories usingnpm audit.
|
The original PR has been updated with the requested changes and is ready to be merged directly. This fork-based PR was only created as a workaround for permission issues, which are no longer relevant. |
Description
This PR is a recreation of #4398 originally authored by @JakubKida. The original PR could not be updated by me directly due to repository permission restrictions, so this PR re-submits the same changes via fork.
Adds support for taxonomy field suggestions in the metadata sidebar fetcher. Taxonomy suggestions are now properly mapped with value and displayValue properties for AI-generated suggestions.
Related issues
N/A
Changes made
useSidebarMetadataFetcherto map taxonomy field suggestions withvalueanddisplayValuepropertiesType of change
[x] New feature (non-breaking change adding functionality)
[ ] Bug fix (non-breaking change addressing an issue)
[ ] Breaking change (fix or feature causing existing functionality to change)
[ ] Code refactor (no functional changes)
[ ] Documentation update
[ ] Performance improvement
[x] Test addition or update
[ ] CI/CD configuration change
Testing done
[x] Unit tests added/updated
[ ] Integration tests added/updated
[x] Manual testing performed
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
[x] Code follows the project's style guidelines
[x] Code is properly documented (comments, JSDoc/docstrings, etc.)
[x] Changes are covered by tests
[x] All tests pass locally
[x] No unnecessary console logs or debugging code
[x] No sensitive information is exposed
[x] No new warnings or errors are introduced
[x] PR title follows conventional commit format
Summary by CodeRabbit
Bug Fixes
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.