Răsfoiți Sursa

Fix grouping issue with sequence ID

binwiederhier 3 săptămâni în urmă
părinte
comite
5ba1c71140
4 a modificat fișierele cu 58 adăugiri și 30 ștergeri
  1. 32 14
      web/public/sw.js
  2. 7 5
      web/src/app/Notifier.js
  3. 10 2
      web/src/app/notificationUtils.js
  4. 9 9
      web/src/components/hooks.js

+ 32 - 14
web/public/sw.js

@@ -4,7 +4,7 @@ import { NavigationRoute, registerRoute } from "workbox-routing";
 import { NetworkFirst } from "workbox-strategies";
 import { clientsClaim } from "workbox-core";
 import { dbAsync } from "../src/app/db";
-import { badge, icon, messageWithSequenceId, toNotificationParams } from "../src/app/notificationUtils";
+import { badge, icon, messageWithSequenceId, notificationTag, toNotificationParams } from "../src/app/notificationUtils";
 import initI18n from "../src/app/i18n";
 import {
   EVENT_MESSAGE,
@@ -38,6 +38,13 @@ const handlePushMessage = async (data) => {
 
   console.log("[ServiceWorker] Message received", data);
 
+  // Look up subscription for baseUrl and topic
+  const subscription = await db.subscriptions.get(subscriptionId);
+  if (!subscription) {
+    console.log("[ServiceWorker] Subscription not found", subscriptionId);
+    return;
+  }
+
   // Delete existing notification with same sequence ID (if any)
   const sequenceId = message.sequence_id || message.id;
   if (sequenceId) {
@@ -65,10 +72,11 @@ const handlePushMessage = async (data) => {
 
   await self.registration.showNotification(
     ...toNotificationParams({
-      subscriptionId,
       message,
       defaultTitle: message.topic,
       topicRoute: new URL(message.topic, self.location.origin).toString(),
+      baseUrl: subscription.baseUrl,
+      topic: subscription.topic,
     })
   );
 };
@@ -81,18 +89,23 @@ const handlePushMessageDelete = async (data) => {
   const db = await dbAsync();
   console.log("[ServiceWorker] Deleting notification sequence", data);
 
+  // Look up subscription for baseUrl and topic
+  const subscription = await db.subscriptions.get(subscriptionId);
+  if (!subscription) {
+    console.log("[ServiceWorker] Subscription not found", subscriptionId);
+    return;
+  }
+
   // Delete notification with the same sequence_id
   const sequenceId = message.sequence_id;
   if (sequenceId) {
     await db.notifications.where({ subscriptionId, sequenceId }).delete();
   }
 
-  // Close browser notification with matching tag
-  const tag = message.sequence_id || message.id;
-  if (tag) {
-    const notifications = await self.registration.getNotifications({ tag });
-    notifications.forEach((notification) => notification.close());
-  }
+  // Close browser notification with matching tag (scoped by topic)
+  const tag = notificationTag(subscription.baseUrl, subscription.topic, message.sequence_id || message.id);
+  const notifications = await self.registration.getNotifications({ tag });
+  notifications.forEach((notification) => notification.close());
 
   // Update subscription last message id (for ?since=... queries)
   await db.subscriptions.update(subscriptionId, {
@@ -108,18 +121,23 @@ const handlePushMessageClear = async (data) => {
   const db = await dbAsync();
   console.log("[ServiceWorker] Marking notification as read", data);
 
+  // Look up subscription for baseUrl and topic
+  const subscription = await db.subscriptions.get(subscriptionId);
+  if (!subscription) {
+    console.log("[ServiceWorker] Subscription not found", subscriptionId);
+    return;
+  }
+
   // Mark notification as read (set new = 0)
   const sequenceId = message.sequence_id;
   if (sequenceId) {
     await db.notifications.where({ subscriptionId, sequenceId }).modify({ new: 0 });
   }
 
-  // Close browser notification with matching tag
-  const tag = message.sequence_id || message.id;
-  if (tag) {
-    const notifications = await self.registration.getNotifications({ tag });
-    notifications.forEach((notification) => notification.close());
-  }
+  // Close browser notification with matching tag (scoped by topic)
+  const tag = notificationTag(subscription.baseUrl, subscription.topic, message.sequence_id || message.id);
+  const notifications = await self.registration.getNotifications({ tag });
+  notifications.forEach((notification) => notification.close());
 
   // Update subscription last message id (for ?since=... queries)
   await db.subscriptions.update(subscriptionId, {

+ 7 - 5
web/src/app/Notifier.js

@@ -1,5 +1,5 @@
 import { playSound, topicDisplayName, topicShortUrl, urlB64ToUint8Array } from "./utils";
-import { toNotificationParams } from "./notificationUtils";
+import { notificationTag, toNotificationParams } from "./notificationUtils";
 import prefs from "./Prefs";
 import routes from "../components/routes";
 
@@ -23,21 +23,23 @@ class Notifier {
     const registration = await this.serviceWorkerRegistration();
     await registration.showNotification(
       ...toNotificationParams({
-        subscriptionId: subscription.id,
         message: notification,
         defaultTitle,
         topicRoute: new URL(routes.forSubscription(subscription), window.location.origin).toString(),
+        baseUrl: subscription.baseUrl,
+        topic: subscription.topic,
       })
     );
   }
 
-  async cancel(notification) {
+  async cancel(subscription, notification) {
     if (!this.supported()) {
       return;
     }
     try {
-      const tag = notification.sequence_id || notification.id;
-      console.log(`[Notifier] Cancelling notification with ${tag}`);
+      const sequenceId = notification.sequence_id || notification.id;
+      const tag = notificationTag(subscription.baseUrl, subscription.topic, sequenceId);
+      console.log(`[Notifier] Cancelling notification with tag ${tag}`);
       const registration = await this.serviceWorkerRegistration();
       const notifications = await registration.getNotifications({ tag });
       notifications.forEach((n) => n.close());

+ 10 - 2
web/src/app/notificationUtils.js

@@ -50,8 +50,16 @@ export const isImage = (attachment) => {
 export const icon = "/static/images/ntfy.png";
 export const badge = "/static/images/mask-icon.svg";
 
-export const toNotificationParams = ({ message, defaultTitle, topicRoute }) => {
+/**
+ * Computes a unique notification tag scoped by baseUrl, topic, and sequence ID.
+ * This ensures notifications from different topics with the same sequence ID don't collide.
+ */
+export const notificationTag = (baseUrl, topic, sequenceId) => `${baseUrl}/${topic}/${sequenceId}`;
+
+export const toNotificationParams = ({ message, defaultTitle, topicRoute, baseUrl, topic }) => {
   const image = isImage(message.attachment) ? message.attachment.url : undefined;
+  const sequenceId = message.sequence_id || message.id;
+  const tag = notificationTag(baseUrl, topic, sequenceId);
 
   // https://developer.mozilla.org/en-US/docs/Web/API/Notifications_API
   return [
@@ -62,7 +70,7 @@ export const toNotificationParams = ({ message, defaultTitle, topicRoute }) => {
       icon,
       image,
       timestamp: message.time * 1000,
-      tag: message.sequence_id || message.id, // Update notification if there is a sequence ID
+      tag, // Scoped by baseUrl/topic/sequenceId to avoid cross-topic collisions
       renotify: true,
       silent: false,
       // This is used by the notification onclick event

+ 9 - 9
web/src/components/hooks.js

@@ -51,7 +51,7 @@ export const useConnectionListeners = (account, subscriptions, users, webPushTop
         }
       };
 
-      const handleNotification = async (subscriptionId, notification) => {
+      const handleNotification = async (subscription, notification) => {
         // This logic is (partially) duplicated in
         // - Android: SubscriberService::onNotificationReceived()
         // - Android: FirebaseService::onMessageReceived()
@@ -59,20 +59,20 @@ export const useConnectionListeners = (account, subscriptions, users, webPushTop
         // - Web app: sw.js:handleMessage(), sw.js:handleMessageClear(), ...
 
         if (notification.event === EVENT_MESSAGE_DELETE && notification.sequence_id) {
-          await subscriptionManager.deleteNotificationBySequenceId(subscriptionId, notification.sequence_id);
-          await notifier.cancel(notification);
+          await subscriptionManager.deleteNotificationBySequenceId(subscription.id, notification.sequence_id);
+          await notifier.cancel(subscription, notification);
         } else if (notification.event === EVENT_MESSAGE_CLEAR && notification.sequence_id) {
-          await subscriptionManager.markNotificationReadBySequenceId(subscriptionId, notification.sequence_id);
-          await notifier.cancel(notification);
+          await subscriptionManager.markNotificationReadBySequenceId(subscription.id, notification.sequence_id);
+          await notifier.cancel(subscription, notification);
         } else {
           // Regular message: delete existing and add new
           const sequenceId = notification.sequence_id || notification.id;
           if (sequenceId) {
-            await subscriptionManager.deleteNotificationBySequenceId(subscriptionId, sequenceId);
+            await subscriptionManager.deleteNotificationBySequenceId(subscription.id, sequenceId);
           }
-          const added = await subscriptionManager.addNotification(subscriptionId, notification);
+          const added = await subscriptionManager.addNotification(subscription.id, notification);
           if (added) {
-            await subscriptionManager.notify(subscriptionId, notification);
+            await subscriptionManager.notify(subscription.id, notification);
           }
         }
       };
@@ -89,7 +89,7 @@ export const useConnectionListeners = (account, subscriptions, users, webPushTop
         if (subscription.internal) {
           await handleInternalMessage(message);
         } else {
-          await handleNotification(subscriptionId, message);
+          await handleNotification(subscription, message);
         }
       };