From b3a1199493a5974353449e2b3f1b30d9eabbff59 Mon Sep 17 00:00:00 2001 From: Andrei Prigorshnev Date: Mon, 26 Feb 2024 13:40:48 +0000 Subject: [PATCH] FEATURE: Hide user status when user is hiding public profile and presence (#24300) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Users can hide their public profile and presence information by checking “Hide my public profile and presence features” on the `u/{username}/preferences/interface` page. In that case, we also don't want to return user status from the server. This work has been started in https://github.com/discourse/discourse/pull/23946. The current PR fixes all the remaining places in Core. Note that the actual fix is quite simple – https://github.com/discourse/discourse/pull/24300/commits/a5802f484db92ff3f7e535ed25e1591d1f19b03b. But we had a fair amount of duplication in the code responsible for the user status serialization, so I had to dry that up first. The refactoring as well as adding some additional tests is the main part of this PR. --- app/controllers/users_controller.rb | 13 ++-- app/models/user_search.rb | 1 + app/serializers/basic_user_serializer.rb | 2 + .../basic_user_with_status_serializer.rb | 13 ---- app/serializers/concerns/user_status_mixin.rb | 16 +++++ app/serializers/current_user_serializer.rb | 15 ++--- app/serializers/found_user_serializer.rb | 2 + .../found_user_with_status_serializer.rb | 11 ++-- app/serializers/group_user_serializer.rb | 16 +++-- ...roup_user_with_custom_fields_serializer.rb | 15 ++--- app/serializers/post_serializer.rb | 4 +- app/serializers/user_card_serializer.rb | 18 +++--- .../user_with_custom_fields_serializer.rb | 3 +- lib/presence_channel.rb | 2 +- .../chat/chatable_user_serializer.rb | 2 +- .../serializers/chat/chatables_serializer.rb | 8 ++- .../chat/direct_message_serializer.rb | 16 +++-- .../serializers/chat/message_serializer.rb | 11 ++-- .../chat/message_user_serializer.rb | 2 +- .../thread_original_message_serializer.rb | 10 ++- ...ith_custom_fields_and_status_serializer.rb | 21 ------- .../api/channel_threads_controller_spec.rb | 34 +++++----- .../chat/chat_message_serializer_spec.rb | 50 +++++++++++++++ .../chat/chatable_user_serializer_spec.rb | 22 +++++++ .../chat/direct_message_serializer_spec.rb | 16 +++-- ...thread_original_message_serializer_spec.rb | 62 +++++++++++++++++++ .../spec/services/chat/update_message_spec.rb | 2 +- .../serializers/basic_user_serializer_spec.rb | 60 +++++++++++++++++- .../basic_user_with_status_serializer_spec.rb | 44 ------------- .../concerns/user_status_mixin_spec.rb | 43 +++++++++++++ .../serializers/found_user_serializer_spec.rb | 55 ++++++++++++++++ ...user_with_custom_fields_serializer_spec.rb | 17 +++++ spec/serializers/post_serializer_spec.rb | 29 +++++++++ ...user_with_custom_fields_serializer_spec.rb | 17 +++++ 34 files changed, 480 insertions(+), 172 deletions(-) delete mode 100644 app/serializers/basic_user_with_status_serializer.rb create mode 100644 app/serializers/concerns/user_status_mixin.rb delete mode 100644 plugins/chat/app/serializers/chat/user_with_custom_fields_and_status_serializer.rb create mode 100644 plugins/chat/spec/serializer/chat/chatable_user_serializer_spec.rb create mode 100644 plugins/chat/spec/serializer/chat/thread_original_message_serializer_spec.rb delete mode 100644 spec/serializers/basic_user_with_status_serializer_spec.rb create mode 100644 spec/serializers/concerns/user_status_mixin_spec.rb create mode 100644 spec/serializers/group_user_with_custom_fields_serializer_spec.rb create mode 100644 spec/serializers/user_with_custom_fields_serializer_spec.rb diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 47540f46e70..50f502592cd 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -1237,7 +1237,7 @@ class UsersController < ApplicationController if usernames.blank? UserSearch.new(term, options).search else - User.where(username_lower: usernames).limit(limit) + User.where(username_lower: usernames).includes(:user_option).limit(limit) end to_render = serialize_found_users(results) @@ -2238,9 +2238,12 @@ class UsersController < ApplicationController end def serialize_found_users(users) - each_serializer = - SiteSetting.enable_user_status? ? FoundUserWithStatusSerializer : FoundUserSerializer - - { users: ActiveModel::ArraySerializer.new(users, each_serializer: each_serializer).as_json } + serializer = + ActiveModel::ArraySerializer.new( + users, + each_serializer: FoundUserSerializer, + include_status: true, + ) + { users: serializer.as_json } end end diff --git a/app/models/user_search.rb b/app/models/user_search.rb index 01b5c418b91..ba48c37ea5f 100644 --- a/app/models/user_search.rb +++ b/app/models/user_search.rb @@ -208,6 +208,7 @@ class UserSearch ) x on uid = users.id", ).order("rn") + results = results.includes(:user_option) results = results.includes(:user_status) if SiteSetting.enable_user_status results diff --git a/app/serializers/basic_user_serializer.rb b/app/serializers/basic_user_serializer.rb index 769002d0f00..5d9922600b3 100644 --- a/app/serializers/basic_user_serializer.rb +++ b/app/serializers/basic_user_serializer.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class BasicUserSerializer < ApplicationSerializer + include UserStatusMixin + attributes :id, :username, :name, :avatar_template def name diff --git a/app/serializers/basic_user_with_status_serializer.rb b/app/serializers/basic_user_with_status_serializer.rb deleted file mode 100644 index 8eb1a7ce2d6..00000000000 --- a/app/serializers/basic_user_with_status_serializer.rb +++ /dev/null @@ -1,13 +0,0 @@ -# frozen_string_literal: true - -class BasicUserWithStatusSerializer < BasicUserSerializer - attributes :status - - def include_status? - SiteSetting.enable_user_status && user.has_status? - end - - def status - UserStatusSerializer.new(user.user_status, root: false) - end -end diff --git a/app/serializers/concerns/user_status_mixin.rb b/app/serializers/concerns/user_status_mixin.rb new file mode 100644 index 00000000000..a0ffb7a952b --- /dev/null +++ b/app/serializers/concerns/user_status_mixin.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module UserStatusMixin + def self.included(klass) + klass.attributes :status + end + + def include_status? + @options[:include_status] && SiteSetting.enable_user_status && + !object.user_option&.hide_profile_and_presence && object.has_status? + end + + def status + UserStatusSerializer.new(object.user_status, root: false).as_json + end +end diff --git a/app/serializers/current_user_serializer.rb b/app/serializers/current_user_serializer.rb index d6634e2bdb4..2e6823cbd09 100644 --- a/app/serializers/current_user_serializer.rb +++ b/app/serializers/current_user_serializer.rb @@ -3,6 +3,7 @@ class CurrentUserSerializer < BasicUserSerializer include UserTagNotificationsMixin include UserSidebarMixin + include UserStatusMixin attributes :name, :unread_notifications, @@ -65,7 +66,6 @@ class CurrentUserSerializer < BasicUserSerializer :can_review, :draft_count, :pending_posts_count, - :status, :grouped_unread_notifications, :display_sidebar_tags, :sidebar_tags, @@ -82,6 +82,11 @@ class CurrentUserSerializer < BasicUserSerializer has_one :user_option, embed: :object, serializer: CurrentUserOptionSerializer + def initialize(object, options = {}) + super + options[:include_status] = true + end + def sidebar_sections SidebarSection .public_sections @@ -304,14 +309,6 @@ class CurrentUserSerializer < BasicUserSerializer Draft.has_topic_draft(object) end - def include_status? - SiteSetting.enable_user_status && object.has_status? - end - - def status - UserStatusSerializer.new(object.user_status, root: false) - end - def unseen_reviewable_count Reviewable.unseen_reviewable_count(object) end diff --git a/app/serializers/found_user_serializer.rb b/app/serializers/found_user_serializer.rb index 4c2da44e614..e128c6eb954 100644 --- a/app/serializers/found_user_serializer.rb +++ b/app/serializers/found_user_serializer.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class FoundUserSerializer < ApplicationSerializer + include UserStatusMixin + attributes :id, :username, :name, :avatar_template def include_name? diff --git a/app/serializers/found_user_with_status_serializer.rb b/app/serializers/found_user_with_status_serializer.rb index 7e4278e22f0..5aa95d83512 100644 --- a/app/serializers/found_user_with_status_serializer.rb +++ b/app/serializers/found_user_with_status_serializer.rb @@ -1,13 +1,10 @@ # frozen_string_literal: true class FoundUserWithStatusSerializer < FoundUserSerializer - attributes :status + include UserStatusMixin - def include_status? - SiteSetting.enable_user_status && object.has_status? - end - - def status - UserStatusSerializer.new(object.user_status, root: false) + def initialize(object, options = {}) + super + options[:include_status] = true end end diff --git a/app/serializers/group_user_serializer.rb b/app/serializers/group_user_serializer.rb index bc286d109b7..c6627f8409b 100644 --- a/app/serializers/group_user_serializer.rb +++ b/app/serializers/group_user_serializer.rb @@ -2,8 +2,14 @@ class GroupUserSerializer < BasicUserSerializer include UserPrimaryGroupMixin + include UserStatusMixin - attributes :name, :title, :last_posted_at, :last_seen_at, :added_at, :timezone, :status + attributes :name, :title, :last_posted_at, :last_seen_at, :added_at, :timezone + + def initialize(object, options = {}) + super + options[:include_status] = true + end def timezone user.user_option.timezone @@ -12,12 +18,4 @@ class GroupUserSerializer < BasicUserSerializer def include_added_at? object.respond_to? :added_at end - - def include_status? - SiteSetting.enable_user_status && user.has_status? - end - - def status - UserStatusSerializer.new(user.user_status, root: false) - end end diff --git a/app/serializers/group_user_with_custom_fields_serializer.rb b/app/serializers/group_user_with_custom_fields_serializer.rb index fb3ec5ac66b..10c7291fbbc 100644 --- a/app/serializers/group_user_with_custom_fields_serializer.rb +++ b/app/serializers/group_user_with_custom_fields_serializer.rb @@ -3,7 +3,12 @@ class GroupUserWithCustomFieldsSerializer < UserWithCustomFieldsSerializer include UserPrimaryGroupMixin - attributes :name, :title, :last_posted_at, :last_seen_at, :added_at, :timezone, :status + attributes :name, :title, :last_posted_at, :last_seen_at, :added_at, :timezone + + def initialize(object, options = {}) + super + options[:include_status] = true + end def timezone user.user_option.timezone @@ -12,12 +17,4 @@ class GroupUserWithCustomFieldsSerializer < UserWithCustomFieldsSerializer def include_added_at? object.respond_to? :added_at end - - def include_status? - SiteSetting.enable_user_status && user.has_status? - end - - def status - UserStatusSerializer.new(user.user_status, root: false) - end end diff --git a/app/serializers/post_serializer.rb b/app/serializers/post_serializer.rb index 5da2ad41f8b..e5efff27707 100644 --- a/app/serializers/post_serializer.rb +++ b/app/serializers/post_serializer.rb @@ -581,12 +581,12 @@ class PostSerializer < BasicPostSerializer if @topic_view && (mentioned_users = @topic_view.mentioned_users[object.id]) mentioned_users else - query = User + query = User.includes(:user_option) query = query.includes(:user_status) if SiteSetting.enable_user_status query = query.where(username: object.mentions) end - users.map { |user| BasicUserWithStatusSerializer.new(user, root: false) } + users.map { |user| BasicUserSerializer.new(user, root: false, include_status: true).as_json } end def include_mentioned_users? diff --git a/app/serializers/user_card_serializer.rb b/app/serializers/user_card_serializer.rb index 301082cab61..b7c322d3c91 100644 --- a/app/serializers/user_card_serializer.rb +++ b/app/serializers/user_card_serializer.rb @@ -1,8 +1,15 @@ # frozen_string_literal: true class UserCardSerializer < BasicUserSerializer + include UserStatusMixin + attr_accessor :topic_post_count + def initialize(object, options = {}) + super + options[:include_status] = true + end + def self.staff_attributes(*attrs) attributes(*attrs) attrs.each do |attr| @@ -70,8 +77,7 @@ class UserCardSerializer < BasicUserSerializer :flair_color, :featured_topic, :timezone, - :pending_posts_count, - :status + :pending_posts_count untrusted_attributes :bio_excerpt, :website, :website_name, :location, :card_background_upload_url @@ -223,14 +229,6 @@ class UserCardSerializer < BasicUserSerializer object.card_background_upload&.url end - def include_status? - SiteSetting.enable_user_status && user.has_status? - end - - def status - UserStatusSerializer.new(user.user_status, root: false) - end - private def custom_field_keys diff --git a/app/serializers/user_with_custom_fields_serializer.rb b/app/serializers/user_with_custom_fields_serializer.rb index 10cb152990b..913424f84fe 100644 --- a/app/serializers/user_with_custom_fields_serializer.rb +++ b/app/serializers/user_with_custom_fields_serializer.rb @@ -2,7 +2,8 @@ # A basic user serializer, with custom fields class UserWithCustomFieldsSerializer < BasicUserSerializer - attributes :custom_fields + include UserStatusMixin + attribute :custom_fields def custom_fields fields = custom_field_keys diff --git a/lib/presence_channel.rb b/lib/presence_channel.rb index 3e4854cd964..a8994ae00bf 100644 --- a/lib/presence_channel.rb +++ b/lib/presence_channel.rb @@ -336,7 +336,7 @@ class PresenceChannel else message["leaving_user_ids"] = leaving_user_ids if leaving_user_ids.present? if entering_user_ids.present? - users = User.where(id: entering_user_ids) + users = User.where(id: entering_user_ids).includes(:user_option) message["entering_users"] = ActiveModel::ArraySerializer.new( users, each_serializer: BasicUserSerializer, diff --git a/plugins/chat/app/serializers/chat/chatable_user_serializer.rb b/plugins/chat/app/serializers/chat/chatable_user_serializer.rb index 1509174439e..deae8dd4004 100644 --- a/plugins/chat/app/serializers/chat/chatable_user_serializer.rb +++ b/plugins/chat/app/serializers/chat/chatable_user_serializer.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module Chat - class ChatableUserSerializer < ::Chat::UserWithCustomFieldsAndStatusSerializer + class ChatableUserSerializer < UserWithCustomFieldsSerializer attributes :can_chat, :has_chat_enabled def can_chat diff --git a/plugins/chat/app/serializers/chat/chatables_serializer.rb b/plugins/chat/app/serializers/chat/chatables_serializer.rb index dd35af1f6b6..73b03922596 100644 --- a/plugins/chat/app/serializers/chat/chatables_serializer.rb +++ b/plugins/chat/app/serializers/chat/chatables_serializer.rb @@ -12,7 +12,13 @@ module Chat .map do |user| { identifier: "u-#{user.id}", - model: ::Chat::ChatableUserSerializer.new(user, scope: scope, root: false), + model: + ::Chat::ChatableUserSerializer.new( + user, + scope: scope, + root: false, + include_status: true, + ), type: "user", } end diff --git a/plugins/chat/app/serializers/chat/direct_message_serializer.rb b/plugins/chat/app/serializers/chat/direct_message_serializer.rb index 1c857ae2dd6..31028391ddb 100644 --- a/plugins/chat/app/serializers/chat/direct_message_serializer.rb +++ b/plugins/chat/app/serializers/chat/direct_message_serializer.rb @@ -2,15 +2,21 @@ module Chat class DirectMessageSerializer < ApplicationSerializer - attribute :group - - has_many :users, serializer: ::Chat::ChatableUserSerializer, embed: :objects + attributes :group, :users def users users = object.direct_message_users.map(&:user).map { |u| u || Chat::NullUser.new } + users = users - [scope.user] if users.count > 1 - return users - [scope.user] if users.count > 1 - users + serializer = + ActiveModel::ArraySerializer.new( + users, + each_serializer: ::Chat::ChatableUserSerializer, + scope: scope, + include_status: true, + ) + + serializer.as_json end end end diff --git a/plugins/chat/app/serializers/chat/message_serializer.rb b/plugins/chat/app/serializers/chat/message_serializer.rb index f9ae2e69e46..acccb091432 100644 --- a/plugins/chat/app/serializers/chat/message_serializer.rb +++ b/plugins/chat/app/serializers/chat/message_serializer.rb @@ -18,6 +18,7 @@ module Chat *( BASIC_ATTRIBUTES + %i[ + user mentioned_users reactions bookmark @@ -30,7 +31,6 @@ module Chat ), ) - has_one :user, serializer: Chat::MessageUserSerializer, embed: :objects has_one :chat_webhook_event, serializer: Chat::WebhookEventSerializer, embed: :objects has_one :in_reply_to, serializer: Chat::InReplyToSerializer, embed: :objects has_many :uploads, serializer: ::UploadSerializer, embed: :objects @@ -38,11 +38,12 @@ module Chat def mentioned_users object .user_mentions + .includes(user: :user_option) .limit(SiteSetting.max_mentions_per_chat_message) .map(&:user) .compact .sort_by(&:id) - .map { |user| BasicUserWithStatusSerializer.new(user, root: false) } + .map { |user| BasicUserSerializer.new(user, root: false, include_status: true) } .as_json end @@ -51,7 +52,8 @@ module Chat end def user - object.user || Chat::NullUser.new + user = object.user || Chat::NullUser.new + MessageUserSerializer.new(user, root: false, include_status: true).as_json end def excerpt @@ -61,6 +63,7 @@ module Chat def reactions object .reactions + .includes(user: :user_option) .group_by(&:emoji) .map do |emoji, reactions| next unless Emoji.exists?(emoji) @@ -169,7 +172,7 @@ module Chat if sym == :notify_user && ( - scope.current_user == user || user.bot? || + scope.current_user == user || object.user.bot? || !scope.current_user.in_any_groups?(SiteSetting.personal_message_enabled_groups_map) ) next diff --git a/plugins/chat/app/serializers/chat/message_user_serializer.rb b/plugins/chat/app/serializers/chat/message_user_serializer.rb index 92d222a6782..ddfa3872f7b 100644 --- a/plugins/chat/app/serializers/chat/message_user_serializer.rb +++ b/plugins/chat/app/serializers/chat/message_user_serializer.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module Chat - class MessageUserSerializer < BasicUserWithStatusSerializer + class MessageUserSerializer < BasicUserSerializer attributes :moderator?, :admin?, :staff?, :moderator?, :new_user?, :primary_group_name def moderator? diff --git a/plugins/chat/app/serializers/chat/thread_original_message_serializer.rb b/plugins/chat/app/serializers/chat/thread_original_message_serializer.rb index 1afcca20799..e686f1059b1 100644 --- a/plugins/chat/app/serializers/chat/thread_original_message_serializer.rb +++ b/plugins/chat/app/serializers/chat/thread_original_message_serializer.rb @@ -9,7 +9,8 @@ module Chat :excerpt, :chat_channel_id, :deleted_at, - :mentioned_users + :mentioned_users, + :user def excerpt object.censored_excerpt @@ -18,14 +19,17 @@ module Chat def mentioned_users object .user_mentions + .includes(user: :user_option) .limit(SiteSetting.max_mentions_per_chat_message) .map(&:user) .compact .sort_by(&:id) - .map { |user| BasicUserWithStatusSerializer.new(user, root: false) } + .map { |user| BasicUserSerializer.new(user, root: false, include_status: true) } .as_json end - has_one :user, serializer: BasicUserWithStatusSerializer, embed: :objects + def user + BasicUserSerializer.new(object.user, root: false, include_status: true).as_json + end end end diff --git a/plugins/chat/app/serializers/chat/user_with_custom_fields_and_status_serializer.rb b/plugins/chat/app/serializers/chat/user_with_custom_fields_and_status_serializer.rb deleted file mode 100644 index b27adf075ac..00000000000 --- a/plugins/chat/app/serializers/chat/user_with_custom_fields_and_status_serializer.rb +++ /dev/null @@ -1,21 +0,0 @@ -# frozen_string_literal: true - -module Chat - class UserWithCustomFieldsAndStatusSerializer < ::UserWithCustomFieldsSerializer - attributes :status - - def include_status? - predicate = SiteSetting.enable_user_status && user.has_status? - - if user.association(:user_option).loaded? - predicate = predicate && !user.user_option.hide_profile_and_presence - end - - predicate - end - - def status - ::UserStatusSerializer.new(user.user_status, root: false) - end - end -end diff --git a/plugins/chat/spec/requests/chat/api/channel_threads_controller_spec.rb b/plugins/chat/spec/requests/chat/api/channel_threads_controller_spec.rb index 537939dbae2..d766da3deff 100644 --- a/plugins/chat/spec/requests/chat/api/channel_threads_controller_spec.rb +++ b/plugins/chat/spec/requests/chat/api/channel_threads_controller_spec.rb @@ -128,26 +128,24 @@ RSpec.describe Chat::Api::ChannelThreadsController do get "/chat/api/channels/#{public_channel.id}/threads" expect(response.status).to eq(200) - expect( + + mentioned_users = response.parsed_body["threads"] .find { |thread| thread["id"] == thread_1.id } - .dig("original_message", "mentioned_users"), - ).to eq( - [ - { - "avatar_template" => User.system_avatar_template(current_user.username), - "id" => current_user.id, - "name" => current_user.name, - "username" => current_user.username, - }, - { - "avatar_template" => - User.system_avatar_template(thread_2.original_message_user.username), - "id" => thread_2.original_message_user.id, - "name" => thread_2.original_message_user.name, - "username" => thread_2.original_message_user.username, - }, - ], + .dig("original_message", "mentioned_users") + expect(mentioned_users.count).to be(2) + expect(mentioned_users[0]).to include( + "avatar_template" => User.system_avatar_template(current_user.username), + "id" => current_user.id, + "name" => current_user.name, + "username" => current_user.username, + ) + second_user = thread_2.original_message_user + expect(mentioned_users[1]).to include( + "avatar_template" => User.system_avatar_template(second_user.username), + "id" => second_user.id, + "name" => second_user.name, + "username" => second_user.username, ) end diff --git a/plugins/chat/spec/serializer/chat/chat_message_serializer_spec.rb b/plugins/chat/spec/serializer/chat/chat_message_serializer_spec.rb index 135108624b9..d4623631012 100644 --- a/plugins/chat/spec/serializer/chat/chat_message_serializer_spec.rb +++ b/plugins/chat/spec/serializer/chat/chat_message_serializer_spec.rb @@ -62,6 +62,21 @@ describe Chat::MessageSerializer do expect(serializer.as_json[:user][:username]).to eq(I18n.t("chat.deleted_chat_username")) end end + + context "with user status" do + it "adds status to user if status is enabled" do + message_1.user.set_status!("test", "heart") + SiteSetting.enable_user_status = true + json = serializer.as_json + expect(json[:user][:status]).to be_present + end + + it "does not add status to user if status is disabled" do + SiteSetting.enable_user_status = false + json = serializer.as_json + expect(json[:user][:status]).to be_nil + end + end end describe "#deleted_at" do @@ -233,6 +248,41 @@ describe Chat::MessageSerializer do expect { serializer.as_json }.not_to raise_error expect(serializer.as_json[:mentioned_users]).to be_empty end + + context "with user status" do + fab!(:user_status) { Fabricate(:user_status) } + fab!(:mentioned_user) { Fabricate(:user, user_status: user_status) } + fab!(:message) do + Fabricate( + :chat_message, + message: + "there should be a mention here, but since we're fabricating objects it doesn't matter", + ) + end + fab!(:chat_mention) do + Fabricate(:user_chat_mention, chat_message: message, user: mentioned_user) + end + + it "adds status to mentioned users when status is enabled" do + SiteSetting.enable_user_status = true + + serializer = described_class.new(message, scope: guardian, root: nil) + json = serializer.as_json + + expect(json[:mentioned_users][0][:status]).not_to be_nil + expect(json[:mentioned_users][0][:status][:description]).to eq(user_status.description) + expect(json[:mentioned_users][0][:status][:emoji]).to eq(user_status.emoji) + end + + it "does not add status to mentioned users when status is enabled" do + SiteSetting.enable_user_status = false + + serializer = described_class.new(message, scope: guardian, root: nil) + json = serializer.as_json + + expect(json[:mentioned_users][0][:status]).to be_nil + end + end end describe "threading data" do diff --git a/plugins/chat/spec/serializer/chat/chatable_user_serializer_spec.rb b/plugins/chat/spec/serializer/chat/chatable_user_serializer_spec.rb new file mode 100644 index 00000000000..0b7c0c07e8a --- /dev/null +++ b/plugins/chat/spec/serializer/chat/chatable_user_serializer_spec.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true +RSpec.describe Chat::ChatableUserSerializer do + fab!(:user) { Fabricate(:user) } + subject(:serializer) { described_class.new(user, scope: Guardian.new(user), root: false) } + + it "serializes a user" do + json = serializer.as_json + + expect(json).to eq( + { + id: user.id, + username: user.username, + name: user.name, + avatar_template: user.avatar_template, + custom_fields: { + }, + can_chat: false, + has_chat_enabled: false, + }, + ) + end +end diff --git a/plugins/chat/spec/serializer/chat/direct_message_serializer_spec.rb b/plugins/chat/spec/serializer/chat/direct_message_serializer_spec.rb index 3315b2c37fe..62166118d29 100644 --- a/plugins/chat/spec/serializer/chat/direct_message_serializer_spec.rb +++ b/plugins/chat/spec/serializer/chat/direct_message_serializer_spec.rb @@ -10,8 +10,9 @@ describe Chat::DirectMessageSerializer do direct_message = Fabricate.build(:direct_message, users: [me, you]) serializer = described_class.new(direct_message, scope: Guardian.new(me), root: false) + json = serializer.as_json - expect(serializer.users).to eq([you]) + expect(json[:users].map { |u| u[:username] }).to eq([you.username]) end it "returns you both if there are three of us" do @@ -21,8 +22,11 @@ describe Chat::DirectMessageSerializer do direct_message = Fabricate.build(:direct_message, users: [me, you, other_you]) serializer = described_class.new(direct_message, scope: Guardian.new(me), root: false) + json = serializer.as_json - expect(serializer.users).to match_array([you, other_you]) + expect(json[:users].map { |u| u[:username] }).to match_array( + [you.username, other_you.username], + ) end it "returns me if there is only me" do @@ -30,8 +34,9 @@ describe Chat::DirectMessageSerializer do direct_message = Fabricate.build(:direct_message, users: [me]) serializer = described_class.new(direct_message, scope: Guardian.new(me), root: false) + json = serializer.as_json - expect(serializer.users).to eq([me]) + expect(json[:users].map { |u| u[:username] }).to eq([me.username]) end context "when a user is destroyed" do @@ -43,9 +48,10 @@ describe Chat::DirectMessageSerializer do you.destroy! serializer = - described_class.new(direct_message.reload, scope: Guardian.new(me), root: false).as_json + described_class.new(direct_message.reload, scope: Guardian.new(me), root: false) + json = serializer.as_json - expect(serializer[:users][0][:username]).to eq(I18n.t("chat.deleted_chat_username")) + expect(json[:users][0][:username]).to eq(I18n.t("chat.deleted_chat_username")) end end end diff --git a/plugins/chat/spec/serializer/chat/thread_original_message_serializer_spec.rb b/plugins/chat/spec/serializer/chat/thread_original_message_serializer_spec.rb new file mode 100644 index 00000000000..55a15503af6 --- /dev/null +++ b/plugins/chat/spec/serializer/chat/thread_original_message_serializer_spec.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe Chat::ThreadOriginalMessageSerializer do + describe "#user" do + fab!(:user_status) { Fabricate(:user_status) } + fab!(:user) { Fabricate(:user, user_status: user_status) } + fab!(:message) { Fabricate(:chat_message, user: user) } + + subject(:serializer) { described_class.new(message, root: nil) } + + it "adds status to user if status is enabled" do + SiteSetting.enable_user_status = true + + json = serializer.as_json + + expect(json[:user][:status]).to be_present + expect(json[:user][:status][:description]).to eq(user_status.description) + expect(json[:user][:status][:emoji]).to eq(user_status.emoji) + end + + it "does not add status user if status is disabled" do + SiteSetting.enable_user_status = false + json = serializer.as_json + expect(json[:user][:status]).to be_nil + end + end + + context "with mentions" do + fab!(:user_status) { Fabricate(:user_status) } + fab!(:mentioned_user) { Fabricate(:user, user_status: user_status) } + fab!(:message) do + Fabricate( + :chat_message, + message: + "there should be a mention here, but since we're fabricating objects it doesn't matter", + ) + end + fab!(:chat_mention) do + Fabricate(:user_chat_mention, chat_message: message, user: mentioned_user) + end + + subject(:serializer) { described_class.new(message, root: nil) } + + it "adds status to mentioned users if status is enabled" do + SiteSetting.enable_user_status = true + + json = serializer.as_json + + expect(json[:mentioned_users][0][:status]).to be_present + expect(json[:mentioned_users][0][:status][:description]).to eq(user_status.description) + expect(json[:mentioned_users][0][:status][:emoji]).to eq(user_status.emoji) + end + + it "does not add status to mentioned users if status is disabled" do + SiteSetting.enable_user_status = false + json = serializer.as_json + expect(json[:mentioned_users][0][:status]).to be_nil + end + end +end diff --git a/plugins/chat/spec/services/chat/update_message_spec.rb b/plugins/chat/spec/services/chat/update_message_spec.rb index 0b7efefbbb9..d46bc20aa9c 100644 --- a/plugins/chat/spec/services/chat/update_message_spec.rb +++ b/plugins/chat/spec/services/chat/update_message_spec.rb @@ -309,7 +309,7 @@ RSpec.describe Chat::UpdateMessage do expect(mentioned_user["id"]).to eq(user2.id) expect(mentioned_user["username"]).to eq(user2.username) expect(mentioned_user["status"]).to be_present - expect(mentioned_user["status"].slice(:description, :emoji)).to eq(status) + expect(mentioned_user["status"].symbolize_keys.slice(:description, :emoji)).to eq(status) end it "doesn't add mentioned user's status to the message bus message when status is disabled" do diff --git a/spec/serializers/basic_user_serializer_spec.rb b/spec/serializers/basic_user_serializer_spec.rb index 5c76ebe458f..250319eefb0 100644 --- a/spec/serializers/basic_user_serializer_spec.rb +++ b/spec/serializers/basic_user_serializer_spec.rb @@ -1,9 +1,10 @@ # frozen_string_literal: true RSpec.describe BasicUserSerializer do + fab!(:user) { Fabricate(:user) } + let(:serializer) { BasicUserSerializer.new(user, scope: Guardian.new(user), root: false) } + describe "#as_json" do - let(:user) { Fabricate.build(:user) } - let(:serializer) { BasicUserSerializer.new(user, scope: Guardian.new(user), root: false) } let(:json) { serializer.as_json } it "returns the username" do @@ -27,4 +28,59 @@ RSpec.describe BasicUserSerializer do expect(json[:name]).to eq(nil) end end + + describe "#status" do + fab!(:user_status) { Fabricate(:user_status) } + + before { user.user_status = user_status } + + describe "when status is enabled in settings" do + before { SiteSetting.enable_user_status = true } + + it "doesn't add status by default" do + serializer = BasicUserSerializer.new(user, root: false) + json = serializer.as_json + + expect(json.keys).not_to include :status + end + + context "when including user status" do + let(:serializer) { BasicUserSerializer.new(user, root: false, include_status: true) } + + it "adds status if `include_status: true` has been passed" do + json = serializer.as_json + expect(json[:status]).to_not be_nil do |status| + expect(status.description).to eq(user_status.description) + expect(status.emoji).to eq(user_status.emoji) + end + end + + it "doesn't add expired user status" do + user.user_status.ends_at = 1.minutes.ago + json = serializer.as_json + expect(json.keys).not_to include :status + end + + it "doesn't return status if user doesn't have it set" do + user.clear_status! + user.reload + + json = serializer.as_json + + expect(json.keys).not_to include :status + end + end + end + + describe "when status is disabled in settings" do + before { SiteSetting.enable_user_status = false } + + it "doesn't add user status" do + serializer = BasicUserSerializer.new(user, root: false, include_status: true) + json = serializer.as_json + + expect(json.keys).not_to include :status + end + end + end end diff --git a/spec/serializers/basic_user_with_status_serializer_spec.rb b/spec/serializers/basic_user_with_status_serializer_spec.rb deleted file mode 100644 index 52b87a11c32..00000000000 --- a/spec/serializers/basic_user_with_status_serializer_spec.rb +++ /dev/null @@ -1,44 +0,0 @@ -# frozen_string_literal: true - -RSpec.describe BasicUserWithStatusSerializer do - fab!(:user_status) - fab!(:user) { Fabricate(:user, user_status: user_status) } - let(:serializer) { described_class.new(user, scope: Guardian.new(user), root: false) } - - it "adds user status when enabled" do - SiteSetting.enable_user_status = true - - json = serializer.as_json - - expect(json[:status]).to_not be_nil do |status| - expect(status.description).to eq(user_status.description) - expect(status.emoji).to eq(user_status.emoji) - end - end - - it "doesn't add user status when status is disabled in site settings" do - SiteSetting.enable_user_status = false - json = serializer.as_json - expect(json.keys).not_to include :status - end - - it "doesn't add expired user status" do - SiteSetting.enable_user_status = true - - user.user_status.ends_at = 1.minutes.ago - serializer = described_class.new(user, scope: Guardian.new(user), root: false) - json = serializer.as_json - - expect(json.keys).not_to include :status - end - - it "doesn't return status if user doesn't have it set" do - SiteSetting.enable_user_status = true - - user.clear_status! - user.reload - json = serializer.as_json - - expect(json.keys).not_to include :status - end -end diff --git a/spec/serializers/concerns/user_status_mixin_spec.rb b/spec/serializers/concerns/user_status_mixin_spec.rb new file mode 100644 index 00000000000..d3dd2e636b2 --- /dev/null +++ b/spec/serializers/concerns/user_status_mixin_spec.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +RSpec.describe UserStatusMixin do + fab!(:user_status) { Fabricate(:user_status) } + fab!(:user) { Fabricate(:user, user_status: user_status) } + + class DummySerializer < ApplicationSerializer + include UserStatusMixin + end + + context "when user status is disabled" do + before { SiteSetting.enable_user_status = false } + + it "doesn't include status" do + serializer = DummySerializer.new(user, root: false, include_status: true) + json = serializer.as_json + expect(json[:status]).to be_nil + end + end + + context "when user status is enabled" do + before { SiteSetting.enable_user_status = true } + + it "doesn't include status by default" do + serializer = DummySerializer.new(user, root: false) + json = serializer.as_json + expect(json[:status]).to be_nil + end + + it "includes status when include_status option is passed" do + serializer = DummySerializer.new(user, root: false, include_status: true) + json = serializer.as_json + expect(json[:status]).to be_present + end + + it "doesn't include status if user hid profile and presence" do + user.user_option.hide_profile_and_presence = true + serializer = DummySerializer.new(user, root: false, include_status: true) + json = serializer.as_json + expect(json[:status]).to be_nil + end + end +end diff --git a/spec/serializers/found_user_serializer_spec.rb b/spec/serializers/found_user_serializer_spec.rb index 84426d1919f..95c47e831e2 100644 --- a/spec/serializers/found_user_serializer_spec.rb +++ b/spec/serializers/found_user_serializer_spec.rb @@ -26,4 +26,59 @@ RSpec.describe FoundUserSerializer do expect(json.keys).not_to include :name end end + + describe "#status" do + fab!(:user_status) { Fabricate(:user_status) } + + before { user.user_status = user_status } + + context "when status is enabled in site settings" do + before { SiteSetting.enable_user_status = true } + + it "doesn't add user status by default" do + serializer = FoundUserSerializer.new(user, root: false) + json = serializer.as_json + expect(json.keys).not_to include :status + end + + context "when including user status" do + let(:serializer) { FoundUserSerializer.new(user, root: false, include_status: true) } + + it "adds user status when enabled" do + json = serializer.as_json + + expect(json[:status]).to_not be_nil do |status| + expect(status.description).to eq(user_status.description) + expect(status.emoji).to eq(user_status.emoji) + end + end + + it "doesn't add expired user status" do + user.user_status.ends_at = 1.minutes.ago + serializer = described_class.new(user, scope: Guardian.new(user), root: false) + json = serializer.as_json + + expect(json.keys).not_to include :status + end + + it "doesn't return status if user doesn't have it" do + user.clear_status! + user.reload + json = serializer.as_json + + expect(json.keys).not_to include :status + end + end + end + + context "when status is disabled in site settings" do + before { SiteSetting.enable_user_status = false } + let(:serializer) { FoundUserSerializer.new(user, root: false, include_status: true) } + + it "doesn't add user status" do + json = serializer.as_json + expect(json.keys).not_to include :status + end + end + end end diff --git a/spec/serializers/group_user_with_custom_fields_serializer_spec.rb b/spec/serializers/group_user_with_custom_fields_serializer_spec.rb new file mode 100644 index 00000000000..fbbf1653d99 --- /dev/null +++ b/spec/serializers/group_user_with_custom_fields_serializer_spec.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +RSpec.describe GroupUserWithCustomFieldsSerializer do + describe "#status" do + fab!(:user_status) + fab!(:user) { Fabricate(:user, user_status: user_status) } + + it "adds user status when enabled in site settings" do + SiteSetting.enable_user_status = true + + serializer = described_class.new(user, scope: Guardian.new(user), root: false) + json = serializer.as_json + + expect(json[:status]).to be_present + end + end +end diff --git a/spec/serializers/post_serializer_spec.rb b/spec/serializers/post_serializer_spec.rb index d6058e9fba3..30aabec9c96 100644 --- a/spec/serializers/post_serializer_spec.rb +++ b/spec/serializers/post_serializer_spec.rb @@ -362,6 +362,35 @@ RSpec.describe PostSerializer do end end + context "with mentions" do + fab!(:user_status) + fab!(:user) { Fabricate(:user) } + fab!(:user1) { Fabricate(:user, user_status: user_status) } + fab!(:post) { Fabricate(:post, user: user, raw: "Hey @#{user1.username}") } + let(:serializer) { described_class.new(post, scope: Guardian.new(user), root: false) } + + it "returns mentioned users with user status when user status is enabled" do + SiteSetting.enable_user_status = true + + json = serializer.as_json + + expect(json[:mentioned_users]).to be_present + expect(json[:mentioned_users].length).to be(1) + expect(json[:mentioned_users][0]).to_not be_nil + expect(json[:mentioned_users][0][:id]).to eq(user1.id) + expect(json[:mentioned_users][0][:username]).to eq(user1.username) + expect(json[:mentioned_users][0][:name]).to eq(user1.name) + expect(json[:mentioned_users][0][:status][:description]).to eq(user_status.description) + expect(json[:mentioned_users][0][:status][:emoji]).to eq(user_status.emoji) + end + + it "doesn't return mentioned users when user status is disabled" do + SiteSetting.enable_user_status = false + json = serializer.as_json + expect(json[:mentioned_users]).to be_nil + end + end + describe "#user_status" do fab!(:user_status) fab!(:user) { Fabricate(:user, user_status: user_status) } diff --git a/spec/serializers/user_with_custom_fields_serializer_spec.rb b/spec/serializers/user_with_custom_fields_serializer_spec.rb new file mode 100644 index 00000000000..d85c641334a --- /dev/null +++ b/spec/serializers/user_with_custom_fields_serializer_spec.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true +RSpec.describe UserWithCustomFieldsSerializer do + describe "#status" do + fab!(:user_status) + fab!(:user) { Fabricate(:user, user_status: user_status) } + + it "adds user status when enabled in site settings" do + SiteSetting.enable_user_status = true + + serializer = + described_class.new(user, scope: Guardian.new(user), root: false, include_status: true) + json = serializer.as_json + + expect(json[:status]).to be_present + end + end +end