From 5c40b3380a1cfdbc8086a0b5e3ddb7cf41b9c2d0 Mon Sep 17 00:00:00 2001
From: Thor Galle <thorgalle@gmail.com>
Date: Fri, 6 Oct 2023 17:57:24 +0300
Subject: [PATCH] fix: send FCM notifications with click links matching client
 hosts

---
 api/src/chat.js                   | 16 +++++++++++-----
 src/lib/types/PushRegistration.ts |  1 +
 src/service-worker.ts             |  5 +++++
 3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/api/src/chat.js b/api/src/chat.js
index 0ddaefd8..579b742a 100644
--- a/api/src/chat.js
+++ b/api/src/chat.js
@@ -121,12 +121,18 @@ exports.onMessageCreate = async (snap, context) => {
     await Promise.all(
       pushRegistrations
         .filter((pR) => pR.status === 'active')
-        .map((pR) =>
-          sendNotification({
+        .map(({ host, fcmToken }) => {
+          return sendNotification({
             ...commonPayload,
-            fcmToken: pR.fcmToken
-          })
-        )
+            // Override the messageUrl with the host from the push registration, so it also works for beta.welcometomygarden.org.
+            // Firebase's JS SDK filters out non-matching hosts on click events in their service worker code
+            // probably with reason, because: https://developer.mozilla.org/en-US/docs/Web/API/Clients/openWindow#parameters :
+            //   > A string representing the URL of the client you want to open in the window.
+            //   > **Generally this value must be a URL from the same origin as the calling script.**
+            messageUrl: `https://${host}/chat/${normalizeName(nameParts[0])}/${chatId}`,
+            fcmToken
+          });
+        })
     );
   } catch (ex) {
     console.log(ex);
diff --git a/src/lib/types/PushRegistration.ts b/src/lib/types/PushRegistration.ts
index df3d71d9..b315f6c2 100644
--- a/src/lib/types/PushRegistration.ts
+++ b/src/lib/types/PushRegistration.ts
@@ -43,6 +43,7 @@ export type FirebasePushRegistration = {
   /** The host that this subscription was created for. This could be useful when we have multiple apps
    * connecting to the same Firestore. Format without protocol.
    * On localhost testing, it seems that the port matters for Web Push registrations.
+   * This does not include the protocol! Also, no trailing slash.
    */
   host: string;
   /**
diff --git a/src/service-worker.ts b/src/service-worker.ts
index 711466c0..ec4c5679 100644
--- a/src/service-worker.ts
+++ b/src/service-worker.ts
@@ -150,6 +150,11 @@ const messaging = getMessaging(firebaseApp);
 // Probably this internally does: self.addEventListener('push', ... ), and then
 // it check if the app is in the foreground or background. If it is in the foreground,
 // it will forward the data to the onMessage() handler on the main app instead.
+//
+// NOTE: THIS TAKES OVER CERTAIN LISTENERS, AND BLOCKS THEM FROM BEING LISTENED TO HERE
+// If we register our own listeners above this (or possibly getMessaging()), then we can get the notificationclick event (tested).
+// https://github.com/firebase/quickstart-js/issues/194#issuecomment-361353318
+// https://github.com/firebase/firebase-js-sdk/blob/cbfd14cfb27cda8a6de74be5d138ea9e6de09fe9/packages/messaging/src/listeners/sw-listeners.ts#L128
 onBackgroundMessage(messaging, (payload) => {
   console.log('[service worker] Received background message ', payload);
   // Firebase will already send the notification.