FEATURE: Limit the amount of assigned topics an user can have. (#29)

* FEATURE: Limit the amount of assigned topics an user can have.

* Do not enfore the limit when self-assigning

* Avoid computing the query when self-assigning. Do not count self-assigns when enforcing the limit

app/controllers/discourse_assign/assign_controller.rb

@@ -7,13 +7,16 @@ module DiscourseAssign
       users += User
         .where('admin OR moderator')
         .where('users.id <> ?', current_user.id)
-        .joins("join (
-                       SELECT value::integer user_id, MAX(created_at) last_assigned
-                       FROM topic_custom_fields
-                       WHERE name = 'assigned_to_id'
-                       GROUP BY value::integer
-                      ) as X ON X.user_id = users.id")
-        .order('X.last_assigned DESC')
+        .joins(<<~SQL
+          JOIN(
+                SELECT value::integer user_id, MAX(created_at) last_assigned
+                FROM topic_custom_fields
+                WHERE name = 'assigned_to_id'
+                GROUP BY value::integer
+                HAVING COUNT(*) < #{SiteSetting.max_assigned_topics}
+              ) as X ON X.user_id = users.id
+        SQL
+        ).order('X.last_assigned DESC')
         .limit(6)
 
       render json: ActiveModel::ArraySerializer.new(users,
@@ -62,17 +65,15 @@ module DiscourseAssign
 
       raise Discourse::NotFound unless assign_to
 
-      if topic.custom_fields && topic.custom_fields['assigned_to_id'] == assign_to.id.to_s
-        return render json: { failed: I18n.t('discourse_assign.already_assigned', username: username) }, status: 400
-      end
-
-      assigner = TopicAssigner.new(topic, current_user)
-
       # perhaps?
       #Scheduler::Defer.later "assign topic" do
-      assigner.assign(assign_to)
+      assign = TopicAssigner.new(topic, current_user).assign(assign_to)
 
-      render json: success_json
+      if assign[:success]
+        render json: success_json
+      else
+        render json: translate_failure(assign[:reason], assign_to), status: 400
+      end
     end
 
     def unassign_all
@@ -80,5 +81,16 @@ module DiscourseAssign
       TopicAssigner.unassign_all(user, current_user)
       render json: success_json
     end
+
+    private
+
+    def translate_failure(reason, user)
+      if reason == :already_assigned
+        { error: I18n.t('discourse_assign.already_assigned', username: user.username) }
+      else
+        max = SiteSetting.max_assigned_topics
+        { error: I18n.t('discourse_assign.too_many_assigns', username: user.username, max: max) }
+      end
+    end
   end
 end

config/locales/server.en.yml

@@ -14,11 +14,13 @@ en:
     flags_require_assign: "When enabled, flags cannot be handled unless assigned to a user."
     remind_assigns: "Remind users about pending assigns."
     remind_assigns_frequency: "Frequency for reminding users about assigned topics."
+    max_assigns_per_user: "Maximum number of topics that can be assigned to a user."
   discourse_assign:
     assigned_to: "Topic assigned to @%{username}"
     unassigned: "Topic was unassigned"
     already_claimed: "That topic has already been claimed."
     already_assigned: 'Topic is already assigned to @%{username}'
+    too_many_assigns: "@%{username} has already reached the maximum number of assigned topics (%{max})."
     flag_assigned: "Sorry, that flag's topic is assigned to another user"
     flag_unclaimed: "You must claim that topic before acting on the flag"
     topic_assigned_excerpt: "assigned you the topic '%{title}'"

config/settings.yml

@@ -23,3 +23,7 @@ plugins:
     client: true
     enum: "RemindAssignsFrequencySiteSettings"
     default: 0
+  max_assigned_topics:
+    client: true
+    default: 10
+    min: 1

lib/topic_assigner.rb

@@ -130,8 +130,22 @@ SQL
     User.real.staff.pluck(:id)
   end
 
+  def can_assign_to?(user)
+    return true if @assigned_by.id == user.id
+
+    assigned_total = TopicCustomField
+      .where('name = ? OR name = ?', ASSIGNED_TO_ID, ASSIGNED_BY_ID)
+      .where(value: user.id)
+      .group(:topic_id)
+      .having('COUNT(*) = 1')
+      .count.length
+
+    assigned_total < SiteSetting.max_assigned_topics
+  end
+
   def assign(assign_to, silent: false)
-    return false if @topic.custom_fields[ASSIGNED_TO_ID] == assign_to.id.to_s
+    return { success: false, reason: :already_assigned } if @topic.custom_fields && @topic.custom_fields[ASSIGNED_TO_ID] == assign_to.id.to_s
+    return { success: false, reason: :too_many_assigns } unless can_assign_to?(assign_to)
 
     @topic.custom_fields[ASSIGNED_TO_ID] = assign_to.id
     @topic.custom_fields[ASSIGNED_BY_ID] = @assigned_by.id
@@ -223,7 +237,7 @@ SQL
       )
     end
 
-    true
+    { success: true }
   end
 
   def unassign(silent: false)

spec/lib/topic_assigner_spec.rb

@@ -119,13 +119,48 @@ RSpec.describe TopicAssigner do
       SiteSetting.assign_mailer_enabled = true
 
       Email::Sender.any_instance.expects(:send).once
-      expect(assigner.assign(moderator)).to eq(true)
+      expect(assigned_to?(moderator)).to eq(true)
 
       Email::Sender.any_instance.expects(:send).never
-      expect(assigner.assign(moderator)).to eq(false)
+      expect(assigned_to?(moderator)).to eq(false)
 
       Email::Sender.any_instance.expects(:send).once
-      expect(assigner.assign(Fabricate(:moderator))).to eq(true)
+      expect(assigned_to?(Fabricate(:moderator))).to eq(true)
+    end
+
+    def assigned_to?(moderator)
+      assigner.assign(moderator).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 = TopicAssigner.new(another_post.topic, moderator2).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 = TopicAssigner.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)
+      TopicAssigner.new(another_post.topic, moderator).assign(moderator)
+
+      second_assign = assigner.assign(moderator)
+
+      expect(second_assign[:success]).to eq(true)
     end
   end
 

