From 8144730ebb7a98df0ab1acdc2d4dc28fc1d7a336 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Fri, 17 Feb 2023 17:07:44 +0100 Subject: [PATCH] FIX: correctly add user info data to message serializer (#20348) Previous commit https://github.com/discourse/discourse/commit/479c0a3051210da82b127c31088926c4f2a1074a was done with the assumption that this info was defined on user serializer but it was actually defined on post serializer in core. This commit extends the user serializer for messages to add this data to the user. Also correctly adds serializer test to ensure we actually have this data. --- .../chat/app/controllers/chat_controller.rb | 2 +- .../serializers/chat_message_serializer.rb | 2 +- .../chat_message_user_serializer.rb | 26 +++++++ .../discourse/components/chat-message-info.js | 4 +- plugins/chat/plugin.rb | 1 + .../chat_message_user_serializer_spec.rb | 67 +++++++++++++++++++ .../components/chat-message-info-test.js | 5 -- 7 files changed, 97 insertions(+), 10 deletions(-) create mode 100644 plugins/chat/app/serializers/chat_message_user_serializer.rb create mode 100644 plugins/chat/spec/serializer/chat_message_user_serializer_spec.rb diff --git a/plugins/chat/app/controllers/chat_controller.rb b/plugins/chat/app/controllers/chat_controller.rb index 3ac2770f5f..ffcad435e5 100644 --- a/plugins/chat/app/controllers/chat_controller.rb +++ b/plugins/chat/app/controllers/chat_controller.rb @@ -446,7 +446,7 @@ class Chat::ChatController < Chat::ChatBaseController ChatMessage .includes(in_reply_to: [:user, chat_webhook_event: [:incoming_chat_webhook]]) .includes(:revisions) - .includes(:user) + .includes(user: :primary_group) .includes(chat_webhook_event: :incoming_chat_webhook) .includes(reactions: :user) .includes(:bookmarks) diff --git a/plugins/chat/app/serializers/chat_message_serializer.rb b/plugins/chat/app/serializers/chat_message_serializer.rb index e13ba2bb94..66e9653540 100644 --- a/plugins/chat/app/serializers/chat_message_serializer.rb +++ b/plugins/chat/app/serializers/chat_message_serializer.rb @@ -17,7 +17,7 @@ class ChatMessageSerializer < ApplicationSerializer :thread_id, :chat_channel_id - has_one :user, serializer: BasicUserWithStatusSerializer, embed: :objects + has_one :user, serializer: ChatMessageUserSerializer, embed: :objects has_one :chat_webhook_event, serializer: ChatWebhookEventSerializer, embed: :objects has_one :in_reply_to, serializer: ChatInReplyToSerializer, embed: :objects has_many :uploads, serializer: UploadSerializer, embed: :objects diff --git a/plugins/chat/app/serializers/chat_message_user_serializer.rb b/plugins/chat/app/serializers/chat_message_user_serializer.rb new file mode 100644 index 0000000000..25e3f2518b --- /dev/null +++ b/plugins/chat/app/serializers/chat_message_user_serializer.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +class ChatMessageUserSerializer < BasicUserWithStatusSerializer + attributes :moderator?, :admin?, :staff?, :moderator?, :new_user?, :primary_group_name + + def moderator? + !!(object&.moderator?) + end + + def admin? + !!(object&.admin?) + end + + def staff? + !!(object&.staff?) + end + + def new_user? + object.trust_level == TrustLevel[0] + end + + def primary_group_name + return nil unless object && object.primary_group_id + object.primary_group.name if object.primary_group + end +end diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-message-info.js b/plugins/chat/assets/javascripts/discourse/components/chat-message-info.js index c33af19bca..9347fa2d25 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-message-info.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat-message-info.js @@ -18,6 +18,7 @@ export default class ChatMessageInfo extends Component { get usernameClasses() { const user = this.#user; + const classes = this.prioritizeName ? ["is-full-name"] : ["is-username"]; if (!user) { return classes; @@ -31,9 +32,6 @@ export default class ChatMessageInfo extends Component { if (user.moderator) { classes.push("is-moderator"); } - if (user.groupModerator) { - classes.push("is-category-moderator"); - } if (user.new_user) { classes.push("is-new-user"); } diff --git a/plugins/chat/plugin.rb b/plugins/chat/plugin.rb index 71cf26a309..b68743b24d 100644 --- a/plugins/chat/plugin.rb +++ b/plugins/chat/plugin.rb @@ -148,6 +148,7 @@ after_initialize do load File.expand_path("../app/models/reviewable_chat_message.rb", __FILE__) load File.expand_path("../app/models/chat_view.rb", __FILE__) load File.expand_path("../app/models/category_channel.rb", __FILE__) + load File.expand_path("../app/serializers/chat_message_user_serializer.rb", __FILE__) load File.expand_path("../app/serializers/structured_channel_serializer.rb", __FILE__) load File.expand_path("../app/serializers/chat_webhook_event_serializer.rb", __FILE__) load File.expand_path("../app/serializers/chat_in_reply_to_serializer.rb", __FILE__) diff --git a/plugins/chat/spec/serializer/chat_message_user_serializer_spec.rb b/plugins/chat/spec/serializer/chat_message_user_serializer_spec.rb new file mode 100644 index 0000000000..270f331902 --- /dev/null +++ b/plugins/chat/spec/serializer/chat_message_user_serializer_spec.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe ChatMessageUserSerializer do + subject do + user = Fabricate(:user, **params) + guardian = Guardian.new(user) + described_class.new(user, scope: guardian, root: nil).as_json + end + + let(:params) do + { trust_level: TrustLevel[1], admin: false, moderator: false, primary_group_id: nil } + end + + context "with default user" do + it "displays user as regular" do + expect(subject[:new_user]).to eq(false) + expect(subject[:staff]).to eq(false) + expect(subject[:admin]).to eq(false) + expect(subject[:moderator]).to eq(false) + expect(subject[:primary_group_name]).to be_blank + end + end + + context "when user is TL0" do + before { params[:trust_level] = TrustLevel[0] } + + it "displays user as new" do + expect(subject[:new_user]).to eq(true) + end + end + + context "when user is staff" do + before { params[:admin] = true } + + it "displays user as staff" do + expect(subject[:staff]).to eq(true) + end + end + + context "when user is admin" do + before { params[:admin] = true } + + it "displays user as admin" do + expect(subject[:admin]).to eq(true) + end + end + + context "when user is moderator" do + before { params[:moderator] = true } + + it "displays user as moderator" do + expect(subject[:moderator]).to eq(true) + end + end + + context "when user has a primary group" do + fab!(:group) { Fabricate(:group) } + + before { params[:primary_group_id] = group.id } + + it "displays user as moderator" do + expect(subject[:primary_group_name]).to eq(group.name) + end + end +end diff --git a/plugins/chat/test/javascripts/components/chat-message-info-test.js b/plugins/chat/test/javascripts/components/chat-message-info-test.js index 989656e803..2875e2a73c 100644 --- a/plugins/chat/test/javascripts/components/chat-message-info-test.js +++ b/plugins/chat/test/javascripts/components/chat-message-info-test.js @@ -135,7 +135,6 @@ module("Discourse Chat | Component | chat-message-info", function (hooks) { username: "discobot", admin: true, moderator: true, - groupModerator: true, new_user: true, primary_group_name: "foo", }, @@ -147,7 +146,6 @@ module("Discourse Chat | Component | chat-message-info", function (hooks) { assert.dom(".chat-message-info__username.is-staff").exists(); assert.dom(".chat-message-info__username.is-admin").exists(); assert.dom(".chat-message-info__username.is-moderator").exists(); - assert.dom(".chat-message-info__username.is-category-moderator ").exists(); assert.dom(".chat-message-info__username.is-new-user").exists(); assert.dom(".chat-message-info__username.group--foo").exists(); }); @@ -160,9 +158,6 @@ module("Discourse Chat | Component | chat-message-info", function (hooks) { assert.dom(".chat-message-info__username.is-staff").doesNotExist(); assert.dom(".chat-message-info__username.is-admin").doesNotExist(); assert.dom(".chat-message-info__username.is-moderator").doesNotExist(); - assert - .dom(".chat-message-info__username.is-category-moderator ") - .doesNotExist(); assert.dom(".chat-message-info__username.is-new-user").doesNotExist(); assert.dom(".chat-message-info__username.group--foo").doesNotExist(); });