FEATURE: Add assign note (#326)

app/controllers/discourse_assign/assign_controller.rb

@@ -48,6 +48,7 @@ module DiscourseAssign
       target_type = params.require(:target_type)
       username = params.permit(:username)['username']
       group_name = params.permit(:group_name)['group_name']
+      note = params.permit(:note)['note'].presence
 
       assign_to = username.present? ? User.find_by(username_lower: username.downcase) : Group.where("LOWER(name) = ?", group_name.downcase).first
 
@@ -56,7 +57,7 @@ module DiscourseAssign
       target = target_type.constantize.where(id: target_id).first
       raise Discourse::NotFound unless target
 
-      assign = Assigner.new(target, current_user).assign(assign_to)
+      assign = Assigner.new(target, current_user).assign(assign_to, note: note)
 
       if assign[:success]
         render json: success_json

app/models/assignment.rb

@@ -37,6 +37,7 @@ end
 #  target_id           :integer          not null
 #  target_type         :string           not null
 #  active              :boolean          default(TRUE)
+#  note                :string
 #
 # Indexes
 #

assets/javascripts/discourse-assign/controllers/assign-user.js

@@ -1,10 +1,10 @@
 import Controller, { inject as controller } from "@ember/controller";
+import { action } from "@ember/object";
+import { not, or } from "@ember/object/computed";
 import { inject as service } from "@ember/service";
+import { isEmpty } from "@ember/utils";
 import { ajax } from "discourse/lib/ajax";
 import { popupAjaxError } from "discourse/lib/ajax-error";
-import { not, or } from "@ember/object/computed";
-import { isEmpty } from "@ember/utils";
-import { action } from "@ember/object";
 
 export default Controller.extend({
   topicBulkActions: controller(),
@@ -71,6 +71,7 @@ export default Controller.extend({
         group_name: this.get("model.group_name"),
         target_id: this.get("model.target.id"),
         target_type: this.get("model.targetType"),
+        note: this.get("model.note"),
       },
     })
       .then(() => {
@@ -99,10 +100,6 @@ export default Controller.extend({
         "model.allowedGroups": this.taskActions.allowedGroups,
       });
     }
-
-    if (name) {
-      return this.assign();
-    }
   },
 
   @action

assets/javascripts/discourse-assign/initializers/extend-for-assigns.js

@@ -768,6 +768,8 @@ function initialize(api) {
             post = topic.postStream.posts.find((p) => p.id === data.post_id);
           }
           const target = post || topic;
+
+          target.set("assignment_note", null);
           if (data.assigned_type === "User") {
             target.set(
               "assigned_to_user_id",

assets/javascripts/discourse/services/task-actions.js

@@ -38,6 +38,7 @@ export default Service.extend({
         group_name: target.assigned_to_group?.name,
         target,
         targetType: options.targetType,
+        note: target.assignment_note,
       },
     });
   },
@@ -49,6 +50,7 @@ export default Service.extend({
         username: user.username,
         target_id: target.id,
         target_type: targetType,
+        note: target.assignment_note,
       },
     });
   },

assets/javascripts/discourse/templates/modal/assign-user.hbs

@@ -25,6 +25,8 @@
         </a>
       {{/each}}
     </div>
+    <label>{{i18n "discourse_assign.assign_modal.note_label"}}</label>
+    {{textarea value=model.note}}
   </div>
 {{/d-modal-body}}
 

assets/stylesheets/assigns.scss

