From 1dfbc84896dc1946850639c499ad284709a6e8f3 Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Tue, 30 Aug 2022 19:56:03 +0300 Subject: [PATCH] FIX: Count only active assignments when checking limits (#375) Only 5 assignments can exist simultaneously for a topic or its post. The code that counted the assignments did not ignore the inactive assignments which made the limit check more strict than it should have been. --- lib/assigner.rb | 2 +- spec/lib/assigner_spec.rb | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/lib/assigner.rb b/lib/assigner.rb index f3cbecb..5452681 100644 --- a/lib/assigner.rb +++ b/lib/assigner.rb @@ -213,7 +213,7 @@ class ::Assigner assign_to.is_a?(User) ? :forbidden_assign_to : :forbidden_group_assign_to when already_assigned?(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 + when Assignment.where(topic: topic, active: true).count >= ASSIGNMENTS_PER_TOPIC_LIMIT :too_many_assigns_for_topic when !can_assign_to?(assign_to) :too_many_assigns diff --git a/spec/lib/assigner_spec.rb b/spec/lib/assigner_spec.rb index 3c1b450..79a17cf 100644 --- a/spec/lib/assigner_spec.rb +++ b/spec/lib/assigner_spec.rb @@ -174,6 +174,31 @@ RSpec.describe Assigner do end context "forbidden reasons" do + it "doesn't assign if the topic has more than 5 assignments" do + other_post = nil + + # Assign many posts to reach the limit + 1.upto(described_class::ASSIGNMENTS_PER_TOPIC_LIMIT) do + other_post = Fabricate(:post, topic: topic) + user = Fabricate(:moderator, groups: [assign_allowed_group]) + status = described_class.new(other_post, admin).assign(user) + expect(status[:success]).to eq(true) + end + + # Assigning one more post is not allowed + post = Fabricate(:post, topic: topic) + status = described_class.new(post, admin).assign(moderator) + expect(status[:success]).to eq(false) + expect(status[:reason]).to eq(:too_many_assigns_for_topic) + + # Delete a post to mark the assignment as inactive + PostDestroyer.new(admin, other_post).destroy + + # Try assigning again + status = described_class.new(post, admin).assign(moderator) + expect(status[:success]).to eq(true) + end + it "doesn't assign if the user has too many assigned topics" do SiteSetting.max_assigned_topics = 1 another_post = Fabricate.build(:post)