diff --git a/app/models/assignment.rb b/app/models/assignment.rb index 58479e5..bf28baf 100644 --- a/app/models/assignment.rb +++ b/app/models/assignment.rb @@ -23,20 +23,30 @@ class Assignment < ActiveRecord::Base validate :validate_status, if: -> { SiteSetting.enable_assign_status } - def self.valid_type?(type) - VALID_TYPES.include?(type.downcase) - end + class << self + def valid_type?(type) + VALID_TYPES.include?(type.downcase) + end - def self.statuses - SiteSetting.assign_statuses.split("|") - end + def statuses + SiteSetting.assign_statuses.split("|") + end - def self.default_status - Assignment.statuses.first - end + def default_status + Assignment.statuses.first + end - def self.status_enabled? - SiteSetting.enable_assign_status + def status_enabled? + SiteSetting.enable_assign_status + end + + def deactivate!(topic:) + active.where(topic: topic).find_each(&:deactivate!) + end + + def reactivate!(topic:) + inactive.where(topic: topic).find_each(&:reactivate!) + end end def assigned_to_user? @@ -67,6 +77,23 @@ class Assignment < ActiveRecord::Base end end + def reactivate! + return unless target + update!(active: true) + Jobs.enqueue(:assign_notification, assignment_id: id) + end + + def deactivate! + update!(active: false) + Jobs.enqueue( + :unassign_notification, + topic_id: topic_id, + assigned_to_id: assigned_to_id, + assigned_to_type: assigned_to_type, + assignment_id: id, + ) + end + private def default_status diff --git a/plugin.rb b/plugin.rb index b95ed8f..bbee0e8 100644 --- a/plugin.rb +++ b/plugin.rb @@ -784,7 +784,7 @@ after_initialize do on(:topic_status_updated) do |topic, status, enabled| if SiteSetting.unassign_on_close && (status == "closed" || status == "autoclosed") && enabled && - Assignment.exists?(topic_id: topic.id, active: true) + Assignment.active.exists?(topic: topic) assigner = ::Assigner.new(topic, Discourse.system_user) assigner.unassign(silent: true, deactivate: true) @@ -799,19 +799,15 @@ after_initialize do end if SiteSetting.reassign_on_open && (status == "closed" || status == "autoclosed") && !enabled && - Assignment.exists?(topic_id: topic.id, active: false) - Assignment.where(topic_id: topic.id, target_type: "Topic").update_all(active: true) - Assignment - .where(topic_id: topic.id, target_type: "Post") - .joins("INNER JOIN posts ON posts.id = target_id AND posts.deleted_at IS NULL") - .update_all(active: true) + Assignment.inactive.exists?(topic: topic) + Assignment.reactivate!(topic: topic) MessageBus.publish("/topic/#{topic.id}", reload_topic: true, refresh_stream: true) end end on(:post_destroyed) do |post| - if Assignment.exists?(target_type: "Post", target_id: post.id, active: true) - Assignment.where(target_type: "Post", target_id: post.id).update_all(active: false) + if Assignment.active.exists?(target: post) + post.assignment.deactivate! MessageBus.publish("/topic/#{post.topic_id}", reload_topic: true, refresh_stream: true) end @@ -830,9 +826,8 @@ after_initialize do end on(:post_recovered) do |post| - if SiteSetting.reassign_on_open && - Assignment.where(target_type: "Post", target_id: post.id, active: false) - Assignment.where(target_type: "Post", target_id: post.id).update_all(active: true) + if SiteSetting.reassign_on_open && Assignment.inactive.exists?(target: post) + post.assignment.reactivate! MessageBus.publish("/topic/#{post.topic_id}", reload_topic: true, refresh_stream: true) end end @@ -847,14 +842,7 @@ after_initialize do next if !SiteSetting.unassign_on_group_archive next if !info[:group] - Assignment - .inactive - .where(topic: topic) - .find_each do |assignment| - next unless assignment.target - assignment.update!(active: true) - Jobs.enqueue(:assign_notification, assignment_id: assignment.id) - end + Assignment.reactivate!(topic: topic) end on(:archive_message) do |info| @@ -866,19 +854,7 @@ after_initialize do next if !SiteSetting.unassign_on_group_archive next if !info[:group] - Assignment - .active - .where(topic: topic) - .find_each do |assignment| - assignment.update!(active: false) - Jobs.enqueue( - :unassign_notification, - topic_id: topic.id, - assigned_to_id: assignment.assigned_to_id, - assigned_to_type: assignment.assigned_to_type, - assignment_id: assignment.id, - ) - end + Assignment.deactivate!(topic: topic) end on(:user_added_to_group) do |user, group, automatic:| diff --git a/spec/models/assignment_spec.rb b/spec/models/assignment_spec.rb index 1b29305..f3c4188 100644 --- a/spec/models/assignment_spec.rb +++ b/spec/models/assignment_spec.rb @@ -28,6 +28,36 @@ RSpec.describe Assignment do end end + describe ".deactivate!" do + subject(:deactivate!) { described_class.deactivate!(topic: topic) } + + let!(:assignment1) { Fabricate(:topic_assignment) } + let!(:assignment2) { Fabricate(:post_assignment, topic: topic) } + let!(:assignment3) { Fabricate(:post_assignment) } + let(:topic) { assignment1.topic } + + it "deactivates each assignment of the provided topic" do + deactivate! + expect([assignment1, assignment2].map(&:reload).map(&:active?)).to all eq false + expect(assignment3.reload).to be_active + end + end + + describe ".reactivate!" do + subject(:reactivate!) { described_class.reactivate!(topic: topic) } + + let!(:assignment1) { Fabricate(:topic_assignment, active: false) } + let!(:assignment2) { Fabricate(:post_assignment, topic: topic, active: false) } + let!(:assignment3) { Fabricate(:post_assignment, active: false) } + let(:topic) { assignment1.topic } + + it "reactivates each assignment of the provided topic" do + reactivate! + expect([assignment1, assignment2].map(&:reload)).to all be_active + expect(assignment3.reload).not_to be_active + end + end + describe "#assigned_users" do subject(:assigned_users) { assignment.assigned_users } @@ -170,4 +200,52 @@ RSpec.describe Assignment do end end end + + describe "#reactivate!" do + subject(:reactivate!) { assignment.reactivate! } + + fab!(:assignment) { Fabricate.create(:topic_assignment, active: false) } + + context "when target does not exist" do + before { assignment.target.delete } + + it "does nothing" do + expect { reactivate! }.not_to change { assignment.reload.active } + end + end + + context "when target exists" do + it "sets the assignment as active" do + expect { reactivate! }.to change { assignment.reload.active? }.to true + end + + it "enqueues a job to create notifications" do + reactivate! + expect_job_enqueued(job: Jobs::AssignNotification, args: { assignment_id: assignment.id }) + end + end + end + + describe "#deactivate!" do + subject(:deactivate!) { assignment.deactivate! } + + fab!(:assignment) { Fabricate.create(:topic_assignment) } + + it "sets the assignment as inactive" do + expect { deactivate! }.to change { assignment.reload.active? }.to false + end + + it "enqueues a job to delete notifications" do + deactivate! + expect_job_enqueued( + job: Jobs::UnassignNotification, + args: { + topic_id: assignment.topic_id, + assigned_to_id: assignment.assigned_to_id, + assigned_to_type: assignment.assigned_to_type, + assignment_id: assignment.id, + }, + ) + end + end end diff --git a/spec/plugin_spec.rb b/spec/plugin_spec.rb index 1537b1f..fd69deb 100644 --- a/spec/plugin_spec.rb +++ b/spec/plugin_spec.rb @@ -55,5 +55,96 @@ RSpec.describe DiscourseAssign do ).to eq(false) end end + + describe "on 'topic_status_updated'" do + context "when closing a topic" do + let!(:first_assignment) { Fabricate(:topic_assignment) } + let!(:second_assignment) { Fabricate(:post_assignment, topic: topic) } + let(:topic) { first_assignment.topic } + + before do + SiteSetting.unassign_on_close = true + topic.update_status("closed", true, Discourse.system_user) + end + + it "deactivates existing assignments" do + [first_assignment, second_assignment].each do |assignment| + assignment.reload + expect(assignment).not_to be_active + expect_job_enqueued( + job: Jobs::UnassignNotification, + args: { + topic_id: assignment.topic_id, + assignment_id: assignment.id, + assigned_to_id: assignment.assigned_to_id, + assigned_to_type: assignment.assigned_to_type, + }, + ) + end + end + end + + context "when reopening a topic" do + let!(:topic) { Fabricate(:closed_topic) } + let!(:first_assignment) { Fabricate(:topic_assignment, topic: topic, active: false) } + let!(:second_assignment) { Fabricate(:post_assignment, topic: topic, active: false) } + + before do + SiteSetting.reassign_on_open = true + topic.update_status("closed", false, Discourse.system_user) + end + + it "reactivates existing assignments" do + [first_assignment, second_assignment].each do |assignment| + assignment.reload + expect(assignment).to be_active + expect_job_enqueued( + job: Jobs::AssignNotification, + args: { + assignment_id: assignment.id, + }, + ) + end + end + end + end + + describe "on 'post_destroyed'" do + let!(:assignment) { Fabricate(:post_assignment) } + let(:post) { assignment.target } + + before { PostDestroyer.new(Discourse.system_user, post).destroy } + + it "deactivates the existing assignment" do + assignment.reload + expect(assignment).not_to be_active + expect_job_enqueued( + job: Jobs::UnassignNotification, + args: { + topic_id: assignment.topic_id, + assignment_id: assignment.id, + assigned_to_id: assignment.assigned_to_id, + assigned_to_type: assignment.assigned_to_type, + }, + ) + end + end + + describe "on 'post_recovered'" do + let!(:assignment) { Fabricate(:post_assignment, active: false) } + let(:post) { assignment.target } + + before do + SiteSetting.reassign_on_open = true + post.trash! + PostDestroyer.new(Discourse.system_user, post).recover + end + + it "reactivates the existing assignment" do + assignment.reload + expect(assignment).to be_active + expect_job_enqueued(job: Jobs::AssignNotification, args: { assignment_id: assignment.id }) + end + end end end