From e8216ae9e7eacdce9f84b295e3f51b13a4acb98f Mon Sep 17 00:00:00 2001
From: nimbleghost <132819643+nimbleghost@users.noreply.github.com>
Date: Thu, 29 Jun 2023 00:02:18 +0200
Subject: [PATCH] Fix resubscribing when notifications are re-granted

(case: from denied to granted)
---
 web/public/static/langs/en.json    |  6 +++---
 web/src/app/Notifier.js            |  3 ---
 web/src/app/SubscriptionManager.js |  9 +++++----
 web/src/components/hooks.js        | 10 +++++-----
 4 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/web/public/static/langs/en.json b/web/public/static/langs/en.json
index 0b74c6e2..3f2947e4 100644
--- a/web/public/static/langs/en.json
+++ b/web/public/static/langs/en.json
@@ -55,14 +55,14 @@
   "nav_upgrade_banner_label": "Upgrade to ntfy Pro",
   "nav_upgrade_banner_description": "Reserve topics, more messages & emails, and larger attachments",
   "alert_notification_permission_required_title": "Notifications are disabled",
-  "alert_notification_permission_required_description": "Grant your browser permission to display desktop notifications.",
+  "alert_notification_permission_required_description": "Grant your browser permission to display desktop notifications",
   "alert_notification_permission_required_button": "Grant now",
   "alert_notification_permission_denied_title": "Notifications are blocked",
-  "alert_notification_permission_denied_description": "Please re-enable them in your browser and refresh the page to receive notifications",
+  "alert_notification_permission_denied_description": "Please re-enable them in your browser",
   "alert_notification_ios_install_required_title": "iOS install required",
   "alert_notification_ios_install_required_description": "Click on the Share icon and Add to Home Screen to enable notifications on iOS",
   "alert_not_supported_title": "Notifications not supported",
-  "alert_not_supported_description": "Notifications are not supported in your browser.",
+  "alert_not_supported_description": "Notifications are not supported in your browser",
   "alert_not_supported_context_description": "Notifications are only supported over HTTPS. This is a limitation of the <mdnLink>Notifications API</mdnLink>.",
   "notifications_list": "Notifications list",
   "notifications_list_item": "Notification",
diff --git a/web/src/app/Notifier.js b/web/src/app/Notifier.js
index 7f394ae9..4089746a 100644
--- a/web/src/app/Notifier.js
+++ b/web/src/app/Notifier.js
@@ -44,9 +44,6 @@ class Notifier {
   }
 
   async webPushSubscription(hasWebPushTopics) {
-    if (!this.pushPossible()) {
-      throw new Error("Unsupported or denied");
-    }
     const pushManager = await this.pushManager();
     const existingSubscription = await pushManager.getSubscription();
     if (existingSubscription) {
diff --git a/web/src/app/SubscriptionManager.js b/web/src/app/SubscriptionManager.js
index 28beb455..de99b642 100644
--- a/web/src/app/SubscriptionManager.js
+++ b/web/src/app/SubscriptionManager.js
@@ -27,7 +27,7 @@ class SubscriptionManager {
    * It is important to note that "mutedUntil" must be part of the where() query, otherwise the Dexie live query
    * will not react to it, and the Web Push topics will not be updated when the user mutes a topic.
    */
-  async webPushTopics(pushPossible = notifier.pushPossible()) {
+  async webPushTopics(pushPossible) {
     if (!pushPossible) {
       return [];
     }
@@ -120,13 +120,14 @@ class SubscriptionManager {
     );
   }
 
-  async updateWebPushSubscriptions(presetTopics) {
-    const topics = presetTopics ?? (await this.webPushTopics());
+  async updateWebPushSubscriptions(topics) {
     const hasWebPushTopics = topics.length > 0;
     const browserSubscription = await notifier.webPushSubscription(hasWebPushTopics);
 
     if (!browserSubscription) {
-      console.log("[SubscriptionManager] No browser subscription currently exists, so web push was never enabled. Skipping.");
+      console.log(
+        "[SubscriptionManager] No browser subscription currently exists, so web push was never enabled or the notification permission was removed. Skipping."
+      );
       return;
     }
 
diff --git a/web/src/components/hooks.js b/web/src/components/hooks.js
index 6d5f3d51..519d4c6a 100644
--- a/web/src/components/hooks.js
+++ b/web/src/components/hooks.js
@@ -168,12 +168,12 @@ export const useNotificationPermissionListener = (query) => {
  * the service worker, since the service worker cannot play sounds.
  */
 const useWebPushListener = (topics) => {
-  const [lastTopics, setLastTopics] = useState();
+  const [prevUpdate, setPrevUpdate] = useState();
   const pushPossible = useNotificationPermissionListener(() => notifier.pushPossible());
 
   useEffect(() => {
-    const topicsChanged = JSON.stringify(topics) !== JSON.stringify(lastTopics);
-    if (!pushPossible || !topicsChanged) {
+    const nextUpdate = JSON.stringify({ topics, pushPossible });
+    if (topics === undefined || nextUpdate === prevUpdate) {
       return;
     }
 
@@ -181,12 +181,12 @@ const useWebPushListener = (topics) => {
       try {
         console.log("[useWebPushListener] Refreshing web push subscriptions", topics);
         await subscriptionManager.updateWebPushSubscriptions(topics);
-        setLastTopics(topics);
+        setPrevUpdate(nextUpdate);
       } catch (e) {
         console.error("[useWebPushListener] Error refreshing web push subscriptions", e);
       }
     })();
-  }, [topics, lastTopics]);
+  }, [topics, pushPossible, prevUpdate]);
 
   useEffect(() => {
     const onMessage = () => {