Skip to content

Commit

Permalink
FEATURE: Hide user status when user is hiding public profile and pres…
Browse files Browse the repository at this point in the history
…ence (discourse#24300)

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 discourse#23946. 
The current PR fixes all the remaining places in Core.

Note that the actual fix is quite simple – discourse@a5802f4. 
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.
  • Loading branch information
AndrewPrigorshnev authored Feb 26, 2024
1 parent 41790f7 commit b3a1199
Show file tree
Hide file tree
Showing 34 changed files with 480 additions and 172 deletions.
13 changes: 8 additions & 5 deletions app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1237,7 +1237,7 @@ def search_users
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)

Expand Down Expand Up @@ -2238,9 +2238,12 @@ def render_invite_error(message)
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
1 change: 1 addition & 0 deletions app/models/user_search.rb
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ def search
) x on uid = users.id",
).order("rn")

results = results.includes(:user_option)
results = results.includes(:user_status) if SiteSetting.enable_user_status

results
Expand Down
2 changes: 2 additions & 0 deletions app/serializers/basic_user_serializer.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# frozen_string_literal: true

class BasicUserSerializer < ApplicationSerializer
include UserStatusMixin

attributes :id, :username, :name, :avatar_template

def name
Expand Down
13 changes: 0 additions & 13 deletions app/serializers/basic_user_with_status_serializer.rb

This file was deleted.

16 changes: 16 additions & 0 deletions app/serializers/concerns/user_status_mixin.rb
Original file line number Diff line number Diff line change
@@ -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
15 changes: 6 additions & 9 deletions app/serializers/current_user_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
class CurrentUserSerializer < BasicUserSerializer
include UserTagNotificationsMixin
include UserSidebarMixin
include UserStatusMixin

attributes :name,
:unread_notifications,
Expand Down Expand Up @@ -65,7 +66,6 @@ class CurrentUserSerializer < BasicUserSerializer
:can_review,
:draft_count,
:pending_posts_count,
:status,
:grouped_unread_notifications,
:display_sidebar_tags,
:sidebar_tags,
Expand All @@ -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
Expand Down Expand Up @@ -304,14 +309,6 @@ def include_has_topic_draft?
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
Expand Down
2 changes: 2 additions & 0 deletions app/serializers/found_user_serializer.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# frozen_string_literal: true

class FoundUserSerializer < ApplicationSerializer
include UserStatusMixin

attributes :id, :username, :name, :avatar_template

def include_name?
Expand Down
11 changes: 4 additions & 7 deletions app/serializers/found_user_with_status_serializer.rb
Original file line number Diff line number Diff line change
@@ -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
16 changes: 7 additions & 9 deletions app/serializers/group_user_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -12,12 +18,4 @@ def timezone
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
15 changes: 6 additions & 9 deletions app/serializers/group_user_with_custom_fields_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -12,12 +17,4 @@ def timezone
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
4 changes: 2 additions & 2 deletions app/serializers/post_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -581,12 +581,12 @@ def mentioned_users
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?
Expand Down
18 changes: 8 additions & 10 deletions app/serializers/user_card_serializer.rb
Original file line number Diff line number Diff line change
@@ -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|
Expand Down Expand Up @@ -70,8 +77,7 @@ def self.untrusted_attributes(*attrs)
:flair_color,
:featured_topic,
:timezone,
:pending_posts_count,
:status
:pending_posts_count

untrusted_attributes :bio_excerpt, :website, :website_name, :location, :card_background_upload_url

Expand Down Expand Up @@ -223,14 +229,6 @@ def card_background_upload_url
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
Expand Down
3 changes: 2 additions & 1 deletion app/serializers/user_with_custom_fields_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/presence_channel.rb
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ def publish_message(entering_user_ids: nil, leaving_user_ids: nil)
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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down
8 changes: 7 additions & 1 deletion plugins/chat/app/serializers/chat/chatables_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,13 @@ def users
.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
Expand Down
16 changes: 11 additions & 5 deletions plugins/chat/app/serializers/chat/direct_message_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

serializer =
ActiveModel::ArraySerializer.new(
users,
each_serializer: ::Chat::ChatableUserSerializer,
scope: scope,
include_status: true,
)

return users - [scope.user] if users.count > 1
users
serializer.as_json
end
end
end
11 changes: 7 additions & 4 deletions plugins/chat/app/serializers/chat/message_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ class MessageSerializer < ::ApplicationSerializer
*(
BASIC_ATTRIBUTES +
%i[
user
mentioned_users
reactions
bookmark
Expand All @@ -30,19 +31,19 @@ class MessageSerializer < ::ApplicationSerializer
),
)

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

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

Expand All @@ -51,7 +52,8 @@ def channel
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
Expand All @@ -61,6 +63,7 @@ def excerpt
def reactions
object
.reactions
.includes(user: :user_option)
.group_by(&:emoji)
.map do |emoji, reactions|
next unless Emoji.exists?(emoji)
Expand Down Expand Up @@ -169,7 +172,7 @@ def available_flags

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
Expand Down
Original file line number Diff line number Diff line change
@@ -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?
Expand Down
Loading

0 comments on commit b3a1199

Please sign in to comment.