FIX: Restore dismissing the first notification (#10433)

* FIX: Restore dismissing the first notification

Reverts the temporary fix (8e4fea897e) and restores the feature introduced in e638d43f0a.

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.
This commit is contained in:
Jarek Radosz 2020-12-08 01:11:35 +01:00 committed by GitHub
parent c69bb5d5be
commit 6b464d1b8d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 61 additions and 4 deletions

View File

@ -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,
});
}
},

View File

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