docs(templates): record phase 2+3 completion in status doc
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.
This commit is contained in:
Joseph Doherty
@@ -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
|
||||
(`"<parent>.<slot>"`), 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
|
||||
`<parent>.<slot>` 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.
|
||||
|
||||
Reference in New Issue
Block a user