diff --git a/.github/linters/.tflint_workspaces.hcl b/.github/linters/.tflint_workspaces.hcl index bfb0c85e2..86862b463 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" { diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e6ae40c4..f38fcdc08 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)) diff --git a/api_app/_version.py b/api_app/_version.py index 6623c5202..2cb28789f 100644 --- a/api_app/_version.py +++ b/api_app/_version.py @@ -1 +1 @@ -__version__ = "0.25.14" +__version__ = "0.25.15" diff --git a/api_app/db/repositories/resources.py b/api_app/db/repositories/resources.py index efa8af841..a5fb29d93 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 "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()) + 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,10 +133,20 @@ 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: - 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 +203,56 @@ 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): + 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) - # validate the PATCH data against a cut down version of the full template. + # 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 + 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"] = {} + + pipeline_properties = self._get_pipeline_properties(enriched_template) + 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: + # 1. Installing (all properties allowed) + # 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 + 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 + ) + or prop_name in pipeline_properties + ): update_template["properties"][prop_name] = prop self._validate_resource_parameters(resource_patch.dict(), update_template) 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 60b8569c5..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 @@ -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,191 @@ 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 (new install) + patch = ResourcePatch(isEnabled=True, properties={'vm_size': 'huge'}) + with pytest.raises(ValidationError): + await resource_repo.validate_patch(patch, template_repo, template, strings.RESOURCE_ACTION_INSTALL) - # check it's invalid when sending a bad value + # check it's invalid when sending a bad value (update) 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_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) + + +@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) diff --git a/ui/app/package-lock.json b/ui/app/package-lock.json index 31f47e8ad..dda864d67 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 c13d6de91..8f6aa8f14 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": { diff --git a/ui/app/src/components/shared/ConfirmUpgradeResource.test.tsx b/ui/app/src/components/shared/ConfirmUpgradeResource.test.tsx index e355cb74c..1617ffb32 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"; @@ -11,9 +11,29 @@ import { CostResource } from "../../models/costs"; 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" }, + HttpMethod: { Patch: "PATCH", Get: "GET" }, ResultType: { JSON: "JSON" }, })); @@ -23,7 +43,7 @@ vi.mock("../../hooks/customReduxHooks", () => ({ vi.mock("../shared/notifications/operationsSlice", () => ({ addUpdateOperation: vi.fn(), - default: (state: { items: unknown[] } = { items: [] }) => state + default: (state: any = { items: [] }) => state })); // Mock FluentUI components using centralized mocks @@ -46,13 +66,11 @@ vi.mock("@fluentui/react", async () => { }; }); -vi.mock("./ExceptionLayout", () => { - const ExceptionLayout = ({ e }: any) => ( +vi.mock("./ExceptionLayout", () => ({ + ExceptionLayout: ({ e }: any) => (
{e.userMessage}
- ); - ExceptionLayout.displayName = 'ExceptionLayout'; - return { ExceptionLayout }; -}); + ), +})); const mockAvailableUpgrades: AvailableUpgrade[] = [ { version: "1.1.0", forceUpdateRequired: false }, @@ -132,6 +150,17 @@ describe("ConfirmUpgradeResource Component", () => { 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", () => { @@ -193,7 +222,7 @@ describe("ConfirmUpgradeResource Component", () => { expect(upgradeButton).toBeDisabled(); }); - it("enables upgrade button when version is selected", () => { + it("enables upgrade button when version is selected", async () => { renderWithWorkspaceContext( { const dropdown = screen.getByTestId("dropdown"); fireEvent.change(dropdown, { target: { value: "1.1.0" } }); - const upgradeButton = screen.getByTestId("primary-button"); - expect(upgradeButton).not.toBeDisabled(); + // 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.mockResolvedValue({ operation: mockOperation }); + 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( { 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); @@ -232,7 +282,10 @@ describe("ConfirmUpgradeResource Component", () => { mockResource.resourcePath, "PATCH", mockWorkspaceContext.workspaceApplicationIdURI, - { templateVersion: "1.1.0" }, + expect.objectContaining({ + templateVersion: "1.1.0", + properties: expect.any(Object), + }), "JSON", undefined, undefined, @@ -245,9 +298,29 @@ describe("ConfirmUpgradeResource Component", () => { }); it("shows loading spinner during API call", async () => { - mockApiCall.mockImplementation( - () => new Promise((resolve) => setTimeout(resolve, 100)) - ); + 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 and click upgrade + // 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); @@ -268,8 +349,20 @@ describe("ConfirmUpgradeResource Component", () => { }); it("displays error when API call fails", async () => { - const error = new Error("Network error"); - mockApiCall.mockRejectedValue(error); + 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 and click upgrade + // 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 act(async () => { + fireEvent.click(upgradeButton); + }); await waitFor(() => { expect(screen.getByTestId("exception-layout")).toBeInTheDocument(); @@ -293,7 +396,20 @@ describe("ConfirmUpgradeResource Component", () => { it("uses workspace auth for workspace service resources", async () => { const mockOperation = { id: "operation-id", status: "running" }; - mockApiCall.mockResolvedValue({ operation: mockOperation }); + 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 and click upgrade + // 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); @@ -329,7 +453,20 @@ describe("ConfirmUpgradeResource Component", () => { resourceType: ResourceType.SharedService, }; const mockOperation = { id: "operation-id", status: "running" }; - mockApiCall.mockResolvedValue({ operation: mockOperation }); + 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 and click upgrade + // 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); @@ -383,4 +528,325 @@ describe("ConfirmUpgradeResource Component", () => { // 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 + ); + }); + }); + + 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 8967b08e0..ca99e981b 100644 --- a/ui/app/src/components/shared/ConfirmUpgradeResource.tsx +++ b/ui/app/src/components/shared/ConfirmUpgradeResource.tsx @@ -8,9 +8,12 @@ import { MessageBar, MessageBarType, Icon, + Stack, } from "@fluentui/react"; -import React, { useContext, useState } 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"; import { HttpMethod, ResultType, @@ -23,24 +26,114 @@ 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 conditional schemas +const collectConditionalKeys = (entry: any): string[] => { + const keys: string[] = []; + 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 that reference any of the new properties. +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 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); + } + } + }); + 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 [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); + 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?`, @@ -56,18 +149,180 @@ 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); + setRemovedProperties([]); + 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 = ""; + + fetchUrl = `${templateGetPath}/${props.resource.templateName}?version=${selectedVersion}`; + + const newTemplate = await apiCall( + fetchUrl, + HttpMethod.Get, + templateUsesWsAuth ? 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, + templateUsesWsAuth ? workspaceCtx.workspaceApplicationIdURI : undefined, + undefined, + ResultType.JSON, + ); + currentTemplateRef.current = currentTemplate; + } + + // Use full fetched schema from API + setNewTemplateSchema(newTemplate); + + const newSchemaProps = newTemplate?.properties || {}; + const currentProps = currentTemplate?.properties || {}; + + const newKeys = getAllPropertyKeys(newSchemaProps); + const currentKeys = getAllPropertyKeys(currentProps); + const newPropKeys = newKeys.filter((k) => !currentKeys.includes(k)); + const removedPropsArray = currentKeys.filter((k) => !newKeys.includes(k)); + + // 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 = 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) + const classNames = propertyUiSchema?.classNames || propertyUiSchema?.['ui:classNames']; + const isHidden = classNames?.includes('tre-hidden'); + return !isHidden; + }); + + setNewPropertiesToFill(visibleNewPropKeys); + setRemovedProperties(removedPropsArray); + + // 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( + newPropKeysToSend.reduce((acc, key) => { + // Get top-level portion of the key + const topKey = key.split('.')[0]; + const defaultValue = newTemplate?.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 }; + + body.properties = newPropertyValues; + let op = await apiCall( props.resource.resourcePath, HttpMethod.Patch, - wsAuth ? workspaceCtx.workspaceApplicationIdURI : undefined, + instanceUsesWsAuth ? workspaceCtx.workspaceApplicationIdURI : undefined, body, ResultType.JSON, undefined, @@ -77,12 +332,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 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, allNewProperties) + : null; + + // 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 + ? { ...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 }, + }; + const onRenderOption = (option: any): JSX.Element => { return (
@@ -129,6 +410,35 @@ export const ConfirmUpgradeResource: React.FunctionComponent< Upgrading the template version is irreversible. + + {loadingSchema && } + {!loadingSchema && removedProperties.length > 0 && ( + + Warning: The following properties are no longer present in the template and will be removed: {removedProperties.join(', ')} + + )} + {!loadingSchema && allNewProperties.length > 0 && ( + + {newPropertiesToFill.length > 0 && ( + + You must specify values for new properties: + + )} + + {finalSchema && ( +
setNewPropertyValues(e.formData)} + /> + )} + + )} + 0 && + newPropertiesToFill.some( + (key) => newPropertyValues[key] === "" || newPropertyValues[key] === undefined, + )) + } text="Upgrade" onClick={() => upgradeCall()} /> @@ -156,7 +472,7 @@ export const ConfirmUpgradeResource: React.FunctionComponent< /> )} {requestLoadingState === LoadingState.Error && ( - + )}