From fb3dcd3f73ad6299a9403cdb848810727591f256 Mon Sep 17 00:00:00 2001 From: violet Date: Thu, 22 Oct 2026 12:25:04 -0400 Subject: [PATCH 1/7] Specify cosigning conference testimony Draft 1 of the cosign feature requirements: triggers, functional and non-functional requirements, failure behaviour and what is out of scope. The UI it describes is the client designer's /cosign-viz mockup, pinned at k-g-k/maple-conference@19bd101. Closes hyphacoop/maple#91 Co-Authored-By: Claude Opus 5.5 --- docs/cosign-feature-spec.md | 80 +++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 docs/cosign-feature-spec.md diff --git a/docs/cosign-feature-spec.md b/docs/cosign-feature-spec.md new file mode 100644 index 000000000..29f5c3ba4 --- /dev/null +++ b/docs/cosign-feature-spec.md @@ -0,0 +1,80 @@ +# Cosign feature requirements + +## Context + +MAPLE provides users with the ability to submit testimony on bills. + +We want to add a feature that allows a user to "cosign" someone else's +testimony, adding a +1 to what the other is saying, and an optional note +explaining why they are cosigning. The total number of cosigns is to appear +next to the testimony. This will be initially rolled out to conference +committee testimonies only. + +## 1. Purpose + +The user must be able to press a button to cosign testimony for a conference committee, and see that cosignature reflected. + +## 2. Triggers + +What triggers the feature? + +| ID | Trigger Type | Source | Resulting action | +| ---- | ---------------------------- | --------------- | ---------------- | +| TR-1 | Pressing the "cosign" button | User on Web UI | FR-1, FR-5 | +| TR-2 | Pressing the edit button | User on Web UI | FR-3 | +| TR-3 | Deleting parent testimony | Admin on Web UI | FR-4 | + +## 3. Functional Requirements + +| ID | Requirement | Trigger | Acceptance criterion | Priority | +| ---- | ------------------------------------------------------------------------------------------ | ------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------- | +| FR-1 | The cosign panel pops up and allows the user to cosign testimony | TR-1 | The testimony card that's been cosigned gains a "Cosigned" count, a number of districts, a last cosigned date, and the new testimony grouped under it (noteless cosigns appear only if "show cosigns without content" is ticked) | Must | +| FR-2 | Cosigned testimony appears in newsletters and the newsfeed | FR-1 | A new piece of cosigning testimony is included in the email update to followers of the conference or the cosigner | Must | +| FR-3 | An "edit cosign" view appears allowing the user to change the statement (and nothing else) | TR-2 | A piece of cosigned testimony has its justification sentence edited; it's not possible to change its position or to turn it into a plain testimony | Must | +| FR-4 | Deleting the parent testimony cascades to deleting cosigns | TR-3 | Deleting parent testimony deletes cosigns of it | Must | +| FR-5 | A logged out user that presses cosign is taken to the login flow | TR-1 | It's possible to initiate login by pressing cosign | Must | +| FR-6 | Cosigned testimony copies the parent's position, and the position is not editable | FR-1 | Editing a parent's position does not edit the cosign's, and the tally reflects this | Must | +| FR-7 | Cosigning a cosign or your own testimony is an error | FR-1 | There is no cosign button on co-signatures, or on the logged in user's testimony, and calling the server-side to do an invalid cosign fails | Must | +| FR-8 | The conference shows a bar with its position totals, cosigns included | FR-1 | On submitting a cosign, the conference's position tallies change | Must | + +## 4. Platform Constraints + +| ID | Constraint area | Requirement | +| ---- | --------------- | --------------------------------------------------------------------------------------------------------------------- | +| PC-1 | Infrastructure | Firestore data must be denormalized and querying can only happen through a set of indexes that may need to be updated | +| PC-2 | Infrastructure | Any server-side actions must be implemented as a cloud-function with clear triggers | + +## 5. Non-Functional Requirements + +| ID | Category | Requirement | Target | +| ----- | -------------- | ------------------------------------------------------------------------------------------------------------------------- | ------ | +| NFR-1 | Data integrity | No migration of existing records | N/A | +| NFR-2 | Consistency | Cosigns behave like other testimony vis-a-vis sharing and restrictions for posting (e.g. "only while conference is open") | N/A | + +## 6. Failure Behaviour + +| ID | Failure condition | Expected behaviour | Retry/backoff | Alert? | +| ---- | ---------------------------- | ------------------------------ | ------------- | ------ | +| FB-1 | Parent deleted mid-cosign | Cosign fails, user is told | N/A | No | +| FB-2 | Conference closes mid-cosign | Cosign fails, user is told | N/A | No | +| FB-3 | Double submission | Exactly one cosign is recorded | N/A | No | + +## 7. Open Questions + +| ID | Question | Status | Resolution | +| --- | -------- | ------ | ---------- | + +## 8. Out of Scope + +| ID | Category | Anti-Requirement | Target | +| ----- | -------------- | --------------------------------------------------------------------- | ------ | +| OOS-1 | Functional | No cosigns in non-conference bills | N/A | +| OOS-2 | Functional | No sending cosigns as a digest to the org whose testimony is cosigned | N/A | +| OOS-3 | Functional | No private notes for orgs | N/A | +| OOS-4 | Infrastructure | No ATProto scope whatsoever | N/A | + +## Change Log + +| Version | Date | Change | +| ------- | ---------- | ------------- | +| 0.1 | 2026-10-22 | Initial draft | From 24d6b551856f2105d6a05baa5e127eed80ef76ee Mon Sep 17 00:00:00 2001 From: violet Date: Tue, 27 Oct 2026 12:07:14 -0400 Subject: [PATCH 2/7] Record a cosign as testimony that points at its parent A cosign is conference testimony that names the publication it seconds, by id and author, in two new fields that are both set or both null. It is published through the same path as other testimony: publish reads the parent in its transaction, refuses a parent that is gone, the cosigner's own, itself a cosign, or on another conference, and copies the parent's position on first publish and keeps it on every edit. A published cosign stays a cosign of that parent, and plain testimony stays plain. A cosign's note may be empty, and it takes no attachment; other testimony still needs content. The Firestore rules already allow an empty draft, so they are unchanged. Existing testimony has no reference and reads as before. Closes hyphacoop/maple#93 Verified: yarn test:integration-ci on the six suites CI runs (83 passed), including new cosign cases for filing, note edits keeping the copied position after the parent changes, a double submit leaving one cosign, and each refusal; functions unit tests from src (128 passed); type checks (root errors only in untracked local scratch scripts); prettier check; lint on the changed files. Not verified: a cosign published against the dev backend. Co-Authored-By: Claude Opus 5.5 --- functions/src/testimony/positions.test.ts | 31 +++ functions/src/testimony/publishTestimony.ts | 66 ++++- functions/src/testimony/types.ts | 32 ++- tests/integration/testimony.test.ts | 284 +++++++++++++++++++- 4 files changed, 406 insertions(+), 7 deletions(-) diff --git a/functions/src/testimony/positions.test.ts b/functions/src/testimony/positions.test.ts index 43cdd912f..c0c73c335 100644 --- a/functions/src/testimony/positions.test.ts +++ b/functions/src/testimony/positions.test.ts @@ -40,3 +40,34 @@ describe("DraftTestimony target", () => { ).toBe(false) }) }) + +describe("DraftTestimony cosign", () => { + const conference = { + ...draft, + conferenceId: "data-privacy", + position: "passNothing" + } + const parent = { cosignOfId: "parent-1", cosignOfAuthorUid: "author-1" } + + it("lets a cosign have an empty note", () => { + expect( + DraftTestimony.guard({ ...conference, ...parent, content: "" }) + ).toBe(true) + expect(DraftTestimony.guard({ ...conference, content: "" })).toBe(false) + }) + + it("names both the parent's id and author", () => { + expect( + DraftTestimony.guard({ ...conference, cosignOfId: "parent-1" }) + ).toBe(false) + expect( + DraftTestimony.guard({ ...conference, cosignOfAuthorUid: "author-1" }) + ).toBe(false) + }) + + it("takes no attachment on a cosign", () => { + expect( + DraftTestimony.guard({ ...conference, ...parent, attachmentId: "a-1" }) + ).toBe(false) + }) +}) diff --git a/functions/src/testimony/publishTestimony.ts b/functions/src/testimony/publishTestimony.ts index 76638b679..788773651 100644 --- a/functions/src/testimony/publishTestimony.ts +++ b/functions/src/testimony/publishTestimony.ts @@ -72,6 +72,7 @@ class PublishTestimonyTransaction { private targetData?: DocumentData private publicationRef!: DocumentReference private currentPublication?: Testimony + private position!: Testimony["position"] private profile?: any private attachments!: PublishedAttachmentState @@ -84,6 +85,7 @@ class PublishTestimonyTransaction { async run() { await this.resolveDraft() await this.resolvePublication() + await this.resolveCosign() await this.resolveProfile() await this.resolveAttachments() @@ -98,14 +100,16 @@ class PublishTestimonyTransaction { billTitle: this.bill.content.Title, content: this.draft.content, court: this.draft.court, - position: this.draft.position, + position: this.position, ...publishInfo, attachmentId: this.attachments.id, draftAttachmentId: this.attachments.draftId, fullName: this.profile?.fullName ?? "Anonymous", public: this.profile?.public ?? true, ballotQuestionId: this.bqId, - conferenceId: this.conferenceId + conferenceId: this.conferenceId, + cosignOfId: this.draft.cosignOfId ?? null, + cosignOfAuthorUid: this.draft.cosignOfAuthorUid ?? null } if (this.profile?.representative?.id) { newPublication.representativeId = this.profile.representative.id @@ -241,6 +245,64 @@ class PublishTestimonyTransaction { this.bill = Bill.checkWithDefaults(billSnap.data()) } + /** A cosign seconds someone else's testimony on the same conference, and + * takes its position when first published. It stays a cosign of that + * testimony, and plain testimony stays plain. */ + private async resolveCosign() { + const { cosignOfId } = this.draft + const parent = cosignOfId ? await this.resolveParent() : undefined + const current = this.currentPublication + + if (current && (current.cosignOfId ?? null) !== (cosignOfId ?? null)) { + throw fail( + "failed-precondition", + current.cosignOfId + ? `Testimony ${current.id} cosigns ${current.cosignOfId} and stays a cosign of it` + : `Testimony ${current.id} is not a cosign and cannot become one` + ) + } + + this.position = parent + ? current?.position ?? parent.position + : this.draft.position + } + + private async resolveParent() { + const { cosignOfId, cosignOfAuthorUid } = this.draft + + if (!this.conferenceId) { + throw fail( + "failed-precondition", + "Only conference testimony can be cosigned" + ) + } + + const parentSnap = await this.t.get( + db.doc(`/users/${cosignOfAuthorUid}/publishedTestimony/${cosignOfId}`) + ) + if (!parentSnap.exists) { + throw fail( + "failed-precondition", + `Cosigned testimony ${cosignOfId} no longer exists` + ) + } + const parent = Testimony.checkWithDefaults(parentSnap.data()) + + if (parent.authorUid === this.uid) { + throw fail("failed-precondition", "You cannot cosign your own testimony") + } + if (parent.cosignOfId) { + throw fail("failed-precondition", "A cosign cannot be cosigned") + } + if (targetOf(parent).ref.path !== this.target.ref.path) { + throw fail( + "failed-precondition", + `Cosigned testimony ${cosignOfId} is not on conference ${this.conferenceId}` + ) + } + return parent + } + private async resolveProfile() { const ref = db.doc(`/profiles/${this.uid}`), profileSnap = await this.t.get(ref) diff --git a/functions/src/testimony/types.ts b/functions/src/testimony/types.ts index b2905c2c5..2fe5cdd2e 100644 --- a/functions/src/testimony/types.ts +++ b/functions/src/testimony/types.ts @@ -34,16 +34,20 @@ const BaseTestimony = R({ billId: Id, court: Number, position: Position, - content: RtString.withConstraint( - s => s.length > 0 && s.length < maxTestimonyLength - ), + /** Empty only on a cosign, whose note is optional. */ + content: RtString.withConstraint(s => s.length < maxTestimonyLength), attachmentId: Maybe(RtString), /** Only present if testimony was edited (has a version greater than 1) */ editReason: Maybe(RtString), ballotQuestionId: Maybe(RtString), /** Set when the testimony targets a committee of conference rather than the * bill alone. `billId` is then the conference's canonical bill. */ - conferenceId: Maybe(RtString) + conferenceId: Maybe(RtString), + /** Set when the testimony cosigns another: the cosigned publication's id + * and its author, which together locate it. A cosign takes its parent's + * position when first published and keeps it. */ + cosignOfId: Maybe(RtString), + cosignOfAuthorUid: Maybe(RtString) }) export type Testimony = Static @@ -107,12 +111,32 @@ export const oneTarget = ({ !(ballotQuestionId && conferenceId) || "Testimony targets a ballot question or a conference, not both" +/** A cosign names both its parent's id and author, and carries a note at + * most: no attachment, and possibly no content. Anything else has content. */ +export const cosignShape = ({ + content, + attachmentId, + cosignOfId, + cosignOfAuthorUid +}: { + content: string + attachmentId?: string | null + cosignOfId?: string | null + cosignOfAuthorUid?: string | null +}) => { + if (!cosignOfId !== !cosignOfAuthorUid) + return "A cosign names its parent's id and author" + if (cosignOfId) return !attachmentId || "A cosign takes no attachment" + return content.length > 0 || "Content is empty" +} + export type DraftTestimony = Static export const DraftTestimony = BaseTestimony.extend({ publishedVersion: Optional(Number) }) .withConstraint(positionMatchesTarget) .withConstraint(oneTarget) + .withConstraint(cosignShape) export const countsByPositions = { endorse: "endorseCount", diff --git a/tests/integration/testimony.test.ts b/tests/integration/testimony.test.ts index 0e39e836f..0082f3f6c 100644 --- a/tests/integration/testimony.test.ts +++ b/tests/integration/testimony.test.ts @@ -16,7 +16,8 @@ import { getProfile, signInTestAdmin, signInUser1, - signInUser2 + signInUser2, + signInUser3 } from "./common" type BaseTestimony = { @@ -35,6 +36,8 @@ type BaseTestimony = { editReason?: string ballotQuestionId?: string | null conferenceId?: string | null + cosignOfId?: string | null + cosignOfAuthorUid?: string | null } type DraftTestimony = BaseTestimony & { @@ -729,6 +732,264 @@ describe("publishTestimony", () => { publishTestimony({ draftId: conferenceDraftId }) ).rejects.toThrow(`takes testimony on ${billId}`) }) + + describe("cosign", () => { + let parentId: string, uid2: string + + // User 1 testifies on the conference, then user 2 signs in. + beforeEach(async () => { + const { draftId: parentDraftId } = await createConferenceDraft( + uid, + billId, + conferenceId, + "passSenateVersion" + ) + parentId = (await publishTestimony({ draftId: parentDraftId })).data + .publicationId + uid2 = (await signInUser2()).uid + }) + + const cosignOf = () => ({ cosignOfId: parentId, cosignOfAuthorUid: uid }) + + it("Files a cosign with no note, at its parent's position", async () => { + const { draftId: cosignDraftId } = await createCosignDraft( + uid2, + billId, + conferenceId, + cosignOf() + ) + const res = await publishTestimony({ draftId: cosignDraftId }) + + const publication = await getPublication(uid2, res.data.publicationId) + expect(publication.cosignOfId).toBe(parentId) + expect(publication.cosignOfAuthorUid).toBe(uid) + expect(publication.content).toBe("") + expect(publication.position).toBe("passSenateVersion") + + const conference = await getConference(conferenceId) + expect(conference.testimonyCount).toBe(2) + expect(conference.passSenateVersionCount).toBe(2) + expect(conference.passNothingCount).toBe(0) + }) + + it("Keeps the copied position through edits to the note and the parent", async () => { + const { draftId: cosignDraftId } = await createCosignDraft( + uid2, + billId, + conferenceId, + cosignOf() + ) + const res = await publishTestimony({ draftId: cosignDraftId }) + + await testDb + .doc(`/users/${uid}/publishedTestimony/${parentId}`) + .update({ position: "passHouseVersion" }) + await updateDoc(refs.draftTestimony(uid2, cosignDraftId), { + content: "Because it is right", + position: "passHouseVersion", + editReason: "Say why" + }) + await publishTestimony({ draftId: cosignDraftId }) + + const publication = await getPublication(uid2, res.data.publicationId) + expect(publication.content).toBe("Because it is right") + expect(publication.version).toBe(2) + expect(publication.position).toBe("passSenateVersion") + }) + + it("Records exactly one cosign on a double submit", async () => { + const { draftId: cosignDraftId } = await createCosignDraft( + uid2, + billId, + conferenceId, + cosignOf() + ) + await Promise.allSettled([ + publishTestimony({ draftId: cosignDraftId }), + publishTestimony({ draftId: cosignDraftId }) + ]) + + const published = await testDb + .collection(`/users/${uid2}/publishedTestimony`) + .where("conferenceId", "==", conferenceId) + .get() + expect(published.size).toBe(1) + expect((await getConference(conferenceId)).testimonyCount).toBe(2) + }) + + it("Refuses a cosign of a cosign", async () => { + const { draftId: cosignDraftId } = await createCosignDraft( + uid2, + billId, + conferenceId, + cosignOf() + ) + const cosignId = (await publishTestimony({ draftId: cosignDraftId })) + .data.publicationId + + await signInUser1() + const { draftId } = await createCosignDraft(uid, billId, conferenceId, { + cosignOfId: cosignId, + cosignOfAuthorUid: uid2 + }) + await expect(publishTestimony({ draftId })).rejects.toThrow( + "A cosign cannot be cosigned" + ) + }) + + it("Refuses a cosign of your own testimony", async () => { + await signInUser1() + const { draftId } = await createCosignDraft( + uid, + billId, + conferenceId, + cosignOf() + ) + await expect(publishTestimony({ draftId })).rejects.toThrow( + "cannot cosign your own testimony" + ) + }) + + it("Refuses a cosign whose parent is gone", async () => { + const { draftId } = await createCosignDraft( + uid2, + billId, + conferenceId, + cosignOf() + ) + await testDb + .doc(`/users/${uid}/publishedTestimony/${parentId}`) + .delete() + await expect(publishTestimony({ draftId })).rejects.toThrow( + "no longer exists" + ) + }) + + it("Refuses a cosign of testimony on another conference", async () => { + const otherConferenceId = await createConference(billId) + const { draftId } = await createCosignDraft( + uid2, + billId, + otherConferenceId, + cosignOf() + ) + await expect(publishTestimony({ draftId })).rejects.toThrow( + `is not on conference ${otherConferenceId}` + ) + }) + + it("Refuses a cosign of bill testimony", async () => { + const { draftId } = await createDraft(uid2, billId) + await updateDoc(refs.draftTestimony(uid2, draftId), cosignOf()) + await expect(publishTestimony({ draftId })).rejects.toThrow( + "Only conference testimony can be cosigned" + ) + }) + + it("Refuses a cosign on a closed conference", async () => { + const { draftId } = await createCosignDraft( + uid2, + billId, + conferenceId, + cosignOf() + ) + await testDb + .doc( + `/generalCourts/${currentGeneralCourt}/conferences/${conferenceId}` + ) + .update({ status: "reportFiled" }) + await expect(publishTestimony({ draftId })).rejects.toThrow( + "not accepting testimony" + ) + }) + + it("Refuses turning plain testimony into a cosign", async () => { + const { draftId } = await createConferenceDraft( + uid2, + billId, + conferenceId, + "passNothing" + ) + await publishTestimony({ draftId }) + await updateDoc(refs.draftTestimony(uid2, draftId), { + ...cosignOf(), + editReason: "Cosign instead" + }) + await expect(publishTestimony({ draftId })).rejects.toThrow( + "is not a cosign and cannot become one" + ) + }) + + it("Refuses turning a cosign into plain testimony", async () => { + const { draftId } = await createCosignDraft( + uid2, + billId, + conferenceId, + cosignOf() + ) + await publishTestimony({ draftId }) + await updateDoc(refs.draftTestimony(uid2, draftId), { + cosignOfId: null, + cosignOfAuthorUid: null, + content: "My own words", + editReason: "Testify instead" + }) + await expect(publishTestimony({ draftId })).rejects.toThrow( + `cosigns ${parentId} and stays a cosign of it` + ) + }) + + it("Refuses a cosign moving to another parent", async () => { + const { draftId } = await createCosignDraft( + uid2, + billId, + conferenceId, + cosignOf() + ) + await publishTestimony({ draftId }) + + const uid3 = (await signInUser3()).uid + const { draftId: otherDraftId } = await createConferenceDraft( + uid3, + billId, + conferenceId, + "passNothing" + ) + const otherParentId = ( + await publishTestimony({ draftId: otherDraftId }) + ).data.publicationId + + await signInUser2() + await updateDoc(refs.draftTestimony(uid2, draftId), { + cosignOfId: otherParentId, + cosignOfAuthorUid: uid3, + editReason: "Cosign another" + }) + await expect(publishTestimony({ draftId })).rejects.toThrow( + `cosigns ${parentId} and stays a cosign of it` + ) + }) + + it("Refuses an empty note on plain testimony, half a reference and an attachment", async () => { + const drafts = [ + { content: "" }, + { cosignOfId: parentId }, + { ...cosignOf(), attachmentId: "an-attachment" } + ] + for (const fields of drafts) { + const { draftId } = await createConferenceDraft( + uid2, + billId, + conferenceId, + "passNothing" + ) + await updateDoc(refs.draftTestimony(uid2, draftId), fields) + await expect(publishTestimony({ draftId })).rejects.toThrow( + "failed validation" + ) + } + }) + }) }) }) @@ -998,6 +1259,27 @@ async function createConferenceDraft( return { draftId } } +/** A cosign draft with no note. Its position is the client's guess; the + * server copies the parent's. */ +async function createCosignDraft( + uid: string, + billId: string, + conferenceId: string, + cosignOf: { cosignOfId: string; cosignOfAuthorUid: string } +) { + const { draftId } = await createConferenceDraft( + uid, + billId, + conferenceId, + "passNothing" + ) + await updateDoc(refs.draftTestimony(uid, draftId), { + content: "", + ...cosignOf + }) + return { draftId } +} + async function createDraft( uid: string, billId: string, From 8fa5d736b5244c6d6e1d51cf2109b6f1a6457ff9 Mon Sep 17 00:00:00 2001 From: violet Date: Tue, 27 Oct 2026 13:06:03 -0400 Subject: [PATCH 3/7] Record when testimony was first published publishedAt moves with every edit, so nothing on a publication said when it was first filed. Publish now sets firstPublishedAt on a first publication and keeps it through edits. Editing older testimony that lacks it leaves it missing rather than stamping the edit time, so the functions deploy and the backfill can run in either order. backfillTestimonyFirstPublishedAt fills it from the archive, which every publish has written to since 2022 and deletes keep. It takes the earliest archived version with the publication's id, or, for archives from before July 2022 that carry no id, with the same bill, court and target; the earlier wins. A version 1 publication with no archive is its own first. It has --dry-run, skips documents already set, and is listed in scripts/README.md; its integration test joins the CI list. Verified: yarn test:integration-ci on the six CI suites plus the new backfill suite (94 passed), including first publication kept through an edit, older testimony left for the backfill, and the backfill's id, pre-July-2022 scope, admin-deleted republish, version 1, already-set and dry-run cases; functions type check; root type check; prettier; lint on the changed files. Not verified: the backfill against dev or prod data, including how many publications it leaves with no first publication found. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/repo-checks.yml | 1 + functions/src/testimony/publishTestimony.ts | 6 + functions/src/testimony/types.ts | 6 +- scripts/README.md | 17 +++ .../backfillTestimonyFirstPublishedAt.ts | 94 ++++++++++++++++ .../backfillTestimonyFirstPublishedAt.test.ts | 104 ++++++++++++++++++ tests/integration/testimony.test.ts | 32 ++++++ 7 files changed, 259 insertions(+), 1 deletion(-) create mode 100644 scripts/firebase-admin/backfillTestimonyFirstPublishedAt.ts create mode 100644 tests/integration/backfillTestimonyFirstPublishedAt.test.ts diff --git a/.github/workflows/repo-checks.yml b/.github/workflows/repo-checks.yml index 50545d919..422d85f4e 100644 --- a/.github/workflows/repo-checks.yml +++ b/.github/workflows/repo-checks.yml @@ -91,3 +91,4 @@ jobs: tests/integration/profile.test.ts tests/integration/ballotQuestions.test.ts tests/integration/backfillTestimonyScope.test.ts + tests/integration/backfillTestimonyFirstPublishedAt.test.ts diff --git a/functions/src/testimony/publishTestimony.ts b/functions/src/testimony/publishTestimony.ts index 788773651..aad3b5905 100644 --- a/functions/src/testimony/publishTestimony.ts +++ b/functions/src/testimony/publishTestimony.ts @@ -111,6 +111,12 @@ class PublishTestimonyTransaction { cosignOfId: this.draft.cosignOfId ?? null, cosignOfAuthorUid: this.draft.cosignOfAuthorUid ?? null } + // Only a first publication knows when that was; on older testimony an edit + // leaves the field for the backfill to fill from the archive. + const firstPublishedAt = this.currentPublication + ? this.currentPublication.firstPublishedAt + : publishInfo.publishedAt + if (firstPublishedAt) newPublication.firstPublishedAt = firstPublishedAt if (this.profile?.representative?.id) { newPublication.representativeId = this.profile.representative.id } diff --git a/functions/src/testimony/types.ts b/functions/src/testimony/types.ts index 2fe5cdd2e..b0918abe9 100644 --- a/functions/src/testimony/types.ts +++ b/functions/src/testimony/types.ts @@ -67,7 +67,11 @@ export const Testimony = withDefaults( senatorDistrict: Optional(RtString), representativeDistrict: Optional(RtString), draftAttachmentId: Maybe(RtString), - fullName: RtString + fullName: RtString, + /** When the testimony was first published. Edits keep it, unlike + * `publishedAt`. Missing on testimony published before it was recorded, + * until backfillTestimonyFirstPublishedAt fills it from the archive. */ + firstPublishedAt: Optional(InstanceOf(Timestamp)) }), { authorRole: "user", diff --git a/scripts/README.md b/scripts/README.md index 178215621..39baaaa40 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -45,6 +45,23 @@ yarn firebase-admin run-script backfillTestimonyScope --env dev --- +#### `backfillTestimonyFirstPublishedAt` + +Sets `firstPublishedAt` on every `publishedTestimony` document missing it, from the earliest archived version of that publication. A version 1 publication with no archive gets its own `publishedAt`. + +**Why it exists:** `publishedAt` moves with every edit, so it doesn't say when testimony was first published. Publish records `firstPublishedAt` only on a first publication, so testimony published before then lacks it. The cosign tally's last-cosigned time reads it. + +**Idempotent:** Fields already set are left alone. `--dry-run` prints the counts and writes nothing. Each write runs the testimony triggers once; the version and text are unchanged, so none of them notify, but testimony that was never embedded gets embedded. + +**When to run:** Per environment, dev then prod, any time after the functions deploy. Can be deleted once run everywhere. + +```sh +yarn firebase-admin run-script backfillTestimonyFirstPublishedAt --env dev --dry-run +yarn firebase-admin run-script backfillTestimonyFirstPublishedAt --env dev +``` + +--- + #### `syncBallotQuestions` Upserts ballot question records from local YAML files into the `ballotQuestions` Firestore collection. Each file is validated against the `BallotQuestion` type before being written; invalid files abort with an error. All writes are committed atomically in a single Firestore batch. diff --git a/scripts/firebase-admin/backfillTestimonyFirstPublishedAt.ts b/scripts/firebase-admin/backfillTestimonyFirstPublishedAt.ts new file mode 100644 index 000000000..be69e892d --- /dev/null +++ b/scripts/firebase-admin/backfillTestimonyFirstPublishedAt.ts @@ -0,0 +1,94 @@ +/* + Backfills firstPublishedAt onto every publishedTestimony missing it, from the + earliest archived version of that publication. + + Every publish archives a copy under the author's archivedTestimony, and + deletes keep the archives. Archives carry the publication's id since July + 2022, so a publication an admin deleted doesn't lend its date to the + author's later one. Archives from before then have no id and are matched by + bill, court and target; the earlier match wins, since testimony first + published before July 2022 has both. A version 1 publication with no + archive is its own first publication. + + Safe to re-run: fields already set are left alone. Pass --dry-run to count + without writing. Can be removed after running against all environments. +*/ +import { minBy } from "lodash" +import { Boolean, Optional, Record } from "runtypes" +import { Timestamp } from "../../functions/src/firebase" +import { Script } from "./types" + +const Args = Record({ dryRun: Optional(Boolean) }) + +const scopeFields = ["billId", "court", "ballotQuestionId", "conferenceId"] + +/** Archives and publications that share these share a bill and target. */ +const scopeKey = (data: FirebaseFirestore.DocumentData) => + JSON.stringify(scopeFields.map(f => data[f] ?? null)) + +const earlier = (...times: (Timestamp | undefined)[]) => + minBy(times, t => t?.toMillis()) + +const authorOf = (doc: FirebaseFirestore.DocumentSnapshot) => + doc.ref.parent.parent!.id + +export const script: Script = async ({ db, args }) => { + const { dryRun = false } = Args.check(args) + const writer = db.bulkWriter() + + const [published, archived] = await Promise.all([ + db + .collectionGroup("publishedTestimony") + .select("firstPublishedAt", "publishedAt", "version", ...scopeFields) + .get(), + db + .collectionGroup("archivedTestimony") + .select("id", "publishedAt", ...scopeFields) + .get() + ]) + + // The earliest archived publishedAt per author and publication id, and for + // archives with no id, per author and scope. + const byId = new Map() + const byScope = new Map() + archived.forEach(doc => { + const data = doc.data() + const author = authorOf(doc) + const [map, key] = data.id + ? [byId, `${author}/${data.id}`] + : [byScope, `${author}/${scopeKey(data)}`] + const first = earlier(map.get(key), data.publishedAt) + if (first) map.set(key, first) + }) + + const counts = { fromArchive: 0, fromVersion1: 0, none: 0 } + published.forEach(doc => { + const data = doc.data() + const author = authorOf(doc) + if (data.firstPublishedAt) return + + const fromArchive = earlier( + byId.get(`${author}/${doc.id}`), + byScope.get(`${author}/${scopeKey(data)}`) + ) + const fromVersion1 = data.version === 1 ? data.publishedAt : undefined + const source = fromArchive + ? "fromArchive" + : fromVersion1 + ? "fromVersion1" + : "none" + counts[source]++ + if (source !== "none" && !dryRun) + writer.update(doc.ref, { firstPublishedAt: fromArchive ?? fromVersion1 }) + }) + + await writer.close() + const set = counts.fromArchive + counts.fromVersion1 + console.log( + `publishedTestimony: ${published.size} documents, already set ${ + published.size - set - counts.none + }; ${dryRun ? "would set" : "set"} ${counts.fromArchive} from archives, ${ + counts.fromVersion1 + } from version 1; ${counts.none} with no first publication found` + ) +} diff --git a/tests/integration/backfillTestimonyFirstPublishedAt.test.ts b/tests/integration/backfillTestimonyFirstPublishedAt.test.ts new file mode 100644 index 000000000..2d719cbb5 --- /dev/null +++ b/tests/integration/backfillTestimonyFirstPublishedAt.test.ts @@ -0,0 +1,104 @@ +import { Timestamp } from "firebase-admin/firestore" +import { testDb, terminateFirebase } from "../testUtils" +import { script } from "../../scripts/firebase-admin/backfillTestimonyFirstPublishedAt" + +const user = testDb.collection("users").doc("backfill-first-published-user") +const published = user.collection("publishedTestimony") +const archived = user.collection("archivedTestimony") +const at = (day: number) => Timestamp.fromMillis(day * 86_400_000) + +/** A publication complete enough that the testimony triggers handle it. */ +const publication = (billId: string, version: number, day: number) => ({ + billId, + court: 193, + version, + publishedAt: at(day), + billTitle: "An Act", + authorUid: "backfill-first-published-user", + authorRole: "user", + fullName: "A Tester", + position: "endorse", + content: "Testimony" +}) +/** An archived version; those from before July 2022 have no id. */ +const archive = (billId: string, day: number, id?: string) => ({ + billId, + court: 193, + publishedAt: at(day), + ...(id && { id }) +}) + +const docs = { + // Edited twice, with archives that carry its id. + edited: published.doc("edited"), + editedV1: archived.doc("edited-v1"), + editedV3: archived.doc("edited-v3"), + // First published before archives carried an id, edited after. + legacy: published.doc("legacy"), + legacyV1: archived.doc("legacy-v1"), + legacyV2: archived.doc("legacy-v2"), + // Republished after an admin deleted the first publication. + republished: published.doc("republished"), + deletedV1: archived.doc("deleted-v1"), + republishedV2: archived.doc("republished-v2"), + // No archive at all. + unarchived: published.doc("unarchived"), + alreadySet: published.doc("already-set") +} + +const run = (args = {}) => + script({ db: testDb, args: { env: "local", argv: [], ...args } } as any) + +const firstPublishedAt = async (d: FirebaseFirestore.DocumentReference) => + (await d.get()).data()?.firstPublishedAt + +beforeEach(async () => { + await Promise.all([ + docs.edited.set(publication("H1", 3, 30)), + docs.editedV1.set(archive("H1", 10, "edited")), + docs.editedV3.set(archive("H1", 30, "edited")), + docs.legacy.set(publication("H2", 2, 20)), + docs.legacyV1.set(archive("H2", 5)), + docs.legacyV2.set(archive("H2", 20, "legacy")), + docs.republished.set(publication("H3", 2, 40)), + docs.deletedV1.set(archive("H3", 1, "deleted")), + docs.republishedV2.set(archive("H3", 40, "republished")), + docs.unarchived.set(publication("H4", 1, 50)), + docs.alreadySet.set({ + ...publication("H5", 2, 70), + firstPublishedAt: at(60) + }) + ]) +}) + +afterAll(async () => { + await Promise.all(Object.values(docs).map(d => d.delete())) + await terminateFirebase() +}) + +it("sets each publication's earliest archived version", async () => { + await run() + + expect(await firstPublishedAt(docs.edited)).toEqual(at(10)) + expect(await firstPublishedAt(docs.legacy)).toEqual(at(5)) + expect(await firstPublishedAt(docs.republished)).toEqual(at(40)) +}) + +it("takes a version 1 publication with no archive as its own first", async () => { + await run() + + expect(await firstPublishedAt(docs.unarchived)).toEqual(at(50)) +}) + +it("leaves a first publication time already set", async () => { + await run() + + expect(await firstPublishedAt(docs.alreadySet)).toEqual(at(60)) +}) + +it("writes nothing on a dry run", async () => { + await run({ dryRun: true }) + + for (const d of [docs.edited, docs.legacy, docs.unarchived]) + expect(await firstPublishedAt(d)).toBeUndefined() +}) diff --git a/tests/integration/testimony.test.ts b/tests/integration/testimony.test.ts index 0082f3f6c..1f5742dca 100644 --- a/tests/integration/testimony.test.ts +++ b/tests/integration/testimony.test.ts @@ -49,6 +49,7 @@ type Testimony = BaseTestimony & { authorDisplayName: string version: number publishedAt: Timestamp + firstPublishedAt?: Timestamp } const refs = { @@ -289,6 +290,37 @@ describe("publishTestimony", () => { await getDoc(doc(firestore, `/users/${uid}/archivedTestimony/test-id`)) }) + it("Keeps the first publication time through edits", async () => { + const { publicationId } = (await publishTestimony({ draftId })).data + const first = await getPublication(uid, publicationId) + expect(first.firstPublishedAt).toEqual(first.publishedAt) + + await updateDoc(refs.draftTestimony(uid, draftId), { + content: "updated content", + editReason: "edit reason" + }) + await publishTestimony({ draftId }) + const edited = await getPublication(uid, publicationId) + expect(edited.publishedAt).not.toEqual(first.publishedAt) + expect(edited.firstPublishedAt).toEqual(first.firstPublishedAt) + }) + + it("Leaves the first publication time to the backfill on older testimony", async () => { + const { publicationId } = (await publishTestimony({ draftId })).data + await testDb + .doc(`/users/${uid}/publishedTestimony/${publicationId}`) + .update({ firstPublishedAt: FieldValue.delete() }) + + await updateDoc(refs.draftTestimony(uid, draftId), { + content: "updated content", + editReason: "edit reason" + }) + await publishTestimony({ draftId }) + expect(await getPublication(uid, publicationId)).not.toHaveProperty( + "firstPublishedAt" + ) + }) + describe("attachments", () => { it("copies drafts to published and archived files", async () => { const attachmentId = nanoid() From eb3a881fa7d3e8390802bf6cb175b963b12e5231 Mon Sep 17 00:00:00 2001 From: violet Date: Tue, 27 Oct 2026 13:06:17 -0400 Subject: [PATCH 4/7] Keep a cosign tally on the testimony being cosigned Cosigned testimony now carries how many have cosigned it, how many distinct Senate and House districts they come from, and when the latest cosign was first published. A trigger on published testimony recounts them from the cosigns whenever a write changes what a cosign contributes, inside a transaction that reads the parent and every cosign of it. Retried or reordered events therefore land on the same figures, and an edit to a note writes nothing. A cosigner with no district on file counts toward the total only. The recount reads cosigns through a new collection-group index on cosignOfId. Publish replaces the publication on every version, so it now carries the tally across when the parent's author edits. It also records the author's districts, which it never had: it read profile fields that don't exist rather than the district on the profile's senator and representative. Closes hyphacoop/maple#94 Verified: yarn test:integration-ci on the six CI suites plus the backfill suite, with Typesense running as in CI (94 passed), including the tally following cosigns published from shared and distinct districts and with none, a note edit not counting as a new cosign, an admin deleting the newest cosign, the parent's author editing, and districts recorded on plain testimony; cosignTally unit tests and the rest of functions src (132 passed); functions and root type checks; prettier; lint on the changed files. Not verified: the trigger or the cosignOfId index on the dev backend, or under a burst of concurrent cosigns. Co-Authored-By: Claude Opus 5.5 --- firestore.indexes.json | 11 ++ functions/src/index.ts | 1 + functions/src/testimony/cosignTally.test.ts | 59 +++++++ functions/src/testimony/cosignTally.ts | 96 +++++++++++ functions/src/testimony/index.ts | 1 + functions/src/testimony/publishTestimony.ts | 13 +- functions/src/testimony/types.ts | 7 +- tests/integration/testimony.test.ts | 176 +++++++++++++++++++- 8 files changed, 356 insertions(+), 8 deletions(-) create mode 100644 functions/src/testimony/cosignTally.test.ts create mode 100644 functions/src/testimony/cosignTally.ts diff --git a/firestore.indexes.json b/firestore.indexes.json index 97950fce8..e74757bea 100644 --- a/firestore.indexes.json +++ b/firestore.indexes.json @@ -1257,6 +1257,17 @@ "queryScope": "COLLECTION_GROUP" } ] + }, + { + "collectionGroup": "publishedTestimony", + "fieldPath": "cosignOfId", + "ttl": false, + "indexes": [ + { + "order": "ASCENDING", + "queryScope": "COLLECTION_GROUP" + } + ] } ] } diff --git a/functions/src/index.ts b/functions/src/index.ts index 5a217d6f5..88efad2e1 100644 --- a/functions/src/index.ts +++ b/functions/src/index.ts @@ -38,6 +38,7 @@ export { checkSearchIndexVersion, searchHealthCheck } from "./search" export { deleteTestimonyV2, publishTestimonyV2, + updateCosignTally, runTestimonyBackfillChunk, syncTestimonyToSearchIndex, upgradeTestimonySearchIndex, diff --git a/functions/src/testimony/cosignTally.test.ts b/functions/src/testimony/cosignTally.test.ts new file mode 100644 index 000000000..6b6844590 --- /dev/null +++ b/functions/src/testimony/cosignTally.test.ts @@ -0,0 +1,59 @@ +import { Timestamp } from "../firebase" +import { cosignTally } from "./cosignTally" + +const at = (millis: number) => Timestamp.fromMillis(millis) + +describe("cosignTally", () => { + it("tallies no cosigns as zero, with no last cosign", () => { + expect(cosignTally([])).toEqual({ + cosignCount: 0, + cosignSenateDistrictCount: 0, + cosignHouseDistrictCount: 0, + lastCosignedAt: undefined + }) + }) + + it("counts distinct districts per chamber", () => { + expect( + cosignTally([ + { + senatorDistrict: "First Essex", + representativeDistrict: "3rd Essex", + firstPublishedAt: at(1) + }, + { + senatorDistrict: "First Essex", + representativeDistrict: "4th Essex", + firstPublishedAt: at(2) + } + ]) + ).toMatchObject({ + cosignCount: 2, + cosignSenateDistrictCount: 1, + cosignHouseDistrictCount: 2 + }) + }) + + it("counts a cosigner with no district toward the total only", () => { + expect( + cosignTally([ + { senatorDistrict: "First Essex", firstPublishedAt: at(1) }, + { firstPublishedAt: at(2) } + ]) + ).toMatchObject({ + cosignCount: 2, + cosignSenateDistrictCount: 1, + cosignHouseDistrictCount: 0 + }) + }) + + it("takes the latest first filing as the last cosign", () => { + expect( + cosignTally([ + { firstPublishedAt: at(2000) }, + { firstPublishedAt: at(3000) }, + { firstPublishedAt: at(1000) } + ]).lastCosignedAt + ).toEqual(at(3000)) + }) +}) diff --git a/functions/src/testimony/cosignTally.ts b/functions/src/testimony/cosignTally.ts new file mode 100644 index 000000000..3fa6799f1 --- /dev/null +++ b/functions/src/testimony/cosignTally.ts @@ -0,0 +1,96 @@ +import { onDocumentWritten } from "firebase-functions/v2/firestore" +import { isEqual, maxBy, pick } from "lodash" +import { db, FieldValue } from "../firebase" +import { Testimony } from "./types" + +/** The figures cosigned testimony carries about its cosigns. */ +export const cosignTallyFields = [ + "cosignCount", + "cosignSenateDistrictCount", + "cosignHouseDistrictCount", + "lastCosignedAt" +] as const + +/** What the tally reads from each cosign. */ +const cosignFields = [ + "senatorDistrict", + "representativeDistrict", + "firstPublishedAt" +] as const + +type Cosign = Pick + +/** Tallies a testimony's cosigns: how many, from how many distinct Senate and + * House districts, and when the latest was first filed. A cosigner with no + * district on file counts toward the total only. With no cosigns there is no + * `lastCosignedAt`. */ +export function cosignTally(cosigns: Cosign[]) { + const distinct = (districts: (string | undefined)[]) => + new Set(districts.filter(Boolean)).size + const lastCosignedAt = + maxBy(cosigns, c => c.firstPublishedAt?.toMillis())?.firstPublishedAt ?? + undefined + return { + cosignCount: cosigns.length, + cosignSenateDistrictCount: distinct(cosigns.map(c => c.senatorDistrict)), + cosignHouseDistrictCount: distinct( + cosigns.map(c => c.representativeDistrict) + ), + ...(lastCosignedAt && { lastCosignedAt }) + } +} + +/** Keeps the tally on cosigned testimony. It recounts from the cosigns on + * every write that changes what a cosign contributes, so a retried or + * reordered event lands on the same figures. */ +export const updateCosignTally = onDocumentWritten( + { + document: "/users/{uid}/publishedTestimony/{testimonyId}", + timeoutSeconds: 30, + retry: true + }, + async event => { + const change = event.data + if (!change) return + const cosign = (change.after.data() ?? change.before.data()) as + | Pick + | undefined + if (!cosign?.cosignOfId || !cosign.cosignOfAuthorUid) return + const { cosignOfId, cosignOfAuthorUid } = cosign + // An edit to the note leaves the tally as it was. + if ( + change.before.exists && + change.after.exists && + isEqual( + pick(change.before.data(), cosignFields), + pick(change.after.data(), cosignFields) + ) + ) + return + + await db.runTransaction(async t => { + const parentRef = db.doc( + `/users/${cosignOfAuthorUid}/publishedTestimony/${cosignOfId}` + ) + const [parentSnap, cosigns] = await Promise.all([ + t.get(parentRef), + t.get( + db + .collectionGroup("publishedTestimony") + .where("cosignOfId", "==", cosignOfId) + .select(...cosignFields) + ) + ]) + // A deleted parent takes its cosigns with it. + if (!parentSnap.exists) return + + const tally = cosignTally(cosigns.docs.map(d => d.data() as Cosign)) + if (isEqual(pick(parentSnap.data(), cosignTallyFields), tally)) return + + t.update(parentRef, { + ...tally, + lastCosignedAt: tally.lastCosignedAt ?? FieldValue.delete() + }) + }) + } +) diff --git a/functions/src/testimony/index.ts b/functions/src/testimony/index.ts index 75e50c3f7..db945ec2f 100644 --- a/functions/src/testimony/index.ts +++ b/functions/src/testimony/index.ts @@ -2,3 +2,4 @@ export * from "./deleteTestimony" export * from "./publishTestimony" export * from "./search" export * from "./resolveReport" +export * from "./cosignTally" diff --git a/functions/src/testimony/publishTestimony.ts b/functions/src/testimony/publishTestimony.ts index aad3b5905..7dc6388c1 100644 --- a/functions/src/testimony/publishTestimony.ts +++ b/functions/src/testimony/publishTestimony.ts @@ -6,6 +6,7 @@ import { import { onCall } from "firebase-functions/v2/https" import type { CallableRequest } from "firebase-functions/v2/https" import * as logger from "firebase-functions/logger" +import { pick } from "lodash" import { nanoid } from "nanoid" import { Record } from "runtypes" import { Bill } from "../bills/types" @@ -14,6 +15,7 @@ import { checkAuth, checkRequest, DocUpdate, fail, Id } from "../common" import { db, FieldValue, Timestamp } from "../firebase" import { supportedGeneralCourts, takesTestimony } from "../shared" import { Attachments, PublishedAttachmentState } from "./attachments" +import { cosignTallyFields } from "./cosignTally" import { targetCountsUpdate, targetOf, TestimonyTarget } from "./target" import { DraftTestimony, Testimony } from "./types" @@ -123,10 +125,10 @@ class PublishTestimonyTransaction { if (this.profile?.senator?.id) { newPublication.senatorId = this.profile.senator.id } - if (this.profile?.senatorDistrict) { + if (this.profile?.senator?.district) { newPublication.senatorDistrict = this.profile.senator.district } - if (this.profile?.representativeDistrict) { + if (this.profile?.representative?.district) { newPublication.representativeDistrict = this.profile.representative.district } @@ -142,8 +144,13 @@ class PublishTestimonyTransaction { } } + /** The tally of cosigns belongs to the cosigns' trigger, so a new version + * keeps it. The archive records what the author wrote, without it. */ private setPublication(newPublication: Testimony) { - this.t.set(this.publicationRef, newPublication) + this.t.set(this.publicationRef, { + ...newPublication, + ...pick(this.currentPublication, cosignTallyFields) + }) } private createArchive(newPublication: Testimony) { diff --git a/functions/src/testimony/types.ts b/functions/src/testimony/types.ts index b0918abe9..bfb920fb6 100644 --- a/functions/src/testimony/types.ts +++ b/functions/src/testimony/types.ts @@ -71,7 +71,12 @@ export const Testimony = withDefaults( /** When the testimony was first published. Edits keep it, unlike * `publishedAt`. Missing on testimony published before it was recorded, * until backfillTestimonyFirstPublishedAt fills it from the archive. */ - firstPublishedAt: Optional(InstanceOf(Timestamp)) + firstPublishedAt: Optional(InstanceOf(Timestamp)), + /** On cosigned testimony, the tally of its cosigns. See cosignTally. */ + cosignCount: Optional(Number), + cosignSenateDistrictCount: Optional(Number), + cosignHouseDistrictCount: Optional(Number), + lastCosignedAt: Optional(InstanceOf(Timestamp)) }), { authorRole: "user", diff --git a/tests/integration/testimony.test.ts b/tests/integration/testimony.test.ts index 1f5742dca..a5c0c31de 100644 --- a/tests/integration/testimony.test.ts +++ b/tests/integration/testimony.test.ts @@ -1,3 +1,4 @@ +import { waitFor } from "@testing-library/react" import { currentGeneralCourt } from "functions/src/shared" import { FieldValue } from "firebase-admin/firestore" import { signOut, User } from "firebase/auth" @@ -17,7 +18,8 @@ import { signInTestAdmin, signInUser1, signInUser2, - signInUser3 + signInUser3, + signInUser4 } from "./common" type BaseTestimony = { @@ -50,6 +52,7 @@ type Testimony = BaseTestimony & { version: number publishedAt: Timestamp firstPublishedAt?: Timestamp + cosignCount?: number } const refs = { @@ -766,16 +769,16 @@ describe("publishTestimony", () => { }) describe("cosign", () => { - let parentId: string, uid2: string + let parentId: string, parentDraftId: string, uid2: string // User 1 testifies on the conference, then user 2 signs in. beforeEach(async () => { - const { draftId: parentDraftId } = await createConferenceDraft( + ;({ draftId: parentDraftId } = await createConferenceDraft( uid, billId, conferenceId, "passSenateVersion" - ) + )) parentId = (await publishTestimony({ draftId: parentDraftId })).data .publicationId uid2 = (await signInUser2()).uid @@ -1021,6 +1024,171 @@ describe("publishTestimony", () => { ) } }) + + describe("tally", () => { + // Cosigners' districts, set on their profiles and restored after. + let savedProfiles: [ReturnType, any][] + const setDistricts = async ( + uid: string, + senate?: string, + house?: string + ) => { + const ref = testDb.doc(`/profiles/${uid}`) + savedProfiles.push([ref, (await ref.get()).data()]) + const member = (district: string) => ({ + id: `member-${district}`, + name: "A Member", + district + }) + await ref.set( + { + senator: senate ? member(senate) : FieldValue.delete(), + representative: house ? member(house) : FieldValue.delete() + }, + { merge: true } + ) + } + beforeEach(() => { + savedProfiles = [] + }) + afterEach(async () => { + for (const [ref, profile] of savedProfiles) { + if (profile) await ref.set(profile) + else await ref.delete() + } + }) + + // A trigger writes the tally, and the emulator runs triggers one at a + // time, so the first wait sits behind every earlier test's events. + const tallyTimeout = 60_000 + const getParent = () => getPublication(uid, parentId) + const expectTally = (tally: object) => + waitFor(async () => expect(await getParent()).toMatchObject(tally), { + interval: 100, + timeout: 30_000 + }) + const cosign = async (signIn: () => Promise) => { + const cosigner = (await signIn()).uid + const { draftId } = await createCosignDraft( + cosigner, + billId, + conferenceId, + cosignOf() + ) + const { publicationId } = (await publishTestimony({ draftId })).data + return { + uid: cosigner, + draftId, + publicationId, + publication: await getPublication(cosigner, publicationId) + } + } + + it( + "Counts cosigns and their distinct districts on the parent", + async () => { + await setDistricts(uid2, "First Essex", "3rd Essex") + const uid3 = (await signInUser3()).uid + const uid4 = (await signInUser4()).uid + await setDistricts(uid3, "First Essex", "4th Essex") + await setDistricts(uid4) + + await cosign(signInUser2) + await cosign(signInUser3) + const last = await cosign(signInUser4) + + expect(last.publication.firstPublishedAt).toEqual( + last.publication.publishedAt + ) + await expectTally({ + cosignCount: 3, + cosignSenateDistrictCount: 1, + cosignHouseDistrictCount: 2, + lastCosignedAt: last.publication.firstPublishedAt + }) + }, + tallyTimeout + ) + + it( + "Doesn't count an edit to a note as a new cosign", + async () => { + const first = await cosign(signInUser2) + await expectTally({ cosignCount: 1 }) + + await updateDoc(refs.draftTestimony(uid2, first.draftId), { + content: "Because it is right", + editReason: "Say why" + }) + await publishTestimony({ draftId: first.draftId }) + + const second = await cosign(signInUser3) + await expectTally({ + cosignCount: 2, + lastCosignedAt: second.publication.firstPublishedAt + }) + }, + tallyTimeout + ) + + it( + "Drops a cosign deleted by an admin", + async () => { + const first = await cosign(signInUser2) + const second = await cosign(signInUser3) + await expectTally({ + cosignCount: 2, + lastCosignedAt: second.publication.firstPublishedAt + }) + + await signInTestAdmin() + await deleteTestimony({ + uid: second.uid, + publicationId: second.publicationId + }) + await expectTally({ + cosignCount: 1, + lastCosignedAt: first.publication.firstPublishedAt + }) + }, + tallyTimeout + ) + + it( + "Keeps the tally when the parent's author edits", + async () => { + await cosign(signInUser2) + await expectTally({ cosignCount: 1 }) + + await signInUser1() + await updateDoc(refs.draftTestimony(uid, parentDraftId), { + content: "updated content", + editReason: "edit reason" + }) + await publishTestimony({ draftId: parentDraftId }) + + const parent = await getParent() + expect(parent.version).toBe(2) + expect(parent.cosignCount).toBe(1) + }, + tallyTimeout + ) + + it("Records the author's districts on any testimony", async () => { + await setDistricts(uid2, "First Essex", "3rd Essex") + const { draftId } = await createConferenceDraft( + uid2, + billId, + conferenceId, + "passNothing" + ) + const { publicationId } = (await publishTestimony({ draftId })).data + expect(await getPublication(uid2, publicationId)).toMatchObject({ + senatorDistrict: "First Essex", + representativeDistrict: "3rd Essex" + }) + }) + }) }) }) }) From bf02886d435b3752eeed00eed859ab9ed3567c5f Mon Sep 17 00:00:00 2001 From: violet Date: Tue, 27 Oct 2026 13:19:50 -0400 Subject: [PATCH 5/7] Delete a testimony's cosigns along with it Deleting testimony now deletes its cosigns, each through the same performDeleteTestimony as any other delete, so each cosign comes off the conference's totals and its cosigner's draft loses its published version. The sweep runs after the parent is gone, when publish already refuses new cosigns, and runs even if the parent was already deleted, so deleting it again finishes a cascade that stopped partway. Moderation removal now deletes inside resolveReport through the same function. It used to resolve the report and then call a Next API route that deleted the publication without touching the bill or conference counts. The route came from codeforboston/maple#1046, written while the deleteTestimony callable was disabled by policy and could only delete the caller's own testimony. The admin version of the callable merged the same day, but the moderation page kept calling the route, so moderator removals have left counts high since. The route, its client helper and the extra archive copy it wrote are gone, since publish already archives every version. The cosign tally trigger reads the parent first and stops when it is gone, rather than reading every remaining cosign on each delete of a cascade. Closes hyphacoop/maple#95 Verified: yarn test:integration-ci on the seven CI suites with Typesense (97 passed), including a parent with two cosigns deleted by an admin and removed by a moderator (no cosigns left, drafts cleared, conference totals back to zero), one cosign deleted leaving the parent and the other, and the existing moderation case now checking the publication is gone; testimony and moderation suites again after the tally change (72 passed); functions unit tests from src (132 passed); functions type check; root type check (errors only in untracked local scratch scripts); prettier check; eslint on the changed files. Not verified: the cascade or the moderation removal against the dev backend, or a parent with thousands of cosigns, which are deleted one at a time within the callable's timeout. Co-Authored-By: Claude Opus 5.5 --- components/api/delete-testimony.ts | 14 ---- components/moderation/RemoveTestimony.tsx | 16 +--- functions/src/testimony/cosignTally.ts | 19 ++--- functions/src/testimony/deleteTestimony.ts | 17 ++++ functions/src/testimony/resolveReport.ts | 19 +---- pages/api/users/[uid]/testimony/[tid].ts | 67 --------------- tests/integration/moderation.test.ts | 1 + tests/integration/testimony.test.ts | 98 ++++++++++++++++++---- 8 files changed, 114 insertions(+), 137 deletions(-) delete mode 100644 components/api/delete-testimony.ts delete mode 100644 pages/api/users/[uid]/testimony/[tid].ts diff --git a/components/api/delete-testimony.ts b/components/api/delete-testimony.ts deleted file mode 100644 index cec26956a..000000000 --- a/components/api/delete-testimony.ts +++ /dev/null @@ -1,14 +0,0 @@ -import { mapleClient } from "./maple-client" - -/** - * Admin-only route for deleting testimony. - * - * Moves testimony from published to archived. - * - * @param uid user id, author of the testimony - * @param tid testimony id - * @returns 204 response (no body) if successful, or 4XX if not. - */ -export async function deleteTestimony(uid: string, tid: string) { - return mapleClient.delete(`/api/users/${uid}/testimony/${tid}`) -} diff --git a/components/moderation/RemoveTestimony.tsx b/components/moderation/RemoveTestimony.tsx index 70bc93bc6..b8c810507 100644 --- a/components/moderation/RemoveTestimony.tsx +++ b/components/moderation/RemoveTestimony.tsx @@ -1,5 +1,4 @@ import { Card, CardContent, CardHeader, Stack } from "@mui/material" -import { deleteTestimony } from "components/api/delete-testimony" import { resolveReport } from "components/db" import { getAuth } from "firebase/auth" import { doc, getDoc } from "firebase/firestore" @@ -20,8 +19,6 @@ export const onSubmitReport = async ( reportId: string, resolution: Resolution, reason: string, - authorUid: string, - testimonyId: string, refresh: () => void ) => { const r = await resolveReport({ @@ -34,10 +31,6 @@ export const onSubmitReport = async ( alert(r.data.status) } - if (resolution === "remove-testimony") { - // If removing testimony, call deleteTestimony to move testimony from 'published' to 'archived' - const res = await deleteTestimony(authorUid, testimonyId) - } refresh() } @@ -57,14 +50,7 @@ export function RemoveTestimonyForm({ report }: { report: Report }) { const onSubmit: FormEventHandler = async e => { e.preventDefault() if (resolution && reason && report.id) { - await onSubmitReport( - report.id, - resolution, - reason, - report.authorUid, - report.testimonyId, - refresh - ) + await onSubmitReport(report.id, resolution, reason, refresh) redirect("list", "reports") } else { console.log("one of these not defined", resolution, reason, report.id) diff --git a/functions/src/testimony/cosignTally.ts b/functions/src/testimony/cosignTally.ts index 3fa6799f1..3e60f2f2d 100644 --- a/functions/src/testimony/cosignTally.ts +++ b/functions/src/testimony/cosignTally.ts @@ -72,17 +72,16 @@ export const updateCosignTally = onDocumentWritten( const parentRef = db.doc( `/users/${cosignOfAuthorUid}/publishedTestimony/${cosignOfId}` ) - const [parentSnap, cosigns] = await Promise.all([ - t.get(parentRef), - t.get( - db - .collectionGroup("publishedTestimony") - .where("cosignOfId", "==", cosignOfId) - .select(...cosignFields) - ) - ]) - // A deleted parent takes its cosigns with it. + // A deleted parent takes its cosigns with it, so there is nothing to + // count, and reading them would cost every cosign the cascade deletes. + const parentSnap = await t.get(parentRef) if (!parentSnap.exists) return + const cosigns = await t.get( + db + .collectionGroup("publishedTestimony") + .where("cosignOfId", "==", cosignOfId) + .select(...cosignFields) + ) const tally = cosignTally(cosigns.docs.map(d => d.data() as Cosign)) if (isEqual(pick(parentSnap.data(), cosignTallyFields), tally)) return diff --git a/functions/src/testimony/deleteTestimony.ts b/functions/src/testimony/deleteTestimony.ts index 58c3e5791..81d7f6894 100644 --- a/functions/src/testimony/deleteTestimony.ts +++ b/functions/src/testimony/deleteTestimony.ts @@ -54,9 +54,26 @@ export const performDeleteTestimony = async ( const attachments = new Attachments() await attachments.applyDelete(output.attachmentId) + await deleteCosignsOf(publicationId) + return { deleted: output.deleted } } +/** Deletes a publication's cosigns, each the way any testimony is deleted. + * Publish refuses a parent that is gone, so once the parent is deleted no new + * cosign can appear. This runs even when the parent was already gone, so + * deleting it again finishes a cascade that stopped partway. */ +const deleteCosignsOf = async (publicationId: string) => { + const cosigns = await db + .collectionGroup("publishedTestimony") + .where("cosignOfId", "==", publicationId) + .select() + .get() + for (const cosign of cosigns.docs) { + await performDeleteTestimony(cosign.ref.parent.parent!.id, cosign.id) + } +} + type TransactionOutput = { deleted: boolean; attachmentId?: Maybe } class DeleteTestimonyTransaction { private t diff --git a/functions/src/testimony/resolveReport.ts b/functions/src/testimony/resolveReport.ts index 140eb7755..7034ed6f2 100644 --- a/functions/src/testimony/resolveReport.ts +++ b/functions/src/testimony/resolveReport.ts @@ -3,7 +3,7 @@ import type { CallableRequest } from "firebase-functions/v2/https" import { db } from "../firebase" import { z } from "zod" import { fail, checkRequestZod, checkAuth, checkAdmin } from "../common" -// import { performDeleteTestimony } from "./deleteTestimony" +import { performDeleteTestimony } from "./deleteTestimony" import { first } from "lodash" import { Testimony } from "./types" @@ -53,20 +53,9 @@ export const resolveReportV2 = onCall( .get() .then(d => d.data()?.fullName) - // ***archived testiomny Id === published testimony Id*** - - // 4 Get the archived testimony document - // const archivedTestimonyId = await db - // .collection(`/users/${testimony.authorUid}/archivedTestimony`) - // .where("billId", "==", testimony.billId) - // .where("court", "==", testimony.court) - // .where("version", "==", testimony.version) - // .limit(1) - // .get() - // .then(snap => { - // if (snap.empty) return testimony.id // throw fail("not-found", "Archived testimony not found") - // return snap.docs[0].id - // }) + // 4. Remove the testimony the way any testimony is deleted + if (resolution === "remove-testimony") + await performDeleteTestimony(testimony.authorUid, testimonyId) // 5. Update the report const resolutionObj: any = { diff --git a/pages/api/users/[uid]/testimony/[tid].ts b/pages/api/users/[uid]/testimony/[tid].ts deleted file mode 100644 index 590e07a7b..000000000 --- a/pages/api/users/[uid]/testimony/[tid].ts +++ /dev/null @@ -1,67 +0,0 @@ -import { NextApiRequest, NextApiResponse } from "next" -import { z } from "zod" -import { db } from "../../../../../components/server-api/init-firebase-admin" -import { ensureAdminAuthenticated } from "../../../../../components/server-api/middleware-fns" - -export default async function handler( - req: NextApiRequest, - res: NextApiResponse -) { - switch (req.method) { - case "DELETE": - return await _delete(req, res) - default: - res.status(404).end() - } -} - -const QuerySchema = z.object({ - uid: z.string(), - tid: z.string() -}) - -/** - * Admin-only route for deleting testimony. - * - * Moves testimony from published to archived in firestore. - * - * 204 no content if successful. - */ -async function _delete(req: NextApiRequest, res: NextApiResponse) { - const { query } = req - const token = await ensureAdminAuthenticated(req, res) - if (!token) { - return - } - const queryValidation = QuerySchema.safeParse(query) - if (!queryValidation.success) { - return res.status(400).json({ - error: queryValidation.error - }) - } - const { uid, tid } = queryValidation.data - console.log(uid, tid) - - const testimonyRef = db.doc(`users/${uid}/publishedTestimony/${tid}`) - const testimonySnapshot = await testimonyRef.get() - if (!testimonySnapshot.exists) { - return res.status(404).json({ - error: "Testimony doesn't exist." - }) - } - const testimony = testimonySnapshot.data() - - const moveToArchivedBatch = db.batch() - - // remove from published - moveToArchivedBatch.delete(testimonyRef) - - // add to archived - const archivedRef = db.doc(`users/${uid}/archivedTestimony/${tid}`) - moveToArchivedBatch.set(archivedRef, { ...testimony }) - - await moveToArchivedBatch.commit() - - // ChatGPT recommmended this as the best return code for a DELETE - res.status(204).end() -} diff --git a/tests/integration/moderation.test.ts b/tests/integration/moderation.test.ts index cb93fbd92..e3f0d7dd2 100644 --- a/tests/integration/moderation.test.ts +++ b/tests/integration/moderation.test.ts @@ -92,6 +92,7 @@ describe("moderate testimony", () => { report = (await reportRef.get()).data() as Report expect(report.resolution?.resolution).toEqual("remove-testimony") + expect((await pubRef.get()).exists).toBeFalsy() await reportRef.delete() const testDraftRef = testDb.doc( diff --git a/tests/integration/testimony.test.ts b/tests/integration/testimony.test.ts index a5c0c31de..8ae6d2aa2 100644 --- a/tests/integration/testimony.test.ts +++ b/tests/integration/testimony.test.ts @@ -9,8 +9,10 @@ import { nanoid } from "nanoid" import { auth, firestore, functions, storage } from "../../components/firebase" import { terminateFirebase, testDb, testStorage } from "../testUtils" import { script as backfillTestimonyScope } from "../../scripts/firebase-admin/backfillTestimonyScope" +import { resolveReport } from "components/db" import { createFakeBill, + createNewReport, expectPermissionDenied, expectStorageUnauthorized, getBill, @@ -785,6 +787,22 @@ describe("publishTestimony", () => { }) const cosignOf = () => ({ cosignOfId: parentId, cosignOfAuthorUid: uid }) + const cosign = async (signIn: () => Promise) => { + const cosigner = (await signIn()).uid + const { draftId } = await createCosignDraft( + cosigner, + billId, + conferenceId, + cosignOf() + ) + const { publicationId } = (await publishTestimony({ draftId })).data + return { + uid: cosigner, + draftId, + publicationId, + publication: await getPublication(cosigner, publicationId) + } + } it("Files a cosign with no note, at its parent's position", async () => { const { draftId: cosignDraftId } = await createCosignDraft( @@ -1067,22 +1085,6 @@ describe("publishTestimony", () => { interval: 100, timeout: 30_000 }) - const cosign = async (signIn: () => Promise) => { - const cosigner = (await signIn()).uid - const { draftId } = await createCosignDraft( - cosigner, - billId, - conferenceId, - cosignOf() - ) - const { publicationId } = (await publishTestimony({ draftId })).data - return { - uid: cosigner, - draftId, - publicationId, - publication: await getPublication(cosigner, publicationId) - } - } it( "Counts cosigns and their distinct districts on the parent", @@ -1189,6 +1191,70 @@ describe("publishTestimony", () => { }) }) }) + + describe("deleting the parent", () => { + // User 2 and user 3 cosign the parent. + let cosigns: Awaited>[] + beforeEach(async () => { + cosigns = [await cosign(signInUser2), await cosign(signInUser3)] + expect(await getConference(conferenceId)).toMatchObject({ + testimonyCount: 3, + passSenateVersionCount: 3 + }) + await signInTestAdmin() + }) + + const expectCosignsGone = async () => { + for (const c of cosigns) { + expect(await getPublication(c.uid, c.publicationId)).toBeUndefined() + const draft = await testDb + .doc(paths.draftTestimony(c.uid, c.draftId)) + .get() + expect(draft.data()!.publishedVersion).toBeUndefined() + } + expect(await getConference(conferenceId)).toMatchObject({ + testimonyCount: 0, + passSenateVersionCount: 0 + }) + } + + it("Deletes its cosigns when an admin deletes it", async () => { + await deleteTestimony({ uid, publicationId: parentId }) + + expect(await getPublication(uid, parentId)).toBeUndefined() + await expectCosignsGone() + }) + + it("Deletes its cosigns when a moderator removes it", async () => { + const { reportId } = await createNewReport(uid, parentId) + const result = await resolveReport({ + reportId, + resolution: "remove-testimony", + reason: "important reason" + }) + + expect(result.data.status).toBe("success") + expect(await getPublication(uid, parentId)).toBeUndefined() + await expectCosignsGone() + }) + + it("Leaves the parent and other cosigns when one cosign is deleted", async () => { + const [first, second] = cosigns + await deleteTestimony({ + uid: first.uid, + publicationId: first.publicationId + }) + + expect(await getPublication(uid, parentId)).toBeDefined() + expect( + await getPublication(second.uid, second.publicationId) + ).toBeDefined() + expect(await getConference(conferenceId)).toMatchObject({ + testimonyCount: 2, + passSenateVersionCount: 2 + }) + }) + }) }) }) }) From ed913bd029653a0a9aeb1b8135e2cd115d1808d9 Mon Sep 17 00:00:00 2001 From: violet Date: Tue, 27 Oct 2026 14:28:06 -0400 Subject: [PATCH 6/7] Drop the moderation report's empty archive id The report list and detail showed an archive id read from resolution.archiveTestimonyId, a field nothing writes; resolveReport wrote archivedTestimonyId instead. Since removal no longer writes a separate archive copy, that value was only the testimony id the report already carries, so the column, its label and the field are gone. RemoveTestimony also loses imports and a type it never used, among them firebase-admin, a server package, in a browser component. Verified: moderation integration suite (5 passed); root type check (errors only in untracked local scratch scripts); functions type check; eslint on the changed files; prettier check. Not verified: the moderation report pages in a browser. Co-Authored-By: Claude Opus 5.5 --- components/moderation/EditReports.tsx | 2 -- components/moderation/ListReports.tsx | 4 ---- components/moderation/RemoveTestimony.tsx | 11 +---------- components/moderation/types.ts | 1 - functions/src/testimony/resolveReport.ts | 3 +-- public/locales/en/moderation.json | 1 - 6 files changed, 2 insertions(+), 20 deletions(-) diff --git a/components/moderation/EditReports.tsx b/components/moderation/EditReports.tsx index 1f5e5a539..460c0dfd9 100644 --- a/components/moderation/EditReports.tsx +++ b/components/moderation/EditReports.tsx @@ -81,8 +81,6 @@ export const ReportContent = ({ - - {report.reason}
diff --git a/components/moderation/ListReports.tsx b/components/moderation/ListReports.tsx index ebafa5d54..1f40b1646 100644 --- a/components/moderation/ListReports.tsx +++ b/components/moderation/ListReports.tsx @@ -34,10 +34,6 @@ export function ListReports() { - diff --git a/components/moderation/RemoveTestimony.tsx b/components/moderation/RemoveTestimony.tsx index b8c810507..b101bccee 100644 --- a/components/moderation/RemoveTestimony.tsx +++ b/components/moderation/RemoveTestimony.tsx @@ -1,19 +1,10 @@ import { Card, CardContent, CardHeader, Stack } from "@mui/material" import { resolveReport } from "components/db" import { getAuth } from "firebase/auth" -import { doc, getDoc } from "firebase/firestore" import { Timestamp } from "functions/src/firebase" import { FormEventHandler, useState } from "react" -import { useRedirect, useNotify, useRefresh } from "react-admin" +import { useRedirect, useRefresh } from "react-admin" import { Report, Resolution } from "." -import { firestore } from "components/firebase" -import { refreshToken } from "firebase-admin/app" - -export type ReportResponseValues = { - reportId: string - resolution: Resolution - reason: string -} export const onSubmitReport = async ( reportId: string, diff --git a/components/moderation/types.ts b/components/moderation/types.ts index d7a3efa4d..98f34c64c 100644 --- a/components/moderation/types.ts +++ b/components/moderation/types.ts @@ -25,7 +25,6 @@ export type ReportResolution = { moderatorUid: string resolvedAt: Timestamp authorUid?: string - archivedTestimonyId: string } export const modifyAccount = httpsCallable<{ uid: string; role: Role }, void>( diff --git a/functions/src/testimony/resolveReport.ts b/functions/src/testimony/resolveReport.ts index 7034ed6f2..5d9eb8c87 100644 --- a/functions/src/testimony/resolveReport.ts +++ b/functions/src/testimony/resolveReport.ts @@ -62,8 +62,7 @@ export const resolveReportV2 = onCall( resolution, moderatorUid, resolvedAt: new Date(), - authorUid: testimony.authorUid, - archivedTestimonyId: testimonyId + authorUid: testimony.authorUid } if (reason) resolutionObj.reason = reason if (moderatorName) resolutionObj.moderatorName = moderatorName diff --git a/public/locales/en/moderation.json b/public/locales/en/moderation.json index 0fc0c7f8a..3dcc131b2 100644 --- a/public/locales/en/moderation.json +++ b/public/locales/en/moderation.json @@ -1,7 +1,6 @@ { "resolved": "resolved", "reportId": "report id", - "archivedId": "archived id", "status": "status", "resolution": "resolution", "moderatedBy": "moderated by", From a9a87d8703e4a69f507c462f5e954cdb7eb4ef5e Mon Sep 17 00:00:00 2001 From: violet Date: Tue, 27 Oct 2026 14:54:45 -0400 Subject: [PATCH 7/7] Skip incomplete or unchanged testimony in the notification trigger The trigger that records a notification event for each new publication crashed when a publication lacked the court, bill title or author: court.toString() threw, and add() rejects undefined fields. Each crash restarts the emulator's function worker. It now skips a publication whose event would have an undefined field, logging which ones. On every other write it queried notificationEvents before checking the version, so each cosign tally update and backfilled document paid a query. It now returns before the query when the version is unchanged. The handler is split out from the trigger, as the ballot question trigger does, so it can be unit tested. Closes hyphacoop/maple#114 Verified: unit tests cover a complete creation, creations missing billTitle, court or authorUid, a tally-only update (no database call) and a version change. The testimony and firstPublishedAt backfill integration suites pass (71) with no crash from this trigger; 193 writes returned at the version check and one incomplete fixture was skipped. Not verified: on the dev project. Co-Authored-By: Claude Opus 5.5 --- ...timonySubmissionNotificationEvents.test.ts | 117 ++++++++++++ ...teTestimonySubmissionNotificationEvents.ts | 174 ++++++++++-------- 2 files changed, 211 insertions(+), 80 deletions(-) create mode 100644 functions/src/notifications/populateTestimonySubmissionNotificationEvents.test.ts diff --git a/functions/src/notifications/populateTestimonySubmissionNotificationEvents.test.ts b/functions/src/notifications/populateTestimonySubmissionNotificationEvents.test.ts new file mode 100644 index 000000000..4dc4d36ba --- /dev/null +++ b/functions/src/notifications/populateTestimonySubmissionNotificationEvents.test.ts @@ -0,0 +1,117 @@ +import { populateTestimonySubmissionNotificationEventsHandler } from "./populateTestimonySubmissionNotificationEvents" + +jest.mock("firebase-functions/v2/firestore", () => ({ + onDocumentWritten: jest.fn() +})) + +jest.mock("../firebase", () => ({ + db: { + collection: jest.fn() + }, + Timestamp: { + now: jest.fn(() => ({ seconds: 1000 })) + } +})) + +const mockDb = require("../firebase").db + +const mockAdd = jest.fn().mockResolvedValue({}) +const mockUpdate = jest.fn().mockResolvedValue({}) +const mockDoc = jest.fn().mockReturnValue({ update: mockUpdate }) + +const setupCollection = (empty: boolean, docs: { id: string }[] = []) => { + const query = { + where: jest.fn(), + get: jest.fn().mockResolvedValue({ empty, docs }), + add: mockAdd, + doc: mockDoc + } + query.where.mockReturnValue(query) + mockDb.collection.mockReturnValue(query) +} + +const makeTestimony = (overrides: object = {}) => ({ + court: 194, + billId: "H1234", + billTitle: "An Act relative to testing", + authorUid: "author-uid", + authorRole: "user", + fullName: "Test Author", + position: "endorse", + content: "Testimony content", + version: 1, + ...overrides +}) + +const makeSnapshot = (before: object | null, after: object | null) => ({ + before: { exists: before !== null, data: () => before }, + after: { exists: after !== null, data: () => after } +}) + +const makeEvent = (before: object | null, after: object | null) => ({ + data: makeSnapshot(before, after), + params: { userId: "author-uid", testimonyId: "testimony-id" } +}) + +beforeEach(() => { + jest.clearAllMocks() +}) + +describe("populateTestimonySubmissionNotificationEventsHandler", () => { + it("creates a notificationEvent for new testimony", async () => { + setupCollection(true) + + const event = makeEvent(null, makeTestimony()) + await populateTestimonySubmissionNotificationEventsHandler(event as any) + + expect(mockAdd).toHaveBeenCalledWith( + expect.objectContaining({ + type: "testimony", + billCourt: "194", + billId: "H1234", + billName: "An Act relative to testing", + userId: "author-uid", + testimonyId: "testimony-id", + testimonyVersion: 1 + }) + ) + }) + + it.each(["billTitle", "court", "authorUid"])( + "skips new testimony missing %s", + async field => { + setupCollection(true) + + const event = makeEvent(null, makeTestimony({ [field]: undefined })) + await populateTestimonySubmissionNotificationEventsHandler(event as any) + + expect(mockAdd).not.toHaveBeenCalled() + } + ) + + it("does nothing when the version is unchanged", async () => { + const event = makeEvent(makeTestimony(), makeTestimony({ cosignCount: 1 })) + await populateTestimonySubmissionNotificationEventsHandler(event as any) + + expect(mockDb.collection).not.toHaveBeenCalled() + }) + + it("updates an existing notificationEvent when the version changes", async () => { + const existingDocId = "existing-event-id" + setupCollection(false, [{ id: existingDocId }]) + + const event = makeEvent( + makeTestimony(), + makeTestimony({ version: 2, content: "Edited content" }) + ) + await populateTestimonySubmissionNotificationEventsHandler(event as any) + + expect(mockDoc).toHaveBeenCalledWith(existingDocId) + expect(mockUpdate).toHaveBeenCalledWith( + expect.objectContaining({ + testimonyContent: "Edited content", + testimonyVersion: 2 + }) + ) + }) +}) diff --git a/functions/src/notifications/populateTestimonySubmissionNotificationEvents.ts b/functions/src/notifications/populateTestimonySubmissionNotificationEvents.ts index 3f4cec732..aa40b1111 100644 --- a/functions/src/notifications/populateTestimonySubmissionNotificationEvents.ts +++ b/functions/src/notifications/populateTestimonySubmissionNotificationEvents.ts @@ -4,100 +4,114 @@ // Creates a notification document in the user's notification feed for each active subscription. // Import necessary Firebase modules -import { onDocumentWritten } from "firebase-functions/v2/firestore" -import { getFirestore } from "firebase-admin/firestore" -import { Timestamp } from "../firebase" +import { + Change, + DocumentSnapshot, + FirestoreEvent, + onDocumentWritten +} from "firebase-functions/v2/firestore" +import { db, Timestamp } from "../firebase" import { TestimonySubmissionNotification } from "./types" -// Get a reference to the Firestore database -const db = getFirestore() - -// Define the populateOrgNotificationEvents function -export const populateTestimonySubmissionNotificationEvents = onDocumentWritten( - "/users/{userId}/publishedTestimony/{testimonyId}", - async event => { - const snapshot = event.data - if (!snapshot || !snapshot.after.exists) { - console.error("New snapshot does not exist") - return - } - - const documentCreated = !snapshot.before.exists +export const populateTestimonySubmissionNotificationEventsHandler = async ( + event: FirestoreEvent< + Change | undefined, + { userId: string; testimonyId: string } + > +) => { + const snapshot = event.data + if (!snapshot || !snapshot.after.exists) { + console.error("New snapshot does not exist") + return + } - const oldData = snapshot.before.data() - const newData = snapshot.after.data() + const documentCreated = !snapshot.before.exists - // New testimony added - if (documentCreated) { - console.log("New document created") + const oldData = snapshot.before.data() + const newData = snapshot.after.data() - const bqId: string | null = newData?.ballotQuestionId ?? null - const bqCourt: number | null = bqId - ? (newData?.court as number) ?? null - : null + // New testimony added + if (documentCreated) { + console.log("New document created") - const newNotificationEvent: TestimonySubmissionNotification = { - type: "testimony", + const bqId: string | null = newData?.ballotQuestionId ?? null + const bqCourt: number | null = bqId + ? (newData?.court as number) ?? null + : null - billCourt: newData?.court.toString(), - billId: newData?.billId, - billName: newData?.billTitle, + const newNotificationEvent: TestimonySubmissionNotification = { + type: "testimony", - userId: newData?.authorUid, - userRole: newData?.authorRole, - testimonyId: event.params.testimonyId, - testimonyUser: newData?.fullName, - testimonyPosition: newData?.position, - testimonyContent: newData?.content, - testimonyVersion: newData?.version, + billCourt: newData?.court?.toString(), + billId: newData?.billId, + billName: newData?.billTitle, - ballotQuestionId: bqId, - ballotQuestionCourt: bqCourt, - conferenceId: newData?.conferenceId ?? null, + userId: newData?.authorUid, + userRole: newData?.authorRole, + testimonyId: event.params.testimonyId, + testimonyUser: newData?.fullName, + testimonyPosition: newData?.position, + testimonyContent: newData?.content, + testimonyVersion: newData?.version, - updateTime: Timestamp.now() - } + ballotQuestionId: bqId, + ballotQuestionCourt: bqCourt, + conferenceId: newData?.conferenceId ?? null, - await db.collection("/notificationEvents").add(newNotificationEvent) + updateTime: Timestamp.now() + } + // A publication missing what a notification needs (older documents, + // test fixtures) would make add() reject undefined fields. + const missing = Object.entries(newNotificationEvent) + .filter(([, value]) => value === undefined) + .map(([key]) => key) + if (missing.length > 0) { + console.warn( + `Testimony ${event.params.testimonyId} lacks ${missing.join( + ", " + )}, skipping notification event` + ) return } - const oldVersion = oldData?.version - const newVersion = newData?.version - - const testimonyChanged = oldVersion !== newVersion - console.log(`oldVersion: ${oldVersion}, newVersion: ${newVersion}`) - - const notificationEventSnapshot = await db - .collection("/notificationEvents") - .where("type", "==", "org") - .where("billCourt", "==", newData?.court.toString()) - .where("billId", "==", newData?.billId) - .where("authorUid", "==", newData?.authorUid) - .get() - - console.log( - `${notificationEventSnapshot.docs} ${notificationEventSnapshot.docs.length}` - ) - - if (!notificationEventSnapshot.empty) { - const notificationEventId = notificationEventSnapshot.docs[0].id - - if (testimonyChanged) { - console.log("Testimony changed") - - // Update the existing notification event - await db - .collection("/notificationEvents") - .doc(notificationEventId) - .update({ - testimonyPosition: newData?.position, - testimonyContent: newData?.content, - testimonyVersion: newData?.version, - updateTime: Timestamp.now() - }) - } - } + await db.collection("/notificationEvents").add(newNotificationEvent) + + return + } + + const oldVersion = oldData?.version + const newVersion = newData?.version + + console.log(`oldVersion: ${oldVersion}, newVersion: ${newVersion}`) + if (oldVersion === newVersion) return + + const notificationEventSnapshot = await db + .collection("/notificationEvents") + .where("type", "==", "org") + .where("billCourt", "==", newData?.court.toString()) + .where("billId", "==", newData?.billId) + .where("authorUid", "==", newData?.authorUid) + .get() + + console.log( + `${notificationEventSnapshot.docs} ${notificationEventSnapshot.docs.length}` + ) + + if (!notificationEventSnapshot.empty) { + const notificationEventId = notificationEventSnapshot.docs[0].id + + // Update the existing notification event + await db.collection("/notificationEvents").doc(notificationEventId).update({ + testimonyPosition: newData?.position, + testimonyContent: newData?.content, + testimonyVersion: newData?.version, + updateTime: Timestamp.now() + }) } +} + +export const populateTestimonySubmissionNotificationEvents = onDocumentWritten( + "/users/{userId}/publishedTestimony/{testimonyId}", + populateTestimonySubmissionNotificationEventsHandler )