From 3755bad03c6622824983f670e3910a36342dd81d Mon Sep 17 00:00:00 2001 From: Andrei Prigorshnev Date: Tue, 9 Aug 2022 14:54:33 +0400 Subject: [PATCH] DEV: return user status on the user search route (#17716) --- app/controllers/users_controller.rb | 14 ++++-- app/models/user.rb | 2 +- app/models/user_search.rb | 8 +++- app/serializers/found_user_serializer.rb | 9 ++++ .../found_user_with_status_serializer.rb | 13 ++++++ spec/requests/users_controller_spec.rb | 36 +++++++++++++++ .../serializers/found_user_serializer_spec.rb | 21 +++++++++ .../found_user_with_status_serializer_spec.rb | 46 +++++++++++++++++++ 8 files changed, 142 insertions(+), 7 deletions(-) create mode 100644 app/serializers/found_user_serializer.rb create mode 100644 app/serializers/found_user_with_status_serializer.rb create mode 100644 spec/serializers/found_user_serializer_spec.rb create mode 100644 spec/serializers/found_user_with_status_serializer_spec.rb diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index b43281481cf..e35ff0e34f8 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -1193,11 +1193,7 @@ class UsersController < ApplicationController options[:category_id] = category_id if category_id results = UserSearch.new(term, options).search - - user_fields = [:username, :upload_avatar_template] - user_fields << :name if SiteSetting.enable_names? - - to_render = { users: results.as_json(only: user_fields, methods: [:avatar_template]) } + to_render = serialize_found_users(results) # blank term is only handy for in-topic search of users after @ # we do not want group results ever if term is blank @@ -1969,4 +1965,12 @@ class UsersController < ApplicationController 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 } + end end diff --git a/app/models/user.rb b/app/models/user.rb index a22dc3ad7b2..c828313239d 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1619,7 +1619,7 @@ class User < ActiveRecord::Base publish_user_status(nil) end - def set_status!(description, emoji, ends_at) + def set_status!(description, emoji, ends_at = nil) status = { description: description, emoji: emoji, diff --git a/app/models/user_search.rb b/app/models/user_search.rb index c7d4fdff8d5..baa25dfffac 100644 --- a/app/models/user_search.rb +++ b/app/models/user_search.rb @@ -179,10 +179,16 @@ class UserSearch ids = search_ids return User.where("0=1") if ids.empty? - User.joins("JOIN (SELECT unnest uid, row_number() OVER () AS rn + results = User.joins("JOIN (SELECT unnest uid, row_number() OVER () AS rn FROM unnest('{#{ids.join(",")}}'::int[]) ) x on uid = users.id") .order("rn") + + if SiteSetting.enable_user_status + results = results.includes(:user_status) + end + + results end end diff --git a/app/serializers/found_user_serializer.rb b/app/serializers/found_user_serializer.rb new file mode 100644 index 00000000000..6bdf045563f --- /dev/null +++ b/app/serializers/found_user_serializer.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class FoundUserSerializer < ApplicationSerializer + attributes :username, :name, :avatar_template + + def include_name? + SiteSetting.enable_names? + end +end diff --git a/app/serializers/found_user_with_status_serializer.rb b/app/serializers/found_user_with_status_serializer.rb new file mode 100644 index 00000000000..7e4278e22f0 --- /dev/null +++ b/app/serializers/found_user_with_status_serializer.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class FoundUserWithStatusSerializer < FoundUserSerializer + attributes :status + + def include_status? + SiteSetting.enable_user_status && object.has_status? + end + + def status + UserStatusSerializer.new(object.user_status, root: false) + end +end diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 3e4503ac36c..632cc38cb44 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -4536,6 +4536,42 @@ RSpec.describe UsersController do expect(json["users"].size).to eq(limit) end end + + it "returns avatar_template" do + get "/u/search/users.json", params: { term: user.username } + expect(response.status).to eq(200) + json = response.parsed_body + expect(json["users"][0]).to have_key("avatar_template") + expect(json["users"][0]["avatar_template"]).to eq("/letter_avatar_proxy/v4/letter/j/f475e1/{size}.png") + end + + describe "#status" do + it "returns user status if enabled in site settings" do + SiteSetting.enable_user_status = true + emoji = "tooth" + description = "off to dentist" + user.set_status!(description, emoji) + + get "/u/search/users.json", params: { term: user.name } + + expect(response.status).to eq(200) + json = response.parsed_body + expect(json["users"][0]).to have_key("status") + expect(json["users"][0]["status"]["description"]).to eq(description) + expect(json["users"][0]["status"]["emoji"]).to eq(emoji) + end + + it "doesn't return user status if disabled in site settings" do + SiteSetting.enable_user_status = false + user.set_status!("off to dentist", "tooth") + + get "/u/search/users.json", params: { term: user.name } + + expect(response.status).to eq(200) + json = response.parsed_body + expect(json["users"][0]).not_to have_key("status") + end + end end describe '#email_login' do diff --git a/spec/serializers/found_user_serializer_spec.rb b/spec/serializers/found_user_serializer_spec.rb new file mode 100644 index 00000000000..ed76d371b59 --- /dev/null +++ b/spec/serializers/found_user_serializer_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +RSpec.describe FoundUserSerializer do + fab!(:user) { Fabricate(:user) } + let(:serializer) { described_class.new(user, root: false) } + + describe '#name' do + it "returns name if enabled in site settings" do + SiteSetting.enable_names = true + json = serializer.as_json + expect(json.keys).to include :name + expect(json[:name]).to eq(user.name) + end + + it "doesn't return name if disabled in site settings" do + SiteSetting.enable_names = false + json = serializer.as_json + expect(json.keys).not_to include :name + end + end +end diff --git a/spec/serializers/found_user_with_status_serializer_spec.rb b/spec/serializers/found_user_with_status_serializer_spec.rb new file mode 100644 index 00000000000..6254f557152 --- /dev/null +++ b/spec/serializers/found_user_with_status_serializer_spec.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +RSpec.describe FoundUserWithStatusSerializer do + fab!(:user_status) { Fabricate(:user_status) } + fab!(:user) { Fabricate(:user, user_status: user_status) } + let(:serializer) { described_class.new(user, root: false) } + + describe "#status" do + 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 disabled" 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" do + SiteSetting.enable_user_status = true + + user.clear_status! + user.reload + json = serializer.as_json + + expect(json.keys).not_to include :status + end + end +end