diff --git a/docs/document-axes/README.md b/docs/document-axes/README.md index 3c1b45473b..39e6432ff4 100644 --- a/docs/document-axes/README.md +++ b/docs/document-axes/README.md @@ -29,13 +29,14 @@ flips the rows it delivers **in the same PR**, and names the stage/ticket under | `canonical` (single pointed-to doc for a slot) | pointer slots addressed as container + owner + label, rule-enforced | done | CLUE-524; class+unit slots added CLUE-550 Stage 2; CLUE-610 gave every slot an explicit owner segment, taken from the document's `uid` | | `concurrent` (multi-writer vs single-writer) | stored per-doc; rule-readable; `DocumentModel` prop sourced from Firestore at open | done | CLUE-550 Stage 1 | | `kind` (preset/cohort tag: defaults, presentation, templates) | stored per-doc tag; dereferenced only in the kind registry | done | CLUE-550 Stage 1 (stored + registry seeded); titles resolved by kind Stage 2; presentation wired by CLUE-610 (workspace title bar reads the registry; no consumer branches on kind); CLUE-610 also scopes a unit-declared kind's definition to its unit — see "Static and dynamic kinds" in [axes.md](./axes.md) | -| `owner` (who the document belongs to) | creation: kind-declared `ownerType` → owner `uid` (`getDocumentOwner`) plus a group owner's stored `groupId` (`getDocumentOwnerFields`); read: `hasGroupOwner(doc)` and `hasClassOwner(doc)`, each over the uid prefix its minter used | in progress | CLUE-550 Stage 2 (creation-side owner derivation registry-declared for all kinds); CLUE-610 (both guards; `groupId` moved onto this axis, off the container, and then off the owner *question* — the `uid` is the sole authority). Still to come: the user level, and a getter that returns which owner a document has rather than testing for one | -| `container` (where the document is kept: class → classUnit → offering) | creation: kind-declared `containerType`, stamped by `getDocumentLocationFields(kind, ctx)`; read: `isInClassUnitContainer(doc)`, over an `offeringId` now surfaced on both metadata types | in progress | CLUE-550 Stage 2 (creation side, every kind); CLUE-610 (`containerType` replaces `scopeType`, with no group level — a group document is kept in the offering and owned by the group; the edit gate switched from a curriculum test to this one). Still to come: a guard for the class level, and a getter returning the container | -| `curriculum` (what the document is about: none → unit → investigation → problem) | creation: fixed by the kind's `containerType`, since every container above the class is identified by a curriculum coordinate; read: `getCurriculumLabel(doc)` | in progress | CLUE-550 Stage 2 (creation side, every kind); CLUE-610 (the label, and the unit level states its absent fields explicitly so it is queryable). No consumer asks a yes/no curriculum question, so no guard exists | +| `owner` (who the document belongs to) | creation: `ownerType`, from the kind's axis profile → owner `uid` (`getDocumentOwner`) plus a group owner's stored `groupId` (`getDocumentOwnerFields`); read: `hasGroupOwner(doc)` and `hasClassOwner(doc)`, each over the uid prefix its minter used | in progress | CLUE-550 Stage 2 (creation-side owner derivation registry-declared for all kinds); CLUE-610 (both guards; `groupId` moved onto this axis, off the container, and then off the owner *question* — the `uid` is the sole authority). Still to come: the user level, and a getter that returns which owner a document has rather than testing for one | +| `container` (where the document is kept: class → classUnit → offering) | creation: `containerType`, from the kind's axis profile, stamped by `getDocumentLocationFields(kind, ctx)`; read: `isInClassUnitContainer(doc)`, over an `offeringId` now surfaced on both metadata types | in progress | CLUE-550 Stage 2 (creation side, every kind); CLUE-610 (`containerType` replaces `scopeType`, with no group level — a group document is kept in the offering and owned by the group; the edit gate switched from a curriculum test to this one). Still to come: a guard for the class level, and a getter returning the container | +| `curriculum` (what the document is about: none → unit → investigation → problem) | creation: fixed by the profile's `containerType`, since every container above the class is identified by a curriculum coordinate; read: `getCurriculumLabel(doc)` | in progress | CLUE-550 Stage 2 (creation side, every kind); CLUE-610 (the label, and the unit level states its absent fields explicitly so it is queryable). No consumer asks a yes/no curriculum question, so no guard exists | | `permissions` (composed grant set) | permission-policy grants (referenced policy) + stored per-doc grants | not started | Consumers are spread widely and each composes the answer by hand; the edit gate `canUserEditDocument` is called out as a worked example, including its `type` branch for published documents. See "Not covered yet" in [reading-axes-in-code.md](./reading-axes-in-code.md) for what a named policy would replace | -| kind registry (by-kind view) | `register`/`get` map keyed on `kind`; `fn(doc)` API | done | CLUE-550 Stage 1 | +| kind registry (by-kind view) | `register`/`get` map keyed on `kind`; `fn(doc)` API | done | CLUE-550 Stage 1; CLUE-610 moved the axis values off each kind and onto a named **axis profile** it registers against (`document-axis-profiles.ts`), so the set of axis combinations is enumerable in one file and a unit config can join one but not invent one | +| axis profiles (named axis-value bundles) | code-defined profiles; each document stores the name of the one it was created from (`axisProfile`), read by migrations only | done | CLUE-610 — see "Axis profiles" in [axes.md](./axes.md). Deliberately absent from every runtime type, so behavior is still decided by the axis guards | | behavior modules (by-behavior view) | `fn(doc)` reading axis getters / registry; never branch on `kind` | in progress | CLUE-550 Stage 1 (history + write-sync on concurrent; read-access + rules-delete on group type, interim until the permissions axis); CLUE-550 Stage 3a (collaborative thumbnail treatment reads `concurrent`); CLUE-610 (edit gate `canUserEditDocument` and the collaborative title bar read `concurrent`) | -| creation factory (the one `kind → axis` bridge) | reads registry defaults, stamps axis values on a new doc | in progress | CLUE-550 Stage 2 (per-slot class-wide canonical creation; owner and location fields stamped from the kind's `ownerType`/`containerType`) | +| creation factory (the one `kind → axis` bridge) | reads the profile the kind registers, stamps its axis values and its name on a new doc | in progress | CLUE-550 Stage 2 (per-slot class-wide canonical creation; owner and location fields stamped from the kind's `ownerType`/`containerType`) | Status values: `not started` / `in progress` / `done`. @@ -63,8 +64,9 @@ rather than by type, a whole-unit listener keeps them visible under the investig filters, presentation reads `concurrent` and the kind registry, and one predicate (`canUserEditDocument`) gates every Edit button. It also settles how these axes are modeled in code: consumers read narrow named guards over the stored fields, with no level enum and no unified struct; -a kind declares `ownerType` and `containerType`; and `groupId` sits on the owner axis, so there is no -group container level (see [reading-axes-in-code.md](./reading-axes-in-code.md)). Canonical slots follow: +a kind names an axis profile, which declares `ownerType` and `containerType`; and `groupId` sits on the +owner axis, so there is no group container level (see +[reading-axes-in-code.md](./reading-axes-in-code.md)). Canonical slots follow: each is addressed as its container plus its owner plus a label, with the owner segment read straight from the document's `uid` so the pointer path no longer depends on `groupId` at all. Both owner guards read the `uid` the same way, which leaves it the single authority on who a document belongs to; the stored diff --git a/docs/document-axes/axes.md b/docs/document-axes/axes.md index 523e218726..9f07a020db 100644 --- a/docs/document-axes/axes.md +++ b/docs/document-axes/axes.md @@ -336,13 +336,52 @@ entries to a unit config: it needs a configuration source loaded independently o association on the document naming that source. Until such a source exists, personal-like presets stay static. -**How a kind sets that today.** A kind declares only `containerType`, and `getDocumentLocationFields` -derives both the container and curriculum axes' fields from it, so a document's curriculum position -currently follows from its container: `class` yields no unit, `classUnit` the unit, `offering` the -problem. The bound is stated on `curriculum` because that is what rule 2 constrains, and it survives -the coupling being broken — a kind declaring a class container *and* a unit curriculum would stamp +**How a kind sets that today.** A kind's profile declares only `containerType`, and +`getDocumentLocationFields` derives both the container and curriculum axes' fields from it, so a +document's curriculum position currently follows from its container: `class` yields no unit, `classUnit` +the unit, `offering` the problem. The bound is stated on `curriculum` because that is what rule 2 +constrains, and it survives the coupling being broken — a profile declaring a class container *and* a unit curriculum would stamp `unit` and satisfy rule 2, which a container-based bound would wrongly forbid. +## Axis profiles — naming a combination of axis values + +The axes describe a document one question at a time, but people do not talk that way. Nobody says "a +unit-level, class-owned, canonical, concurrent document" — they say **a class-wide document**. That +shorthand is not sloppiness; it names a real thing, a *combination* of positions that documents are +actually created at. This section gives that thing a name: an **axis profile**. + +A profile is a named bundle of axis values. `classWide` is one. So are the two CLUE has always had without +naming them — the bundle shared by personal documents, learning logs, and their publications, and the +bundle shared by problem, planning, and problem-like publications. They are defined in one place, which +makes that place the complete list of axis combinations the application supports. + +**A profile is not a `kind`.** Many kinds share one: what makes a learning log different from a personal +document is presentation and its creation recipe, not any axis. `kind` says which preset a document came +from; its profile says where that preset put it on the axes. + +**A unit may add kinds; it may not add axis combinations.** This is what lets `kind` stay open-ended while +the set of axis combinations stays closed. A unit config declares kinds, but it declares no axis values — +every kind is declared against a profile, and profiles are not authorable. So a configuration can add a +document to an existing combination and cannot invent one, and the set of combinations stays reviewable in +one place rather than growing with the units. + +**A profile is recorded, not resolved.** A document records which profile it was created from. That exists +for one reason: a migration that changes what a profile means has to find every document created from it, +and selecting those by their axis values would mean querying the very fields the migration is there to +change — a query that has to be rewritten every time the answer moves. +Because it is provenance rather than a cache, it stays true after such a migration: it says which profile +the document was made from, not what its axes hold now. + +**Nothing in the running application reads that record.** It is deliberately out of the app's reach: only +something reading stored documents directly — a migration, or offline analysis — can see it. That keeps the +axes themselves the only way to ask how a document behaves, which is the point of this whole folder. A +document behaves the way it does because of where it sits on the axes, never because of the name of the +bundle it started at. + +Where the profiles are defined, and how a kind names one, is in +[reading-axes-in-code.md](./reading-axes-in-code.md); the stored record is in +[metadata-fields.md](../document-metadata/metadata-fields.md). + ### `permissions` — who may do what **What it is.** The permission set: who may `read`, `write`, `publish`, `copy`, and whether the content diff --git a/docs/document-axes/reading-axes-in-code.md b/docs/document-axes/reading-axes-in-code.md index 02e7f72c21..d9b817c196 100644 --- a/docs/document-axes/reading-axes-in-code.md +++ b/docs/document-axes/reading-axes-in-code.md @@ -38,13 +38,25 @@ guard for it exists. ## How a kind declares its axes Those fields are stamped at creation from what the kind registered in -`src/models/document/document-kinds.ts`. A kind declares two things: +`src/models/document/document-kinds.ts`. A kind does not spell out axis values: it names an **axis +profile** (`src/models/document/document-axis-profiles.ts`), and the profile declares them. Many kinds +share one — what separates a learning log from a personal document is presentation and its creation +recipe, not any axis — and the profiles file is therefore the complete list of axis combinations the +application creates documents at. See "Axis profiles" in [axes.md](./axes.md). + +A profile declares two knobs, plus `concurrent`: - `ownerType` — `"user"`, `"group"`, or `"class"`. It picks the owner `uid` (`getDocumentOwner`) and, for a group owner, the `groupId` stored beside it (`getDocumentOwnerFields`). - `containerType` — `"class"`, `"classUnit"`, or `"offering"`. It picks the fields that say where the document is kept and what it is about (`getDocumentLocationFields`). +The profile's name is stamped onto the document it creates, in `axisProfile`, and that record is +deliberately declared on no runtime type — not `IDocumentMetadata`, not `DocumentMetadataModel`, not +`DocumentModel` — so reading it back would mean widening a type first. Only a migration or offline +analysis, reading Firestore directly, can see it. See the +[`axisProfile` field](../document-metadata/metadata-fields.md#axisprofile) for what is stored and where. + One knob covers both container and curriculum because every container above the class is *identified by* a curriculum coordinate — a classUnit by its unit, an offering by its problem — so a kind has no curriculum position left to choose separately. `getDocumentLocationFields` is named for the pair rather diff --git a/docs/document-metadata/metadata-fields.md b/docs/document-metadata/metadata-fields.md index 546b2a6ab6..4334728477 100644 --- a/docs/document-metadata/metadata-fields.md +++ b/docs/document-metadata/metadata-fields.md @@ -42,6 +42,7 @@ so this table doubles as a migration-progress view. | `strategies` | Firestore | commented docs | `DocumentMetadataModel.strategies` | Yes, class-wide | | `lastHistoryEntry` | Firestore | concurrent-history docs | not surfaced | No | | `canonical` | Firestore | group | not surfaced | No | +| `axisProfile` | Firestore | group | not surfaced — deliberately | No | | `offeringId` | Firestore + RTDB | problem family | `DocumentModel.offeringId`, `DocumentMetadataModel.offeringId` | No — immutable | | `groupId` | Firestore | group (the **owning** group) | `DocumentModel.groupId`, `DocumentMetadataModel.groupId` | No — immutable | @@ -79,6 +80,9 @@ prop, which is why they show as "not surfaced" above. They still reach `Document properties the model does not declare — pinned by the `typecheck` tests in [mst.test.ts](../../src/models/mst.test.ts). +`axisProfile` relies on the same behavior, but by design rather than by omission: leaving it undeclared is +what keeps the running app from reading it. See its section below. + ### Derived (no stored field) Held on the runtime model only, computed from something other than the document's own metadata. Listed @@ -320,7 +324,7 @@ declare it. Anything reading properties from the Firestore metadata should treat `undefined`) - **Applies to:** group documents — the group that **owns** the document - **Runtime:** `DocumentModel.groupId`, `DocumentMetadataModel.groupId` -- **Updated by:** nothing — creation only, stamped from the kind's `ownerType` by +- **Updated by:** nothing — creation only, stamped from the `ownerType` of the kind's axis profile by `getDocumentOwnerFields` - **Reactive:** No — immutable @@ -347,8 +351,8 @@ holds. - **Reactive:** Yes, to group membership changes — not to document changes The group the **user who owns** the document belongs to. Set only where the owner is a user (`ownerType: -"user"` in the kind registry): a group- or class-owned document's synthetic `uid` is not a member of any -group, so there is nobody to look up. A fact about that user's membership rather than about the document, +"user"` on the kind's axis profile): a group- or class-owned document's synthetic `uid` is not a member +of any group, so there is nobody to look up. A fact about that user's membership rather than about the document, which is why it is derived rather than stored: a student's group changes, and a frozen copy would go stale. The four-up view (`getProblemDocumentsForGroup`), Student Work routing, and the content listener's "whose documents do I monitor" test all want this one. @@ -387,6 +391,38 @@ identity of a document. `key` is also the document's `treeId` for the history system. For group documents `uid` is a synthetic value derived from the group (`group_{offeringId}_{groupId}`) rather than a real user id. +### `axisProfile` + +The name of the [axis profile](../document-axes/axes.md#axis-profiles--naming-a-combination-of-axis-values) +the document was created from — the named bundle of axis values it started at (`classWide`, `group`, +`personalLike`, `problemLike`). + +- **Stores:** Firestore only +- **Location:** `documents/{key}.axisProfile` +- **Applies to:** `type: "group"` documents (group + class-wide) — the same gate as `kind` +- **Runtime:** **none, deliberately** — declared on no runtime type +- **Updated by:** nothing — creation only, from the registered kind's profile +- **Reactive:** No + +Exists for migrations. A migration that changes what a profile means has to find every document created +from it; selecting those by their axis values would mean querying the fields the migration is about to +change, and would need rewriting each time they move. Because the field records *provenance* — which +profile the document was made from — it stays true after such a migration rather than going stale. + +The `type: "group"` gate is transitional, and is the same gate `kind` uses for the same reason: a value +stamped before a type's kind is settled is a value we would have to migrate afterwards. It widens as each +type is converted, and is deleted once they all are — see +["Which documents get stamped"](../document-axes/target-architecture.md#which-documents-get-stamped--a-gate-that-narrows-as-types-are-converted) +in the target architecture. So `axisProfile` arrives on a document by the same route `kind` does, and +outlives `type`: the end state is every document carrying a profile and a kind, and `type` carrying no +meaning at all. + +It is absent from `IDocumentMetadata`, `DocumentMetadataModel`, and `DocumentModel` on purpose, so the +running app cannot read it and cannot come to branch on it; the axis guards stay the only way to ask how a +document behaves. Reading it would mean widening a type first, which is a visible change rather than an +accident. Like `canonical`, it survives `DocumentMetadataStore`'s validation because MST's `typecheck` +ignores undeclared properties (see the note under the summary tables). + ### `unit`, `investigation`, `problem` - **Stores:** Firestore + RTDB *(declared)* diff --git a/firebase-test/src/documents-rules.test.ts b/firebase-test/src/documents-rules.test.ts index 37f42ab922..904efa2fcb 100644 --- a/firebase-test/src/documents-rules.test.ts +++ b/firebase-test/src/documents-rules.test.ts @@ -222,6 +222,23 @@ describe("Firestore security rules", () => { await expectUpdateToFail(db, kDocumentDocPath, { title: "new-title", context_id: otherClass }); }); + it("authenticated teachers can't stamp an axisProfile onto a document that has none", async () => { + // The profile a document was created from is what a later migration selects on, so a client that + // could write it could aim a migration at documents it was never made from. Only creation (as the + // document's author) and the service-account backfill script, which bypasses these rules, set it. + db = initFirestore(teacherAuth); + await specClassDoc(thisClass, teacherId); + await adminWriteDoc(kDocumentDocPath, specDocumentDoc()); + await expectUpdateToFail(db, kDocumentDocPath, { title: "new-title", axisProfile: "classWide" }); + }); + + it("authenticated teachers can't update user documents' read-only axisProfile field", async () => { + db = initFirestore(teacherAuth); + await specClassDoc(thisClass, teacherId); + await adminWriteDoc(kDocumentDocPath, specDocumentDoc({ add: { axisProfile: "group" } })); + await expectUpdateToFail(db, kDocumentDocPath, { title: "new-title", axisProfile: "classWide" }); + }); + it("authenticated teachers can't update other teachers' documents", async () => { db = initFirestore(teacher2Auth); await specClassDoc(thisClass, teacherId); diff --git a/firestore.rules b/firestore.rules index a9a58496df..7cdf557cbd 100644 --- a/firestore.rules +++ b/firestore.rules @@ -91,8 +91,12 @@ service cloud.firestore { request.resource.data.keys().hasAll(["uid", "unit", "problem", "section", "path", "network"]); } + // `axisProfile` records which axis profile a document was created from, and is what a migration + // selects a cohort by, so a client able to write it could aim a migration at documents that were + // never made from that profile. It is settable only at creation: the one backfill that writes it + // afterwards (scripts/backfill-group-document-axes.ts) runs as a service account, past these rules. function preservesReadOnlyDocumentFields() { - let readOnlyFieldsSet = ["uid", "type", "key", "createdAt", "context_id"].toSet(); + let readOnlyFieldsSet = ["uid", "type", "key", "createdAt", "context_id", "axisProfile"].toSet(); let affectedFieldsSet = request.resource.data.diff(resource.data).affectedKeys(); return !affectedFieldsSet.hasAny(readOnlyFieldsSet); } diff --git a/scripts/backfill-group-document-axes.test.ts b/scripts/backfill-group-document-axes.test.ts index 0c8cac251a..68074cf634 100644 --- a/scripts/backfill-group-document-axes.test.ts +++ b/scripts/backfill-group-document-axes.test.ts @@ -1,5 +1,6 @@ import type { Firestore } from "firebase-admin/firestore"; import { backfillGroupDocumentAxes } from "./backfill-group-document-axes"; +import { kClassWideProfile, kGroupProfile } from "../src/models/document/document-axis-profiles"; // Minimal Firestore-admin stand-in: a collection-group query returning canned docs, and a batch recorder. function makeDb(docs: any[]) { @@ -17,10 +18,10 @@ function makeDb(docs: any[]) { }; } -// A group-scoped document: carries a groupId. -const mkGroupDoc = (key: string, concurrent?: boolean) => ({ +// A group-scoped document: carries a groupId. `fields` supplies whatever else it already has. +const mkGroupDoc = (key: string, concurrent?: boolean, fields: Record = {}) => ({ ref: { path: `authed/p/documents/${key}` }, - get: (field: string) => ({ concurrent, groupId: "3" } as Record)[field], + get: (field: string) => ({ concurrent, groupId: "3", ...fields } as Record)[field], }); // A class-wide document: no groupId. `fields` supplies whatever scope fields it already has. const mkClassWideDoc = (key: string, fields: Record = {}) => ({ @@ -31,28 +32,34 @@ const mkClassWideDoc = (key: string, fields: Record = {}) => ({ const quiet = { log: () => undefined }; describe("backfillGroupDocumentAxes", () => { - it("dry run reports both passes and writes nothing", async () => { + it("dry run reports every pass and writes nothing", async () => { const db = makeDb([mkGroupDoc("a"), mkGroupDoc("b", true), mkClassWideDoc("c")]); const res = await backfillGroupDocumentAxes(db as unknown as Firestore, { dryRun: true, ...quiet }); - expect(res).toEqual({ total: 3, concurrentUpdated: 0, scopeUpdated: 0 }); + expect(res).toEqual({ total: 3, concurrentUpdated: 0, scopeUpdated: 0, profileUpdated: 0 }); expect(db.writes.length).toBe(0); }); it("stamps concurrent+kind only on group-scoped docs missing concurrent", async () => { - const db = makeDb([mkGroupDoc("a"), mkGroupDoc("b", true), mkGroupDoc("c")]); + const db = makeDb([ + mkGroupDoc("a"), + mkGroupDoc("b", true, { axisProfile: kGroupProfile.name }), + mkGroupDoc("c") + ]); const res = await backfillGroupDocumentAxes(db as unknown as Firestore, { dryRun: false, ...quiet }); - expect(res).toEqual({ total: 3, concurrentUpdated: 2, scopeUpdated: 0 }); + expect(res).toEqual({ total: 3, concurrentUpdated: 2, scopeUpdated: 0, profileUpdated: 2 }); expect(db.writes.map((w: any) => w.ref.path)).toEqual(["authed/p/documents/a", "authed/p/documents/c"]); expect(db.writes[0]).toMatchObject({ data: { concurrent: true, kind: "group" }, opts: { merge: true } }); }); it("stamps null curriculum scope only on class-wide docs that lack it", async () => { const db = makeDb([ - mkClassWideDoc("old"), // needs both fields - mkClassWideDoc("new", { investigation: null, problem: null }) // already migrated + // needs both fields + mkClassWideDoc("old", { axisProfile: kClassWideProfile.name }), + // already migrated + mkClassWideDoc("new", { axisProfile: kClassWideProfile.name, investigation: null, problem: null }) ]); const res = await backfillGroupDocumentAxes(db as unknown as Firestore, { dryRun: false, ...quiet }); - expect(res).toEqual({ total: 2, concurrentUpdated: 0, scopeUpdated: 1 }); + expect(res).toEqual({ total: 2, concurrentUpdated: 0, scopeUpdated: 1, profileUpdated: 0 }); expect(db.writes).toEqual([{ ref: { path: "authed/p/documents/old" }, data: { investigation: null, problem: null }, @@ -69,13 +76,43 @@ describe("backfillGroupDocumentAxes", () => { expect(db.writes.every((w: any) => w.data.kind === undefined)).toBe(true); }); + it("records the profile each document was created from, chosen by the same scope split", async () => { + // The profile is what a later migration selects on, so every document the query returns needs one. + // Scope decides which: a groupId means the group profile, its absence the class-wide one. Expected + // against the profile definitions themselves — the script repeats their names as literals, so + // nothing else would catch a rename. + const db = makeDb([ + mkGroupDoc("g", true), + mkClassWideDoc("cw", { investigation: null, problem: null }) + ]); + const res = await backfillGroupDocumentAxes(db as unknown as Firestore, { dryRun: false, ...quiet }); + expect(res.profileUpdated).toBe(2); + expect(db.writes).toEqual([ + { ref: { path: "authed/p/documents/g" }, data: { axisProfile: kGroupProfile.name }, opts: { merge: true } }, + { ref: { path: "authed/p/documents/cw" }, data: { axisProfile: kClassWideProfile.name }, + opts: { merge: true } } + ]); + }); + + it("writes a document once, carrying every field that document is missing", async () => { + // The profile pass overlaps the other two, and two batched writes to one document would cost twice + // as much for the same result. + const db = makeDb([mkGroupDoc("g")]); + await backfillGroupDocumentAxes(db as unknown as Firestore, { dryRun: false, ...quiet }); + expect(db.writes).toEqual([{ + ref: { path: "authed/p/documents/g" }, + data: { axisProfile: kGroupProfile.name, concurrent: true, kind: "group" }, + opts: { merge: true } + }]); + }); + it("is idempotent — a fully-migrated set writes nothing", async () => { const db = makeDb([ - mkGroupDoc("a", true), - mkClassWideDoc("c", { investigation: null, problem: null }) + mkGroupDoc("a", true, { axisProfile: kGroupProfile.name }), + mkClassWideDoc("c", { axisProfile: kClassWideProfile.name, investigation: null, problem: null }) ]); const res = await backfillGroupDocumentAxes(db as unknown as Firestore, { dryRun: false, ...quiet }); - expect(res).toEqual({ total: 2, concurrentUpdated: 0, scopeUpdated: 0 }); + expect(res).toEqual({ total: 2, concurrentUpdated: 0, scopeUpdated: 0, profileUpdated: 0 }); expect(db.writes.length).toBe(0); }); }); diff --git a/scripts/backfill-group-document-axes.ts b/scripts/backfill-group-document-axes.ts index deab4c6232..1f2b15428d 100644 --- a/scripts/backfill-group-document-axes.ts +++ b/scripts/backfill-group-document-axes.ts @@ -1,16 +1,26 @@ // Normalize the stored axes of `type == "group"` documents — both regular group documents and // class-wide collaborative documents, which share that transitional type. // -// Two independent passes over one collection-group query, selected by scope so they cover disjoint -// sets. A group-scoped document carries a groupId; a class-wide document does not. +// Three passes over one collection-group query, committed as ONE merged write per document. Scope +// selects which pass a document falls into: a group-scoped document carries a groupId; a class-wide +// document does not. // +// missing `axisProfile` -> { axisProfile: "group" | "classWide" } // group-scoped, missing `concurrent` -> { concurrent: true, kind: "group" } // class-wide, missing curriculum scope -> { investigation: null, problem: null } // -// The first pass restores the concurrent history manager for group documents that carry no stored -// `concurrent` value. The second states a class-wide document's absent curriculum scope -// explicitly, which is what makes it findable by Sort Work's unit-scoped query. Both are additive, -// idempotent, and batched. +// The profile pass records which axis profile a document was created from +// (src/models/document/document-axis-profiles.ts), which documents created since that landed already +// carry. It is what a later migration selects on when it changes what a profile means, so it has to +// cover the documents that predate the field. It is also the last migration that has to identify a +// document by its axis values rather than by a stored cohort key — which is why it derives the name +// from scope here rather than from `kind`: a class-wide document's kind is whatever its unit declared, +// and a script that runs outside the app cannot resolve unit-declared kinds through the registry. +// +// The second pass restores the concurrent history manager for group documents that carry no stored +// `concurrent` value. The third states a class-wide document's absent curriculum scope +// explicitly, which is what makes it findable by Sort Work's unit-scoped query. All three are +// additive, idempotent, and batched. // // Requires a Firebase service account key at scripts/serviceAccountKey.json (see scripts/README.md). // The `documents` collection-group query needs a single-field COLLECTION_GROUP index on `type` @@ -41,10 +51,17 @@ export interface BackfillResult { total: number; concurrentUpdated: number; scopeUpdated: number; + profileUpdated: number; } +// The profile names, kept in sync with src/models/document/document-axis-profiles.ts by the unit test. +// They are repeated rather than imported because scripts/ compiles on its own, against its own +// package.json, and does not resolve modules from src. +const kGroupProfileName = "group"; +const kClassWideProfileName = "classWide"; + /** - * Run both backfill passes. Pure (no admin initialization) so it can be unit-tested with a mock + * Run all three backfill passes. Pure (no admin initialization) so it can be unit-tested with a mock * Firestore. */ export async function backfillGroupDocumentAxes( @@ -59,23 +76,44 @@ export async function backfillGroupDocumentAxes( const needingConcurrent = snap.docs.filter((d) => !!d.get("groupId") && d.get("concurrent") !== true); const needingScope = snap.docs.filter((d) => !d.get("groupId") && (d.get("investigation") === undefined || d.get("problem") === undefined)); + const needingProfile = snap.docs.filter((d) => !d.get("axisProfile")); log(`group-typed docs: ${snap.size} total, ` + - `${needingConcurrent.length} missing concurrent, ${needingScope.length} missing curriculum scope`); + `${needingConcurrent.length} missing concurrent, ${needingScope.length} missing curriculum scope, ` + + `${needingProfile.length} missing an axis profile`); if (dryRun) { log("DRY RUN — set APPLY=1 to write"); - return { total: snap.size, concurrentUpdated: 0, scopeUpdated: 0 }; + return { total: snap.size, concurrentUpdated: 0, scopeUpdated: 0, profileUpdated: 0 }; } - const writes = [ - ...needingConcurrent.map((d) => ({ ref: d.ref, data: { concurrent: true, kind: "group" } })), - ...needingScope.map((d) => ({ ref: d.ref, data: { investigation: null, problem: null } })), - ]; + // One merged write per document, not one per pass: the profile pass overlaps both of the others, and + // two batched writes to the same document would cost twice as much for the same result. + const fields = new Map>(); + const fieldsFor = (ref: FirebaseFirestore.DocumentReference) => { + const existing = fields.get(ref); + if (existing) return existing; + const created: Record = {}; + fields.set(ref, created); + return created; + }; + + for (const d of needingProfile) { + // Scope is the only thing that separates the two profiles this query can return, and it is the same + // split the passes below select on: a groupId means the group profile, its absence the class-wide one. + const axisProfile = d.get("groupId") ? kGroupProfileName : kClassWideProfileName; + Object.assign(fieldsFor(d.ref), { axisProfile }); + } + for (const d of needingConcurrent) { + Object.assign(fieldsFor(d.ref), { concurrent: true, kind: "group" }); + } + for (const d of needingScope) { + Object.assign(fieldsFor(d.ref), { investigation: null, problem: null }); + } let batch = db.batch(); let n = 0; - for (const write of writes) { - batch.set(write.ref, write.data, { merge: true }); + for (const [ref, data] of fields) { + batch.set(ref, data, { merge: true }); if (++n % 400 === 0) { await batch.commit(); batch = db.batch(); @@ -85,11 +123,13 @@ export async function backfillGroupDocumentAxes( await batch.commit(); } - log(`updated ${needingConcurrent.length} concurrent, ${needingScope.length} curriculum scope`); + log(`updated ${needingConcurrent.length} concurrent, ${needingScope.length} curriculum scope, ` + + `${needingProfile.length} axis profile`); return { total: snap.size, concurrentUpdated: needingConcurrent.length, scopeUpdated: needingScope.length, + profileUpdated: needingProfile.length, }; } diff --git a/src/lib/db.test.ts b/src/lib/db.test.ts index 41fab19578..5839a5da97 100644 --- a/src/lib/db.test.ts +++ b/src/lib/db.test.ts @@ -3,7 +3,10 @@ import { createDocumentsModelWithRequiredDocuments, DocumentsModel } from "../mo import { DBDocument } from "./db-types"; import { createDocumentModel, DocumentModelType } from "../models/document/document"; import { DocumentContentModel } from "../models/document/document-content"; -import { registerDocumentKind, resetDocumentKindRegistryForTests } from "../models/document/document-kinds"; +import { kClassWideProfile } from "../models/document/document-axis-profiles"; +import { + registerClassWideDocumentKind, registerDocumentKind, resetDocumentKindRegistryForTests +} from "../models/document/document-kinds"; import { GroupDocument, LearningLogDocument, PersonalDocument, PlanningDocument, ProblemDocument } from "../models/document/document-types"; @@ -370,7 +373,45 @@ describe("db", () => { documentKey: "gk", type: GroupDocument, kind: GroupDocument, owner: "group_off-1_3", createdAt: 123 }); expect(written).toMatchObject({ kind: "group", concurrent: true }); - expect(setPayloads[0]).toMatchObject({ kind: "group", concurrent: true }); + expect(setPayloads[0]).toMatchObject({ kind: "group", concurrent: true, axisProfile: "group" }); + }); + + it("stamps the axis profile a class-wide document is created at, without reaching the runtime", async () => { + const setPayloads: any[] = []; + mockFirestore.mockImplementation(() => ({ + doc: () => ({ + get: () => Promise.resolve({ exists: false }), + set: (data: any) => { setPayloads.push(data); return Promise.resolve(); } + }) + })); + registerClassWideDocumentKind("testProfileStamp", "DQB", "msu"); + await db.connect({ appMode: "test", stores, dontStartListeners: true }); + await db.createFirestoreMetadataDocument({ + documentKey: "dqb", type: GroupDocument, kind: "testProfileStamp", owner: "class_c1", createdAt: 123 + }); + // Every kind a unit declares lands on this one profile, which is what makes the profile — not the + // kind — the cohort a migration can select on. + expect(setPayloads[0]).toMatchObject({ kind: "testProfileStamp", axisProfile: "classWide" }); + // The value is written, but `createFirestoreMetadataDocument` returns `IDocumentMetadata`, which does + // not declare it, so a consumer cannot read it back without widening a type first. The barrier is at + // the type level; see document-axis-profiles.test.ts for the runtime model staying clear of it. + }); + + it("does NOT stamp an axis profile on a personal document", async () => { + // Same gate as `kind`: only type:"group" documents are stamped, so nothing is written that would have + // to be migrated if the other types' kinds are reorganized. + const setPayloads: any[] = []; + mockFirestore.mockImplementation(() => ({ + doc: () => ({ + get: () => Promise.resolve({ exists: false }), + set: (data: any) => { setPayloads.push(data); return Promise.resolve(); } + }) + })); + await db.connect({ appMode: "test", stores, dontStartListeners: true }); + await db.createFirestoreMetadataDocument({ + documentKey: "pk", type: PersonalDocument, kind: PersonalDocument, owner: "user-1", createdAt: 123 + }); + expect(setPayloads[0]).not.toHaveProperty("axisProfile"); }); it("does NOT stamp kind/concurrent on a personal document", async () => { @@ -419,7 +460,7 @@ describe("db", () => { // getDocumentLocationFields returns the class `unit` (read from the stores' current unit). The authored title // is registered too, to prove it is resolved by kind and NOT persisted into the Firestore metadata. registerDocumentKind("drivingQuestionBoard", { - metadataFields: { concurrent: true }, ownerType: "class", containerType: "classUnit", + profile: kClassWideProfile, title: "Driving Question Board" }); // Rebuild stores with the classHash (→ context_id) and the current unit code the class-wide scope uses. @@ -481,7 +522,7 @@ describe("db", () => { // createDeclaredClassWideDocuments registers a declared kind before asking for its document, and // getDocumentOwner throws for an unregistered kind rather than defaulting the owner to the caller. registerDocumentKind("drivingQuestionBoard", { - metadataFields: { concurrent: true }, ownerType: "class", containerType: "classUnit", + profile: kClassWideProfile, title: "DQB", unit: "msu" }); }); diff --git a/src/lib/db.ts b/src/lib/db.ts index 2d512151cf..94b16eed3c 100644 --- a/src/lib/db.ts +++ b/src/lib/db.ts @@ -34,7 +34,7 @@ import { getSimpleDocumentPath, IDocumentMetadata, IGetImageDataParams, IPublishSupportParams } from "../../shared/shared"; import { getDocumentKindMetadataFields, getDocumentLocationFields, getDocumentOwner, getDocumentOwnerFields, - getDocumentOwnerType, IDocumentOwnerContext, registerClassWideDocumentKind + getDocumentAxisProfileName, getDocumentOwnerType, IDocumentOwnerContext, registerClassWideDocumentKind } from "../models/document/document-kinds"; import { getClassOwnerId } from "../models/document/document-axes"; import { getFirebaseFunction } from "../hooks/use-firebase-function"; @@ -126,6 +126,26 @@ interface IGetOrCreateCanonicalDocumentOpts { findLegacy?: () => Promise; } +/** + * The metadata shape written at creation: everything `IDocumentMetadata` declares, plus the fields only the + * write side knows about. + * + * `axisProfile` is deliberately absent from `IDocumentMetadata`, `DocumentMetadataModel`, and + * `DocumentModel`, so it is not reachable from the running app. It exists for migrations and offline + * analysis, which read Firestore directly. Leaving it undeclared is what keeps it from becoming a thing the + * runtime branches on — the axes stay the only way to ask how a document behaves, and a read of the profile + * would have to add the field to a type first, which is a reviewable act rather than an accident. + * + * Undeclared fields survive the trip: `DocumentMetadataStore` typechecks raw Firestore data against + * `DocumentMetadataModel`, and MST's `typecheck` ignores properties a model does not declare (pinned in + * src/models/mst.test.ts). `canonical` already relies on this. + */ +type IDocumentMetadataAtCreation = IDocumentMetadata & { + context_id: string; + network: string | null; + axisProfile?: string; +}; + interface ICreateFirestoreMetadataDocumentOpts { documentKey: string; type: DBDocumentType; @@ -582,7 +602,7 @@ export class DB { }); } - async createFirestoreMetadataDocument(opts: ICreateFirestoreMetadataDocumentOpts) { + async createFirestoreMetadataDocument(opts: ICreateFirestoreMetadataDocumentOpts): Promise { const { documentKey, type, kind, owner, createdAt, title } = opts; const { user } = this.stores; const userContext = this.stores.userContextProvider.userContext; @@ -627,7 +647,13 @@ export class DB { // is converted — see "Which documents get stamped" in docs/document-axes/target-architecture.md. const kindFields = type === GroupDocument ? getDocumentKindMetadataFields(kind) : {}; - const firestoreMetadata: IDocumentMetadata & { context_id: string; network: string | null } = { + // The axis profile the document is created at, recorded so a later migration can select every document + // made from one profile without querying the axis fields it is there to change. Gated with the kind + // fields above, for the same reason. + const profileName = type === GroupDocument ? getDocumentAxisProfileName(kind) : undefined; + const profileField = profileName ? { axisProfile: profileName } : {}; + + const firestoreMetadata: IDocumentMetadataAtCreation = { type, createdAt, // A creation-time snapshot that rules read back; storing it here is problematic — see the @@ -639,7 +665,8 @@ export class DB { ...titleInfo, ...ownerFields, ...locationFields, - ...kindFields + ...kindFields, + ...profileField }; await documentRef.set(firestoreMetadata); return firestoreMetadata; diff --git a/src/models/document/document-axis-profiles.test.ts b/src/models/document/document-axis-profiles.test.ts new file mode 100644 index 0000000000..3f83edf58f --- /dev/null +++ b/src/models/document/document-axis-profiles.test.ts @@ -0,0 +1,43 @@ +import { typecheck } from "mobx-state-tree"; +import { DocumentMetadataModel } from "./document-metadata-model"; +import { + kClassWideProfile, kDocumentAxisProfiles, kGroupProfile, kPersonalLikeProfile, kProblemLikeProfile +} from "./document-axis-profiles"; + +describe("document axis profiles", () => { + it("lists every profile, each with a distinct name", () => { + // The stored name identifies a document's cohort for migrations, so two profiles sharing one would + // merge cohorts that are not the same set of documents. + const names = kDocumentAxisProfiles.map(p => p.name); + expect(names).toEqual(["problemLike", "personalLike", "group", "classWide"]); + expect(new Set(names).size).toBe(names.length); + }); + + it("pins each profile's position on the axes", () => { + // These are the axis combinations the application creates documents at. A change here changes what + // some existing document means, so it should be a deliberate edit with a migration behind it. + expect(kProblemLikeProfile).toEqual({ + name: "problemLike", ownerType: "user", containerType: "offering" + }); + expect(kPersonalLikeProfile).toEqual({ + name: "personalLike", ownerType: "user", containerType: "class" + }); + expect(kGroupProfile).toEqual({ + name: "group", ownerType: "group", containerType: "offering", concurrent: true + }); + expect(kClassWideProfile).toEqual({ + name: "classWide", ownerType: "class", containerType: "classUnit", concurrent: true + }); + }); + + it("does not reach the runtime metadata model", () => { + // `axisProfile` is stamped into Firestore but declared on no runtime type, so the running app cannot + // read it and cannot come to depend on it. DocumentMetadataStore validates raw Firestore data against + // this model, so the field has to survive validation while staying undeclared. + expect(() => typecheck(DocumentMetadataModel, { + uid: "class_c1", type: "group", key: "dqb-1", axisProfile: "classWide" + } as any)).not.toThrow(); + expect(DocumentMetadataModel.create({ uid: "class_c1", type: "group", key: "dqb-1" })) + .not.toHaveProperty("axisProfile"); + }); +}); diff --git a/src/models/document/document-axis-profiles.ts b/src/models/document/document-axis-profiles.ts new file mode 100644 index 0000000000..ec82991d97 --- /dev/null +++ b/src/models/document/document-axis-profiles.ts @@ -0,0 +1,99 @@ +/** + * The axis profiles: every combination of axis values CLUE creates documents at. + * + * A profile is a named bundle of positions on the axes — who owns a document, where it is kept, what it + * is about, whether several people may write it (see docs/document-axes/axes.md). It is the thing people + * already mean by "a class-wide document" or "a problem document": a shorthand for a set of coordinates + * that no single stored field holds. + * + * Profiles are why a unit may add kinds but may not add axis combinations, which is what lets `kind` stay + * open-ended while the set of combinations stays closed. A unit config may declare a new kind, but every + * kind is registered against one of the profiles below, so a configuration can add a document to an + * existing combination and cannot invent a new one. This file is therefore the complete list of axis + * combinations the application supports — the one place to look to check that they are all intended and + * all handled. + * + * | profile | owner | container | curriculum | concurrent | kinds today | + * |----------------|-------|-----------|------------|------------|------------------------------------------------| + * | `problemLike` | user | offering | problem | no | problem, planning, problemPublication, support | + * | `personalLike` | user | class | none | no | personal, learningLog, and their publications | + * | `group` | group | offering | problem | yes | group | + * | `classWide` | class | classUnit | unit | yes | whatever units declare (driving question board) | + * + * The curriculum column is not declared here: `containerType` fixes it, because every container above the + * class is identified by a curriculum coordinate (see getDocumentLocationFields). + */ + +/** + * How a profile's `owner` axis (authoring identity / provenance, stored as the document's `uid`) is derived + * at creation: "user" → the creating user; "group" → the synthetic group owner (`group__`); + * "class" → the synthetic class owner (`class_`), shared by the whole class. + */ +export type DocumentOwnerType = "user" | "group" | "class"; + +/** + * Which container a profile's documents are kept in, and with it their curriculum reach: "class" → the + * class, about no unit; "classUnit" → the class's copy of one unit, about that unit; "offering" → one + * assignment of a problem to a class, about that problem. + * + * One knob sets the container and curriculum axes values because that is the most convenient. + */ +export type DocumentContainerType = "class" | "classUnit" | "offering"; + +export interface IDocumentAxisProfile { + /** + * The profile's name, stamped on every document created from it. + * + * Stored so a document records which profile it was made from, which is what a later migration selects + * on. Selecting instead by the axis values themselves would mean querying the very fields a migration + * is there to change, and would have to be rewritten whenever they moved. + */ + name: string; + /** How this profile's owner uid is derived. */ + ownerType: DocumentOwnerType; + /** Which container this profile's documents live in, and their curriculum reach. */ + containerType: DocumentContainerType; + /** Multi-writer, merged through the concurrent history manager. Absent means single-writer. */ + concurrent?: boolean; +} + +/** A user's own work on one assigned problem, and the read-only publications made from it. */ +export const kProblemLikeProfile: IDocumentAxisProfile = { + name: "problemLike", + ownerType: "user", + containerType: "offering" +}; + +/** A user's own work that follows them between assignments, about no particular content. */ +export const kPersonalLikeProfile: IDocumentAxisProfile = { + name: "personalLike", + ownerType: "user", + containerType: "class" +}; + +/** + * One group's shared document for one assignment. Kept in the offering like the problem documents beside + * it; what makes it the group's is its owner. + */ +export const kGroupProfile: IDocumentAxisProfile = { + name: "group", + ownerType: "group", + containerType: "offering", + concurrent: true +}; + +/** + * One document shared by a whole class for a whole unit. Differs from `group` on owner, container, and + * curriculum alone — which is the demonstration that the axes, not the type, decide how a document behaves. + */ +export const kClassWideProfile: IDocumentAxisProfile = { + name: "classWide", + ownerType: "class", + containerType: "classUnit", + concurrent: true +}; + +/** Every profile, for enumeration — tests, tooling, and anything auditing the set as a whole. */ +export const kDocumentAxisProfiles: readonly IDocumentAxisProfile[] = [ + kProblemLikeProfile, kPersonalLikeProfile, kGroupProfile, kClassWideProfile +]; diff --git a/src/models/document/document-kinds.test.ts b/src/models/document/document-kinds.test.ts index 5750c8373c..ae611e7466 100644 --- a/src/models/document/document-kinds.test.ts +++ b/src/models/document/document-kinds.test.ts @@ -1,6 +1,12 @@ -import { GroupDocument, PersonalDocument, ProblemDocument } from "./document-types"; import { - getDocumentKindInfo, getDocumentKindLabel, getDocumentKindMetadataFields, getDocumentOwner, + kClassWideProfile, kGroupProfile, kPersonalLikeProfile +} from "./document-axis-profiles"; +import { + GroupDocument, LearningLogDocument, PersonalDocument, ProblemDocument +} from "./document-types"; +import { + getDocumentAxisProfileName, getDocumentKindInfo, getDocumentKindLabel, getDocumentKindMetadataFields, + getDocumentOwner, registerClassWideDocumentKind, getDocumentOwnerFields, getDocumentOwnerType, getDocumentLocationFields, getDocumentTitle, getKindDefinitionFor, isValidDocumentKind, registerDocumentKind, resetDocumentKindRegistryForTests } from "./document-kinds"; @@ -19,7 +25,7 @@ describe("isValidDocumentKind", () => { describe("document kinds registry", () => { it("resolves the built-in group kind as concurrent", () => { - expect(getDocumentKindInfo(GroupDocument)?.metadataFields.concurrent).toBe(true); + expect(getDocumentKindInfo(GroupDocument)?.profile.concurrent).toBe(true); }); it("returns undefined for unregistered or missing kinds", () => { @@ -30,23 +36,23 @@ describe("document kinds registry", () => { it("registerDocumentKind adds new kinds", () => { registerDocumentKind("testAddedKind", - { metadataFields: { concurrent: true }, ownerType: "user", containerType: "class" }); - expect(getDocumentKindInfo("testAddedKind")?.metadataFields.concurrent).toBe(true); + { profile: kGroupProfile }); + expect(getDocumentKindInfo("testAddedKind")?.profile.concurrent).toBe(true); }); it("registerDocumentKind throws when a kind is registered more than once", () => { registerDocumentKind("testDuplicateKind", - { metadataFields: {}, ownerType: "user", containerType: "class" }); + { profile: kPersonalLikeProfile }); expect(() => registerDocumentKind("testDuplicateKind", - { metadataFields: {}, ownerType: "user", containerType: "class" })).toThrow(/already registered/); + { profile: kPersonalLikeProfile })).toThrow(/already registered/); // built-in kinds are registered at module load, so re-registering one throws too expect(() => registerDocumentKind(GroupDocument, - { metadataFields: { concurrent: true }, ownerType: "group", containerType: "offering" })).toThrow(); + { profile: kGroupProfile })).toThrow(); }); it("registerDocumentKind throws for a kind that is not a valid camelCase identifier", () => { expect(() => registerDocumentKind("not-camel-case", - { metadataFields: {}, ownerType: "user", containerType: "class" })).toThrow(/not a valid identifier/); + { profile: kPersonalLikeProfile })).toThrow(/not a valid identifier/); }); describe("getDocumentKindMetadataFields", () => { @@ -63,6 +69,32 @@ describe("document kinds registry", () => { }); }); + describe("getDocumentAxisProfileName", () => { + it("names the profile each built-in kind is created at", () => { + expect(getDocumentAxisProfileName(GroupDocument)).toBe("group"); + expect(getDocumentAxisProfileName(PersonalDocument)).toBe("personalLike"); + expect(getDocumentAxisProfileName(ProblemDocument)).toBe("problemLike"); + }); + + it("gives kinds that share a profile the same name", () => { + // What separates a personal document from a learning log is presentation, not any axis, so both + // belong to one migration cohort. + expect(getDocumentAxisProfileName(LearningLogDocument)) + .toBe(getDocumentAxisProfileName(PersonalDocument)); + }); + + it("names the class-wide profile for a kind a unit declares", () => { + // The config supplies no axis values, so every declared kind lands on this one profile. + registerClassWideDocumentKind("testProfileDqb", "DQB", "sas"); + expect(getDocumentAxisProfileName("testProfileDqb")).toBe("classWide"); + }); + + it("returns undefined for an unregistered kind", () => { + expect(getDocumentAxisProfileName("unregisteredKind")).toBeUndefined(); + expect(getDocumentAxisProfileName(undefined)).toBeUndefined(); + }); + }); + describe("owner", () => { const ctx = { userId: "u-1", groupOwnerId: "group_off_3", classOwnerId: "class_c1" }; @@ -88,7 +120,7 @@ describe("document kinds registry", () => { it("resolves a class kind to the class owner", () => { registerDocumentKind("testDqb", - { metadataFields: { concurrent: true }, ownerType: "class", containerType: "classUnit" }); + { profile: kClassWideProfile }); expect(getDocumentOwnerType("testDqb")).toBe("class"); expect(getDocumentOwner("testDqb", ctx)).toBe("class_c1"); }); @@ -99,7 +131,7 @@ describe("document kinds registry", () => { expect(() => getDocumentOwner(GroupDocument, { userId: "u-1" })) .toThrow(/Cannot create a group-owned document/); registerDocumentKind("testClassKindNoOwner", - { metadataFields: {}, ownerType: "class", containerType: "classUnit" }); + { profile: kClassWideProfile }); expect(() => getDocumentOwner("testClassKindNoOwner", { userId: "u-1" })) .toThrow(/Cannot create a class-owned document/); }); @@ -136,7 +168,7 @@ describe("document kinds registry", () => { it("returns the unit and context_id for a class-unit kind, stating the absent curriculum explicitly", () => { registerDocumentKind("testWordWall", - { metadataFields: { concurrent: true }, ownerType: "class", containerType: "classUnit" }); + { profile: kClassWideProfile }); expect(getDocumentLocationFields("testWordWall", ctx)).toEqual({ unit: "msu", context_id: "class-h", investigation: null, problem: null }); @@ -156,8 +188,8 @@ describe("document kinds registry", () => { describe("getKindDefinitionFor", () => { it("resolves a built-in kind for a document from any unit, since no unit declared it", () => { - expect(getKindDefinitionFor({ kind: GroupDocument, unit: "sas" })?.ownerType).toBe("group"); - expect(getKindDefinitionFor({ kind: GroupDocument })?.ownerType).toBe("group"); + expect(getKindDefinitionFor({ kind: GroupDocument, unit: "sas" })?.profile.ownerType).toBe("group"); + expect(getKindDefinitionFor({ kind: GroupDocument })?.profile.ownerType).toBe("group"); }); it("returns undefined when the kind is unregistered or absent", () => { @@ -169,7 +201,7 @@ describe("document kinds registry", () => { beforeEach(() => { resetDocumentKindRegistryForTests(); registerDocumentKind("testScopedKind", { - metadataFields: { concurrent: true }, ownerType: "class", containerType: "classUnit", + profile: kClassWideProfile, title: "Driving Question Board", unit: "sas" }); }); @@ -194,7 +226,7 @@ describe("document kinds registry", () => { describe("title", () => { it("returns a class-wide kind's registered static title", () => { registerDocumentKind("testDqbTitle", { - metadataFields: { concurrent: true }, ownerType: "class", containerType: "classUnit", + profile: kClassWideProfile, title: "Driving Question Board" }); expect(getDocumentTitle({ kind: "testDqbTitle", type: GroupDocument })).toBe("Driving Question Board"); @@ -224,7 +256,7 @@ describe("document kinds registry", () => { beforeEach(() => { resetDocumentKindRegistryForTests(); registerDocumentKind("testUnitDeclaredKind", { - metadataFields: { concurrent: true }, ownerType: "class", containerType: "classUnit", + profile: kClassWideProfile, title: "Driving Question Board", unit: "sas" }); }); @@ -265,9 +297,7 @@ describe("document kinds registry", () => { beforeEach(() => { resetDocumentKindRegistryForTests(); - registerDocumentKind("testClassWideKind", { - metadataFields: { concurrent: true }, ownerType: "class", containerType: "classUnit" - }); + registerDocumentKind("testClassWideKind", { profile: kClassWideProfile }); }); it("stamps the unit and class, and states the absent curriculum explicitly", () => { diff --git a/src/models/document/document-kinds.ts b/src/models/document/document-kinds.ts index 8104e4a98b..a236439b35 100644 --- a/src/models/document/document-kinds.ts +++ b/src/models/document/document-kinds.ts @@ -1,5 +1,9 @@ import { upperFirst } from "lodash"; import { IDocumentMetadata } from "../../../shared/shared"; +import { + DocumentOwnerType, IDocumentAxisProfile, kClassWideProfile, kGroupProfile, kPersonalLikeProfile, + kProblemLikeProfile +} from "./document-axis-profiles"; import { GroupDocument, LearningLogDocument, LearningLogPublication, PersonalDocument, PersonalPublication, PlanningDocument, @@ -12,31 +16,15 @@ import { */ export type IDocumentKindMetadataFields = Pick; -/** - * How a kind's `owner` axis (authoring identity / provenance, stored as the document's `uid`) is derived - * at creation: "user" → the creating user; "group" → the synthetic group owner (`group__`); - * "class" → the synthetic class owner (`class_`), shared by the whole class. Defaults to "user". - */ -export type DocumentOwnerType = "user" | "group" | "class"; - -/** - * Which container a kind's documents are kept in, and with it their curriculum reach: "class" → the class, - * about no unit; "classUnit" → the class's copy of one unit, about that unit; "offering" → one assignment of - * a problem to a class, about that problem. - * - * One knob sets the container and curriculum axes values because that is the most convenient. - */ -export type DocumentContainerType = "class" | "classUnit" | "offering"; - export interface IDocumentKindInfo { /** The kind key. Matches the value stored in a document's `kind` field. */ kind: string; - /** This kind's stamped fields, without `kind` — getDocumentKindMetadataFields adds it back. */ - metadataFields: Omit; - /** How this kind's owner uid is derived. */ - ownerType: DocumentOwnerType; - /** Which container this kind's documents live in, and their curriculum reach. */ - containerType: DocumentContainerType; + /** + * The axis profile this kind's documents are created at — where they sit on every axis. Several kinds + * share one: what distinguishes a personal document from a learning log is presentation and creation + * recipe, not any axis (see document-axis-profiles.ts). + */ + profile: IDocumentAxisProfile; /** * Static document display title. Leave undefined for dynamic titles. */ @@ -133,15 +121,24 @@ export function getKindDefinitionFor(doc: IKindScopedDocumentFields): IDocumentK return info; } -/** A kind's full stamp set (its metadataFields plus the `kind` key), or `{}` if the kind is unregistered. */ +/** A kind's full stamp set (the `kind` key plus the axis fields its profile fixes), or `{}` if unregistered. */ export function getDocumentKindMetadataFields(kind?: string|null): IDocumentKindMetadataFields { const info = getDocumentKindInfo(kind); if (!info) return {}; - return { kind: info.kind, ...info.metadataFields }; + return { kind: info.kind, ...(info.profile.concurrent ? { concurrent: true } : {}) }; +} + +/** + * The name of the axis profile a document of this kind is created at, or undefined if the kind is + * unregistered. Stamped onto the document as its record of which profile it was made from; see + * IDocumentAxisProfile.name for why that is stored rather than recomputed. + */ +export function getDocumentAxisProfileName(kind?: string|null): string | undefined { + return getDocumentKindInfo(kind)?.profile.name; } export function getDocumentOwnerType(kind?: string|null): DocumentOwnerType { - return getDocumentKindInfo(kind)?.ownerType ?? "user"; + return getDocumentKindInfo(kind)?.profile.ownerType ?? "user"; } /** @@ -161,7 +158,7 @@ export function getDocumentOwner(kind: string|null|undefined, ctx: IDocumentOwne if (!info) { throw new Error(`Cannot resolve the owner of unregistered document kind "${kind}"`); } - switch (info.ownerType) { + switch (info.profile.ownerType) { case "group": return requireOwnerId(ctx.groupOwnerId, kind, "group"); case "class": return requireOwnerId(ctx.classOwnerId, kind, "class"); case "user": return ctx.userId; @@ -218,7 +215,7 @@ export interface IDocumentLocationContext { export function getDocumentLocationFields( kind: string|null|undefined, ctx: IDocumentLocationContext ): IDocumentLocationContext { - switch (getDocumentKindInfo(kind)?.containerType) { + switch (getDocumentKindInfo(kind)?.profile.containerType) { case "classUnit": return { unit: ctx.unit, context_id: ctx.context_id, @@ -295,50 +292,28 @@ export function getDocumentKindLabel(kind?: string | null): string | undefined { /** * Register a kind declared by a unit's `classWideDocuments` configuration. Every class-wide collaborative - * document has the same shape — concurrent, owned by the synthetic class owner, kept in the class's copy of - * the unit and about that unit and nothing narrower — so only the kind key, the authored title, and the - * declaring unit come from the configuration. The title is registered rather than stored per document so it - * resolves live by kind (see getDocumentTitle). Throws like registerDocumentKind when the kind is malformed - * or already registered. + * document sits at the same place on every axis, so the configuration supplies no axis values at all — it + * names the kind and its title, and the class-wide profile supplies the rest. A unit config can therefore + * add a document to an existing axis combination but cannot invent one. The title is registered rather than + * stored per document so it resolves live by kind (see getDocumentTitle). Throws like registerDocumentKind + * when the kind is malformed or already registered. */ export function registerClassWideDocumentKind(kind: string, title: string, unit: string) { - registerDocumentKind(kind, { - metadataFields: { concurrent: true }, - ownerType: "class", - containerType: "classUnit", - title, - unit - }); + registerDocumentKind(kind, { profile: kClassWideProfile, title, unit }); } function registerBuiltInDocumentKinds() { - // A group document is kept in the offering, like the problem documents beside it; what makes it a group's - // is its owner, which is also where its stored `groupId` comes from (see getDocumentOwnerFields). - registerDocumentKind(GroupDocument, { - metadataFields: { concurrent: true }, - ownerType: "group", - containerType: "offering" - }); + registerDocumentKind(GroupDocument, { profile: kGroupProfile }); - const personalLikeKindInfo = { - metadataFields: { }, - ownerType: "user", - containerType: "class" - } as const; - registerDocumentKind(PersonalDocument, personalLikeKindInfo); - registerDocumentKind(LearningLogDocument, personalLikeKindInfo); - registerDocumentKind(PersonalPublication, personalLikeKindInfo); - registerDocumentKind(LearningLogPublication, personalLikeKindInfo); + registerDocumentKind(PersonalDocument, { profile: kPersonalLikeProfile }); + registerDocumentKind(LearningLogDocument, { profile: kPersonalLikeProfile }); + registerDocumentKind(PersonalPublication, { profile: kPersonalLikeProfile }); + registerDocumentKind(LearningLogPublication, { profile: kPersonalLikeProfile }); - const problemLikeKindInfo = { - metadataFields: { }, - ownerType: "user", - containerType: "offering" - } as const; - registerDocumentKind(PlanningDocument, problemLikeKindInfo); - registerDocumentKind(ProblemDocument, problemLikeKindInfo); - registerDocumentKind(ProblemPublication, problemLikeKindInfo); - registerDocumentKind(SupportPublication, problemLikeKindInfo); + registerDocumentKind(PlanningDocument, { profile: kProblemLikeProfile }); + registerDocumentKind(ProblemDocument, { profile: kProblemLikeProfile }); + registerDocumentKind(ProblemPublication, { profile: kProblemLikeProfile }); + registerDocumentKind(SupportPublication, { profile: kProblemLikeProfile }); } registerBuiltInDocumentKinds(); diff --git a/src/models/document/document-utils.test.ts b/src/models/document/document-utils.test.ts index c098a25553..1450008939 100644 --- a/src/models/document/document-utils.test.ts +++ b/src/models/document/document-utils.test.ts @@ -7,6 +7,7 @@ import { getGroupOwnerId } from "./document-axes"; import { ExemplarDocument, GroupDocument, PersonalDocument, ProblemDocument, ProblemPublication, SupportPublication } from "./document-types"; import { canUserEditDocument, getDocumentDisplayTitle, isDocumentAccessibleToUser } from "./document-utils"; +import { kClassWideProfile } from "./document-axis-profiles"; import { registerDocumentKind } from "./document-kinds"; import { unitConfigDefaults } from "../../test-fixtures/sample-unit-configurations"; @@ -195,7 +196,7 @@ describe("document utils", () => { test("a class-wide document uses its kind's registered title (resolved by kind, not stored)", () => { registerDocumentKind("testClassWideTitle", { - metadataFields: { concurrent: true }, ownerType: "class", containerType: "classUnit", + profile: kClassWideProfile, title: "Driving Question Board" }); const metadata = DocumentMetadataModel.create({ @@ -223,7 +224,7 @@ describe("document utils", () => { test("does not borrow the current unit's title for another unit's document of the same kind", () => { registerDocumentKind("testSharedKind", { - metadataFields: { concurrent: true }, ownerType: "class", containerType: "classUnit", + profile: kClassWideProfile, title: "Our Big Questions", unit: "test" }); const ownUnitDoc = DocumentMetadataModel.create({