fix: support mid-migration metadata templates - #4653
Open
JakubKida wants to merge 14 commits into
Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughMetadata APIs and types now support namespace-aware templates and namespace-only instances across SCOPED, MIGRATION, and FINAL modes. The sidebar adds namespace browsing, template creation/editing, schema loading, and metadata refresh flows backed by live or mocked APIs. ChangesNamespace metadata workflow
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Contributor
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
JakubKida
force-pushed
the
metadata-namespace-migration-support
branch
from
June 26, 2026 09:38
74d0d83 to
c73a5f2
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@s.md`:
- Around line 2280-2286: The LIST Cascade Policy FINAL description is using the
wrong operation wording; update the text in the metadata cascade policies
documentation so it says listing/retrieval instead of creation. Locate the
endpoint description around the GET metadata_cascade_policies entry and replace
the misleading phrase in the final section while keeping the rest of the spec
reference intact.
- Around line 2068-2072: The GET Cascade Policy (SCOPED) description is using
the wrong action wording, referring to “creation” in a retrieval endpoint.
Update the endpoint description text in the relevant GET metadata cascade policy
entry so it clearly says retrieval/fetching instead of creation, keeping the
rest of the spec reference unchanged.
- Around line 1832-1843: The LIST Instance FINAL description includes an
irrelevant scope restriction that belongs to other operations. Update the text
in the metadata listing section so it only describes the LIST Instance behavior,
and remove the “If global is passed instead of box.metadata, returns 400”
sentence from the listing entry. Use the surrounding metadata endpoint section
and its LIST Instance heading to locate the affected copy-pasted wording.
- Around line 2229-2235: The LIST Cascade Policy migration text uses the wrong
operation wording in the endpoint description; update the copy in the metadata
cascade policies section so it says “listing” or “retrieval” instead of
“creation.” Locate the affected wording in the migration docs around the GET
metadata_cascade_policies entry and replace the misleading phrase while keeping
the rest of the spec reference unchanged.
- Around line 2185-2191: The operation description for the LIST Cascade Policy
SCOPED endpoint is incorrect because it says “creation” instead of describing a
list/retrieval action. Update the text in the metadata cascade policy section so
the wording matches the GET metadata_cascade_policies endpoint and the
surrounding “LIST Cascade Policy” context, using the relevant SCOPED entry to
locate it.
- Around line 2147-2152: The GET Cascade Policy FINAL description uses the wrong
action term, since this endpoint is for retrieval rather than creation. Update
the wording in the metadata cascade policy GET section so the sentence about
scoped and namespaced cascade policy behavior refers to retrieval or fetching,
and keep the rest of the endpoint description aligned with the GET operation
name and path.
- Around line 2103-2108: The GET Metadata Cascade Policy migration description
is using the wrong operation wording, referring to “creation” for a retrieval
endpoint. Update the text in the GET Cascade Policy section so it describes
fetching/retrieving the policy rather than creating it, keeping the rest of the
endpoint details and spec reference unchanged.
- Line 1: The PR title contains a typo: update “temolates” to “templates” in the
title text so it matches the intended wording.
In `@src/api/Metadata.js`:
- Around line 565-575: The template lookup in Metadata.js can incorrectly match
a namespace-only template when $scope is undefined because the primary
templates.find in the scope-based path matches on undefined scope and blocks the
namespace fallback. Update the template selection logic around the namespace and
scope checks so the first lookup only runs when scope is actually defined, and
let the namespace-based fallback handle namespace-only instances in
MIGRATION/FINAL mode using templateKey plus namespace.
- Around line 423-431: The schema cache in getSchemaByTemplateKey currently uses
only templateKey, which can return the wrong result when scope changes between
calls. Update the cache key generation and lookup in
Metadata.getSchemaByTemplateKey/getMetadataTemplateSchemaCacheKey so it
incorporates scope alongside templateKey, and ensure the same scope-aware key is
used for both cache.has and cache.get before calling
getMetadataTemplateSchemaUrl.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9cbcb786-2c6b-4362-ad31-a1b633615294
📒 Files selected for processing (3)
s.mdsrc/api/Metadata.jssrc/common/types/metadata.js
| @@ -0,0 +1,2390 @@ | |||
| Namespaces API Specification | |||
Contributor
There was a problem hiding this comment.
Fix typo in PR title: "temolates" → "templates"
The PR title contains a typo: "temolates" should be "templates".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@s.md` at line 1, The PR title contains a typo: update “temolates” to
“templates” in the title text so it matches the intended wording.
Comment on lines
+1832
to
+1843
| 2.5.3 FINAL | ||
| Full URL: | ||
| GET https://api.box.com/2.0/files/{file_id}/metadata | ||
|
|
||
| GET https://api.box.com/2.0/folders/{folder_id}/metadata | ||
|
|
||
| Both scoped and namespaced instance retrieval is allowed. | ||
| If global is passed instead of box.metadata, returns 400. | ||
| See the full specs: | ||
| https://developer.box.com/reference/get-files-id-metadata | ||
| https://developer.box.com/reference/get-folders-id-metadata | ||
|
|
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Remove irrelevant restriction from LIST Instance (FINAL)
Line 1839 states "If global is passed instead of box.metadata, returns 400" but the LIST Instance endpoint path (/files/{file_id}/metadata) does not include a scope/namespace path parameter. This appears to be a copy-paste error from GET/CREATE/UPDATE/DELETE sections.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@s.md` around lines 1832 - 1843, The LIST Instance FINAL description includes
an irrelevant scope restriction that belongs to other operations. Update the
text in the metadata listing section so it only describes the LIST Instance
behavior, and remove the “If global is passed instead of box.metadata, returns
400” sentence from the listing entry. Use the surrounding metadata endpoint
section and its LIST Instance heading to locate the affected copy-pasted
wording.
Comment on lines
+2068
to
+2072
| GET https://api.box.com/2.0/metadata_cascade_policies/{metadata_cascade_policy_id} | ||
|
|
||
| Only scoped cascade policy creation is allowed. | ||
| See the full spec https://developer.box.com/reference/get-metadata-cascade-policies-id | ||
|
|
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix incorrect operation description in GET Cascade Policy (SCOPED)
Line 2070 says "Only scoped cascade policy creation is allowed" but this is the GET endpoint (3.3 GET Cascade Policy). Should be "retrieval" or similar.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@s.md` around lines 2068 - 2072, The GET Cascade Policy (SCOPED) description
is using the wrong action wording, referring to “creation” in a retrieval
endpoint. Update the endpoint description text in the relevant GET metadata
cascade policy entry so it clearly says retrieval/fetching instead of creation,
keeping the rest of the spec reference unchanged.
Comment on lines
+2103
to
+2108
| 3.3.2 MIGRATION | ||
| Full URL: | ||
| GET https://api.box.com/2.0/metadata_cascade_policies/{metadata_cascade_policy_id} | ||
|
|
||
| Both scoped and namespaced cascade policy creation is allowed. | ||
| See the full spec https://developer.box.com/reference/get-metadata-cascade-policies-id |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix incorrect operation description in GET Cascade Policy (MIGRATION)
Line 2107 says "Both scoped and namespaced cascade policy creation is allowed" but this is the GET endpoint. Should be "retrieval" or similar.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@s.md` around lines 2103 - 2108, The GET Metadata Cascade Policy migration
description is using the wrong operation wording, referring to “creation” for a
retrieval endpoint. Update the text in the GET Cascade Policy section so it
describes fetching/retrieving the policy rather than creating it, keeping the
rest of the endpoint details and spec reference unchanged.
Comment on lines
+2147
to
+2152
| 3.3.3 FINAL | ||
| Full URL: | ||
| GET https://api.box.com/2.0/metadata_cascade_policies/{metadata_cascade_policy_id} | ||
|
|
||
| Both scoped and namespaced cascade policy creation is allowed. | ||
| See the full spec https://developer.box.com/reference/get-metadata-cascade-policies-id |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix incorrect operation description in GET Cascade Policy (FINAL)
Line 2151 says "Both scoped and namespaced cascade policy creation is allowed" but this is the GET endpoint. Should be "retrieval" or similar.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@s.md` around lines 2147 - 2152, The GET Cascade Policy FINAL description uses
the wrong action term, since this endpoint is for retrieval rather than
creation. Update the wording in the metadata cascade policy GET section so the
sentence about scoped and namespaced cascade policy behavior refers to retrieval
or fetching, and keep the rest of the endpoint description aligned with the GET
operation name and path.
Comment on lines
+2185
to
+2191
| 3.4.1 SCOPED | ||
| Full URL: | ||
| GET https://api.box.com/2.0/metadata_cascade_policies | ||
|
|
||
| Only scoped cascade policy creation is allowed. | ||
| See the full spec https://developer.box.com/reference/get-metadata-cascade-policies | ||
|
|
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix incorrect operation description in LIST Cascade Policy (SCOPED)
Line 2189 says "Only scoped cascade policy creation is allowed" but this is the LIST endpoint (3.4 LIST Cascade Policy). Should be "listing" or "retrieval".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@s.md` around lines 2185 - 2191, The operation description for the LIST
Cascade Policy SCOPED endpoint is incorrect because it says “creation” instead
of describing a list/retrieval action. Update the text in the metadata cascade
policy section so the wording matches the GET metadata_cascade_policies endpoint
and the surrounding “LIST Cascade Policy” context, using the relevant SCOPED
entry to locate it.
Comment on lines
+2229
to
+2235
| 3.4.2 MIGRATION | ||
| Full URL: | ||
| GET https://api.box.com/2.0/metadata_cascade_policies | ||
|
|
||
| Both scoped and namespaced cascade policy creation is allowed. | ||
| See the full spec https://developer.box.com/reference/get-metadata-cascade-policies | ||
|
|
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix incorrect operation description in LIST Cascade Policy (MIGRATION)
Line 2233 says "Both scoped and namespaced cascade policy creation is allowed" but this is the LIST endpoint. Should be "listing" or "retrieval".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@s.md` around lines 2229 - 2235, The LIST Cascade Policy migration text uses
the wrong operation wording in the endpoint description; update the copy in the
metadata cascade policies section so it says “listing” or “retrieval” instead of
“creation.” Locate the affected wording in the migration docs around the GET
metadata_cascade_policies entry and replace the misleading phrase while keeping
the rest of the spec reference unchanged.
Comment on lines
+2280
to
+2286
| 3.4.3 FINAL | ||
| Full URL: | ||
| GET https://api.box.com/2.0/metadata_cascade_policies | ||
|
|
||
| Both scoped and namespaced cascade policy creation is allowed. | ||
| See the full spec https://developer.box.com/reference/get-metadata-cascade-policies | ||
|
|
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix incorrect operation description in LIST Cascade Policy (FINAL)
Line 2284 says "Both scoped and namespaced cascade policy creation is allowed" but this is the LIST endpoint. Should be "listing" or "retrieval".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@s.md` around lines 2280 - 2286, The LIST Cascade Policy FINAL description is
using the wrong operation wording; update the text in the metadata cascade
policies documentation so it says listing/retrieval instead of creation. Locate
the endpoint description around the GET metadata_cascade_policies entry and
replace the misleading phrase in the final section while keeping the rest of the
spec reference intact.
JakubKida
force-pushed
the
metadata-namespace-migration-support
branch
from
June 26, 2026 09:48
c73a5f2 to
c4696a8
Compare
|
|
JakubKida
changed the title
JakubKida
force-pushed
the
metadata-namespace-migration-support
branch
from
July 14, 2026 07:15
c4696a8 to
62f27b5
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/api/Metadata.js (1)
126-128: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winSchema cache key still ignores
scope/namespace — cross-scope collisions unresolved.
getMetadataTemplateSchemaCacheKeykeys solely ontemplateKey, butgetMetadataTemplateSchemaUrl(Line 706) now varies the URL byscope/namespace. AtemplateKeyshared across an enterprise scope and a namespace FQN (exactly the MIGRATION-mode scenario this PR targets) will read/write the wrong cached schema after the first fetch.🐛 Proposed fix
- getMetadataTemplateSchemaCacheKey(templateKey: string): string { - return `${CACHE_PREFIX_METADATA}template_schema_${templateKey}`; + getMetadataTemplateSchemaCacheKey(templateKey: string, scope?: string = METADATA_SCOPE_ENTERPRISE): string { + return `${CACHE_PREFIX_METADATA}template_schema_${scope}_${templateKey}`; }- const key = this.getMetadataTemplateSchemaCacheKey(templateKey); + const key = this.getMetadataTemplateSchemaCacheKey(templateKey, scope);Also applies to: 694-710
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api/Metadata.js` around lines 126 - 128, Update getMetadataTemplateSchemaCacheKey to include the same scope/namespace discriminator used by getMetadataTemplateSchemaUrl, so identical templateKey values in different scopes produce distinct cache keys. Propagate the scope or namespace parameter through all callers, including the schema fetch flow around getMetadataTemplateSchemaUrl, while preserving existing key formatting for the default scope.
♻️ Duplicate comments (1)
src/api/Metadata.js (1)
843-857: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPrimary scope match still short-circuits the namespace fallback when
$scopeis absent.For namespace-only instances,
scopeisundefined. The primary matcht.scope === scopesucceeds for any namespace-only template with matchingtemplateKey(sincet.scopeis alsoundefined), regardless ofnamespace, so the fallback on Line 851-853 never runs when multiple namespace-only templates share atemplateKey. NotedeleteMetadata(Line 1735-1738) in this same diff already uses the correct gated pattern (scope ? ... : ...).🐛 Proposed fix
- let template = templates.find(t => t.templateKey === templateKey && t.scope === scope); + let template = scope + ? templates.find(t => t.templateKey === templateKey && t.scope === scope) + : undefined;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api/Metadata.js` around lines 843 - 857, Update the primary template lookup in the metadata resolution flow to match by scope only when scope is defined, using the gated pattern established by deleteMetadata. For namespace-only instances, skip the scope lookup so the namespace-based fallback selects the template matching namespace, while preserving scoped-instance behavior.
🧹 Nitpick comments (1)
src/elements/content-sidebar/MetadataTemplateDropdown.tsx (1)
72-82: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winProduction code parses a mock-specific ID encoding format.
This fallback decodes ids using the
"fqn||templateKey"delimiter format that, per the comment, only the mock API produces. Coupling production dropdown logic to a mock implementation detail is fragile — if a real API id ever coincidentally contains||, this would silently mis-parsenamespaceFqn/templateKey. Consider passing the browser-shape template object itself (or its resolved namespace/templateKey) through the event instead of re-deriving it from an encoded id string.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/elements/content-sidebar/MetadataTemplateDropdown.tsx` around lines 72 - 82, Remove the mock-specific "fqn||templateKey" parsing from the fallback in MetadataTemplateDropdown and pass the resolved browser-shape template data, including namespaceFqn and templateKey, through the edit event instead. Update the surrounding lookup/event flow so real API IDs are never interpreted as encoded mock IDs, while preserving editing for resolved and newly created templates.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/api/Metadata.js`:
- Around line 428-458: Update updateMetadataTemplate to evict the metadata
template schema cache after a successful update, using
getMetadataTemplateSchemaCacheKey(templateKey). Include the relevant scope in
the cache key generation so namespace and enterprise schemas for the same
templateKey cannot share cached entries.
- Around line 1032-1055: The getEnterpriseScopeFromInstances method must extract
the enterprise scope from dot-delimited namespace values as well as path-like
values. Update the $namespace parsing so names such as enterprise_123456.legal
return enterprise_123456, while preserving the existing $scope precedence and
null behavior.
- Around line 1111-1133: Update the non-SCOPED branch around
getEnterpriseScopeFromInstances so enterpriseScope is resolved from an
authoritative bootstrap source before template fetching, rather than relying
only on instances. Preserve the instances-derived value as a fallback if
appropriate, and use the resolved enterprise FQN when gating and calling
getTemplates for enterprise templates so files without metadata can still reach
template management.
In `@src/api/metadataNamespaceMocks.js`:
- Line 200: Update mockUpdateMetadataTemplate so its successCallback receives
the fully updated metadata template, including existing properties such as id,
displayName, fields, and the updated templateKey, instead of the current stub
object; preserve the existing success flow.
In `@src/elements/content-sidebar/hooks/useMetadataTemplateEditor.tsx`:
- Around line 2-8: Add the missing `@box/metadata-template-editor` package to
package.json dependencies so imports in
src/elements/content-sidebar/hooks/useMetadataTemplateEditor.tsx (lines 2-8) and
src/elements/content-sidebar/MetadataSidebarRedesign.tsx (lines 20-24) resolve
successfully; no direct source changes are needed at either import site.
In `@src/elements/content-sidebar/hooks/useSidebarMetadataFetcher.ts`:
- Around line 291-293: Update the template lookup in extractSuggestions to
compare the candidate template’s scope or namespace directly against the
function’s scope parameter, while retaining the templateKey match. Remove the
truthiness-only condition so templates from different scopes cannot be selected.
In `@src/elements/content-sidebar/MetadataSidebarRedesign.tsx`:
- Around line 176-179: Guard the template lookup in the enterprise ID
initialization so it safely handles an undefined templates value, matching the
existing checks in isViewMode and metadataDropdown. Update the expression using
templates.find(...) to fall back to an empty collection while preserving the
existing enterprise scope filtering and numeric ID extraction.
---
Outside diff comments:
In `@src/api/Metadata.js`:
- Around line 126-128: Update getMetadataTemplateSchemaCacheKey to include the
same scope/namespace discriminator used by getMetadataTemplateSchemaUrl, so
identical templateKey values in different scopes produce distinct cache keys.
Propagate the scope or namespace parameter through all callers, including the
schema fetch flow around getMetadataTemplateSchemaUrl, while preserving existing
key formatting for the default scope.
---
Duplicate comments:
In `@src/api/Metadata.js`:
- Around line 843-857: Update the primary template lookup in the metadata
resolution flow to match by scope only when scope is defined, using the gated
pattern established by deleteMetadata. For namespace-only instances, skip the
scope lookup so the namespace-based fallback selects the template matching
namespace, while preserving scoped-instance behavior.
---
Nitpick comments:
In `@src/elements/content-sidebar/MetadataTemplateDropdown.tsx`:
- Around line 72-82: Remove the mock-specific "fqn||templateKey" parsing from
the fallback in MetadataTemplateDropdown and pass the resolved browser-shape
template data, including namespaceFqn and templateKey, through the edit event
instead. Update the surrounding lookup/event flow so real API IDs are never
interpreted as encoded mock IDs, while preserving editing for resolved and newly
created templates.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8a483e3e-d013-45db-97e8-22bd0b44e780
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (16)
package.jsonsrc/api/Base.jssrc/api/Metadata.jssrc/api/metadataNamespaceMocks.jssrc/common/types/api.jssrc/common/types/metadata.jssrc/constants.jssrc/elements/content-sidebar/MetadataInstanceEditor.tsxsrc/elements/content-sidebar/MetadataSidebarRedesign.tsxsrc/elements/content-sidebar/MetadataTemplateDropdown.tsxsrc/elements/content-sidebar/__tests__/MetadataSidebarRedesign.test.tsxsrc/elements/content-sidebar/hooks/useMetadataNamespaceMode.tssrc/elements/content-sidebar/hooks/useMetadataTemplateEditor.tsxsrc/elements/content-sidebar/hooks/useMetadataTemplateEventService.tssrc/elements/content-sidebar/hooks/useMetadataTemplateItemsService.tssrc/elements/content-sidebar/hooks/useSidebarMetadataFetcher.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/common/types/metadata.js
Comment on lines
+428
to
+458
| async updateMetadataTemplate( | ||
| file: BoxItem, | ||
| namespaceFqn: string, | ||
| templateKey: string, | ||
| patchItems: Array<Object>, | ||
| successCallback: Function, | ||
| errorCallback: ElementsErrorCallback, | ||
| ): Promise<void> { | ||
| // TODO(MDX-2136): remove next two lines when namespace API is deployed | ||
| if (IS_NAMESPACE_API_MOCKED) { | ||
| mockUpdateMetadataTemplate(file, namespaceFqn, templateKey, patchItems, successCallback); | ||
| return; | ||
| } | ||
|
|
||
| const { id }: BoxItem = file; | ||
| this.errorCode = ERROR_CODE_UPDATE_METADATA_TEMPLATE; | ||
| const url = this.getMetadataTemplateSchemaUrl(templateKey, namespaceFqn); | ||
| try { | ||
| const response = await this.xhr.put({ | ||
| url, | ||
| id: getTypedFileId(id), | ||
| headers: { [HEADER_CONTENT_TYPE]: 'application/json-patch+json' }, | ||
| data: patchItems, | ||
| }); | ||
| if (!this.isDestroyed()) { | ||
| successCallback(getProp(response, 'data')); | ||
| } | ||
| } catch (e) { | ||
| errorCallback(e, this.errorCode); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map Metadata.js structure and locate cache-related symbols
ast-grep outline src/api/Metadata.js --view expanded || true
printf '\n--- Search for cache key and invalidation symbols ---\n'
rg -n "getMetadataTemplateSchemaCacheKey|getSchemaByTemplateKey|cache\.unset|cache\.unsetAll|updateMetadataTemplate|metadata template schema" src/api/Metadata.js
printf '\n--- Relevant surrounding lines ---\n'
nl -ba src/api/Metadata.js | sed -n '360,760p'Repository: box/box-ui-elements
Length of output: 814
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for all references to schema cache key generation and invalidation across the repo
rg -n "getMetadataTemplateSchemaCacheKey|getSchemaByTemplateKey|unsetAll\(|unset\(" srcRepository: box/box-ui-elements
Length of output: 3850
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1080,1225p' src/api/Metadata.js
printf '\n--- schema fetch ---\n'
sed -n '684,725p' src/api/Metadata.js
printf '\n--- updateMetadataTemplate ---\n'
sed -n '428,470p' src/api/Metadata.jsRepository: box/box-ui-elements
Length of output: 8860
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the exact methods around the cache.unset calls and the schema cache helpers
python3 - <<'PY'
from pathlib import Path
p = Path('src/api/Metadata.js')
lines = p.read_text().splitlines()
for start, end in [(1080, 1225), (684, 725), (1100, 1225), (100, 140)]:
print(f"\n--- lines {start}-{end} ---")
for i in range(start-1, min(end, len(lines))):
print(f"{i+1}: {lines[i]}")
PYRepository: box/box-ui-elements
Length of output: 15194
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('src/api/Metadata.js')
lines = p.read_text().splitlines()
for start, end in [(220, 280), (720, 820), (820, 920)]:
print(f"\n--- lines {start}-{end} ---")
for i in range(start-1, min(end, len(lines))):
print(f"{i+1}: {lines[i]}")
PY
printf '\n--- callers of getSchemaByTemplateKey ---\n'
rg -n "getSchemaByTemplateKey\(" srcRepository: box/box-ui-elements
Length of output: 12725
Clear the metadata template schema cache after updates
updateMetadataTemplate never evicts getMetadataTemplateSchemaCacheKey(templateKey), so a cached schema can stay stale for the rest of the session. The cache key also ignores scope, which can mix schemas for the same templateKey across namespace/enterprise variants.
🧰 Tools
🪛 Biome (2.5.3)
[error] 429-429: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
(parse)
[error] 430-430: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
(parse)
[error] 431-431: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
(parse)
[error] 432-432: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
(parse)
[error] 433-433: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
(parse)
[error] 434-434: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
(parse)
[error] 435-435: return type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
(parse)
[error] 442-442: type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
(parse)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/api/Metadata.js` around lines 428 - 458, Update updateMetadataTemplate to
evict the metadata template schema cache after a successful update, using
getMetadataTemplateSchemaCacheKey(templateKey). Include the relevant scope in
the cache key generation so namespace and enterprise schemas for the same
templateKey cannot share cached entries.
JakubKida
force-pushed
the
metadata-namespace-migration-support
branch
from
July 15, 2026 09:40
62f27b5 to
064bd17
Compare
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/api/Metadata.js`:
- Around line 1755-1766: Guard the findIndex results in both metadata cache
removal branches before calling splice: only remove an entry when the returned
index is non-negative. Apply this to the instance lookup and the editor lookup
in the surrounding metadata update logic, preserving the existing matching
predicates and preventing splice(-1, 1) from removing an unrelated last entry.
In `@src/api/metadataNamespaceMocks.js`:
- Line 16: Update IS_NAMESPACE_API_MOCKED in metadataNamespaceMocks.js to
default to the live backend instead of being hardcoded true. Gate the in-memory
namespace mocks behind the existing development or test configuration so list,
create, and update flows use mocks only in those environments.
In `@src/elements/content-sidebar/hooks/useMetadataNamespaceMode.ts`:
- Around line 47-70: Update the useEffect in useMetadataNamespaceMode to use a
local cancellation flag scoped to each effect invocation instead of the shared
cancelledRef, setting it in the cleanup and checking it before applying results.
Cast the Promise result from getMetadataNamespaceMode to MetadataScopeMode |
null before passing it to setMode.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 881464c5-9db4-4df6-be32-68bcde47a46d
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (16)
package.jsonsrc/api/Base.jssrc/api/Metadata.jssrc/api/metadataNamespaceMocks.jssrc/common/types/api.jssrc/common/types/metadata.jssrc/constants.jssrc/elements/content-sidebar/MetadataInstanceEditor.tsxsrc/elements/content-sidebar/MetadataSidebarRedesign.tsxsrc/elements/content-sidebar/MetadataTemplateDropdown.tsxsrc/elements/content-sidebar/__tests__/MetadataSidebarRedesign.test.tsxsrc/elements/content-sidebar/hooks/useMetadataNamespaceMode.tssrc/elements/content-sidebar/hooks/useMetadataTemplateEditor.tsxsrc/elements/content-sidebar/hooks/useMetadataTemplateEventService.tssrc/elements/content-sidebar/hooks/useMetadataTemplateItemsService.tssrc/elements/content-sidebar/hooks/useSidebarMetadataFetcher.ts
🚧 Files skipped from review as they are similar to previous changes (12)
- src/common/types/api.js
- src/common/types/metadata.js
- src/elements/content-sidebar/MetadataInstanceEditor.tsx
- package.json
- src/elements/content-sidebar/hooks/useMetadataTemplateEventService.ts
- src/elements/content-sidebar/hooks/useSidebarMetadataFetcher.ts
- src/elements/content-sidebar/hooks/useMetadataTemplateEditor.tsx
- src/elements/content-sidebar/hooks/useMetadataTemplateItemsService.ts
- src/elements/content-sidebar/MetadataTemplateDropdown.tsx
- src/elements/content-sidebar/tests/MetadataSidebarRedesign.test.tsx
- src/constants.js
- src/elements/content-sidebar/MetadataSidebarRedesign.tsx
Comment on lines
+1755
to
+1766
| instance => | ||
| instance.templateKey === templateKey && | ||
| (scope ? instance.scope === scope : instance.namespace === namespace), | ||
| ), | ||
| 1, | ||
| ); | ||
| } else { | ||
| metadata.editors.splice( | ||
| metadata.editors.findIndex( | ||
| editor => editor.template.scope === scope && editor.template.templateKey === templateKey, | ||
| editor => | ||
| editor.template.templateKey === templateKey && | ||
| (scope ? editor.template.scope === scope : editor.template.namespace === namespace), |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Guard findIndex results before splicing the cache.
When no matching instance or editor exists, findIndex returns -1, causing splice(-1, 1) to remove the last unrelated entry.
Proposed fix
- metadata.instances.splice(metadata.instances.findIndex(instance => /* match */), 1);
+ const instanceIndex = metadata.instances.findIndex(instance => /* match */);
+ if (instanceIndex >= 0) {
+ metadata.instances.splice(instanceIndex, 1);
+ }
- metadata.editors.splice(metadata.editors.findIndex(editor => /* match */), 1);
+ const editorIndex = metadata.editors.findIndex(editor => /* match */);
+ if (editorIndex >= 0) {
+ metadata.editors.splice(editorIndex, 1);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| instance => | |
| instance.templateKey === templateKey && | |
| (scope ? instance.scope === scope : instance.namespace === namespace), | |
| ), | |
| 1, | |
| ); | |
| } else { | |
| metadata.editors.splice( | |
| metadata.editors.findIndex( | |
| editor => editor.template.scope === scope && editor.template.templateKey === templateKey, | |
| editor => | |
| editor.template.templateKey === templateKey && | |
| (scope ? editor.template.scope === scope : editor.template.namespace === namespace), | |
| const instanceIndex = metadata.instances.findIndex( | |
| instance => | |
| instance.templateKey === templateKey && | |
| (scope ? instance.scope === scope : instance.namespace === namespace), | |
| ); | |
| if (instanceIndex >= 0) { | |
| metadata.instances.splice(instanceIndex, 1); | |
| } | |
| } else { | |
| const editorIndex = metadata.editors.findIndex( | |
| editor => | |
| editor.template.templateKey === templateKey && | |
| (scope ? editor.template.scope === scope : editor.template.namespace === namespace), | |
| ); | |
| if (editorIndex >= 0) { | |
| metadata.editors.splice(editorIndex, 1); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/api/Metadata.js` around lines 1755 - 1766, Guard the findIndex results in
both metadata cache removal branches before calling splice: only remove an entry
when the returned index is non-negative. Apply this to the instance lookup and
the editor lookup in the surrounding metadata update logic, preserving the
existing matching predicates and preventing splice(-1, 1) from removing an
unrelated last entry.
JakubKida
force-pushed
the
metadata-namespace-migration-support
branch
from
July 16, 2026 12:33
4145a1c to
7b3fcd8
Compare
JakubKida
force-pushed
the
metadata-namespace-migration-support
branch
3 times, most recently
from
July 24, 2026 14:08
d42825c to
f9ee5bb
Compare
JakubKida
force-pushed
the
metadata-namespace-migration-support
branch
from
August 3, 2026 13:09
d435ca2 to
fd99409
Compare
jpan-box
reviewed
Aug 3, 2026
jpan-box
reviewed
Aug 3, 2026
jpan-box
reviewed
Aug 3, 2026
JakubKida
force-pushed
the
metadata-namespace-migration-support
branch
from
August 4, 2026 09:45
9a47365 to
47c2478
Compare
jpan-box
reviewed
Aug 4, 2026
| * The modal JSX to render somewhere stable in the tree (e.g. beside | ||
| * `SidebarContent`). `null` when the editor is closed. | ||
| */ | ||
| modal: React.ReactNode; |
Contributor
There was a problem hiding this comment.
hmmm i guess there is nothing technically wrong with having a hook return JSX, i'm just not used to seeing it. i would more expect there to be a standalone component to contain the JSX, and then that component uses this hook to manage the data.
i'm fine with doing things your way unless this piques anyone else's interest!
jpan-box
reviewed
Aug 4, 2026
| @@ -342,6 +351,7 @@ export const ERROR_CODE_UPDATE_TASK_COLLABORATOR = 'update_task_collaborator_err | |||
| export const ERROR_CODE_UPDATE_COMMENT = 'update_comment_error'; | |||
| export const ERROR_CODE_UPDATE_SKILLS = 'update_skills_error'; | |||
| export const ERROR_CODE_UPDATE_METADATA = 'update_metadata_error'; | |||
Contributor
There was a problem hiding this comment.
just curious since this could be confusing - is update_metadata_error for updating metadata instances? or some other metadata issue other than template?
jpan-box
reviewed
Aug 4, 2026
| "@box/content-field": "^1.40.23", | ||
| "@box/copy-input": "^1.42.16", | ||
| "@box/item-icon": "^3.2.0", | ||
| "@box/metadata-editor": "^2.2.11", |
Contributor
There was a problem hiding this comment.
ope this one does not match either https://github.com/box/box-ui-elements/pull/4653/changes#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519R145 quick update pls?
jpan-box
reviewed
Aug 4, 2026
| "@chromatic-com/storybook": "^5.2.1", | ||
| "@commitlint/cli": "^19.8.0", | ||
| "@commitlint/config-conventional": "^19.8.0", | ||
| "@dnd-kit/core": "^6.1.0", |
Contributor
There was a problem hiding this comment.
just calling out, other box-internal parent apps may need to add these as new deps which they may not be happy about. just something to look out for when developing shared features -> buie. i assume you're working on this same task though so it may be you doing that work
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR adds metadata namespace migration support to the redesigned Content Sidebar metadata experience.
Box metadata is moving from legacy scoped templates (
enterprise/enterprise_<id>) toward namespaced templates (hierarchical FQNs under an enterprise root). Enterprises can be in one of three modes:Without this work, the redesigned sidebar assumes SCOPED semantics: it fetches enterprise templates with the
enterpriseshorthand, matches instances only by$scope, and cannot browse or manage templates in a namespace tree. That breaks (or silently mis-handles) files in MIGRATION/FINAL.This PR wires the sidebar and Metadata API so that, when the namespaces opt-in feature is enabled, we:
When the opt-in flag is off, behavior stays on the legacy path.
Screenshots/Videos
N/A for API/hook wiring. For UI review, open a file in an enterprise in MIGRATION or FINAL with
metadata.namespacesOptIn.enabledon and confirm:Related issues
N/A (public repo)
Changes made
API layer
Metadata.js— mode-awaregetMetadatapaths:enterprisetemplates (unchanged intent)enterpriseshorthand where namespaces are requiredMetadataNamespaces.js— dedicated surface for namespace HTTP:Metadataso callers keep usingapi.getMetadataAPI()metadataNamespaceUtils.js— pure helpers for enterprise-root resolution, mode resolution, scope-vs-namespace selection, and “externally owned” checksmetadataNamespaceMocks.js— temporary mocks for namespace endpoints until the live API is fully available in all environmentsBase.js/ types / constants — shared header/constants/types for namespace mode fields, error codes, andbox-versionwhere neededSidebar hooks
useCurrentUserEnterpriseId—GET /users/me?fields=enterprise→ numeric id +enterprise_<id>FQNuseMetadataNamespaceMode— enterprise configurations →SCOPED|MIGRATION|FINALuseMetadataNamespaceContext— composition hook: feature flag + the two hooks above +isTemplateManagementEnableduseSidebarMetadataFetcher— accepts namespace context (enterpriseFqn,metadataNamespaceMode) and forwards it intogetMetadata; can refetch when authoritative mode/FQN becomes available after the initial loaduseMetadataTemplateItemsService—ItemsServicefor the template browser (namespaces / templates / client-side search)useMetadataTemplateEditor/useMetadataTemplateEventService— create/edit modal + dropdown event wiringUI
MetadataSidebarRedesign— usesuseMetadataNamespaceContext, passes context into the fetcher, and gates template-management UI on non-SCOPED modeMetadataTemplateDropdown— chooses legacy flat dropdown vs browser-backed dropdown; wires create/edit into the template editorDependencies
@box/metadata-editor@box/metadata-template-editor(and related peer deps used by the editor/browser)Tests
Type of change
Testing done
Notable local suites:
nvm use yarn test \ src/api/__tests__/Metadata.test.js \ src/api/__tests__/metadataNamespaceUtils.test.js \ src/elements/content-sidebar/__tests__/useMetadataNamespaceContext.test.tsx \ src/elements/content-sidebar/__tests__/useMetadataNamespaceMode.test.ts \ src/elements/content-sidebar/__tests__/useCurrentUserEnterpriseId.test.ts \ src/elements/content-sidebar/__tests__/useMetadataTemplateItemsService.test.tsx \ src/elements/content-sidebar/__tests__/useMetadataTemplateEditor.test.tsx \ src/elements/content-sidebar/__tests__/useSidebarMetadataFetcher.test.tsx \ src/elements/content-sidebar/__tests__/MetadataSidebarRedesign.test.tsx \ --no-coverageDependencies
@box/metadata-editorupgraded@box/metadata-template-editoraddedDeployment notes
metadata.namespacesOptIn.enabled. With the flag off, the sidebar stays on the legacy SCOPED path (no enterprise-config / namespace list calls from this wiring).How to test
How to review
Suggested order:
metadataNamespaceUtils.js— pure rules for mode / FQN / matchingMetadataNamespaces.js+Metadata.jsgetMetadata paths — SCOPED vs namespaced fetch and instance→template resolutionuseMetadataNamespaceContext→ fetcher options → template items/editor servicesMetadataSidebarRedesign+MetadataTemplateDropdown— feature gating and UI wiringPay attention to:
enterpriseshorthand in MIGRATION/FINAL template fetches$namespace(no$scope) during migrationSelf-review checklist
Running package.json scripts
package.json, please executenvm use.Additional notes
fileIdonly, in parallel with the file GET used for permissions — sogetMetadatacan receive an authoritative enterprise root in MIGRATION/FINAL without waiting on the full file payload.