DEV: Add bookmarks tab to the new user menu (#17814)

Some of the changes in this commit are extracted from https://github.com/discourse/discourse/pull/17379.

The bookmarks tab in the new user menu is different from the other tabs in that it can display a mixture of notifications and bookmarks. When there are unread bookmark reminder notifications, the tab displays all of these notifications at the top and fills the remaining space in the menu with the rest of the bookmarks. The bubble/badge count on the bookmarks tab indicates how many unread bookmark reminder notifications there are.

On the technical aspect, since this commit introduces a new `bookmark-item` component, we've done some refactoring so that all 3 "item" components (`notification-item`, `reviewable-item` and the new `bookmark-item`) inherit from a base component and get identical HTML structure so they all look consistent.

Internal tickets: t70584 and t65045.
This commit is contained in:
Osama Sayegh 2022-08-08 17:24:04 +03:00 committed by GitHub
parent 94ac8611f4
commit 4fdb275683
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
47 changed files with 1024 additions and 135 deletions

View File

@ -0,0 +1,36 @@
import UserMenuItem from "discourse/components/user-menu/menu-item";
import { NO_REMINDER_ICON } from "discourse/models/bookmark";
export default class UserMenuBookmarkItem extends UserMenuItem {
get className() {
return "bookmark";
}
get linkHref() {
return this.bookmark.bookmarkable_url;
}
get linkTitle() {
return this.bookmark.name;
}
get icon() {
return NO_REMINDER_ICON;
}
get label() {
return this.bookmark.user?.username;
}
get description() {
return this.bookmark.title;
}
get topicId() {
return this.bookmark.topic_id;
}
get bookmark() {
return this.args.item;
}
}

View File

@ -0,0 +1 @@
{{component this.component item=@item}}

View File

@ -0,0 +1,12 @@
import GlimmerComponent from "discourse/components/glimmer";
import Notification from "discourse/models/notification";
export default class UserMenuBookmarkNotificationItem extends GlimmerComponent {
get component() {
if (this.args.item.constructor === Notification) {
return "user-menu/notification-item";
} else {
return "user-menu/bookmark-item";
}
}
}

View File

@ -0,0 +1,10 @@
<div class="empty-state">
<span class="empty-state-title">
{{i18n "user.no_bookmarks_title"}}
</span>
<div class="empty-state-body">
<p>
{{html-safe (i18n "user.no_bookmarks_body" icon=(d-icon "bookmark"))}}
</p>
</div>
</div>

View File

@ -0,0 +1,72 @@
import UserMenuNotificationsList from "discourse/components/user-menu/notifications-list";
import { ajax } from "discourse/lib/ajax";
import Notification from "discourse/models/notification";
import showModal from "discourse/lib/show-modal";
import I18n from "I18n";
export default class UserMenuBookmarksList extends UserMenuNotificationsList {
get dismissTypes() {
return ["bookmark_reminder"];
}
get showAllHref() {
return `${this.currentUser.path}/activity/bookmarks`;
}
get showAllTitle() {
return I18n.t("user_menu.view_all_bookmarks");
}
get showDismiss() {
return this.#unreadBookmarkRemindersCount > 0;
}
get dismissTitle() {
return I18n.t("user.dismiss_bookmarks_tooltip");
}
get itemsCacheKey() {
return "user-menu-bookmarks-tab";
}
get itemComponent() {
return "user-menu/bookmark-notification-item";
}
get emptyStateComponent() {
return "user-menu/bookmarks-list-empty-state";
}
get #unreadBookmarkRemindersCount() {
const key = `grouped_unread_high_priority_notifications.${this.site.notification_types.bookmark_reminder}`;
// we're retrieving the value with get() so that Ember tracks the property
// and re-renders the UI when it changes.
// we can stop using `get()` when the User model is refactored into native
// class with @tracked properties.
return this.currentUser.get(key) || 0;
}
fetchItems() {
return ajax(`/u/${this.currentUser.username}/user-menu-bookmarks`).then(
(data) => {
const content = [];
data.notifications.forEach((notification) => {
content.push(Notification.create(notification));
});
content.push(...data.bookmarks);
return content;
}
);
}
dismissWarningModal() {
const modalController = showModal("dismiss-notification-confirmation");
modalController.set(
"confirmationMessage",
I18n.t("notifications.dismiss_confirmation.body.bookmarks", {
count: this.#unreadBookmarkRemindersCount,
})
);
return modalController;
}
}

View File

@ -5,6 +5,10 @@ export default class UserMenuLikesNotificationsList extends UserMenuNotification
return ["liked", "liked_consolidated"]; return ["liked", "liked_consolidated"];
} }
get dismissTypes() {
return this.filterByTypes;
}
dismissWarningModal() { dismissWarningModal() {
return null; return null;
} }

View File

@ -5,6 +5,10 @@ export default class UserMenuMentionsNotificationsList extends UserMenuNotificat
return ["mentioned"]; return ["mentioned"];
} }
get dismissTypes() {
return this.filterByTypes;
}
dismissWarningModal() { dismissWarningModal() {
return null; return null;
} }

View File

