From 4d05e3edab1c2968980316ef6e1d0ef7e5bdca82 Mon Sep 17 00:00:00 2001 From: Osama Sayegh Date: Wed, 5 Oct 2022 12:30:02 +0300 Subject: [PATCH] DEV: Include pending reviewables in the main tab in the user menu (#18471) This commit makes pending reviewables show up in the main tab (a.k.a. "all notifications" tab). Pending reviewables along with unread notifications are always shown first and they're sorted based on their creation date (most recent comes first). The dismiss button currently only shows up if there are unread notifications and it doesn't dismiss pending reviewables. We may follow up with another change soon that allows makes the dismiss button work with reviewables and remove them from the list without taking any action on them. Follow-up to https://github.com/discourse/discourse/commit/079450c9e496ac9bed8ff491ceb1e4ac0b83448f. --- .../app/components/user-menu/messages-list.js | 48 ++---- .../user-menu/notifications-list.js | 80 ++++++++-- .../discourse/app/lib/utilities.js | 26 ++++ .../discourse/app/models/notification.js | 6 + .../user-menu/notifications-list-test.js | 124 ++++++++++++++- .../tests/unit/lib/utilities-test.js | 40 +++++ app/controllers/notifications_controller.rb | 29 ++-- app/controllers/reviewables_controller.rb | 6 +- app/models/reviewable.rb | 8 + .../basic_reviewable_serializer.rb | 2 +- .../requests/notifications_controller_spec.rb | 142 +++++++++++------- .../common_basic_reviewable_serializer.rb | 8 + 12 files changed, 394 insertions(+), 125 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/user-menu/messages-list.js b/app/assets/javascripts/discourse/app/components/user-menu/messages-list.js index 7d916ec668b..192967c3ca0 100644 --- a/app/assets/javascripts/discourse/app/components/user-menu/messages-list.js +++ b/app/assets/javascripts/discourse/app/components/user-menu/messages-list.js @@ -6,18 +6,7 @@ import I18n from "I18n"; import UserMenuNotificationItem from "discourse/lib/user-menu/notification-item"; import UserMenuMessageItem from "discourse/lib/user-menu/message-item"; import Topic from "discourse/models/topic"; - -function parseDateString(date) { - if (date) { - return new Date(date); - } -} - -async function initializeNotifications(rawList) { - const notifications = rawList.map((n) => Notification.create(n)); - await Notification.applyTransformations(notifications); - return notifications; -} +import { mergeSortedLists } from "discourse/lib/utilities"; export default class UserMenuMessagesList extends UserMenuNotificationsList { get dismissTypes() { @@ -63,7 +52,7 @@ export default class UserMenuMessagesList extends UserMenuNotificationsList { ); const content = []; - const unreadNotifications = await initializeNotifications( + const unreadNotifications = await Notification.initializeNotifications( data.unread_notifications ); unreadNotifications.forEach((notification) => { @@ -80,38 +69,29 @@ export default class UserMenuMessagesList extends UserMenuNotificationsList { const topics = data.topics.map((t) => Topic.create(t)); await Topic.applyTransformations(topics); - const readNotifications = await initializeNotifications( + const readNotifications = await Notification.initializeNotifications( data.read_notifications ); - let latestReadNotificationDate = parseDateString( - readNotifications[0]?.created_at - ); - let latestMessageDate = parseDateString(topics[0]?.bumped_at); - - while (latestReadNotificationDate || latestMessageDate) { - if ( - !latestReadNotificationDate || - (latestMessageDate && latestReadNotificationDate < latestMessageDate) - ) { - content.push(new UserMenuMessageItem({ message: topics[0] })); - topics.shift(); - latestMessageDate = parseDateString(topics[0]?.bumped_at); - } else { + mergeSortedLists(readNotifications, topics, (notification, topic) => { + const notificationCreatedAt = new Date(notification.created_at); + const topicBumpedAt = new Date(topic.bumped_at); + return topicBumpedAt > notificationCreatedAt; + }).forEach((item) => { + if (item instanceof Notification) { content.push( new UserMenuNotificationItem({ - notification: readNotifications[0], + notification: item, currentUser: this.currentUser, siteSettings: this.siteSettings, site: this.site, }) ); - readNotifications.shift(); - latestReadNotificationDate = parseDateString( - readNotifications[0]?.created_at - ); + } else { + content.push(new UserMenuMessageItem({ message: item })); } - } + }); + return content; } diff --git a/app/assets/javascripts/discourse/app/components/user-menu/notifications-list.js b/app/assets/javascripts/discourse/app/components/user-menu/notifications-list.js index a585b5a06fc..efe1cadf523 100644 --- a/app/assets/javascripts/discourse/app/components/user-menu/notifications-list.js +++ b/app/assets/javascripts/discourse/app/components/user-menu/notifications-list.js @@ -2,11 +2,16 @@ import UserMenuItemsList from "discourse/components/user-menu/items-list"; import I18n from "I18n"; import { action } from "@ember/object"; import { ajax } from "discourse/lib/ajax"; -import { postRNWebviewMessage } from "discourse/lib/utilities"; +import { + mergeSortedLists, + postRNWebviewMessage, +} from "discourse/lib/utilities"; import showModal from "discourse/lib/show-modal"; import { inject as service } from "@ember/service"; import UserMenuNotificationItem from "discourse/lib/user-menu/notification-item"; import Notification from "discourse/models/notification"; +import UserMenuReviewable from "discourse/models/user-menu-reviewable"; +import UserMenuReviewableItem from "discourse/lib/user-menu/reviewable-item"; export default class UserMenuNotificationsList extends UserMenuItemsList { @service currentUser; @@ -31,7 +36,11 @@ export default class UserMenuNotificationsList extends UserMenuItemsList { } get showDismiss() { - return this.items.some((item) => !item.notification.read); + return Object.keys( + this.currentUser.get("grouped_unread_notifications") || {} + ).any((key) => { + return this.currentUser.get(`grouped_unread_notifications.${key}`) > 0; + }); } get dismissTitle() { @@ -60,27 +69,70 @@ export default class UserMenuNotificationsList extends UserMenuItemsList { limit: 30, recent: true, bump_last_seen_reviewable: true, - silent: this.currentUser.enforcedSecondFactor, }; + if (this.currentUser.enforcedSecondFactor) { + params.silent = true; + } + const types = this.filterByTypes; if (types?.length > 0) { params.filter_by_types = types.join(","); params.silent = true; } - const collection = await this.store - .findStale("notification", params) - .refresh(); - const notifications = collection.content; - await Notification.applyTransformations(notifications); - return notifications.map((notification) => { - return new UserMenuNotificationItem({ - notification, - currentUser: this.currentUser, - siteSettings: this.siteSettings, - site: this.site, + + const content = []; + const data = await ajax("/notifications", { data: params }); + + const notifications = await Notification.initializeNotifications( + data.notifications + ); + + const reviewables = data.pending_reviewables?.map((r) => + UserMenuReviewable.create(r) + ); + + if (reviewables?.length) { + const firstReadNotificationIndex = notifications.findIndex((n) => n.read); + const unreadNotifications = notifications.splice( + 0, + firstReadNotificationIndex + ); + mergeSortedLists( + unreadNotifications, + reviewables, + (notification, reviewable) => { + const notificationCreatedAt = new Date(notification.created_at); + const reviewableCreatedAt = new Date(reviewable.created_at); + return reviewableCreatedAt > notificationCreatedAt; + } + ).forEach((item) => { + const props = { + currentUser: this.currentUser, + siteSettings: this.siteSettings, + site: this.site, + }; + if (item instanceof Notification) { + props.notification = item; + content.push(new UserMenuNotificationItem(props)); + } else { + props.reviewable = item; + content.push(new UserMenuReviewableItem(props)); + } }); + } + + notifications.forEach((notification) => { + content.push( + new UserMenuNotificationItem({ + notification, + currentUser: this.currentUser, + siteSettings: this.siteSettings, + site: this.site, + }) + ); }); + return content; } dismissWarningModal() { diff --git a/app/assets/javascripts/discourse/app/lib/utilities.js b/app/assets/javascripts/discourse/app/lib/utilities.js index a54b3c6c674..d2ae39cdea6 100644 --- a/app/assets/javascripts/discourse/app/lib/utilities.js +++ b/app/assets/javascripts/discourse/app/lib/utilities.js @@ -606,5 +606,31 @@ function clipboardCopyFallback(text) { return success; } +// this function takes 2 sorted lists and returns another sorted list that +// contains both of the original lists. +// you need to provide a callback as the 3rd argument that will be called with +// an item from the first list (1st callback argument) and another item from +// the second list (2nd callback argument). The callback should return true if +// its 2nd argument should go before its 1st argument and return false +// otherwise. +export function mergeSortedLists(list1, list2, comparator) { + let index1 = 0; + let index2 = 0; + const merged = []; + while (index1 < list1.length || index2 < list2.length) { + if ( + index1 === list1.length || + (index2 < list2.length && comparator(list1[index1], list2[index2])) + ) { + merged.push(list2[index2]); + index2++; + } else { + merged.push(list1[index1]); + index1++; + } + } + return merged; +} + // This prevents a mini racer crash export default {}; diff --git a/app/assets/javascripts/discourse/app/models/notification.js b/app/assets/javascripts/discourse/app/models/notification.js index 000aa0c10ae..b49d8003c2f 100644 --- a/app/assets/javascripts/discourse/app/models/notification.js +++ b/app/assets/javascripts/discourse/app/models/notification.js @@ -7,5 +7,11 @@ export default class Notification extends RestModel { await applyModelTransformations("notification", notifications); } + static async initializeNotifications(rawList) { + const notifications = rawList.map((n) => this.create(n)); + await this.applyTransformations(notifications); + return notifications; + } + @tracked read; } diff --git a/app/assets/javascripts/discourse/tests/integration/components/user-menu/notifications-list-test.js b/app/assets/javascripts/discourse/tests/integration/components/user-menu/notifications-list-test.js index cf1baf30b83..1c8c6e4fd83 100644 --- a/app/assets/javascripts/discourse/tests/integration/components/user-menu/notifications-list-test.js +++ b/app/assets/javascripts/discourse/tests/integration/components/user-menu/notifications-list-test.js @@ -1,10 +1,11 @@ import { module, test } from "qunit"; import { setupRenderingTest } from "discourse/tests/helpers/component-test"; -import { exists, query } from "discourse/tests/helpers/qunit-helpers"; +import { exists, query, queryAll } from "discourse/tests/helpers/qunit-helpers"; import { click, render } from "@ember/test-helpers"; import { cloneJSON } from "discourse-common/lib/object"; import NotificationFixtures from "discourse/tests/fixtures/notification-fixtures"; import { hbs } from "ember-cli-htmlbars"; +import { NOTIFICATION_TYPES } from "discourse/tests/fixtures/concerns/notification-types"; import pretender, { response } from "discourse/tests/helpers/create-pretender"; import I18n from "I18n"; @@ -78,11 +79,10 @@ module( ); }); - test("has a dismiss button if some notifications are not read", async function (assert) { - notificationsData.forEach((notification) => { - notification.read = true; + test("has a dismiss button if some notification types have unread notifications", async function (assert) { + this.currentUser.set("grouped_unread_notifications", { + [NOTIFICATION_TYPES.mentioned]: 1, }); - notificationsData[0].read = false; await render(template); const dismissButton = query( ".panel-body-bottom .btn.notifications-dismiss" @@ -109,9 +109,8 @@ module( test("dismiss button makes a request to the server and then refreshes the notifications list", async function (assert) { await render(template); - notificationsData = getNotificationsData(); - notificationsData.forEach((notification) => { - notification.read = true; + this.currentUser.set("grouped_unread_notifications", { + [NOTIFICATION_TYPES.mentioned]: 1, }); assert.strictEqual(notificationsFetches, 1); await click(".panel-body-bottom .btn.notifications-dismiss"); @@ -126,5 +125,114 @@ module( "dismiss button is not shown" ); }); + + test("all notifications tab shows pending reviewables and sorts them with unread notifications based on their creation date", async function (assert) { + pretender.get("/notifications", () => { + return response({ + notifications: [ + { + id: 6, + user_id: 1, + notification_type: NOTIFICATION_TYPES.mentioned, + read: false, + high_priority: false, + created_at: "2021-11-25T19:31:13.241Z", + post_number: 6, + topic_id: 10, + fancy_title: "Unread notification #01", + slug: "unread-notification-01", + data: { + topic_title: "Unread notification #01", + original_post_id: 20, + original_post_type: 1, + original_username: "discobot", + revision_number: null, + display_username: "discobot", + }, + }, + { + id: 13, + user_id: 1, + notification_type: NOTIFICATION_TYPES.replied, + read: false, + high_priority: false, + created_at: "2021-08-25T19:31:13.241Z", + post_number: 6, + topic_id: 10, + fancy_title: "Unread notification #02", + slug: "unread-notification-02", + data: { + topic_title: "Unread notification #02", + original_post_id: 20, + original_post_type: 1, + original_username: "discobot", + revision_number: null, + display_username: "discobot", + }, + }, + { + id: 81, + user_id: 1, + notification_type: NOTIFICATION_TYPES.mentioned, + read: true, + high_priority: false, + created_at: "2022-10-25T19:31:13.241Z", + post_number: 6, + topic_id: 10, + fancy_title: "Read notification #01", + slug: "read-notification-01", + data: { + topic_title: "Read notification #01", + original_post_id: 20, + original_post_type: 1, + original_username: "discobot", + revision_number: null, + display_username: "discobot", + }, + }, + ], + pending_reviewables: [ + { + flagger_username: "sayo2", + id: 83, + pending: true, + topic_fancy_title: "anything hello world 0011", + type: "ReviewableQueuedPost", + created_at: "2022-09-25T19:31:13.241Z", + }, + { + flagger_username: "sayo2", + id: 78, + pending: true, + topic_fancy_title: "anything hello world 0033", + type: "ReviewableQueuedPost", + created_at: "2021-06-25T19:31:13.241Z", + }, + ], + }); + }); + await render(template); + const items = queryAll("ul li"); + assert.ok( + items[0].textContent.includes("hello world 0011"), + "the first pending reviewable is displayed 1st because it's most recent among pending reviewables and unread notifications" + ); + assert.ok( + items[1].textContent.includes("Unread notification #01"), + "the first unread notification is displayed 2nd because it's the 2nd most recent among pending reviewables and unread notifications" + ); + assert.ok( + items[2].textContent.includes("Unread notification #02"), + "the second unread notification is displayed 3rd because it's the 3rd most recent among pending reviewables and unread notifications" + ); + assert.ok( + items[3].textContent.includes("hello world 0033"), + "the second pending reviewable is displayed 4th because it's the 4th most recent among pending reviewables and unread notifications" + ); + assert.ok( + items[4].textContent.includes("Read notification #01"), + "read notifications come after the pending reviewables and unread notifications" + ); + }); } ); diff --git a/app/assets/javascripts/discourse/tests/unit/lib/utilities-test.js b/app/assets/javascripts/discourse/tests/unit/lib/utilities-test.js index ebc2f66d705..6bb987c852c 100644 --- a/app/assets/javascripts/discourse/tests/unit/lib/utilities-test.js +++ b/app/assets/javascripts/discourse/tests/unit/lib/utilities-test.js @@ -12,6 +12,7 @@ import { getRawSize, inCodeBlock, initializeDefaultHomepage, + mergeSortedLists, setCaretPosition, setDefaultHomepage, slugify, @@ -288,6 +289,45 @@ discourseModule("Unit | Utilities", function () { } }); }); + + test("mergeSortedLists", function (assert) { + const comparator = (a, b) => b > a; + assert.deepEqual( + mergeSortedLists([], [1, 2, 3], comparator), + [1, 2, 3], + "it doesn't error when the first list is blank" + ); + assert.deepEqual( + mergeSortedLists([3, 2, 1], [], comparator), + [3, 2, 1], + "it doesn't error when the second list is blank" + ); + assert.deepEqual( + mergeSortedLists([], [], comparator), + [], + "it doesn't error when the both lists are blank" + ); + assert.deepEqual( + mergeSortedLists([5, 4, 0, -1], [1], comparator), + [5, 4, 1, 0, -1], + "it correctly merges lists when one list has 1 item only" + ); + assert.deepEqual( + mergeSortedLists([2], [1], comparator), + [2, 1], + "it correctly merges lists when both lists has 1 item each" + ); + assert.deepEqual( + mergeSortedLists([1], [1], comparator), + [1, 1], + "it correctly merges lists when both lists has 1 item and their items are identical" + ); + assert.deepEqual( + mergeSortedLists([5, 4, 3, 2, 1], [6, 2, 1], comparator), + [6, 5, 4, 3, 2, 2, 1, 1], + "it correctly merges lists that share common items" + ); + }); }); discourseModule("Unit | Utilities | clipboard", function (hooks) { diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb index a33964575d7..5566dd488e3 100644 --- a/app/controllers/notifications_controller.rb +++ b/app/controllers/notifications_controller.rb @@ -30,8 +30,11 @@ class NotificationsController < ApplicationController limit = (params[:limit] || 15).to_i limit = 50 if limit > 50 + include_reviewables = false if SiteSetting.enable_experimental_sidebar_hamburger notifications = Notification.prioritized_list(current_user, count: limit, types: notification_types) + # notification_types is blank for the "all notifications" user menu tab + include_reviewables = notification_types.blank? && guardian.can_see_review_queue? else notifications = Notification.recent_report(current_user, limit, notification_types) end @@ -43,26 +46,28 @@ class NotificationsController < ApplicationController end end - if !params.has_key?(:silent) && params[:bump_last_seen_reviewable] && !@readonly_mode + if !params.has_key?(:silent) && params[:bump_last_seen_reviewable] && !@readonly_mode && include_reviewables 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 + # concurrency issues where the user object returned by the + # current_user controller method is changed by the time the deferred + # block is executed + User.find_by(id: current_user_id)&.bump_last_seen_reviewable! end end - render_json_dump( + json = { notifications: serialize_data(notifications, NotificationSerializer), seen_notification_id: current_user.seen_notification_id - ) + } + if include_reviewables + json[:pending_reviewables] = Reviewable.basic_serializers_for_list( + Reviewable.user_menu_list_for(current_user), + current_user + ).as_json + end + render_json_dump(json) else offset = params[:offset].to_i diff --git a/app/controllers/reviewables_controller.rb b/app/controllers/reviewables_controller.rb index 06d87927e4b..c6f0a35faee 100644 --- a/app/controllers/reviewables_controller.rb +++ b/app/controllers/reviewables_controller.rb @@ -71,9 +71,11 @@ class ReviewablesController < ApplicationController end def user_menu_list - reviewables = Reviewable.list_for(current_user, limit: 30, status: :pending).to_a json = { - reviewables: reviewables.map! { |r| r.basic_serializer.new(r, scope: guardian, root: nil).as_json } + reviewables: Reviewable.basic_serializers_for_list( + Reviewable.user_menu_list_for(current_user), + current_user + ).as_json } render_json_dump(json, rest_serializer: true) end diff --git a/app/models/reviewable.rb b/app/models/reviewable.rb index b8f57b2b1fc..b955e77af0d 100644 --- a/app/models/reviewable.rb +++ b/app/models/reviewable.rb @@ -534,6 +534,14 @@ class Reviewable < ActiveRecord::Base results end + def self.user_menu_list_for(user, limit: 30) + list_for(user, limit: limit, status: :pending).to_a + end + + def self.basic_serializers_for_list(reviewables, user) + reviewables.map { |r| r.basic_serializer.new(r, scope: user.guardian, root: nil) } + end + def serializer self.class.serializer_for(self) end diff --git a/app/serializers/basic_reviewable_serializer.rb b/app/serializers/basic_reviewable_serializer.rb index 35d678fd43f..a2411c799f8 100644 --- a/app/serializers/basic_reviewable_serializer.rb +++ b/app/serializers/basic_reviewable_serializer.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class BasicReviewableSerializer < ApplicationSerializer - attributes :flagger_username, :id, :type, :pending + attributes :flagger_username, :id, :type, :pending, :created_at def flagger_username object.created_by&.username diff --git a/spec/requests/notifications_controller_spec.rb b/spec/requests/notifications_controller_spec.rb index 2bade5ac1e4..f263fef37da 100644 --- a/spec/requests/notifications_controller_spec.rb +++ b/spec/requests/notifications_controller_spec.rb @@ -87,60 +87,6 @@ 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) @@ -202,6 +148,7 @@ RSpec.describe NotificationsController do created_at: 4.minutes.ago ) end + fab!(:pending_reviewable) { Fabricate(:reviewable) } it "gets notifications list with unread ones at the top when the setting is enabled" do SiteSetting.enable_experimental_sidebar_hamburger = true @@ -228,6 +175,93 @@ RSpec.describe NotificationsController do read_high_priority.id ]) end + + it "should not bump last seen reviewable in readonly mode" do + SiteSetting.enable_experimental_sidebar_hamburger = true + user.update!(admin: true) + Discourse.received_redis_readonly! + 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 } + ensure + Discourse.clear_redis_readonly! + end + + it "should not bump last seen reviewable if the user can't see reviewables" do + SiteSetting.enable_experimental_sidebar_hamburger = true + 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 + SiteSetting.enable_experimental_sidebar_hamburger = true + user.update!(admin: true) + 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 + SiteSetting.enable_experimental_sidebar_hamburger = true + user.update!(admin: true) + expect { + get "/notifications.json", params: { recent: true } + expect(response.status).to eq(200) + }.not_to change { user.reload.last_seen_reviewable_id } + end + + it "bumps last_seen_reviewable_id" do + SiteSetting.enable_experimental_sidebar_hamburger = true + user.update!(admin: true) + expect(user.last_seen_reviewable_id).to eq(nil) + get "/notifications.json", params: { recent: true, bump_last_seen_reviewable: true } + expect(user.reload.last_seen_reviewable_id).to eq(pending_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 "includes pending reviewables when the setting is enabled" do + SiteSetting.enable_experimental_sidebar_hamburger = true + user.update!(admin: true) + pending_reviewable2 = Fabricate(:reviewable, created_at: 4.minutes.ago) + Fabricate(:reviewable, status: Reviewable.statuses[:approved]) + Fabricate(:reviewable, status: Reviewable.statuses[:rejected]) + + get "/notifications.json", params: { recent: true } + expect(response.status).to eq(200) + expect(response.parsed_body["pending_reviewables"].map { |r| r["id"] }).to eq([ + pending_reviewable.id, + pending_reviewable2.id + ]) + end + + it "doesn't include reviewables when the setting is disabled" do + SiteSetting.enable_experimental_sidebar_hamburger = false + user.update!(admin: true) + + get "/notifications.json", params: { recent: true } + expect(response.status).to eq(200) + expect(response.parsed_body.key?("pending_reviewables")).to eq(false) + end + + it "doesn't include reviewables if the user can't see the review queue" do + SiteSetting.enable_experimental_sidebar_hamburger = true + user.update!(admin: false) + + get "/notifications.json", params: { recent: true } + expect(response.status).to eq(200) + expect(response.parsed_body.key?("pending_reviewables")).to eq(false) + end end context "when filter_by_types param is present" do diff --git a/spec/support/common_basic_reviewable_serializer.rb b/spec/support/common_basic_reviewable_serializer.rb index c439ddb15bb..76680076c4e 100644 --- a/spec/support/common_basic_reviewable_serializer.rb +++ b/spec/support/common_basic_reviewable_serializer.rb @@ -38,4 +38,12 @@ shared_examples "basic reviewable attributes" do expect(subject[:flagger_username]).to eq("gg.osama") end end + + describe "#created_at" do + it "serializes the reviewable's created_at field correctly" do + time = 10.minutes.ago + reviewable.update!(created_at: time) + expect(subject[:created_at]).to eq(time) + end + end end