From 8b8b85c839073411a7655ba7dff16b7e594ab30d Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 12 May 2026 08:31:20 -0400 Subject: [PATCH] docs(templates): record phase 2+3 completion in status doc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 1 → 3 marked done; remaining work is phases 4-9. Sanity script now targets the post-Phase-3 commit (03a8c4a) and notes the pre-existing NU1608 build error in IntegrationTests / Host.Tests so future sessions don't chase a phantom regression. --- .../2026-05-12-derive-on-compose-status.md | 150 +++++++----------- 1 file changed, 55 insertions(+), 95 deletions(-) diff --git a/docs/plans/2026-05-12-derive-on-compose-status.md b/docs/plans/2026-05-12-derive-on-compose-status.md index cb73570..64c1b92 100644 --- a/docs/plans/2026-05-12-derive-on-compose-status.md +++ b/docs/plans/2026-05-12-derive-on-compose-status.md @@ -8,15 +8,19 @@ ## Where we are -**Branch**: `feature/templates-folder-hierarchy`. Already pushed. +**Branch**: `feature/templates-folder-hierarchy`. -**Last commit on this feature**: `5615f3d` — *Phase 1 complete, additive -schema only*. +**Last commit on this feature**: `03a8c4a` — *Phase 3 complete, data +migration in place*. + +**Phases 1–3 done**. Compose now derives; existing compositions migrate +on next `dotnet ef database update`. Everything from Phase 4 onward is +still pending. **All test suites currently green**: - `tests/ScadaLink.CentralUI.Tests` — 159 passing - `tests/ScadaLink.SiteRuntime.Tests` — 129 passing -- `tests/ScadaLink.TemplateEngine.Tests` — 199 passing +- `tests/ScadaLink.TemplateEngine.Tests` — 205 passing (+6 phase 2 tests) ## Design decisions already made (from the brainstorm) @@ -34,6 +38,40 @@ User picked the **full Aveva model** with all four customization scopes: Commits: `6854843` (design doc) + `a968cef` (decisions recorded) + `5615f3d`. +## Done — Phase 2: Compose flow change + +Commit: `fa86750`. + +- `TemplateService.AddCompositionAsync` builds a derived template + (`"."`), copies base attributes/scripts with + `IsInherited=true`, then composes the derived (not the base). Sets + `OwnerCompositionId` back-ref after the composition's Id is known. +- Composing a derived template is rejected — only bases can be composed. +- `DeleteCompositionAsync` cascade-deletes the slot-owned derived + template (`IsDerived=true` and `OwnerCompositionId==compositionId`). +- `DeleteTemplateAsync` blocks direct deletion of derived templates and + splits the inheritor check into regular children vs. derivatives — the + derivative branch labels each by `'OwnerName' (as 'SlotName')`. +- `TemplateDeletionService.CanDeleteTemplateAsync` mirrors the same + derivative-aware checks. + +## Done — Phase 3: Migration of existing compositions + +Commit: `03a8c4a`. Migration `20260512122746_MigrateCompositionsToDerived`. + +- Pre-flight aborts with a descriptive error if any + `.` derived name would collide. +- Cursor-walks every `TemplateComposition` whose target is `IsDerived=0`, + inserts a derived template, copies attributes/scripts with + `IsInherited=1`, then repoints `ComposedTemplateId`. +- Idempotent (only touches non-derived targets), so re-runs are safe. +- `Down()` reverses by repointing compositions to `ParentTemplateId` and + dropping the derived templates. + +The migration was NOT verified against a live SQL Server in this +session — run `bash docker/deploy.sh` (or `dotnet ef database update`) +once with seeded test data to confirm shape. + Files touched in `5615f3d`: - `src/ScadaLink.Commons/Entities/Templates/Template.cs` @@ -51,89 +89,7 @@ Files touched in `5615f3d`: **No behavior changes**. New fields are never read or written yet. -## Remaining — phases 2 through 9 - -Each phase ships independently. The cumulative behavior change fully lands at -phase 3. - -### Phase 2: Compose flow change - -`TemplateService.AddCompositionAsync` currently creates a `TemplateComposition` -that references a base template directly. Change it so NEW compositions: - -1. Create a new `Template` entity: - - `Name = $"{parentTemplate.Name}.{instanceName}"` - - `ParentTemplateId = composedTemplateId` (the base) - - `IsDerived = true` - - `FolderId = null` (or inherit from parent? — pick null; derived - templates aren't visible in the tree) -2. Copy the base's `Attributes` into the new template marked - `IsInherited = true`. Same for `Scripts` and `Alarms`. -3. Create the `TemplateComposition` with `ComposedTemplateId = newTemplate.Id` - (the derived, not the base). -4. Set the derived template's `OwnerCompositionId = composition.Id` (back-ref). - -Files to touch: - -- `src/ScadaLink.TemplateEngine/TemplateService.cs` — extend - `AddCompositionAsync` (currently around line 290+). -- `src/ScadaLink.TemplateEngine/TemplateService.cs` — also extend - `DeleteCompositionAsync` to cascade-delete the derived template. -- `src/ScadaLink.TemplateEngine/TemplateService.cs` — extend `DeleteTemplateAsync` - to block when the template is composed (already happens) AND when - derivatives exist; reword the error. -- Validation: derived template names must remain unique (the base existing - uniqueness index on `Template.Name` covers this — but if `Pump.TempSensor` - collides with a user-created template of that name, the add fails. Add a - guard with a clear error message.) - -Tests to add (`tests/ScadaLink.TemplateEngine.Tests/`): - -- AddCompositionAsync creates a derived template. -- The derived template inherits attributes / scripts with `IsInherited = true`. -- DeleteCompositionAsync cascades to remove the derived template. -- Deleting a base with derivatives is blocked with a message that lists them. - -### Phase 3: Migration of existing compositions - -EF Core data migration that runs on next startup. Two paths: - -**Option A (simpler) — write the migration as a custom SQL/code step** - -In `src/ScadaLink.ConfigurationDatabase/Migrations/`, generate a new migration -called e.g. `MigrateCompositionsToDerived`. In its `Up` method, use raw -`migrationBuilder.Sql(...)` or a delegate to: - -``` -FOREACH composition IN TemplateComposition WHERE ComposedTemplate.IsDerived = false: - derived := INSERT INTO Templates ( - Name = parent.Name || '.' || composition.InstanceName, - ParentTemplateId = composition.ComposedTemplateId, - IsDerived = true, - OwnerCompositionId = composition.Id) - INSERT INTO TemplateAttributes (Name, Value, DataType, IsLocked, Description, - DataSourceReference, IsInherited=true, - LockedInDerived=false, TemplateId=derived.Id) - FROM TemplateAttributes WHERE TemplateId = composition.ComposedTemplateId - same for TemplateScripts and TemplateAlarms - UPDATE TemplateComposition SET ComposedTemplateId = derived.Id WHERE Id = composition.Id -``` - -Run as raw SQL to keep the migration deterministic across providers (SQL -Server vs SQLite if used in dev). - -**Option B — service-layer one-time migration** - -Add a startup hook that runs once (controlled by a flag column) and uses -`TemplateService` directly. Cleaner for complex logic but more code. - -**Pick Option A** unless the logic doesn't fit comfortably in SQL. - -Validation post-migration: every `TemplateComposition.ComposedTemplateId` -should now reference a template with `IsDerived = true`. - -Tests: integration test that seeds pre-migration data, runs the migration, -asserts the new shape. +## Remaining — phases 4 through 9 ### Phase 4: Inherit/override resolution in flattening @@ -238,14 +194,12 @@ After compaction, the future session should: 1. Read this file. Read the design doc `2026-05-12-derive-on-compose-design.md` for context. -2. Run `git log --oneline -15` to confirm the branch is at `5615f3d` or +2. Run `git log --oneline -15` to confirm the branch is at `03a8c4a` or later. 3. Check test status: `dotnet test` across the three suites named above. -4. Ask the user which phase to tackle next (or proceed from phase 2 if the +4. Ask the user which phase to tackle next (or proceed from phase 4 if the user has explicitly said "continue"). -5. Each phase is its own commit. Prefer phase 2 + phase 3 together because - phase 2 alone leaves new and existing compositions in different shapes - until phase 3's migration runs. +5. Each phase is its own commit. ## Pre-existing capability worth knowing @@ -260,11 +214,17 @@ The multi-parent picker introduced in `0139c9c` becomes mostly dormant after phase 3 (no template should be composed by multiple parents anymore through the new flow). Plan to remove it in phase 9. -## Quick sanity script (run before phase 2) +## Quick sanity script (run before phase 4) ```bash git status --short # should be clean -git log --oneline -5 # top should include 5615f3d -dotnet build # 0 errors, 0 warnings +git log --oneline -5 # top should include 03a8c4a +dotnet build src/ScadaLink.TemplateEngine src/ScadaLink.ConfigurationDatabase dotnet test tests/ScadaLink.TemplateEngine.Tests/ScadaLink.TemplateEngine.Tests.csproj ``` + +Note: the full `dotnet build` of the solution fails with NU1608 in +`ScadaLink.IntegrationTests` and `ScadaLink.Host.Tests` due to a +pre-existing `Microsoft.CodeAnalysis.Common` 4.13 vs 5.0 mismatch — not +related to the derive-on-compose work. Build the three suites listed in +"Where we are" individually.