From f45f49169fd4188a6c91cf2fc8b3ac5a8d33b266 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Wed, 10 Dec 2025 15:23:09 +0000 Subject: [PATCH 01/32] [WIP] Submitting new properties during upgrade --- api_app/_version.py | 2 +- api_app/db/repositories/resources.py | 23 +- ui/app/package-lock.json | 4 +- ui/app/package.json | 2 +- .../shared/ConfirmUpgradeResource.tsx | 229 +++++++++++++++++- 5 files changed, 246 insertions(+), 14 deletions(-) diff --git a/api_app/_version.py b/api_app/_version.py index 4e32d0c791..a5e1b0f66d 100644 --- a/api_app/_version.py +++ b/api_app/_version.py @@ -1 +1 @@ -__version__ = "0.25.4" +__version__ = "0.25.5" diff --git a/api_app/db/repositories/resources.py b/api_app/db/repositories/resources.py index efa8af841e..3bdfc307fa 100644 --- a/api_app/db/repositories/resources.py +++ b/api_app/db/repositories/resources.py @@ -126,7 +126,7 @@ async def patch_resource(self, resource: Resource, resource_patch: ResourcePatch resource.templateVersion = resource_patch.templateVersion if resource_patch.properties is not None and len(resource_patch.properties) > 0: - self.validate_patch(resource_patch, resource_template_repo, resource_template, resource_action) + await self.validate_patch(resource_patch, resource_template_repo, resource_template, resource_action) # if we're here then we're valid - update the props + persist resource.properties.update(resource_patch.properties) @@ -183,16 +183,31 @@ async def validate_template_version_patch(self, resource: Resource, resource_pat except EntityDoesNotExist: raise TargetTemplateVersionDoesNotExist(f"Template '{resource_template.name}' not found for resource type '{resource_template.resourceType}' with target template version '{resource_patch.templateVersion}'") - def validate_patch(self, resource_patch: ResourcePatch, resource_template_repo: ResourceTemplateRepository, resource_template: ResourceTemplate, resource_action: str): + async def validate_patch(self, resource_patch: ResourcePatch, resource_template_repo: ResourceTemplateRepository, resource_template: ResourceTemplate, resource_action: str): # get the enriched (combined) template enriched_template = resource_template_repo.enrich_template(resource_template, is_update=True) - # validate the PATCH data against a cut down version of the full template. + # get the schema for the target version if upgrade is happening + if resource_patch.templateVersion is not None: + # fetch the template for the target version + target_template = await resource_template_repo.get_template_by_name_and_version(resource_template.name, resource_patch.templateVersion, resource_template.resourceType) + enriched_template = resource_template_repo.enrich_template(target_template, is_update=True) + + # # validate the PATCH data against the target schema. update_template = copy.deepcopy(enriched_template) update_template["required"] = [] update_template["properties"] = {} for prop_name, prop in enriched_template["properties"].items(): - if (resource_action == RESOURCE_ACTION_INSTALL or prop.get("updateable", False) is True): + # Allow property if installing, explicitly updateable, or when upgrading and the new property is being added in this patch + if ( + resource_action == RESOURCE_ACTION_INSTALL + or prop.get("updateable", False) is True + or ( + resource_patch.templateVersion is not None + and resource_patch.properties is not None + and prop_name in resource_patch.properties + ) + ): update_template["properties"][prop_name] = prop self._validate_resource_parameters(resource_patch.dict(), update_template) diff --git a/ui/app/package-lock.json b/ui/app/package-lock.json index b0fbb60cf3..8c4aafd216 100644 --- a/ui/app/package-lock.json +++ b/ui/app/package-lock.json @@ -1,12 +1,12 @@ { "name": "tre-ui", - "version": "0.8.19", + "version": "0.8.20", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "tre-ui", - "version": "0.8.19", + "version": "0.8.20", "dependencies": { "@azure/msal-browser": "^2.35.0", "@azure/msal-react": "^1.5.12", diff --git a/ui/app/package.json b/ui/app/package.json index 345c0297ef..3400a0c5f3 100644 --- a/ui/app/package.json +++ b/ui/app/package.json @@ -1,6 +1,6 @@ { "name": "tre-ui", - "version": "0.8.19", + "version": "0.8.20", "private": true, "type": "module", "dependencies": { diff --git a/ui/app/src/components/shared/ConfirmUpgradeResource.tsx b/ui/app/src/components/shared/ConfirmUpgradeResource.tsx index 8967b08e01..3742928ad4 100644 --- a/ui/app/src/components/shared/ConfirmUpgradeResource.tsx +++ b/ui/app/src/components/shared/ConfirmUpgradeResource.tsx @@ -8,8 +8,9 @@ import { MessageBar, MessageBarType, Icon, + Stack, } from "@fluentui/react"; -import React, { useContext, useState } from "react"; +import React, { useContext, useState, useEffect } from "react"; import { AvailableUpgrade, Resource } from "../../models/resource"; import { HttpMethod, @@ -23,24 +24,102 @@ import { LoadingState } from "../../models/loadingState"; import { ExceptionLayout } from "./ExceptionLayout"; import { useAppDispatch } from "../../hooks/customReduxHooks"; import { addUpdateOperation } from "../shared/notifications/operationsSlice"; +import Form from "@rjsf/fluent-ui"; +import validator from "@rjsf/validator-ajv8"; interface ConfirmUpgradeProps { resource: Resource; onDismiss: () => void; } +// Utility to get all property keys from template schema's properties object recursively, flattening nested if needed +const getAllPropertyKeys = (properties: any, prefix = ""): string[] => { + if (!properties) return []; + let keys: string[] = []; + for (const [key, value] of Object.entries(properties)) { + if (value && typeof value === "object" && 'properties' in value) { + // recur for nested properties + keys = keys.concat(getAllPropertyKeys(value["properties"], prefix + key + ".")); + } else { + keys.push(prefix + key); + } + } + return keys; +}; + +// Utility to build a reduced schema with only given keys and their nested schema (depth 1), including required +const buildReducedSchema = (fullSchema: any, keys: string[]): any => { + if (!fullSchema || !fullSchema.properties) return null; + const reducedProperties: any = {}; + const required: string[] = []; + + keys.forEach((key) => { + // Only allow top-level property keys (no nested with dots) for simplicity here + const topKey = key.split('.')[0]; + if (fullSchema.properties[topKey]) { + if (!reducedProperties[topKey]) { + reducedProperties[topKey] = fullSchema.properties[topKey]; + if (fullSchema.required && fullSchema.required.includes(topKey)) { + required.push(topKey); + } + } + } + }); + + return { + type: "object", + properties: reducedProperties, + required: required.length > 0 ? required : undefined, + }; +}; + +// Utility to collect direct property keys referenced inside an 'if' schema (simple/general) +const collectIfKeys = (ifSchema: any): string[] => { + const keys: string[] = []; + if (!ifSchema) return keys; + // handle simple 'properties' usage + if (ifSchema.properties) { + keys.push(...Object.keys(ifSchema.properties)); + } + // handle consts inside properties: { properties: { foo: { const: ... } } } + // other complex constructs are ignored for simplicity + return keys; +}; + +// Extract conditional blocks where the 'if' references any of the new keys. Include their then/else blocks. +const extractConditionalBlocks = (schema: any, newKeys: string[]) => { + const conditionalEntries: any[] = []; + if (!schema) return { allOf: [] }; + const allOf = schema.allOf || []; + allOf.forEach((entry: any) => { + if (entry && entry.if) { + const ifKeys = collectIfKeys(entry.if); + // include entry if any ifKey matches a new key (top-level match) + if (ifKeys.some((k) => newKeys.some((nk) => nk.split('.')[0] === k))) { + conditionalEntries.push(entry); + } + } + }); + return { allOf: conditionalEntries }; +}; + export const ConfirmUpgradeResource: React.FunctionComponent< ConfirmUpgradeProps > = (props: ConfirmUpgradeProps) => { const apiCall = useAuthApiCall(); const [selectedVersion, setSelectedVersion] = useState(""); - const [apiError, setApiError] = useState({} as APIError); + const [apiError, setApiError] = useState(null); const [requestLoadingState, setRequestLoadingState] = useState( LoadingState.Ok, ); const workspaceCtx = useContext(WorkspaceContext); const dispatch = useAppDispatch(); + const [newPropertiesToFill, setNewPropertiesToFill] = useState([]); + const [newPropertyValues, setNewPropertyValues] = useState({}); + const [loadingSchema, setLoadingSchema] = useState(false); + const [newTemplateSchema, setNewTemplateSchema] = useState(null); + const upgradeProps = { type: DialogType.normal, title: `Upgrade Template Version?`, @@ -60,10 +139,94 @@ export const ConfirmUpgradeResource: React.FunctionComponent< props.resource.resourceType === ResourceType.WorkspaceService || props.resource.resourceType === ResourceType.UserResource; + // Fetch new template schema and identify new properties missing in current resource + useEffect(() => { + if (!selectedVersion) { + setNewPropertiesToFill([]); + setNewPropertyValues({}); + setNewTemplateSchema(null); + return; + } + + const fetchNewTemplateSchema = async () => { + setLoadingSchema(true); + setApiError(null); + try { + let fetchUrl = ""; + if (props.resource.resourceType === ResourceType.Workspace) { + fetchUrl = `/workspace-templates/${props.resource.templateName}?version=${selectedVersion}`; + } else { + const templateType = props.resource.resourceType.toLowerCase(); + fetchUrl = `/templates/${templateType}/${selectedVersion}`; + } + + const res = await apiCall( + fetchUrl, + HttpMethod.Get, + wsAuth ? workspaceCtx.workspaceApplicationIdURI : undefined, + undefined, + ResultType.JSON, + ); + + // Use full fetched schema from API + setNewTemplateSchema(res); + + const newSchemaProps = res?.properties || {}; + const currentProps = props.resource.properties || {}; + + const newKeys = getAllPropertyKeys(newSchemaProps); + const currentKeys = getAllPropertyKeys(currentProps); + + const newPropKeys = newKeys.filter((k) => !currentKeys.includes(k)); + + setNewPropertiesToFill(newPropKeys); + + // prefill newPropertyValues with schema defaults or empty string + setNewPropertyValues( + newPropKeys.reduce((acc, key) => { + // Get top-level portion of the key + const topKey = key.split('.')[0]; + const defaultValue = res?.properties?.[topKey]?.default; + acc[key] = defaultValue !== undefined ? defaultValue : ''; + return acc; + }, {} as any), + ); + } catch (err: any) { + if (!err.userMessage) { + err.userMessage = "Failed to fetch new template schema"; + } + setApiError(err); + } finally { + setLoadingSchema(false); + } + }; + + fetchNewTemplateSchema(); + }, [selectedVersion]); + const upgradeCall = async () => { setRequestLoadingState(LoadingState.Loading); try { - let body = { templateVersion: selectedVersion }; + let body: any = { templateVersion: selectedVersion }; + + // Patch only the new properties inside the 'properties' field + if (!body.properties) { + body.properties = {}; + } + console.log("New property values to set:", newPropertyValues); + Object.keys(newPropertyValues).forEach((key) => { + const val = newPropertyValues[key]; + body.properties[key] = val; + }); + console.log("Final upgrade body:", body); + + // Include other optional properties if applicable + // Object.keys(newPropertyValues).forEach((key) => { + // if (newPropertiesToFill.includes(key)) { + // body.properties[key] = newPropertyValues[key]; + // } + // }); + let op = await apiCall( props.resource.resourcePath, HttpMethod.Patch, @@ -77,12 +240,38 @@ export const ConfirmUpgradeResource: React.FunctionComponent< dispatch(addUpdateOperation(op.operation)); props.onDismiss(); } catch (err: any) { - err.userMessage = "Failed to upgrade resource"; + if (!err.userMessage) { + err.userMessage = "Failed to upgrade resource"; + } setApiError(err); setRequestLoadingState(LoadingState.Error); } }; + // Use buildReducedSchema to include only new properties + const reducedSchemaProperties = newTemplateSchema + ? buildReducedSchema(newTemplateSchema, newPropertiesToFill) + : null; + + // Extract any conditional blocks from full schema, filtered by new properties + const conditionalBlocks = newTemplateSchema ? extractConditionalBlocks(newTemplateSchema, newPropertiesToFill) : {}; + + // Compose final schema combining reduced properties with conditional blocks + const finalSchema = reducedSchemaProperties + ? { ...reducedSchemaProperties, ...conditionalBlocks } + : null; + + // UI schema override: hide the form's submit button because we use external Upgrade button + // start with existing UI order and classNames from full schema uiSchema + const baseUiSchema = newTemplateSchema?.uiSchema || {}; + + // Compose final uiSchema merging baseUiSchema with our overrides + const uiSchema = { + ...baseUiSchema, + "ui:submitButtonOptions": { norender: true }, + // overview: { "ui:widget": "textarea" }, + }; + const onRenderOption = (option: any): JSX.Element => { return (
@@ -129,6 +318,28 @@ export const ConfirmUpgradeResource: React.FunctionComponent< Upgrading the template version is irreversible. + + {loadingSchema && } + + {!loadingSchema && newPropertiesToFill.length > 0 && ( + + + You must specify values for new properties: + + + {finalSchema && ( +
setNewPropertyValues(e.formData)} + /> + )} + + )} + 0 && + Object.values(newPropertyValues).some( + (v) => v === "" || v === undefined, + )) + } text="Upgrade" onClick={() => upgradeCall()} /> @@ -156,7 +373,7 @@ export const ConfirmUpgradeResource: React.FunctionComponent< /> )} {requestLoadingState === LoadingState.Error && ( - + )} From 52de907c92f3147503dd1c9e81fc210b22612a18 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Wed, 10 Dec 2025 15:39:21 +0000 Subject: [PATCH 02/32] fix(ui): Handle conditional properties in upgrade form This commit aligns the resource upgrade process with the update process by correctly handling conditional properties in the JSON schema. - The schema generation logic in `ConfirmUpgradeResource.tsx` is updated to include conditional blocks (`if`/`then`/`else`) when the condition is based on an existing property. - New read-only properties are now submitted during the upgrade process. --- .../shared/ConfirmUpgradeResource.tsx | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/ui/app/src/components/shared/ConfirmUpgradeResource.tsx b/ui/app/src/components/shared/ConfirmUpgradeResource.tsx index 3742928ad4..213a49a4c7 100644 --- a/ui/app/src/components/shared/ConfirmUpgradeResource.tsx +++ b/ui/app/src/components/shared/ConfirmUpgradeResource.tsx @@ -73,29 +73,31 @@ const buildReducedSchema = (fullSchema: any, keys: string[]): any => { }; }; -// Utility to collect direct property keys referenced inside an 'if' schema (simple/general) -const collectIfKeys = (ifSchema: any): string[] => { +// Utility to collect direct property keys referenced inside conditional schemas +const collectConditionalKeys = (entry: any): string[] => { const keys: string[] = []; - if (!ifSchema) return keys; - // handle simple 'properties' usage - if (ifSchema.properties) { - keys.push(...Object.keys(ifSchema.properties)); - } - // handle consts inside properties: { properties: { foo: { const: ... } } } - // other complex constructs are ignored for simplicity - return keys; + if (!entry) return keys; + const collect = (schemaPart: any) => { + if (schemaPart && schemaPart.properties) { + keys.push(...Object.keys(schemaPart.properties)); + } + }; + collect(entry.if); + collect(entry.then); + collect(entry.else); + return [...new Set(keys)]; }; -// Extract conditional blocks where the 'if' references any of the new keys. Include their then/else blocks. +// Extract conditional blocks that reference any of the new keys. const extractConditionalBlocks = (schema: any, newKeys: string[]) => { const conditionalEntries: any[] = []; if (!schema) return { allOf: [] }; const allOf = schema.allOf || []; allOf.forEach((entry: any) => { if (entry && entry.if) { - const ifKeys = collectIfKeys(entry.if); - // include entry if any ifKey matches a new key (top-level match) - if (ifKeys.some((k) => newKeys.some((nk) => nk.split('.')[0] === k))) { + const conditionalKeys = collectConditionalKeys(entry); + // include entry if any conditionalKey matches a new key (top-level match) + if (conditionalKeys.some((k) => newKeys.some((nk) => nk.split('.')[0] === k))) { conditionalEntries.push(entry); } } From 25ded790502613ff2d65c095d7a5b7a3d5aa5d0b Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Wed, 10 Dec 2025 15:52:24 +0000 Subject: [PATCH 03/32] fix(ui): Handle conditional properties in upgrade form This commit aligns the resource upgrade process with the update process by correctly handling conditional properties in the JSON schema. - The schema generation logic in `ConfirmUpgradeResource.tsx` is updated to include conditional blocks (`if`/`then`/`else`) when the condition is based on an existing property. - New read-only properties are now submitted during the upgrade process. - The `liveOmit` prop is added to the form to prevent the submission of unevaluated properties from conditionally hidden fields. --- ui/app/src/components/shared/ConfirmUpgradeResource.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/ui/app/src/components/shared/ConfirmUpgradeResource.tsx b/ui/app/src/components/shared/ConfirmUpgradeResource.tsx index 213a49a4c7..78abe7e8bc 100644 --- a/ui/app/src/components/shared/ConfirmUpgradeResource.tsx +++ b/ui/app/src/components/shared/ConfirmUpgradeResource.tsx @@ -331,6 +331,7 @@ export const ConfirmUpgradeResource: React.FunctionComponent< {finalSchema && ( Date: Thu, 11 Dec 2025 16:03:41 +0000 Subject: [PATCH 04/32] support all types of template --- .../shared/ConfirmUpgradeResource.tsx | 68 ++++++++++++------- 1 file changed, 43 insertions(+), 25 deletions(-) diff --git a/ui/app/src/components/shared/ConfirmUpgradeResource.tsx b/ui/app/src/components/shared/ConfirmUpgradeResource.tsx index 78abe7e8bc..a2a94c4067 100644 --- a/ui/app/src/components/shared/ConfirmUpgradeResource.tsx +++ b/ui/app/src/components/shared/ConfirmUpgradeResource.tsx @@ -12,6 +12,8 @@ import { } from "@fluentui/react"; import React, { useContext, useState, useEffect } from "react"; import { AvailableUpgrade, Resource } from "../../models/resource"; +import { ApiEndpoint } from "../../models/apiEndpoints"; +import { WorkspaceService } from "../../models/workspaceService"; import { HttpMethod, ResultType, @@ -88,7 +90,7 @@ const collectConditionalKeys = (entry: any): string[] => { return [...new Set(keys)]; }; -// Extract conditional blocks that reference any of the new keys. +// Extract conditional blocks that reference any of the new properties. const extractConditionalBlocks = (schema: any, newKeys: string[]) => { const conditionalEntries: any[] = []; if (!schema) return { allOf: [] }; @@ -150,17 +152,50 @@ export const ConfirmUpgradeResource: React.FunctionComponent< return; } + // Construct API paths for templates of specified resourceType + let templateListPath; + // Usually, the GET path would be `${templateGetPath}/${selectedTemplate}`, but there's an exception for user resources + let templateGetPath; + + let workspaceApplicationIdURI = undefined; + switch (props.resource.resourceType) { + case ResourceType.Workspace: + templateListPath = ApiEndpoint.WorkspaceTemplates; + templateGetPath = templateListPath; + break; + case ResourceType.WorkspaceService: + templateListPath = ApiEndpoint.WorkspaceServiceTemplates; + templateGetPath = templateListPath; + break; + case ResourceType.SharedService: + templateListPath = ApiEndpoint.SharedServiceTemplates; + templateGetPath = templateListPath; + break; + case ResourceType.UserResource: + if (props.resource.properties.parentWorkspaceService) { + // If we are upgrading a user resource, parent resource must have a workspaceId + const workspaceId = (props.resource.properties.parentWorkspaceService as WorkspaceService) + .workspaceId; + templateListPath = `${ApiEndpoint.Workspaces}/${workspaceId}/${ApiEndpoint.WorkspaceServiceTemplates}/${props.resource.properties.parentWorkspaceService.templateName}/${ApiEndpoint.UserResourceTemplates}`; + templateGetPath = `${ApiEndpoint.WorkspaceServiceTemplates}/${props.resource.properties.parentWorkspaceService.templateName}/${ApiEndpoint.UserResourceTemplates}`; + workspaceApplicationIdURI = props.resource.properties.parentWorkspaceService.workspaceApplicationIdURI; + break; + } else { + throw Error( + "Parent workspace service must be passed as prop when creating user resource.", + ); + } + default: + throw Error("Unsupported resource type."); + } + const fetchNewTemplateSchema = async () => { setLoadingSchema(true); setApiError(null); try { let fetchUrl = ""; - if (props.resource.resourceType === ResourceType.Workspace) { - fetchUrl = `/workspace-templates/${props.resource.templateName}?version=${selectedVersion}`; - } else { - const templateType = props.resource.resourceType.toLowerCase(); - fetchUrl = `/templates/${templateType}/${selectedVersion}`; - } + + fetchUrl = `${templateGetPath}/${props.resource.templateName}?version=${selectedVersion}`; const res = await apiCall( fetchUrl, @@ -211,23 +246,7 @@ export const ConfirmUpgradeResource: React.FunctionComponent< try { let body: any = { templateVersion: selectedVersion }; - // Patch only the new properties inside the 'properties' field - if (!body.properties) { - body.properties = {}; - } - console.log("New property values to set:", newPropertyValues); - Object.keys(newPropertyValues).forEach((key) => { - const val = newPropertyValues[key]; - body.properties[key] = val; - }); - console.log("Final upgrade body:", body); - - // Include other optional properties if applicable - // Object.keys(newPropertyValues).forEach((key) => { - // if (newPropertiesToFill.includes(key)) { - // body.properties[key] = newPropertyValues[key]; - // } - // }); + body.properties = newPropertyValues; let op = await apiCall( props.resource.resourcePath, @@ -271,7 +290,6 @@ export const ConfirmUpgradeResource: React.FunctionComponent< const uiSchema = { ...baseUiSchema, "ui:submitButtonOptions": { norender: true }, - // overview: { "ui:widget": "textarea" }, }; const onRenderOption = (option: any): JSX.Element => { From 1797f336a60fe5bbb1e53bcf93c328dae4cb639c Mon Sep 17 00:00:00 2001 From: James Chapman Date: Thu, 11 Dec 2025 17:14:45 +0000 Subject: [PATCH 05/32] show properties to be removed --- .../shared/ConfirmUpgradeResource.tsx | 34 ++++++++++++++----- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/ui/app/src/components/shared/ConfirmUpgradeResource.tsx b/ui/app/src/components/shared/ConfirmUpgradeResource.tsx index a2a94c4067..0a3bd3fb22 100644 --- a/ui/app/src/components/shared/ConfirmUpgradeResource.tsx +++ b/ui/app/src/components/shared/ConfirmUpgradeResource.tsx @@ -123,6 +123,7 @@ export const ConfirmUpgradeResource: React.FunctionComponent< const [newPropertyValues, setNewPropertyValues] = useState({}); const [loadingSchema, setLoadingSchema] = useState(false); const [newTemplateSchema, setNewTemplateSchema] = useState(null); + const [removedProperties, setRemovedProperties] = useState([]); const upgradeProps = { type: DialogType.normal, @@ -149,6 +150,7 @@ export const ConfirmUpgradeResource: React.FunctionComponent< setNewPropertiesToFill([]); setNewPropertyValues({}); setNewTemplateSchema(null); + setRemovedProperties([]); return; } @@ -197,7 +199,7 @@ export const ConfirmUpgradeResource: React.FunctionComponent< fetchUrl = `${templateGetPath}/${props.resource.templateName}?version=${selectedVersion}`; - const res = await apiCall( + const newTemplate = await apiCall( fetchUrl, HttpMethod.Get, wsAuth ? workspaceCtx.workspaceApplicationIdURI : undefined, @@ -205,25 +207,37 @@ export const ConfirmUpgradeResource: React.FunctionComponent< ResultType.JSON, ); + const currentTemplate = await apiCall( + `${templateGetPath}/${props.resource.templateName}?version=${props.resource.templateVersion}`, + HttpMethod.Get, + wsAuth ? workspaceCtx.workspaceApplicationIdURI : undefined, + undefined, + ResultType.JSON, + ); + // Use full fetched schema from API - setNewTemplateSchema(res); + setNewTemplateSchema(newTemplate); - const newSchemaProps = res?.properties || {}; - const currentProps = props.resource.properties || {}; + const newSchemaProps = newTemplate?.properties || {}; + const currentProps = currentTemplate?.properties || {}; const newKeys = getAllPropertyKeys(newSchemaProps); + console.log("New template property keys:", newKeys); const currentKeys = getAllPropertyKeys(currentProps); - + console.log("Current resource property keys:", currentKeys); const newPropKeys = newKeys.filter((k) => !currentKeys.includes(k)); - + console.log("Identified new property keys to fill:", newPropKeys); + const removedPropsArray = currentKeys.filter((k) => !newKeys.includes(k)); + console.log("Identified removed property keys:", removedPropsArray); setNewPropertiesToFill(newPropKeys); + setRemovedProperties(removedPropsArray); // prefill newPropertyValues with schema defaults or empty string setNewPropertyValues( newPropKeys.reduce((acc, key) => { // Get top-level portion of the key const topKey = key.split('.')[0]; - const defaultValue = res?.properties?.[topKey]?.default; + const defaultValue = newTemplate?.properties?.[topKey]?.default; acc[key] = defaultValue !== undefined ? defaultValue : ''; return acc; }, {} as any), @@ -340,7 +354,11 @@ export const ConfirmUpgradeResource: React.FunctionComponent< {loadingSchema && } - + {!loadingSchema && removedProperties.length > 0 && ( + + Warning: The following properties are no longer present and will be removed: {removedProperties.join(', ')} + + )} {!loadingSchema && newPropertiesToFill.length > 0 && ( From 01f3245c221034dbc242a71bb46a1e231356388f Mon Sep 17 00:00:00 2001 From: James Chapman Date: Thu, 11 Dec 2025 17:15:12 +0000 Subject: [PATCH 06/32] remove logging --- ui/app/src/components/shared/ConfirmUpgradeResource.tsx | 4 ---- 1 file changed, 4 deletions(-) diff --git a/ui/app/src/components/shared/ConfirmUpgradeResource.tsx b/ui/app/src/components/shared/ConfirmUpgradeResource.tsx index 0a3bd3fb22..b274f9486e 100644 --- a/ui/app/src/components/shared/ConfirmUpgradeResource.tsx +++ b/ui/app/src/components/shared/ConfirmUpgradeResource.tsx @@ -222,13 +222,9 @@ export const ConfirmUpgradeResource: React.FunctionComponent< const currentProps = currentTemplate?.properties || {}; const newKeys = getAllPropertyKeys(newSchemaProps); - console.log("New template property keys:", newKeys); const currentKeys = getAllPropertyKeys(currentProps); - console.log("Current resource property keys:", currentKeys); const newPropKeys = newKeys.filter((k) => !currentKeys.includes(k)); - console.log("Identified new property keys to fill:", newPropKeys); const removedPropsArray = currentKeys.filter((k) => !newKeys.includes(k)); - console.log("Identified removed property keys:", removedPropsArray); setNewPropertiesToFill(newPropKeys); setRemovedProperties(removedPropsArray); From 4b66ea543c353da528fc986d8350d2f37220191a Mon Sep 17 00:00:00 2001 From: James Chapman Date: Fri, 12 Dec 2025 10:46:27 +0000 Subject: [PATCH 07/32] update warning message --- ui/app/src/components/shared/ConfirmUpgradeResource.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ui/app/src/components/shared/ConfirmUpgradeResource.tsx b/ui/app/src/components/shared/ConfirmUpgradeResource.tsx index b274f9486e..263938c660 100644 --- a/ui/app/src/components/shared/ConfirmUpgradeResource.tsx +++ b/ui/app/src/components/shared/ConfirmUpgradeResource.tsx @@ -159,7 +159,7 @@ export const ConfirmUpgradeResource: React.FunctionComponent< // Usually, the GET path would be `${templateGetPath}/${selectedTemplate}`, but there's an exception for user resources let templateGetPath; - let workspaceApplicationIdURI = undefined; + // let workspaceApplicationIdURI = undefined; switch (props.resource.resourceType) { case ResourceType.Workspace: templateListPath = ApiEndpoint.WorkspaceTemplates; @@ -180,7 +180,7 @@ export const ConfirmUpgradeResource: React.FunctionComponent< .workspaceId; templateListPath = `${ApiEndpoint.Workspaces}/${workspaceId}/${ApiEndpoint.WorkspaceServiceTemplates}/${props.resource.properties.parentWorkspaceService.templateName}/${ApiEndpoint.UserResourceTemplates}`; templateGetPath = `${ApiEndpoint.WorkspaceServiceTemplates}/${props.resource.properties.parentWorkspaceService.templateName}/${ApiEndpoint.UserResourceTemplates}`; - workspaceApplicationIdURI = props.resource.properties.parentWorkspaceService.workspaceApplicationIdURI; + // workspaceApplicationIdURI = props.resource.properties.parentWorkspaceService.workspaceApplicationIdURI; break; } else { throw Error( @@ -352,7 +352,7 @@ export const ConfirmUpgradeResource: React.FunctionComponent< {loadingSchema && } {!loadingSchema && removedProperties.length > 0 && ( - Warning: The following properties are no longer present and will be removed: {removedProperties.join(', ')} + Warning: The following properties are no longer present in the template and will be removed: {removedProperties.join(', ')} )} {!loadingSchema && newPropertiesToFill.length > 0 && ( From f6c044eb5bd76af0af588bfe4436149c3955ce6e Mon Sep 17 00:00:00 2001 From: James Chapman Date: Fri, 12 Dec 2025 16:55:59 +0000 Subject: [PATCH 08/32] add capability to remove properties that no longer exist in the template --- api_app/db/repositories/resources.py | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/api_app/db/repositories/resources.py b/api_app/db/repositories/resources.py index 3bdfc307fa..938fc47b56 100644 --- a/api_app/db/repositories/resources.py +++ b/api_app/db/repositories/resources.py @@ -111,6 +111,16 @@ async def validate_input_against_template(self, template_name: str, resource_inp return parse_obj_as(ResourceTemplate, template) + def _get_all_property_keys_from_template(self, resource_template: ResourceTemplate) -> set: + properties = set(resource_template.properties.keys()) + if resource_template.allOf: + for condition in resource_template.allOf: + if 'then' in condition and 'properties' in condition['then']: + properties.update(condition['then']['properties'].keys()) + if 'else' in condition and 'properties' in condition['else']: + properties.update(condition['else']['properties'].keys()) + return properties + async def patch_resource(self, resource: Resource, resource_patch: ResourcePatch, resource_template: ResourceTemplate, etag: str, resource_template_repo: ResourceTemplateRepository, resource_history_repo: ResourceHistoryRepository, user: User, resource_action: str, force_version_update: bool = False) -> Tuple[Resource, ResourceTemplate]: await resource_history_repo.create_resource_history_item(resource) # now update the resource props @@ -123,6 +133,16 @@ async def patch_resource(self, resource: Resource, resource_patch: ResourcePatch if resource_patch.templateVersion is not None: await self.validate_template_version_patch(resource, resource_patch, resource_template_repo, resource_template, force_version_update) + new_template = await resource_template_repo.get_template_by_name_and_version(resource.templateName, resource_patch.templateVersion, resource.resourceType) + + old_properties = self._get_all_property_keys_from_template(resource_template) + new_properties = self._get_all_property_keys_from_template(new_template) + + properties_to_remove = old_properties - new_properties + for prop in properties_to_remove: + if prop in resource.properties: + del resource.properties[prop] + resource.templateVersion = resource_patch.templateVersion if resource_patch.properties is not None and len(resource_patch.properties) > 0: @@ -193,7 +213,7 @@ async def validate_patch(self, resource_patch: ResourcePatch, resource_template_ target_template = await resource_template_repo.get_template_by_name_and_version(resource_template.name, resource_patch.templateVersion, resource_template.resourceType) enriched_template = resource_template_repo.enrich_template(target_template, is_update=True) - # # validate the PATCH data against the target schema. + # validate the PATCH data against the target schema. update_template = copy.deepcopy(enriched_template) update_template["required"] = [] update_template["properties"] = {} From d06037d330b5dce0b190c9c532839404cd9ed638 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Mon, 15 Dec 2025 17:05:59 +0000 Subject: [PATCH 09/32] add tests and resolve issue causing failing test --- api_app/db/repositories/resources.py | 20 +- .../test_resource_repository.py | 178 +++++++++++++++++- 2 files changed, 184 insertions(+), 14 deletions(-) diff --git a/api_app/db/repositories/resources.py b/api_app/db/repositories/resources.py index 938fc47b56..bbf3f30eba 100644 --- a/api_app/db/repositories/resources.py +++ b/api_app/db/repositories/resources.py @@ -113,12 +113,12 @@ async def validate_input_against_template(self, template_name: str, resource_inp def _get_all_property_keys_from_template(self, resource_template: ResourceTemplate) -> set: properties = set(resource_template.properties.keys()) - if resource_template.allOf: + if "allOf" in resource_template: for condition in resource_template.allOf: - if 'then' in condition and 'properties' in condition['then']: - properties.update(condition['then']['properties'].keys()) - if 'else' in condition and 'properties' in condition['else']: - properties.update(condition['else']['properties'].keys()) + if "then" in condition and "properties" in condition["then"]: + properties.update(condition["then"]["properties"].keys()) + if "else" in condition and "properties" in condition["else"]: + properties.update(condition["else"]["properties"].keys()) return properties async def patch_resource(self, resource: Resource, resource_patch: ResourcePatch, resource_template: ResourceTemplate, etag: str, resource_template_repo: ResourceTemplateRepository, resource_history_repo: ResourceHistoryRepository, user: User, resource_action: str, force_version_update: bool = False) -> Tuple[Resource, ResourceTemplate]: @@ -207,6 +207,9 @@ async def validate_patch(self, resource_patch: ResourcePatch, resource_template_ # get the enriched (combined) template enriched_template = resource_template_repo.enrich_template(resource_template, is_update=True) + # get the old template properties for comparison during upgrades + old_template_properties = set(enriched_template["properties"].keys()) + # get the schema for the target version if upgrade is happening if resource_patch.templateVersion is not None: # fetch the template for the target version @@ -218,14 +221,19 @@ async def validate_patch(self, resource_patch: ResourcePatch, resource_template_ update_template["required"] = [] update_template["properties"] = {} for prop_name, prop in enriched_template["properties"].items(): - # Allow property if installing, explicitly updateable, or when upgrading and the new property is being added in this patch + # Allow property if: + # 1. Installing (all properties allowed) + # 2. Property is explicitly updateable + # 3. Upgrading and the property is NEW (not in old template) and being added in this patch if ( resource_action == RESOURCE_ACTION_INSTALL or prop.get("updateable", False) is True or ( resource_patch.templateVersion is not None + and resource_patch.templateVersion != resource_template.version and resource_patch.properties is not None and prop_name in resource_patch.properties + and prop_name not in old_template_properties ) ): update_template["properties"][prop_name] = prop diff --git a/api_app/tests_ma/test_db/test_repositories/test_resource_repository.py b/api_app/tests_ma/test_db/test_repositories/test_resource_repository.py index 60b8569c53..e734b36f33 100644 --- a/api_app/tests_ma/test_db/test_repositories/test_resource_repository.py +++ b/api_app/tests_ma/test_db/test_repositories/test_resource_repository.py @@ -4,6 +4,7 @@ import pytest import pytest_asyncio from mock import patch, MagicMock +from pydantic import parse_obj_as from jsonschema.exceptions import ValidationError from resources import strings @@ -140,6 +141,58 @@ def sample_nested_template() -> ResourceTemplate: ).dict(exclude_none=True) +def sample_resource_template_with_new_property(version: str = "0.2.0") -> dict: + """ + Returns a template similar to sample_resource_template but with an additional + 'new_property' that is not updateable. Useful for testing template upgrades. + """ + return ResourceTemplate( + id="123", + name="tre-user-resource", + description="description", + version=version, + resourceType=ResourceType.UserResource, + current=True, + required=['os_image', 'title'], + properties={ + 'title': { + 'type': 'string', + 'title': 'Title of the resource' + }, + 'os_image': { + 'type': 'string', + 'title': 'Windows image', + 'description': 'Select Windows image to use for VM', + 'enum': [ + 'Windows 10', + 'Server 2019 Data Science VM' + ], + 'updateable': False + }, + 'vm_size': { + 'type': 'string', + 'title': 'VM Size', + 'description': 'Select Windows image to use for VM', + 'enum': [ + 'small', + 'large' + ], + 'updateable': True + }, + 'new_property': { + 'type': 'string', + 'title': 'New non-updateable property', + 'enum': [ + 'value1', + 'value2' + ], + 'updateable': False + } + }, + actions=[] + ).dict(exclude_none=True) + + @pytest.mark.asyncio @patch("db.repositories.resources.ResourceRepository._get_enriched_template") @patch("db.repositories.resources.ResourceRepository._validate_resource_parameters", return_value=None) @@ -370,10 +423,11 @@ async def test_patch_resource_preserves_property_history(_, __, ___, resource_re resource_repo.update_item_with_etag.assert_called_with(expected_resource, etag) +@pytest.mark.asyncio @patch('db.repositories.resources.ResourceTemplateRepository.enrich_template') -def test_validate_patch_with_good_fields_passes(template_repo, resource_repo): +async def test_validate_patch_with_good_fields_passes(template_repo, resource_repo): """ - Make sure that patch is NOT valid when non-updateable fields are included + Make sure that patch is valid when updateable fields are included """ template_repo.enrich_template = MagicMock(return_value=sample_resource_template()) @@ -381,11 +435,12 @@ def test_validate_patch_with_good_fields_passes(template_repo, resource_repo): # check it's valid when updating a single updateable prop patch = ResourcePatch(isEnabled=True, properties={'vm_size': 'large'}) - resource_repo.validate_patch(patch, template_repo, template, strings.RESOURCE_ACTION_UPDATE) + await resource_repo.validate_patch(patch, template_repo, template, strings.RESOURCE_ACTION_UPDATE) +@pytest.mark.asyncio @patch('db.repositories.resources.ResourceTemplateRepository.enrich_template') -def test_validate_patch_with_bad_fields_fails(template_repo, resource_repo): +async def test_validate_patch_with_bad_fields_fails(template_repo, resource_repo): """ Make sure that patch is NOT valid when non-updateable fields are included """ @@ -396,14 +451,121 @@ def test_validate_patch_with_bad_fields_fails(template_repo, resource_repo): # check it's invalid when sending an unexpected field patch = ResourcePatch(isEnabled=True, properties={'vm_size': 'large', 'unexpected_field': 'surprise!'}) with pytest.raises(ValidationError): - resource_repo.validate_patch(patch, template_repo, template, strings.RESOURCE_ACTION_INSTALL) + await resource_repo.validate_patch(patch, template_repo, template, strings.RESOURCE_ACTION_UPDATE) - # check it's invalid when sending a bad value + # check it's invalid when sending a bad value (new install) patch = ResourcePatch(isEnabled=True, properties={'vm_size': 'huge'}) with pytest.raises(ValidationError): - resource_repo.validate_patch(patch, template_repo, template, strings.RESOURCE_ACTION_INSTALL) + await resource_repo.validate_patch(patch, template_repo, template, strings.RESOURCE_ACTION_INSTALL) + + # check it's invalid when sending a bad value (update) + patch = ResourcePatch(isEnabled=True, properties={'vm_size': 'huge'}) + with pytest.raises(ValidationError): + await resource_repo.validate_patch(patch, template_repo, template, strings.RESOURCE_ACTION_UPDATE) # check it's invalid when trying to update a non-updateable field patch = ResourcePatch(isEnabled=True, properties={'vm_size': 'large', 'os_image': 'linux'}) with pytest.raises(ValidationError): - resource_repo.validate_patch(patch, template_repo, template, strings.RESOURCE_ACTION_INSTALL) + await resource_repo.validate_patch(patch, template_repo, template, strings.RESOURCE_ACTION_UPDATE) + + +@pytest.mark.asyncio +@patch('db.repositories.resources.ResourceTemplateRepository.get_template_by_name_and_version') +@patch('db.repositories.resources.ResourceTemplateRepository.enrich_template') +async def test_validate_patch_allows_new_non_updateable_property_during_upgrade(enrich_template_mock, get_template_mock, resource_repo): + """ + Test that during a template upgrade, new properties (not in old version) can be specified + even if they are marked as updateable: false in the new template version + """ + # Old template has os_image and vm_size + old_template = sample_resource_template() + old_template['version'] = '0.1.0' + + # New template adds a new property 'new_property' that is not updateable + new_template = sample_resource_template_with_new_property(version='0.2.0') + + # Mock the template repository + template_repo = MagicMock() + template_repo.get_template_by_name_and_version = AsyncMock(return_value=parse_obj_as(ResourceTemplate, new_template)) + template_repo.enrich_template = MagicMock(side_effect=[old_template, new_template]) + + # Patch includes the new property during upgrade - this should be ALLOWED + patch = ResourcePatch(templateVersion='0.2.0', properties={'new_property': 'value1'}) + + # This should NOT raise a ValidationError + await resource_repo.validate_patch(patch, template_repo, parse_obj_as(ResourceTemplate, old_template), strings.RESOURCE_ACTION_UPDATE) + + +@pytest.mark.asyncio +@patch('db.repositories.resources.ResourceTemplateRepository.get_template_by_name_and_version') +@patch('db.repositories.resources.ResourceTemplateRepository.enrich_template') +async def test_validate_patch_rejects_existing_non_updateable_property_during_upgrade(enrich_template_mock, get_template_mock, resource_repo): + """ + Test that during a template upgrade, existing non-updateable properties still cannot be modified + """ + # Old template has os_image (non-updateable) and vm_size (updateable) + old_template = sample_resource_template() + + # New template is the same but version 0.2.0 + new_template = copy.deepcopy(old_template) + + # Mock the template repository + template_repo = MagicMock() + template_repo.get_template_by_name_and_version = AsyncMock(return_value=parse_obj_as(ResourceTemplate, new_template)) + template_repo.enrich_template = MagicMock(side_effect=[old_template, new_template]) + + # Try to update existing non-updateable property during upgrade - this should FAIL + patch = ResourcePatch(templateVersion='0.2.0', properties={'os_image': 'Windows 10'}) + + with pytest.raises(ValidationError): + await resource_repo.validate_patch(patch, template_repo, parse_obj_as(ResourceTemplate, old_template), strings.RESOURCE_ACTION_UPDATE) + + +@pytest.mark.asyncio +@patch('db.repositories.resources.ResourceTemplateRepository.get_template_by_name_and_version') +@patch('db.repositories.resources.ResourceTemplateRepository.enrich_template') +async def test_validate_patch_allows_updateable_property_during_upgrade(enrich_template_mock, get_template_mock, resource_repo): + """ + Test that during a template upgrade, updateable properties can still be modified + """ + # Old template 0.1.0 + old_template = sample_resource_template() + + # New template 0.2.0 + new_template = copy.deepcopy(old_template) + + # Mock the template repository + template_repo = MagicMock() + template_repo.get_template_by_name_and_version = AsyncMock(return_value=parse_obj_as(ResourceTemplate, new_template)) + template_repo.enrich_template = MagicMock(side_effect=[old_template, new_template]) + + # Update existing updateable property during upgrade - this should work + patch = ResourcePatch(templateVersion='0.2.0', properties={'vm_size': 'large'}) + + # This should NOT raise a ValidationError + await resource_repo.validate_patch(patch, template_repo, parse_obj_as(ResourceTemplate, old_template), strings.RESOURCE_ACTION_UPDATE) + + +@pytest.mark.asyncio +@patch('db.repositories.resources.ResourceTemplateRepository.get_template_by_name_and_version') +@patch('db.repositories.resources.ResourceTemplateRepository.enrich_template') +async def test_validate_patch_allows_mix_of_new_and_updateable_properties_during_upgrade(enrich_template_mock, get_template_mock, resource_repo): + """ + Test that during upgrade, you can specify both new non-updateable properties and existing updateable properties + """ + # Old template + old_template = sample_resource_template() + + # New template adds new_property + new_template = sample_resource_template_with_new_property(version='0.2.0') + + # Mock the template repository + template_repo = MagicMock() + template_repo.get_template_by_name_and_version = AsyncMock(return_value=parse_obj_as(ResourceTemplate, new_template)) + template_repo.enrich_template = MagicMock(side_effect=[old_template, new_template]) + + # Patch with both new non-updateable property and existing updateable property + patch = ResourcePatch(templateVersion='0.2.0', properties={'new_property': 'value1', 'vm_size': 'large'}) + + # This should NOT raise a ValidationError + await resource_repo.validate_patch(patch, template_repo, parse_obj_as(ResourceTemplate, old_template), strings.RESOURCE_ACTION_UPDATE) From 7ad0a7af56080e3817b86b491a3d19da11c91145 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Tue, 16 Dec 2025 09:48:33 +0000 Subject: [PATCH 10/32] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index edcbaa8586..376a3194e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ ENHANCEMENTS: * API: Replace HTTP_422_UNPROCESSABLE_ENTITY response with HTTP_422_UNPROCESSABLE_CONTENT as per RFC 9110 ([#4742](https://github.com/microsoft/AzureTRE/issues/4742)) * Change Group.ReadWrite.All permission to Group.Create for AUTO_WORKSPACE_GROUP_CREATION ([#4772](https://github.com/microsoft/AzureTRE/issues/4772)) * Make workspace shared storage quota updateable ([#4314](https://github.com/microsoft/AzureTRE/issues/4314)) +* Allow new template properties to be specified during template upgrades. Remove Template properties that no longer exist. BUG FIXES: * Fix circular dependancy in base workspace. ([#4756](https://github.com/microsoft/AzureTRE/pull/4756)) From a27267727cb36878a5a832ad0766a8440a9292b4 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Tue, 16 Dec 2025 11:06:23 +0000 Subject: [PATCH 11/32] cache currrent template --- .../shared/ConfirmUpgradeResource.tsx | 33 ++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/ui/app/src/components/shared/ConfirmUpgradeResource.tsx b/ui/app/src/components/shared/ConfirmUpgradeResource.tsx index 263938c660..2e09f81e97 100644 --- a/ui/app/src/components/shared/ConfirmUpgradeResource.tsx +++ b/ui/app/src/components/shared/ConfirmUpgradeResource.tsx @@ -10,7 +10,7 @@ import { Icon, Stack, } from "@fluentui/react"; -import React, { useContext, useState, useEffect } from "react"; +import React, { useContext, useState, useEffect, useRef } from "react"; import { AvailableUpgrade, Resource } from "../../models/resource"; import { ApiEndpoint } from "../../models/apiEndpoints"; import { WorkspaceService } from "../../models/workspaceService"; @@ -125,6 +125,14 @@ export const ConfirmUpgradeResource: React.FunctionComponent< const [newTemplateSchema, setNewTemplateSchema] = useState(null); const [removedProperties, setRemovedProperties] = useState([]); + // Cache for current template to avoid refetching the same template repeatedly while selecting versions + const currentTemplateRef = useRef(null); + + // Invalidate cache if the resource's template name or current template version changes + useEffect(() => { + currentTemplateRef.current = null; + }, [props.resource.templateName, props.resource.templateVersion]); + const upgradeProps = { type: DialogType.normal, title: `Upgrade Template Version?`, @@ -207,13 +215,20 @@ export const ConfirmUpgradeResource: React.FunctionComponent< ResultType.JSON, ); - const currentTemplate = await apiCall( - `${templateGetPath}/${props.resource.templateName}?version=${props.resource.templateVersion}`, - HttpMethod.Get, - wsAuth ? workspaceCtx.workspaceApplicationIdURI : undefined, - undefined, - ResultType.JSON, - ); + // Reuse cached current template if available to avoid redundant network calls + let currentTemplate; + if (currentTemplateRef.current) { + currentTemplate = currentTemplateRef.current; + } else { + currentTemplate = await apiCall( + `${templateGetPath}/${props.resource.templateName}?version=${props.resource.templateVersion}`, + HttpMethod.Get, + wsAuth ? workspaceCtx.workspaceApplicationIdURI : undefined, + undefined, + ResultType.JSON, + ); + currentTemplateRef.current = currentTemplate; + } // Use full fetched schema from API setNewTemplateSchema(newTemplate); @@ -223,8 +238,10 @@ export const ConfirmUpgradeResource: React.FunctionComponent< const newKeys = getAllPropertyKeys(newSchemaProps); const currentKeys = getAllPropertyKeys(currentProps); + const newPropKeys = newKeys.filter((k) => !currentKeys.includes(k)); const removedPropsArray = currentKeys.filter((k) => !newKeys.includes(k)); + setNewPropertiesToFill(newPropKeys); setRemovedProperties(removedPropsArray); From b02794e84eb0b20857c64ee14dd4798a24098c8c Mon Sep 17 00:00:00 2001 From: James Chapman Date: Fri, 19 Dec 2025 17:45:24 +0000 Subject: [PATCH 12/32] Add test --- .../shared/ConfirmUpgradeResource.test.tsx | 704 ++++++++++++++++++ 1 file changed, 704 insertions(+) create mode 100644 ui/app/src/components/shared/ConfirmUpgradeResource.test.tsx diff --git a/ui/app/src/components/shared/ConfirmUpgradeResource.test.tsx b/ui/app/src/components/shared/ConfirmUpgradeResource.test.tsx new file mode 100644 index 0000000000..ac034b69e5 --- /dev/null +++ b/ui/app/src/components/shared/ConfirmUpgradeResource.test.tsx @@ -0,0 +1,704 @@ +import React from "react"; +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { render, screen, fireEvent, waitFor, createPartialFluentUIMock } from "../../test-utils"; +import { ConfirmUpgradeResource } from "./ConfirmUpgradeResource"; +import { Resource, AvailableUpgrade } from "../../models/resource"; +import { ResourceType } from "../../models/resourceType"; +import { WorkspaceContext } from "../../contexts/WorkspaceContext"; +import { CostResource } from "../../models/costs"; + +// Mock dependencies +const mockApiCall = vi.fn(); +const mockDispatch = vi.fn(); + +// Mock template schemas +const mockCurrentTemplateSchema = { + properties: { + display_name: { type: "string" }, + resource_key: { type: "string" }, + existing_property: { type: "string" }, + }, + required: ["display_name"], +}; + +const mockNewTemplateSchema = { + properties: { + display_name: { type: "string" }, + resource_key: { type: "string" }, + new_property: { type: "string", default: "default_value" }, + }, + required: ["display_name"], + uiSchema: {}, +}; + +vi.mock("../../hooks/useAuthApiCall", () => ({ + useAuthApiCall: () => mockApiCall, + HttpMethod: { Patch: "PATCH", Get: "GET" }, + ResultType: { JSON: "JSON" }, +})); + +vi.mock("../../hooks/customReduxHooks", () => ({ + useAppDispatch: () => mockDispatch, +})); + +vi.mock("../shared/notifications/operationsSlice", () => ({ + addUpdateOperation: vi.fn(), + default: (state: any = { items: [] }) => state +})); + +// Mock FluentUI components using centralized mocks +vi.mock("@fluentui/react", async () => { + const actual = await vi.importActual("@fluentui/react"); + return { + ...actual, + ...createPartialFluentUIMock([ + 'Dialog', + 'DialogFooter', + 'DialogType', + 'PrimaryButton', + 'DefaultButton', + 'Dropdown', + 'Spinner', + 'MessageBar', + 'MessageBarType', + 'Icon' + ]), + }; +}); + +vi.mock("./ExceptionLayout", () => ({ + ExceptionLayout: ({ e }: any) => ( +
{e.userMessage}
+ ), +})); + +const mockAvailableUpgrades: AvailableUpgrade[] = [ + { version: "1.1.0", forceUpdateRequired: false }, + { version: "1.2.0", forceUpdateRequired: false }, + { version: "2.0.0", forceUpdateRequired: true }, +]; + +const mockResource: Resource = { + id: "test-resource-id", + resourceType: ResourceType.WorkspaceService, + templateName: "test-template", + templateVersion: "1.0.0", + resourcePath: "/workspaces/test-workspace/workspace-services/test-resource-id", + resourceVersion: 1, + isEnabled: true, + properties: { + display_name: "Test Resource", + }, + _etag: "test-etag", + updatedWhen: Date.now(), + deploymentStatus: "deployed", + availableUpgrades: mockAvailableUpgrades, + history: [], + user: { + id: "test-user-id", + name: "Test User", + email: "test@example.com", + roleAssignments: [], + roles: ["workspace_researcher"], + }, +}; + +const mockWorkspaceContext = { + costs: [] as CostResource[], + workspace: { + id: "test-workspace-id", + isEnabled: true, + resourcePath: "/workspaces/test-workspace-id", + resourceVersion: 1, + resourceType: ResourceType.Workspace, + templateName: "base", + templateVersion: "1.0.0", + availableUpgrades: [], + deploymentStatus: "deployed", + updatedWhen: Date.now(), + history: [], + _etag: "test-etag", + properties: { + display_name: "Test Workspace", + }, + user: { + id: "test-user-id", + name: "Test User", + email: "test@example.com", + roleAssignments: [], + roles: ["workspace_owner"], + }, + workspaceURL: "https://workspace.example.com", + }, + workspaceApplicationIdURI: "test-app-id-uri", + roles: ["workspace_owner"], + setCosts: vi.fn(), + setRoles: vi.fn(), + setWorkspace: vi.fn(), +}; + +const renderWithWorkspaceContext = (component: React.ReactElement) => { + return render( + + {component} + + ); +}; + +describe("ConfirmUpgradeResource Component", () => { + const mockOnDismiss = vi.fn(); + + beforeEach(() => { + vi.clearAllMocks(); + // Mock API call to return templates for GET requests + mockApiCall.mockImplementation((url, method) => { + if (method === "GET" && url.includes("?version=")) { + if (url.includes("version=1.0.0")) { + return Promise.resolve(mockCurrentTemplateSchema); + } else { + return Promise.resolve(mockNewTemplateSchema); + } + } + return Promise.resolve({ operation: { id: "operation-id", status: "running" } }); + }); + }); + + it("renders upgrade dialog with correct title and content", () => { + renderWithWorkspaceContext( + + ); + + expect(screen.getByTestId("dialog-title")).toHaveTextContent( + "Upgrade Template Version?" + ); + expect(screen.getByTestId("dialog-subtext")).toHaveTextContent( + "Are you sure you want upgrade the template version of Test Resource from version 1.0.0?" + ); + }); + + it("shows warning message about irreversible upgrade", () => { + renderWithWorkspaceContext( + + ); + + expect(screen.getByTestId("message-bar")).toBeInTheDocument(); + expect(screen.getByText("Upgrading the template version is irreversible.")).toBeInTheDocument(); + }); + + it("renders dropdown with available upgrade versions", () => { + renderWithWorkspaceContext( + + ); + + const dropdown = screen.getByTestId("dropdown"); + expect(dropdown).toBeInTheDocument(); + + // Check that non-major upgrades are included (force update required = false) + expect(screen.getByText("1.1.0")).toBeInTheDocument(); + expect(screen.getByText("1.2.0")).toBeInTheDocument(); + + // Major upgrade (force update required = true) should not be included in regular dropdown + expect(screen.queryByText("2.0.0")).not.toBeInTheDocument(); + }); + + it("disables upgrade button when no version is selected", () => { + renderWithWorkspaceContext( + + ); + + const upgradeButton = screen.getByTestId("primary-button"); + expect(upgradeButton).toBeDisabled(); + }); + + it("enables upgrade button when version is selected", async () => { + renderWithWorkspaceContext( + + ); + + const dropdown = screen.getByTestId("dropdown"); + fireEvent.change(dropdown, { target: { value: "1.1.0" } }); + + // Wait for schema to load and button to become enabled + await waitFor(() => { + const upgradeButton = screen.getByTestId("primary-button"); + expect(upgradeButton).not.toBeDisabled(); + }); + }); + + it("calls API with selected version on upgrade", async () => { + const mockOperation = { id: "operation-id", status: "running" }; + mockApiCall.mockImplementation((url, method) => { + if (method === "PATCH") { + return Promise.resolve({ operation: mockOperation }); + } + // Handle GET requests for schemas + if (method === "GET" && url.includes("?version=")) { + if (url.includes("version=1.0.0")) { + return Promise.resolve(mockCurrentTemplateSchema); + } else { + return Promise.resolve(mockNewTemplateSchema); + } + } + return Promise.resolve({ operation: mockOperation }); + }); + + renderWithWorkspaceContext( + + ); + + // Select a version + const dropdown = screen.getByTestId("dropdown"); + fireEvent.change(dropdown, { target: { value: "1.1.0" } }); + + // Wait for schema to load + await waitFor(() => { + expect(screen.queryByText("Loading new template schema...")).not.toBeInTheDocument(); + }); + + // Click upgrade + const upgradeButton = screen.getByTestId("primary-button"); + fireEvent.click(upgradeButton); + + await waitFor(() => { + expect(mockApiCall).toHaveBeenCalledWith( + mockResource.resourcePath, + "PATCH", + mockWorkspaceContext.workspaceApplicationIdURI, + expect.objectContaining({ + templateVersion: "1.1.0", + properties: expect.any(Object), + }), + "JSON", + undefined, + undefined, + mockResource._etag + ); + }); + + expect(mockDispatch).toHaveBeenCalled(); + expect(mockOnDismiss).toHaveBeenCalled(); + }); + + it("shows loading spinner during API call", async () => { + mockApiCall.mockImplementation((url, method) => { + if (method === "PATCH") { + return new Promise((resolve) => + setTimeout( + () => { + resolve({ operation: { id: "operation-id", status: "running" } }); + }, + 100 + ) + ); + } + // Handle GET requests for schemas + if (method === "GET" && url.includes("?version=")) { + if (url.includes("version=1.0.0")) { + return Promise.resolve(mockCurrentTemplateSchema); + } else { + return Promise.resolve(mockNewTemplateSchema); + } + } + return Promise.resolve({ + operation: { id: "operation-id", status: "running" }, + }); + }); + + renderWithWorkspaceContext( + + ); + + // Select a version + const dropdown = screen.getByTestId("dropdown"); + fireEvent.change(dropdown, { target: { value: "1.1.0" } }); + + // Wait for schema to load + await waitFor(() => { + expect( + screen.queryByText("Loading new template schema...") + ).not.toBeInTheDocument(); + }); + + // Click upgrade and check for loading spinner + const upgradeButton = screen.getByTestId("primary-button"); + fireEvent.click(upgradeButton); + + expect(screen.getByTestId("spinner")).toBeInTheDocument(); + expect(screen.getByText("Sending request...")).toBeInTheDocument(); + }); + + it("displays error when API call fails", async () => { + mockApiCall.mockImplementation((url, method) => { + if (method === "PATCH") { + return Promise.reject(new Error("Network error")); + } + // Handle GET requests for schemas + if (method === "GET" && url.includes("?version=")) { + if (url.includes("version=1.0.0")) { + return Promise.resolve(mockCurrentTemplateSchema); + } else { + return Promise.resolve(mockNewTemplateSchema); + } + } + return Promise.reject(new Error("Network error")); + }); + + renderWithWorkspaceContext( + + ); + + // Select a version + const dropdown = screen.getByTestId("dropdown"); + fireEvent.change(dropdown, { target: { value: "1.1.0" } }); + + // Wait for schema to load + await waitFor(() => { + expect( + screen.queryByText("Loading new template schema...") + ).not.toBeInTheDocument(); + }); + + // Click upgrade + const upgradeButton = screen.getByTestId("primary-button"); + fireEvent.click(upgradeButton); + + await waitFor(() => { + expect(screen.getByTestId("exception-layout")).toBeInTheDocument(); + expect(screen.getByText("Failed to upgrade resource")).toBeInTheDocument(); + }); + }); + + it("uses workspace auth for workspace service resources", async () => { + const mockOperation = { id: "operation-id", status: "running" }; + mockApiCall.mockImplementation((url, method) => { + if (method === "PATCH") { + return Promise.resolve({ operation: mockOperation }); + } + // Handle GET requests for schemas + if (method === "GET" && url.includes("?version=")) { + if (url.includes("version=1.0.0")) { + return Promise.resolve(mockCurrentTemplateSchema); + } else { + return Promise.resolve(mockNewTemplateSchema); + } + } + return Promise.resolve({ operation: mockOperation }); + }); + + renderWithWorkspaceContext( + + ); + + // Select a version + const dropdown = screen.getByTestId("dropdown"); + fireEvent.change(dropdown, { target: { value: "1.1.0" } }); + + // Wait for schema to load + await waitFor(() => { + expect( + screen.queryByText("Loading new template schema...") + ).not.toBeInTheDocument(); + }); + + // Click upgrade + const upgradeButton = screen.getByTestId("primary-button"); + fireEvent.click(upgradeButton); + + await waitFor(() => { + expect(mockApiCall).toHaveBeenCalledWith( + expect.any(String), + "PATCH", + mockWorkspaceContext.workspaceApplicationIdURI, // should use workspace auth + expect.any(Object), + "JSON", + undefined, + undefined, + expect.any(String) + ); + }); + }); + + it("does not use workspace auth for shared service resources", async () => { + const sharedServiceResource = { + ...mockResource, + resourceType: ResourceType.SharedService, + }; + const mockOperation = { id: "operation-id", status: "running" }; + mockApiCall.mockImplementation((url, method) => { + if (method === "PATCH") { + return Promise.resolve({ operation: mockOperation }); + } + // Handle GET requests for schemas + if (method === "GET" && url.includes("?version=")) { + if (url.includes("version=1.0.0")) { + return Promise.resolve(mockCurrentTemplateSchema); + } else { + return Promise.resolve(mockNewTemplateSchema); + } + } + return Promise.resolve({ operation: mockOperation }); + }); + + renderWithWorkspaceContext( + + ); + + // Select a version + const dropdown = screen.getByTestId("dropdown"); + fireEvent.change(dropdown, { target: { value: "1.1.0" } }); + + // Wait for schema to load + await waitFor(() => { + expect( + screen.queryByText("Loading new template schema...") + ).not.toBeInTheDocument(); + }); + + // Click upgrade + const upgradeButton = screen.getByTestId("primary-button"); + fireEvent.click(upgradeButton); + + await waitFor(() => { + expect(mockApiCall).toHaveBeenCalledWith( + expect.any(String), + "PATCH", + undefined, // should not use workspace auth + expect.any(Object), + "JSON", + undefined, + undefined, + expect.any(String) + ); + }); + }); + + it("filters out major upgrades from dropdown options", () => { + const resourceWithMajorUpgrade = { + ...mockResource, + availableUpgrades: [ + { version: "1.1.0", forceUpdateRequired: false }, + { version: "2.0.0", forceUpdateRequired: true }, + { version: "1.2.0", forceUpdateRequired: false }, + ], + }; + + renderWithWorkspaceContext( + + ); + + // Minor updates should be available + expect(screen.getByText("1.1.0")).toBeInTheDocument(); + expect(screen.getByText("1.2.0")).toBeInTheDocument(); + + // Major update should not be available in dropdown + expect(screen.queryByText("2.0.0")).not.toBeInTheDocument(); + }); + + it("displays form when new properties need to be added", async () => { + renderWithWorkspaceContext( + + ); + + // Select a version that has new properties + const dropdown = screen.getByTestId("dropdown"); + fireEvent.change(dropdown, { target: { value: "1.1.0" } }); + + // Wait for schema to load + await waitFor(() => { + expect( + screen.queryByText("Loading new template schema...") + ).not.toBeInTheDocument(); + }); + + // Should show info message about new properties + expect(screen.getByText("You must specify values for new properties:")).toBeInTheDocument(); + + // The form input for new_property should be rendered + expect(screen.getByDisplayValue("default_value")).toBeInTheDocument(); + }); + + it("displays warning about removed properties", async () => { + renderWithWorkspaceContext( + + ); + + // Select a version + const dropdown = screen.getByTestId("dropdown"); + fireEvent.change(dropdown, { target: { value: "1.1.0" } }); + + // Wait for schema to load + await waitFor(() => { + expect( + screen.queryByText("Loading new template schema...") + ).not.toBeInTheDocument(); + }); + + // Should show warning about removed properties + expect(screen.getByText(/Warning: The following properties are no longer present/)).toBeInTheDocument(); + expect(screen.getByText(/existing_property/)).toBeInTheDocument(); + }); + + it("disables upgrade button when required new properties are cleared", async () => { + renderWithWorkspaceContext( + + ); + + // Select a version + const dropdown = screen.getByTestId("dropdown"); + fireEvent.change(dropdown, { target: { value: "1.1.0" } }); + + // Wait for schema to load + await waitFor(() => { + expect( + screen.queryByText("Loading new template schema...") + ).not.toBeInTheDocument(); + }); + + // Find the input field and clear it + const inputField = screen.getByDisplayValue("default_value"); + fireEvent.change(inputField, { target: { value: "" } }); + + // Button should now be disabled because the required property is empty + await waitFor(() => { + const upgradeButton = screen.getByTestId("primary-button"); + expect(upgradeButton).toBeDisabled(); + }); + }); + + it("enables upgrade button when all new properties are filled in", async () => { + renderWithWorkspaceContext( + + ); + + // Select a version + const dropdown = screen.getByTestId("dropdown"); + fireEvent.change(dropdown, { target: { value: "1.1.0" } }); + + // Wait for schema to load + await waitFor(() => { + expect( + screen.queryByText("Loading new template schema...") + ).not.toBeInTheDocument(); + }); + + // Find the new_property input field and fill it + const inputField = screen.getByDisplayValue("default_value"); + fireEvent.change(inputField, { target: { value: "filled_value" } }); + + // Button should now be enabled + await waitFor(() => { + const upgradeButton = screen.getByTestId("primary-button"); + expect(upgradeButton).not.toBeDisabled(); + }); + }); + + it("includes new property values in upgrade API call", async () => { + const mockOperation = { id: "operation-id", status: "running" }; + mockApiCall.mockImplementation((url, method) => { + if (method === "PATCH") { + return Promise.resolve({ operation: mockOperation }); + } + // Handle GET requests for schemas + if (method === "GET" && url.includes("?version=")) { + if (url.includes("version=1.0.0")) { + return Promise.resolve(mockCurrentTemplateSchema); + } else { + return Promise.resolve(mockNewTemplateSchema); + } + } + return Promise.resolve({ operation: mockOperation }); + }); + + renderWithWorkspaceContext( + + ); + + // Select a version + const dropdown = screen.getByTestId("dropdown"); + fireEvent.change(dropdown, { target: { value: "1.1.0" } }); + + // Wait for schema to load + await waitFor(() => { + expect( + screen.queryByText("Loading new template schema...") + ).not.toBeInTheDocument(); + }); + + // Fill in the new property + const inputField = screen.getByDisplayValue("default_value"); + fireEvent.change(inputField, { target: { value: "custom_value" } }); + + // Click upgrade + await waitFor(() => { + const upgradeButton = screen.getByTestId("primary-button"); + fireEvent.click(upgradeButton); + }); + + // Verify the API call includes the new property value + await waitFor(() => { + expect(mockApiCall).toHaveBeenCalledWith( + mockResource.resourcePath, + "PATCH", + mockWorkspaceContext.workspaceApplicationIdURI, + expect.objectContaining({ + templateVersion: "1.1.0", + properties: expect.objectContaining({ + new_property: "custom_value", + }), + }), + "JSON", + undefined, + undefined, + mockResource._etag + ); + }); + }); +}); From e99a78f5f24721076f2bab3754ee6e68b226e961 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Mon, 22 Dec 2025 17:32:18 +0000 Subject: [PATCH 13/32] fix ws template auth --- ui/app/src/components/shared/ConfirmUpgradeResource.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/ui/app/src/components/shared/ConfirmUpgradeResource.tsx b/ui/app/src/components/shared/ConfirmUpgradeResource.tsx index 2e09f81e97..b2dbf2b4eb 100644 --- a/ui/app/src/components/shared/ConfirmUpgradeResource.tsx +++ b/ui/app/src/components/shared/ConfirmUpgradeResource.tsx @@ -188,7 +188,6 @@ export const ConfirmUpgradeResource: React.FunctionComponent< .workspaceId; templateListPath = `${ApiEndpoint.Workspaces}/${workspaceId}/${ApiEndpoint.WorkspaceServiceTemplates}/${props.resource.properties.parentWorkspaceService.templateName}/${ApiEndpoint.UserResourceTemplates}`; templateGetPath = `${ApiEndpoint.WorkspaceServiceTemplates}/${props.resource.properties.parentWorkspaceService.templateName}/${ApiEndpoint.UserResourceTemplates}`; - // workspaceApplicationIdURI = props.resource.properties.parentWorkspaceService.workspaceApplicationIdURI; break; } else { throw Error( @@ -210,7 +209,7 @@ export const ConfirmUpgradeResource: React.FunctionComponent< const newTemplate = await apiCall( fetchUrl, HttpMethod.Get, - wsAuth ? workspaceCtx.workspaceApplicationIdURI : undefined, + props.resource.resourceType === ResourceType.UserResource ? workspaceCtx.workspaceApplicationIdURI : undefined, undefined, ResultType.JSON, ); @@ -223,7 +222,7 @@ export const ConfirmUpgradeResource: React.FunctionComponent< currentTemplate = await apiCall( `${templateGetPath}/${props.resource.templateName}?version=${props.resource.templateVersion}`, HttpMethod.Get, - wsAuth ? workspaceCtx.workspaceApplicationIdURI : undefined, + props.resource.resourceType === ResourceType.UserResource ? workspaceCtx.workspaceApplicationIdURI : undefined, undefined, ResultType.JSON, ); From 1d0b51b7427a47fb480a97433cc4766e83e6ad67 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Wed, 24 Dec 2025 14:46:07 +0000 Subject: [PATCH 14/32] ui version bump --- ui/app/package-lock.json | 4 ++-- ui/app/package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ui/app/package-lock.json b/ui/app/package-lock.json index 8c4aafd216..56d958e735 100644 --- a/ui/app/package-lock.json +++ b/ui/app/package-lock.json @@ -1,12 +1,12 @@ { "name": "tre-ui", - "version": "0.8.20", + "version": "0.8.21", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "tre-ui", - "version": "0.8.20", + "version": "0.8.21", "dependencies": { "@azure/msal-browser": "^2.35.0", "@azure/msal-react": "^1.5.12", diff --git a/ui/app/package.json b/ui/app/package.json index 3400a0c5f3..2ed0b4bef2 100644 --- a/ui/app/package.json +++ b/ui/app/package.json @@ -1,6 +1,6 @@ { "name": "tre-ui", - "version": "0.8.20", + "version": "0.8.21", "private": true, "type": "module", "dependencies": { From 2ec1049c511e80759c6bd6ad196b529ce0e18f64 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Mon, 5 Jan 2026 11:40:14 +0000 Subject: [PATCH 15/32] bump version --- api_app/_version.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api_app/_version.py b/api_app/_version.py index 5461be1190..4f3cae6929 100644 --- a/api_app/_version.py +++ b/api_app/_version.py @@ -1 +1 @@ -__version__ = "0.25.7" +__version__ = "0.25.8" From d6b61c89a69a9664f5f6d29d91f0fb0bad6d3aa6 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Tue, 13 Jan 2026 17:30:05 +0000 Subject: [PATCH 16/32] fix: use correct auth fix: allow upgrades on hidden properties --- .../shared/ConfirmUpgradeResource.tsx | 59 +++++++++++++------ 1 file changed, 42 insertions(+), 17 deletions(-) diff --git a/ui/app/src/components/shared/ConfirmUpgradeResource.tsx b/ui/app/src/components/shared/ConfirmUpgradeResource.tsx index b2dbf2b4eb..ef0726b946 100644 --- a/ui/app/src/components/shared/ConfirmUpgradeResource.tsx +++ b/ui/app/src/components/shared/ConfirmUpgradeResource.tsx @@ -119,7 +119,8 @@ export const ConfirmUpgradeResource: React.FunctionComponent< const workspaceCtx = useContext(WorkspaceContext); const dispatch = useAppDispatch(); - const [newPropertiesToFill, setNewPropertiesToFill] = useState([]); + const [allNewProperties, setAllNewProperties] = useState([]); // All new properties including hidden ones + const [newPropertiesToFill, setNewPropertiesToFill] = useState([]); // Only visible properties const [newPropertyValues, setNewPropertyValues] = useState({}); const [loadingSchema, setLoadingSchema] = useState(false); const [newTemplateSchema, setNewTemplateSchema] = useState(null); @@ -148,13 +149,21 @@ export const ConfirmUpgradeResource: React.FunctionComponent< styles: dialogStyles, }; - const wsAuth = + // Template GET endpoints (templateGetPath) always use TRE API authentication, + // even for UserResource templates, because they use paths like: + // /workspace-service-templates/{name} (not /workspaces/{id}/...) + const templateUsesWsAuth = false; + + // However, the actual resource instance upgrade operation (PATCH) uses workspace auth + // for WorkspaceService and UserResource instances + const instanceUsesWsAuth = props.resource.resourceType === ResourceType.WorkspaceService || props.resource.resourceType === ResourceType.UserResource; // Fetch new template schema and identify new properties missing in current resource useEffect(() => { if (!selectedVersion) { + setAllNewProperties([]); setNewPropertiesToFill([]); setNewPropertyValues({}); setNewTemplateSchema(null); @@ -188,6 +197,7 @@ export const ConfirmUpgradeResource: React.FunctionComponent< .workspaceId; templateListPath = `${ApiEndpoint.Workspaces}/${workspaceId}/${ApiEndpoint.WorkspaceServiceTemplates}/${props.resource.properties.parentWorkspaceService.templateName}/${ApiEndpoint.UserResourceTemplates}`; templateGetPath = `${ApiEndpoint.WorkspaceServiceTemplates}/${props.resource.properties.parentWorkspaceService.templateName}/${ApiEndpoint.UserResourceTemplates}`; + // workspaceApplicationIdURI = props.resource.properties.parentWorkspaceService.workspaceApplicationIdURI; break; } else { throw Error( @@ -209,7 +219,7 @@ export const ConfirmUpgradeResource: React.FunctionComponent< const newTemplate = await apiCall( fetchUrl, HttpMethod.Get, - props.resource.resourceType === ResourceType.UserResource ? workspaceCtx.workspaceApplicationIdURI : undefined, + templateUsesWsAuth ? workspaceCtx.workspaceApplicationIdURI : undefined, undefined, ResultType.JSON, ); @@ -222,7 +232,7 @@ export const ConfirmUpgradeResource: React.FunctionComponent< currentTemplate = await apiCall( `${templateGetPath}/${props.resource.templateName}?version=${props.resource.templateVersion}`, HttpMethod.Get, - props.resource.resourceType === ResourceType.UserResource ? workspaceCtx.workspaceApplicationIdURI : undefined, + templateUsesWsAuth ? workspaceCtx.workspaceApplicationIdURI : undefined, undefined, ResultType.JSON, ); @@ -237,11 +247,23 @@ export const ConfirmUpgradeResource: React.FunctionComponent< const newKeys = getAllPropertyKeys(newSchemaProps); const currentKeys = getAllPropertyKeys(currentProps); - const newPropKeys = newKeys.filter((k) => !currentKeys.includes(k)); const removedPropsArray = currentKeys.filter((k) => !newKeys.includes(k)); - setNewPropertiesToFill(newPropKeys); + // Store all new properties (including hidden ones) for schema building + setAllNewProperties(newPropKeys); + + // Filter out properties that are hidden (tre-hidden) - they don't need user input + const uiSchema = newTemplate?.uiSchema || {}; + const visibleNewPropKeys = newPropKeys.filter((key) => { + const topKey = key.split('.')[0]; + const propertyUiSchema = uiSchema[topKey]; + // Check if property has "tre-hidden" in its classNames + const isHidden = propertyUiSchema?.classNames?.includes('tre-hidden'); + return !isHidden; + }); + + setNewPropertiesToFill(visibleNewPropKeys); setRemovedProperties(removedPropsArray); // prefill newPropertyValues with schema defaults or empty string @@ -277,7 +299,7 @@ export const ConfirmUpgradeResource: React.FunctionComponent< let op = await apiCall( props.resource.resourcePath, HttpMethod.Patch, - wsAuth ? workspaceCtx.workspaceApplicationIdURI : undefined, + instanceUsesWsAuth ? workspaceCtx.workspaceApplicationIdURI : undefined, body, ResultType.JSON, undefined, @@ -295,13 +317,14 @@ export const ConfirmUpgradeResource: React.FunctionComponent< } }; - // Use buildReducedSchema to include only new properties + // Use buildReducedSchema to include all new properties (including hidden ones) + // Hidden properties will be rendered but not shown due to tre-hidden CSS class const reducedSchemaProperties = newTemplateSchema - ? buildReducedSchema(newTemplateSchema, newPropertiesToFill) + ? buildReducedSchema(newTemplateSchema, allNewProperties) : null; - // Extract any conditional blocks from full schema, filtered by new properties - const conditionalBlocks = newTemplateSchema ? extractConditionalBlocks(newTemplateSchema, newPropertiesToFill) : {}; + // Extract any conditional blocks from full schema, filtered by all new properties + const conditionalBlocks = newTemplateSchema ? extractConditionalBlocks(newTemplateSchema, allNewProperties) : {}; // Compose final schema combining reduced properties with conditional blocks const finalSchema = reducedSchemaProperties @@ -371,11 +394,13 @@ export const ConfirmUpgradeResource: React.FunctionComponent< Warning: The following properties are no longer present in the template and will be removed: {removedProperties.join(', ')} )} - {!loadingSchema && newPropertiesToFill.length > 0 && ( + {!loadingSchema && allNewProperties.length > 0 && ( - - You must specify values for new properties: - + {newPropertiesToFill.length > 0 && ( + + You must specify values for new properties: + + )} {finalSchema && ( 0 && - Object.values(newPropertyValues).some( - (v) => v === "" || v === undefined, + newPropertiesToFill.some( + (key) => newPropertyValues[key] === "" || newPropertyValues[key] === undefined, )) } text="Upgrade" From aa48f114020270d0494f13a079b91d537678748a Mon Sep 17 00:00:00 2001 From: James Chapman Date: Wed, 14 Jan 2026 16:57:57 +0000 Subject: [PATCH 17/32] test: enhance ConfirmUpgradeResource tests for template fetching and property visibility --- .../shared/ConfirmUpgradeResource.test.tsx | 146 ++++++++++++++++++ .../shared/ConfirmUpgradeResource.tsx | 5 +- 2 files changed, 149 insertions(+), 2 deletions(-) diff --git a/ui/app/src/components/shared/ConfirmUpgradeResource.test.tsx b/ui/app/src/components/shared/ConfirmUpgradeResource.test.tsx index ac034b69e5..19cd081b69 100644 --- a/ui/app/src/components/shared/ConfirmUpgradeResource.test.tsx +++ b/ui/app/src/components/shared/ConfirmUpgradeResource.test.tsx @@ -701,4 +701,150 @@ describe("ConfirmUpgradeResource Component", () => { ); }); }); + + it("does not use workspace auth for template GET requests even for workspace services", async () => { + // Track all API calls + const apiCalls: any[] = []; + mockApiCall.mockImplementation((url, method, auth, ...rest) => { + apiCalls.push({ url, method, auth }); + if (method === "GET" && url.includes("?version=")) { + if (url.includes("version=1.0.0")) { + return Promise.resolve(mockCurrentTemplateSchema); + } else { + return Promise.resolve(mockNewTemplateSchema); + } + } + return Promise.resolve({ operation: { id: "operation-id", status: "running" } }); + }); + + renderWithWorkspaceContext( + + ); + + // Select a version to trigger template fetching + const dropdown = screen.getByTestId("dropdown"); + fireEvent.change(dropdown, { target: { value: "1.1.0" } }); + + // Wait for schema to load + await waitFor(() => { + expect( + screen.queryByText("Loading new template schema...") + ).not.toBeInTheDocument(); + }); + + // Verify that GET requests for templates did NOT use workspace auth + const getRequests = apiCalls.filter((call) => call.method === "GET" && call.url.includes("?version=")); + expect(getRequests.length).toBeGreaterThan(0); + getRequests.forEach((call) => { + expect(call.auth).toBeUndefined(); // Templates should not use workspace auth + }); + }); + + it("hides message and enables upgrade button when all new properties are hidden with tre-hidden", async () => { + const templateWithHiddenProperties = { + properties: { + display_name: { type: "string" }, + resource_key: { type: "string" }, + hidden_property: { type: "string", default: "hidden_value" }, + }, + required: ["display_name"], + uiSchema: { + hidden_property: { + classNames: "tre-hidden", + }, + }, + }; + + mockApiCall.mockImplementation((url, method) => { + if (method === "GET" && url.includes("?version=")) { + if (url.includes("version=1.0.0")) { + return Promise.resolve(mockCurrentTemplateSchema); + } else { + return Promise.resolve(templateWithHiddenProperties); + } + } + return Promise.resolve({ operation: { id: "operation-id", status: "running" } }); + }); + + renderWithWorkspaceContext( + + ); + + // Select a version + const dropdown = screen.getByTestId("dropdown"); + fireEvent.change(dropdown, { target: { value: "1.1.0" } }); + + // Wait for schema to load + await waitFor(() => { + expect( + screen.queryByText("Loading new template schema...") + ).not.toBeInTheDocument(); + }); + + // Should NOT show the "You must specify values" message because all properties are hidden + expect(screen.queryByText("You must specify values for new properties:")).not.toBeInTheDocument(); + + // Button should be enabled immediately + const upgradeButton = screen.getByTestId("primary-button"); + expect(upgradeButton).not.toBeDisabled(); + }); + + it("shows message and validates only visible properties when mix of visible and hidden properties", async () => { + const templateWithMixedProperties = { + properties: { + display_name: { type: "string" }, + resource_key: { type: "string" }, + visible_property: { type: "string" }, + hidden_property: { type: "string", default: "hidden_value" }, + }, + required: ["display_name"], + uiSchema: { + hidden_property: { + classNames: "tre-hidden", + }, + }, + }; + + mockApiCall.mockImplementation((url, method) => { + if (method === "GET" && url.includes("?version=")) { + if (url.includes("version=1.0.0")) { + return Promise.resolve(mockCurrentTemplateSchema); + } else { + return Promise.resolve(templateWithMixedProperties); + } + } + return Promise.resolve({ operation: { id: "operation-id", status: "running" } }); + }); + + renderWithWorkspaceContext( + + ); + + // Select a version + const dropdown = screen.getByTestId("dropdown"); + fireEvent.change(dropdown, { target: { value: "1.1.0" } }); + + // Wait for schema to load + await waitFor(() => { + expect( + screen.queryByText("Loading new template schema...") + ).not.toBeInTheDocument(); + }); + + // Should show the message because there's at least one visible property + expect(screen.getByText("You must specify values for new properties:")).toBeInTheDocument(); + + // Button should be disabled because visible_property is empty + const upgradeButton = screen.getByTestId("primary-button"); + expect(upgradeButton).toBeDisabled(); + }); }); diff --git a/ui/app/src/components/shared/ConfirmUpgradeResource.tsx b/ui/app/src/components/shared/ConfirmUpgradeResource.tsx index ef0726b946..dda6f79ae8 100644 --- a/ui/app/src/components/shared/ConfirmUpgradeResource.tsx +++ b/ui/app/src/components/shared/ConfirmUpgradeResource.tsx @@ -258,8 +258,9 @@ export const ConfirmUpgradeResource: React.FunctionComponent< const visibleNewPropKeys = newPropKeys.filter((key) => { const topKey = key.split('.')[0]; const propertyUiSchema = uiSchema[topKey]; - // Check if property has "tre-hidden" in its classNames - const isHidden = propertyUiSchema?.classNames?.includes('tre-hidden'); + // Check if property has "tre-hidden" in its classNames (support both old and new format) + const classNames = propertyUiSchema?.classNames || propertyUiSchema?.['ui:classNames']; + const isHidden = classNames?.includes('tre-hidden'); return !isHidden; }); From 62af5907fe300265fbadb3178fbc5a15a7544336 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Thu, 15 Jan 2026 15:59:54 +0000 Subject: [PATCH 18/32] update test --- .../src/components/shared/ConfirmUpgradeResource.test.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ui/app/src/components/shared/ConfirmUpgradeResource.test.tsx b/ui/app/src/components/shared/ConfirmUpgradeResource.test.tsx index 19cd081b69..1617ffb32d 100644 --- a/ui/app/src/components/shared/ConfirmUpgradeResource.test.tsx +++ b/ui/app/src/components/shared/ConfirmUpgradeResource.test.tsx @@ -1,6 +1,6 @@ import React from "react"; import { describe, it, expect, vi, beforeEach } from "vitest"; -import { render, screen, fireEvent, waitFor, createPartialFluentUIMock } from "../../test-utils"; +import { act, render, screen, fireEvent, waitFor, createPartialFluentUIMock } from "../../test-utils"; import { ConfirmUpgradeResource } from "./ConfirmUpgradeResource"; import { Resource, AvailableUpgrade } from "../../models/resource"; import { ResourceType } from "../../models/resourceType"; @@ -384,7 +384,9 @@ describe("ConfirmUpgradeResource Component", () => { // Click upgrade const upgradeButton = screen.getByTestId("primary-button"); - fireEvent.click(upgradeButton); + await act(async () => { + fireEvent.click(upgradeButton); + }); await waitFor(() => { expect(screen.getByTestId("exception-layout")).toBeInTheDocument(); From fdf358900e3d83e54787e929b7e8496e6a5094cc Mon Sep 17 00:00:00 2001 From: James Chapman Date: Fri, 16 Jan 2026 14:21:52 +0000 Subject: [PATCH 19/32] add method to extract pipeline properties for resource validation --- api_app/db/repositories/resources.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/api_app/db/repositories/resources.py b/api_app/db/repositories/resources.py index bbf3f30eba..a5fb29d933 100644 --- a/api_app/db/repositories/resources.py +++ b/api_app/db/repositories/resources.py @@ -203,6 +203,18 @@ async def validate_template_version_patch(self, resource: Resource, resource_pat except EntityDoesNotExist: raise TargetTemplateVersionDoesNotExist(f"Template '{resource_template.name}' not found for resource type '{resource_template.resourceType}' with target template version '{resource_patch.templateVersion}'") + def _get_pipeline_properties(self, enriched_template) -> List[str]: + properties = [] + pipeline = enriched_template.get("pipeline") + if pipeline: + for phase in ["install", "upgrade"]: + if phase in pipeline and pipeline[phase]: + for step in pipeline[phase]: + if "properties" in step and step["properties"]: + for prop in step["properties"]: + properties.append(prop["name"]) + return properties + async def validate_patch(self, resource_patch: ResourcePatch, resource_template_repo: ResourceTemplateRepository, resource_template: ResourceTemplate, resource_action: str): # get the enriched (combined) template enriched_template = resource_template_repo.enrich_template(resource_template, is_update=True) @@ -220,11 +232,15 @@ async def validate_patch(self, resource_patch: ResourcePatch, resource_template_ update_template = copy.deepcopy(enriched_template) update_template["required"] = [] update_template["properties"] = {} + + pipeline_properties = self._get_pipeline_properties(enriched_template) + for prop_name, prop in enriched_template["properties"].items(): # Allow property if: # 1. Installing (all properties allowed) - # 2. Property is explicitly updateable + # 2. Property is explicitly updateable (updateable: true in template) # 3. Upgrading and the property is NEW (not in old template) and being added in this patch + # 4. Property is set using a parent property using {{ resource.parent.properties.my_parent_property }} if ( resource_action == RESOURCE_ACTION_INSTALL or prop.get("updateable", False) is True @@ -235,6 +251,7 @@ async def validate_patch(self, resource_patch: ResourcePatch, resource_template_ and prop_name in resource_patch.properties and prop_name not in old_template_properties ) + or prop_name in pipeline_properties ): update_template["properties"][prop_name] = prop From e85b25c37de14b3f80fed40f0c39644e79477468 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Fri, 16 Jan 2026 14:22:48 +0000 Subject: [PATCH 20/32] add tests --- .../test_resource_repository.py | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/api_app/tests_ma/test_db/test_repositories/test_resource_repository.py b/api_app/tests_ma/test_db/test_repositories/test_resource_repository.py index e734b36f33..42f01176bf 100644 --- a/api_app/tests_ma/test_db/test_repositories/test_resource_repository.py +++ b/api_app/tests_ma/test_db/test_repositories/test_resource_repository.py @@ -569,3 +569,73 @@ async def test_validate_patch_allows_mix_of_new_and_updateable_properties_during # This should NOT raise a ValidationError await resource_repo.validate_patch(patch, template_repo, parse_obj_as(ResourceTemplate, old_template), strings.RESOURCE_ACTION_UPDATE) + + +@pytest.mark.asyncio +@patch('db.repositories.resources.ResourceTemplateRepository.enrich_template') +async def test_validate_patch_allows_install_pipeline_property(template_repo, resource_repo): + """ + Make sure that patch is valid when a property is present in install pipeline substitution + even if it is not updateable in the main properties. + """ + + template_dict = sample_resource_template() + # Add a pipeline with substitution for 'my_inherited_property' + # And add 'my_inherited_property' to properties as not updateable + template_dict['properties']['my_inherited_property'] = { + 'type': 'string', + 'updateable': False + } + template_dict['pipeline'] = { + 'upgrade': [ + { + 'stepId': 'main', + 'properties': [ + {'name': 'my_inherited_property', 'value': '{{ resource.parent.properties.my_inherited_property }}', 'type': 'string'} + ] + } + ] + } + + template_repo.enrich_template = MagicMock(return_value=template_dict) + template = parse_obj_as(ResourceTemplate, template_dict) + + # Patch attempting to send my_inherited_property. + # This simulates the UI sending the full object back, including the substituted value. + patch = ResourcePatch(isEnabled=True, properties={'my_inherited_property': ''}) + + # This should pass validation because my_inherited_property is in the pipeline + await resource_repo.validate_patch(patch, template_repo, template, strings.RESOURCE_ACTION_UPDATE) + + +@pytest.mark.asyncio +@patch('db.repositories.resources.ResourceTemplateRepository.enrich_template') +async def test_validate_patch_allows_upgrade_pipeline_property(template_repo, resource_repo): + """ + Make sure that patch is valid when a property is present in upgrade pipeline substitution + even if it is not updateable in the main properties. + """ + + template_dict = sample_resource_template() + template_dict['properties']['my_inherited_property'] = { + 'type': 'string', + 'updateable': False + } + template_dict['pipeline'] = { + 'upgrade': [ + { + 'stepId': 'main', + 'properties': [ + {'name': 'my_inherited_property', 'value': '{{ resource.parent.properties.my_inherited_property }}', 'type': 'string'} + ] + } + ] + } + + template_repo.enrich_template = MagicMock(return_value=template_dict) + template = parse_obj_as(ResourceTemplate, template_dict) + + patch = ResourcePatch(isEnabled=True, properties={'my_inherited_property': ''}) + + # This should pass validation because my_inherited_property is in the upgrade pipeline + await resource_repo.validate_patch(patch, template_repo, template, strings.RESOURCE_ACTION_UPDATE) From 9f733ab2421528f2978b86a2f3d1ba83f54ed425 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Fri, 16 Jan 2026 14:23:16 +0000 Subject: [PATCH 21/32] update api version --- api_app/_version.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api_app/_version.py b/api_app/_version.py index 4f3cae6929..4f36e25fb3 100644 --- a/api_app/_version.py +++ b/api_app/_version.py @@ -1 +1 @@ -__version__ = "0.25.8" +__version__ = "0.25.10" From 710fb3ccc472a4c1e7ef3f2ed2ec540c3a591a7d Mon Sep 17 00:00:00 2001 From: James Chapman <196318169+JC-wk@users.noreply.github.com> Date: Tue, 20 Jan 2026 12:26:33 +0000 Subject: [PATCH 22/32] Add ESLint config file paths for validation --- .github/workflows/build_validation_develop.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/build_validation_develop.yml b/.github/workflows/build_validation_develop.yml index 21276a47d7..03f63760bc 100644 --- a/.github/workflows/build_validation_develop.yml +++ b/.github/workflows/build_validation_develop.yml @@ -102,6 +102,8 @@ jobs: VALIDATE_DOCKERFILE_HADOLINT: true VALIDATE_TSX: true VALIDATE_TYPESCRIPT_ES: true + TYPESCRIPT_ES_CONFIG_FILE: ui/app/eslint.config.js + TSX_CONFIG_FILE: ui/app/eslint.config.js - name: Docs validation if: ${{ steps.filter.outputs.docs == 'true' }} From ef84720a6ee93ec0d2bf3813ec87e0beae9134aa Mon Sep 17 00:00:00 2001 From: James Chapman Date: Tue, 27 Jan 2026 16:12:57 +0000 Subject: [PATCH 23/32] allow property substitution without an update to the parent workspace in the pipeline --- api_app/service_bus/helpers.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/api_app/service_bus/helpers.py b/api_app/service_bus/helpers.py index 65acff1e49..56ff47a724 100644 --- a/api_app/service_bus/helpers.py +++ b/api_app/service_bus/helpers.py @@ -76,9 +76,15 @@ async def update_resource_for_step(operation_step: OperationStep, resource_repo: pipeline_primary_action = parent_template_pipeline_dict[primary_action] is_first_main_step = pipeline_primary_action and len(pipeline_primary_action) == 1 and pipeline_primary_action[0]['stepId'] == 'main' - if not pipeline_primary_action or is_first_main_step: + + if not pipeline_primary_action: return step_resource + if is_first_main_step: + single_step = pipeline_primary_action[0] + if not single_step.get('properties'): + return step_resource + # get the template step template_step = None for step in parent_template_pipeline_dict[primary_action]: From 7b9025ada5eadfe1833f395774ea445710b0c235 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Tue, 27 Jan 2026 16:16:59 +0000 Subject: [PATCH 24/32] use .allOf property --- api_app/db/repositories/resources.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api_app/db/repositories/resources.py b/api_app/db/repositories/resources.py index a5fb29d933..2d16ed0f1d 100644 --- a/api_app/db/repositories/resources.py +++ b/api_app/db/repositories/resources.py @@ -113,7 +113,7 @@ async def validate_input_against_template(self, template_name: str, resource_inp def _get_all_property_keys_from_template(self, resource_template: ResourceTemplate) -> set: properties = set(resource_template.properties.keys()) - if "allOf" in resource_template: + if resource_template.allOf: for condition in resource_template.allOf: if "then" in condition and "properties" in condition["then"]: properties.update(condition["then"]["properties"].keys()) From dbaef801c812a0aa994075f1accd9f9aae3b1a25 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Tue, 3 Feb 2026 11:27:59 +0000 Subject: [PATCH 25/32] update versions --- api_app/_version.py | 2 +- ui/app/package-lock.json | 4 ++-- ui/app/package.json | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/api_app/_version.py b/api_app/_version.py index e6e3369ba2..6623c5202f 100644 --- a/api_app/_version.py +++ b/api_app/_version.py @@ -1 +1 @@ -__version__ = "0.25.13" +__version__ = "0.25.14" diff --git a/ui/app/package-lock.json b/ui/app/package-lock.json index 31f47e8ad6..dda864d67c 100644 --- a/ui/app/package-lock.json +++ b/ui/app/package-lock.json @@ -1,12 +1,12 @@ { "name": "tre-ui", - "version": "0.8.24", + "version": "0.8.26", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "tre-ui", - "version": "0.8.24", + "version": "0.8.26", "dependencies": { "@azure/msal-browser": "^2.35.0", "@azure/msal-react": "^1.5.12", diff --git a/ui/app/package.json b/ui/app/package.json index c13d6de91f..8f6aa8f146 100644 --- a/ui/app/package.json +++ b/ui/app/package.json @@ -1,6 +1,6 @@ { "name": "tre-ui", - "version": "0.8.25", + "version": "0.8.26", "private": true, "type": "module", "dependencies": { From 47c941d71dd2d3262d8f0c03125e6a6a7a1f127a Mon Sep 17 00:00:00 2001 From: James Chapman Date: Tue, 3 Feb 2026 11:28:28 +0000 Subject: [PATCH 26/32] filter out properties set via pipeliine --- .../shared/ConfirmUpgradeResource.tsx | 32 ++++++++++++++++--- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/ui/app/src/components/shared/ConfirmUpgradeResource.tsx b/ui/app/src/components/shared/ConfirmUpgradeResource.tsx index dda6f79ae8..ca99e981b9 100644 --- a/ui/app/src/components/shared/ConfirmUpgradeResource.tsx +++ b/ui/app/src/components/shared/ConfirmUpgradeResource.tsx @@ -250,12 +250,27 @@ export const ConfirmUpgradeResource: React.FunctionComponent< const newPropKeys = newKeys.filter((k) => !currentKeys.includes(k)); const removedPropsArray = currentKeys.filter((k) => !newKeys.includes(k)); - // Store all new properties (including hidden ones) for schema building - setAllNewProperties(newPropKeys); + // Get properties defined in pipeline upgrade steps - these should NOT be sent by UI + const pipelineProps = new Set(); + if (newTemplate?.pipeline?.upgrade) { + newTemplate.pipeline.upgrade.forEach((step: any) => { + if (step.properties) { + step.properties.forEach((prop: any) => { + pipelineProps.add(prop.name); + }); + } + }); + } + + // Filter out properties that are in the pipeline - they will be substituted by the backend + const newPropKeysWithoutPipeline = newPropKeys.filter((key) => { + const topKey = key.split('.')[0]; + return !pipelineProps.has(topKey); + }); // Filter out properties that are hidden (tre-hidden) - they don't need user input const uiSchema = newTemplate?.uiSchema || {}; - const visibleNewPropKeys = newPropKeys.filter((key) => { + const visibleNewPropKeys = newPropKeysWithoutPipeline.filter((key) => { const topKey = key.split('.')[0]; const propertyUiSchema = uiSchema[topKey]; // Check if property has "tre-hidden" in its classNames (support both old and new format) @@ -267,9 +282,16 @@ export const ConfirmUpgradeResource: React.FunctionComponent< setNewPropertiesToFill(visibleNewPropKeys); setRemovedProperties(removedPropsArray); - // prefill newPropertyValues with schema defaults or empty string + // Include ALL new properties not in pipeline to be sent to API + // This ensures hidden properties with defaults are correctly passed + const newPropKeysToSend = newPropKeysWithoutPipeline; + + // Set allNewProperties to the filtered list (for schema building) + setAllNewProperties(newPropKeysToSend); + + // prefill newPropertyValues with schema defaults or empty string (excluding pipeline properties) setNewPropertyValues( - newPropKeys.reduce((acc, key) => { + newPropKeysToSend.reduce((acc, key) => { // Get top-level portion of the key const topKey = key.split('.')[0]; const defaultValue = newTemplate?.properties?.[topKey]?.default; From 9b37fe6f7aee8858134f66ce4f938f311bf104e5 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Tue, 3 Feb 2026 13:03:26 +0000 Subject: [PATCH 27/32] revert allOf change --- api_app/db/repositories/resources.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api_app/db/repositories/resources.py b/api_app/db/repositories/resources.py index 2d16ed0f1d..a5fb29d933 100644 --- a/api_app/db/repositories/resources.py +++ b/api_app/db/repositories/resources.py @@ -113,7 +113,7 @@ async def validate_input_against_template(self, template_name: str, resource_inp def _get_all_property_keys_from_template(self, resource_template: ResourceTemplate) -> set: properties = set(resource_template.properties.keys()) - if resource_template.allOf: + if "allOf" in resource_template: for condition in resource_template.allOf: if "then" in condition and "properties" in condition["then"]: properties.update(condition["then"]["properties"].keys()) From 675418b005a2bbf82c1bcbbb6e387ac1dacd101e Mon Sep 17 00:00:00 2001 From: James Chapman Date: Tue, 3 Feb 2026 13:05:53 +0000 Subject: [PATCH 28/32] revert tests --- .../test_routes/test_shared_services.py | 28 +++++++++++++++++-- .../test_api/test_routes/test_workspaces.py | 4 +-- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/api_app/tests_ma/test_api/test_routes/test_shared_services.py b/api_app/tests_ma/test_api/test_routes/test_shared_services.py index 2d0f6e3965..62d3b02f6c 100644 --- a/api_app/tests_ma/test_api/test_routes/test_shared_services.py +++ b/api_app/tests_ma/test_api/test_routes/test_shared_services.py @@ -5,7 +5,8 @@ from mock import patch from fastapi import status -from models.domain.resource import ResourceHistoryItem +from models.domain.resource import ResourceHistoryItem, ResourceType +from models.domain.resource_template import ResourceTemplate from tests_ma.test_api.conftest import create_admin_user, create_test_user from .test_workspaces import FAKE_CREATE_TIMESTAMP, FAKE_UPDATE_TIMESTAMP, OPERATION_ID, sample_resource_operation @@ -52,6 +53,27 @@ def sample_shared_service(shared_service_id=SHARED_SERVICE_ID): ) +def sample_resource_template() -> ResourceTemplate: + return ResourceTemplate(id="123", + name="tre-shared-service-base", + description="description", + version="0.1.0", + resourceType=ResourceType.SharedService, + current=True, + required=['display_name', 'description'], + properties={ + 'display_name': { + 'type': 'string', + 'title': 'Title of the resource' + }, + 'description': { + 'type': 'string', + 'title': 'Description of the resource' + }, + }, + actions=[]) + + def sample_resource_history(history_length, shared_service_id=SHARED_SERVICE_ID) -> ResourceHistoryItem: resource_history = [] user = create_test_user() @@ -222,7 +244,7 @@ async def test_patch_shared_service_patches_shared_service(self, _, update_item_ @patch("api.routes.shared_services.ResourceHistoryRepository.save_item", return_value=AsyncMock()) @patch("api.routes.shared_services.SharedServiceRepository.get_timestamp", return_value=FAKE_UPDATE_TIMESTAMP) @patch("api.dependencies.shared_services.SharedServiceRepository.get_shared_service_by_id", return_value=sample_shared_service(SHARED_SERVICE_ID)) - @patch("api.routes.shared_services.ResourceTemplateRepository.get_template_by_name_and_version", return_value=sample_shared_service()) + @patch("api.routes.shared_services.ResourceTemplateRepository.get_template_by_name_and_version", return_value=sample_resource_template()) @patch("api.routes.shared_services.SharedServiceRepository.update_item_with_etag", return_value=sample_shared_service()) @patch("api.routes.shared_services.send_resource_request_message", return_value=sample_resource_operation(resource_id=SHARED_SERVICE_ID, operation_id=OPERATION_ID)) async def test_patch_shared_service_with_upgrade_minor_version_patches_shared_service(self, _, update_item_mock, __, ___, ____, _____, app, client): @@ -244,7 +266,7 @@ async def test_patch_shared_service_with_upgrade_minor_version_patches_shared_se @patch("api.routes.shared_services.ResourceHistoryRepository.save_item", return_value=AsyncMock()) @patch("api.routes.shared_services.SharedServiceRepository.get_timestamp", return_value=FAKE_UPDATE_TIMESTAMP) @patch("api.dependencies.shared_services.SharedServiceRepository.get_shared_service_by_id", return_value=sample_shared_service(SHARED_SERVICE_ID)) - @patch("api.routes.shared_services.ResourceTemplateRepository.get_template_by_name_and_version", return_value=sample_shared_service()) + @patch("api.routes.shared_services.ResourceTemplateRepository.get_template_by_name_and_version", return_value=sample_resource_template()) @patch("api.routes.shared_services.SharedServiceRepository.update_item_with_etag", return_value=sample_shared_service()) @patch("api.routes.shared_services.send_resource_request_message", return_value=sample_resource_operation(resource_id=SHARED_SERVICE_ID, operation_id=OPERATION_ID)) async def test_patch_shared_service_with_upgrade_major_version_and_force_update_patches_shared_service(self, _, update_item_mock, __, ___, ____, _____, app, client): diff --git a/api_app/tests_ma/test_api/test_routes/test_workspaces.py b/api_app/tests_ma/test_api/test_routes/test_workspaces.py index 5e97af84f2..a1aa13ecd7 100644 --- a/api_app/tests_ma/test_api/test_routes/test_workspaces.py +++ b/api_app/tests_ma/test_api/test_routes/test_workspaces.py @@ -564,7 +564,7 @@ async def test_patch_workspaces_with_upgrade_major_version_returns_bad_request(s @patch("api.routes.workspaces.send_resource_request_message", return_value=sample_resource_operation(resource_id=WORKSPACE_ID, operation_id=OPERATION_ID)) @patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id", return_value=sample_workspace()) @patch("api.routes.workspaces.WorkspaceRepository.update_item_with_etag", return_value=sample_workspace()) - @patch("api.routes.workspaces.ResourceTemplateRepository.get_template_by_name_and_version", return_value=sample_workspace()) + @patch("api.routes.workspaces.ResourceTemplateRepository.get_template_by_name_and_version", return_value=sample_resource_template()) @patch("api.routes.workspaces.WorkspaceRepository.get_timestamp", return_value=FAKE_UPDATE_TIMESTAMP) async def test_patch_workspaces_with_upgrade_major_version_and_force_update_returns_patched_workspace(self, _, __, update_item_mock, ___, ____, _____, ______, app, client): workspace_patch = {"templateVersion": "2.0.0"} @@ -611,7 +611,7 @@ async def test_patch_workspaces_with_downgrade_version_returns_bad_request(self, @patch("api.routes.workspaces.send_resource_request_message", return_value=sample_resource_operation(resource_id=WORKSPACE_ID, operation_id=OPERATION_ID)) @patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id", return_value=sample_workspace()) @patch("api.routes.workspaces.WorkspaceRepository.update_item_with_etag", return_value=sample_workspace()) - @patch("api.routes.workspaces.ResourceTemplateRepository.get_template_by_name_and_version", return_value=sample_workspace()) + @patch("api.routes.workspaces.ResourceTemplateRepository.get_template_by_name_and_version", return_value=sample_resource_template()) @patch("api.routes.workspaces.WorkspaceRepository.get_timestamp", return_value=FAKE_UPDATE_TIMESTAMP) async def test_patch_workspaces_with_upgrade_minor_version_patches_workspace(self, _, __, update_item_mock, ___, ____, _____, ______, app, client): workspace_patch = {"templateVersion": "0.2.0"} From 9151b266064190701925148cfe04078632a136b4 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Tue, 3 Feb 2026 13:06:59 +0000 Subject: [PATCH 29/32] correctly revert tests --- .../test_routes/test_shared_services.py | 28 ++----------------- .../test_api/test_routes/test_workspaces.py | 4 +-- 2 files changed, 5 insertions(+), 27 deletions(-) diff --git a/api_app/tests_ma/test_api/test_routes/test_shared_services.py b/api_app/tests_ma/test_api/test_routes/test_shared_services.py index 62d3b02f6c..2d0f6e3965 100644 --- a/api_app/tests_ma/test_api/test_routes/test_shared_services.py +++ b/api_app/tests_ma/test_api/test_routes/test_shared_services.py @@ -5,8 +5,7 @@ from mock import patch from fastapi import status -from models.domain.resource import ResourceHistoryItem, ResourceType -from models.domain.resource_template import ResourceTemplate +from models.domain.resource import ResourceHistoryItem from tests_ma.test_api.conftest import create_admin_user, create_test_user from .test_workspaces import FAKE_CREATE_TIMESTAMP, FAKE_UPDATE_TIMESTAMP, OPERATION_ID, sample_resource_operation @@ -53,27 +52,6 @@ def sample_shared_service(shared_service_id=SHARED_SERVICE_ID): ) -def sample_resource_template() -> ResourceTemplate: - return ResourceTemplate(id="123", - name="tre-shared-service-base", - description="description", - version="0.1.0", - resourceType=ResourceType.SharedService, - current=True, - required=['display_name', 'description'], - properties={ - 'display_name': { - 'type': 'string', - 'title': 'Title of the resource' - }, - 'description': { - 'type': 'string', - 'title': 'Description of the resource' - }, - }, - actions=[]) - - def sample_resource_history(history_length, shared_service_id=SHARED_SERVICE_ID) -> ResourceHistoryItem: resource_history = [] user = create_test_user() @@ -244,7 +222,7 @@ async def test_patch_shared_service_patches_shared_service(self, _, update_item_ @patch("api.routes.shared_services.ResourceHistoryRepository.save_item", return_value=AsyncMock()) @patch("api.routes.shared_services.SharedServiceRepository.get_timestamp", return_value=FAKE_UPDATE_TIMESTAMP) @patch("api.dependencies.shared_services.SharedServiceRepository.get_shared_service_by_id", return_value=sample_shared_service(SHARED_SERVICE_ID)) - @patch("api.routes.shared_services.ResourceTemplateRepository.get_template_by_name_and_version", return_value=sample_resource_template()) + @patch("api.routes.shared_services.ResourceTemplateRepository.get_template_by_name_and_version", return_value=sample_shared_service()) @patch("api.routes.shared_services.SharedServiceRepository.update_item_with_etag", return_value=sample_shared_service()) @patch("api.routes.shared_services.send_resource_request_message", return_value=sample_resource_operation(resource_id=SHARED_SERVICE_ID, operation_id=OPERATION_ID)) async def test_patch_shared_service_with_upgrade_minor_version_patches_shared_service(self, _, update_item_mock, __, ___, ____, _____, app, client): @@ -266,7 +244,7 @@ async def test_patch_shared_service_with_upgrade_minor_version_patches_shared_se @patch("api.routes.shared_services.ResourceHistoryRepository.save_item", return_value=AsyncMock()) @patch("api.routes.shared_services.SharedServiceRepository.get_timestamp", return_value=FAKE_UPDATE_TIMESTAMP) @patch("api.dependencies.shared_services.SharedServiceRepository.get_shared_service_by_id", return_value=sample_shared_service(SHARED_SERVICE_ID)) - @patch("api.routes.shared_services.ResourceTemplateRepository.get_template_by_name_and_version", return_value=sample_resource_template()) + @patch("api.routes.shared_services.ResourceTemplateRepository.get_template_by_name_and_version", return_value=sample_shared_service()) @patch("api.routes.shared_services.SharedServiceRepository.update_item_with_etag", return_value=sample_shared_service()) @patch("api.routes.shared_services.send_resource_request_message", return_value=sample_resource_operation(resource_id=SHARED_SERVICE_ID, operation_id=OPERATION_ID)) async def test_patch_shared_service_with_upgrade_major_version_and_force_update_patches_shared_service(self, _, update_item_mock, __, ___, ____, _____, app, client): diff --git a/api_app/tests_ma/test_api/test_routes/test_workspaces.py b/api_app/tests_ma/test_api/test_routes/test_workspaces.py index a1aa13ecd7..5e97af84f2 100644 --- a/api_app/tests_ma/test_api/test_routes/test_workspaces.py +++ b/api_app/tests_ma/test_api/test_routes/test_workspaces.py @@ -564,7 +564,7 @@ async def test_patch_workspaces_with_upgrade_major_version_returns_bad_request(s @patch("api.routes.workspaces.send_resource_request_message", return_value=sample_resource_operation(resource_id=WORKSPACE_ID, operation_id=OPERATION_ID)) @patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id", return_value=sample_workspace()) @patch("api.routes.workspaces.WorkspaceRepository.update_item_with_etag", return_value=sample_workspace()) - @patch("api.routes.workspaces.ResourceTemplateRepository.get_template_by_name_and_version", return_value=sample_resource_template()) + @patch("api.routes.workspaces.ResourceTemplateRepository.get_template_by_name_and_version", return_value=sample_workspace()) @patch("api.routes.workspaces.WorkspaceRepository.get_timestamp", return_value=FAKE_UPDATE_TIMESTAMP) async def test_patch_workspaces_with_upgrade_major_version_and_force_update_returns_patched_workspace(self, _, __, update_item_mock, ___, ____, _____, ______, app, client): workspace_patch = {"templateVersion": "2.0.0"} @@ -611,7 +611,7 @@ async def test_patch_workspaces_with_downgrade_version_returns_bad_request(self, @patch("api.routes.workspaces.send_resource_request_message", return_value=sample_resource_operation(resource_id=WORKSPACE_ID, operation_id=OPERATION_ID)) @patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id", return_value=sample_workspace()) @patch("api.routes.workspaces.WorkspaceRepository.update_item_with_etag", return_value=sample_workspace()) - @patch("api.routes.workspaces.ResourceTemplateRepository.get_template_by_name_and_version", return_value=sample_resource_template()) + @patch("api.routes.workspaces.ResourceTemplateRepository.get_template_by_name_and_version", return_value=sample_workspace()) @patch("api.routes.workspaces.WorkspaceRepository.get_timestamp", return_value=FAKE_UPDATE_TIMESTAMP) async def test_patch_workspaces_with_upgrade_minor_version_patches_workspace(self, _, __, update_item_mock, ___, ____, _____, ______, app, client): workspace_patch = {"templateVersion": "0.2.0"} From 4c9235eef574d21799e7cf324d1ab806be0c06ab Mon Sep 17 00:00:00 2001 From: James Chapman Date: Fri, 6 Feb 2026 17:12:15 +0000 Subject: [PATCH 30/32] changelog --- CHANGELOG.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e4cafcdba..f38fcdc084 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ * _No changes yet_ ENHANCEMENTS: -* _No changes yet_ +* Allow new template properties to be specified during template upgrades. Remove Template properties that no longer exist. BUG FIXES: * Fix property substitution not occuring where there is only a main step in the pipeline ([#4824](https://github.com/microsoft/AzureTRE/issues/4824)) @@ -24,7 +24,6 @@ ENHANCEMENTS: * API: Replace HTTP_422_UNPROCESSABLE_ENTITY response with HTTP_422_UNPROCESSABLE_CONTENT as per RFC 9110 ([#4742](https://github.com/microsoft/AzureTRE/issues/4742)) * Change Group.ReadWrite.All permission to Group.Create for AUTO_WORKSPACE_GROUP_CREATION ([#4772](https://github.com/microsoft/AzureTRE/issues/4772)) * Make workspace shared storage quota updateable ([#4314](https://github.com/microsoft/AzureTRE/issues/4314)) -* Allow new template properties to be specified during template upgrades. Remove Template properties that no longer exist. * Implement UI testing with vitest ([#4794](https://github.com/microsoft/AzureTRE/pull/4794)) * Update Porter, AzureCLI, Terraform and its providers across the solution ([#4799](https://github.com/microsoft/AzureTRE/issues/4799)) * Update `api_healthcheck.sh` script with fixed 10-second check intervals and 7-minute timeout for improved API health monitoring ([#4807](https://github.com/microsoft/AzureTRE/issues/4807)) From 519d21f23fd609a370efe29f76a379091fade665 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Fri, 6 Feb 2026 17:12:43 +0000 Subject: [PATCH 31/32] api ver --- api_app/_version.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api_app/_version.py b/api_app/_version.py index 6623c5202f..2cb28789f2 100644 --- a/api_app/_version.py +++ b/api_app/_version.py @@ -1 +1 @@ -__version__ = "0.25.14" +__version__ = "0.25.15" From a58e20946ac966d01b866d185f35f7d873e6a392 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Fri, 6 Feb 2026 17:16:36 +0000 Subject: [PATCH 32/32] fix lint --- .github/linters/.tflint_workspaces.hcl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/linters/.tflint_workspaces.hcl b/.github/linters/.tflint_workspaces.hcl index bfb0c85e20..86862b463b 100644 --- a/.github/linters/.tflint_workspaces.hcl +++ b/.github/linters/.tflint_workspaces.hcl @@ -7,6 +7,8 @@ config { plugin "azurerm" { enabled = true + version = "0.30.0" + source = "github.com/terraform-linters/tflint-ruleset-azurerm" } rule "azurerm_resource_missing_tags" {