@ -7,15 +7,12 @@
{{d-icon this.icon}} {{d-icon this.icon}}
<div> <div>
{{#if this.label}} {{#if this.label}}
<span class={{concat "notification-label " this.labelWrapperClasses}}> <span class={{concat "item-label " this.labelClass}}>
{{this.label}} {{this.label}}
</span> </span>
{{/if}} {{/if}}
{{#if this.description}} {{#if this.description}}
<span <span class={{concat "item-description " this.descriptionClass}} data-topic-id={{this.topicId}}>
class={{concat "notification-description " this.descriptionWrapperClasses}}
data-topic-id={{this.topicId}}
>
{{this.description}} {{this.description}}
</span> </span>
{{/if}} {{/if}}

View File

@ -0,0 +1,35 @@
import GlimmerComponent from "discourse/components/glimmer";
import { action } from "@ember/object";
export default class UserMenuItem extends GlimmerComponent {
get className() {}
get linkHref() {
throw new Error("not implemented");
}
get linkTitle() {
throw new Error("not implemented");
}
get icon() {
throw new Error("not implemented");
}
get label() {
throw new Error("not implemented");
}
get labelClass() {}
get description() {
throw new Error("not implemented");
}
get descriptionClass() {}
get topicId() {}
@action
onClick() {}
}

View File

@ -1,6 +1,7 @@
import GlimmerComponent from "discourse/components/glimmer"; import GlimmerComponent from "discourse/components/glimmer";
import { tracked } from "@glimmer/tracking"; import { tracked } from "@glimmer/tracking";
import { action } from "@ember/object"; import { action } from "@ember/object";
import { NO_REMINDER_ICON } from "discourse/models/bookmark";
import UserMenuTab from "discourse/lib/user-menu/tab"; import UserMenuTab from "discourse/lib/user-menu/tab";
const DEFAULT_TAB_ID = "all-notifications"; const DEFAULT_TAB_ID = "all-notifications";
@ -69,6 +70,24 @@ const CORE_TOP_TABS = [
} }
}, },
class extends UserMenuTab {
get id() {
return "bookmarks";
}
get icon() {
return NO_REMINDER_ICON;
}
get panelComponent() {
return "user-menu/bookmarks-list";
}
get count() {
return this.getUnreadCountForType("bookmark_reminder");
}
},
class extends UserMenuTab { class extends UserMenuTab {
get id() { get id() {
return REVIEW_QUEUE_TAB_ID; return REVIEW_QUEUE_TAB_ID;

View File

@ -1,11 +1,11 @@
import GlimmerComponent from "discourse/components/glimmer"; import UserMenuItem from "discourse/components/user-menu/menu-item";
import { setTransientHeader } from "discourse/lib/ajax"; import { setTransientHeader } from "discourse/lib/ajax";
import { action } from "@ember/object"; import { action } from "@ember/object";
import { getRenderDirector } from "discourse/lib/notification-item"; import { getRenderDirector } from "discourse/lib/notification-item";
import getURL from "discourse-common/lib/get-url"; import getURL from "discourse-common/lib/get-url";
import cookie from "discourse/lib/cookie"; import cookie from "discourse/lib/cookie";
export default class UserMenuNotificationItem extends GlimmerComponent { export default class UserMenuNotificationItem extends UserMenuItem {
constructor() { constructor() {
super(...arguments); super(...arguments);
this.renderDirector = getRenderDirector( this.renderDirector = getRenderDirector(
@ -18,9 +18,11 @@ export default class UserMenuNotificationItem extends GlimmerComponent {
} }
get className() { get className() {
const classes = []; const classes = ["notification"];
if (this.notification.read) { if (this.notification.read) {
classes.push("read"); classes.push("read");
} else {
classes.push("unread");
} }
if (this.#notificationName) { if (this.#notificationName) {
classes.push(this.#notificationName.replace(/_/g, "-")); classes.push(this.#notificationName.replace(/_/g, "-"));
@ -51,16 +53,20 @@ export default class UserMenuNotificationItem extends GlimmerComponent {
return this.renderDirector.label; return this.renderDirector.label;
} }
get labelWrapperClasses() { get labelClass() {
return this.renderDirector.labelWrapperClasses?.join(" ") || ""; return this.renderDirector.labelClasses?.join(" ") || "";
} }
get description() { get description() {
return this.renderDirector.description; return this.renderDirector.description;
} }
get descriptionWrapperClasses() { get descriptionClass() {
return this.renderDirector.descriptionWrapperClasses?.join(" ") || ""; return this.renderDirector.descriptionClasses?.join(" ") || "";
}
get topicId() {
return this.notification.topic_id;
} }
get notification() { get notification() {

View File

@ -10,6 +10,10 @@ export default class UserMenuNotificationsList extends UserMenuItemsList {
return null; return null;
} }
get dismissTypes() {
return null;
}
get showAllHref() { get showAllHref() {
return `${this.currentUser.path}/notifications`; return `${this.currentUser.path}/notifications`;
} }
@ -70,8 +74,10 @@ export default class UserMenuNotificationsList extends UserMenuItemsList {
if (this.currentUser.unread_high_priority_notifications > 0) { if (this.currentUser.unread_high_priority_notifications > 0) {
const modalController = showModal("dismiss-notification-confirmation"); const modalController = showModal("dismiss-notification-confirmation");
modalController.set( modalController.set(
"count", "confirmationMessage",
this.currentUser.unread_high_priority_notifications I18n.t("notifications.dismiss_confirmation.body.default", {
count: this.currentUser.unread_high_priority_notifications,
})
); );
return modalController; return modalController;
} }
@ -80,7 +86,7 @@ export default class UserMenuNotificationsList extends UserMenuItemsList {
@action @action
dismissButtonClick() { dismissButtonClick() {
const opts = { type: "PUT" }; const opts = { type: "PUT" };
const dismissTypes = this.filterByTypes; const dismissTypes = this.dismissTypes;
if (dismissTypes?.length > 0) { if (dismissTypes?.length > 0) {
opts.data = { dismiss_types: dismissTypes.join(",") }; opts.data = { dismiss_types: dismissTypes.join(",") };
} }

View File

@ -5,6 +5,10 @@ export default class UserMenuRepliesNotificationsList extends UserMenuNotificati
return ["replied"]; return ["replied"];
} }
get dismissTypes() {
return this.filterByTypes;
}
dismissWarningModal() { dismissWarningModal() {
return null; return null;
} }

View File

@ -1,9 +0,0 @@
<li class={{unless this.reviewable.pending "reviewed"}}>
<LinkTo @route="review.show" @model={{this.reviewable.id}}>
{{d-icon this.icon}}
<div>
<span class="reviewable-label">{{this.actor}}</span>
<span class="reviewable-description">{{this.description}}</span>
</div>
</LinkTo>
</li>

View File

@ -1,7 +1,8 @@
import GlimmerComponent from "discourse/components/glimmer"; import UserMenuItem from "discourse/components/user-menu/menu-item";
import getURL from "discourse-common/lib/get-url";
import { getRenderDirector } from "discourse/lib/reviewable-item"; import { getRenderDirector } from "discourse/lib/reviewable-item";
export default class UserMenuReviewableItem extends GlimmerComponent { export default class UserMenuReviewableItem extends UserMenuItem {
constructor() { constructor() {
super(...arguments); super(...arguments);
this.reviewable = this.args.item; this.reviewable = this.args.item;
@ -14,15 +15,34 @@ export default class UserMenuReviewableItem extends GlimmerComponent {
); );
} }
get actor() { get className() {
const classes = ["reviewable"];
if (this.reviewable.pending) {
classes.push("pending");
} else {
classes.push("reviewed");
}
return classes.join(" ");
}
get linkHref() {
return getURL(`/review/${this.reviewable.id}`);
}
get linkTitle() {
// TODO(osama): add title
return "";
}
get icon() {
return this.renderDirector.icon;
}
get label() {
return this.renderDirector.actor; return this.renderDirector.actor;
} }
get description() { get description() {
return this.renderDirector.description; return this.renderDirector.description;
} }
get icon() {
return this.renderDirector.icon;
}
} }

View File

@ -63,7 +63,12 @@ export default Controller.extend({
if (unreadHighPriorityNotifications > 0) { if (unreadHighPriorityNotifications > 0) {
showModal("dismiss-notification-confirmation").setProperties({ showModal("dismiss-notification-confirmation").setProperties({
count: unreadHighPriorityNotifications, confirmationMessage: I18n.t(
"notifications.dismiss_confirmation.body.default",
{
count: unreadHighPriorityNotifications,
}
),
dismissNotifications: () => this.markRead(), dismissNotifications: () => this.markRead(),
}); });
} else { } else {

View File

@ -15,8 +15,16 @@ export default {
const appEvents = container.lookup("service:app-events"); const appEvents = container.lookup("service:app-events");
appEvents.on("notifications:changed", () => { appEvents.on("notifications:changed", () => {
const notifications = let notifications;
user.unread_notifications + user.unread_high_priority_notifications; if (user.redesigned_user_menu_enabled) {
notifications = user.all_unread_notifications_count;
if (user.unseen_reviewable_count) {
notifications += user.unseen_reviewable_count;
}
} else {
notifications =
user.unread_notifications + user.unread_high_priority_notifications;
}
navigator.setAppBadge(notifications); navigator.setAppBadge(notifications);
}); });

View File

@ -51,6 +51,8 @@ export default {
data.unread_high_priority_notifications, data.unread_high_priority_notifications,
read_first_notification: data.read_first_notification, read_first_notification: data.read_first_notification,
all_unread_notifications_count: data.all_unread_notifications_count, all_unread_notifications_count: data.all_unread_notifications_count,
grouped_unread_high_priority_notifications:
data.grouped_unread_high_priority_notifications,
}); });
if ( if (

View File

@ -82,16 +82,16 @@ export default class NotificationItemBase {
onClick() {} onClick() {}
/** /**
* @returns {string[]} Include additional classes to the label's wrapper <span>. * @returns {string[]} Include additional classes to the label.
*/ */
get labelWrapperClasses() { get labelClasses() {
return []; return [];
} }
/** /**
* @returns {string[]} Include additional classes to the description's wrapper <span>. * @returns {string[]} Include additional classes to the description.
*/ */
get descriptionWrapperClasses() { get descriptionClasses() {
return []; return [];
} }

View File

@ -5,7 +5,7 @@ export default class extends NotificationItemBase {
return `${this.username} @${this.notification.data.group_name}`; return `${this.username} @${this.notification.data.group_name}`;
} }
get labelWrapperClasses() { get labelClasses() {
return ["mention-group", "notify"]; return ["mention-group", "notify"];
} }
} }

View File

@ -20,7 +20,7 @@ export default class extends NotificationItemBase {
} }
} }
get labelWrapperClasses() { get labelClasses() {
if (this.count === 2) { if (this.count === 2) {
return ["double-user"]; return ["double-user"];
} else if (this.count > 2) { } else if (this.count > 2) {

View File

@ -24,4 +24,13 @@ export default class UserMenuTab {
get icon() { get icon() {
throw new Error("not implemented"); throw new Error("not implemented");
} }
getUnreadCountForType(type) {
const key = `grouped_unread_high_priority_notifications.${this.site.notification_types[type]}`;
// we're retrieving the value with get() so that Ember tracks the property
// and re-renders the UI when it changes.
// we can stop using `get()` when the User model is refactored into native
// class with @tracked properties.
return this.currentUser.get(key) || 0;
}
} }

View File

@ -1,5 +1,5 @@
<DModalBody @headerClass="hidden" @class="dismiss-notification-confirmation"> <DModalBody @headerClass="hidden" @class="dismiss-notification-confirmation">
{{i18n "notifications.dismiss_confirmation.body" count=this.count}} {{this.confirmationMessage}}
</DModalBody> </DModalBody>
<div class="modal-footer"> <div class="modal-footer">

View File

@ -2,6 +2,7 @@ import discourseLater from "discourse-common/lib/later";
import { createWidget } from "discourse/widgets/widget"; import { createWidget } from "discourse/widgets/widget";
import { h } from "virtual-dom"; import { h } from "virtual-dom";
import showModal from "discourse/lib/show-modal"; import showModal from "discourse/lib/show-modal";
import I18n from "I18n";
const UserMenuAction = { const UserMenuAction = {
QUICK_ACCESS: "quickAccess", QUICK_ACCESS: "quickAccess",
@ -256,7 +257,12 @@ export default createWidget("user-menu", {
if (unreadHighPriorityNotifications > 0) { if (unreadHighPriorityNotifications > 0) {
return showModal("dismiss-notification-confirmation").setProperties({ return showModal("dismiss-notification-confirmation").setProperties({
count: unreadHighPriorityNotifications, confirmationMessage: I18n.t(
"notifications.dismiss_confirmation.body.default",
{
count: unreadHighPriorityNotifications,
}
),
dismissNotifications: () => this.state.markRead(), dismissNotifications: () => this.state.markRead(),
}); });
} else { } else {

View File

@ -34,7 +34,7 @@ acceptance("Dismiss notification confirmation", function (needs) {
assert.strictEqual( assert.strictEqual(
query(".dismiss-notification-confirmation-modal .modal-body").innerText, query(".dismiss-notification-confirmation-modal .modal-body").innerText,
I18n.t("notifications.dismiss_confirmation.body", { count: 2 }) I18n.t("notifications.dismiss_confirmation.body.default", { count: 2 })
); );
}); });

View File

@ -1,11 +1,15 @@
import { click, visit } from "@ember/test-helpers"; import { click, visit } from "@ember/test-helpers";
import { import {
acceptance, acceptance,
exists,
loggedInUser,
publishToMessageBus,
query, query,
updateCurrentUser,
} from "discourse/tests/helpers/qunit-helpers"; } from "discourse/tests/helpers/qunit-helpers";
import { test } from "qunit"; import { test } from "qunit";
import { cloneJSON } from "discourse-common/lib/object"; import { cloneJSON } from "discourse-common/lib/object";
import { NOTIFICATION_TYPES } from "discourse/tests/fixtures/concerns/notification-types";
import UserMenuFixtures from "discourse/tests/fixtures/user-menu";
import TopicFixtures from "discourse/tests/fixtures/topic"; import TopicFixtures from "discourse/tests/fixtures/topic";
import I18n from "I18n"; import I18n from "I18n";
@ -46,19 +50,39 @@ acceptance("User menu - Dismiss button", function (needs) {
needs.user({ needs.user({
redesigned_user_menu_enabled: true, redesigned_user_menu_enabled: true,
unread_high_priority_notifications: 10, unread_high_priority_notifications: 10,
grouped_unread_high_priority_notifications: {
[NOTIFICATION_TYPES.bookmark_reminder]: 103,
},
}); });
let markRead = false; let markRead = false;
let markReadRequestBody;
needs.pretender((server, helper) => { needs.pretender((server, helper) => {
server.put("/notifications/mark-read", () => { server.put("/notifications/mark-read", (request) => {
markReadRequestBody = request.requestBody;
markRead = true; markRead = true;
return helper.response({ success: true }); return helper.response({ success: true });
}); });
server.get("/u/eviltrout/user-menu-bookmarks", () => {
if (markRead) {
const copy = cloneJSON(
UserMenuFixtures["/u/:username/user-menu-bookmarks"]
);
copy.notifications = [];
return helper.response(copy);
} else {
return helper.response(
UserMenuFixtures["/u/:username/user-menu-bookmarks"]
);
}
});
}); });
needs.hooks.afterEach(() => { needs.hooks.afterEach(() => {
markRead = false; markRead = false;
markReadRequestBody = null;
}); });
test("shows confirmation modal for the all-notifications list", async function (assert) { test("shows confirmation modal for the all-notifications list", async function (assert) {
@ -68,14 +92,16 @@ acceptance("User menu - Dismiss button", function (needs) {
await click(".user-menu .notifications-dismiss"); await click(".user-menu .notifications-dismiss");
assert.strictEqual( assert.strictEqual(
query(".dismiss-notification-confirmation").textContent.trim(), query(".dismiss-notification-confirmation").textContent.trim(),
I18n.t("notifications.dismiss_confirmation.body", { count: 10 }), I18n.t("notifications.dismiss_confirmation.body.default", { count: 10 }),
"confirmation modal is shown when there are unread high pri notifications" "confirmation modal is shown when there are unread high pri notifications"
); );
assert.notOk(markRead, "mark-read request isn't sent"); assert.notOk(markRead, "mark-read request isn't sent");
await click(".modal-footer .btn-default"); // click cancel on the dismiss modal await click(".modal-footer .btn-default"); // click cancel on the dismiss modal
updateCurrentUser({ unread_high_priority_notifications: 0 }); await publishToMessageBus(`/notification/${loggedInUser().id}`, {
unread_high_priority_notifications: 0,
});
await click(".user-menu .notifications-dismiss"); await click(".user-menu .notifications-dismiss");
assert.ok( assert.ok(
markRead, markRead,
@ -83,7 +109,64 @@ acceptance("User menu - Dismiss button", function (needs) {
); );
}); });
test("doesn't show confirmation modal for the likes notifications panel/list", async function (assert) { test("shows confirmation modal for the bookmarks list", async function (assert) {
await visit("/");
await click(".d-header-icons .current-user");
assert.strictEqual(
query("#user-menu-button-bookmarks .badge-notification").textContent,
"103",
"bookmarks tab has bubble with count"
);
await click("#user-menu-button-bookmarks");
assert.ok(
exists("#quick-access-bookmarks ul li.notification"),
"bookmark reminder notifications are visible"
);
assert.ok(
exists("#quick-access-bookmarks ul li.bookmark"),
"bookmarks are visible"
);
await click(".user-menu .notifications-dismiss");
assert.strictEqual(
query(".dismiss-notification-confirmation").textContent.trim(),
I18n.t("notifications.dismiss_confirmation.body.bookmarks", {
count: 103,
}),
"confirmation modal is shown when there are unread bookmark reminder notifications"
);
assert.notOk(markRead, "mark-read request isn't sent");
await click(".modal-footer .btn-primary"); // confirm dismiss on the dismiss modal
await publishToMessageBus(`/notification/${loggedInUser().id}`, {
grouped_unread_high_priority_notifications: {},
});
assert.notOk(
exists("#quick-access-bookmarks ul li.notification"),
"bookmark reminder notifications are gone"
);
assert.ok(
exists("#quick-access-bookmarks ul li.bookmark"),
"bookmarks are still visible"
);
assert.notOk(
exists("#user-menu-button-bookmarks .badge-notification"),
"bookmarks tab no longer has bubble"
);
assert.ok(markRead, "mark-read request is sent");
assert.strictEqual(
markReadRequestBody,
"dismiss_types=bookmark_reminder",
"mark-read request specifies bookmark_reminder types"
);
assert.notOk(exists(".user-menu .notifications-dismiss"));
});
test("doesn't show confirmation modal for the likes notifications list", async function (assert) {
await visit("/"); await visit("/");
await click(".d-header-icons .current-user"); await click(".d-header-icons .current-user");

View File

@ -0,0 +1,66 @@
export default {
"/u/:username/user-menu-bookmarks": {
notifications: [
{
id: 1713,
user_id: 1,
notification_type: 24,
read: false,
high_priority: true,
created_at: "2022-08-05T17:27:24.873Z",
post_number: 1,
topic_id: 249,
fancy_title: "Test event hello world!",
slug: "test-event-hello-world",
data: {
title: "Test event hello world!",
bookmarkable_url: "/t/test-event-hello-world/249/1",
display_username: "osama",
bookmark_name: "",
bookmark_id: 11,
},
},
],
bookmarks: [
{
id: 6,
created_at: "2022-08-05T06:09:39.559Z",
updated_at: "2022-08-05T06:11:27.246Z",
name: "",
reminder_at: "2022-08-05T06:10:42.223Z",
reminder_at_ics_start: "20220805T061042Z",
reminder_at_ics_end: "20220805T071042Z",
pinned: false,
title: "Test poll topic hello world",
fancy_title: "Test poll topic hello world",
excerpt: "poll",
bookmarkable_id: 1009,
bookmarkable_type: "Post",
bookmarkable_url:
"http://localhost:4200/t/test-poll-topic-hello-world/227/1",
tags: [],
tags_descriptions: {},
truncated: true,
topic_id: 227,
linked_post_number: 1,
deleted: false,
hidden: false,
category_id: 1,
closed: false,
archived: false,
archetype: "regular",
highest_post_number: 1,
last_read_post_number: 1,
bumped_at: "2022-04-21T15:14:37.359Z",
slug: "test-poll-topic-hello-world",
user: {
id: 1,
username: "osama",
name: "Osama",
avatar_template:
"/letter_avatar_proxy/v4/letter/o/f05b48/{size}.png",
},
},
],
}
}

View File

@ -0,0 +1,87 @@
import { module, test } from "qunit";
import { setupRenderingTest } from "discourse/tests/helpers/component-test";
import { query } from "discourse/tests/helpers/qunit-helpers";
import { render } from "@ember/test-helpers";
import { deepMerge } from "discourse-common/lib/object";
import Notification from "discourse/models/notification";
import { hbs } from "ember-cli-htmlbars";
function getBookmark(overrides = {}) {
return Notification.create(
deepMerge(
{
id: 6,
created_at: "2022-08-05T06:09:39.559Z",
updated_at: "2022-08-05T06:11:27.246Z",
name: "",
reminder_at: "2022-08-05T06:10:42.223Z",
reminder_at_ics_start: "20220805T061042Z",
reminder_at_ics_end: "20220805T071042Z",
pinned: false,
title: "Test poll topic hello world",
fancy_title: "Test poll topic hello world",
excerpt: "poll",
bookmarkable_id: 1009,
bookmarkable_type: "Post",
bookmarkable_url: "http://localhost:4200/t/this-bookmarkable-url/227/1",
tags: [],
tags_descriptions: {},
truncated: true,
topic_id: 227,
linked_post_number: 1,
deleted: false,
hidden: false,
category_id: 1,
closed: false,
archived: false,
archetype: "regular",
highest_post_number: 45,
last_read_post_number: 31,
bumped_at: "2022-04-21T15:14:37.359Z",
slug: "test-poll-topic-hello-world",
user: {
id: 1,
username: "somebody",
name: "Mr. Somebody",
avatar_template: "/letter_avatar_proxy/v4/letter/o/f05b48/{size}.png",
},
},
overrides
)
);
}
module("Integration | Component | user-menu | bookmark-item", function (hooks) {
setupRenderingTest(hooks);
const template = hbs`<UserMenu::BookmarkItem @item={{this.bookmark}}/>`;
test("uses bookmarkable_url for the href", async function (assert) {
this.set("bookmark", getBookmark());
await render(template);
assert.ok(
query("li.bookmark a").href.endsWith("/t/this-bookmarkable-url/227/1")
);
});
test("item label is the bookmarked post author", async function (assert) {
this.set(
"bookmark",
getBookmark({ user: { username: "bookmarkPostAuthor" } })
);
await render(template);
assert.strictEqual(
query("li.bookmark .item-label").textContent.trim(),
"bookmarkPostAuthor"
);
});
test("item description is the bookmark title", async function (assert) {
this.set("bookmark", getBookmark({ title: "Custom bookmark title" }));
await render(template);
assert.strictEqual(
query("li.bookmark .item-description").textContent.trim(),
"Custom bookmark title"
);
});
});

View File

@ -0,0 +1,90 @@
import { module, test } from "qunit";
import { setupRenderingTest } from "discourse/tests/helpers/component-test";
import { exists, query, queryAll } from "discourse/tests/helpers/qunit-helpers";
import { render, settled } from "@ember/test-helpers";
import { NOTIFICATION_TYPES } from "discourse/tests/fixtures/concerns/notification-types";
import { hbs } from "ember-cli-htmlbars";
import pretender, { response } from "discourse/tests/helpers/create-pretender";
import I18n from "I18n";
module(
"Integration | Component | user-menu | bookmarks-list",
function (hooks) {
setupRenderingTest(hooks);
const template = hbs`<UserMenu::BookmarksList/>`;
test("renders notifications on top and bookmarks on bottom", async function (assert) {
await render(template);
const items = queryAll("ul li");
assert.strictEqual(items.length, 2);
assert.ok(items[0].classList.contains("notification"));
assert.ok(items[0].classList.contains("unread"));
assert.ok(items[0].classList.contains("bookmark-reminder"));
assert.ok(items[1].classList.contains("bookmark"));
});
test("show all link", async function (assert) {
await render(template);
const link = query(".panel-body-bottom .show-all");
assert.ok(
link.href.endsWith("/u/eviltrout/activity/bookmarks"),
"links to the bookmarks page"
);
assert.strictEqual(
link.title,
I18n.t("user_menu.view_all_bookmarks"),
"has a title"
);
});
test("dismiss button", async function (assert) {
this.currentUser.set("grouped_unread_high_priority_notifications", {
[NOTIFICATION_TYPES.bookmark_reminder]: 72,
});
await render(template);
const dismiss = query(".panel-body-bottom .notifications-dismiss");
assert.ok(
dismiss,
"dismiss button is shown if the user has unread bookmark_reminder notifications"
);
assert.strictEqual(
dismiss.title,
I18n.t("user.dismiss_bookmarks_tooltip"),
"dismiss button has a title"
);
this.currentUser.set("grouped_unread_high_priority_notifications", {});
await settled();
assert.notOk(
exists(".panel-body-bottom .notifications-dismiss"),
"dismiss button is not shown if the user no unread bookmark_reminder notifications"
);
});
test("empty state (aka blank page syndrome)", async function (assert) {
pretender.get("/u/eviltrout/user-menu-bookmarks", () => {
return response({ notifications: [], bookmarks: [] });
});
await render(template);
assert.strictEqual(
query(".empty-state-title").textContent.trim(),
I18n.t("user.no_bookmarks_title"),
"empty state title is shown"
);
assert.strictEqual(
query(".empty-state-body").textContent.trim(),
I18n.t("user.no_bookmarks_body", { icon: "" }).trim(),
"empty state body is shown"
);
assert.ok(
exists(".empty-state-body svg.d-icon-bookmark"),
"icon is correctly rendered in the empty state body"
);
});
}
);

View File

@ -16,9 +16,9 @@ module("Integration | Component | user-menu", function (hooks) {
const activeTab = query(".top-tabs.tabs-list .btn.active"); const activeTab = query(".top-tabs.tabs-list .btn.active");
assert.strictEqual(activeTab.id, "user-menu-button-all-notifications"); assert.strictEqual(activeTab.id, "user-menu-button-all-notifications");
const notifications = queryAll("#quick-access-all-notifications ul li"); const notifications = queryAll("#quick-access-all-notifications ul li");
assert.strictEqual(notifications[0].className, "edited"); assert.ok(notifications[0].classList.contains("edited"));
assert.strictEqual(notifications[1].className, "replied"); assert.ok(notifications[1].classList.contains("replied"));
assert.strictEqual(notifications[2].className, "liked-consolidated"); assert.ok(notifications[2].classList.contains("liked-consolidated"));
}); });
test("notifications panel has a11y attributes", async function (assert) { test("notifications panel has a11y attributes", async function (assert) {
@ -48,7 +48,7 @@ module("Integration | Component | user-menu", function (hooks) {
test("the menu has a group of tabs at the top", async function (assert) { test("the menu has a group of tabs at the top", async function (assert) {
await render(template); await render(template);
const tabs = queryAll(".top-tabs.tabs-list .btn"); const tabs = queryAll(".top-tabs.tabs-list .btn");
assert.strictEqual(tabs.length, 4); assert.strictEqual(tabs.length, 5);
["all-notifications", "replies", "mentions", "likes"].forEach( ["all-notifications", "replies", "mentions", "likes"].forEach(
(tab, index) => { (tab, index) => {
assert.strictEqual(tabs[index].id, `user-menu-button-${tab}`); assert.strictEqual(tabs[index].id, `user-menu-button-${tab}`);
@ -67,7 +67,7 @@ module("Integration | Component | user-menu", function (hooks) {
assert.strictEqual(tabs.length, 1); assert.strictEqual(tabs.length, 1);
const preferencesTab = tabs[0]; const preferencesTab = tabs[0];
assert.ok(preferencesTab.href.endsWith("/u/eviltrout/preferences")); assert.ok(preferencesTab.href.endsWith("/u/eviltrout/preferences"));
assert.strictEqual(preferencesTab.dataset.tabNumber, "4"); assert.strictEqual(preferencesTab.dataset.tabNumber, "5");
assert.strictEqual(preferencesTab.getAttribute("tabindex"), "-1"); assert.strictEqual(preferencesTab.getAttribute("tabindex"), "-1");
}); });
@ -77,11 +77,11 @@ module("Integration | Component | user-menu", function (hooks) {
assert.ok(!exists("#user-menu-button-likes")); assert.ok(!exists("#user-menu-button-likes"));
const tabs = Array.from(queryAll(".tabs-list .btn")); // top and bottom tabs const tabs = Array.from(queryAll(".tabs-list .btn")); // top and bottom tabs
assert.strictEqual(tabs.length, 4); assert.strictEqual(tabs.length, 5);
assert.deepEqual( assert.deepEqual(
tabs.map((t) => t.dataset.tabNumber), tabs.map((t) => t.dataset.tabNumber),
["0", "1", "2", "3"], ["0", "1", "2", "3", "4"],
"data-tab-number of the tabs has no gaps when the likes tab is hidden" "data-tab-number of the tabs has no gaps when the likes tab is hidden"
); );
}); });
@ -90,14 +90,14 @@ module("Integration | Component | user-menu", function (hooks) {
this.currentUser.set("can_review", true); this.currentUser.set("can_review", true);
await render(template); await render(template);
const tab = query("#user-menu-button-review-queue"); const tab = query("#user-menu-button-review-queue");
assert.strictEqual(tab.dataset.tabNumber, "4"); assert.strictEqual(tab.dataset.tabNumber, "5");
const tabs = Array.from(queryAll(".tabs-list .btn")); // top and bottom tabs const tabs = Array.from(queryAll(".tabs-list .btn")); // top and bottom tabs
assert.strictEqual(tabs.length, 6); assert.strictEqual(tabs.length, 7);
assert.deepEqual( assert.deepEqual(
tabs.map((t) => t.dataset.tabNumber), tabs.map((t) => t.dataset.tabNumber),
["0", "1", "2", "3", "4", "5"], ["0", "1", "2", "3", "4", "5", "6"],
"data-tab-number of the tabs has no gaps when the reviewables tab is show" "data-tab-number of the tabs has no gaps when the reviewables tab is show"
); );
}); });

View File

@ -43,12 +43,12 @@ module(
const template = hbs`<UserMenu::NotificationItem @item={{this.notification}}/>`; const template = hbs`<UserMenu::NotificationItem @item={{this.notification}}/>`;
test("pushes `read` to the classList if the notification is read", async function (assert) { test("pushes `read` to the classList if the notification is read and `unread` if it isn't", async function (assert) {
this.set("notification", getNotification()); this.set("notification", getNotification());
this.notification.read = false; this.notification.read = false;
await render(template); await render(template);
assert.ok(!exists("li.read")); assert.notOk(exists("li.read"));
assert.ok(exists("li")); assert.ok(exists("li.unread"));
this.notification.read = true; this.notification.read = true;
await settled(); await settled();
@ -57,13 +57,17 @@ module(
exists("li.read"), exists("li.read"),
"the item re-renders when the read property is updated" "the item re-renders when the read property is updated"
); );
assert.notOk(
exists("li.unread"),
"the item re-renders when the read property is updated"
);
}); });
test("pushes the notification type name to the classList", async function (assert) { test("pushes the notification type name to the classList", async function (assert) {
this.set("notification", getNotification()); this.set("notification", getNotification());
await render(template); await render(template);
let item = query("li"); let item = query("li");
assert.strictEqual(item.className, "mentioned"); assert.ok(item.classList.contains("mentioned"));
this.set( this.set(
"notification", "notification",
@ -128,8 +132,8 @@ module(
test("has elements for label and description", async function (assert) { test("has elements for label and description", async function (assert) {
this.set("notification", getNotification()); this.set("notification", getNotification());
await render(template); await render(template);
const label = query("li a .notification-label"); const label = query("li a .item-label");
const description = query("li a .notification-description"); const description = query("li a .item-description");
assert.strictEqual( assert.strictEqual(
label.textContent.trim(), label.textContent.trim(),
@ -152,7 +156,7 @@ module(
}) })
); );
await render(template); await render(template);
const description = query("li a .notification-description"); const description = query("li a .item-description");
assert.strictEqual( assert.strictEqual(
description.textContent.trim(), description.textContent.trim(),
@ -170,7 +174,7 @@ module(
); );
await render(template); await render(template);
assert.ok( assert.ok(
exists("li a .notification-description img.emoji"), exists("li a .item-description img.emoji"),
"emojis are unescaped when fancy_title is used for description" "emojis are unescaped when fancy_title is used for description"
); );
}); });
@ -186,7 +190,7 @@ module(
}) })
); );
await render(template); await render(template);
const description = query("li a .notification-description"); const description = query("li a .item-description");
assert.strictEqual( assert.strictEqual(
description.textContent.trim(), description.textContent.trim(),
@ -226,11 +230,11 @@ module(
return "notification description 123 <script>"; return "notification description 123 <script>";
} }
get labelWrapperClasses() { get labelClasses() {
return ["label-wrapper-1"]; return ["label-wrapper-1"];
} }
get descriptionWrapperClasses() { get descriptionClasses() {
return ["description-class-1"]; return ["description-class-1"];
} }
}; };
@ -265,7 +269,7 @@ module(
assert.ok(exists("svg.d-icon-wrench"), "icon is customized"); assert.ok(exists("svg.d-icon-wrench"), "icon is customized");
const label = query("li .notification-label"); const label = query("li .item-label");
assert.ok( assert.ok(
label.classList.contains("label-wrapper-1"), label.classList.contains("label-wrapper-1"),
"label wrapper has additional classes" "label wrapper has additional classes"
@ -276,7 +280,7 @@ module(
"label content is customized" "label content is customized"
); );
const description = query(".notification-description"); const description = query(".item-description");
assert.ok( assert.ok(
description.classList.contains("description-class-1"), description.classList.contains("description-class-1"),
"description has additional classes" "description has additional classes"
@ -314,10 +318,7 @@ module(
); );
await render(template); await render(template);
assert.notOk( assert.notOk(exists(".item-description"), "description is not rendered");
exists(".notification-description"),
"description is not rendered"
);
assert.ok( assert.ok(
query("li").textContent.trim(), query("li").textContent.trim(),
"notification label", "notification label",
@ -356,7 +357,7 @@ module(
"notification description", "notification description",
"only notification description is displayed" "only notification description is displayed"
); );
assert.notOk(exists(".notification-label"), "label is not rendered"); assert.notOk(exists(".item-label"), "label is not rendered");
}); });
test("custom click handlers", async function (assert) { test("custom click handlers", async function (assert) {

View File

@ -46,8 +46,8 @@ module(
this.set("item", getReviewable()); this.set("item", getReviewable());
await render(template); await render(template);
const label = query("li .reviewable-label"); const label = query("li .item-label");
const description = query("li .reviewable-description"); const description = query("li .item-description");
assert.strictEqual( assert.strictEqual(
label.textContent.trim(), label.textContent.trim(),
"sayo2", "sayo2",
@ -65,7 +65,7 @@ module(
test("the item's label is a placeholder that indicates deleted user if flagger_username is absent", async function (assert) { test("the item's label is a placeholder that indicates deleted user if flagger_username is absent", async function (assert) {
this.set("item", getReviewable({ flagger_username: null })); this.set("item", getReviewable({ flagger_username: null }));
await render(template); await render(template);
const label = query("li .reviewable-label"); const label = query("li .item-label");
assert.strictEqual( assert.strictEqual(
label.textContent.trim(), label.textContent.trim(),
I18n.t("user_menu.reviewable.deleted_user") I18n.t("user_menu.reviewable.deleted_user")

View File

@ -5,6 +5,7 @@ import sinon from "sinon";
import EmberObject from "@ember/object"; import EmberObject from "@ember/object";
import User from "discourse/models/user"; import User from "discourse/models/user";
import pretender from "discourse/tests/helpers/create-pretender"; import pretender from "discourse/tests/helpers/create-pretender";
import I18n from "I18n";
discourseModule("Unit | Controller | user-notifications", function () { discourseModule("Unit | Controller | user-notifications", function () {
test("Mark read marks all models read when response is 200", async function (assert) { test("Mark read marks all models read when response is 200", async function (assert) {
@ -78,7 +79,10 @@ discourseModule("Unit | Controller | user-notifications", function () {
controller.send("resetNew"); controller.send("resetNew");
assert.strictEqual(capturedProperties.count, 1); assert.strictEqual(
capturedProperties.confirmationMessage,
I18n.t("notifications.dismiss_confirmation.body.default", { count: 1 })
);
capturedProperties.dismissNotifications(); capturedProperties.dismissNotifications();
assert.strictEqual(markReadFake.callCount, 1); assert.strictEqual(markReadFake.callCount, 1);
}); });

View File

@ -152,9 +152,31 @@
white-space: unset; white-space: unset;
} }
.notification-label { .item-label {
color: var(--primary); color: var(--primary);
} }
li {
background-color: var(--secondary);
&.unread,
&.pending {
background-color: var(--tertiary-low);
}
&:hover {
background-color: var(--highlight-medium);
outline: none;
}
&:focus-within {
background: var(--highlight-medium);
a {
// we don't need the link focus because we're styling the parent
outline: 0;
}
}
}
} }
} }
@ -426,8 +448,7 @@
color: var(--danger); color: var(--danger);
} }
} }
.read, .read {
.reviewed {
background-color: var(--secondary); background-color: var(--secondary);
} }
.none { .none {

View File

@ -12,14 +12,14 @@ class UsersController < ApplicationController
:notification_level, :revoke_auth_token, :register_second_factor_security_key, :notification_level, :revoke_auth_token, :register_second_factor_security_key,
:create_second_factor_security_key, :feature_topic, :clear_featured_topic, :create_second_factor_security_key, :feature_topic, :clear_featured_topic,
:bookmarks, :invited, :check_sso_email, :check_sso_payload, :bookmarks, :invited, :check_sso_email, :check_sso_payload,
:recent_searches, :reset_recent_searches :recent_searches, :reset_recent_searches, :user_menu_bookmarks
] ]
skip_before_action :check_xhr, only: [ skip_before_action :check_xhr, only: [
:show, :badges, :password_reset_show, :password_reset_update, :update, :account_created, :show, :badges, :password_reset_show, :password_reset_update, :update, :account_created,
:activate_account, :perform_account_activation, :avatar, :activate_account, :perform_account_activation, :avatar,
:my_redirect, :toggle_anon, :admin_login, :confirm_admin, :email_login, :summary, :my_redirect, :toggle_anon, :admin_login, :confirm_admin, :email_login, :summary,
:feature_topic, :clear_featured_topic, :bookmarks :feature_topic, :clear_featured_topic, :bookmarks, :user_menu_bookmarks
] ]
before_action :second_factor_check_confirmed_password, only: [ before_action :second_factor_check_confirmed_password, only: [
@ -1740,6 +1740,62 @@ class UsersController < ApplicationController
end end
end end
USER_MENU_BOOKMARKS_LIST_LIMIT = 20
def user_menu_bookmarks
if !current_user.username_equals_to?(params[:username])
raise Discourse::InvalidAccess.new("username doesn't match current_user's username")
end
reminder_notifications = current_user
.notifications
.visible
.includes(:topic)
.where(read: false, notification_type: Notification.types[:bookmark_reminder])
.limit(USER_MENU_BOOKMARKS_LIST_LIMIT)
.to_a
if reminder_notifications.size < USER_MENU_BOOKMARKS_LIST_LIMIT
exclude_bookmark_ids = reminder_notifications
.filter_map { |notification| notification.data_hash[:bookmark_id] }
bookmark_list = UserBookmarkList.new(
user: current_user,
guardian: guardian,
params: {
per_page: USER_MENU_BOOKMARKS_LIST_LIMIT - reminder_notifications.size
}
)
bookmark_list.load do |query|
if exclude_bookmark_ids.present?
query.where("bookmarks.id NOT IN (?)", exclude_bookmark_ids)
end
end
end
if reminder_notifications.present?
serialized_notifications = ActiveModel::ArraySerializer.new(
reminder_notifications,
each_serializer: NotificationSerializer,
scope: guardian
)
end
if bookmark_list
bookmark_list.bookmark_serializer_opts = { link_to_first_unread_post: true }
serialized_bookmarks = serialize_data(
bookmark_list,
UserBookmarkListSerializer,
scope: guardian,
root: false
)[:bookmarks]
end
render json: {
notifications: serialized_notifications || [],
bookmarks: serialized_bookmarks || []
}
end
private private
def clean_custom_field_values(field) def clean_custom_field_values(field)

View File

@ -538,6 +538,25 @@ class User < ActiveRecord::Base
DB.query_single(sql, user_id: id, high_priority: high_priority)[0].to_i DB.query_single(sql, user_id: id, high_priority: high_priority)[0].to_i
end end
MAX_UNREAD_HIGH_PRI_BACKLOG = 200
def grouped_unread_high_priority_notifications
results = DB.query(<<~SQL, user_id: self.id, limit: MAX_UNREAD_HIGH_PRI_BACKLOG)
SELECT X.notification_type AS type, COUNT(*) FROM (
SELECT n.notification_type
FROM notifications n
LEFT JOIN topics t ON t.id = n.topic_id
WHERE t.deleted_at IS NULL
AND n.high_priority
AND n.user_id = :user_id
AND NOT n.read
LIMIT :limit
) AS X
GROUP BY X.notification_type
SQL
results.map! { |row| [row.type, row.count] }
results.to_h
end
### ###
# DEPRECATED: This is only maintained for backwards compat until v2.5. There # DEPRECATED: This is only maintained for backwards compat until v2.5. There
# may be inconsistencies with counts in the UI because of this, because unread # may be inconsistencies with counts in the UI because of this, because unread
@ -711,6 +730,7 @@ class User < ActiveRecord::Base
if self.redesigned_user_menu_enabled? if self.redesigned_user_menu_enabled?
payload[:all_unread_notifications_count] = all_unread_notifications_count payload[:all_unread_notifications_count] = all_unread_notifications_count
payload[:grouped_unread_high_priority_notifications] = grouped_unread_high_priority_notifications
end end
MessageBus.publish("/notification/#{id}", payload, user_ids: [id]) MessageBus.publish("/notification/#{id}", payload, user_ids: [id])

View File

@ -6,7 +6,7 @@ class UserBookmarkList
PER_PAGE = 20 PER_PAGE = 20
attr_reader :bookmarks, :per_page attr_reader :bookmarks, :per_page
attr_accessor :more_bookmarks_url attr_accessor :more_bookmarks_url, :bookmark_serializer_opts
def initialize(user:, guardian:, params:) def initialize(user:, guardian:, params:)
@user = user @user = user
@ -17,10 +17,11 @@ class UserBookmarkList
@params[:per_page] = PER_PAGE if @params[:per_page] > PER_PAGE @params[:per_page] = PER_PAGE if @params[:per_page] > PER_PAGE
@bookmarks = [] @bookmarks = []
@bookmark_serializer_opts = {}
end end
def load def load(&blk)
@bookmarks = BookmarkQuery.new(user: @user, guardian: @guardian, params: @params).list_all @bookmarks = BookmarkQuery.new(user: @user, guardian: @guardian, params: @params).list_all(&blk)
@bookmarks @bookmarks
end end

View File

@ -77,6 +77,7 @@ class CurrentUserSerializer < BasicUserSerializer
:sidebar_category_ids, :sidebar_category_ids,
:sidebar_tag_names, :sidebar_tag_names,
:likes_notifications_disabled, :likes_notifications_disabled,
:grouped_unread_high_priority_notifications,
:redesigned_user_menu_enabled :redesigned_user_menu_enabled
delegate :user_stat, to: :object, private: true delegate :user_stat, to: :object, private: true
@ -345,6 +346,10 @@ class CurrentUserSerializer < BasicUserSerializer
redesigned_user_menu_enabled redesigned_user_menu_enabled
end end
def include_grouped_unread_high_priority_notifications?
redesigned_user_menu_enabled
end
def include_unseen_reviewable_count? def include_unseen_reviewable_count?
redesigned_user_menu_enabled redesigned_user_menu_enabled
end end

View File

@ -5,7 +5,12 @@ class UserBookmarkListSerializer < ApplicationSerializer
def bookmarks def bookmarks
object.bookmarks.map do |bm| object.bookmarks.map do |bm|
bm.registered_bookmarkable.serializer.new(bm, scope: scope, root: false) bm.registered_bookmarkable.serializer.new(
bm,
**object.bookmark_serializer_opts,
scope: scope,
root: false
)
end end
end end

View File

@ -56,7 +56,11 @@ class UserTopicBookmarkSerializer < UserPostTopicBookmarkBaseSerializer
# NOTE: In the UI there are special topic-status and topic-link components to # NOTE: In the UI there are special topic-status and topic-link components to
# display the topic URL, this is only used for certain routes like the .ics bookmarks. # display the topic URL, this is only used for certain routes like the .ics bookmarks.
def bookmarkable_url def bookmarkable_url
topic.url if @options[:link_to_first_unread_post]
Topic.url(topic_id, slug, (last_read_post_number || 0) + 1)
else
topic.url
end
end end
private private

View File

@ -1121,6 +1121,7 @@ en:
dismiss: "Dismiss" dismiss: "Dismiss"
dismiss_notifications: "Dismiss All" dismiss_notifications: "Dismiss All"
dismiss_notifications_tooltip: "Mark all unread notifications as read" dismiss_notifications_tooltip: "Mark all unread notifications as read"
dismiss_bookmarks_tooltip: "Mark all unread bookmark reminders as read"
no_messages_title: "You dont have any messages" no_messages_title: "You dont have any messages"
no_messages_body: > no_messages_body: >
Need to have a direct personal conversation with someone, outside the normal conversational flow? Message them by selecting their avatar and using the %{icon} message button.<br><br> Need to have a direct personal conversation with someone, outside the normal conversational flow? Message them by selecting their avatar and using the %{icon} message button.<br><br>
@ -2363,8 +2364,12 @@ en:
votes_released: "%{description} - completed" votes_released: "%{description} - completed"
dismiss_confirmation: dismiss_confirmation:
body: body:
one: "Are you sure? You have %{count} important notification." default:
other: "Are you sure? You have %{count} important notifications." one: "Are you sure? You have %{count} important notification."
other: "Are you sure? You have %{count} important notifications."
bookmarks:
one: "Are you sure? You have %{count} unread bookmark reminder."
other: "Are you sure? You have %{count} unread bookmark reminders."
dismiss: "Dismiss" dismiss: "Dismiss"
cancel: "Cancel" cancel: "Cancel"
@ -2557,6 +2562,7 @@ en:
generic_no_items: "There are no items in this list." generic_no_items: "There are no items in this list."
sr_menu_tabs: "Menu tabs" sr_menu_tabs: "Menu tabs"
view_all_notifications: "view all notifications" view_all_notifications: "view all notifications"
view_all_bookmarks: "view all bookmarks"
reviewable: reviewable:
view_all: "view all review items" view_all: "view all review items"
queue: "Queue" queue: "Queue"

View File

@ -519,6 +519,7 @@ Discourse::Application.routes.draw do
get "#{root_path}/:username/activity/:filter" => "users#show", constraints: { username: RouteFormat.username } get "#{root_path}/:username/activity/:filter" => "users#show", constraints: { username: RouteFormat.username }
get "#{root_path}/:username/badges" => "users#badges", constraints: { username: RouteFormat.username } get "#{root_path}/:username/badges" => "users#badges", constraints: { username: RouteFormat.username }
get "#{root_path}/:username/bookmarks" => "users#bookmarks", constraints: { username: RouteFormat.username, format: /(json|ics)/ } get "#{root_path}/:username/bookmarks" => "users#bookmarks", constraints: { username: RouteFormat.username, format: /(json|ics)/ }
get "#{root_path}/:username/user-menu-bookmarks" => "users#user_menu_bookmarks", constraints: { username: RouteFormat.username, format: :json }
get "#{root_path}/:username/notifications" => "users#show", constraints: { username: RouteFormat.username } get "#{root_path}/:username/notifications" => "users#show", constraints: { username: RouteFormat.username }
get "#{root_path}/:username/notifications/:filter" => "users#show", constraints: { username: RouteFormat.username } get "#{root_path}/:username/notifications/:filter" => "users#show", constraints: { username: RouteFormat.username }
delete "#{root_path}/:username" => "users#destroy", constraints: { username: RouteFormat.username } delete "#{root_path}/:username" => "users#destroy", constraints: { username: RouteFormat.username }

View File

@ -34,7 +34,7 @@ class BookmarkQuery
@limit = @params[:limit].present? ? @params[:limit].to_i : @params[:per_page] @limit = @params[:limit].present? ? @params[:limit].to_i : @params[:per_page]
end end
def list_all def list_all(&blk)
search_term = @params[:q] search_term = @params[:q]
ts_query = search_term.present? ? Search.ts_query(term: search_term) : nil ts_query = search_term.present? ? Search.ts_query(term: search_term) : nil
search_term_wildcard = search_term.present? ? "%#{search_term}%" : nil search_term_wildcard = search_term.present? ? "%#{search_term}%" : nil
@ -75,6 +75,10 @@ class BookmarkQuery
results = results.offset(@page * @params[:per_page]) results = results.offset(@page * @params[:per_page])
end end
if updated_results = blk&.call(results)
results = updated_results
end
results = results.limit(@limit).to_a results = results.limit(@limit).to_a
BookmarkQuery.preload(results, self) BookmarkQuery.preload(results, self)
results results

View File

@ -2059,7 +2059,7 @@ RSpec.describe User do
end end
context "with redesigned_user_menu_enabled on" do context "with redesigned_user_menu_enabled on" do
it "adds all_unread_notifications_count to the payload" do it "adds all_unread_notifications and grouped_unread_high_priority_notifications to the payload" do
user.update!(admin: true) user.update!(admin: true)
user.enable_redesigned_user_menu user.enable_redesigned_user_menu
Fabricate(:notification, user: user) Fabricate(:notification, user: user)
@ -2071,6 +2071,7 @@ RSpec.describe User do
message = messages.first message = messages.first
expect(message.data[:all_unread_notifications_count]).to eq(2) expect(message.data[:all_unread_notifications_count]).to eq(2)
expect(message.data[:grouped_unread_high_priority_notifications]).to eq({ 15 => 1 })
ensure ensure
user.disable_redesigned_user_menu user.disable_redesigned_user_menu
end end
@ -2799,6 +2800,28 @@ RSpec.describe User do
end end
end end
describe "#grouped_unread_high_priority_notifications" do
it "returns a map of high priority types to their unread count" do
Fabricate(:notification, user: user, notification_type: 1, high_priority: true, read: true)
Fabricate(:notification, user: user, notification_type: 1, high_priority: true, read: false)
Fabricate(:notification, user: user, notification_type: 1, high_priority: false, read: true)
Fabricate(:notification, user: user, notification_type: 1, high_priority: false, read: false)
Fabricate(:notification, user: user, notification_type: 2, high_priority: true, read: false, topic: nil)
Fabricate(:notification, user: user, notification_type: 3, high_priority: true, read: false).tap do |n|
n.topic.trash!(Fabricate(:admin))
end
Fabricate(:notification, user: user, notification_type: 3, high_priority: false, read: true)
# notification for another user. it shouldn't be included
Fabricate(:notification, notification_type: 4, high_priority: true, read: false)
expect(user.grouped_unread_high_priority_notifications).to eq({ 1 => 1, 2 => 1 })
end
end
describe "#all_unread_notifications_count" do describe "#all_unread_notifications_count" do
it "returns count of unseen and unread high priority and normal priority notifications" do it "returns count of unseen and unread high priority and normal priority notifications" do
Fabricate(:notification, user: user, high_priority: true, read: false) Fabricate(:notification, user: user, high_priority: true, read: false)

View File

@ -5559,6 +5559,146 @@ RSpec.describe UsersController do
end end
end end
describe "#user_menu_bookmarks" do
fab!(:post) { Fabricate(:post) }
fab!(:topic) { Fabricate(:post).topic }
fab!(:bookmark_with_reminder) { Fabricate(:bookmark, user: user, bookmarkable: post) }
fab!(:bookmark_without_reminder) { Fabricate(:bookmark, user: user, bookmarkable: topic) }
before do
TopicUser.change(user.id, post.topic.id, total_msecs_viewed: 1)
TopicUser.change(user.id, topic.id, total_msecs_viewed: 1)
BookmarkReminderNotificationHandler
.new(bookmark_with_reminder)
.send_notification
end
context "when logged out" do
it "responds with 404" do
get "/u/#{user.username}/user-menu-bookmarks"
expect(response.status).to eq(404)
end
end
context "when logged in" do
before do
sign_in(user)
end
it "responds with 403 when requesting bookmarks list of another user" do
get "/u/#{user1.username}/user-menu-bookmarks"
expect(response.status).to eq(403)
end
it "sends an array of unread bookmark_reminder notifications" do
bookmark_with_reminder2 = Fabricate(:bookmark, user: user, bookmarkable: Fabricate(:post))
TopicUser.change(user.id, bookmark_with_reminder2.bookmarkable.topic, total_msecs_viewed: 1)
BookmarkReminderNotificationHandler
.new(bookmark_with_reminder2)
.send_notification
user
.notifications
.where(notification_type: Notification.types[:bookmark_reminder])
.where("data::json ->> 'bookmark_id' = ?", bookmark_with_reminder2.id.to_s)
.first
.update!(read: true)
get "/u/#{user.username}/user-menu-bookmarks"
expect(response.status).to eq(200)
notifications = response.parsed_body["notifications"]
expect(notifications.size).to eq(1)
expect(notifications.first["data"]["bookmark_id"]).to eq(bookmark_with_reminder.id)
end
it "responds with an array of bookmarks that are not associated with any of the unread bookmark_reminder notifications" do
get "/u/#{user.username}/user-menu-bookmarks"
expect(response.status).to eq(200)
bookmarks = response.parsed_body["bookmarks"]
expect(bookmarks.size).to eq(1)
expect(bookmarks.first["id"]).to eq(bookmark_without_reminder.id)
bookmark_reminder = user
.notifications
.where(notification_type: Notification.types[:bookmark_reminder])
.where("data::json ->> 'bookmark_id' = ?", bookmark_with_reminder.id.to_s)
.first
bookmark_reminder.update!(read: true)
get "/u/#{user.username}/user-menu-bookmarks"
expect(response.status).to eq(200)
bookmarks = response.parsed_body["bookmarks"]
expect(bookmarks.map { |bookmark| bookmark["id"] }).to contain_exactly(
bookmark_with_reminder.id,
bookmark_without_reminder.id
)
data = bookmark_reminder.data_hash
data.delete(:bookmark_id)
bookmark_reminder.update!(data: data.to_json, read: false)
get "/u/#{user.username}/user-menu-bookmarks"
expect(response.status).to eq(200)
notifications = response.parsed_body["notifications"]
expect(notifications.size).to eq(1)
expect(notifications.first["data"]["bookmark_id"]).to be_nil
bookmarks = response.parsed_body["bookmarks"]
expect(bookmarks.map { |bookmark| bookmark["id"] }).to contain_exactly(
bookmark_with_reminder.id,
bookmark_without_reminder.id
)
end
it "fills up the remaining of the USER_MENU_BOOKMARKS_LIST_LIMIT limit with bookmarks" do
bookmark2 = Fabricate(
:bookmark,
user: user,
bookmarkable: Fabricate(:post, topic: topic)
)
stub_const(UsersController, "USER_MENU_BOOKMARKS_LIST_LIMIT", 2) do
get "/u/#{user.username}/user-menu-bookmarks"
end
expect(response.status).to eq(200)
notifications = response.parsed_body["notifications"]
expect(notifications.size).to eq(1)
bookmarks = response.parsed_body["bookmarks"]
expect(bookmarks.size).to eq(1)
stub_const(UsersController, "USER_MENU_BOOKMARKS_LIST_LIMIT", 3) do
get "/u/#{user.username}/user-menu-bookmarks"
end
expect(response.status).to eq(200)
notifications = response.parsed_body["notifications"]
expect(notifications.size).to eq(1)
bookmarks = response.parsed_body["bookmarks"]
expect(bookmarks.size).to eq(2)
BookmarkReminderNotificationHandler.new(bookmark2).send_notification
stub_const(UsersController, "USER_MENU_BOOKMARKS_LIST_LIMIT", 3) do
get "/u/#{user.username}/user-menu-bookmarks"
end
expect(response.status).to eq(200)
notifications = response.parsed_body["notifications"]
expect(notifications.size).to eq(2)
bookmarks = response.parsed_body["bookmarks"]
expect(bookmarks.size).to eq(1)
end
end
end
def create_second_factor_security_key def create_second_factor_security_key
sign_in(user1) sign_in(user1)
stub_secure_session_confirmed stub_secure_session_confirmed

View File

@ -1,29 +1,32 @@
# frozen_string_literal: true # frozen_string_literal: true
RSpec.describe UserPostBookmarkSerializer do RSpec.describe UserPostBookmarkSerializer do
let(:whisperers_group) { Fabricate(:group) }
let(:user) { Fabricate(:user) } let(:user) { Fabricate(:user) }
let(:post) { Fabricate(:post, user: user, topic: topic) }
let(:topic) { Fabricate(:topic) } let(:topic) { Fabricate(:topic) }
let(:post) { Fabricate(:post, user: user, topic: topic) }
let!(:bookmark) { Fabricate(:bookmark, name: 'Test', user: user, bookmarkable: post) } let!(:bookmark) { Fabricate(:bookmark, name: 'Test', user: user, bookmarkable: post) }
before do describe "#highest_post_number" do
SiteSetting.enable_whispers = true let(:whisperers_group) { Fabricate(:group) }
SiteSetting.whispers_allowed_groups = "#{whisperers_group.id}"
end
it "uses the correct highest_post_number column based on whether the user is whisperer" do before do
Fabricate(:post, topic: topic) SiteSetting.enable_whispers = true
Fabricate(:post, topic: topic) SiteSetting.whispers_allowed_groups = "#{whisperers_group.id}"
Fabricate(:whisper, topic: topic) end
topic.reload
bookmark.reload
serializer = UserPostBookmarkSerializer.new(bookmark, scope: Guardian.new(user))
expect(serializer.highest_post_number).to eq(3) it "uses the correct highest_post_number column based on whether the user is whisperer" do
Fabricate(:post, topic: topic)
Fabricate(:post, topic: topic)
Fabricate(:whisper, topic: topic)
topic.reload
bookmark.reload
serializer = UserPostBookmarkSerializer.new(bookmark, scope: Guardian.new(user))
user.groups << whisperers_group expect(serializer.highest_post_number).to eq(3)
expect(serializer.highest_post_number).to eq(4) user.groups << whisperers_group
expect(serializer.highest_post_number).to eq(4)
end
end end
end end

View File

@ -6,33 +6,55 @@ RSpec.describe UserTopicBookmarkSerializer do
let!(:post) { Fabricate(:post, topic: topic) } let!(:post) { Fabricate(:post, topic: topic) }
let!(:bookmark) { Fabricate(:bookmark, name: 'Test', user: user, bookmarkable: topic) } let!(:bookmark) { Fabricate(:bookmark, name: 'Test', user: user, bookmarkable: topic) }
it "uses the last_read_post_number + 1 for the bookmarks excerpt" do describe "#excerpt" do
next_unread_post = Fabricate(:post_with_long_raw_content, topic: bookmark.bookmarkable) it "uses the last_read_post_number + 1 for the bookmarks excerpt" do
Fabricate(:post_with_external_links, topic: bookmark.bookmarkable) next_unread_post = Fabricate(:post_with_long_raw_content, topic: bookmark.bookmarkable)
bookmark.reload Fabricate(:post_with_external_links, topic: bookmark.bookmarkable)
TopicUser.change(user.id, bookmark.bookmarkable.id, { last_read_post_number: post.post_number }) bookmark.reload
serializer = UserTopicBookmarkSerializer.new(bookmark, scope: Guardian.new(user)) TopicUser.change(user.id, bookmark.bookmarkable.id, { last_read_post_number: post.post_number })
expect(serializer.excerpt).to eq(PrettyText.excerpt(next_unread_post.cooked, 300, keep_emoji_images: true)) serializer = UserTopicBookmarkSerializer.new(bookmark, scope: Guardian.new(user))
expect(serializer.excerpt).to eq(PrettyText.excerpt(next_unread_post.cooked, 300, keep_emoji_images: true))
end
it "does not use a small post for the last unread cooked post" do
small_action_post = Fabricate(:small_action, topic: bookmark.bookmarkable)
next_unread_post = Fabricate(:post_with_long_raw_content, topic: bookmark.bookmarkable)
Fabricate(:post_with_external_links, topic: bookmark.bookmarkable)
bookmark.reload
TopicUser.change(user.id, bookmark.bookmarkable.id, { last_read_post_number: post.post_number })
serializer = UserTopicBookmarkSerializer.new(bookmark, scope: Guardian.new(user))
expect(serializer.excerpt).to eq(PrettyText.excerpt(next_unread_post.cooked, 300, keep_emoji_images: true))
end
it "handles the last read post in the topic being a small post by getting the last read regular post" do
last_regular_post = Fabricate(:post_with_long_raw_content, topic: bookmark.bookmarkable)
small_action_post = Fabricate(:small_action, topic: bookmark.bookmarkable)
bookmark.reload
topic.reload
TopicUser.change(user.id, bookmark.bookmarkable.id, { last_read_post_number: small_action_post.post_number })
serializer = UserTopicBookmarkSerializer.new(bookmark, scope: Guardian.new(user))
expect(serializer.cooked).to eq(last_regular_post.cooked)
expect(serializer.excerpt).to eq(PrettyText.excerpt(last_regular_post.cooked, 300, keep_emoji_images: true))
end
end end
it "does not use a small post for the last unread cooked post" do describe "#bookmarkable_url" do
small_action_post = Fabricate(:small_action, topic: bookmark.bookmarkable) context "with the link_to_first_unread_post option" do
next_unread_post = Fabricate(:post_with_long_raw_content, topic: bookmark.bookmarkable) it "is a full topic URL to the first unread post in the topic when the option is set" do
Fabricate(:post_with_external_links, topic: bookmark.bookmarkable) TopicUser.change(user.id, bookmark.bookmarkable.id, { last_read_post_number: post.post_number })
bookmark.reload serializer = UserTopicBookmarkSerializer.new(
TopicUser.change(user.id, bookmark.bookmarkable.id, { last_read_post_number: post.post_number }) bookmark,
serializer = UserTopicBookmarkSerializer.new(bookmark, scope: Guardian.new(user)) scope: Guardian.new(user),
expect(serializer.excerpt).to eq(PrettyText.excerpt(next_unread_post.cooked, 300, keep_emoji_images: true)) link_to_first_unread_post: true
end )
expect(serializer.bookmarkable_url).to end_with("/t/#{topic.slug}/#{topic.id}/#{post.post_number + 1}")
end
it "handles the last read post in the topic being a small post by getting the last read regular post" do it "is a full topic URL to the first post in the topic when the option isn't set" do
last_regular_post = Fabricate(:post_with_long_raw_content, topic: bookmark.bookmarkable) TopicUser.change(user.id, bookmark.bookmarkable.id, { last_read_post_number: post.post_number })
small_action_post = Fabricate(:small_action, topic: bookmark.bookmarkable) serializer = UserTopicBookmarkSerializer.new(bookmark, scope: Guardian.new(user))
bookmark.reload expect(serializer.bookmarkable_url).to end_with("/t/#{topic.slug}/#{topic.id}")
topic.reload end
TopicUser.change(user.id, bookmark.bookmarkable.id, { last_read_post_number: small_action_post.post_number }) end
serializer = UserTopicBookmarkSerializer.new(bookmark, scope: Guardian.new(user))
expect(serializer.cooked).to eq(last_regular_post.cooked)
expect(serializer.excerpt).to eq(PrettyText.excerpt(last_regular_post.cooked, 300, keep_emoji_images: true))
end end
end end