Skip to content

fix: support mid-migration metadata templates #4653

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.

Sign up for GitHub

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

Open
JakubKida wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: support mid-migration metadata templates #4653

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 32 additions & 12 deletions src/api/Metadata.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,13 +193,20 @@ class Metadata extends File {
}

/**
* API URL for getting metadata template schema by template key
* API URL for getting metadata template schema by template key.
*
* In SCOPED mode the path segment is the `enterprise` shorthand.
* In MIGRATION/FINAL mode the API requires the full scope value (e.g.
* `enterprise_123456`) or a namespace FQN, so callers should pass the
* resolved scope/namespace when operating in those modes.
*
* @param {string} templateKey - metadata template key
* @param {string} [scope] - scope or namespace FQN; defaults to the
* `enterprise` shorthand for backward compatibility with SCOPED mode
* @return {string} API url for getting template schema by template key
*/
getMetadataTemplateSchemaUrl(templateKey: string): string {
return `${this.getMetadataTemplateUrl()}/enterprise/${templateKey}/schema`;
getMetadataTemplateSchemaUrl(templateKey: string, scope?: string = METADATA_SCOPE_ENTERPRISE): string {
return `${this.getMetadataTemplateUrl()}/${scope}/${templateKey}/schema`;
}

/**
Expand Down Expand Up @@ -403,25 +410,27 @@ class Metadata extends File {
}

/**
* Gets metadata template schema by template key
* Gets metadata template schema by template key.
*
* In MIGRATION/FINAL mode, pass the full scope (e.g. `enterprise_123456`)
* or namespace FQN as `scope` so the correct URL path segment is used.
* Omitting `scope` falls back to the `enterprise` shorthand (SCOPED mode).
*
* @param {string} templateKey - template key
* @param {string} [scope] - scope or namespace FQN (defaults to `enterprise`)
* @return {Promise} Promise object of metadata template
*/
async getSchemaByTemplateKey(templateKey: string): Promise<MetadataTemplateSchemaResponse> {
async getSchemaByTemplateKey(templateKey: string, scope?: string): Promise<MetadataTemplateSchemaResponse> {
const cache: APICache = this.getCache();
const key = this.getMetadataTemplateSchemaCacheKey(templateKey);

// Return cached value if it exists
if (cache.has(key)) {
return cache.get(key);
}

// Fetch from API if not cached
const url = this.getMetadataTemplateSchemaUrl(templateKey);
const url = this.getMetadataTemplateSchemaUrl(templateKey, scope);
Comment on lines +423 to +431

@coderabbitai coderabbitai Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Cache key must include scope to avoid cross-scope collisions.

getMetadataTemplateSchemaCacheKey keys solely on templateKey, but the schema URL now varies by scope. After the first fetch for a given templateKey, a subsequent call with a different scope (e.g. enterprise_123456 vs a namespace FQN) returns the wrong cached schema and never hits the scope-specific URL built on Line 431. This is exactly the MIGRATION/FINAL multi-scope scenario this PR targets.

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);
🧰 Tools
🪛 Biome (2.5.0)

[error] 423-423: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.

(parse)

[error] 423-423: optional parameters are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.

(parse)

[error] 423-423: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.

(parse)

[error] 423-423: return type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.

(parse)

[error] 424-424: 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 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.

const response = await this.xhr.get({ url });

// Cache the response
cache.set(key, response);

return response;
Expand Down Expand Up @@ -553,10 +562,21 @@ class Metadata extends File {
const instanceId = instance.$id;
const templateKey = instance.$template;
const scope = instance.$scope;
const template = templates.find(t => t.templateKey === templateKey && t.scope === scope);
const namespace = instance.$namespace;

// Primary match: by scope (SCOPED mode; also works for enterprise-scoped
// instances in MIGRATION mode where $scope is still populated).
let template = templates.find(t => t.templateKey === templateKey && t.scope === scope);

// Fallback match: by namespace for namespace-only instances in
// MIGRATION/FINAL mode where $scope is absent.
if (!template && namespace) {
template = templates.find(t => t.templateKey === templateKey && t.namespace === namespace);
}
Comment on lines +565 to +575

@coderabbitai coderabbitai Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm how `scope` is derived in getTemplateForInstance (definition not shown)
rg -nP --type=js -C3 'getTemplateForInstance' src/api/Metadata.js

Repository: box/box-ui-elements

Length of output: 1191

🏁 Script executed:

sed -n '557,600p' src/api/Metadata.js

Repository: box/box-ui-elements

Length of output: 2389

Primary scope match can short-circuit the namespace fallback when $scope is absent.

For namespace-only instances, scope is undefined. The primary match t.scope === scope succeeds for any template with undefined scope, selecting purely by templateKey and ignoring namespace. When multiple namespace-only templates share a templateKey across different namespaces, the wrong template is chosen and the namespace fallback never executes.

Gate the primary match on a defined scope to ensure the namespace fallback runs correctly:

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;
📝 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
const namespace = instance.$namespace;
// Primary match: by scope (SCOPED mode; also works for enterprise-scoped
// instances in MIGRATION mode where $scope is still populated).
let template = templates.find(t => t.templateKey === templateKey && t.scope === scope);
// Fallback match: by namespace for namespace-only instances in
// MIGRATION/FINAL mode where $scope is absent.
if (!template && namespace) {
template = templates.find(t => t.templateKey === templateKey && t.namespace === namespace);
}
const namespace = instance.$namespace;
// Primary match: by scope (SCOPED mode; also works for enterprise-scoped
// instances in MIGRATION mode where $scope is still populated).
let template = scope
? templates.find(t => t.templateKey === templateKey && t.scope === scope)
: undefined;
// Fallback match: by namespace for namespace-only instances in
// MIGRATION/FINAL mode where $scope is absent.
if (!template && namespace) {
template = templates.find(t => t.templateKey === templateKey && t.namespace === namespace);
}
🤖 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 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.


// Enterprise scopes are always enterprise_XXXXX
if (!template && scope.startsWith(METADATA_SCOPE_ENTERPRISE)) {
// Enterprise scopes are always enterprise_XXXXX; use optional chaining
// to guard against namespace-only instances where $scope is undefined.
if (!template && scope?.startsWith(METADATA_SCOPE_ENTERPRISE)) {
// Any missing template is likely from another enterprise (e.g. collaborated file);
// Templates array has no pagination so we can assume cross-enterprise as it contains all templates.
const crossEnterpriseTemplates = await this.getTemplates(id, scope, instanceId, true);
Expand Down
6 changes: 4 additions & 2 deletions src/common/types/metadata.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ type MetadataTemplate = {
hidden?: boolean,
id: string,
isHidden?: boolean,
scope: string, // V2
namespace?: string, // MIGRATION/FINAL modes
scope: string, // V2; absent for namespace-only templates in FINAL mode
templateKey: string, // V3
};

Expand Down Expand Up @@ -115,8 +116,9 @@ type MetadataInstance = {
type MetadataInstanceV2 = {
$canEdit: boolean,
$id: string,
$namespace?: string, // MIGRATION/FINAL modes
$parent: string,
$scope: string,
$scope?: string, // absent for namespace-only instances in MIGRATION/FINAL modes
$template: string,
$type: string,
$typeVersion: number,
Expand Down
Loading