diff --git a/app/models/assignment.rb b/app/models/assignment.rb index f687235..1563ab6 100644 --- a/app/models/assignment.rb +++ b/app/models/assignment.rb @@ -22,3 +22,26 @@ class Assignment < ActiveRecord::Base assigned_to_type == 'Group' end end + +# == Schema Information +# +# Table name: assignments +# +# id :bigint not null, primary key +# topic_id :integer not null +# assigned_to_id :integer not null +# assigned_by_user_id :integer not null +# created_at :datetime not null +# updated_at :datetime not null +# assigned_to_type :string not null +# target_id :integer not null +# target_type :string not null +# active :boolean default(TRUE) +# +# Indexes +# +# index_assignments_on_active (active) +# index_assignments_on_assigned_to_id_and_assigned_to_type (assigned_to_id,assigned_to_type) +# index_assignments_on_target_id_and_target_type (target_id,target_type) UNIQUE +# unique_target_and_assigned (assigned_to_id,assigned_to_type,target_id,target_type) UNIQUE +# diff --git a/app/serializers/assigned_topic_serializer.rb b/app/serializers/assigned_topic_serializer.rb index a0a9b46..80b0894 100644 --- a/app/serializers/assigned_topic_serializer.rb +++ b/app/serializers/assigned_topic_serializer.rb @@ -17,7 +17,7 @@ class AssignedTopicSerializer < BasicTopicSerializer end def include_assigned_to_user? - object.assignment.assigned_to_user? + object.assignment.assigned_to_user? && object.assignment.active end def assigned_to_group @@ -25,6 +25,6 @@ class AssignedTopicSerializer < BasicTopicSerializer end def include_assigned_to_group? - object.assigned_to.is_a?(Group) + object.assignment.assigned_to_group? && object.assignment.active end end diff --git a/assets/javascripts/discourse-assign/initializers/extend-for-assigns.js.es6 b/assets/javascripts/discourse-assign/initializers/extend-for-assigns.js.es6 index 1c009f6..c117757 100644 --- a/assets/javascripts/discourse-assign/initializers/extend-for-assigns.js.es6 +++ b/assets/javascripts/discourse-assign/initializers/extend-for-assigns.js.es6 @@ -478,9 +478,7 @@ function initialize(api) { api.attachWidgetAction("post", "unassignPost", function () { const taskActions = getOwner(this).lookup("service:task-actions"); taskActions.unassign(this.model.id, "Post").then(() => { - delete this.model.topic.indirectly_assigned_to[ - this.model.post_number - ]; + delete this.model.topic.indirectly_assigned_to[this.model.id]; }); }); } @@ -593,14 +591,18 @@ function initialize(api) { assignedToIndirectly = Object.entries( topic.get("indirectly_assigned_to") ).map(([key, value]) => { - value.assignedToPostId = key; + value.assigned_to.assignedToPostId = key; return value; }); } else { assignedToIndirectly = []; } const assignedTo = [] - .concat(assignedToUser, assignedToGroup, assignedToIndirectly) + .concat( + assignedToUser, + assignedToGroup, + assignedToIndirectly.map((assigned) => assigned.assigned_to) + ) .filter((element) => element) .flat() .uniqBy((assignee) => assignee.assign_path); @@ -707,8 +709,9 @@ function initialize(api) { ); } if (indirectlyAssignedTo) { - Object.keys(indirectlyAssignedTo).map((postNumber) => { - const assignee = indirectlyAssignedTo[postNumber]; + Object.keys(indirectlyAssignedTo).map((postId) => { + const assignee = indirectlyAssignedTo[postId].assigned_to; + const postNumber = indirectlyAssignedTo[postId].post_number; assigneeElements.push( h("span.assignee", [ h( @@ -811,7 +814,7 @@ function initialize(api) { }); } else { postAssignment = - postModel.topic.indirectly_assigned_to?.[postModel.post_number]; + postModel.topic.indirectly_assigned_to?.[postModel.id]?.assigned_to; if (postAssignment?.username) { assignedToUser = postAssignment; } diff --git a/assets/javascripts/discourse/components/assign-actions-dropdown.js.es6 b/assets/javascripts/discourse/components/assign-actions-dropdown.js.es6 index 128ff4e..ad1177b 100644 --- a/assets/javascripts/discourse/components/assign-actions-dropdown.js.es6 +++ b/assets/javascripts/discourse/components/assign-actions-dropdown.js.es6 @@ -32,7 +32,8 @@ export default DropdownSelectBoxComponent.extend({ if (this.topic.indirectly_assigned_to) { Object.entries(this.topic.indirectly_assigned_to).forEach((entry) => { - const [postId, assignee] = entry; + const [postId, assignment_map] = entry; + const assignee = assignment_map.assigned_to; options = options.concat({ id: `unassign_post_${postId}`, icon: assignee.username ? "user-times" : "group-times", diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 1529ce3..125030c 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -9,6 +9,7 @@ en: assign_other_regex: "Regex that needs to pass for assigning topics to others via mention. Example 'your list'." unassign_on_group_archive: "When a message is archived by a group, unassign message (reassign if moved back to inbox)" unassign_on_close: "When a topic is closed unassign topic" + reassign_on_open: "When a topic is opened reassign previously assigned users/groups" assign_mailer: "When to send notification email for assignments" remind_assigns: "Remind users about pending assigns." remind_assigns_frequency: "Frequency for reminding users about assigned topics." diff --git a/config/settings.yml b/config/settings.yml index 9f521ae..08940f3 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -11,6 +11,7 @@ plugins: assign_other_regex: "" unassign_on_close: false unassign_on_group_archive: false + reassign_on_open: false assigns_user_url_path: client: true default: "/u/{username}/activity/assigned" diff --git a/db/migrate/20211206060512_add_active_to_assignments.rb b/db/migrate/20211206060512_add_active_to_assignments.rb new file mode 100644 index 0000000..497e404 --- /dev/null +++ b/db/migrate/20211206060512_add_active_to_assignments.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +class AddActiveToAssignments < ActiveRecord::Migration[6.1] + def change + add_column :assignments, :active, :boolean, default: true + add_index :assignments, :active + end +end diff --git a/db/post_migrate/20211206081254_move_prev_assigned_custom_fields_to_assignments.rb b/db/post_migrate/20211206081254_move_prev_assigned_custom_fields_to_assignments.rb new file mode 100644 index 0000000..d8e7723 --- /dev/null +++ b/db/post_migrate/20211206081254_move_prev_assigned_custom_fields_to_assignments.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +class MovePrevAssignedCustomFieldsToAssignments < ActiveRecord::Migration[6.1] + def up + execute <<~SQL + INSERT INTO assignments (assigned_to_id, assigned_to_type, assigned_by_user_id, topic_id, target_id, target_type, created_at, updated_at, active) + SELECT + prev_assigned_to_id.value::integer, + prev_assigned_to_type.value, + #{Discourse::SYSTEM_USER_ID}, + prev_assigned_to_type.topic_id, + prev_assigned_to_type.topic_id, + 'Topic', + prev_assigned_to_type.created_at, + prev_assigned_to_type.updated_at, + false + FROM topic_custom_fields prev_assigned_to_type + INNER JOIN topic_custom_fields prev_assigned_to_id ON prev_assigned_to_type.topic_id = prev_assigned_to_id.topic_id + WHERE prev_assigned_to_type.name = 'prev_assigned_to_type' + AND prev_assigned_to_id.name = 'prev_assigned_to_id' + ORDER BY prev_assigned_to_type.created_at DESC + ON CONFLICT DO NOTHING + SQL + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/lib/assigner.rb b/lib/assigner.rb index fa3b506..d91fe17 100644 --- a/lib/assigner.rb +++ b/lib/assigner.rb @@ -188,9 +188,9 @@ class ::Assigner topic.private_message? ? :forbidden_group_assignee_not_pm_participant : :forbidden_group_assignee_cant_see_topic when !can_be_assigned?(assign_to) assign_to.is_a?(User) ? :forbidden_assign_to : :forbidden_group_assign_to - when topic.assignment&.assigned_to_id == assign_to.id && topic.assignment&.assigned_to_type == type + when topic.assignment&.assigned_to_id == assign_to.id && topic.assignment&.assigned_to_type == type && topic.assignment.active == true assign_to.is_a?(User) ? :already_assigned : :group_already_assigned - when @target.is_a?(Topic) && Assignment.where(topic_id: topic.id, target_type: "Post").any? { |assignment| assignment.assigned_to_id == assign_to.id && assignment.assigned_to_type == type } + when @target.is_a?(Topic) && Assignment.where(topic_id: topic.id, target_type: "Post", active: true).any? { |assignment| assignment.assigned_to_id == assign_to.id && assignment.assigned_to_type == type } assign_to.is_a?(User) ? :already_assigned : :group_already_assigned when Assignment.where(topic: topic).count >= ASSIGNMENTS_PER_TOPIC_LIMIT :too_many_assigns_for_topic @@ -263,7 +263,7 @@ class ::Assigner custom_fields = { "action_code_who" => assign_to.is_a?(User) ? assign_to.username : assign_to.name } if post_target? - custom_fields.merge!("action_code_path" => "/p/#{@target.id}") + custom_fields.merge!({ "action_code_path" => "/p/#{@target.id}", "action_code_post_id" => @target.id }) end topic.add_moderator_post( @@ -305,9 +305,9 @@ class ::Assigner { success: true } end - def unassign(silent: false) + def unassign(silent: false, deactivate: false) if assignment = @target.assignment - assignment.destroy! + deactivate ? assignment.update!(active: false) : assignment.destroy! return if first_post.blank? @@ -344,6 +344,7 @@ class ::Assigner if post_target? custom_fields.merge!("action_code_path" => "/p/#{@target.id}") + custom_fields.merge!("action_code_post_id" => @target.id) end topic.add_moderator_post( diff --git a/lib/discourse_assign/helpers.rb b/lib/discourse_assign/helpers.rb index bb3dc8a..8deea1a 100644 --- a/lib/discourse_assign/helpers.rb +++ b/lib/discourse_assign/helpers.rb @@ -28,11 +28,14 @@ module DiscourseAssign end def self.build_indirectly_assigned_to(post_assignments, topic) - post_assignments.map do |post_id, assigned_to| + post_assignments.map do |post_id, assigned_map| + assigned_to = assigned_map[:assigned_to] + post_number = assigned_map[:post_number] + if (assigned_to.is_a?(User)) - [post_id, build_assigned_to_user(assigned_to, topic)] + [post_id, { assigned_to: build_assigned_to_user(assigned_to, topic), post_number: post_number }] elsif assigned_to.is_a?(Group) - [post_id, build_assigned_to_group(assigned_to, topic)] + [post_id, { assigned_to: build_assigned_to_group(assigned_to, topic), post_number: post_number }] end end.to_h end diff --git a/plugin.rb b/plugin.rb index b21ca64..83558ab 100644 --- a/plugin.rb +++ b/plugin.rb @@ -73,8 +73,6 @@ after_initialize do frequency_field = PendingAssignsReminder::REMINDERS_FREQUENCY register_editable_user_custom_field frequency_field User.register_custom_field_type frequency_field, :integer - Topic.register_custom_field_type "prev_assigned_to_id", :integer - Topic.register_custom_field_type "prev_assigned_to_type", :string DiscoursePluginRegistry.serialized_current_user_fields << frequency_field add_to_serializer(:user, :reminders_frequency) do RemindAssignsFrequencySiteSettings.values @@ -87,7 +85,8 @@ after_initialize do ON topics.id = a.topic_id AND a.assigned_to_id IS NOT NULL SQL .where(<<~SQL, group_id: object.id) - ( + a.active AND + (( a.assigned_to_type = 'User' AND a.assigned_to_id IN ( SELECT group_users.user_id FROM group_users @@ -95,7 +94,7 @@ after_initialize do ) ) OR ( a.assigned_to_type = 'Group' AND a.assigned_to_id = :group_id - ) + )) SQL .where("topics.deleted_at IS NULL") .count @@ -213,7 +212,7 @@ after_initialize do allowed_access = SiteSetting.assigns_public || can_assign if allowed_access && topics.length > 0 - assignments = Assignment.strict_loading.where(topic: topics) + assignments = Assignment.strict_loading.where(topic: topics, active: true).includes(:target) assignments_map = assignments.group_by(&:topic_id) user_ids = assignments.filter { |assignment| assignment.assigned_to_user? }.map(&:assigned_to_id) @@ -228,8 +227,9 @@ after_initialize do indirectly_assigned_to = {} assignments&.each do |assignment| next if assignment.target_type == "Topic" - next indirectly_assigned_to[assignment.target_id] = users_map[assignment.assigned_to_id] if assignment&.assigned_to_user? - next indirectly_assigned_to[assignment.target_id] = groups_map[assignment.assigned_to_id] if assignment&.assigned_to_group? + next if !assignment.target + next indirectly_assigned_to[assignment.target_id] = { assigned_to: users_map[assignment.assigned_to_id], post_number: assignment.target.post_number } if assignment&.assigned_to_user? + next indirectly_assigned_to[assignment.target_id] = { assigned_to: groups_map[assignment.assigned_to_id], post_number: assignment.target.post_number } if assignment&.assigned_to_group? end&.compact&.uniq assigned_to = @@ -284,13 +284,13 @@ after_initialize do if name == "nobody" next results - .joins("LEFT JOIN assignments a ON a.topic_id = topics.id") + .joins("LEFT JOIN assignments a ON a.topic_id = topics.id AND active") .where("a.assigned_to_id IS NULL") end if name == "*" next results - .joins("JOIN assignments a ON a.topic_id = topics.id") + .joins("JOIN assignments a ON a.topic_id = topics.id AND active") .where("a.assigned_to_id IS NOT NULL") end @@ -299,7 +299,7 @@ after_initialize do if user_id next results - .joins("JOIN assignments a ON a.topic_id = topics.id") + .joins("JOIN assignments a ON a.topic_id = topics.id AND active") .where("a.assigned_to_id = ? AND a.assigned_to_type = 'User'", user_id) end @@ -307,7 +307,7 @@ after_initialize do if group_id next results - .joins("JOIN assignments a ON a.topic_id = topics.id") + .joins("JOIN assignments a ON a.topic_id = topics.id AND active") .where("a.assigned_to_id = ? AND a.assigned_to_type = 'Group'", group_id) end @@ -321,7 +321,7 @@ after_initialize do SELECT topic_id FROM assignments LEFT JOIN group_users ON group_users.user_id = :user_id WHERE - assigned_to_id = :user_id AND assigned_to_type = 'User' + assigned_to_id = :user_id AND assigned_to_type = 'User' AND active SQL if @options[:filter] != :direct @@ -345,6 +345,7 @@ after_initialize do WHERE ( assigned_to_id = :group_id AND assigned_to_type = 'Group' ) + AND active SQL if @options[:filter] != :direct @@ -375,8 +376,9 @@ after_initialize do list = list.where(<<~SQL, user_id: user.id, group_ids: group_ids) topics.id IN ( SELECT topic_id FROM assignments WHERE - (assigned_to_id = :user_id AND assigned_to_type = 'User') OR - (assigned_to_id IN (:group_ids) AND assigned_to_type = 'Group') + active AND + ((assigned_to_id = :user_id AND assigned_to_type = 'User') OR + (assigned_to_id IN (:group_ids) AND assigned_to_type = 'Group')) ) SQL end @@ -423,13 +425,13 @@ after_initialize do # Topic add_to_class(:topic, :assigned_to) do return @assigned_to if defined?(@assigned_to) - @assigned_to = assignment&.assigned_to + @assigned_to = assignment.assigned_to if assignment&.active end add_to_class(:topic, :indirectly_assigned_to) do return @indirectly_assigned_to if defined?(@indirectly_assigned_to) - @indirectly_assigned_to = Assignment.where(topic_id: id, target_type: "Post").inject({}) do |acc, assignment| - acc[assignment.target.post_number] = assignment.assigned_to if assignment.target + @indirectly_assigned_to = Assignment.where(topic_id: id, target_type: "Post", active: true).includes(:target).inject({}) do |acc, assignment| + acc[assignment.target_id] = { assigned_to: assignment.assigned_to, post_number: assignment.target.post_number } if assignment.target acc end end @@ -619,7 +621,7 @@ after_initialize do end add_to_serializer(:post, 'include_assigned_to_user?') do - (SiteSetting.assigns_public || scope.can_assign?) && object.assignment&.assigned_to&.is_a?(User) + (SiteSetting.assigns_public || scope.can_assign?) && object.assignment&.assigned_to&.is_a?(User) && object.assignment.active end add_to_serializer(:post, :assigned_to_group, false) do @@ -627,7 +629,7 @@ after_initialize do end add_to_serializer(:post, 'include_assigned_to_group?') do - (SiteSetting.assigns_public || scope.can_assign?) && object.assignment&.assigned_to&.is_a?(Group) + (SiteSetting.assigns_public || scope.can_assign?) && object.assignment&.assigned_to&.is_a?(Group) && object.assignment.active end # CurrentUser serializer @@ -699,7 +701,38 @@ after_initialize do on(:topic_status_updated) do |topic, status, enabled| if SiteSetting.unassign_on_close && (status == 'closed' || status == 'autoclosed') && enabled assigner = ::Assigner.new(topic, Discourse.system_user) - assigner.unassign(silent: true) + assigner.unassign(silent: true, deactivate: true) + + topic.posts.joins(:assignment).find_each do |post| + assigner = ::Assigner.new(post, Discourse.system_user) + assigner.unassign(silent: true, deactivate: true) + end + end + + if SiteSetting.reassign_on_open && (status == 'closed' || status == 'autoclosed') && !enabled + 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) + end + end + + on(:post_destroyed) do |post| + if SiteSetting.unassign_on_close + Assignment.where(target_type: "Post", target_id: post.id).update_all(active: false) + end + + # small actions have to be destroyed as link is incorrect + PostCustomField.where(name: "action_code_post_id", value: post.id).find_each do |post_custom_field| + next if ![Post.types[:small_action], Post.types[:whisper]].include?(post_custom_field.post.post_type) + post_custom_field.post.destroy + end + end + + on(:post_recovered) do |post| + if SiteSetting.reassign_on_open + Assignment.where(target_type: "Post", target_id: post.id).update_all(active: true) end end @@ -711,21 +744,17 @@ after_initialize do next if !SiteSetting.unassign_on_group_archive next if !info[:group] - previous_assigned_to_id = topic.custom_fields["prev_assigned_to_id"]&.to_i - next if !previous_assigned_to_id - - assigned_type = topic.custom_fields["prev_assigned_to_type"] - assigned_class = assigned_type == "Group" ? Group : User - previous_assigned_to = assigned_class.find_by(id: previous_assigned_to_id) - - if previous_assigned_to - assigner = Assigner.new(topic, Discourse.system_user) - assigner.assign(previous_assigned_to, silent: true) + Assignment.where(topic_id: topic.id, active: false).find_each do |assignment| + next unless assignment.target + assignment.update!(active: true) + Jobs.enqueue(:assign_notification, + topic_id: topic.id, + post_id: assignment.target_type.is_a?(Topic) ? topic.first_post.id : assignment.target.id, + assigned_to_id: assignment.assigned_to_id, + assigned_to_type: assignment.assigned_to_type, + assigned_by_id: assignment.assigned_by_user_id, + silent: true) end - - topic.custom_fields.delete("prev_assigned_to_id") - topic.custom_fields.delete("prev_assigned_to_type") - topic.save! end on(:archive_message) do |info| @@ -737,12 +766,13 @@ after_initialize do next if !SiteSetting.unassign_on_group_archive next if !info[:group] - topic.custom_fields["prev_assigned_to_id"] = topic.assignment.assigned_to_id - topic.custom_fields["prev_assigned_to_type"] = topic.assignment.assigned_to_type - topic.save! - - assigner = Assigner.new(topic, Discourse.system_user) - assigner.unassign(silent: true) + Assignment.where(topic_id: topic.id, active: true).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) + end end on(:user_removed_from_group) do |user, group| diff --git a/spec/integration/assign_spec.rb b/spec/integration/assign_spec.rb index 95a4c8b..2d5785a 100644 --- a/spec/integration/assign_spec.rb +++ b/spec/integration/assign_spec.rb @@ -78,14 +78,11 @@ describe 'integration tests' do assigner.assign(user) GroupArchivedMessage.archive!(group.id, pm.reload) - expect(pm.assignment).to eq(nil) - expect(pm.custom_fields["prev_assigned_to_id"]).to eq(user.id) - expect(pm.custom_fields["prev_assigned_to_type"]).to eq("User") + expect(pm.assignment.active).to be false GroupArchivedMessage.move_to_inbox!(group.id, pm.reload) + expect(pm.assignment.active).to be true expect(pm.assignment.assigned_to).to eq(user) - expect(pm.custom_fields["prev_assigned_to_id"]).to eq(nil) - expect(pm.custom_fields["prev_assigned_to_type"]).to eq(nil) end it "unassign and assign group if unassign_on_group_archive" do @@ -94,11 +91,10 @@ describe 'integration tests' do assigner.assign(group) GroupArchivedMessage.archive!(group.id, pm.reload) - expect(pm.assignment).to eq(nil) - expect(pm.custom_fields["prev_assigned_to_id"]).to eq(group.id) - expect(pm.custom_fields["prev_assigned_to_type"]).to eq("Group") + expect(pm.assignment.active).to be false GroupArchivedMessage.move_to_inbox!(group.id, pm.reload) + expect(pm.assignment.active).to be true expect(pm.assignment.assigned_to).to eq(group) end end diff --git a/spec/lib/assigner_spec.rb b/spec/lib/assigner_spec.rb index c01737f..a2269a6 100644 --- a/spec/lib/assigner_spec.rb +++ b/spec/lib/assigner_spec.rb @@ -25,8 +25,8 @@ RSpec.describe Assigner do let(:secure_category) { Fabricate(:private_category, group: Fabricate(:group)) } let(:secure_topic) { Fabricate(:post).topic.tap { |t| t.update(category: secure_category) } } let(:moderator) { Fabricate(:moderator, groups: [assign_allowed_group]) } - let(:moderator2) { Fabricate(:moderator, groups: [assign_allowed_group]) } - let(:assigner) { described_class.new(topic, moderator2) } + let(:moderator_2) { Fabricate(:moderator, groups: [assign_allowed_group]) } + let(:assigner) { described_class.new(topic, moderator_2) } let(:assigner_self) { described_class.new(topic, moderator) } it "can assign and unassign correctly" do @@ -127,7 +127,7 @@ RSpec.describe Assigner do another_post = Fabricate.build(:post) assigner.assign(moderator) - second_assign = described_class.new(another_post.topic, moderator2).assign(moderator) + second_assign = described_class.new(another_post.topic, moderator_2).assign(moderator) expect(second_assign[:success]).to eq(false) expect(second_assign[:reason]).to eq(:too_many_assigns) @@ -150,7 +150,7 @@ RSpec.describe Assigner do first_assign = assigner.assign(moderator) # reached limit so stop - second_assign = described_class.new(Fabricate(:topic), moderator2).assign(moderator) + second_assign = described_class.new(Fabricate(:topic), moderator_2).assign(moderator) # self assign has a bypass third_assign = described_class.new(another_post.topic, moderator).assign(moderator) @@ -274,32 +274,123 @@ RSpec.describe Assigner do let(:post) { Fabricate(:post) } let(:topic) { post.topic } let(:moderator) { Fabricate(:moderator, groups: [assign_allowed_group]) } - let(:assigner) { described_class.new(topic, moderator) } - before do - SiteSetting.unassign_on_close = true + context 'topic' do + let(:assigner) { described_class.new(topic, moderator) } - assigner.assign(moderator) + before do + SiteSetting.unassign_on_close = true + assigner.assign(moderator) + end + + it "unassigns on topic closed" do + topic.update_status("closed", true, moderator) + expect(TopicQuery.new(moderator, assigned: moderator.username).list_latest.topics).to be_blank + end + + it "unassigns on topic autoclosed" do + topic.update_status("autoclosed", true, moderator) + expect(TopicQuery.new(moderator, assigned: moderator.username).list_latest.topics).to be_blank + end + + it "does not unassign on topic open" do + topic.update_status("closed", false, moderator) + expect(TopicQuery.new(moderator, assigned: moderator.username).list_latest.topics).to eq([topic]) + end + + it "does not unassign on automatic topic open" do + topic.update_status("autoclosed", false, moderator) + expect(TopicQuery.new(moderator, assigned: moderator.username).list_latest.topics).to eq([topic]) + end end - it "unassigns on topic closed" do - topic.update_status("closed", true, moderator) - expect(TopicQuery.new(moderator, assigned: moderator.username).list_latest.topics).to be_blank + context 'post' do + let(:post_2) { Fabricate(:post, topic: topic) } + let(:assigner) { described_class.new(post_2, moderator) } + + before do + SiteSetting.unassign_on_close = true + SiteSetting.reassign_on_open = true + + assigner.assign(moderator) + end + + it 'deactivates post assignments when topic is closed' do + assigner.assign(moderator) + + expect(post_2.assignment.active).to be true + + topic.update_status("closed", true, moderator) + expect(post_2.assignment.reload.active).to be false + end + + it 'deactivates post assignments when post is deleted and activate when recovered' do + assigner.assign(moderator) + + expect(post_2.assignment.active).to be true + + PostDestroyer.new(moderator, post_2).destroy + expect(post_2.assignment.reload.active).to be false + + PostDestroyer.new(moderator, post_2).recover + expect(post_2.assignment.reload.active).to be true + end + + it 'deletes post small action for deleted post' do + assigner.assign(moderator) + small_action_post = PostCustomField.where(name: "action_code_post_id").first.post + + PostDestroyer.new(moderator, post_2).destroy + expect { small_action_post.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + end + end + + context "reassign_on_open" do + let(:post) { Fabricate(:post) } + let(:topic) { post.topic } + let(:moderator) { Fabricate(:moderator, groups: [assign_allowed_group]) } + + context 'topic' do + let(:assigner) { described_class.new(topic, moderator) } + + before do + SiteSetting.unassign_on_close = true + SiteSetting.reassign_on_open = true + assigner.assign(moderator) + end + + it "reassigns on topic open" do + topic.update_status("closed", true, moderator) + expect(TopicQuery.new(moderator, assigned: moderator.username).list_latest.topics).to be_blank + + topic.update_status("closed", false, moderator) + expect(TopicQuery.new(moderator, assigned: moderator.username).list_latest.topics).to eq([topic]) + end end - it "unassigns on topic autoclosed" do - topic.update_status("autoclosed", true, moderator) - expect(TopicQuery.new(moderator, assigned: moderator.username).list_latest.topics).to be_blank - end + context 'post' do + let(:post_2) { Fabricate(:post, topic: topic) } + let(:assigner) { described_class.new(post_2, moderator) } - it "does not unassign on topic open" do - topic.update_status("closed", false, moderator) - expect(TopicQuery.new(moderator, assigned: moderator.username).list_latest.topics).to eq([topic]) - end + before do + SiteSetting.unassign_on_close = true + SiteSetting.reassign_on_open = true - it "does not unassign on automatic topic open" do - topic.update_status("autoclosed", false, moderator) - expect(TopicQuery.new(moderator, assigned: moderator.username).list_latest.topics).to eq([topic]) + assigner.assign(moderator) + end + + it 'reassigns post on topic open' do + assigner.assign(moderator) + + expect(post_2.assignment.active).to be true + + topic.update_status("closed", true, moderator) + expect(post_2.assignment.reload.active).to be false + + topic.update_status("closed", false, moderator) + expect(post_2.assignment.reload.active).to be true + end end end @@ -307,7 +398,7 @@ RSpec.describe Assigner do let(:post) { Fabricate(:post) } let(:topic) { post.topic } let(:moderator) { Fabricate(:moderator, groups: [assign_allowed_group]) } - let(:moderator2) { Fabricate(:moderator, groups: [assign_allowed_group]) } + let(:moderator_2) { Fabricate(:moderator, groups: [assign_allowed_group]) } it "send an email if set to 'always'" do SiteSetting.assign_mailer = AssignMailer.levels[:always] @@ -326,7 +417,7 @@ RSpec.describe Assigner do it "doesn't send an email if the assigner and assignee are not different" do SiteSetting.assign_mailer = AssignMailer.levels[:different_users] - expect { described_class.new(topic, moderator).assign(moderator2) } + expect { described_class.new(topic, moderator).assign(moderator_2) } .to change { ActionMailer::Base.deliveries.size }.by(1) end @@ -340,7 +431,7 @@ RSpec.describe Assigner do it "doesn't send an email" do SiteSetting.assign_mailer = AssignMailer.levels[:never] - expect { described_class.new(topic, moderator).assign(moderator2) } + expect { described_class.new(topic, moderator).assign(moderator_2) } .to change { ActionMailer::Base.deliveries.size }.by(0) end end diff --git a/spec/serializers/post_serializer_spec.rb b/spec/serializers/post_serializer_spec.rb new file mode 100644 index 0000000..80742fd --- /dev/null +++ b/spec/serializers/post_serializer_spec.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +require 'rails_helper' +require_relative '../support/assign_allowed_group' + +RSpec.describe PostSerializer do + fab!(:user) { Fabricate(:user) } + fab!(:topic) { Fabricate(:topic) } + fab!(:post) { Fabricate(:post, topic: topic) } + let(:guardian) { Guardian.new(user) } + + include_context 'A group that is allowed to assign' + + before do + SiteSetting.assign_enabled = true + add_to_assign_allowed_group(user) + end + + it "includes assigned user in serializer" do + Assigner.new(post, user).assign(user) + serializer = PostSerializer.new(post, scope: guardian) + expect(serializer.as_json[:post][:assigned_to_user].id).to eq(user.id) + expect(serializer.as_json[:post][:assigned_to_group]).to be nil + end + + it "includes assigned group in serializer" do + Assigner.new(post, user).assign(assign_allowed_group) + serializer = PostSerializer.new(post, scope: guardian) + expect(serializer.as_json[:post][:assigned_to_group].id).to eq(assign_allowed_group.id) + expect(serializer.as_json[:post][:assigned_to_user]).to be nil + end +end diff --git a/test/javascripts/acceptance/assigned-topic-test.js.es6 b/test/javascripts/acceptance/assigned-topic-test.js.es6 index 944639f..2ae8d23 100644 --- a/test/javascripts/acceptance/assigned-topic-test.js.es6 +++ b/test/javascripts/acceptance/assigned-topic-test.js.es6 @@ -22,7 +22,10 @@ function assignCurrentUserToTopic(needs) { }; topic["indirectly_assigned_to"] = { 2: { - name: "Developers", + assigned_to: { + name: "Developers", + }, + post_number: 2, }, }; return helper.response(topic); @@ -50,7 +53,10 @@ function assignNewUserToTopic(needs) { }; topic["indirectly_assigned_to"] = { 2: { - name: "Developers", + assigned_to: { + name: "Developers", + }, + post_number: 2, }, }; return helper.response(topic);