fix: preserve ntfy action buttons on reminder replay

* fix: reactivate notification action groups for reminder replay * fix: annotate notification action token test rows
2026-05-13 21:25:37 +02:00
parent 827d1adc35
commit a95c6e3657
5 changed files with 222 additions and 24 deletions
@@ -213,10 +213,10 @@ async function replaceNtfyNotificationSequence(options: {
 	originalMessage: string;
 	action: NotificationMutationAction;
 	viewUrl: string | null;
-}): Promise<boolean> {
+}): Promise<{ replaced: boolean; providerMessageId?: string }> {
 	const normalizedSequenceId = options.sequenceId.trim();
 	if (normalizedSequenceId.length === 0) {
-		return false;
+		return { replaced: false };
 	}

 	const [settings] = await db
@@ -225,12 +225,12 @@ async function replaceNtfyNotificationSequence(options: {
 		.where(eq(userSettings.userId, options.userId));

 	if (!settings?.shoutrrrEnabled || !settings.shoutrrrUrl) {
-		return false;
+		return { replaced: false };
 	}

 	const sanitized = sanitizeNotificationUrl(settings.shoutrrrUrl);
 	if ("error" in sanitized || !sanitized.isNtfy) {
-		return false;
+		return { replaced: false };
 	}

 	const labels = getNotificationActionLabels(options.language);
@@ -247,7 +247,7 @@ async function replaceNtfyNotificationSequence(options: {
 		throw new Error(result.error ?? "Failed to replace ntfy notification");
 	}

-	return true;
+	return { replaced: true, providerMessageId: result.providerMessageId };
 }

 function renderPage(options: {
@@ -585,9 +585,10 @@ export async function notificationActionRoutes(app: FastifyInstance) {

 			const recordedText = getActionRecordedText(language, action);
 			let replacedNtfyNotification = false;
+			const previousNtfyMessageId = record.group.ntfyOriginalMessageId.trim();

 			try {
-				replacedNtfyNotification = await replaceNtfyNotificationSequence({
+				const replacementResult = await replaceNtfyNotificationSequence({
 					userId: record.group.userId,
 					sequenceId: record.group.sequenceId,
 					language,
@@ -596,6 +597,33 @@ export async function notificationActionRoutes(app: FastifyInstance) {
 					action,
 					viewUrl: record.viewUrl,
 				});
+				replacedNtfyNotification = replacementResult.replaced;
+
+				if (replacementResult.providerMessageId) {
+					await db
+						.update(notificationActionGroups)
+						.set({ ntfyOriginalMessageId: replacementResult.providerMessageId, updatedAt: new Date() })
+						.where(eq(notificationActionGroups.id, record.group.id));
+				}
+
+				if (
+					replacementResult.replaced &&
+					previousNtfyMessageId.length > 0 &&
+					previousNtfyMessageId !== replacementResult.providerMessageId
+				) {
+					try {
+						await deleteNtfyNotificationSequence(record.group.userId, previousNtfyMessageId);
+					} catch (error) {
+						request.log.warn(
+							buildNotificationActionLogContext(record, {
+								requestedAction: action,
+								originalMessageId: previousNtfyMessageId,
+								error,
+							}),
+							"[NotificationActions] Failed to delete original ntfy notification after replacement"
+						);
+					}
+				}
 			} catch (error) {
 				request.log.warn(
 					buildNotificationActionLogContext(record, { requestedAction: action, error }),
@@ -149,6 +149,10 @@ async function createActionTokens(groupId: number): Promise<Record<ActiveTokenKi
 	);
 }

+async function resetActionTokens(groupId: number): Promise<void> {
+	await db.delete(notificationActionTokens).where(eq(notificationActionTokens.groupId, groupId));
+}
+
 export async function createNotificationActionContext(input: {
 	userId: number;
 	title: string;
@@ -198,21 +202,47 @@ export async function createNotificationActionContext(input: {
 		);

 	if (!group) {
-		[group] = await db
-			.insert(notificationActionGroups)
-			.values({
-				userId: input.userId,
-				groupKey,
-				sequenceId,
-				doseIdsJson: JSON.stringify(uniqueDoseIds),
-				title: input.title,
-				message: input.message,
-				language: input.language,
-				scheduledFor: input.scheduledFor,
-				expiresAt,
-				updatedAt: now,
-			})
-			.returning();
+		const [existingGroup] = await db
+			.select()
+			.from(notificationActionGroups)
+			.where(eq(notificationActionGroups.groupKey, groupKey));
+
+		if (existingGroup) {
+			await resetActionTokens(existingGroup.id);
+			[group] = await db
+				.update(notificationActionGroups)
+				.set({
+					sequenceId,
+					ntfyOriginalMessageId: "",
+					doseIdsJson: JSON.stringify(uniqueDoseIds),
+					title: input.title,
+					message: input.message,
+					language: input.language,
+					scheduledFor: input.scheduledFor,
+					expiresAt,
+					resolvedAction: null,
+					resolvedAt: null,
+					updatedAt: now,
+				})
+				.where(eq(notificationActionGroups.id, existingGroup.id))
+				.returning();
+		} else {
+			[group] = await db
+				.insert(notificationActionGroups)
+				.values({
+					userId: input.userId,
+					groupKey,
+					sequenceId,
+					doseIdsJson: JSON.stringify(uniqueDoseIds),
+					title: input.title,
+					message: input.message,
+					language: input.language,
+					scheduledFor: input.scheduledFor,
+					expiresAt,
+					updatedAt: now,
+				})
+				.returning();
+		}
 	}

 	const tokens = await createActionTokens(group.id);
@@ -350,7 +350,12 @@ describe("notification action routes", () => {
 			shoutrrrUrl: "ntfy://user:pass@ntfy.example.com/medassist",
 		});
 		const { takenToken, context } = await seedContext({ userId, doseId: "5-0-1736064000000" });
+		await testClient.execute({
+			sql: "UPDATE notification_action_groups SET ntfy_original_message_id = ? WHERE user_id = ?",
+			args: ["ntfy-msg-1", userId],
+		});
 		fetchMock.mockResolvedValueOnce({ ok: true, json: () => Promise.resolve({ id: "ntfy-msg-2" }) });
+		fetchMock.mockResolvedValueOnce({ ok: true, text: () => Promise.resolve("") });

 		const response = await app.inject({
 			method: "POST",
@@ -358,7 +363,7 @@ describe("notification action routes", () => {
 		});

 		expect(response.statusCode).toBe(200);
-		expect(fetchMock).toHaveBeenCalledTimes(1);
+		expect(fetchMock).toHaveBeenCalledTimes(2);

 		const [targetUrl, requestInit] = fetchMock.mock.calls[0] ?? [];
 		expect(targetUrl).toBe("https://ntfy.example.com/medassist");
@@ -383,6 +388,21 @@ describe("notification action routes", () => {
 				clear: false,
 			},
 		]);
+
+		const [deleteUrl, deleteInit] = fetchMock.mock.calls[1] ?? [];
+		expect(deleteUrl).toBe("https://ntfy.example.com/medassist/ntfy-msg-1");
+		expect(deleteInit).toEqual(
+			expect.objectContaining({
+				method: "DELETE",
+				headers: expect.objectContaining({ Authorization: expect.stringMatching(/^Basic /) }),
+			})
+		);
+
+		const groupRow = await testClient.execute({
+			sql: "SELECT ntfy_original_message_id FROM notification_action_groups WHERE user_id = ?",
+			args: [userId],
+		});
+		expect(groupRow.rows).toEqual([expect.objectContaining({ ntfy_original_message_id: "ntfy-msg-2" })]);
 	});

 	it("replaces the original ntfy notification after a skip action with a view-only confirmation", async () => {
@@ -393,7 +413,12 @@ describe("notification action routes", () => {
 			shoutrrrUrl: "ntfy://user:pass@ntfy.example.com/medassist",
 		});
 		const { skipToken, context } = await seedContext({ userId, doseId: "5-0-1736064000000" });
+		await testClient.execute({
+			sql: "UPDATE notification_action_groups SET ntfy_original_message_id = ? WHERE user_id = ?",
+			args: ["ntfy-msg-7", userId],
+		});
 		fetchMock.mockResolvedValueOnce({ ok: true, json: () => Promise.resolve({ id: "ntfy-msg-3" }) });
+		fetchMock.mockResolvedValueOnce({ ok: true, text: () => Promise.resolve("") });

 		const response = await app.inject({
 			method: "POST",
@@ -401,7 +426,7 @@ describe("notification action routes", () => {
 		});

 		expect(response.statusCode).toBe(200);
-		expect(fetchMock).toHaveBeenCalledTimes(1);
+		expect(fetchMock).toHaveBeenCalledTimes(2);

 		const [targetUrl, requestInit] = fetchMock.mock.calls[0] ?? [];
 		expect(targetUrl).toBe("https://ntfy.example.com/medassist");
@@ -416,6 +441,21 @@ describe("notification action routes", () => {
 				}),
 			})
 		);
+
+		const [deleteUrl, deleteInit] = fetchMock.mock.calls[1] ?? [];
+		expect(deleteUrl).toBe("https://ntfy.example.com/medassist/ntfy-msg-7");
+		expect(deleteInit).toEqual(
+			expect.objectContaining({
+				method: "DELETE",
+				headers: expect.objectContaining({ Authorization: expect.stringMatching(/^Basic /) }),
+			})
+		);
+
+		const groupRow = await testClient.execute({
+			sql: "SELECT ntfy_original_message_id FROM notification_action_groups WHERE user_id = ?",
+			args: [userId],
+		});
+		expect(groupRow.rows).toEqual([expect.objectContaining({ ntfy_original_message_id: "ntfy-msg-3" })]);
 	});

 	it("warns when ntfy replacement, delete, and fallback clear all fail", async () => {
@@ -39,6 +39,11 @@ function extractToken(url: string): string {
 	return url.split("/").at(-1) ?? "";
 }

+type ActionTokenRow = {
+	kind: string | null;
+	token_hash: string | null;
+};
+
 async function clearTables() {
 	await testClient.execute("DELETE FROM notification_action_tokens");
 	await testClient.execute("DELETE FROM notification_action_groups");
@@ -181,6 +186,97 @@ describe("notification-actions-service", () => {
 		expect(Number(tokens.rows[0].count)).toBe(6);
 	});

+	it("reactivates a resolved group with the same key instead of inserting a duplicate", async () => {
+		const userId = await createUser("notify-actions-reactivate");
+		const scheduledFor = new Date("2026-01-05T08:00:00.000Z");
+		const doseIds = ["9-1-1736064000000", "9-0-1736064000000"];
+
+		const first = await createNotificationActionContext({
+			userId,
+			title: "Reminder",
+			message: "Take your medication now",
+			doseIds,
+			scheduledFor,
+			publicAppUrl: mockedEnv.PUBLIC_APP_URL,
+			language: "en",
+		});
+
+		expect(first).toMatchObject({
+			groupId: expect.any(Number),
+			respondUrl: expect.stringContaining("/api/notification-actions/"),
+			sequenceId: expect.stringMatching(/^medassist-/),
+			viewUrl: "https://app.example.com/dashboard?day=2026-01-05&dose=9-0-1736064000000",
+		});
+
+		const firstGroupId = first!.groupId!;
+		const firstSequenceId = first!.sequenceId!;
+		const firstRespondToken = extractToken(first!.respondUrl!);
+		const firstTokenRows = await testClient.execute({
+			sql: "SELECT kind, token_hash FROM notification_action_tokens WHERE group_id = ? ORDER BY kind ASC",
+			args: [firstGroupId],
+		});
+		expect(firstTokenRows.rows).toHaveLength(3);
+
+		await testClient.execute({
+			sql: "UPDATE notification_action_groups SET resolved_action = 'taken', resolved_at = ?, ntfy_original_message_id = 'old-message-id' WHERE id = ?",
+			args: [new Date(), firstGroupId],
+		});
+
+		const second = await createNotificationActionContext({
+			userId,
+			title: "Reminder",
+			message: "Take your medication now",
+			doseIds,
+			scheduledFor,
+			publicAppUrl: mockedEnv.PUBLIC_APP_URL,
+			language: "en",
+		});
+
+		expect(second).toMatchObject({
+			groupId: firstGroupId,
+			respondUrl: expect.stringContaining("/api/notification-actions/"),
+			sequenceId: expect.stringMatching(/^medassist-/),
+			viewUrl: "https://app.example.com/dashboard?day=2026-01-05&dose=9-0-1736064000000",
+		});
+		expect(second?.sequenceId).toBe(firstSequenceId);
+
+		const groups = await testClient.execute(
+			"SELECT id, sequence_id, resolved_action, resolved_at, ntfy_original_message_id FROM notification_action_groups"
+		);
+		expect(groups.rows).toHaveLength(1);
+		expect(groups.rows[0]).toEqual(
+			expect.objectContaining({
+				id: firstGroupId,
+				sequence_id: second?.sequenceId,
+				resolved_action: null,
+				resolved_at: null,
+				ntfy_original_message_id: "",
+			})
+		);
+
+		const secondTokenRows = await testClient.execute({
+			sql: "SELECT kind, token_hash FROM notification_action_tokens WHERE group_id = ? ORDER BY kind ASC",
+			args: [firstGroupId],
+		});
+		expect(secondTokenRows.rows).toHaveLength(3);
+		expect(secondTokenRows.rows.map((row: ActionTokenRow) => row.kind)).toEqual(["respond", "skip", "taken"]);
+
+		const firstTokenHashes = new Set(firstTokenRows.rows.map((row: ActionTokenRow) => String(row.token_hash)));
+		const secondTokenHashes = new Set(secondTokenRows.rows.map((row: ActionTokenRow) => String(row.token_hash)));
+		expect(secondTokenHashes.size).toBe(3);
+		expect([...secondTokenHashes].every((tokenHash) => !firstTokenHashes.has(tokenHash))).toBe(true);
+
+		expect(await getNotificationActionTokenRecord(firstRespondToken)).toBeNull();
+
+		const secondRespondToken = extractToken(second!.respondUrl!);
+		const secondRespondRecord = await getNotificationActionTokenRecord(secondRespondToken);
+		expect(secondRespondRecord).toMatchObject({
+			doseIds: ["9-0-1736064000000", "9-1-1736064000000"],
+			viewUrl: "https://app.example.com/dashboard?day=2026-01-05&dose=9-0-1736064000000",
+		});
+		expect(secondRespondRecord?.group.id).toBe(firstGroupId);
+	});
+
 	it("prefers a non-local CORS origin when PUBLIC_APP_URL points to localhost", async () => {
 		const userId = await createUser("notify-actions-mobile");
 		const scheduledFor = new Date("2026-01-05T08:00:00.000Z");