From b4cb7e6f5fce6e070c3b56fe403886b9c15c1aa0 Mon Sep 17 00:00:00 2001
From: Joseph Doherty <dohejw01@gmail.com>
Date: Mon, 11 May 2026 21:29:21 -0400
Subject: [PATCH] feat(templates): lock ParentTemplateId after creation
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Template inheritance is set once at create time and immutable on update.
UpdateTemplateAsync now returns "Parent template cannot be changed after
creation." when the caller sends a parent that differs from the stored
value — server-side enforcement covers UI, ManagementService, and CLI.
TemplateEdit renders the parent as static plaintext rather than an
editable dropdown; TemplateCreate's parent picker is unchanged.
---
 .../Pages/Design/TemplateEdit.razor           | 12 ++---
 .../TemplateService.cs                        | 22 ++------
 .../TemplateServiceTests.cs                   | 51 ++++++++++++-------
 3 files changed, 43 insertions(+), 42 deletions(-)

diff --git a/src/ScadaLink.CentralUI/Components/Pages/Design/TemplateEdit.razor b/src/ScadaLink.CentralUI/Components/Pages/Design/TemplateEdit.razor
index 1a14f48..6938ea3 100644
--- a/src/ScadaLink.CentralUI/Components/Pages/Design/TemplateEdit.razor
+++ b/src/ScadaLink.CentralUI/Components/Pages/Design/TemplateEdit.razor
@@ -212,13 +212,11 @@
                     </div>
                     <div class="col-md-3">
                         <label class="form-label small">Parent Template</label>
-                        <select class="form-select form-select-sm" @bind="_editParentId">
-                            <option value="0">(None)</option>
-                            @foreach (var t in _templates.Where(t => t.Id != _selectedTemplate.Id))
-                            {
-                                <option value="@t.Id">@t.Name</option>
-                            }
-                        </select>
+                        <div class="form-control-plaintext form-control-sm">
+                            @(_selectedTemplate.ParentTemplateId is int pid
+                                ? _templates.FirstOrDefault(t => t.Id == pid)?.Name ?? $"#{pid}"
+                                : "(none)")
+                        </div>
                     </div>
                     <div class="col-md-2">
                         <button class="btn btn-primary btn-sm" @onclick="UpdateTemplateProperties">Save Properties</button>
diff --git a/src/ScadaLink.TemplateEngine/TemplateService.cs b/src/ScadaLink.TemplateEngine/TemplateService.cs
index 48ced9d..28df3cd 100644
--- a/src/ScadaLink.TemplateEngine/TemplateService.cs
+++ b/src/ScadaLink.TemplateEngine/TemplateService.cs
@@ -83,28 +83,16 @@ public class TemplateService
         if (template == null)
             return Result<Template>.Failure($"Template with ID {templateId} not found.");
 
-        // Validate parent change
-        if (parentTemplateId.HasValue && parentTemplateId.Value != (template.ParentTemplateId ?? 0))
+        // ParentTemplateId is immutable after creation — set once at create time.
+        // Reject any attempt to change it (null→value, value→null, or value→other).
+        if (parentTemplateId != template.ParentTemplateId)
         {
-            var parent = await _repository.GetTemplateByIdAsync(parentTemplateId.Value, cancellationToken);
-            if (parent == null)
-                return Result<Template>.Failure($"Parent template with ID {parentTemplateId.Value} not found.");
-
-            // Check inheritance acyclicity
-            var allTemplates = await _repository.GetAllTemplatesAsync(cancellationToken);
-            var cycleError = CycleDetector.DetectInheritanceCycle(templateId, parentTemplateId.Value, allTemplates);
-            if (cycleError != null)
-                return Result<Template>.Failure(cycleError);
-
-            // Check cross-graph cycle
-            var crossCycleError = CycleDetector.DetectCrossGraphCycle(templateId, parentTemplateId, null, allTemplates);
-            if (crossCycleError != null)
-                return Result<Template>.Failure(crossCycleError);
+            return Result<Template>.Failure(
+                "Parent template cannot be changed after creation.");
         }
 
         template.Name = name;
         template.Description = description;
-        template.ParentTemplateId = parentTemplateId;
 
         // Check for naming collisions after the change
         var collisionResult = await ValidateCollisionsAsync(template, cancellationToken);
diff --git a/tests/ScadaLink.TemplateEngine.Tests/TemplateServiceTests.cs b/tests/ScadaLink.TemplateEngine.Tests/TemplateServiceTests.cs
index 8dd3533..d28f51c 100644
--- a/tests/ScadaLink.TemplateEngine.Tests/TemplateServiceTests.cs
+++ b/tests/ScadaLink.TemplateEngine.Tests/TemplateServiceTests.cs
@@ -468,35 +468,50 @@ public class TemplateServiceTests
     // ========================================================================
 
     [Fact]
