DEV: Enable new user menu when experimental sidebar hamburger is enabled (#18133)

When `enable_experimental_sidebar_hamburger` site setting is enabled, we
will switch to rendering the new user menu.
This commit is contained in:
Alan Guo Xiang Tan 2022-09-01 02:15:01 +08:00 committed by GitHub
parent bbf00fd087
commit 0f0048e8e3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 188 additions and 155 deletions

View File

@ -31,35 +31,37 @@ class Jobs::NotifyReviewable < ::Jobs::Base
counts[r.reviewable_by_group_id] += 1 if r.reviewable_by_group_id counts[r.reviewable_by_group_id] += 1 if r.reviewable_by_group_id
end end
redesigned_menu_enabled_user_ids = User.redesigned_user_menu_enabled_user_ids if SiteSetting.enable_experimental_sidebar_hamburger
notify_users(
new_menu_admins = User.real.admins.where(id: redesigned_menu_enabled_user_ids) User.real.admins,
notify_users(new_menu_admins, all_updates[:admins]) all_updates[:admins]
)
legacy_menu_admins = User.real.admins.where("id NOT IN (?)", @contacted).pluck(:id) else
notify_legacy( notify_legacy(
legacy_menu_admins, User.real.admins.pluck(:id),
count: counts[:admins], count: counts[:admins],
updates: all_updates[:admins], updates: all_updates[:admins],
) )
end
if reviewable.reviewable_by_moderator? if reviewable.reviewable_by_moderator?
new_menu_mods = User if SiteSetting.enable_experimental_sidebar_hamburger
.real notify_users(
.moderators User.real.moderators.where("id NOT IN (?)", @contacted),
.where("id IN (?)", redesigned_menu_enabled_user_ids - @contacted.to_a) all_updates[:moderators]
notify_users(new_menu_mods, all_updates[:moderators]) )
else
legacy_menu_mods = User.real.moderators.where("id NOT IN (?)", @contacted).pluck(:id) notify_legacy(
notify_legacy( User.real.moderators.where("id NOT IN (?)", @contacted).pluck(:id),
legacy_menu_mods, count: counts[:moderators],
count: counts[:moderators], updates: all_updates[:moderators],
updates: all_updates[:moderators], )
) end
end end
if SiteSetting.enable_category_group_moderation? && (group = reviewable.reviewable_by_group) if SiteSetting.enable_category_group_moderation? && (group = reviewable.reviewable_by_group)
users = group.users.includes(:group_users).where("users.id NOT IN (?)", @contacted) users = group.users.includes(:group_users).where("users.id NOT IN (?)", @contacted)
users.find_each do |user| users.find_each do |user|
count = 0 count = 0
updates = {} updates = {}
@ -67,12 +69,14 @@ class Jobs::NotifyReviewable < ::Jobs::Base
updates.merge!(all_updates[gu.group_id]) updates.merge!(all_updates[gu.group_id])
count += counts[gu.group_id] count += counts[gu.group_id]
end end
if redesigned_menu_enabled_user_ids.include?(user.id)
if SiteSetting.enable_experimental_sidebar_hamburger
notify_user(user, updates) notify_user(user, updates)
else else
notify_legacy([user.id], count: count, updates: updates) notify_legacy([user.id], count: count, updates: updates)
end end
end end
@contacted += users.pluck(:id) @contacted += users.pluck(:id)
end end
end end

View File

@ -1639,24 +1639,8 @@ class User < ActiveRecord::Base
user_status && !user_status.expired? user_status && !user_status.expired?
end end
REDESIGN_USER_MENU_REDIS_KEY_PREFIX = "redesigned_user_menu_for_user_"
def self.redesigned_user_menu_enabled_user_ids
Discourse.redis.scan_each(match: "#{REDESIGN_USER_MENU_REDIS_KEY_PREFIX}*").map do |key|
key.sub(REDESIGN_USER_MENU_REDIS_KEY_PREFIX, "").to_i
end
end
def redesigned_user_menu_enabled? def redesigned_user_menu_enabled?
Discourse.redis.get("#{REDESIGN_USER_MENU_REDIS_KEY_PREFIX}#{self.id}") == "1" SiteSetting.enable_experimental_sidebar_hamburger
end
def enable_redesigned_user_menu
Discourse.redis.setex("#{REDESIGN_USER_MENU_REDIS_KEY_PREFIX}#{self.id}", 6.months, "1")
end
def disable_redesigned_user_menu
Discourse.redis.del("#{REDESIGN_USER_MENU_REDIS_KEY_PREFIX}#{self.id}")
end end
def sidebar_categories_ids def sidebar_categories_ids

View File

@ -324,10 +324,7 @@ class CurrentUserSerializer < BasicUserSerializer
end end
def redesigned_user_menu_enabled def redesigned_user_menu_enabled
if defined?(@redesigned_user_menu_enabled) object.redesigned_user_menu_enabled?
return @redesigned_user_menu_enabled
end
@redesigned_user_menu_enabled = object.redesigned_user_menu_enabled?
end end
def likes_notifications_disabled def likes_notifications_disabled

View File

@ -4,133 +4,167 @@ RSpec.describe Jobs::NotifyReviewable do
# remove all the legacy stuff here when redesigned_user_menu_enabled is # remove all the legacy stuff here when redesigned_user_menu_enabled is
# removed # removed
describe '#execute' do describe '#execute' do
fab!(:legacy_menu_admin) { Fabricate(:admin, moderator: true) } fab!(:admin) { Fabricate(:admin, moderator: true) }
fab!(:legacy_menu_mod) { Fabricate(:moderator) } fab!(:moderator) { Fabricate(:moderator) }
fab!(:group_user) { Fabricate(:group_user) } fab!(:group_user) { Fabricate(:group_user) }
fab!(:legacy_menu_user) { group_user.user }
fab!(:group) { group_user.group } fab!(:group) { group_user.group }
fab!(:user) { group_user.user }
fab!(:new_menu_admin) { Fabricate(:admin, moderator: true) } it "will notify users of new reviewable content for the new user menu" do
fab!(:new_menu_mod) { Fabricate(:moderator) } SiteSetting.enable_experimental_sidebar_hamburger = true
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 SiteSetting.enable_category_group_moderation = true
GroupUser.create!(group_id: group.id, user_id: legacy_menu_mod.id) GroupUser.create!(group_id: group.id, user_id: moderator.id)
# Content for admins only # Content for admins only
admin_reviewable = Fabricate(:reviewable, reviewable_by_moderator: false) admin_reviewable = Fabricate(:reviewable, reviewable_by_moderator: false)
new_menu_admin.update!(last_seen_reviewable_id: admin_reviewable.id) admin.update!(last_seen_reviewable_id: admin_reviewable.id)
messages = MessageBus.track_publish do messages = MessageBus.track_publish do
described_class.new.execute(reviewable_id: admin_reviewable.id) described_class.new.execute(reviewable_id: admin_reviewable.id)
end end
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(messages.size).to eq(1)
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) admin_message = messages.first
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(admin_message.channel).to eq("/reviewable_counts/#{admin.id}")
expect(messages.any? { |m| m.user_ids.include?(new_menu_user.id) }).to eq(false) expect(admin_message.user_ids).to eq([admin.id])
expect(admin_message.data[:reviewable_count]).to eq(1)
expect(admin_message.data[:unseen_reviewable_count]).to eq(0)
# Content for moderators # Content for moderators
mod_reviewable = Fabricate(:reviewable, reviewable_by_moderator: true) moderator_reviewable = Fabricate(:reviewable, reviewable_by_moderator: true)
messages = MessageBus.track_publish do messages = MessageBus.track_publish do
described_class.new.execute(reviewable_id: mod_reviewable.id) described_class.new.execute(reviewable_id: moderator_reviewable.id)
end end
expect(messages.size).to eq(4) expect(messages.size).to eq(2)
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] } admin_message = messages.find { |m| m.user_ids == [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(admin_message.channel).to eq("/reviewable_counts/#{admin.id}")
expect(legacy_menu_mod_msg.data[:reviewable_count]).to eq(1) expect(admin_message.data[:reviewable_count]).to eq(2)
expect(legacy_menu_mod_msg.channel).to eq("/reviewable_counts") expect(admin_message.data[:unseen_reviewable_count]).to eq(1)
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] } moderator_message = messages.find { |m| m.user_ids == [moderator.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(moderator_message.channel).to eq("/reviewable_counts/#{moderator.id}")
expect(messages.any? { |m| m.user_ids.include?(new_menu_user.id) }).to eq(false) expect(moderator_message.data[:reviewable_count]).to eq(1)
expect(moderator_message.data[:unseen_reviewable_count]).to eq(1)
new_menu_mod.update!(last_seen_reviewable_id: mod_reviewable.id) moderator.update!(last_seen_reviewable_id: moderator_reviewable.id)
# Content for a group # Content for a group
group_reviewable = Fabricate(:reviewable, reviewable_by_moderator: true, reviewable_by_group: group) group_reviewable = Fabricate(:reviewable, reviewable_by_moderator: true, reviewable_by_group: group)
messages = MessageBus.track_publish do messages = MessageBus.track_publish do
described_class.new.execute(reviewable_id: group_reviewable.id) described_class.new.execute(reviewable_id: group_reviewable.id)
end end
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(messages.size).to eq(3)
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] } admin_message = messages.find { |m| m.user_ids == [admin.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(admin_message.channel).to eq("/reviewable_counts/#{admin.id}")
expect(new_menu_mod_msg.data[:reviewable_count]).to eq(2) expect(admin_message.data[:reviewable_count]).to eq(3)
expect(new_menu_mod_msg.channel).to eq("/reviewable_counts/#{new_menu_mod.id}") expect(admin_message.data[:unseen_reviewable_count]).to eq(2)
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] } moderator_message = messages.find { |m| m.user_ids == [moderator.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(moderator_message.channel).to eq("/reviewable_counts/#{moderator.id}")
expect(new_menu_user_msg.data[:reviewable_count]).to eq(1) expect(moderator_message.data[:reviewable_count]).to eq(2)
expect(new_menu_user_msg.channel).to eq("/reviewable_counts/#{new_menu_user.id}") expect(moderator_message.data[:unseen_reviewable_count]).to eq(1)
expect(new_menu_user_msg.data[:unseen_reviewable_count]).to eq(1)
group_user_message = messages.find { |m| m.user_ids == [user.id] }
expect(group_user_message.channel).to eq("/reviewable_counts/#{user.id}")
expect(group_user_message.data[:reviewable_count]).to eq(1)
expect(group_user_message.data[:unseen_reviewable_count]).to eq(1)
end
it "will notify users of new reviewable content for the old user menu" do
SiteSetting.enable_experimental_sidebar_hamburger = false
SiteSetting.enable_category_group_moderation = true
GroupUser.create!(group_id: group.id, user_id: moderator.id)
# Content for admins only
admin_reviewable = Fabricate(:reviewable, reviewable_by_moderator: false)
admin.update!(last_seen_reviewable_id: admin_reviewable.id)
messages = MessageBus.track_publish do
described_class.new.execute(reviewable_id: admin_reviewable.id)
end
expect(messages.size).to eq(1)
admin_message = messages.first
expect(admin_message.channel).to eq("/reviewable_counts")
expect(admin_message.user_ids).to eq([admin.id])
expect(admin_message.data[:reviewable_count]).to eq(1)
expect(admin_message.data.has_key?(:unseen_reviewable_count)).to eq(false)
# Content for moderators
moderator_reviewable = Fabricate(:reviewable, reviewable_by_moderator: true)
messages = MessageBus.track_publish do
described_class.new.execute(reviewable_id: moderator_reviewable.id)
end
expect(messages.size).to eq(2)
admin_message = messages.find { |m| m.user_ids == [admin.id] }
expect(admin_message.channel).to eq("/reviewable_counts")
expect(admin_message.data[:reviewable_count]).to eq(2)
expect(admin_message.data.has_key?(:unseen_reviewable_count)).to eq(false)
moderator_message = messages.find { |m| m.user_ids == [moderator.id] }
expect(moderator_message.channel).to eq("/reviewable_counts")
expect(moderator_message.data[:reviewable_count]).to eq(1)
expect(moderator_message.data.key?(:unseen_reviewable_count)).to eq(false)
moderator.update!(last_seen_reviewable_id: moderator_reviewable.id)
# Content for a group
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
expect(messages.size).to eq(3)
admin_message = messages.find { |m| m.user_ids == [admin.id] }
expect(admin_message.data[:reviewable_count]).to eq(3)
expect(admin_message.channel).to eq("/reviewable_counts")
expect(admin_message.data.key?(:unseen_reviewable_count)).to eq(false)
moderator_message = messages.find { |m| m.user_ids == [moderator.id] }
expect(moderator_message.data[:reviewable_count]).to eq(2)
expect(moderator_message.channel).to eq("/reviewable_counts")
expect(moderator_message.data.key?(:unseen_reviewable_count)).to eq(false)
group_user_message = messages.find { |m| m.user_ids == [user.id] }
expect(group_user_message.data[:reviewable_count]).to eq(1)
expect(group_user_message.channel).to eq("/reviewable_counts")
expect(group_user_message.data.key?(:unseen_reviewable_count)).to eq(false)
end end
it "won't notify a group when disabled" do it "won't notify a group when disabled" do
SiteSetting.enable_category_group_moderation = false SiteSetting.enable_category_group_moderation = false
GroupUser.create!(group_id: group.id, user_id: legacy_menu_mod.id) GroupUser.create!(group_id: group.id, user_id: moderator.id)
GroupUser.create!(group_id: group.id, user_id: new_menu_mod.id) reviewable = Fabricate(:reviewable, reviewable_by_moderator: true, reviewable_by_group: group)
r3 = Fabricate(:reviewable, reviewable_by_moderator: true, reviewable_by_group: group)
messages = MessageBus.track_publish("/reviewable_counts") do messages = MessageBus.track_publish("/reviewable_counts") do
described_class.new.execute(reviewable_id: r3.id) described_class.new.execute(reviewable_id: reviewable.id)
end end
group_msg = messages.find { |m| m.user_ids.include?(legacy_menu_user.id) }
expect(group_msg).to be_blank group_user_message = messages.find { |m| m.user_ids.include?(user.id) }
group_msg = messages.find { |m| m.user_ids.include?(new_menu_user.id) }
expect(group_msg).to be_blank expect(group_user_message).to be_blank
end end
it "respects priority" do it "respects priority" do
@ -138,38 +172,48 @@ RSpec.describe Jobs::NotifyReviewable do
Reviewable.set_priorities(medium: 2.0) Reviewable.set_priorities(medium: 2.0)
SiteSetting.reviewable_default_visibility = 'medium' SiteSetting.reviewable_default_visibility = 'medium'
GroupUser.create!(group_id: group.id, user_id: legacy_menu_mod.id) GroupUser.create!(group_id: group.id, user_id: moderator.id)
# Content for admins only # Content for admins only
r1 = Fabricate(:reviewable, reviewable_by_moderator: false) admin_reviewable = Fabricate(:reviewable, reviewable_by_moderator: false)
messages = MessageBus.track_publish("/reviewable_counts") do messages = MessageBus.track_publish("/reviewable_counts") do
described_class.new.execute(reviewable_id: r1.id) described_class.new.execute(reviewable_id: admin_reviewable.id)
end end
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) admin_message = messages.find { |m| m.user_ids.include?(admin.id) }
expect(admin_message.data[:reviewable_count]).to eq(0)
# Content for moderators # Content for moderators
r2 = Fabricate(:reviewable, reviewable_by_moderator: true) moderator_reviewable = Fabricate(:reviewable, reviewable_by_moderator: true)
messages = MessageBus.track_publish("/reviewable_counts") do messages = MessageBus.track_publish("/reviewable_counts") do
described_class.new.execute(reviewable_id: r2.id) described_class.new.execute(reviewable_id: moderator_reviewable.id)
end end
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) admin_message = messages.find { |m| m.user_ids.include?(admin.id) }
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) expect(admin_message.data[:reviewable_count]).to eq(0)
moderator_message = messages.find { |m| m.user_ids.include?(moderator.id) }
expect(moderator_message.data[:reviewable_count]).to eq(0)
# Content for a group # Content for a group
r3 = Fabricate(:reviewable, reviewable_by_moderator: true, reviewable_by_group: group) group_reviewable = Fabricate(:reviewable, reviewable_by_moderator: true, reviewable_by_group: group)
messages = MessageBus.track_publish("/reviewable_counts") do messages = MessageBus.track_publish("/reviewable_counts") do
described_class.new.execute(reviewable_id: r3.id) described_class.new.execute(reviewable_id: group_reviewable.id)
end end
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) admin_message = messages.find { |m| m.user_ids.include?(admin.id) }
mod_messages = messages.select { |m| m.user_ids.include?(legacy_menu_mod.id) } expect(admin_message.data[:reviewable_count]).to eq(0)
expect(mod_messages.size).to eq(1)
expect(mod_messages[0].data[:reviewable_count]).to eq(0) moderator_messages = messages.select { |m| m.user_ids.include?(moderator.id) }
group_msg = messages.find { |m| m.user_ids.include?(legacy_menu_user.id) } expect(moderator_messages.size).to eq(1)
expect(group_msg.data[:reviewable_count]).to eq(0) expect(moderator_messages[0].data[:reviewable_count]).to eq(0)
group_user_message = messages.find { |m| m.user_ids.include?(user.id) }
expect(group_user_message.data[:reviewable_count]).to eq(0)
end end
end end

