feat(templates): lock ParentTemplateId after creation
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.
This commit is contained in:
@@ -212,13 +212,11 @@
|
|||||||
</div>
|
</div>
|
||||||
<div class="col-md-3">
|
<div class="col-md-3">
|
||||||
<label class="form-label small">Parent Template</label>
|
<label class="form-label small">Parent Template</label>
|
||||||
<select class="form-select form-select-sm" @bind="_editParentId">
|
<div class="form-control-plaintext form-control-sm">
|
||||||
<option value="0">(None)</option>
|
@(_selectedTemplate.ParentTemplateId is int pid
|
||||||
@foreach (var t in _templates.Where(t => t.Id != _selectedTemplate.Id))
|
? _templates.FirstOrDefault(t => t.Id == pid)?.Name ?? $"#{pid}"
|
||||||
{
|
: "(none)")
|
||||||
<option value="@t.Id">@t.Name</option>
|
</div>
|
||||||
}
|
|
||||||
</select>
|
|
||||||
</div>
|
</div>
|
||||||
<div class="col-md-2">
|
<div class="col-md-2">
|
||||||
<button class="btn btn-primary btn-sm" @onclick="UpdateTemplateProperties">Save Properties</button>
|
<button class="btn btn-primary btn-sm" @onclick="UpdateTemplateProperties">Save Properties</button>
|
||||||
|
|||||||
@@ -83,28 +83,16 @@ public class TemplateService
|
|||||||
if (template == null)
|
if (template == null)
|
||||||
return Result<Template>.Failure($"Template with ID {templateId} not found.");
|
return Result<Template>.Failure($"Template with ID {templateId} not found.");
|
||||||
|
|
||||||
// Validate parent change
|
// ParentTemplateId is immutable after creation — set once at create time.
|
||||||
if (parentTemplateId.HasValue && parentTemplateId.Value != (template.ParentTemplateId ?? 0))
|
// 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);
|
return Result<Template>.Failure(
|
||||||
if (parent == null)
|
"Parent template cannot be changed after creation.");
|
||||||
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);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
template.Name = name;
|
template.Name = name;
|
||||||
template.Description = description;
|
template.Description = description;
|
||||||
template.ParentTemplateId = parentTemplateId;
|
|
||||||
|
|
||||||
// Check for naming collisions after the change
|
// Check for naming collisions after the change
|
||||||
var collisionResult = await ValidateCollisionsAsync(template, cancellationToken);
|
var collisionResult = await ValidateCollisionsAsync(template, cancellationToken);
|
||||||
|
|||||||
@@ -468,35 +468,50 @@ public class TemplateServiceTests
|
|||||||
// ========================================================================
|
// ========================================================================
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
public async Task UpdateTemplate_InheritanceCycle_Fails()
|
public async Task UpdateTemplate_ChangeParent_Fails()
|
||||||
{
|
{
|
||||||
var templateA = new Template("A") { Id = 1, ParentTemplateId = null };
|
var parentA = new Template("A") { Id = 1 };
|
||||||
var templateB = new Template("B") { Id = 2, ParentTemplateId = 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);
|
// Attempt to re-parent Child from A (id=1) to B (id=3).
|
||||||
_repoMock.Setup(r => r.GetTemplateByIdAsync(2, It.IsAny<CancellationToken>())).ReturnsAsync(templateB);
|
var result = await _service.UpdateTemplateAsync(2, "Child", null, 3, "admin");
|
||||||
_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");
|
|
||||||
|
|
||||||
Assert.True(result.IsFailure);
|
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]
|
[Fact]
|
||||||
public async Task UpdateTemplate_SelfInheritance_Fails()
|
public async Task UpdateTemplate_ClearParent_Fails()
|
||||||
{
|
{
|
||||||
var template = new Template("Self") { Id = 1 };
|
var child = new Template("Child") { Id = 2, ParentTemplateId = 1 };
|
||||||
_repoMock.Setup(r => r.GetTemplateByIdAsync(1, It.IsAny<CancellationToken>())).ReturnsAsync(template);
|
_repoMock.Setup(r => r.GetTemplateByIdAsync(2, It.IsAny<CancellationToken>())).ReturnsAsync(child);
|
||||||
_repoMock.Setup(r => r.GetAllTemplatesAsync(It.IsAny<CancellationToken>()))
|
|
||||||
.ReturnsAsync(new List<Template> { template });
|
|
||||||
|
|
||||||
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.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]
|
[Fact]
|
||||||
|
|||||||
Reference in New Issue
Block a user