From 416d19af27fc3e33d6f406517e4ea2726881ebf9 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Fri, 18 May 2018 11:28:13 +0800 Subject: [PATCH] FIX: Wrong target user displayed for user actions in activity stream. https://meta.discourse.org/t/wrong-assigned-username-in-activity-list/73816 --- .../components/user-stream-item.js.es6 | 2 +- app/models/user_action.rb | 2 + app/serializers/user_action_serializer.rb | 9 ++++ spec/models/user_action_spec.rb | 54 +++++++++++++------ 4 files changed, 51 insertions(+), 16 deletions(-) diff --git a/app/assets/javascripts/discourse/components/user-stream-item.js.es6 b/app/assets/javascripts/discourse/components/user-stream-item.js.es6 index 103f450b245..b239613c477 100644 --- a/app/assets/javascripts/discourse/components/user-stream-item.js.es6 +++ b/app/assets/javascripts/discourse/components/user-stream-item.js.es6 @@ -11,5 +11,5 @@ export default Ember.Component.extend({ ], moderatorAction: propertyEqual("item.post_type", "site.post_types.moderator_action"), - actionDescription: actionDescription("item.action_code", "item.created_at", "item.username"), + actionDescription: actionDescription("item.action_code", "item.created_at", "item.action_code_who"), }); diff --git a/app/models/user_action.rb b/app/models/user_action.rb index 20a3acde6cd..f36c034b0ec 100644 --- a/app/models/user_action.rb +++ b/app/models/user_action.rb @@ -212,6 +212,7 @@ SQL p.hidden, p.post_type, p.action_code, + pc.value AS action_code_who, p.edit_reason, t.category_id FROM user_actions as a @@ -222,6 +223,7 @@ SQL JOIN users pu on pu.id = COALESCE(p.user_id, t.user_id) JOIN users au on au.id = a.user_id LEFT JOIN categories c on c.id = t.category_id + LEFT JOIN post_custom_fields pc ON pc.post_id = a.target_post_id AND pc.name = 'action_code_who' /*where*/ /*order_by*/ /*offset*/ diff --git a/app/serializers/user_action_serializer.rb b/app/serializers/user_action_serializer.rb index c6abea6a5ce..2aec75ceede 100644 --- a/app/serializers/user_action_serializer.rb +++ b/app/serializers/user_action_serializer.rb @@ -26,6 +26,7 @@ class UserActionSerializer < ApplicationSerializer :hidden, :post_type, :action_code, + :action_code_who, :edit_reason, :category_id, :closed, @@ -79,4 +80,12 @@ class UserActionSerializer < ApplicationSerializer object.topic_archived end + def include_action_code_who? + action_code_who.present? + end + + def action_code_who + object.action_code_who + end + end diff --git a/spec/models/user_action_spec.rb b/spec/models/user_action_spec.rb index 1988fecdf68..6ab8a1537ab 100644 --- a/spec/models/user_action_spec.rb +++ b/spec/models/user_action_spec.rb @@ -9,7 +9,7 @@ describe UserAction do it { is_expected.to validate_presence_of :action_type } it { is_expected.to validate_presence_of :user_id } - describe 'lists' do + describe '#stream' do let(:public_post) { Fabricate(:post) } let(:public_topic) { public_post.topic } @@ -46,8 +46,8 @@ describe UserAction do UserAction.stats(user.id, Guardian.new(viewer)).map { |r| r["action_type"].to_i }.sort end - def stream_count(viewer = nil) - UserAction.stream(user_id: user.id, guardian: Guardian.new(viewer)).count + def stream(viewer = nil) + UserAction.stream(user_id: user.id, guardian: Guardian.new(viewer)) end it 'includes the events correctly' do @@ -56,38 +56,38 @@ describe UserAction do mystats = stats_for_user(user) expecting = [UserAction::NEW_TOPIC, UserAction::NEW_PRIVATE_MESSAGE, UserAction::GOT_PRIVATE_MESSAGE, UserAction::BOOKMARK].sort expect(mystats).to eq(expecting) - expect(stream_count(user)).to eq(4) + + expect(stream(user).map(&:action_type)) + .to contain_exactly(*expecting) other_stats = stats_for_user expecting = [UserAction::NEW_TOPIC] - expect(stream_count).to eq(1) - + expect(stream.map(&:action_type)).to contain_exactly(*expecting) expect(other_stats).to eq(expecting) public_topic.trash!(user) expect(stats_for_user).to eq([]) - expect(stream_count).to eq(0) + expect(stream).to eq([]) # groups category = Fabricate(:category, read_restricted: true) public_topic.recover! - public_topic.category = category - public_topic.save + public_topic.update!(category: category) expect(stats_for_user).to eq([]) - expect(stream_count).to eq(0) + expect(stream).to eq([]) group = Fabricate(:group) u = Fabricate(:coding_horror) group.add(u) - group.save category.set_permissions(group => :full) - category.save + category.save! - expect(stats_for_user(u)).to eq([UserAction::NEW_TOPIC]) - expect(stream_count(u)).to eq(1) + expecting = [UserAction::NEW_TOPIC] + expect(stats_for_user(u)).to eq(expecting) + expect(stream(u).map(&:action_type)).to contain_exactly(*expecting) # duplicate should not exception out log_test_action @@ -103,6 +103,29 @@ describe UserAction do end end + describe 'assignments' do + let(:stream) do + UserAction.stream(user_id: user.id, guardian: Guardian.new(user)) + end + + before do + log_test_action(action_type: UserAction::ASSIGNED) + private_post.custom_fields ||= {} + private_post.custom_fields["action_code_who"] = 'testing' + private_post.custom_fields["random_field"] = 'random_value' + private_post.save! + end + + it 'should include the right attributes in the stream' do + expect(stream.count).to eq(1) + + user_action_row = stream.first + + expect(user_action_row.action_type).to eq(UserAction::ASSIGNED) + expect(user_action_row.action_code_who).to eq('testing') + end + end + describe "mentions" do before do log_test_action(action_type: UserAction::MENTION) @@ -116,7 +139,8 @@ describe UserAction do end it "is returned by the stream" do - expect(stream).to be_present + expect(stream.count).to eq(1) + expect(stream.first.action_type).to eq(UserAction::MENTION) end it "isn't returned when mentions aren't enabled" do