PERF: Use a separate route for user cards, and split user serializer (#8789)

Adds a new route `/u/{username}/card.json`, which has a reduced number of fields. This change is behind a hidden site setting, so we can test compatibility before rolling out.
This commit is contained in:
David Taylor 2020-01-28 11:55:46 +00:00 committed by GitHub
parent c344f43211
commit 25fd2b544a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 281 additions and 209 deletions

View File

@ -140,8 +140,11 @@ export default Component.extend(CardContentsBase, CanCheckEmails, CleansUp, {
this._positionCard($target);
this.setProperties({ visible: true, loading: true });
const args = { stats: false };
args.include_post_count_for = this.get("topic.id");
const args = {
forCard: this.siteSettings.enable_new_user_card_route,
include_post_count_for: this.get("topic.id")
};
User.findByUsername(username, args)
.then(user => {
if (user.topic_post_count) {

View File

@ -536,7 +536,15 @@ const User = RestModel.extend({
const user = this;
return PreloadStore.getAndRemove(`user_${user.get("username")}`, () => {
return ajax(userPath(`${user.get("username")}.json`), { data: options });
const useCardRoute = options && options.forCard;
if (options) delete options.forCard;
const path = useCardRoute
? `${user.get("username")}/card.json`
: `${user.get("username")}.json`;
return ajax(userPath(path), { data: options });
}).then(json => {
if (!isEmpty(json.user.stats)) {
json.user.stats = User.groupStats(

View File

@ -52,7 +52,7 @@ class UsersController < ApplicationController
def index
end
def show
def show(for_card: false)
return redirect_to path('/login') if SiteSetting.hide_user_profiles_from_public && !current_user
@user = fetch_user_from_params(
@ -61,7 +61,8 @@ class UsersController < ApplicationController
user_serializer = nil
if guardian.can_see_profile?(@user)
user_serializer = UserSerializer.new(@user, scope: guardian, root: 'user')
serializer_class = for_card ? UserCardSerializer : UserSerializer
user_serializer = serializer_class.new(@user, scope: guardian, root: 'user')
topic_id = params[:include_post_count_for].to_i
if topic_id != 0
@ -94,6 +95,10 @@ class UsersController < ApplicationController
end
end
def show_card
show(for_card: true)
end
def badges
raise Discourse::NotFound unless SiteSetting.enable_badges?
show

View File

@ -0,0 +1,200 @@
# frozen_string_literal: true
class UserCardSerializer < BasicUserSerializer
attr_accessor :topic_post_count
def self.staff_attributes(*attrs)
attributes(*attrs)
attrs.each do |attr|
define_method "include_#{attr}?" do
scope.is_staff?
end
end
end
def self.private_attributes(*attrs)
attributes(*attrs)
attrs.each do |attr|
define_method "include_#{attr}?" do
can_edit
end
end
end
# attributes that are hidden for TL0 users when seen by anonymous
def self.untrusted_attributes(*attrs)
attributes(*attrs)
attrs.each do |attr|
method_name = "include_#{attr}?"
define_method(method_name) do
return false if scope.restrict_user_fields?(object)
public_send(attr).present?
end
end
end
attributes :email,
:last_posted_at,
:last_seen_at,
:created_at,
:ignored,
:muted,
:can_ignore_user,
:can_mute_user,
:can_send_private_messages,
:can_send_private_message_to_user,
:trust_level,
:moderator,
:admin,
:title,
:suspend_reason,
:suspended_till,
:badge_count,
:user_fields,
:custom_fields,
:topic_post_count,
:time_read,
:recent_time_read,
:primary_group_id,
:primary_group_name,
:primary_group_flair_url,
:primary_group_flair_bg_color,
:primary_group_flair_color,
:featured_topic
untrusted_attributes :bio_excerpt,
:website,
:website_name,
:location,
:card_background_upload_url
staff_attributes :staged
has_many :featured_user_badges, embed: :ids, serializer: UserBadgeSerializer, root: :user_badges
def include_email?
(object.id && object.id == scope.user.try(:id)) ||
(scope.is_staff? && object.staged?)
end
def bio_excerpt
object.user_profile.bio_excerpt(350, keep_newlines: true, keep_emoji_images: true)
end
def location
object.user_profile.location
end
def website
object.user_profile.website
end
def website_name
uri = begin
URI(website.to_s)
rescue URI::Error
end
return if uri.nil? || uri.host.nil?
uri.host.sub(/^www\./, '') + uri.path
end
def include_website_name
website.present?
end
def ignored
scope_ignored_user_ids = scope.user&.ignored_user_ids || []
scope_ignored_user_ids.include?(object.id)
end
def muted
scope_muted_user_ids = scope.user&.muted_user_ids || []
scope_muted_user_ids.include?(object.id)
end
def can_mute_user
scope.can_mute_user?(object)
end
def can_ignore_user
scope.can_ignore_user?(object)
end
# Needed because 'send_private_message_to_user' will always return false
# when the current user is being serialized
def can_send_private_messages
scope.can_send_private_message?(Discourse.system_user)
end
def can_send_private_message_to_user
scope.can_send_private_message?(object) && scope.current_user != object
end
def include_suspend_reason?
scope.can_see_suspension_reason?(object) && object.suspended?
end
def include_suspended_till?
object.suspended?
end
def user_fields
allowed_keys = scope.allowed_user_field_ids(object).map(&:to_s)
object.user_fields&.select { |k, v| allowed_keys.include?(k) }
end
def include_user_fields?
user_fields.present?
end
def custom_fields
fields = User.whitelisted_user_custom_fields(scope)
if scope.can_edit?(object)
fields += DiscoursePluginRegistry.serialized_current_user_fields.to_a
end
if fields.present?
User.custom_fields_for_ids([object.id], fields)[object.id] || {}
else
{}
end
end
def include_topic_post_count?
topic_post_count.present?
end
def time_read
object.user_stat&.time_read
end
def recent_time_read
time = object.recent_time_read
end
def primary_group_name
object.primary_group.try(:name)
end
def primary_group_flair_url
object.try(:primary_group).try(:flair_url)
end
def primary_group_flair_bg_color
object.try(:primary_group).try(:flair_bg_color)
end
def primary_group_flair_color
object.try(:primary_group).try(:flair_color)
end
def featured_topic
object.user_profile.featured_topic
end
def card_background_upload_url
object.card_background_upload&.url
end
end

View File

@ -1,93 +1,26 @@
# frozen_string_literal: true
class UserSerializer < BasicUserSerializer
class UserSerializer < UserCardSerializer
attr_accessor :topic_post_count
def self.staff_attributes(*attrs)
attributes(*attrs)
attrs.each do |attr|
define_method "include_#{attr}?" do
scope.is_staff?
end
end
end
def self.private_attributes(*attrs)
attributes(*attrs)
attrs.each do |attr|
define_method "include_#{attr}?" do
can_edit
end
end
end
# attributes that are hidden for TL0 users when seen by anonymous
def self.untrusted_attributes(*attrs)
attrs.each do |attr|
method_name = "include_#{attr}?"
define_method(method_name) do
return false if scope.restrict_user_fields?(object)
public_send(attr).present?
end
end
end
attributes :name,
:email,
:last_posted_at,
:last_seen_at,
:bio_raw,
attributes :bio_raw,
:bio_cooked,
:created_at,
:website,
:website_name,
:location,
:can_edit,
:can_edit_username,
:can_edit_email,
:can_edit_name,
:ignored,
:muted,
:can_ignore_user,
:can_mute_user,
:can_send_private_messages,
:can_send_private_message_to_user,
:bio_excerpt,
:trust_level,
:moderator,
:admin,
:title,
:suspend_reason,
:suspended_till,
:uploaded_avatar_id,
:badge_count,
:has_title_badges,
:custom_fields,
:user_fields,
:topic_post_count,
:pending_count,
:profile_view_count,
:time_read,
:recent_time_read,
:primary_group_id,
:primary_group_name,
:primary_group_flair_url,
:primary_group_flair_bg_color,
:primary_group_flair_color,
:staged,
:second_factor_enabled,
:second_factor_backup_enabled,
:second_factor_remaining_backup_codes,
:associated_accounts,
:profile_background_upload_url,
:card_background_upload_url,
:featured_topic
:profile_background_upload_url
has_one :invited_by, embed: :object, serializer: BasicUserSerializer
has_many :groups, embed: :object, serializer: BasicGroupSerializer
has_many :group_users, embed: :object, serializer: BasicGroupUserSerializer
has_many :featured_user_badges, embed: :ids, serializer: UserBadgeSerializer, root: :user_badges
has_one :user_option, embed: :object, serializer: UserOptionSerializer
def include_user_option?
@ -123,12 +56,7 @@ class UserSerializer < BasicUserSerializer
untrusted_attributes :bio_raw,
:bio_cooked,
:bio_excerpt,
:location,
:website,
:website_name,
:profile_background_upload_url,
:card_background_upload_url
###
### ATTRIBUTES
@ -148,11 +76,6 @@ class UserSerializer < BasicUserSerializer
object.group_users.order(:group_id)
end
def include_email?
(object.id && object.id == scope.user.try(:id)) ||
(scope.is_staff? && object.staged?)
end
def include_associated_accounts?
(object.id && object.id == scope.user.try(:id))
end
@ -216,28 +139,6 @@ class UserSerializer < BasicUserSerializer
object.user_profile.bio_processed
end
def website
object.user_profile.website
end
def website_name
uri = begin
URI(website.to_s)
rescue URI::Error
end
return if uri.nil? || uri.host.nil?
uri.host.sub(/^www\./, '') + uri.path
end
def include_website_name
website.present?
end
def location
object.user_profile.location
end
def can_edit
scope.can_edit?(object)
end
@ -254,62 +155,6 @@ class UserSerializer < BasicUserSerializer
scope.can_edit_name?(object)
end
def ignored
scope_ignored_user_ids = scope.user&.ignored_user_ids || []
scope_ignored_user_ids.include?(object.id)
end
def muted
scope_muted_user_ids = scope.user&.muted_user_ids || []
scope_muted_user_ids.include?(object.id)
end
def can_mute_user
scope.can_mute_user?(object)
end
def can_ignore_user
scope.can_ignore_user?(object)
end
# Needed because 'send_private_message_to_user' will always return false
# when the current user is being serialized
def can_send_private_messages
scope.can_send_private_message?(Discourse.system_user)
end
def can_send_private_message_to_user
scope.can_send_private_message?(object) && scope.current_user != object
end
def bio_excerpt
object.user_profile.bio_excerpt(350 , keep_newlines: true, keep_emoji_images: true)
end
def include_suspend_reason?
scope.can_see_suspension_reason?(object) && object.suspended?
end
def include_suspended_till?
object.suspended?
end
def primary_group_name
object.primary_group.try(:name)
end
def primary_group_flair_url
object.try(:primary_group).try(:flair_url)
end
def primary_group_flair_bg_color
object.try(:primary_group).try(:flair_bg_color)
end
def primary_group_flair_color
object.try(:primary_group).try(:flair_color)
end
###
### STAFF ATTRIBUTES
###
@ -413,33 +258,6 @@ class UserSerializer < BasicUserSerializer
object.badges.where(allow_title: true).exists?
end
def user_fields
allowed_keys = scope.allowed_user_field_ids(object).map(&:to_s)
object.user_fields&.select { |k, v| allowed_keys.include?(k) }
end
def include_user_fields?
user_fields.present?
end
def include_topic_post_count?
topic_post_count.present?
end
def custom_fields
fields = User.whitelisted_user_custom_fields(scope)
if scope.can_edit?(object)
fields += DiscoursePluginRegistry.serialized_current_user_fields.to_a
end
if fields.present?
User.custom_fields_for_ids([object.id], fields)[object.id] || {}
else
{}
end
end
def pending_count
0
end
@ -448,27 +266,8 @@ class UserSerializer < BasicUserSerializer
object.user_profile.views
end
def time_read
object.user_stat&.time_read
end
def recent_time_read
time = object.recent_time_read
end
def include_staged?
scope.is_staff?
end
def profile_background_upload_url
object.profile_background_upload&.url
end
def card_background_upload_url
object.card_background_upload&.url
end
def featured_topic
object.user_profile.featured_topic
end
end

View File

@ -478,6 +478,7 @@ Discourse::Application.routes.draw do
get "#{root_path}/:username/profile-hidden" => "users#profile_hidden"
put "#{root_path}/:username/feature-topic" => "users#feature_topic", constraints: { username: RouteFormat.username }
put "#{root_path}/:username/clear-featured-topic" => "users#clear_featured_topic", constraints: { username: RouteFormat.username }
get "#{root_path}/:username/card.json" => "users#show_card", format: :json, constraints: { username: RouteFormat.username }
end
get "user-badges/:username.json" => "user_badges#username", constraints: { username: RouteFormat.username }, defaults: { format: :json }

View File

@ -1976,6 +1976,11 @@ uncategorized:
default: ""
hidden: true
enable_new_user_card_route:
default: false
client: true
hidden: true
user_preferences:
default_email_digest_frequency:
enum: "DigestEmailSiteSetting"

View File

@ -2943,6 +2943,57 @@ describe UsersController do
end
end
describe "#show_card" do
context "anon" do
let(:user) { Discourse.system_user }
it "returns success" do
get "/u/#{user.username}/card.json"
expect(response.status).to eq(200)
parsed = JSON.parse(response.body)["user"]
expect(parsed["username"]).to eq(user.username)
expect(parsed["profile_hidden"]).to be_blank
expect(parsed["trust_level"]).to be_present
end
it "should redirect to login page for anonymous user when profiles are hidden" do
SiteSetting.hide_user_profiles_from_public = true
get "/u/#{user.username}/card.json"
expect(response).to redirect_to '/login'
end
end
context "logged in" do
before do
sign_in(user)
end
fab!(:user) { Fabricate(:user) }
it 'works correctly' do
get "/u/#{user.username}/card.json"
expect(response.status).to eq(200)
json = JSON.parse(response.body)
expect(json["user"]["associated_accounts"]).to eq(nil) # Not serialized in card
expect(json["user"]["username"]).to eq(user.username)
end
it "returns not found when the username doesn't exist" do
get "/u/madeuppity/card.json"
expect(response).not_to be_successful
end
it "raises an error on invalid access" do
Guardian.any_instance.expects(:can_see?).with(user).returns(false)
get "/u/#{user.username}/card.json"
expect(response).to be_forbidden
end
end
end
describe '#badges' do
it "renders fine by default" do
get "/u/#{user.username}/badges"