@@ -78,6 +78,8 @@
 
 .assign-suggestions {
   margin-top: 15px;
+  margin-bottom: 15px;
+
   img {
     margin-right: 5px;
     cursor: pointer;

config/locales/client.en.yml

@@ -53,6 +53,7 @@ en:
         reassign_title: "Reassign Topic"
         description: "Enter the name of the user you'd like to assign this topic"
         assign: "Assign"
+        note_label: Note
       assign_post_modal:
         title: "Assign Post"
         description: "Enter the name of the user you'd like to assign this post"

db/migrate/20220429110203_add_note_to_assignments.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+class AddNoteToAssignments < ActiveRecord::Migration[6.1]
+  def change
+    add_column :assignments, :note, :string, null: true
+  end
+end

lib/assigner.rb

@@ -180,7 +180,7 @@ class ::Assigner
     topic.posts.where(post_number: 1).first
   end
 
-  def forbidden_reasons(assign_to:, type:)
+  def forbidden_reasons(assign_to:, type:, note:)
     case
     when assign_to.is_a?(User) && !can_assignee_see_target?(assign_to)
       topic.private_message? ? :forbidden_assignee_not_pm_participant : :forbidden_assignee_cant_see_topic
@@ -188,9 +188,9 @@ class ::Assigner
       topic.private_message? ? :forbidden_group_assignee_not_pm_participant : :forbidden_group_assignee_cant_see_topic
     when !can_be_assigned?(assign_to)
       assign_to.is_a?(User) ? :forbidden_assign_to : :forbidden_group_assign_to
-    when topic.assignment&.assigned_to_id == assign_to.id && topic.assignment&.assigned_to_type == type && topic.assignment.active == true
+    when topic_same_assignee_and_note(assign_to, type, note)
       assign_to.is_a?(User) ? :already_assigned : :group_already_assigned
-    when @target.is_a?(Topic) && Assignment.where(topic_id: topic.id, target_type: "Post", active: true).any? { |assignment| assignment.assigned_to_id == assign_to.id && assignment.assigned_to_type == type }
+    when post_same_assignee_and_note(assign_to, type, note)
       assign_to.is_a?(User) ? :already_assigned : :group_already_assigned
     when Assignment.where(topic: topic).count >= ASSIGNMENTS_PER_TOPIC_LIMIT
       :too_many_assigns_for_topic
@@ -199,10 +199,10 @@ class ::Assigner
     end
   end
 
-  def assign(assign_to, silent: false)
+  def assign(assign_to, note: nil, silent: false)
     type = assign_to.is_a?(User) ? "User" : "Group"
 
-    forbidden_reason = forbidden_reasons(assign_to: assign_to, type: type)
+    forbidden_reason = forbidden_reasons(assign_to: assign_to, type: type, note: note)
     return { success: false, reason: forbidden_reason } if forbidden_reason
 
     action_code = {}
@@ -211,7 +211,7 @@ class ::Assigner
 
     @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)
+    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)
 
     first_post.publish_change_to_clients!(:revised, reload_topic: true)
 
@@ -413,4 +413,20 @@ class ::Assigner
     return "unassigned#{suffix}" if assignment.assigned_to_user?
     return "unassigned_group#{suffix}" if assignment.assigned_to_group?
   end
+
+  def topic_same_assignee_and_note(assign_to, type, note)
+    topic.assignment&.assigned_to_id == assign_to.id &&
+      topic.assignment&.assigned_to_type == type &&
+      topic.assignment.active == true &&
+      topic.assignment&.note == note
+  end
+
+  def post_same_assignee_and_note(assign_to, type, note)
+    @target.is_a?(Topic) &&
+      Assignment.where(topic_id: topic.id, target_type: "Post", active: true).any? do |assignment|
+        assignment.assigned_to_id == assign_to.id &&
+          assignment.assigned_to_type == type &&
+          assignment&.note == note
+      end
+  end
 end

plugin.rb

@@ -484,6 +484,14 @@ after_initialize do
     (SiteSetting.assigns_public || scope.can_assign?) && object.topic.indirectly_assigned_to.present?
   end
 
+  add_to_serializer(:topic_view, :assignment_note, false) do
+    object.topic.assignment.note
+  end
+
+  add_to_serializer(:topic_view, :include_assignment_note?, false) do
+    (SiteSetting.assigns_public || scope.can_assign?) && object.topic.assignment.present?
+  end
+
   # SuggestedTopic serializer
   add_to_serializer(:suggested_topic, :assigned_to_user, false) do
     DiscourseAssign::Helpers.build_assigned_to_user(object.assigned_to, object)
@@ -631,6 +639,14 @@ after_initialize do
     (SiteSetting.assigns_public || scope.can_assign?) && object.assignment&.assigned_to&.is_a?(Group) && object.assignment.active
   end
 
+  add_to_serializer(:post, :assignment_note, false) do
+    object.assignment.note
+  end
+
+  add_to_serializer(:post, :include_assignment_note?, false) do
+    (SiteSetting.assigns_public || scope.can_assign?) && object.assignment.present?
+  end
+
   # CurrentUser serializer
   add_to_serializer(:current_user, :can_assign) do
     object.can_assign?

