FIX: Exclude deleted topics from assignment counts (#84)

app/controllers/discourse_assign/assign_controller.rb

@@ -137,8 +137,10 @@ module DiscourseAssign
       guardian.ensure_can_see_group_members!(group)
 
       members = User
-        .joins("LEFT OUTER JOIN group_users g on users.id=g.user_id LEFT OUTER JOIN user_options uo on uo.user_id=users.id LEFT OUTER JOIN topic_custom_fields tcf ON tcf.value::int = users.id")
-        .where("tcf.name = 'assigned_to_id' AND g.group_id=? AND (users.id > 0)", group.id)
+        .joins("LEFT OUTER JOIN group_users g on users.id=g.user_id")
+        .joins("LEFT OUTER JOIN topic_custom_fields tcf ON tcf.value::int = users.id")
+        .joins("LEFT OUTER JOIN topics t ON t.id = tcf.topic_id")
+        .where("tcf.name = 'assigned_to_id' AND g.group_id=? AND (users.id > 0) AND t.deleted_at IS NULL", group.id)
         .order('COUNT(users.id) DESC')
         .group('users.id')
         .select('users.*, COUNT(users.id) as "assignments_count"')

plugin.rb

@@ -42,7 +42,9 @@ after_initialize do
 
   add_to_serializer(:group_show, :assignment_count) do
     Topic.joins("JOIN topic_custom_fields tcf ON topics.id = tcf.topic_id AND tcf.name = 'assigned_to_id' AND tcf.value IS NOT NULL")
-      .where("tcf.value IN (SELECT group_users.user_id::varchar(255) FROM group_users WHERE (group_id IN (SELECT id FROM groups WHERE name = ?)))", object.name).count
+      .where("tcf.value IN (SELECT group_users.user_id::varchar(255) FROM group_users WHERE (group_id IN (SELECT id FROM groups WHERE name = ?)))", object.name)
+      .where("topics.deleted_at IS NULL")
+      .count
   end
 
   add_to_serializer(:group_show, 'include_assignment_count?') do

spec/requests/assign_controller_spec.rb

@@ -209,13 +209,8 @@ RSpec.describe DiscourseAssign::AssignController do
       add_to_assign_allowed_group(user2)
       add_to_assign_allowed_group(user)
 
-      freeze_time 1.hour.from_now
       TopicAssigner.new(post1.topic, user).assign(user)
-
-      freeze_time 1.hour.from_now
       TopicAssigner.new(post2.topic, user).assign(user2)
-
-      freeze_time 1.hour.from_now
       TopicAssigner.new(post3.topic, user).assign(user)
     end
 

spec/requests/list_controller_spec.rb

@@ -50,17 +50,18 @@ describe ListController do
     fab!(:post1) { Fabricate(:post) }
     fab!(:post2) { Fabricate(:post) }
     fab!(:post3) { Fabricate(:post) }
+    fab!(:topic) { post3.topic }
 
     before do
       add_to_assign_allowed_group(user)
 
       TopicAssigner.new(post1.topic, user).assign(user)
-      TopicAssigner.new(post1.topic, user).assign(user2)
+      TopicAssigner.new(post2.topic, user).assign(user2)
 
       sign_in(user)
     end
 
-    it 'returns user-assigned-topics-list of users in the assigned_allowed_group' do
+    it 'returns user-assigned-topics-list of users in the assigned_allowed_group and doesnt include deleted topic' do
       get "/topics/group-topics-assigned/#{get_assigned_allowed_group_name}.json"
       expect(JSON.parse(response.body)['topic_list']['topics'].map { |t| t['assigned_to_user']['id'] }).to match_array([user.id])
     end
@@ -75,6 +76,27 @@ describe ListController do
       end
       expect(ids).to be_empty
     end
+
+    it 'doesnt returns deleted topics' do
+      sign_in(admin)
+
+      TopicAssigner.new(topic, user).assign(user)
+
+      delete "/t/#{topic.id}.json"
+
+      topic.reload
+
+      id = 0
+      get "/topics/group-topics-assigned/#{get_assigned_allowed_group_name}.json"
+
+      JSON.parse(response.body)['topic_list']['topics'].each do |t|
+        if t['id'] == topic.id
+          id = t.id
+        end
+      end
+
+      expect(id).to eq(0)
+    end
   end
 
   context '#messages_assigned' do
@@ -82,13 +104,12 @@ describe ListController do
 
     fab!(:post1) { Fabricate(:post) }
     fab!(:post2) { Fabricate(:post) }
-    fab!(:post3) { Fabricate(:post) }
 
     before do
       add_to_assign_allowed_group(user)
 
       TopicAssigner.new(post1.topic, user).assign(user)
-      TopicAssigner.new(post1.topic, user).assign(user2)
+      TopicAssigner.new(post2.topic, user).assign(user2)
 
       sign_in(user)
     end

test/javascripts/acceptance/group-assignments-test.js.es6

@@ -1,26 +1,31 @@
 import { acceptance } from "helpers/qunit-helpers";
 import { default as AssignedTopics } from "../fixtures/assigned-group-assignments-fixtures";
+import { default as GroupMembers } from "../fixtures/group-members-fixtures";
 
 acceptance("GroupAssignments", {
   loggedIn: true,
   settings: { assign_enabled: true, assigns_user_url_path: "/" },
   pretend(server, helper) {
     const groupPath = "/topics/group-topics-assigned/discourse.json";
-    const memberPath = "/topics/messages-assigned/awesomerobot.json";
+    const memberPath = "/topics/messages-assigned/ahmedgagan6.json";
+    const getMembersPath = "/assign/members/discourse";
     const groupAssigns = AssignedTopics[groupPath];
     const memberAssigns = AssignedTopics[memberPath];
+    const getMembers = GroupMembers[getMembersPath];
     server.get(groupPath, () => helper.response(groupAssigns));
     server.get(memberPath, () => helper.response(memberAssigns));
+    server.get(getMembersPath, () => helper.response(getMembers));
   }
 });
-QUnit.skip("Group Assignments Everyone", async assert => {
+
+QUnit.test("Group Assignments Everyone", async assert => {
   await visit("/g/discourse/assignments");
   assert.equal(currentPath(), "group.assignments.show");
   assert.ok(find(".topic-list-item").length === 1);
 });
 
-QUnit.skip("Group Assignments Awesomerobot", async assert => {
-  await visit("/g/discourse/assignments/awesomerobot");
+QUnit.test("Group Assignments Ahmedgagan", async assert => {
+  await visit("/g/discourse/assignments/ahmedgagan6");
   assert.equal(currentPath(), "group.assignments.show");
   assert.ok(find(".topic-list-item").length === 1);
 });

test/javascripts/fixtures/assigned-group-assignments-fixtures.js.es6

@@ -83,7 +83,7 @@ export default {
       ]
     }
   },
-  "/topics/messages-assigned/awesomerobot.json": {
+  "/topics/messages-assigned/ahmedgagan6.json": {
     users: [
       {
         id: -2,

test/javascripts/fixtures/group-members-fixtures.js.es6

@@ -0,0 +1,38 @@
+export default {
+  "/assign/members/discourse": {
+    members: [
+      {
+        id: 1,
+        username: "ahmedgagan6",
+        name: null,
+        avatar_template: "/letter_avatar_proxy/v4/letter/a/838e76/{size}.png",
+        assignments_count: 40,
+        username_lower: "ahmedgagan6"
+      },
+      {
+        id: 4,
+        username: "gaganAhmed",
+        name: "Gagan ",
+        avatar_template: "/letter_avatar_proxy/v4/letter/g/8edcca/{size}.png",
+        assignments_count: 17,
+        username_lower: "gaganahmed"
+      },
+      {
+        id: 3,
+        username: "ahmedgagan",
+        name: "",
+        avatar_template: "/user_avatar/localhost/ahmedgagan/{size}/2_2.png",
+        assignments_count: 16,
+        username_lower: "ahmedgagan"
+      },
+      {
+        id: 2,
+        username: "Ahmed",
+        name: "Ahmed Gagan",
+        avatar_template: "/letter_avatar_proxy/v4/letter/a/8c91f0/{size}.png",
+        assignments_count: 15,
+        username_lower: "ahmed"
+      }
+    ]
+  }
+};