FEATURE: active flag for assignments (#264)

When topic is closed, we mark assignments as "inactive". Then when it is reopen and setting reassign_on_open is enabled, we bring back previous assignments.

We were already using custom fields for archive message and move to inbox. I changed custom fields solution to use active flag on Assignment model
This commit is contained in:
Krzysztof Kotlarek 2021-12-13 08:36:14 +01:00 committed by GitHub
parent 3a76a465a0
commit 1a1dffc5e8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 319 additions and 94 deletions

View File

@ -22,3 +22,26 @@ class Assignment < ActiveRecord::Base
assigned_to_type == 'Group' assigned_to_type == 'Group'
end end
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
#

View File

@ -17,7 +17,7 @@ class AssignedTopicSerializer < BasicTopicSerializer
end end
def include_assigned_to_user? def include_assigned_to_user?
object.assignment.assigned_to_user? object.assignment.assigned_to_user? && object.assignment.active
end end
def assigned_to_group def assigned_to_group
@ -25,6 +25,6 @@ class AssignedTopicSerializer < BasicTopicSerializer
end end
def include_assigned_to_group? def include_assigned_to_group?
object.assigned_to.is_a?(Group) object.assignment.assigned_to_group? && object.assignment.active
end end
end end

View File

@ -478,9 +478,7 @@ function initialize(api) {
api.attachWidgetAction("post", "unassignPost", function () { api.attachWidgetAction("post", "unassignPost", function () {
const taskActions = getOwner(this).lookup("service:task-actions"); const taskActions = getOwner(this).lookup("service:task-actions");
taskActions.unassign(this.model.id, "Post").then(() => { taskActions.unassign(this.model.id, "Post").then(() => {
delete this.model.topic.indirectly_assigned_to[ delete this.model.topic.indirectly_assigned_to[this.model.id];
this.model.post_number
];
}); });
}); });
} }
@ -593,14 +591,18 @@ function initialize(api) {
assignedToIndirectly = Object.entries( assignedToIndirectly = Object.entries(
topic.get("indirectly_assigned_to") topic.get("indirectly_assigned_to")
).map(([key, value]) => { ).map(([key, value]) => {
value.assignedToPostId = key; value.assigned_to.assignedToPostId = key;
return value; return value;
}); });
} else { } else {
assignedToIndirectly = []; assignedToIndirectly = [];
} }
const assignedTo = [] const assignedTo = []
.concat(assignedToUser, assignedToGroup, assignedToIndirectly) .concat(
assignedToUser,
assignedToGroup,
assignedToIndirectly.map((assigned) => assigned.assigned_to)
)
.filter((element) => element) .filter((element) => element)
.flat() .flat()
.uniqBy((assignee) => assignee.assign_path); .uniqBy((assignee) => assignee.assign_path);
@ -707,8 +709,9 @@ function initialize(api) {
); );
} }
if (indirectlyAssignedTo) { if (indirectlyAssignedTo) {
Object.keys(indirectlyAssignedTo).map((postNumber) => { Object.keys(indirectlyAssignedTo).map((postId) => {
const assignee = indirectlyAssignedTo[postNumber]; const assignee = indirectlyAssignedTo[postId].assigned_to;
const postNumber = indirectlyAssignedTo[postId].post_number;
assigneeElements.push( assigneeElements.push(
h("span.assignee", [ h("span.assignee", [
h( h(
@ -811,7 +814,7 @@ function initialize(api) {
}); });
} else { } else {
postAssignment = postAssignment =
postModel.topic.indirectly_assigned_to?.[postModel.post_number]; postModel.topic.indirectly_assigned_to?.[postModel.id]?.assigned_to;
if (postAssignment?.username) { if (postAssignment?.username) {
assignedToUser = postAssignment; assignedToUser = postAssignment;
} }

View File

@ -32,7 +32,8 @@ export default DropdownSelectBoxComponent.extend({
if (this.topic.indirectly_assigned_to) { if (this.topic.indirectly_assigned_to) {
Object.entries(this.topic.indirectly_assigned_to).forEach((entry) => { 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({ options = options.concat({
id: `unassign_post_${postId}`, id: `unassign_post_${postId}`,
icon: assignee.username ? "user-times" : "group-times", icon: assignee.username ? "user-times" : "group-times",

View File

@ -9,6 +9,7 @@ en:
assign_other_regex: "Regex that needs to pass for assigning topics to others via mention. Example 'your list'." 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_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" 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" assign_mailer: "When to send notification email for assignments"
remind_assigns: "Remind users about pending assigns." remind_assigns: "Remind users about pending assigns."
remind_assigns_frequency: "Frequency for reminding users about assigned topics." remind_assigns_frequency: "Frequency for reminding users about assigned topics."

View File

@ -11,6 +11,7 @@ plugins:
assign_other_regex: "" assign_other_regex: ""
unassign_on_close: false unassign_on_close: false
unassign_on_group_archive: false unassign_on_group_archive: false
reassign_on_open: false
assigns_user_url_path: assigns_user_url_path:
client: true client: true
default: "/u/{username}/activity/assigned" default: "/u/{username}/activity/assigned"

View File

@ -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

View File

@ -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

View File

@ -188,9 +188,9 @@ class ::Assigner
topic.private_message? ? :forbidden_group_assignee_not_pm_participant : :forbidden_group_assignee_cant_see_topic topic.private_message? ? :forbidden_group_assignee_not_pm_participant : :forbidden_group_assignee_cant_see_topic
when !can_be_assigned?(assign_to) when !can_be_assigned?(assign_to)
assign_to.is_a?(User) ? :forbidden_assign_to : :forbidden_group_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 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 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).count >= ASSIGNMENTS_PER_TOPIC_LIMIT
:too_many_assigns_for_topic :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 } custom_fields = { "action_code_who" => assign_to.is_a?(User) ? assign_to.username : assign_to.name }
if post_target? 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 end
topic.add_moderator_post( topic.add_moderator_post(
@ -305,9 +305,9 @@ class ::Assigner
{ success: true } { success: true }
end end
def unassign(silent: false) def unassign(silent: false, deactivate: false)
if assignment = @target.assignment if assignment = @target.assignment
assignment.destroy! deactivate ? assignment.update!(active: false) : assignment.destroy!
return if first_post.blank? return if first_post.blank?
@ -344,6 +344,7 @@ class ::Assigner
if post_target? if post_target?
custom_fields.merge!("action_code_path" => "/p/#{@target.id}") custom_fields.merge!("action_code_path" => "/p/#{@target.id}")
custom_fields.merge!("action_code_post_id" => @target.id)
end end
topic.add_moderator_post( topic.add_moderator_post(

View File

@ -28,11 +28,14 @@ module DiscourseAssign
end end
def self.build_indirectly_assigned_to(post_assignments, topic) 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)) 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) 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
end.to_h end.to_h
end end

110
plugin.rb
View File

@ -73,8 +73,6 @@ after_initialize do
frequency_field = PendingAssignsReminder::REMINDERS_FREQUENCY frequency_field = PendingAssignsReminder::REMINDERS_FREQUENCY
register_editable_user_custom_field frequency_field register_editable_user_custom_field frequency_field
User.register_custom_field_type frequency_field, :integer 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 DiscoursePluginRegistry.serialized_current_user_fields << frequency_field
add_to_serializer(:user, :reminders_frequency) do add_to_serializer(:user, :reminders_frequency) do
RemindAssignsFrequencySiteSettings.values RemindAssignsFrequencySiteSettings.values
@ -87,7 +85,8 @@ after_initialize do
ON topics.id = a.topic_id AND a.assigned_to_id IS NOT NULL ON topics.id = a.topic_id AND a.assigned_to_id IS NOT NULL
SQL SQL
.where(<<~SQL, group_id: object.id) .where(<<~SQL, group_id: object.id)
( a.active AND
((
a.assigned_to_type = 'User' AND a.assigned_to_id IN ( a.assigned_to_type = 'User' AND a.assigned_to_id IN (
SELECT group_users.user_id SELECT group_users.user_id
FROM group_users FROM group_users
@ -95,7 +94,7 @@ after_initialize do
) )
) OR ( ) OR (
a.assigned_to_type = 'Group' AND a.assigned_to_id = :group_id a.assigned_to_type = 'Group' AND a.assigned_to_id = :group_id
) ))
SQL SQL
.where("topics.deleted_at IS NULL") .where("topics.deleted_at IS NULL")
.count .count
@ -213,7 +212,7 @@ after_initialize do
allowed_access = SiteSetting.assigns_public || can_assign allowed_access = SiteSetting.assigns_public || can_assign
if allowed_access && topics.length > 0 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) assignments_map = assignments.group_by(&:topic_id)
user_ids = assignments.filter { |assignment| assignment.assigned_to_user? }.map(&:assigned_to_id) user_ids = assignments.filter { |assignment| assignment.assigned_to_user? }.map(&:assigned_to_id)
@ -228,8 +227,9 @@ after_initialize do
indirectly_assigned_to = {} indirectly_assigned_to = {}
assignments&.each do |assignment| assignments&.each do |assignment|
next if assignment.target_type == "Topic" 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 if !assignment.target
next indirectly_assigned_to[assignment.target_id] = groups_map[assignment.assigned_to_id] if assignment&.assigned_to_group? 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 end&.compact&.uniq
assigned_to = assigned_to =
@ -284,13 +284,13 @@ after_initialize do
if name == "nobody" if name == "nobody"
next results 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") .where("a.assigned_to_id IS NULL")
end end
if name == "*" if name == "*"
next results 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") .where("a.assigned_to_id IS NOT NULL")
end end
@ -299,7 +299,7 @@ after_initialize do
if user_id if user_id
next results 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) .where("a.assigned_to_id = ? AND a.assigned_to_type = 'User'", user_id)
end end
@ -307,7 +307,7 @@ after_initialize do
if group_id if group_id
next results 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) .where("a.assigned_to_id = ? AND a.assigned_to_type = 'Group'", group_id)
end end
@ -321,7 +321,7 @@ after_initialize do
SELECT topic_id FROM assignments SELECT topic_id FROM assignments
LEFT JOIN group_users ON group_users.user_id = :user_id LEFT JOIN group_users ON group_users.user_id = :user_id
WHERE WHERE
assigned_to_id = :user_id AND assigned_to_type = 'User' assigned_to_id = :user_id AND assigned_to_type = 'User' AND active
SQL SQL
if @options[:filter] != :direct if @options[:filter] != :direct
@ -345,6 +345,7 @@ after_initialize do
WHERE ( WHERE (
assigned_to_id = :group_id AND assigned_to_type = 'Group' assigned_to_id = :group_id AND assigned_to_type = 'Group'
) )
AND active
SQL SQL
if @options[:filter] != :direct if @options[:filter] != :direct
@ -375,8 +376,9 @@ after_initialize do
list = list.where(<<~SQL, user_id: user.id, group_ids: group_ids) list = list.where(<<~SQL, user_id: user.id, group_ids: group_ids)
topics.id IN ( topics.id IN (
SELECT topic_id FROM assignments WHERE SELECT topic_id FROM assignments WHERE
(assigned_to_id = :user_id AND assigned_to_type = 'User') OR active AND
(assigned_to_id IN (:group_ids) AND assigned_to_type = 'Group') ((assigned_to_id = :user_id AND assigned_to_type = 'User') OR
(assigned_to_id IN (:group_ids) AND assigned_to_type = 'Group'))
) )
SQL SQL
end end
@ -423,13 +425,13 @@ after_initialize do
# Topic # Topic
add_to_class(:topic, :assigned_to) do add_to_class(:topic, :assigned_to) do
return @assigned_to if defined?(@assigned_to) return @assigned_to if defined?(@assigned_to)
@assigned_to = assignment&.assigned_to @assigned_to = assignment.assigned_to if assignment&.active
end end
add_to_class(:topic, :indirectly_assigned_to) do add_to_class(:topic, :indirectly_assigned_to) do
return @indirectly_assigned_to if defined?(@indirectly_assigned_to) return @indirectly_assigned_to if defined?(@indirectly_assigned_to)
@indirectly_assigned_to = Assignment.where(topic_id: id, target_type: "Post").inject({}) do |acc, assignment| @indirectly_assigned_to = Assignment.where(topic_id: id, target_type: "Post", active: true).includes(:target).inject({}) do |acc, assignment|
acc[assignment.target.post_number] = assignment.assigned_to if assignment.target acc[assignment.target_id] = { assigned_to: assignment.assigned_to, post_number: assignment.target.post_number } if assignment.target
acc acc
end end
end end
@ -619,7 +621,7 @@ after_initialize do
end end
add_to_serializer(:post, 'include_assigned_to_user?') do 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 end
add_to_serializer(:post, :assigned_to_group, false) do add_to_serializer(:post, :assigned_to_group, false) do
@ -627,7 +629,7 @@ after_initialize do
end end
add_to_serializer(:post, 'include_assigned_to_group?') do 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 end
# CurrentUser serializer # CurrentUser serializer
@ -699,7 +701,38 @@ after_initialize do
on(:topic_status_updated) do |topic, status, enabled| on(:topic_status_updated) do |topic, status, enabled|
if SiteSetting.unassign_on_close && (status == 'closed' || status == 'autoclosed') && enabled if SiteSetting.unassign_on_close && (status == 'closed' || status == 'autoclosed') && enabled
assigner = ::Assigner.new(topic, Discourse.system_user) 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
end end
@ -711,21 +744,17 @@ after_initialize do
next if !SiteSetting.unassign_on_group_archive next if !SiteSetting.unassign_on_group_archive
next if !info[:group] next if !info[:group]
previous_assigned_to_id = topic.custom_fields["prev_assigned_to_id"]&.to_i Assignment.where(topic_id: topic.id, active: false).find_each do |assignment|
next if !previous_assigned_to_id next unless assignment.target
assignment.update!(active: true)
assigned_type = topic.custom_fields["prev_assigned_to_type"] Jobs.enqueue(:assign_notification,
assigned_class = assigned_type == "Group" ? Group : User topic_id: topic.id,
previous_assigned_to = assigned_class.find_by(id: previous_assigned_to_id) post_id: assignment.target_type.is_a?(Topic) ? topic.first_post.id : assignment.target.id,
assigned_to_id: assignment.assigned_to_id,
if previous_assigned_to assigned_to_type: assignment.assigned_to_type,
assigner = Assigner.new(topic, Discourse.system_user) assigned_by_id: assignment.assigned_by_user_id,
assigner.assign(previous_assigned_to, silent: true) silent: true)
end end
topic.custom_fields.delete("prev_assigned_to_id")
topic.custom_fields.delete("prev_assigned_to_type")
topic.save!
end end
on(:archive_message) do |info| on(:archive_message) do |info|
@ -737,12 +766,13 @@ after_initialize do
next if !SiteSetting.unassign_on_group_archive next if !SiteSetting.unassign_on_group_archive
next if !info[:group] next if !info[:group]
topic.custom_fields["prev_assigned_to_id"] = topic.assignment.assigned_to_id Assignment.where(topic_id: topic.id, active: true).find_each do |assignment|
topic.custom_fields["prev_assigned_to_type"] = topic.assignment.assigned_to_type assignment.update!(active: false)
topic.save! Jobs.enqueue(:unassign_notification,
topic_id: topic.id,
assigner = Assigner.new(topic, Discourse.system_user) assigned_to_id: assignment.assigned_to.id,
assigner.unassign(silent: true) assigned_to_type: assignment.assigned_to_type)
end
end end
on(:user_removed_from_group) do |user, group| on(:user_removed_from_group) do |user, group|

View File

@ -78,14 +78,11 @@ describe 'integration tests' do
assigner.assign(user) assigner.assign(user)
GroupArchivedMessage.archive!(group.id, pm.reload) GroupArchivedMessage.archive!(group.id, pm.reload)
expect(pm.assignment).to eq(nil) expect(pm.assignment.active).to be false
expect(pm.custom_fields["prev_assigned_to_id"]).to eq(user.id)
expect(pm.custom_fields["prev_assigned_to_type"]).to eq("User")
GroupArchivedMessage.move_to_inbox!(group.id, pm.reload) 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.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 end
it "unassign and assign group if unassign_on_group_archive" do it "unassign and assign group if unassign_on_group_archive" do
@ -94,11 +91,10 @@ describe 'integration tests' do
assigner.assign(group) assigner.assign(group)
GroupArchivedMessage.archive!(group.id, pm.reload) GroupArchivedMessage.archive!(group.id, pm.reload)
expect(pm.assignment).to eq(nil) expect(pm.assignment.active).to be false
expect(pm.custom_fields["prev_assigned_to_id"]).to eq(group.id)
expect(pm.custom_fields["prev_assigned_to_type"]).to eq("Group")
GroupArchivedMessage.move_to_inbox!(group.id, pm.reload) GroupArchivedMessage.move_to_inbox!(group.id, pm.reload)
expect(pm.assignment.active).to be true
expect(pm.assignment.assigned_to).to eq(group) expect(pm.assignment.assigned_to).to eq(group)
end end
end end

View File

@ -25,8 +25,8 @@ RSpec.describe Assigner do
let(:secure_category) { Fabricate(:private_category, group: Fabricate(:group)) } let(:secure_category) { Fabricate(:private_category, group: Fabricate(:group)) }
let(:secure_topic) { Fabricate(:post).topic.tap { |t| t.update(category: secure_category) } } let(:secure_topic) { Fabricate(:post).topic.tap { |t| t.update(category: secure_category) } }
let(:moderator) { Fabricate(:moderator, groups: [assign_allowed_group]) } 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]) }
let(:assigner) { described_class.new(topic, moderator2) } let(:assigner) { described_class.new(topic, moderator_2) }
let(:assigner_self) { described_class.new(topic, moderator) } let(:assigner_self) { described_class.new(topic, moderator) }
it "can assign and unassign correctly" do it "can assign and unassign correctly" do
@ -127,7 +127,7 @@ RSpec.describe Assigner do
another_post = Fabricate.build(:post) another_post = Fabricate.build(:post)
assigner.assign(moderator) 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[:success]).to eq(false)
expect(second_assign[:reason]).to eq(:too_many_assigns) expect(second_assign[:reason]).to eq(:too_many_assigns)
@ -150,7 +150,7 @@ RSpec.describe Assigner do
first_assign = assigner.assign(moderator) first_assign = assigner.assign(moderator)
# reached limit so stop # 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 # self assign has a bypass
third_assign = described_class.new(another_post.topic, moderator).assign(moderator) third_assign = described_class.new(another_post.topic, moderator).assign(moderator)
@ -274,11 +274,12 @@ RSpec.describe Assigner do
let(:post) { Fabricate(:post) } let(:post) { Fabricate(:post) }
let(:topic) { post.topic } let(:topic) { post.topic }
let(:moderator) { Fabricate(:moderator, groups: [assign_allowed_group]) } let(:moderator) { Fabricate(:moderator, groups: [assign_allowed_group]) }
context 'topic' do
let(:assigner) { described_class.new(topic, moderator) } let(:assigner) { described_class.new(topic, moderator) }
before do before do
SiteSetting.unassign_on_close = true SiteSetting.unassign_on_close = true
assigner.assign(moderator) assigner.assign(moderator)
end end
@ -303,11 +304,101 @@ RSpec.describe Assigner do
end end
end end
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
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 '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
context "assign_emailer" do context "assign_emailer" do
let(:post) { Fabricate(:post) } let(:post) { Fabricate(:post) }
let(:topic) { post.topic } let(:topic) { post.topic }
let(:moderator) { Fabricate(:moderator, groups: [assign_allowed_group]) } 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 it "send an email if set to 'always'" do
SiteSetting.assign_mailer = AssignMailer.levels[:always] 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 it "doesn't send an email if the assigner and assignee are not different" do
SiteSetting.assign_mailer = AssignMailer.levels[:different_users] 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) .to change { ActionMailer::Base.deliveries.size }.by(1)
end end
@ -340,7 +431,7 @@ RSpec.describe Assigner do
it "doesn't send an email" do it "doesn't send an email" do
SiteSetting.assign_mailer = AssignMailer.levels[:never] 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) .to change { ActionMailer::Base.deliveries.size }.by(0)
end end
end end

View File

@ -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

View File

@ -22,8 +22,11 @@ function assignCurrentUserToTopic(needs) {
}; };
topic["indirectly_assigned_to"] = { topic["indirectly_assigned_to"] = {
2: { 2: {
assigned_to: {
name: "Developers", name: "Developers",
}, },
post_number: 2,
},
}; };
return helper.response(topic); return helper.response(topic);
}); });
@ -50,8 +53,11 @@ function assignNewUserToTopic(needs) {
}; };
topic["indirectly_assigned_to"] = { topic["indirectly_assigned_to"] = {
2: { 2: {
assigned_to: {
name: "Developers", name: "Developers",
}, },
post_number: 2,
},
}; };
return helper.response(topic); return helper.response(topic);
}); });