From 6b464d1b8d9507d791f64d480858a61c6e989751 Mon Sep 17 00:00:00 2001
From: Jarek Radosz <jradosz@gmail.com>
Date: Tue, 8 Dec 2020 01:11:35 +0100
Subject: [PATCH] FIX: Restore dismissing the first notification (#10433)

* FIX: Restore dismissing the first notification

Reverts the temporary fix (8e4fea897e68e4b50e548f820dc1f1fdeeeb199d) and restores the feature introduced in e638d43f0a7549a75eb9de57bb8508b36f11543d.

The issue that was the reason for the revert (https://meta.discourse.org/t/logins-redirects-to-missing-notifications-page/149718) was a combination of two bugs:

1. Fixed in this commit - the click listener was accidentally registered also for logged-out users. This meant that the first click on a page always trigger an AJAX call to the notifications endpoint (`/notifications?recent=true&limit=5`), which returned a 403 error. Now, this code is run only when the user is logged in.

2. A still unknown bug that I could not reproduce, which was somehow setting the login redirect cookie to the URL of that previously failed AJAX request.
---
 .../discourse/app/components/site-header.js   |  9 +--
 .../components/site-header-test.js            | 56 +++++++++++++++++++
 2 files changed, 61 insertions(+), 4 deletions(-)
 create mode 100644 app/assets/javascripts/discourse/tests/integration/components/site-header-test.js

diff --git a/app/assets/javascripts/discourse/app/components/site-header.js b/app/assets/javascripts/discourse/app/components/site-header.js
index b1d5146ae0d..578a4a4e47f 100644
--- a/app/assets/javascripts/discourse/app/components/site-header.js
+++ b/app/assets/javascripts/discourse/app/components/site-header.js
@@ -209,6 +209,7 @@ const SiteHeaderComponent = MountWidget.extend(Docking, PanEvents, {
 
     // Allow first notification to be dismissed on a click anywhere
     if (
+      this.currentUser &&
       !this.get("currentUser.read_first_notification") &&
       !this.get("currentUser.enforcedSecondFactor")
     ) {
@@ -216,6 +217,7 @@ const SiteHeaderComponent = MountWidget.extend(Docking, PanEvents, {
         if (
           !e.target.closest("#current-user") &&
           !e.target.closest(".ring-backdrop") &&
+          this.currentUser &&
           !this.get("currentUser.read_first_notification") &&
           !this.get("currentUser.enforcedSecondFactor")
         ) {
@@ -225,10 +227,9 @@ const SiteHeaderComponent = MountWidget.extend(Docking, PanEvents, {
           );
         }
       };
-      // TODO: re-enable event listener
-      //      document.addEventListener("click", this._dismissFirstNotification, {
-      //        once: true
-      //      });
+      document.addEventListener("click", this._dismissFirstNotification, {
+        once: true,
+      });
     }
   },
 
diff --git a/app/assets/javascripts/discourse/tests/integration/components/site-header-test.js b/app/assets/javascripts/discourse/tests/integration/components/site-header-test.js
new file mode 100644
index 00000000000..5481456e362
--- /dev/null
+++ b/app/assets/javascripts/discourse/tests/integration/components/site-header-test.js
@@ -0,0 +1,56 @@
+import {
+  discourseModule,
+  queryAll,
+} from "discourse/tests/helpers/qunit-helpers";
+import componentTest, {
+  setupRenderingTest,
+} from "discourse/tests/helpers/component-test";
+import pretender from "discourse/tests/helpers/create-pretender";
+
+discourseModule("Integration | Component | site-header", function (hooks) {
+  setupRenderingTest(hooks);
+
+  componentTest("first notification mask", {
+    template: `{{site-header}}`,
+
+    beforeEach() {
+      this.set("currentUser.unread_high_priority_notifications", 1);
+      this.set("currentUser.read_first_notification", false);
+    },
+
+    async test(assert) {
+      assert.ok(
+        queryAll(".ring-backdrop").length === 1,
+        "there is the first notification mask"
+      );
+
+      // Click anywhere
+      await click("header.d-header");
+
+      assert.ok(
+        queryAll(".ring-backdrop").length === 0,
+        "it hides the first notification mask"
+      );
+    },
+  });
+
+  componentTest("do not call authenticated endpoints as anonymous", {
+    template: `{{site-header}}`,
+    anonymous: true,
+
+    async test(assert) {
+      assert.ok(
+        queryAll(".ring-backdrop").length === 0,
+        "there is no first notification mask for anonymous users"
+      );
+
+      pretender.get("/notifications", () => {
+        assert.ok(false, "it should not try to refresh notifications");
+        return [403, { "Content-Type": "application/json" }, {}];
+      });
+
+      // Click anywhere
+      await click("header.d-header");
+    },
+  });
+});