FIX: Do not assign users who have been assigned in last 2 weeks (#2)

FIX: Do not assign users who have been assigned in last 2 weeks (#2)

As discussed here https://dev.discourse.org/t/bug-crushing-game-2019-edition/17547/197 if there aren’t many people around bug crushing game gets a little aggressive!

If there is a post custom field assigning a user for the logs game topic created in the last 2 weeks we do not allow that user to be assigned again for around. This is to avoid assigning people frequently in e.g. holiday periods where less users are around to select from the pool.

diff --git a/jobs/scheduled/check_logs_game.rb b/jobs/scheduled/check_logs_game.rb
index 60e8aab..2add1e8 100644
--- a/jobs/scheduled/check_logs_game.rb
+++ b/jobs/scheduled/check_logs_game.rb
@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
 module Jobs
-
   class ::DevAdditions::CheckPolicy < ::Jobs::Scheduled
     every 30.minutes
 
@@ -21,6 +20,7 @@ module Jobs
         @topic = Topic.find(SiteSetting.dev_additions_logs_topic_id)
         @group = Group.find_by(name: SiteSetting.dev_additions_logs_topic_group)
         @random = !!random
+        @recently_assigned_start_time = 2.weeks.ago
       end
 
       def nominee
@@ -43,6 +43,10 @@ module Jobs
             on_holiday?(user_id)
           end
 
+          users_left = users_left.reject do |user_id|
+            recently_assigned_user_ids.include?(user_id)
+          end
+
           if users_left.length > 0
             if @random
               users_left.shuffle!
@@ -84,7 +88,7 @@ module Jobs
       end
 
       def on_holiday?(user_id)
-        UserCustomField.where(name: 'on_holiday', value: 't', user_id: user_id).exists?
+        users_on_holiday.include?(user_id)
       end
 
       def requires_assign?
@@ -94,6 +98,24 @@ module Jobs
           .exists?
       end
 
+      def users_on_holiday
+        @users_on_holiday ||= UserCustomField.where(name: 'on_holiday', value: 't').pluck(:user_id)
+      end
+
+      def recently_assigned_user_ids
+        @recently_assigned_user_ids ||= begin
+          DB.query(<<-SQL, topic_id: @topic.id, recently_assigned_start_time: @recently_assigned_start_time).map(&:id)
+            SELECT users.id
+            FROM posts
+            INNER JOIN post_custom_fields ON post_custom_fields.post_id = posts.id
+            INNER JOIN users ON users.username = post_custom_fields.value
+            WHERE posts.topic_id = :topic_id AND post_custom_fields.name = 'action_code_who'
+            AND posts.action_code = 'assigned' AND post_custom_fields.created_at > :recently_assigned_start_time
+            ORDER BY posts.created_at ASC
+          SQL
+        end
+      end
+
       def user_tzinfo(user_id)
         timezone = UserOption.where(user_id: user_id).pluck(:timezone).first || "UTC"
 
diff --git a/spec/check_logs_game_spec.rb b/spec/check_logs_game_spec.rb
index f00c4ea..c64a271 100644
--- a/spec/check_logs_game_spec.rb
+++ b/spec/check_logs_game_spec.rb
@@ -3,26 +3,28 @@
 require 'rails_helper'
 
 describe DevAdditions::CheckPolicy do
-  it "should automatically assign topic correctly" do
-
-    # clearly require discourse assign to work
-
-    topic = create_post.topic
-
-    user1 = Fabricate(:admin, username: 'aaa')
-    user2 = Fabricate(:admin, username: 'bbb')
-    user3 = Fabricate(:admin, username: 'ccc')
-
+  let!(:topic) { create_post.topic }
+  let!(:user1) { Fabricate(:admin, username: 'aaa') }
+  let!(:user2) { Fabricate(:admin, username: 'bbb') }
+  let!(:user3) { Fabricate(:admin, username: 'ccc') }
+  let!(:group) do
     group = Fabricate(:group)
     group.add(user1)
     group.add(user2)
     group.add(user3)
     group.save
+    group
+  end
 
+  before do
     SiteSetting.dev_additions_logs_topic_id = topic.id
     SiteSetting.dev_additions_logs_topic_group = group.name
     SiteSetting.assign_allowed_on_groups = group.id.to_s
     SiteSetting.assign_enabled = true
+  end
+
+  it "should automatically assign topic correctly" do
+    # clearly require discourse assign to work
 
     # Time.zone is UTC (3rd was a Monday)
     freeze_time(Time.zone.parse('2000-01-03'))
@@ -64,10 +66,45 @@ describe DevAdditions::CheckPolicy do
     expect(topic.reload.custom_fields["assigned_to_id"]).to eq(user3.id.to_s)
 
     # lets start another round
+    #
+    # it is necessary to add another user because of the 2 week restriction and our limited
+    # pool of users in this spec.
+    user4 = Fabricate(:admin, username: 'ddd')
+    group.add(user4)
     freeze_time(Time.zone.parse('2000-01-10 08:00'))
 
     DevAdditions::CheckPolicy.new.execute(random: false)
+    expect(topic.reload.custom_fields["assigned_to_id"]).to eq(user4.id.to_s)
+  end
+
+  it "should not assign the same person more than once in 2 weeks (say for sparse holiday periods)" do
+    # Time.zone is UTC (3rd was a Monday)
+    freeze_time(Time.zone.parse('2000-01-03'))
+    freeze_time(8.hours.from_now)
+    user2.custom_fields["on_holiday"] = "t"
+    user2.save_custom_fields
+
+    # UTC + 4
+    user3.user_option.timezone = "Asia/Baku"
+    user3.user_option.save
+
+    DevAdditions::CheckPolicy.new.execute(random: false)
+
     expect(topic.reload.custom_fields["assigned_to_id"]).to eq(user1.id.to_s)
 
+    # the +4 will bring it to 9am
+    freeze_time(Time.zone.parse('2000-01-04 05:00'))
+    DevAdditions::CheckPolicy.new.execute(random: false)
+
+    expect(topic.reload.custom_fields["assigned_to_id"]).to eq(user3.id.to_s)
+
+    # going back to the original time will not reassign to user_1 because they
+    # were already assigned in the last 2 weeks
+    freeze_time(Time.zone.parse('2000-01-03'))
+    freeze_time(8.hours.from_now)
+
+    DevAdditions::CheckPolicy.new.execute(random: false)
+
+    expect(topic.reload.custom_fields["assigned_to_id"]).to eq(user3.id.to_s)
   end
 end

GitHub sha: b9b7ae00

2 Likes

This commit appears in #2 which was merged by martin.