spec/requests/assign_controller_spec.rb

@@ -6,11 +6,24 @@ RSpec.describe DiscourseAssign::AssignController do
   let(:post) { Fabricate(:post) }
   let(:user2) { Fabricate(:active_user) }
 
+  before { sign_in(user) }
+
+  context "#suggestions" do
+    before { SiteSetting.max_assigned_topics = 1 }
+
+    it 'excludes other users from the suggestions when they already reached the max assigns limit' do
+      another_admin = Fabricate(:admin)
+      TopicAssigner.new(post.topic, user).assign(another_admin)
+
+      get '/assign/suggestions.json'
+      suggestions = JSON.parse(response.body).map { |u| u['username'] }
+
+      expect(suggestions).to contain_exactly(user.username)
+    end
+  end
+
   context '#assign' do
-
     it 'assigns topic to a user' do
-      sign_in(user)
-
       put '/assign/assign.json', params: {
         topic_id: post.topic_id, username: user2.username
       }
@@ -20,8 +33,6 @@ RSpec.describe DiscourseAssign::AssignController do
     end
 
     it 'fails to assign topic to the user if its already assigned to the same user' do
-      sign_in(user)
-
       put '/assign/assign.json', params: {
         topic_id: post.topic_id, username: user2.username
       }
@@ -34,9 +45,25 @@ RSpec.describe DiscourseAssign::AssignController do
       }
 
       expect(response.status).to eq(400)
-      expect(JSON.parse(response.body)['failed']).to eq(I18n.t('discourse_assign.already_assigned', username: user2.username))
+      expect(JSON.parse(response.body)['error']).to eq(I18n.t('discourse_assign.already_assigned', username: user2.username))
     end
 
+    it 'fails to assign topic to the user if they already reached the max assigns limit' do
+      another_admin = Fabricate(:admin)
+      another_post = Fabricate(:post)
+      max_assigns = 1
+      SiteSetting.max_assigned_topics = max_assigns
+      TopicAssigner.new(post.topic, user).assign(another_admin)
+
+      put '/assign/assign.json', params: {
+        topic_id: another_post.topic_id, username: another_admin.username
+      }
+
+      expect(response.status).to eq(400)
+      expect(JSON.parse(response.body)['error']).to eq(
+        I18n.t('discourse_assign.too_many_assigns', username: another_admin.username, max: max_assigns)
+      )
+    end
   end
 
 end