diff --git a/jobs/regular/assign_notification.rb b/jobs/regular/assign_notification.rb index 9da8882..4c3770c 100644 --- a/jobs/regular/assign_notification.rb +++ b/jobs/regular/assign_notification.rb @@ -8,7 +8,7 @@ module Jobs raise Discourse::InvalidParameters.new(:assigned_to_id) if args[:assigned_to_id].nil? raise Discourse::InvalidParameters.new(:assigned_to_type) if args[:assigned_to_type].nil? raise Discourse::InvalidParameters.new(:assigned_by_id) if args[:assigned_by_id].nil? - raise Discourse::InvalidParameters.new(:silent) if args[:silent].nil? + raise Discourse::InvalidParameters.new(:skip_small_action_post) if args[:skip_small_action_post].nil? topic = Topic.find(args[:topic_id]) post = Post.find(args[:post_id]) @@ -37,7 +37,7 @@ module Jobs ) ) - next if args[:silent] + next if args[:skip_small_action_post] Notification.create!( notification_type: Notification.types[:assigned] || Notification.types[:custom], user_id: user.id, diff --git a/lib/assigner.rb b/lib/assigner.rb index ec8c801..b9d06ed 100644 --- a/lib/assigner.rb +++ b/lib/assigner.rb @@ -69,9 +69,9 @@ class ::Assigner if is_last_staff_post?(post) assigner = new(post.topic, post.user) if assign_other - assigner.assign(assign_other, silent: true) + assigner.assign(assign_other, skip_small_action_post: true) elsif assign_self - assigner.assign(assign_self, silent: true) + assigner.assign(assign_self, skip_small_action_post: true) end end end @@ -199,47 +199,33 @@ class ::Assigner end end - def assign(assign_to, note: nil, silent: false) - type = assign_to.is_a?(User) ? "User" : "Group" + def assign(assign_to, note: nil, skip_small_action_post: false) + assigned_to_type = assign_to.is_a?(User) ? "User" : "Group" - forbidden_reason = forbidden_reasons(assign_to: assign_to, type: type, note: note) + forbidden_reason = forbidden_reasons(assign_to: assign_to, type: assigned_to_type, note: note) return { success: false, reason: forbidden_reason } if forbidden_reason action_code = {} action_code[:user] = topic.assignment.present? ? "reassigned" : "assigned" action_code[:group] = topic.assignment.present? ? "reassigned_group" : "assigned_group" - silent = silent || no_assignee_change?(assign_to) + skip_small_action_post = skip_small_action_post || no_assignee_change?(assign_to) @target.assignment&.destroy! - assignment = @target.create_assignment!(assigned_to_id: assign_to.id, assigned_to_type: type, assigned_by_user_id: @assigned_by.id, topic_id: topic.id, note: note) + assignment = @target.create_assignment!(assigned_to_id: assign_to.id, assigned_to_type: assigned_to_type, assigned_by_user_id: @assigned_by.id, topic_id: topic.id, note: note) first_post.publish_change_to_clients!(:revised, reload_topic: true) - serializer = assignment.assigned_to_user? ? BasicUserSerializer : BasicGroupSerializer - Jobs.enqueue(:assign_notification, topic_id: topic.id, post_id: topic_target? ? first_post.id : @target.id, assigned_to_id: assign_to.id, - assigned_to_type: type, + assigned_to_type: assigned_to_type, assigned_by_id: @assigned_by.id, - silent: silent) + skip_small_action_post: skip_small_action_post) - MessageBus.publish( - "/staff/topic-assignment", - { - type: "assigned", - topic_id: topic.id, - post_id: post_target? && @target.id, - post_number: post_target? && @target.post_number, - assigned_type: type, - assigned_to: serializer.new(assign_to, scope: Guardian.new, root: false).as_json, - assignment_note: note, - }, - user_ids: allowed_user_ids - ) + publish_assignment(assignment, assign_to, note) if assignment.assigned_to_user? if !TopicUser.exists?( @@ -262,7 +248,8 @@ class ::Assigner end end end - if !silent + + unless skip_small_action_post custom_fields = { "action_code_who" => assign_to.is_a?(User) ? assign_to.username : assign_to.name } if post_target? @@ -281,9 +268,9 @@ class ::Assigner # Create a webhook event if WebHook.active_web_hooks(:assign).exists? - type = :assigned + assigned_to_type = :assigned payload = { - type: type, + type: assigned_to_type, topic_id: topic.id, topic_title: topic.title, assigned_by_id: @assigned_by.id, @@ -300,7 +287,7 @@ class ::Assigner assigned_to_group_name: assign_to.name, }) end - WebHook.enqueue_assign_hooks(type, payload.to_json) + WebHook.enqueue_assign_hooks(assigned_to_type, payload.to_json) end { success: true } @@ -398,6 +385,23 @@ class ::Assigner private + def publish_assignment(assignment, assign_to, note) + serializer = assignment.assigned_to_user? ? BasicUserSerializer : BasicGroupSerializer + MessageBus.publish( + "/staff/topic-assignment", + { + type: "assigned", + topic_id: topic.id, + post_id: post_target? && @target.id, + post_number: post_target? && @target.post_number, + assigned_type: assignment.assigned_to_type, + assigned_to: serializer.new(assign_to, scope: Guardian.new, root: false).as_json, + assignment_note: note, + }, + user_ids: allowed_user_ids + ) + end + def moderator_post_assign_action_code(assignment, action_code) if assignment.target.is_a?(Post) # posts do not have to handle conditions of 'assign' or 'reassign' diff --git a/plugin.rb b/plugin.rb index 15763e5..ad6d1f0 100644 --- a/plugin.rb +++ b/plugin.rb @@ -816,7 +816,7 @@ after_initialize do assigned_to_id: assignment.assigned_to_id, assigned_to_type: assignment.assigned_to_type, assigned_by_id: assignment.assigned_by_user_id, - silent: true) + skip_small_action_post: true) end end diff --git a/spec/jobs/regular/assign_notification_spec.rb b/spec/jobs/regular/assign_notification_spec.rb index 0aa4eb2..6f639ac 100644 --- a/spec/jobs/regular/assign_notification_spec.rb +++ b/spec/jobs/regular/assign_notification_spec.rb @@ -29,7 +29,7 @@ RSpec.describe Jobs::AssignNotification do it 'sends notification alert' do messages = MessageBus.track_publish("/notification-alert/#{user2.id}") do - described_class.new.execute({ topic_id: topic.id, post_id: post.id, assigned_to_id: user2.id, assigned_to_type: 'User', assigned_by_id: user1.id, silent: false }) + described_class.new.execute({ topic_id: topic.id, post_id: post.id, assigned_to_id: user2.id, assigned_to_type: 'User', assigned_by_id: user1.id, skip_small_action_post: false }) end expect(messages.length).to eq(1) @@ -41,7 +41,7 @@ RSpec.describe Jobs::AssignNotification do assign_allowed_group.add(user) assert_publish_topic_state(pm, user) do - described_class.new.execute({ topic_id: pm.id, post_id: pm_post.id, assigned_to_id: pm.allowed_users.first.id, assigned_to_type: 'User', assigned_by_id: user1.id, silent: false }) + described_class.new.execute({ topic_id: pm.id, post_id: pm_post.id, assigned_to_id: pm.allowed_users.first.id, assigned_to_type: 'User', assigned_by_id: user1.id, skip_small_action_post: false }) end end @@ -58,7 +58,7 @@ RSpec.describe Jobs::AssignNotification do topic_title: topic.title }.to_json ) - described_class.new.execute({ topic_id: topic.id, post_id: post.id, assigned_to_id: user2.id, assigned_to_type: 'User', assigned_by_id: user1.id, silent: false }) + described_class.new.execute({ topic_id: topic.id, post_id: post.id, assigned_to_id: user2.id, assigned_to_type: 'User', assigned_by_id: user1.id, skip_small_action_post: false }) end end @@ -76,19 +76,19 @@ RSpec.describe Jobs::AssignNotification do it 'sends notification alert to all group members' do messages = MessageBus.track_publish("/notification-alert/#{user2.id}") do - described_class.new.execute({ topic_id: topic.id, post_id: post.id, assigned_to_id: group.id, assigned_to_type: 'Group', assigned_by_id: user1.id, silent: false }) + described_class.new.execute({ topic_id: topic.id, post_id: post.id, assigned_to_id: group.id, assigned_to_type: 'Group', assigned_by_id: user1.id, skip_small_action_post: false }) end expect(messages.length).to eq(1) expect(messages.first.data[:excerpt]).to eq("assigned to Developers the topic 'Basic topic title'") messages = MessageBus.track_publish("/notification-alert/#{user3.id}") do - described_class.new.execute({ topic_id: topic.id, post_id: post.id, assigned_to_id: group.id, assigned_to_type: 'Group', assigned_by_id: user1.id, silent: false }) + described_class.new.execute({ topic_id: topic.id, post_id: post.id, assigned_to_id: group.id, assigned_to_type: 'Group', assigned_by_id: user1.id, skip_small_action_post: false }) end expect(messages.length).to eq(1) expect(messages.first.data[:excerpt]).to eq("assigned to Developers the topic 'Basic topic title'") messages = MessageBus.track_publish("/notification-alert/#{user4.id}") do - described_class.new.execute({ topic_id: topic.id, post_id: post.id, assigned_to_id: group.id, assigned_to_type: 'Group', assigned_by_id: user1.id, silent: false }) + described_class.new.execute({ topic_id: topic.id, post_id: post.id, assigned_to_id: group.id, assigned_to_type: 'Group', assigned_by_id: user1.id, skip_small_action_post: false }) end expect(messages.length).to eq(0) end @@ -109,7 +109,7 @@ RSpec.describe Jobs::AssignNotification do ) end - described_class.new.execute({ topic_id: topic.id, post_id: post.id, assigned_to_id: group.id, assigned_to_type: 'Group', assigned_by_id: user1.id, silent: false }) + described_class.new.execute({ topic_id: topic.id, post_id: post.id, assigned_to_id: group.id, assigned_to_type: 'Group', assigned_by_id: user1.id, skip_small_action_post: false }) end end end diff --git a/spec/jobs/regular/unassign_notification_spec.rb b/spec/jobs/regular/unassign_notification_spec.rb index ae087f3..6814d4a 100644 --- a/spec/jobs/regular/unassign_notification_spec.rb +++ b/spec/jobs/regular/unassign_notification_spec.rb @@ -27,7 +27,7 @@ RSpec.describe Jobs::UnassignNotification do context 'User' do it 'deletes notifications' do - Jobs::AssignNotification.new.execute({ topic_id: topic.id, post_id: post.id, assigned_to_id: user2.id, assigned_to_type: 'User', assigned_by_id: user1.id, silent: false }) + Jobs::AssignNotification.new.execute({ topic_id: topic.id, post_id: post.id, assigned_to_id: user2.id, assigned_to_type: 'User', assigned_by_id: user1.id, skip_small_action_post: false }) expect { described_class.new.execute({ topic_id: topic.id, post_id: post.id, assigned_to_id: user2.id, assigned_to_type: 'User' }) @@ -55,7 +55,7 @@ RSpec.describe Jobs::UnassignNotification do end it 'deletes notifications' do - Jobs::AssignNotification.new.execute({ topic_id: topic.id, post_id: post.id, assigned_to_id: group.id, assigned_to_type: 'Group', assigned_by_id: user1.id, silent: false }) + Jobs::AssignNotification.new.execute({ topic_id: topic.id, post_id: post.id, assigned_to_id: group.id, assigned_to_type: 'Group', assigned_by_id: user1.id, skip_small_action_post: false }) expect { described_class.new.execute({ topic_id: topic.id, post_id: post.id, assigned_to_id: group.id, assigned_to_type: 'Group' }) diff --git a/spec/lib/assigner_spec.rb b/spec/lib/assigner_spec.rb index f3e7038..6eabffa 100644 --- a/spec/lib/assigner_spec.rb +++ b/spec/lib/assigner_spec.rb @@ -147,8 +147,6 @@ RSpec.describe Assigner do assigner.assign(assignee).fetch(:success) end - fab!(:admin) { Fabricate(:admin) } - context "forbidden reasons" do it "doesn't assign if the user has too many assigned topics" do SiteSetting.max_assigned_topics = 1 @@ -203,7 +201,7 @@ RSpec.describe Assigner do end it 'fails to assign when the assigned user and note is the same' do - assigner = described_class.new(topic, admin) + assigner = described_class.new(topic, moderator_2) assigner.assign(moderator, note: "note me down") assign = assigner.assign(moderator, note: "note me down") @@ -213,7 +211,7 @@ RSpec.describe Assigner do end it 'allows assign when the assigned user is same but note is different' do - assigner = described_class.new(topic, admin) + assigner = described_class.new(topic, moderator_2) assigner.assign(moderator, note: "note me down") assign = assigner.assign(moderator, note: "note me down again") @@ -222,28 +220,28 @@ RSpec.describe Assigner do end it 'fails to assign when the assigned user cannot view the pm' do - assign = described_class.new(pm, admin).assign(moderator) + assign = described_class.new(pm, moderator_2).assign(moderator) expect(assign[:success]).to eq(false) expect(assign[:reason]).to eq(:forbidden_assignee_not_pm_participant) end it 'fails to assign when not all group members has access to pm' do - assign = described_class.new(pm, admin).assign(moderator.groups.first) + assign = described_class.new(pm, moderator_2).assign(moderator.groups.first) expect(assign[:success]).to eq(false) expect(assign[:reason]).to eq(:forbidden_group_assignee_not_pm_participant) end it 'fails to assign when the assigned user cannot view the topic' do - assign = described_class.new(secure_topic, admin).assign(moderator) + assign = described_class.new(secure_topic, moderator_2).assign(moderator) expect(assign[:success]).to eq(false) expect(assign[:reason]).to eq(:forbidden_assignee_cant_see_topic) end it 'fails to assign when the not all group members can view the topic' do - assign = described_class.new(secure_topic, admin).assign(moderator.groups.first) + assign = described_class.new(secure_topic, moderator_2).assign(moderator.groups.first) expect(assign[:success]).to eq(false) expect(assign[:reason]).to eq(:forbidden_group_assignee_cant_see_topic) @@ -253,7 +251,7 @@ RSpec.describe Assigner do it "assigns the PM to the moderator when it's included in the list of allowed users" do pm.allowed_users << moderator - assign = described_class.new(pm, admin).assign(moderator) + assign = described_class.new(pm, moderator_2).assign(moderator) expect(assign[:success]).to eq(true) end @@ -261,7 +259,7 @@ RSpec.describe Assigner do it "assigns the PM to the moderator when it's a member of an allowed group" do pm.allowed_groups << assign_allowed_group - assign = described_class.new(pm, admin).assign(moderator) + assign = described_class.new(pm, moderator_2).assign(moderator) expect(assign[:success]).to eq(true) end