Fix topic reservation icon in nav bar

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)
 

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!
         });

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");
+  });
+});

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);

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);