FIX: Check if assignment has same user and details (#368)

The check existed, but its implementation was incorrect and it did not
work when the target was a post.

lib/assigner.rb

@@ -211,9 +211,7 @@ class ::Assigner
       end
     when !can_be_assigned?(assign_to)
       assign_to.is_a?(User) ? :forbidden_assign_to : :forbidden_group_assign_to
-    when topic_same_assignee_and_details(assign_to, type, note, status)
+    when already_assigned?(assign_to, type, note, status)
-      assign_to.is_a?(User) ? :already_assigned : :group_already_assigned
-    when post_same_assignee_and_details(assign_to, type, note, status)
       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
@@ -512,31 +510,28 @@ class ::Assigner
     return "unassigned_group#{suffix}" if assignment.assigned_to_group?
   end
 
-  def topic_same_assignee_and_details(assign_to, type, note, status)
+  def already_assigned?(assign_to, type, note, status)
-    topic.assignment&.assigned_to_id == assign_to.id &&
+    return true if assignment_eq?(@target.assignment, assign_to, type, note, status)
-      topic.assignment&.assigned_to_type == type && topic.assignment.active == true &&
-      topic.assignment&.note == note &&
-      (
-        topic.assignment&.status == status ||
-          topic.assignment&.status == Assignment.default_status && status.nil?
-      )
-  end
 
-  def post_same_assignee_and_details(assign_to, type, note, status)
+    # Check if the user is not assigned to any of the posts from the topic
-    @target.is_a?(Topic) &&
+    # they will be assigned to.
-      Assignment
+    if @target.is_a?(Topic)
-        .where(topic_id: topic.id, target_type: "Post", active: true)
+      assignments = Assignment.where(topic_id: topic.id, target_type: "Post", active: true)
-        .any? do |assignment|
+      return true if assignments.any? { |a| assignment_eq?(a, assign_to, type, note, status) }
-          assignment.assigned_to_id == assign_to.id && assignment.assigned_to_type == type &&
+    end
-            assignment&.note == note &&
+
-            (
+    false
-              topic.assignment&.status == status ||
-                topic.assignment&.status == Assignment.default_status && status.nil?
-            )
-        end
   end
 
   def no_assignee_change?(assignee)
     @target.assignment&.assigned_to_id == assignee.id
   end
+
+  def assignment_eq?(assignment, assign_to, type, note, status)
+    return false if !assignment&.active
+    return false if assignment.assigned_to_id != assign_to.id
+    return false if assignment.assigned_to_type != type
+    return false if assignment.note != note
+    assignment.status == status || !status && assignment.status == Assignment.default_status
+  end
 end

spec/lib/assigner_spec.rb

@@ -236,6 +236,16 @@ RSpec.describe Assigner do
         expect(assign[:reason]).to eq(:already_assigned)
       end
 
+      it "fails to assign when the assigned user and note is the same" do
+        assigner = described_class.new(post, moderator_2)
+        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, moderator_2)
         assigner.assign(moderator, note: "note me down")