From 13515a2d4b3c530664d0be49acaac74353a5aa36 Mon Sep 17 00:00:00 2001 From: Nat Date: Tue, 10 May 2022 14:28:33 +0800 Subject: [PATCH 1/2] Shows note in moderator post --- lib/assigner.rb | 2 +- spec/lib/assigner_spec.rb | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/assigner.rb b/lib/assigner.rb index b9d06ede..f571548d 100644 --- a/lib/assigner.rb +++ b/lib/assigner.rb @@ -258,7 +258,7 @@ def assign(assign_to, note: nil, skip_small_action_post: false) topic.add_moderator_post( @assigned_by, - nil, + note, bump: false, post_type: SiteSetting.assigns_public ? Post.types[:small_action] : Post.types[:whisper], action_code: moderator_post_assign_action_code(assignment, action_code), diff --git a/spec/lib/assigner_spec.rb b/spec/lib/assigner_spec.rb index 6eabffaf..867c995a 100644 --- a/spec/lib/assigner_spec.rb +++ b/spec/lib/assigner_spec.rb @@ -49,6 +49,11 @@ expect(topic.assignment.note).to eq "tomtom best mom" end + it "assign with note adds moderator post with note" do + expect { assigner.assign(moderator, note: "tomtom best mom") }.to change { topic.posts.count }.by(1) + expect(topic.posts.last.raw).to eq "tomtom best mom" + end + it "publishes topic assignment after assign and unassign" do messages = MessageBus.track_publish('/staff/topic-assignment') do assigner = described_class.new(topic, moderator_2) From 01237623b4edece10edbb4ed0c6573b567d41cd2 Mon Sep 17 00:00:00 2001 From: Nat Date: Thu, 12 May 2022 22:13:06 +0800 Subject: [PATCH 2/2] Show more specific small action post for note-changes --- config/locales/client.en.yml | 1 + lib/assigner.rb | 73 +++++++++++++++++++++++++----------- spec/lib/assigner_spec.rb | 55 +++++++++++++++++++++++++++ 3 files changed, 108 insertions(+), 21 deletions(-) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index f638e474..e3cdd88b 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -15,6 +15,7 @@ en: unassigned_group_from_post: "unassigned %{who} from post %{when}" reassigned: "Reassigned %{who} %{when}" reassigned_group: "Reassigned %{who} %{when}" + note_change: "changed assignment note for %{who} %{when}" discourse_assign: add_unassigned_filter: "Add 'unassigned' filter to category" cant_act: "You cannot act on flags that have been assigned to other users" diff --git a/lib/assigner.rb b/lib/assigner.rb index f571548d..58a8d20f 100644 --- a/lib/assigner.rb +++ b/lib/assigner.rb @@ -199,12 +199,34 @@ def forbidden_reasons(assign_to:, type:, note:) end end + def update_note(assign_to, note, skip_small_action_post: false) + @target.assignment.update!(note: note) + + queue_notification(assign_to, skip_small_action_post) + + assignment = @target.assignment + publish_assignment(assignment, assign_to, note) + + # email is skipped, for now + + unless skip_small_action_post + action_code = "note_change" + add_small_action_post(action_code, assign_to, note) + end + + { success: true } + end + 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: assigned_to_type, note: note) return { success: false, reason: forbidden_reason } if forbidden_reason + if no_assignee_change?(assign_to) + return update_note(assign_to, note, skip_small_action_post: skip_small_action_post) + end + action_code = {} action_code[:user] = topic.assignment.present? ? "reassigned" : "assigned" action_code[:group] = topic.assignment.present? ? "reassigned_group" : "assigned_group" @@ -217,13 +239,7 @@ def assign(assign_to, note: nil, skip_small_action_post: false) first_post.publish_change_to_clients!(:revised, reload_topic: true) - 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: assigned_to_type, - assigned_by_id: @assigned_by.id, - skip_small_action_post: skip_small_action_post) + queue_notification(assign_to, skip_small_action_post) publish_assignment(assignment, assign_to, note) @@ -250,20 +266,8 @@ def assign(assign_to, note: nil, skip_small_action_post: false) end unless skip_small_action_post - custom_fields = { "action_code_who" => assign_to.is_a?(User) ? assign_to.username : assign_to.name } - - if post_target? - custom_fields.merge!({ "action_code_path" => "/p/#{@target.id}", "action_code_post_id" => @target.id }) - end - - topic.add_moderator_post( - @assigned_by, - note, - bump: false, - post_type: SiteSetting.assigns_public ? Post.types[:small_action] : Post.types[:whisper], - action_code: moderator_post_assign_action_code(assignment, action_code), - custom_fields: custom_fields - ) + post_action_code = moderator_post_assign_action_code(assignment, action_code) + add_small_action_post(post_action_code, assign_to, note) end # Create a webhook event @@ -385,6 +389,33 @@ def unassign(silent: false, deactivate: false) private + def queue_notification(assign_to, skip_small_action_post) + 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: assign_to.is_a?(User) ? "User" : "Group", + assigned_by_id: @assigned_by.id, + skip_small_action_post: skip_small_action_post) + end + + def add_small_action_post(action_code, assign_to, note) + custom_fields = { "action_code_who" => assign_to.is_a?(User) ? assign_to.username : assign_to.name } + + if post_target? + custom_fields.merge!({ "action_code_path" => "/p/#{@target.id}", "action_code_post_id" => @target.id }) + end + + topic.add_moderator_post( + @assigned_by, + note, + bump: false, + post_type: SiteSetting.assigns_public ? Post.types[:small_action] : Post.types[:whisper], + action_code: action_code, + custom_fields: custom_fields + ) + end + def publish_assignment(assignment, assign_to, note) serializer = assignment.assigned_to_user? ? BasicUserSerializer : BasicGroupSerializer MessageBus.publish( diff --git a/spec/lib/assigner_spec.rb b/spec/lib/assigner_spec.rb index 867c995a..b352622a 100644 --- a/spec/lib/assigner_spec.rb +++ b/spec/lib/assigner_spec.rb @@ -274,6 +274,61 @@ def assigned_to?(assignee) described_class.new(secure_category, moderator).assign(moderator) end.to raise_error(Discourse::InvalidAccess) end + + describe "updating notes" do + it "does not recreate assignment if no assignee change" do + assigner.assign(moderator) + + expect do + assigner.assign(moderator, note: "new notes!") + end.to_not change { Assignment.last.id } + end + + it "updates notes" do + assigner.assign(moderator) + + assigner.assign(moderator, note: "new notes!") + + expect(Assignment.last.note).to eq "new notes!" + end + + it "queues notification" do + assigner.assign(moderator) + + expect_enqueued_with(job: :assign_notification) do + assigner.assign(moderator, note: "new notes!") + end + end + + it "publishes topic assignment with note" do + assigner.assign(moderator) + + messages = MessageBus.track_publish('/staff/topic-assignment') do + assigner = described_class.new(topic, moderator_2) + assigner.assign(moderator, note: "new notes!") + end + + expect(messages[0].channel).to eq "/staff/topic-assignment" + expect(messages[0].data).to include({ + type: "assigned", + topic_id: topic.id, + post_id: false, + post_number: false, + assigned_type: "User", + assigned_to: BasicUserSerializer.new(moderator, scope: Guardian.new, root: false).as_json, + assignment_note: "new notes!" + }) + end + + it "adds a note_change small action post" do + assigner.assign(moderator) + + assigner.assign(moderator, note: "new notes!") + + small_action_post = topic.posts.last + expect(small_action_post.action_code).to eq "note_change" + end + end end context "assign_self_regex" do