spec/lib/assigner_spec.rb

@@ -43,6 +43,12 @@ RSpec.describe Assigner do
         .to eq(TopicUser.notification_levels[:tracking])
     end
 
+    it "can assign with note" do
+      assigner.assign(moderator, note: "tomtom best mom")
+
+      expect(topic.assignment.note).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)
@@ -139,86 +145,107 @@ RSpec.describe Assigner do
       assigner.assign(assignee).fetch(:success)
     end
 
-    it "doesn't assign if the user has too many assigned topics" do
-      SiteSetting.max_assigned_topics = 1
-      another_post = Fabricate.build(:post)
-      assigner.assign(moderator)
-
-      second_assign = described_class.new(another_post.topic, moderator_2).assign(moderator)
-
-      expect(second_assign[:success]).to eq(false)
-      expect(second_assign[:reason]).to eq(:too_many_assigns)
-    end
-
-    it "doesn't enforce the limit when self-assigning" do
-      SiteSetting.max_assigned_topics = 1
-      another_post = Fabricate(:post)
-      assigner.assign(moderator)
-
-      second_assign = described_class.new(another_post.topic, moderator).assign(moderator)
-
-      expect(second_assign[:success]).to eq(true)
-    end
-
-    it "doesn't count self-assigns when enforcing the limit" do
-      SiteSetting.max_assigned_topics = 1
-      another_post = Fabricate(:post)
-
-      first_assign = assigner.assign(moderator)
-
-      # reached limit so stop
-      second_assign = described_class.new(Fabricate(:topic), moderator_2).assign(moderator)
-
-      # self assign has a bypass
-      third_assign = described_class.new(another_post.topic, moderator).assign(moderator)
-
-      expect(first_assign[:success]).to eq(true)
-      expect(second_assign[:success]).to eq(false)
-      expect(third_assign[:success]).to eq(true)
-    end
-
-    it "doesn't count inactive assigns when enforcing the limit" do
-      SiteSetting.max_assigned_topics = 1
-      SiteSetting.unassign_on_close = true
-      another_post = Fabricate(:post)
-
-      first_assign = assigner.assign(moderator)
-      topic.update_status("closed", true, Discourse.system_user)
-
-      second_assign = described_class.new(another_post.topic, moderator_2).assign(moderator)
-
-      expect(first_assign[:success]).to eq(true)
-      expect(second_assign[:success]).to eq(true)
-    end
-
     fab!(:admin) { Fabricate(:admin) }
 
-    it 'fails to assign when the assigned user cannot view the pm' do
+    context "forbidden reasons" do
-      assign = described_class.new(pm, admin).assign(moderator)
+      it "doesn't assign if the user has too many assigned topics" do
+        SiteSetting.max_assigned_topics = 1
+        another_post = Fabricate.build(:post)
+        assigner.assign(moderator)
 
-      expect(assign[:success]).to eq(false)
+        second_assign = described_class.new(another_post.topic, moderator_2).assign(moderator)
-      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
+        expect(second_assign[:success]).to eq(false)
-      assign = described_class.new(pm, admin).assign(moderator.groups.first)
+        expect(second_assign[:reason]).to eq(:too_many_assigns)
+      end
 
-      expect(assign[:success]).to eq(false)
+      it "doesn't enforce the limit when self-assigning" do
-      expect(assign[:reason]).to eq(:forbidden_group_assignee_not_pm_participant)
+        SiteSetting.max_assigned_topics = 1
-    end
+        another_post = Fabricate(:post)
+        assigner.assign(moderator)
 
-    it 'fails to assign when the assigned user cannot view the topic' do
+        second_assign = described_class.new(another_post.topic, moderator).assign(moderator)
-      assign = described_class.new(secure_topic, admin).assign(moderator)
 
-      expect(assign[:success]).to eq(false)
+        expect(second_assign[:success]).to eq(true)
-      expect(assign[:reason]).to eq(:forbidden_assignee_cant_see_topic)
+      end
-    end
 
-    it 'fails to assign when the not all group members can view the topic' do
+      it "doesn't count self-assigns when enforcing the limit" do
-      assign = described_class.new(secure_topic, admin).assign(moderator.groups.first)
+        SiteSetting.max_assigned_topics = 1
+        another_post = Fabricate(:post)
 
