diff --git a/docs/releases.md b/docs/releases.md index 7e15ee77..8a8169b1 100644 --- a/docs/releases.md +++ b/docs/releases.md @@ -1974,6 +1974,7 @@ since I do have to reset emails on a regular basis. * Update web app dependencies, including major-version upgrades to Vite (6 -> 8, now Rolldown-based), Material UI (5 -> 9), and Dexie (3 -> 4) ([#1800](https://github.com/binwiederhier/ntfy/pull/1800), [#1764](https://github.com/binwiederhier/ntfy/pull/1764), [#1767](https://github.com/binwiederhier/ntfy/pull/1767), [#1762](https://github.com/binwiederhier/ntfy/pull/1762), [#1766](https://github.com/binwiederhier/ntfy/pull/1766), [#1765](https://github.com/binwiederhier/ntfy/pull/1765), thanks Dependabot) * Play notification sounds in the web app even when the Notification API is unavailable, e.g. over plain HTTP or in browsers without notification support ([#1772](https://github.com/binwiederhier/ntfy/pull/1772), thanks to [@mitya12342](https://github.com/mitya12342) for the contribution) * Stop escaping `<`, `>`, and `&` as `\u003c`/`\u003e`/`\u0026` in JSON responses ([#1511](https://github.com/binwiederhier/ntfy/issues/1511), [#1512](https://github.com/binwiederhier/ntfy/pull/1512), thanks to [@wunter8](https://github.com/wunter8) for the contribution) +* Fix the web app navbar not reflecting a topic reservation (lock icon, and "Reserve topic" -> "Change reservation"/"Remove reservation" menu) until a page reload, by persisting reservation and display-name changes onto already-subscribed topics during account sync ### ntfy Android v1.25.x (UNRELEASED) diff --git a/web/src/app/SubscriptionManager.js b/web/src/app/SubscriptionManager.js index f909778e..b6e28dbc 100644 --- a/web/src/app/SubscriptionManager.js +++ b/web/src/app/SubscriptionManager.js @@ -6,7 +6,7 @@ import { topicUrl } from "./utils"; import { messageWithSequenceId } from "./notificationUtils"; import { EVENT_MESSAGE, EVENT_MESSAGE_CLEAR, EVENT_MESSAGE_DELETE } from "./events"; -class SubscriptionManager { +export class SubscriptionManager { constructor(dbImpl) { this.db = dbImpl; } @@ -65,23 +65,36 @@ class SubscriptionManager { } /** + * Upsert a subscription: create it if it doesn't exist yet, or merge the given fields into the + * existing one. Merging matters for account sync, which passes the remote display name and + * reservation -- without it, reserving/unreserving a topic you're already subscribed to would + * never be reflected locally until the database is recreated (e.g. a fresh login). Local-only + * state such as mutedUntil and last is preserved on merge. + * * @param {string} baseUrl * @param {string} topic * @param {object} opts * @param {boolean} opts.internal * @returns */ - async add(baseUrl, topic, opts = {}) { + async upsert(baseUrl, topic, opts = {}) { const id = topicUrl(baseUrl, topic); const existingSubscription = await this.get(id); if (existingSubscription) { - return existingSubscription; + // Avoid a needless write (and the resulting Dexie live-query churn) when nothing changed. + const changed = Object.keys(opts).some((key) => existingSubscription[key] !== opts[key]); + if (!changed) { + return existingSubscription; + } + const updatedSubscription = { ...existingSubscription, ...opts }; + await this.db.subscriptions.put(updatedSubscription); + return updatedSubscription; } const subscription = { ...opts, - id: topicUrl(baseUrl, topic), + id, baseUrl, topic, mutedUntil: 0, @@ -101,7 +114,9 @@ class SubscriptionManager { remoteSubscriptions.map(async (remote) => { const reservation = remoteReservations?.find((r) => remote.base_url === config.base_url && remote.topic === r.topic) || null; - const local = await this.add(remote.base_url, remote.topic, { + // upsert(): for topics that already exist locally this merges in the latest remote + // display name and reservation (see upsert() for why this matters). + const local = await this.upsert(remote.base_url, remote.topic, { displayName: remote.display_name, // May be undefined reservation, // May be null! }); diff --git a/web/src/app/SubscriptionManager.test.js b/web/src/app/SubscriptionManager.test.js new file mode 100644 index 00000000..935b81e8 --- /dev/null +++ b/web/src/app/SubscriptionManager.test.js @@ -0,0 +1,111 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +// SubscriptionManager pulls in a handful of browser/Dexie-heavy singletons at import time. Mock +// them so the module imports cleanly under the node test environment; the tests construct their +// own SubscriptionManager with an in-memory fake db, so the real db singleton is never used. +vi.mock("./Api", () => ({ default: {} })); +vi.mock("./Notifier", () => ({ default: {} })); +vi.mock("./Prefs", () => ({ default: {} })); +vi.mock("./db", () => ({ default: () => ({}) })); + +const { SubscriptionManager } = await import("./SubscriptionManager"); + +// Minimal in-memory stand-in for the Dexie "subscriptions" table, implementing just the surface +// that syncFromRemote() (and the upsert/remove/update helpers it calls) touches. +const fakeDb = () => { + const rows = new Map(); + return { + rows, + subscriptions: { + get: async (id) => rows.get(id), + put: async (sub) => { + rows.set(sub.id, sub); + }, + update: async (id, changes) => { + const existing = rows.get(id); + if (existing) { + rows.set(id, { ...existing, ...changes }); + } + }, + delete: async (id) => { + rows.delete(id); + }, + toArray: async () => Array.from(rows.values()), + }, + }; +}; + +const baseUrl = "https://ntfy.sh"; + +beforeEach(() => { + vi.spyOn(console, "log").mockImplementation(() => {}); +}); + +describe("SubscriptionManager.upsert", () => { + it("merges fields into an existing subscription without clobbering local-only state", async () => { + const db = fakeDb(); + const manager = new SubscriptionManager(db); + + await manager.upsert(baseUrl, "mytopic"); + await manager.setMutedUntil("https://ntfy.sh/mytopic", 123); + + const reservation = { topic: "mytopic", everyone: "deny-all" }; + await manager.upsert(baseUrl, "mytopic", { displayName: "My Topic", reservation }); + + const stored = db.rows.get("https://ntfy.sh/mytopic"); + expect(stored.reservation).toEqual(reservation); + expect(stored.displayName).toBe("My Topic"); + expect(stored.mutedUntil).toBe(123); // local-only state preserved + }); + + it("does not write when an existing subscription would not change", async () => { + const db = fakeDb(); + const manager = new SubscriptionManager(db); + + await manager.upsert(baseUrl, "mytopic", { internal: true }); + const putSpy = vi.spyOn(db.subscriptions, "put"); + + const result = await manager.upsert(baseUrl, "mytopic", { internal: true }); + + expect(putSpy).not.toHaveBeenCalled(); + expect(result.topic).toBe("mytopic"); + }); +}); + +describe("SubscriptionManager.syncFromRemote", () => { + it("persists a reservation onto a subscription that already exists locally", async () => { + const db = fakeDb(); + const manager = new SubscriptionManager(db); + + // Topic was subscribed to before it was reserved, so it already exists locally without a + // reservation -- exactly the state when a user clicks "Reserve topic" in the navbar. + await manager.upsert(baseUrl, "mytopic"); + expect(db.rows.get("https://ntfy.sh/mytopic").reservation).toBeFalsy(); + + const reservation = { topic: "mytopic", everyone: "deny-all" }; + await manager.syncFromRemote([{ base_url: baseUrl, topic: "mytopic" }], [reservation]); + + expect(db.rows.get("https://ntfy.sh/mytopic").reservation).toEqual(reservation); + }); + + it("clears the reservation when the remote no longer reports one", async () => { + const db = fakeDb(); + const manager = new SubscriptionManager(db); + + await manager.upsert(baseUrl, "mytopic", { reservation: { topic: "mytopic", everyone: "deny-all" } }); + + await manager.syncFromRemote([{ base_url: baseUrl, topic: "mytopic" }], []); + + expect(db.rows.get("https://ntfy.sh/mytopic").reservation).toBeNull(); + }); + + it("updates the display name on an existing subscription", async () => { + const db = fakeDb(); + const manager = new SubscriptionManager(db); + + await manager.upsert(baseUrl, "mytopic"); + await manager.syncFromRemote([{ base_url: baseUrl, topic: "mytopic", display_name: "My Topic" }], []); + + expect(db.rows.get("https://ntfy.sh/mytopic").displayName).toBe("My Topic"); + }); +}); diff --git a/web/src/components/SubscribeDialog.jsx b/web/src/components/SubscribeDialog.jsx index 5249dfb0..d0ceda61 100644 --- a/web/src/components/SubscribeDialog.jsx +++ b/web/src/components/SubscribeDialog.jsx @@ -34,7 +34,7 @@ import prefs from "../app/Prefs"; const publicBaseUrl = "https://ntfy.sh"; export const subscribeTopic = async (baseUrl, topic, opts) => { - const subscription = await subscriptionManager.add(baseUrl, topic, opts); + const subscription = await subscriptionManager.upsert(baseUrl, topic, opts); if (session.exists()) { try { await accountApi.addSubscription(baseUrl, topic); diff --git a/web/src/components/hooks.js b/web/src/components/hooks.js index db832809..b2502b48 100644 --- a/web/src/components/hooks.js +++ b/web/src/components/hooks.js @@ -111,7 +111,7 @@ export const useConnectionListeners = (account, subscriptions, users, webPushTop if (!account || !account.sync_topic) { return; } - subscriptionManager.add(config.base_url, account.sync_topic, { internal: true }); // Dangle! + subscriptionManager.upsert(config.base_url, account.sync_topic, { internal: true }); // Dangle! }, [account]); // When subscriptions or users change, refresh the connections @@ -139,7 +139,7 @@ export const useAutoSubscribe = (subscriptions, selected) => { const baseUrl = params.baseUrl ? expandSecureUrl(params.baseUrl) : config.base_url; console.log(`[Hooks] Auto-subscribing to ${topicUrl(baseUrl, params.topic)}`); (async () => { - const subscription = await subscriptionManager.add(baseUrl, params.topic); + const subscription = await subscriptionManager.upsert(baseUrl, params.topic); if (session.exists()) { try { await accountApi.addSubscription(baseUrl, params.topic);