View File

@ -2067,21 +2067,25 @@ RSpec.describe User do
end end
context "with redesigned_user_menu_enabled on" do context "with redesigned_user_menu_enabled on" do
before do
SiteSetting.enable_experimental_sidebar_hamburger = true
end
it "adds all_unread_notifications and grouped_unread_notifications to the payload" do it "adds all_unread_notifications and grouped_unread_notifications to the payload" do
user.update!(admin: true) user.update!(admin: true)
user.enable_redesigned_user_menu
Fabricate(:notification, user: user, notification_type: 1) Fabricate(:notification, user: user, notification_type: 1)
Fabricate(:notification, notification_type: 15, high_priority: true, read: false, user: user) Fabricate(:notification, notification_type: 15, high_priority: true, read: false, user: user)
messages = MessageBus.track_publish("/notification/#{user.id}") do messages = MessageBus.track_publish("/notification/#{user.id}") do
user.publish_notifications_state user.publish_notifications_state
end end
expect(messages.size).to eq(1) expect(messages.size).to eq(1)
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_notifications]).to eq({ 1 => 1, 15 => 1 }) expect(message.data[:grouped_unread_notifications]).to eq({ 1 => 1, 15 => 1 })
ensure
user.disable_redesigned_user_menu
end end
end end
end end