-      expect(assign[:success]).to eq(false)
+        first_assign = assigner.assign(moderator)
-      expect(assign[:reason]).to eq(:forbidden_group_assignee_cant_see_topic)
+
+        # reached limit so stop
+        second_assign = described_class.new(Fabricate(:topic), moderator_2).assign(moderator)
+
+        # self assign has a bypass
+        third_assign = described_class.new(another_post.topic, moderator).assign(moderator)
+
+        expect(first_assign[:success]).to eq(true)
+        expect(second_assign[:success]).to eq(false)
+        expect(third_assign[:success]).to eq(true)
+      end
+
+      it "doesn't count inactive assigns when enforcing the limit" do
+        SiteSetting.max_assigned_topics = 1
+        SiteSetting.unassign_on_close = true
+        another_post = Fabricate(:post)
+
+        first_assign = assigner.assign(moderator)
+        topic.update_status("closed", true, Discourse.system_user)
+
+        second_assign = described_class.new(another_post.topic, moderator_2).assign(moderator)
+
+        expect(first_assign[:success]).to eq(true)
+        expect(second_assign[:success]).to eq(true)
+      end
+
+      it 'fails to assign when the assigned user and note is the same' do
+        assigner = described_class.new(topic, admin)
+        assigner.assign(moderator, note: "note me down")
+
+        assign = assigner.assign(moderator, note: "note me down")
+
+        expect(assign[:success]).to eq(false)
+        expect(assign[:reason]).to eq(:already_assigned)
+      end
+
+      it 'allows assign when the assigned user is same but note is different' do
+        assigner = described_class.new(topic, admin)
+        assigner.assign(moderator, note: "note me down")
+
+        assign = assigner.assign(moderator, note: "note me down again")
+
+        expect(assign[:success]).to eq(true)
+      end
+
+      it 'fails to assign when the assigned user cannot view the pm' do
+        assign = described_class.new(pm, admin).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)
+
+        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)
+
+        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)
+
+        expect(assign[:success]).to eq(false)
+        expect(assign[:reason]).to eq(:forbidden_group_assignee_cant_see_topic)
+      end
     end
 
     it "assigns the PM to the moderator when it's included in the list of allowed users" do

spec/requests/assign_controller_spec.rb

@@ -116,6 +116,14 @@ RSpec.describe DiscourseAssign::AssignController do
       expect(post.topic.reload.assignment.assigned_to_id).to eq(user2.id)
     end
 
+    it 'assigns topic with note to a user' do
+      put '/assign/assign.json', params: {
+        target_id: post.topic_id, target_type: 'Topic', username: user2.username, note: "do dis pls"
+      }
+
+      expect(post.topic.reload.assignment.note).to eq("do dis pls")
+    end
+
     it 'assigns topic to a group' do
       put '/assign/assign.json', params: {
         target_id: post.topic_id, target_type: 'Topic', group_name: assign_allowed_group.name

spec/serializers/post_serializer_spec.rb

@@ -33,4 +33,10 @@ describe PostSerializer do
     expect(post[:assigned_to_group][:assign_icon]).to eq("group-plus")
     expect(post[:assigned_to_user]).to be(nil)
   end
+
+  it "includes note in serializer" do
+    Assigner.new(post, user).assign(user, note: "tomtom best")
+    serializer = PostSerializer.new(post, scope: guardian)
+    expect(serializer.as_json[:post][:assignment_note]).to eq("tomtom best")
+  end
 end

spec/serializers/topic_view_serializer_spec.rb

@@ -29,4 +29,10 @@ RSpec.describe TopicViewSerializer do
     expect(serializer.as_json[:topic_view][:assigned_to_group][:name]).to eq(assign_allowed_group.name)
     expect(serializer.as_json[:topic_view][:assigned_to_user]).to be nil
   end
+
+  it "includes note in serializer" do
+    Assigner.new(topic, user).assign(user, note: "note me down")
+    serializer = TopicViewSerializer.new(TopicView.new(topic), scope: guardian)
+    expect(serializer.as_json[:topic_view][:assignment_note]).to eq("note me down")
+  end
 end