From 5779385d66f118c728d00a4b7d961727c1a2b035 Mon Sep 17 00:00:00 2001
From: Krzysztof Kotlarek <kotlarek.krzysztof@gmail.com>
Date: Thu, 20 Jan 2022 08:20:21 +1100
Subject: [PATCH] FIX: don't display inactive assignments (#283)

There is an active flag for assignments. It is used to bring assignments back when topic is reopened.

However, when assignment is inactive, it should not be displayed on assigned list or search.
---
 .../discourse_assign/assign_controller.rb            |  2 +-
 plugin.rb                                            | 12 ++++++------
 spec/components/search_spec.rb                       |  4 +++-
 spec/components/topic_query_spec.rb                  |  8 ++++++++
 4 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/app/controllers/discourse_assign/assign_controller.rb b/app/controllers/discourse_assign/assign_controller.rb
index 5b2dfec..fa44a0d 100644
--- a/app/controllers/discourse_assign/assign_controller.rb
+++ b/app/controllers/discourse_assign/assign_controller.rb
@@ -81,7 +81,7 @@ module DiscourseAssign
 
       Topic.preload_custom_fields(topics, TopicList.preloaded_custom_fields)
 
-      topic_assignments = Assignment.where(target_id: topics.map(&:id), target_type: 'Topic').pluck(:target_id, :assigned_to_id).to_h
+      topic_assignments = Assignment.where(target_id: topics.map(&:id), target_type: 'Topic', active: true).pluck(:target_id, :assigned_to_id).to_h
 
       users = User
         .where("users.id IN (?)", topic_assignments.values.uniq)
diff --git a/plugin.rb b/plugin.rb
index 17f8ab7..0ecd113 100644
--- a/plugin.rb
+++ b/plugin.rb
@@ -323,12 +323,12 @@ after_initialize do
       SELECT topic_id FROM assignments
       LEFT JOIN group_users ON group_users.user_id = :user_id
       WHERE
-        assigned_to_id = :user_id AND assigned_to_type = 'User' AND active
+        (assigned_to_id = :user_id AND assigned_to_type = 'User' AND active)
     SQL
 
     if @options[:filter] != :direct
       topic_ids_sql << <<~SQL
-        OR assigned_to_id IN (group_users.group_id) AND assigned_to_type = 'Group'
+        OR (assigned_to_id IN (group_users.group_id) AND assigned_to_type = 'Group' AND active)
       SQL
     end
 
@@ -822,7 +822,7 @@ after_initialize do
     if @guardian.can_assign?
       posts.where(<<~SQL)
         topics.id IN (
-          SELECT a.topic_id FROM assignments a
+          SELECT a.topic_id FROM assignments a WHERE a.active
         )
       SQL
     end
@@ -832,7 +832,7 @@ after_initialize do
     if @guardian.can_assign?
       posts.where(<<~SQL)
         topics.id NOT IN (
-          SELECT a.topic_id FROM assignments a
+          SELECT a.topic_id FROM assignments a WHERE a.active
         )
       SQL
     end
@@ -843,13 +843,13 @@ after_initialize do
       if user_id = User.find_by_username(match)&.id
         posts.where(<<~SQL, user_id)
           topics.id IN (
-            SELECT a.topic_id FROM assignments a WHERE a.assigned_to_id = ? AND a.assigned_to_type = 'User'
+            SELECT a.topic_id FROM assignments a WHERE a.assigned_to_id = ? AND a.assigned_to_type = 'User' AND a.active
           )
         SQL
       elsif group_id = Group.find_by_name(match)&.id
         posts.where(<<~SQL, group_id)
           topics.id IN (
-            SELECT a.topic_id FROM assignments a WHERE a.assigned_to_id = ? AND a.assigned_to_type = 'Group'
+            SELECT a.topic_id FROM assignments a WHERE a.assigned_to_id = ? AND a.assigned_to_type = 'Group' AND a.active
           )
         SQL
       end
diff --git a/spec/components/search_spec.rb b/spec/components/search_spec.rb
index 98be7fc..4fa7c12 100644
--- a/spec/components/search_spec.rb
+++ b/spec/components/search_spec.rb
@@ -20,6 +20,7 @@ describe Search do
     let(:post3) { Fabricate(:post) }
     let(:post4) { Fabricate(:post) }
     let(:post5) { Fabricate(:post, topic: post4.topic) }
+    let(:post6) { Fabricate(:post) }
 
     before do
       add_to_assign_allowed_group(user)
@@ -29,6 +30,7 @@ describe Search do
       Assigner.new(post2.topic, user).assign(user2)
       Assigner.new(post3.topic, user).assign(user)
       Assigner.new(post5, user).assign(user)
+      Assignment.create!(assigned_to: user, assigned_by_user: user, target: post6, topic_id: post6.topic.id, active: false)
     end
 
     it 'can find by status' do
@@ -36,7 +38,7 @@ describe Search do
 
       Assigner.new(post3.topic, user).unassign
 
-      expect(Search.execute('in:unassigned', guardian: Guardian.new(user)).posts.length).to eq(1)
+      expect(Search.execute('in:unassigned', guardian: Guardian.new(user)).posts.length).to eq(2)
       expect(Search.execute("assigned:#{user.username}", guardian: Guardian.new(user)).posts.length).to eq(2)
     end
 
diff --git a/spec/components/topic_query_spec.rb b/spec/components/topic_query_spec.rb
index 2b00399..dfeaee7 100644
--- a/spec/components/topic_query_spec.rb
+++ b/spec/components/topic_query_spec.rb
@@ -38,6 +38,14 @@ describe TopicQuery do
       expect(assigned_messages).to contain_exactly(private_message, topic, group_topic)
     end
 
+    it 'Excludes inactive assignments' do
+      Assignment.update_all(active: false)
+
+      assigned_messages = TopicQuery.new(user, { page: 0 }).list_messages_assigned(user).topics
+
+      expect(assigned_messages).to eq([])
+    end
+
     it 'Excludes topics and PMs not assigned to user' do
       assigned_messages = TopicQuery.new(user2, { page: 0 }).list_messages_assigned(user2).topics