-    public async Task UpdateTemplate_InheritanceCycle_Fails()
+    public async Task UpdateTemplate_ChangeParent_Fails()
     {
-        var templateA = new Template("A") { Id = 1, ParentTemplateId = null };
-        var templateB = new Template("B") { Id = 2, ParentTemplateId = 1 };
+        var parentA = new Template("A") { Id = 1 };
+        var child = new Template("Child") { Id = 2, ParentTemplateId = 1 };
+        _repoMock.Setup(r => r.GetTemplateByIdAsync(2, It.IsAny<CancellationToken>())).ReturnsAsync(child);
 
-        _repoMock.Setup(r => r.GetTemplateByIdAsync(1, It.IsAny<CancellationToken>())).ReturnsAsync(templateA);
-        _repoMock.Setup(r => r.GetTemplateByIdAsync(2, It.IsAny<CancellationToken>())).ReturnsAsync(templateB);
-        _repoMock.Setup(r => r.GetAllTemplatesAsync(It.IsAny<CancellationToken>()))
-            .ReturnsAsync(new List<Template> { templateA, templateB });
-
-        // Try to make A inherit from B (B already inherits from A) => cycle
-        var result = await _service.UpdateTemplateAsync(1, "A", null, 2, "admin");
+        // Attempt to re-parent Child from A (id=1) to B (id=3).
+        var result = await _service.UpdateTemplateAsync(2, "Child", null, 3, "admin");
 
         Assert.True(result.IsFailure);
-        Assert.Contains("cycle", result.Error, StringComparison.OrdinalIgnoreCase);
+        Assert.Contains("cannot be changed", result.Error, StringComparison.OrdinalIgnoreCase);
+        _repoMock.Verify(r => r.UpdateTemplateAsync(It.IsAny<Template>(), It.IsAny<CancellationToken>()), Times.Never);
     }
 
     [Fact]
-    public async Task UpdateTemplate_SelfInheritance_Fails()
+    public async Task UpdateTemplate_ClearParent_Fails()
     {
-        var template = new Template("Self") { Id = 1 };
-        _repoMock.Setup(r => r.GetTemplateByIdAsync(1, It.IsAny<CancellationToken>())).ReturnsAsync(template);
-        _repoMock.Setup(r => r.GetAllTemplatesAsync(It.IsAny<CancellationToken>()))
-            .ReturnsAsync(new List<Template> { template });
+        var child = new Template("Child") { Id = 2, ParentTemplateId = 1 };
+        _repoMock.Setup(r => r.GetTemplateByIdAsync(2, It.IsAny<CancellationToken>())).ReturnsAsync(child);
 
-        var result = await _service.UpdateTemplateAsync(1, "Self", null, 1, "admin");
+        // Attempt to clear the parent.
+        var result = await _service.UpdateTemplateAsync(2, "Child", null, null, "admin");
 
         Assert.True(result.IsFailure);
-        Assert.Contains("itself", result.Error, StringComparison.OrdinalIgnoreCase);
+        Assert.Contains("cannot be changed", result.Error, StringComparison.OrdinalIgnoreCase);
+        _repoMock.Verify(r => r.UpdateTemplateAsync(It.IsAny<Template>(), It.IsAny<CancellationToken>()), Times.Never);
+    }
+
+    [Fact]
+    public async Task UpdateTemplate_SameParent_Succeeds()
+    {
+        var child = new Template("Child") { Id = 2, ParentTemplateId = 1, Description = "old" };
+        _repoMock.Setup(r => r.GetTemplateByIdAsync(2, It.IsAny<CancellationToken>())).ReturnsAsync(child);
+        _repoMock.Setup(r => r.GetAllTemplatesAsync(It.IsAny<CancellationToken>()))
+            .ReturnsAsync(new List<Template> { child });
+
+        // Idempotent pass — same parent value sent on update should succeed and apply name/description changes.
+        var result = await _service.UpdateTemplateAsync(2, "ChildRenamed", "new", 1, "admin");
+
+        Assert.True(result.IsSuccess);
+        Assert.Equal("ChildRenamed", result.Value.Name);
+        Assert.Equal("new", result.Value.Description);
+        Assert.Equal(1, result.Value.ParentTemplateId);
+        _repoMock.Verify(r => r.UpdateTemplateAsync(It.IsAny<Template>(), It.IsAny<CancellationToken>()), Times.Once);
     }
 
     [Fact]