diff --git a/app/assets/javascripts/discourse/app/components/reviewable-item.js b/app/assets/javascripts/discourse/app/components/reviewable-item.js
index 01488c3ecd2..9843038de4e 100644
--- a/app/assets/javascripts/discourse/app/components/reviewable-item.js
+++ b/app/assets/javascripts/discourse/app/components/reviewable-item.js
@@ -156,6 +156,12 @@ export default Component.extend({
performResult.reviewable_count
);
}
+ if (performResult.unseen_reviewable_count !== undefined) {
+ this.currentUser.set(
+ "unseen_reviewable_count",
+ performResult.unseen_reviewable_count
+ );
+ }
if (this.attrs.remove) {
this.attrs.remove(performResult.remove_reviewable_ids);
diff --git a/app/assets/javascripts/discourse/app/components/site-header.js b/app/assets/javascripts/discourse/app/components/site-header.js
index 71cd70a4a3a..b5a4dc205dc 100644
--- a/app/assets/javascripts/discourse/app/components/site-header.js
+++ b/app/assets/javascripts/discourse/app/components/site-header.js
@@ -30,7 +30,9 @@ const SiteHeaderComponent = MountWidget.extend(
@observes(
"currentUser.unread_notifications",
"currentUser.unread_high_priority_notifications",
- "currentUser.reviewable_count",
+ "currentUser.all_unread_notifications_count",
+ "currentUser.reviewable_count", // TODO: remove this when redesigned_user_menu_enabled is removed
+ "currentUser.unseen_reviewable_count",
"session.defaultColorSchemeIsDark",
"session.darkModeAvailable"
)
diff --git a/app/assets/javascripts/discourse/app/initializers/subscribe-user-notifications.js b/app/assets/javascripts/discourse/app/initializers/subscribe-user-notifications.js
index eed101e32fd..be4d54ed2af 100644
--- a/app/assets/javascripts/discourse/app/initializers/subscribe-user-notifications.js
+++ b/app/assets/javascripts/discourse/app/initializers/subscribe-user-notifications.js
@@ -22,38 +22,49 @@ export default {
const user = container.lookup("service:current-user");
const bus = container.lookup("service:message-bus");
const appEvents = container.lookup("service:app-events");
+ const siteSettings = container.lookup("service:site-settings");
if (user) {
- bus.subscribe("/reviewable_counts", (data) => {
- user.set("reviewable_count", data.reviewable_count);
+ const channel = user.enable_redesigned_user_menu
+ ? `/reviewable_counts/${user.id}`
+ : "/reviewable_counts";
+ bus.subscribe(channel, (data) => {
+ if (data.reviewable_count >= 0) {
+ user.set("reviewable_count", data.reviewable_count);
+ }
+ if (user.redesigned_user_menu_enabled) {
+ user.set("unseen_reviewable_count", data.unseen_reviewable_count);
+ }
});
bus.subscribe(
`/notification/${user.get("id")}`,
(data) => {
const store = container.lookup("service:store");
- const oldUnread = user.get("unread_notifications");
- const oldHighPriority = user.get(
- "unread_high_priority_notifications"
- );
+ const oldUnread = user.unread_notifications;
+ const oldHighPriority = user.unread_high_priority_notifications;
+ const oldAllUnread = user.all_unread_notifications_count;
user.setProperties({
unread_notifications: data.unread_notifications,
unread_high_priority_notifications:
data.unread_high_priority_notifications,
read_first_notification: data.read_first_notification,
+ all_unread_notifications_count: data.all_unread_notifications_count,
});
if (
oldUnread !== data.unread_notifications ||
- oldHighPriority !== data.unread_high_priority_notifications
+ oldHighPriority !== data.unread_high_priority_notifications ||
+ oldAllUnread !== data.all_unread_notifications_count
) {
appEvents.trigger("notifications:changed");
if (
site.mobileView &&
(data.unread_notifications - oldUnread > 0 ||
- data.unread_high_priority_notifications - oldHighPriority > 0)
+ data.unread_high_priority_notifications - oldHighPriority > 0 ||
+ data.all_unread_notifications_count - oldAllUnread > 0)
) {
appEvents.trigger("header:update-topic", null, 5000);
}
@@ -74,9 +85,9 @@ export default {
);
if (staleIndex === -1) {
- // high priority and unread notifications are first
let insertPosition = 0;
+ // high priority and unread notifications are first
if (!lastNotification.high_priority || lastNotification.read) {
const nextPosition = oldNotifications.findIndex(
(n) => !n.high_priority || n.read
@@ -122,7 +133,6 @@ export default {
});
const site = container.lookup("site:main");
- const siteSettings = container.lookup("service:site-settings");
const router = container.lookup("router:main");
bus.subscribe("/categories", (data) => {
diff --git a/app/assets/javascripts/discourse/app/routes/review-index.js b/app/assets/javascripts/discourse/app/routes/review-index.js
index 67c16c6a31b..4d89d8154af 100644
--- a/app/assets/javascripts/discourse/app/routes/review-index.js
+++ b/app/assets/javascripts/discourse/app/routes/review-index.js
@@ -22,6 +22,12 @@ export default DiscourseRoute.extend({
if (meta.reviewable_count !== undefined) {
this.currentUser.set("reviewable_count", meta.reviewable_count);
}
+ if (meta.unseen_reviewable_count !== undefined) {
+ this.currentUser.set(
+ "unseen_reviewable_count",
+ meta.unseen_reviewable_count
+ );
+ }
controller.setProperties({
reviewables: model,
@@ -59,7 +65,10 @@ export default DiscourseRoute.extend({
}
});
- this.messageBus.subscribe("/reviewable_counts", (data) => {
+ const channel = this.currentUser.enable_redesigned_user_menu
+ ? `/reviewable_counts/${this.currentUser.id}`
+ : "/reviewable_counts";
+ this.messageBus.subscribe(channel, (data) => {
if (data.updates) {
this.controller.reviewables.forEach((reviewable) => {
const updates = data.updates[reviewable.id];
diff --git a/app/assets/javascripts/discourse/app/widgets/hamburger-menu.js b/app/assets/javascripts/discourse/app/widgets/hamburger-menu.js
index dcdfb2d646f..463c5bf5d12 100644
--- a/app/assets/javascripts/discourse/app/widgets/hamburger-menu.js
+++ b/app/assets/javascripts/discourse/app/widgets/hamburger-menu.js
@@ -355,7 +355,11 @@ export default createWidget("hamburger-menu", {
},
html(attrs, state) {
- if (!state.loaded) {
+ if (
+ this.currentUser &&
+ !this.currentUser.redesigned_user_menu_enabled &&
+ !state.loaded
+ ) {
this.refreshReviewableCount(state);
}
diff --git a/app/assets/javascripts/discourse/app/widgets/header.js b/app/assets/javascripts/discourse/app/widgets/header.js
index f5724e55034..0c445bbe37c 100644
--- a/app/assets/javascripts/discourse/app/widgets/header.js
+++ b/app/assets/javascripts/discourse/app/widgets/header.js
@@ -77,78 +77,109 @@ createWidget("header-notifications", {
if (user.isInDoNotDisturb()) {
contents.push(h("div.do-not-disturb-background", iconNode("moon")));
} else {
- const unreadNotifications = user.get("unread_notifications");
- if (!!unreadNotifications) {
- contents.push(
- this.attach("link", {
- action: attrs.action,
- className: "badge-notification unread-notifications",
- rawLabel: unreadNotifications,
- omitSpan: true,
- title: "notifications.tooltip.regular",
- titleOptions: { count: unreadNotifications },
- })
- );
- }
-
- const unreadHighPriority = user.get("unread_high_priority_notifications");
- if (!!unreadHighPriority) {
- // highlight the avatar if the first ever PM is not read
- if (
- !user.get("read_first_notification") &&
- !user.get("enforcedSecondFactor")
- ) {
- if (!attrs.active && attrs.ringBackdrop) {
- contents.push(h("span.ring"));
- contents.push(h("span.ring-backdrop-spotlight"));
- contents.push(
- h(
- "span.ring-backdrop",
- {},
- h("h1.ring-first-notification", {}, [
- h(
- "span",
- { className: "first-notification" },
- I18n.t("user.first_notification")
- ),
- h("span", { className: "read-later" }, [
- this.attach("link", {
- action: "readLater",
- className: "read-later-link",
- label: "user.skip_new_user_tips.read_later",
- }),
- ]),
- h("span", {}, [
- I18n.t("user.skip_new_user_tips.not_first_time"),
- " ",
- this.attach("link", {
- action: "skipNewUserTips",
- className: "skip-new-user-tips",
- label: "user.skip_new_user_tips.skip_link",
- title: "user.skip_new_user_tips.description",
- }),
- ]),
- ])
- )
- );
+ if (this.currentUser.redesigned_user_menu_enabled) {
+ const unread = user.all_unread_notifications_count || 0;
+ const reviewables = user.unseen_reviewable_count || 0;
+ const count = unread + reviewables;
+ if (count > 0) {
+ if (this._shouldHighlightAvatar()) {
+ this._addAvatarHighlight(contents);
}
+ contents.push(
+ this.attach("link", {
+ action: attrs.action,
+ className: "badge-notification unread-notifications",
+ rawLabel: count,
+ omitSpan: true,
+ title: "notifications.tooltip.regular",
+ titleOptions: { count },
+ })
+ );
+ }
+ } else {
+ const unreadNotifications = user.unread_notifications;
+ if (!!unreadNotifications) {
+ contents.push(
+ this.attach("link", {
+ action: attrs.action,
+ className: "badge-notification unread-notifications",
+ rawLabel: unreadNotifications,
+ omitSpan: true,
+ title: "notifications.tooltip.regular",
+ titleOptions: { count: unreadNotifications },
+ })
+ );
}
- // add the counter for the unread high priority
- contents.push(
- this.attach("link", {
- action: attrs.action,
- className: "badge-notification unread-high-priority-notifications",
- rawLabel: unreadHighPriority,
- omitSpan: true,
- title: "notifications.tooltip.high_priority",
- titleOptions: { count: unreadHighPriority },
- })
- );
+ const unreadHighPriority = user.unread_high_priority_notifications;
+ if (!!unreadHighPriority) {
+ if (this._shouldHighlightAvatar()) {
+ this._addAvatarHighlight(contents);
+ }
+
+ // add the counter for the unread high priority
+ contents.push(
+ this.attach("link", {
+ action: attrs.action,
+ className:
+ "badge-notification unread-high-priority-notifications",
+ rawLabel: unreadHighPriority,
+ omitSpan: true,
+ title: "notifications.tooltip.high_priority",
+ titleOptions: { count: unreadHighPriority },
+ })
+ );
+ }
}
}
return contents;
},
+
+ _shouldHighlightAvatar() {
+ const attrs = this.attrs;
+ const { user } = attrs;
+ return (
+ !user.read_first_notification &&
+ !user.enforcedSecondFactor &&
+ !attrs.active &&
+ attrs.ringBackdrop
+ );
+ },
+
+ _addAvatarHighlight(contents) {
+ contents.push(h("span.ring"));
+ contents.push(h("span.ring-backdrop-spotlight"));
+ contents.push(
+ h(
+ "span.ring-backdrop",
+ {},
+ h("h1.ring-first-notification", {}, [
+ h(
+ "span",
+ { className: "first-notification" },
+ I18n.t("user.first_notification")
+ ),
+ h("span", { className: "read-later" }, [
+ this.attach("link", {
+ action: "readLater",
+ className: "read-later-link",
+ label: "user.skip_new_user_tips.read_later",
+ }),
+ ]),
+ h("span", {}, [
+ I18n.t("user.skip_new_user_tips.not_first_time"),
+ " ",
+ this.attach("link", {
+ action: "skipNewUserTips",
+ className: "skip-new-user-tips",
+ label: "user.skip_new_user_tips.skip_link",
+ title: "user.skip_new_user_tips.description",
+ }),
+ ]),
+ ])
+ )
+ );
+ },
});
createWidget(
@@ -255,7 +286,10 @@ createWidget("header-icons", {
contents() {
let { currentUser } = this;
- if (currentUser && currentUser.reviewable_count) {
+ if (
+ currentUser?.reviewable_count &&
+ !this.currentUser.redesigned_user_menu_enabled
+ ) {
return h(
"div.badge-notification.reviewables",
{
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
index 44d2477df61..26e767d981f 100644
--- a/app/assets/javascripts/discourse/tests/integration/components/site-header-test.js
+++ b/app/assets/javascripts/discourse/tests/integration/components/site-header-test.js
@@ -1,6 +1,6 @@
import { module, test } from "qunit";
import { setupRenderingTest } from "discourse/tests/helpers/component-test";
-import { click, render, waitUntil } from "@ember/test-helpers";
+import { click, render, settled, waitUntil } from "@ember/test-helpers";
import { count, exists, query } from "discourse/tests/helpers/qunit-helpers";
import pretender, { response } from "discourse/tests/helpers/create-pretender";
import { hbs } from "ember-cli-htmlbars";
@@ -50,8 +50,35 @@ module("Integration | Component | site-header", function (hooks) {
await click("header.d-header");
});
+ test("displaying unread and reviewable notifications count when user's notifications and reviewables count are updated", async function (assert) {
+ this.currentUser.set("all_unread_notifications_count", 1);
+ this.currentUser.set("redesigned_user_menu_enabled", true);
+
+ await render(hbs``);
+ let unreadBadge = query(
+ ".header-dropdown-toggle.current-user .unread-notifications"
+ );
+ assert.strictEqual(unreadBadge.textContent, "1");
+
+ this.currentUser.set("all_unread_notifications_count", 5);
+ await settled();
+
+ unreadBadge = query(
+ ".header-dropdown-toggle.current-user .unread-notifications"
+ );
+ assert.strictEqual(unreadBadge.textContent, "5");
+
+ this.currentUser.set("unseen_reviewable_count", 3);
+ await settled();
+
+ unreadBadge = query(
+ ".header-dropdown-toggle.current-user .unread-notifications"
+ );
+ assert.strictEqual(unreadBadge.textContent, "8");
+ });
+
test("user avatar is highlighted when the user receives the first notification", async function (assert) {
- this.currentUser.set("all_unread_notifications", 1);
+ this.currentUser.set("all_unread_notifications_count", 1);
this.currentUser.set("redesigned_user_menu_enabled", true);
this.currentUser.set("read_first_notification", false);
await render(hbs``);
@@ -60,7 +87,7 @@ module("Integration | Component | site-header", function (hooks) {
test("user avatar is not highlighted when the user receives notifications beyond the first one", async function (assert) {
this.currentUser.set("redesigned_user_menu_enabled", true);
- this.currentUser.set("all_unread_notifications", 1);
+ this.currentUser.set("all_unread_notifications_count", 1);
this.currentUser.set("read_first_notification", true);
await render(hbs``);
assert.ok(!exists(".ring-first-notification"));
@@ -75,6 +102,13 @@ module("Integration | Component | site-header", function (hooks) {
assert.strictEqual(pendingReviewablesBadge.textContent, "1");
});
+ test("hamburger menu icon doesn't show pending reviewables count when revamped user menu is enabled", async function (assert) {
+ this.currentUser.set("reviewable_count", 1);
+ this.currentUser.set("redesigned_user_menu_enabled", true);
+ await render(hbs``);
+ assert.ok(!exists(".hamburger-dropdown .badge-notification"));
+ });
+
test("clicking outside the revamped menu closes it", async function (assert) {
this.currentUser.set("redesigned_user_menu_enabled", true);
await render(hbs``);
diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb
index eabc490a31c..21a1a662216 100644
--- a/app/controllers/notifications_controller.rb
+++ b/app/controllers/notifications_controller.rb
@@ -44,6 +44,22 @@ class NotificationsController < ApplicationController
current_user.publish_notifications_state
end
+ if !params.has_key?(:silent) && params[:bump_last_seen_reviewable] && !@readonly_mode
+ current_user_id = current_user.id
+ Scheduler::Defer.later "bump last seen reviewable for user" do
+ # we lookup current_user again in the background thread to avoid
+ # concurrency issues where the objects returned by the current_user
+ # and/or methods are changed by the time the deferred block is
+ # executed
+ user = User.find_by(id: current_user_id)
+ next if user.blank?
+ new_guardian = Guardian.new(user)
+ if new_guardian.can_see_review_queue?
+ user.bump_last_seen_reviewable!
+ end
+ end
+ end
+
render_json_dump(
notifications: serialize_data(notifications, NotificationSerializer),
seen_notification_id: current_user.seen_notification_id
diff --git a/app/controllers/reviewables_controller.rb b/app/controllers/reviewables_controller.rb
index 0f42edd2486..d0385343aa1 100644
--- a/app/controllers/reviewables_controller.rb
+++ b/app/controllers/reviewables_controller.rb
@@ -58,7 +58,8 @@ class ReviewablesController < ApplicationController
end,
meta: filters.merge(
total_rows_reviewables: total_rows, types: meta_types, reviewable_types: Reviewable.types,
- reviewable_count: current_user.reviewable_count
+ reviewable_count: current_user.reviewable_count,
+ unseen_reviewable_count: current_user.unseen_reviewable_count
)
}
if (offset + PER_PAGE) < total_rows
diff --git a/app/jobs/regular/notify_reviewable.rb b/app/jobs/regular/notify_reviewable.rb
index 9c3e729fe22..8d0c4f7c273 100644
--- a/app/jobs/regular/notify_reviewable.rb
+++ b/app/jobs/regular/notify_reviewable.rb
@@ -1,20 +1,13 @@
# frozen_string_literal: true
class Jobs::NotifyReviewable < ::Jobs::Base
-
+ # remove all the legacy stuff here when redesigned_user_menu_enabled is
+ # removed
def execute(args)
return unless reviewable = Reviewable.find_by(id: args[:reviewable_id])
@contacted = Set.new
- counts = Hash.new(0)
-
- Reviewable.default_visible.pending.each do |r|
- counts[:admins] += 1
- counts[:moderators] += 1 if r.reviewable_by_moderator?
- counts[r.reviewable_by_group_id] += 1 if r.reviewable_by_group_id
- end
-
all_updates = Hash.new { |h, k| h[k] = {} }
if args[:updated_reviewable_ids].present?
@@ -30,41 +23,63 @@ class Jobs::NotifyReviewable < ::Jobs::Base
end
end
- # admins
- notify(
- User.real.admins.pluck(:id),
+ counts = Hash.new(0)
+
+ Reviewable.default_visible.pending.each do |r|
+ counts[:admins] += 1
+ counts[:moderators] += 1 if r.reviewable_by_moderator?
+ counts[r.reviewable_by_group_id] += 1 if r.reviewable_by_group_id
+ end
+
+ redesigned_menu_enabled_user_ids = User.redesigned_user_menu_enabled_user_ids
+
+ new_menu_admins = User.real.admins.where(id: redesigned_menu_enabled_user_ids)
+ notify_users(new_menu_admins, all_updates[:admins])
+
+ legacy_menu_admins = User.real.admins.where("id NOT IN (?)", @contacted).pluck(:id)
+ notify_legacy(
+ legacy_menu_admins,
count: counts[:admins],
updates: all_updates[:admins],
)
- # moderators
if reviewable.reviewable_by_moderator?
- notify(
- User.real.moderators.where("id NOT IN (?)", @contacted).pluck(:id),
+ new_menu_mods = User
+ .real
+ .moderators
+ .where("id IN (?)", redesigned_menu_enabled_user_ids - @contacted.to_a)
+ notify_users(new_menu_mods, all_updates[:moderators])
+
+ legacy_menu_mods = User.real.moderators.where("id NOT IN (?)", @contacted).pluck(:id)
+ notify_legacy(
+ legacy_menu_mods,
count: counts[:moderators],
updates: all_updates[:moderators],
)
end
- # category moderators
if SiteSetting.enable_category_group_moderation? && (group = reviewable.reviewable_by_group)
- group.users.includes(:group_users).where("users.id NOT IN (?)", @contacted).find_each do |user|
+ users = group.users.includes(:group_users).where("users.id NOT IN (?)", @contacted)
+ users.find_each do |user|
count = 0
updates = {}
-
user.group_users.each do |gu|
- count += counts[gu.group_id] || 0
- updates.merge!(all_updates[gu.group_id] || {})
+ updates.merge!(all_updates[gu.group_id])
+ count += counts[gu.group_id]
+ end
+ if redesigned_menu_enabled_user_ids.include?(user.id)
+ notify_user(user, updates)
+ else
+ notify_legacy([user.id], count: count, updates: updates)
end
-
- notify([user.id], count: count, updates: updates)
end
+ @contacted += users.pluck(:id)
end
end
protected
- def notify(user_ids, count:, updates:)
+ def notify_legacy(user_ids, count:, updates:)
return if user_ids.blank?
data = { reviewable_count: count }
@@ -74,4 +89,18 @@ class Jobs::NotifyReviewable < ::Jobs::Base
@contacted += user_ids
end
+ def notify_users(users, updates)
+ users.find_each { |user| notify_user(user, updates) }
+ @contacted += users.pluck(:id)
+ end
+
+ def notify_user(user, updates)
+ data = {
+ reviewable_count: user.reviewable_count,
+ unseen_reviewable_count: user.unseen_reviewable_count
+ }
+ data[:updates] = updates if updates.present?
+
+ user.publish_reviewable_counts(data)
+ end
end
diff --git a/app/models/reviewable.rb b/app/models/reviewable.rb
index d27924fc590..b1b5f518f4d 100644
--- a/app/models/reviewable.rb
+++ b/app/models/reviewable.rb
@@ -541,6 +541,17 @@ class Reviewable < ActiveRecord::Base
result
end
+ def self.unseen_list_for(user, preload: true, limit: nil)
+ results = list_for(user, preload: preload, limit: limit)
+ if user.last_seen_reviewable_id
+ results = results.where(
+ "reviewables.id > ?",
+ user.last_seen_reviewable_id
+ )
+ end
+ results
+ end
+
def self.recent_list_with_pending_first(user, limit: 30)
min_score = Reviewable.min_score_for_priority
diff --git a/app/models/user.rb b/app/models/user.rb
index e8b4adc193a..319221e209e 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -487,6 +487,7 @@ class User < ActiveRecord::Base
def reload
@unread_notifications = nil
+ @all_unread_notifications_count = nil
@unread_total_notifications = nil
@unread_pms = nil
@unread_bookmarks = nil
@@ -587,6 +588,29 @@ class User < ActiveRecord::Base
end
end
+ def all_unread_notifications_count
+ @all_unread_notifications_count ||= begin
+ sql = <<~SQL
+ SELECT COUNT(*) FROM (
+ SELECT 1 FROM
+ notifications n
+ LEFT JOIN topics t ON t.id = n.topic_id
+ WHERE t.deleted_at IS NULL AND
+ n.user_id = :user_id AND
+ n.id > :seen_notification_id AND
+ NOT read
+ LIMIT :limit
+ ) AS X
+ SQL
+
+ DB.query_single(sql,
+ user_id: id,
+ seen_notification_id: seen_notification_id,
+ limit: User.max_unread_notifications
+ )[0].to_i
+ end
+ end
+
def total_unread_notifications
@unread_total_notifications ||= notifications.where("read = false").count
end
@@ -595,6 +619,10 @@ class User < ActiveRecord::Base
Reviewable.list_for(self).count
end
+ def unseen_reviewable_count
+ Reviewable.unseen_list_for(self).count
+ end
+
def saw_notification_id(notification_id)
if seen_notification_id.to_i < notification_id.to_i
update_columns(seen_notification_id: notification_id.to_i)
@@ -604,6 +632,29 @@ class User < ActiveRecord::Base
end
end
+ def bump_last_seen_reviewable!
+ query = Reviewable.unseen_list_for(self, preload: false)
+
+ if last_seen_reviewable_id
+ query = query.where("id > ?", last_seen_reviewable_id)
+ end
+ max_reviewable_id = query.maximum(:id)
+
+ if max_reviewable_id
+ update!(last_seen_reviewable_id: max_reviewable_id)
+ publish_reviewable_counts(unseen_reviewable_count: self.unseen_reviewable_count)
+ MessageBus.publish(
+ "/reviewable_counts",
+ { unseen_reviewable_count: self.unseen_reviewable_count },
+ user_ids: [self.id]
+ )
+ end
+ end
+
+ def publish_reviewable_counts(data)
+ MessageBus.publish("/reviewable_counts/#{self.id}", data, user_ids: [self.id])
+ end
+
TRACK_FIRST_NOTIFICATION_READ_DURATION = 1.week.to_i
def read_first_notification?
@@ -663,6 +714,10 @@ class User < ActiveRecord::Base
seen_notification_id: seen_notification_id,
}
+ if self.redesigned_user_menu_enabled?
+ payload[:all_unread_notifications_count] = all_unread_notifications_count
+ end
+
MessageBus.publish("/notification/#{id}", payload, user_ids: [id])
end
diff --git a/app/serializers/current_user_serializer.rb b/app/serializers/current_user_serializer.rb
index d341d014386..df807662f87 100644
--- a/app/serializers/current_user_serializer.rb
+++ b/app/serializers/current_user_serializer.rb
@@ -7,6 +7,7 @@ class CurrentUserSerializer < BasicUserSerializer
:unread_notifications,
:unread_private_messages,
:unread_high_priority_notifications,
+ :all_unread_notifications_count,
:read_first_notification?,
:admin?,
:notification_channel_position,
@@ -43,6 +44,7 @@ class CurrentUserSerializer < BasicUserSerializer
:dismissed_banner_key,
:is_anonymous,
:reviewable_count,
+ :unseen_reviewable_count,
:read_faq?,
:automatically_unpin_topics,
:mailing_list_mode,
@@ -338,4 +340,12 @@ class CurrentUserSerializer < BasicUserSerializer
def likes_notifications_disabled
object.user_option&.likes_notifications_disabled?
end
+
+ def include_all_unread_notifications_count?
+ redesigned_user_menu_enabled
+ end
+
+ def include_unseen_reviewable_count?
+ redesigned_user_menu_enabled
+ end
end
diff --git a/app/serializers/reviewable_perform_result_serializer.rb b/app/serializers/reviewable_perform_result_serializer.rb
index e0a7695eb18..6cd1f6fbdf3 100644
--- a/app/serializers/reviewable_perform_result_serializer.rb
+++ b/app/serializers/reviewable_perform_result_serializer.rb
@@ -10,7 +10,8 @@ class ReviewablePerformResultSerializer < ApplicationSerializer
:created_post_topic_id,
:remove_reviewable_ids,
:version,
- :reviewable_count
+ :reviewable_count,
+ :unseen_reviewable_count
)
def success
@@ -42,6 +43,14 @@ class ReviewablePerformResultSerializer < ApplicationSerializer
end
def reviewable_count
- Reviewable.list_for(scope.user).count
+ scope.user.reviewable_count
+ end
+
+ def unseen_reviewable_count
+ scope.user.unseen_reviewable_count
+ end
+
+ def include_unseen_reviewable_count?
+ scope.user.redesigned_user_menu_enabled?
end
end
diff --git a/spec/jobs/notify_reviewable_spec.rb b/spec/jobs/notify_reviewable_spec.rb
index d54c08f7041..3ade8a22826 100644
--- a/spec/jobs/notify_reviewable_spec.rb
+++ b/spec/jobs/notify_reviewable_spec.rb
@@ -1,102 +1,174 @@
# frozen_string_literal: true
RSpec.describe Jobs::NotifyReviewable do
- describe '.execute' do
- fab!(:admin) { Fabricate(:admin, moderator: true) }
- fab!(:moderator) { Fabricate(:moderator) }
+ # remove all the legacy stuff here when redesigned_user_menu_enabled is
+ # removed
+ describe '#execute' do
+ fab!(:legacy_menu_admin) { Fabricate(:admin, moderator: true) }
+ fab!(:legacy_menu_mod) { Fabricate(:moderator) }
fab!(:group_user) { Fabricate(:group_user) }
- let(:user) { group_user.user }
- let(:group) { group_user.group }
+ fab!(:legacy_menu_user) { group_user.user }
- it "will notify users of new reviewable content" do
+ fab!(:group) { group_user.group }
+
+ fab!(:new_menu_admin) { Fabricate(:admin, moderator: true) }
+ fab!(:new_menu_mod) { Fabricate(:moderator) }
+ fab!(:new_menu_user) { Fabricate(:user, groups: [group]) }
+
+ before do
+ [new_menu_admin, new_menu_mod, new_menu_user].each(&:enable_redesigned_user_menu)
+ end
+
+ after do
+ [new_menu_admin, new_menu_mod, new_menu_user].each(&:disable_redesigned_user_menu)
+ end
+
+ it "will notify users of new reviewable content and respects backward compatibility for the legacy user menu" do
SiteSetting.enable_category_group_moderation = true
- GroupUser.create!(group_id: group.id, user_id: moderator.id)
+ GroupUser.create!(group_id: group.id, user_id: legacy_menu_mod.id)
# Content for admins only
- r1 = Fabricate(:reviewable, reviewable_by_moderator: false)
- messages = MessageBus.track_publish("/reviewable_counts") do
- described_class.new.execute(reviewable_id: r1.id)
+ admin_reviewable = Fabricate(:reviewable, reviewable_by_moderator: false)
+ new_menu_admin.update!(last_seen_reviewable_id: admin_reviewable.id)
+ messages = MessageBus.track_publish do
+ described_class.new.execute(reviewable_id: admin_reviewable.id)
end
- admin_msg = messages.find { |m| m.user_ids.include?(admin.id) }
- expect(admin_msg.data[:reviewable_count]).to eq(1)
- expect(messages.any? { |m| m.user_ids.include?(moderator.id) }).to eq(false)
- expect(messages.any? { |m| m.user_ids.include?(user.id) }).to eq(false)
+ expect(messages.size).to eq(2)
+ legacy_menu_admin_msg = messages.find { |m| m.user_ids.include?(legacy_menu_admin.id) }
+ expect(legacy_menu_admin_msg.data[:reviewable_count]).to eq(1)
+ expect(legacy_menu_admin_msg.channel).to eq("/reviewable_counts")
+ expect(legacy_menu_admin_msg.data.key?(:unseen_reviewable_count)).to eq(false)
+
+ new_menu_admin_msg = messages.find { |m| m.user_ids == [new_menu_admin.id] }
+ expect(new_menu_admin_msg.data[:reviewable_count]).to eq(1)
+ expect(new_menu_admin_msg.channel).to eq("/reviewable_counts/#{new_menu_admin.id}")
+ expect(new_menu_admin_msg.data[:unseen_reviewable_count]).to eq(0)
+
+ expect(messages.any? { |m| m.user_ids.include?(legacy_menu_mod.id) }).to eq(false)
+ expect(messages.any? { |m| m.user_ids.include?(legacy_menu_user.id) }).to eq(false)
+ expect(messages.any? { |m| m.user_ids.include?(new_menu_mod.id) }).to eq(false)
+ expect(messages.any? { |m| m.user_ids.include?(new_menu_user.id) }).to eq(false)
# Content for moderators
- r2 = Fabricate(:reviewable, reviewable_by_moderator: true)
- messages = MessageBus.track_publish("/reviewable_counts") do
- described_class.new.execute(reviewable_id: r2.id)
+ mod_reviewable = Fabricate(:reviewable, reviewable_by_moderator: true)
+ messages = MessageBus.track_publish do
+ described_class.new.execute(reviewable_id: mod_reviewable.id)
end
- admin_msg = messages.find { |m| m.user_ids.include?(admin.id) }
- expect(admin_msg.data[:reviewable_count]).to eq(2)
- mod_msg = messages.find { |m| m.user_ids.include?(moderator.id) }
- expect(mod_msg.data[:reviewable_count]).to eq(1)
- expect(mod_msg.user_ids).to_not include(admin.id)
- expect(messages.any? { |m| m.user_ids.include?(user.id) }).to eq(false)
+ expect(messages.size).to eq(4)
+ legacy_menu_admin_msg = messages.find { |m| m.user_ids == [legacy_menu_admin.id] }
+ expect(legacy_menu_admin_msg.data[:reviewable_count]).to eq(2)
+ expect(legacy_menu_admin_msg.channel).to eq("/reviewable_counts")
+ expect(legacy_menu_admin_msg.data.key?(:unseen_reviewable_count)).to eq(false)
+
+ new_menu_admin_msg = messages.find { |m| m.user_ids == [new_menu_admin.id] }
+ expect(new_menu_admin_msg.data[:reviewable_count]).to eq(2)
+ expect(new_menu_admin_msg.channel).to eq("/reviewable_counts/#{new_menu_admin.id}")
+ expect(new_menu_admin_msg.data[:unseen_reviewable_count]).to eq(1)
+
+ legacy_menu_mod_msg = messages.find { |m| m.user_ids == [legacy_menu_mod.id] }
+ expect(legacy_menu_mod_msg.data[:reviewable_count]).to eq(1)
+ expect(legacy_menu_mod_msg.channel).to eq("/reviewable_counts")
+ expect(legacy_menu_mod_msg.data.key?(:unseen_reviewable_count)).to eq(false)
+
+ new_menu_mod_msg = messages.find { |m| m.user_ids == [new_menu_mod.id] }
+ expect(new_menu_mod_msg.data[:reviewable_count]).to eq(1)
+ expect(new_menu_mod_msg.channel).to eq("/reviewable_counts/#{new_menu_mod.id}")
+ expect(new_menu_mod_msg.data[:unseen_reviewable_count]).to eq(1)
+
+ expect(messages.any? { |m| m.user_ids.include?(legacy_menu_user.id) }).to eq(false)
+ expect(messages.any? { |m| m.user_ids.include?(new_menu_user.id) }).to eq(false)
+
+ new_menu_mod.update!(last_seen_reviewable_id: mod_reviewable.id)
# Content for a group
- r3 = Fabricate(:reviewable, reviewable_by_moderator: true, reviewable_by_group: group)
- messages = MessageBus.track_publish("/reviewable_counts") do
- described_class.new.execute(reviewable_id: r3.id)
+ group_reviewable = Fabricate(:reviewable, reviewable_by_moderator: true, reviewable_by_group: group)
+ messages = MessageBus.track_publish do
+ described_class.new.execute(reviewable_id: group_reviewable.id)
end
- admin_msg = messages.find { |m| m.user_ids.include?(admin.id) }
- expect(admin_msg.data[:reviewable_count]).to eq(3)
- mod_messages = messages.select { |m| m.user_ids.include?(moderator.id) }
- expect(mod_messages.size).to eq(1)
- expect(mod_messages[0].data[:reviewable_count]).to eq(2)
- group_msg = messages.find { |m| m.user_ids.include?(user.id) }
- expect(group_msg.data[:reviewable_count]).to eq(1)
+ expect(messages.size).to eq(6)
+ legacy_menu_admin_msg = messages.find { |m| m.user_ids == [legacy_menu_admin.id] }
+ expect(legacy_menu_admin_msg.data[:reviewable_count]).to eq(3)
+ expect(legacy_menu_admin_msg.channel).to eq("/reviewable_counts")
+ expect(legacy_menu_admin_msg.data.key?(:unseen_reviewable_count)).to eq(false)
+
+ new_menu_admin_msg = messages.find { |m| m.user_ids == [new_menu_admin.id] }
+ expect(new_menu_admin_msg.data[:reviewable_count]).to eq(3)
+ expect(new_menu_admin_msg.channel).to eq("/reviewable_counts/#{new_menu_admin.id}")
+ expect(new_menu_admin_msg.data[:unseen_reviewable_count]).to eq(2)
+
+ legacy_menu_mod_msg = messages.find { |m| m.user_ids == [legacy_menu_mod.id] }
+ expect(legacy_menu_mod_msg.data[:reviewable_count]).to eq(2)
+ expect(legacy_menu_mod_msg.channel).to eq("/reviewable_counts")
+ expect(legacy_menu_mod_msg.data.key?(:unseen_reviewable_count)).to eq(false)
+
+ new_menu_mod_msg = messages.find { |m| m.user_ids == [new_menu_mod.id] }
+ expect(new_menu_mod_msg.data[:reviewable_count]).to eq(2)
+ expect(new_menu_mod_msg.channel).to eq("/reviewable_counts/#{new_menu_mod.id}")
+ expect(new_menu_mod_msg.data[:unseen_reviewable_count]).to eq(1)
+
+ legacy_menu_user_msg = messages.find { |m| m.user_ids == [legacy_menu_user.id] }
+ expect(legacy_menu_user_msg.data[:reviewable_count]).to eq(1)
+ expect(legacy_menu_user_msg.channel).to eq("/reviewable_counts")
+ expect(legacy_menu_user_msg.data.key?(:unseen_reviewable_count)).to eq(false)
+
+ new_menu_user_msg = messages.find { |m| m.user_ids == [new_menu_user.id] }
+ expect(new_menu_user_msg.data[:reviewable_count]).to eq(1)
+ expect(new_menu_user_msg.channel).to eq("/reviewable_counts/#{new_menu_user.id}")
+ expect(new_menu_user_msg.data[:unseen_reviewable_count]).to eq(1)
end
it "won't notify a group when disabled" do
SiteSetting.enable_category_group_moderation = false
- GroupUser.create!(group_id: group.id, user_id: moderator.id)
+ GroupUser.create!(group_id: group.id, user_id: legacy_menu_mod.id)
+ GroupUser.create!(group_id: group.id, user_id: new_menu_mod.id)
r3 = Fabricate(:reviewable, reviewable_by_moderator: true, reviewable_by_group: group)
messages = MessageBus.track_publish("/reviewable_counts") do
described_class.new.execute(reviewable_id: r3.id)
end
- group_msg = messages.find { |m| m.user_ids.include?(user.id) }
+ group_msg = messages.find { |m| m.user_ids.include?(legacy_menu_user.id) }
+ expect(group_msg).to be_blank
+ group_msg = messages.find { |m| m.user_ids.include?(new_menu_user.id) }
expect(group_msg).to be_blank
end
- it "respects visibility" do
+ it "respects priority" do
SiteSetting.enable_category_group_moderation = true
Reviewable.set_priorities(medium: 2.0)
SiteSetting.reviewable_default_visibility = 'medium'
- GroupUser.create!(group_id: group.id, user_id: moderator.id)
+ GroupUser.create!(group_id: group.id, user_id: legacy_menu_mod.id)
# Content for admins only
r1 = Fabricate(:reviewable, reviewable_by_moderator: false)
messages = MessageBus.track_publish("/reviewable_counts") do
described_class.new.execute(reviewable_id: r1.id)
end
- admin_msg = messages.find { |m| m.user_ids.include?(admin.id) }
- expect(admin_msg.data[:reviewable_count]).to eq(0)
+ legacy_menu_admin_msg = messages.find { |m| m.user_ids.include?(legacy_menu_admin.id) }
+ expect(legacy_menu_admin_msg.data[:reviewable_count]).to eq(0)
# Content for moderators
r2 = Fabricate(:reviewable, reviewable_by_moderator: true)
messages = MessageBus.track_publish("/reviewable_counts") do
described_class.new.execute(reviewable_id: r2.id)
end
- admin_msg = messages.find { |m| m.user_ids.include?(admin.id) }
- expect(admin_msg.data[:reviewable_count]).to eq(0)
- mod_msg = messages.find { |m| m.user_ids.include?(moderator.id) }
- expect(mod_msg.data[:reviewable_count]).to eq(0)
+ legacy_menu_admin_msg = messages.find { |m| m.user_ids.include?(legacy_menu_admin.id) }
+ expect(legacy_menu_admin_msg.data[:reviewable_count]).to eq(0)
+ legacy_menu_mod_msg = messages.find { |m| m.user_ids.include?(legacy_menu_mod.id) }
+ expect(legacy_menu_mod_msg.data[:reviewable_count]).to eq(0)
# Content for a group
r3 = Fabricate(:reviewable, reviewable_by_moderator: true, reviewable_by_group: group)
messages = MessageBus.track_publish("/reviewable_counts") do
described_class.new.execute(reviewable_id: r3.id)
end
- admin_msg = messages.find { |m| m.user_ids.include?(admin.id) }
- expect(admin_msg.data[:reviewable_count]).to eq(0)
- mod_messages = messages.select { |m| m.user_ids.include?(moderator.id) }
+ legacy_menu_admin_msg = messages.find { |m| m.user_ids.include?(legacy_menu_admin.id) }
+ expect(legacy_menu_admin_msg.data[:reviewable_count]).to eq(0)
+ mod_messages = messages.select { |m| m.user_ids.include?(legacy_menu_mod.id) }
expect(mod_messages.size).to eq(1)
expect(mod_messages[0].data[:reviewable_count]).to eq(0)
- group_msg = messages.find { |m| m.user_ids.include?(user.id) }
+ group_msg = messages.find { |m| m.user_ids.include?(legacy_menu_user.id) }
expect(group_msg.data[:reviewable_count]).to eq(0)
end
end
diff --git a/spec/models/reviewable_spec.rb b/spec/models/reviewable_spec.rb
index 72acf5e78fa..c404f14ef99 100644
--- a/spec/models/reviewable_spec.rb
+++ b/spec/models/reviewable_spec.rb
@@ -244,6 +244,63 @@ RSpec.describe Reviewable, type: :model do
end
end
+ describe ".unseen_list_for" do
+ fab!(:admin) { Fabricate(:admin) }
+ fab!(:moderator) { Fabricate(:moderator) }
+ fab!(:group) { Fabricate(:group) }
+ fab!(:user) { Fabricate(:user, groups: [group]) }
+ fab!(:admin_reviewable) { Fabricate(:reviewable, reviewable_by_moderator: false) }
+ fab!(:mod_reviewable) { Fabricate(:reviewable, reviewable_by_moderator: true) }
+ fab!(:group_reviewable) {
+ Fabricate(:reviewable, reviewable_by_group: group, reviewable_by_moderator: false)
+ }
+
+ context "for admins" do
+ it "returns a list of pending reviewables that haven't been seen by the user" do
+ list = Reviewable.unseen_list_for(admin, preload: false)
+ expect(list).to contain_exactly(admin_reviewable, mod_reviewable, group_reviewable)
+ admin_reviewable.update!(status: Reviewable.statuses[:approved])
+ list = Reviewable.unseen_list_for(admin, preload: false)
+ expect(list).to contain_exactly(mod_reviewable, group_reviewable)
+ admin.update!(last_seen_reviewable_id: group_reviewable.id)
+ expect(Reviewable.unseen_list_for(admin, preload: false).empty?).to eq(true)
+ end
+ end
+
+ context "for moderators" do
+ it "returns a list of pending reviewables that haven't been seen by the user" do
+ list = Reviewable.unseen_list_for(moderator, preload: false)
+ expect(list).to contain_exactly(mod_reviewable)
+
+ group_reviewable.update!(reviewable_by_moderator: true)
+
+ list = Reviewable.unseen_list_for(moderator, preload: false)
+ expect(list).to contain_exactly(mod_reviewable, group_reviewable)
+
+ moderator.update!(last_seen_reviewable_id: mod_reviewable.id)
+
+ list = Reviewable.unseen_list_for(moderator, preload: false)
+ expect(list).to contain_exactly(group_reviewable)
+ end
+ end
+
+ context "for group moderators" do
+ before do
+ SiteSetting.enable_category_group_moderation = true
+ end
+
+ it "returns a list of pending reviewables that haven't been seen by the user" do
+ list = Reviewable.unseen_list_for(user, preload: false)
+ expect(list).to contain_exactly(group_reviewable)
+
+ user.update!(last_seen_reviewable_id: group_reviewable.id)
+
+ list = Reviewable.unseen_list_for(user, preload: false)
+ expect(list).to be_empty
+ end
+ end
+ end
+
describe ".recent_list_with_pending_first" do
fab!(:pending_reviewable1) do
Fabricate(
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 0e7381c6834..0ff81b202d3 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -2057,6 +2057,24 @@ RSpec.describe User do
expect(message).to eq(nil)
end
+
+ context "with redesigned_user_menu_enabled on" do
+ it "adds all_unread_notifications_count to the payload" do
+ user.update!(admin: true)
+ user.enable_redesigned_user_menu
+ Fabricate(:notification, user: user)
+ Fabricate(:notification, notification_type: 15, high_priority: true, read: false, user: user)
+ messages = MessageBus.track_publish("/notification/#{user.id}") do
+ user.publish_notifications_state
+ end
+ expect(messages.size).to eq(1)
+
+ message = messages.first
+ expect(message.data[:all_unread_notifications_count]).to eq(2)
+ ensure
+ user.disable_redesigned_user_menu
+ end
+ end
end
describe "silenced?" do
@@ -2780,4 +2798,112 @@ RSpec.describe User do
expect(user.whisperer?).to eq(false)
end
end
+
+ describe "#all_unread_notifications_count" do
+ it "returns count of unseen and unread high priority and normal priority notifications" do
+ Fabricate(:notification, user: user, high_priority: true, read: false)
+ n2 = Fabricate(:notification, user: user, high_priority: false, read: false)
+ expect(user.all_unread_notifications_count).to eq(2)
+
+ n2.update!(read: true)
+ user.reload
+
+ expect(user.all_unread_notifications_count).to eq(1)
+
+ user.update!(seen_notification_id: n2.id)
+ user.reload
+
+ expect(user.all_unread_notifications_count).to eq(0)
+
+ n3 = Fabricate(:notification, user: user)
+ user.reload
+
+ expect(user.all_unread_notifications_count).to eq(1)
+
+ n3.topic.trash!(Fabricate(:admin))
+ user.reload
+
+ expect(user.all_unread_notifications_count).to eq(0)
+ end
+ end
+
+ describe "#unseen_reviewable_count" do
+ fab!(:admin_reviewable) { Fabricate(:reviewable, reviewable_by_moderator: false) }
+ fab!(:mod_reviewable) { Fabricate(:reviewable, reviewable_by_moderator: true) }
+ fab!(:group_reviewable) { Fabricate(:reviewable, reviewable_by_moderator: false, reviewable_by_group: group) }
+
+ it "doesn't include reviewables that can't be seen by the user" do
+ SiteSetting.enable_category_group_moderation = true
+ expect(user.unseen_reviewable_count).to eq(0)
+ user.groups << group
+ user.save!
+ expect(user.unseen_reviewable_count).to eq(1)
+ user.update!(moderator: true)
+ expect(user.unseen_reviewable_count).to eq(2)
+ user.update!(admin: true)
+ expect(user.unseen_reviewable_count).to eq(3)
+ end
+
+ it "returns count of unseen reviewables" do
+ user.update!(admin: true)
+ expect(user.unseen_reviewable_count).to eq(3)
+ user.update!(last_seen_reviewable_id: mod_reviewable.id)
+ expect(user.unseen_reviewable_count).to eq(1)
+ user.update!(last_seen_reviewable_id: group_reviewable.id)
+ expect(user.unseen_reviewable_count).to eq(0)
+ end
+ end
+
+ describe "#bump_last_seen_reviewable!" do
+ it "doesn't error if there are no reviewables" do
+ Reviewable.destroy_all
+ user.bump_last_seen_reviewable!
+ expect(user.last_seen_reviewable_id).to eq(nil)
+ end
+
+ it "picks the reviewable of the largest id" do
+ user.update!(admin: true)
+ Fabricate(
+ :reviewable,
+ created_at: 3.minutes.ago,
+ updated_at: 3.minutes.ago,
+ score: 100
+ )
+ reviewable2 = Fabricate(
+ :reviewable,
+ created_at: 30.minutes.ago,
+ updated_at: 30.minutes.ago,
+ score: 10
+ )
+ user.bump_last_seen_reviewable!
+ expect(user.last_seen_reviewable_id).to eq(reviewable2.id)
+ end
+
+ it "stays at the maximum reviewable if there are no new reviewables" do
+ user.update!(admin: true)
+ reviewable = Fabricate(:reviewable)
+ user.bump_last_seen_reviewable!
+ expect(user.last_seen_reviewable_id).to eq(reviewable.id)
+ user.bump_last_seen_reviewable!
+ expect(user.last_seen_reviewable_id).to eq(reviewable.id)
+ end
+
+ it "respects reviewables security" do
+ admin = Fabricate(:admin)
+ moderator = Fabricate(:moderator)
+ group = Fabricate(:group)
+ user.update!(groups: [group])
+ SiteSetting.enable_category_group_moderation = true
+
+ group_reviewable = Fabricate(:reviewable, reviewable_by_moderator: false, reviewable_by_group: group)
+ mod_reviewable = Fabricate(:reviewable, reviewable_by_moderator: true)
+ admin_reviewable = Fabricate(:reviewable, reviewable_by_moderator: false)
+
+ [admin, moderator, user].each(&:bump_last_seen_reviewable!)
+
+ expect(admin.last_seen_reviewable_id).to eq(admin_reviewable.id)
+ expect(moderator.last_seen_reviewable_id).to eq(mod_reviewable.id)
+ expect(user.last_seen_reviewable_id).to eq(group_reviewable.id)
+ end
+ end
end
diff --git a/spec/requests/notifications_controller_spec.rb b/spec/requests/notifications_controller_spec.rb
index 4e44c9f499e..4b87537ba61 100644
--- a/spec/requests/notifications_controller_spec.rb
+++ b/spec/requests/notifications_controller_spec.rb
@@ -87,6 +87,60 @@ RSpec.describe NotificationsController do
Discourse.clear_redis_readonly!
end
+ it "should not bump last seen reviewable in readonly mode" do
+ user.update!(admin: true)
+ Fabricate(:reviewable)
+ Discourse.received_redis_readonly!
+ expect {
+ get "/notifications.json", params: { recent: true }
+ expect(response.status).to eq(200)
+ }.not_to change { user.reload.last_seen_reviewable_id }
+ ensure
+ Discourse.clear_redis_readonly!
+ end
+
+ it "should not bump last seen reviewable if the user can't seen reviewables" do
+ Fabricate(:reviewable)
+ expect {
+ get "/notifications.json", params: { recent: true, bump_last_seen_reviewable: true }
+ expect(response.status).to eq(200)
+ }.not_to change { user.reload.last_seen_reviewable_id }
+ end
+
+ it "should not bump last seen reviewable if the silent param is present" do
+ user.update!(admin: true)
+ Fabricate(:reviewable)
+ expect {
+ get "/notifications.json", params: {
+ recent: true,
+ silent: true,
+ bump_last_seen_reviewable: true
+ }
+ expect(response.status).to eq(200)
+ }.not_to change { user.reload.last_seen_reviewable_id }
+ end
+
+ it "should not bump last seen reviewable if the bump_last_seen_reviewable param is not present" do
+ user.update!(admin: true)
+ Fabricate(:reviewable)
+ expect {
+ get "/notifications.json", params: { recent: true, silent: true }
+ expect(response.status).to eq(200)
+ }.not_to change { user.reload.last_seen_reviewable_id }
+ end
+
+ it "bumps last_seen_reviewable_id" do
+ user.update!(admin: true)
+ expect(user.last_seen_reviewable_id).to eq(nil)
+ reviewable = Fabricate(:reviewable)
+ get "/notifications.json", params: { recent: true, bump_last_seen_reviewable: true }
+ expect(user.reload.last_seen_reviewable_id).to eq(reviewable.id)
+
+ reviewable2 = Fabricate(:reviewable)
+ get "/notifications.json", params: { recent: true, bump_last_seen_reviewable: true }
+ expect(user.reload.last_seen_reviewable_id).to eq(reviewable2.id)
+ end
+
it "get notifications with all filters" do
notification = Fabricate(:notification, user: user)
notification2 = Fabricate(:notification, user: user)
diff --git a/spec/requests/reviewables_controller_spec.rb b/spec/requests/reviewables_controller_spec.rb
index f959ce6fa71..daa6ae11916 100644
--- a/spec/requests/reviewables_controller_spec.rb
+++ b/spec/requests/reviewables_controller_spec.rb
@@ -76,6 +76,7 @@ RSpec.describe ReviewablesController do
expect(json['users'].any? { |u| u['id'] == reviewable.target_created_by_id }).to eq(true)
expect(json['meta']['reviewable_count']).to eq(1)
+ expect(json['meta']['unseen_reviewable_count']).to eq(1)
expect(json['meta']['status']).to eq("pending")
end