Updated CASE and CLR v2 Data Models in database - #1151
Conversation
bjagg
left a comment
There was a problem hiding this comment.
Overview
One file, projects/lif_mdr_database/backup.sql, +19,824/−415. CI green. A 20k-line SQL seed isn't reviewable line by line, so I reviewed it structurally — what tables changed, what rows arrived, and what state they arrived in.
No secrets in the diff. Flagging that first because it's the scariest-looking part: the diff adds the DeveloperApiKeys table, and COPY public."DeveloperApiKeys" ("Id", "OwnerSub", "Label", "KeyPrefix", "KeyHash", ...) FROM stdin; is immediately followed by \. — zero data rows. Only the table definition. No key material of any kind.
The description understates the scope considerably
The description says the CASE and CLR v2 data models were updated. What the diff contains is a full backup.sql regeneration:
| Section | Rows added |
|---|---|
| ValueSetValues | 5,436 |
| Attributes | 2,820 |
| TransformationAttributes | 2,756 |
| EntityAttributeAssociation | 2,499 |
| ValueSetValueMapping | 2,357 |
| Transformations | 1,390 |
| ValueSets | 1,283 |
| Entities | 417 |
| DataModels | 26 (all of them) |
Plus new DDL: the entire tenant_lif_team schema (12+ tables) and the DeveloperApiKeys table — i.e. it also picks up the self-serve-tenant (#884) and developer-keys (#1033) migrations.
That's not necessarily wrong — regenerating the seed from a current database is a reasonable way to refresh it. But a reviewer reading the description would not expect a new schema and 1,390 transformations, and neither would someone bisecting a future problem back to this commit. Worth expanding the description to say "regenerated backup.sql from , which also brings in X and Y."
Two things worth confirming
1. Active test/demo artifacts land in the canonical seed.
Six TestUpdated* models arrive correctly soft-deleted (Deleted = t), but two arrive active:
Test2UpdatedSourceModel(id 25)R1 Demo Source Data Model(id 26)
Along with an active TestNewSourceModel_StateULIF transformation group (with two soft-deleted duplicates of the same name alongside it) and an active R1 Demo Source Data Model_StateU LIF group.
I checked whether this regresses the LDE playground's export-format list, and it does not — those are inbound (→ StateU LIF) groups, and the playground keys off source = 17. So this is seed hygiene rather than a repeat of #1127. Still worth deciding whether test scaffolding belongs in the canonical seed, since anyone restoring from it inherits them.
2. CASE_LIF and CLR v2 / Open Badges v3_LIF transformation groups arrive deleted = t.
For a PR whose stated purpose is updating exactly those two data models, having their transformation groups soft-deleted in the seed seems worth a sentence. If that reflects the current state of the source database, fine — but it's the kind of thing that's much cheaper to confirm now than to rediscover when an export format goes missing.
The broader point
Regenerating backup.sql from a live database means the seed inherits whatever else is in that database — including test models, soft-delete states, and schema migrations that arrived by other routes. That's the same source-of-truth ambiguity I filed as #1145: the repo, dev, and demo have each been the odd one out at least once. This PR is a good moment to note which environment this was captured from, because that's the fact that makes the seed reproducible later.
Verdict
Approve on substance — the data itself looks fine and CI is green. Before merge I'd like:
- An expanded description naming the source environment and the fact that it's a full regeneration (including the new
tenant_lif_teamschema andDeveloperApiKeystable). - A yes/no on the two active test models and the soft-deleted CASE/CLR2 groups.
Neither is a code change — both are confirmations that make this commit legible a year from now.
Also: the PR title becomes the squash subject (squash_merge_commit_title = PR_TITLE), and commitlint.config.mjs requires Issue #XXX: description. "Updated CASE and CLR v2 Data Models in database" won't match — worth retitling if there's a tracking issue.
Description of Change
Both the CASE and the CLR v2 data models have been updated. This PR includes the backup.sql file with those updates applied.
Type of Change
to not work as expected)
Project Area(s) Affected