FEATURE: Assignment target is polymorphic (#218)

Change `topic_id` to polymorphic approach (In the next step we will allow assigning to individual post)

`topic_id` column is still used for efficient display of assigned users on topic list (to avoid scanning posts)
This commit is contained in:
Krzysztof Kotlarek 2021-10-14 09:22:29 +11:00 committed by GitHub
parent 247c74ecfd
commit dc8f43fbb1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
28 changed files with 264 additions and 170 deletions

View File

@ -31,27 +31,34 @@ module DiscourseAssign
end
def unassign
topic_id = params.require(:topic_id)
topic = Topic.find(topic_id.to_i)
assigner = TopicAssigner.new(topic, current_user)
target_id = params.require(:target_id)
target_type = params.require(:target_type)
raise Discourse::NotFound if !Assignment.valid_type?(target_type)
target = target_type.constantize.where(id: target_id).first
raise Discourse::NotFound unless target
assigner = Assigner.new(target, current_user)
assigner.unassign
render json: success_json
end
def assign
topic_id = params.require(:topic_id)
target_id = params.require(:target_id)
target_type = params.require(:target_type)
username = params.permit(:username)['username']
group_name = params.permit(:group_name)['group_name']
topic = Topic.find(topic_id.to_i)
assign_to = username.present? ? User.find_by(username_lower: username.downcase) : Group.where("LOWER(name) = ?", group_name.downcase).first
raise Discourse::NotFound unless assign_to
raise Discourse::NotFound if !Assignment.valid_type?(target_type)
target = target_type.constantize.where(id: target_id).first
raise Discourse::NotFound unless target
# perhaps?
#Scheduler::Defer.later "assign topic" do
assign = TopicAssigner.new(topic, current_user).assign(assign_to)
assign = Assigner.new(target, current_user).assign(assign_to)
if assign[:success]
render json: success_json
@ -69,17 +76,17 @@ module DiscourseAssign
topics = Topic
.includes(:tags)
.includes(:user)
.joins("JOIN assignments a ON a.topic_id = topics.id AND a.assigned_to_id IS NOT NULL")
.joins("JOIN assignments a ON a.target_id = topics.id AND a.target_type = 'Topic' AND a.assigned_to_id IS NOT NULL")
.order("a.assigned_to_id, topics.bumped_at desc")
.offset(offset)
.limit(limit)
Topic.preload_custom_fields(topics, TopicList.preloaded_custom_fields)
assignments = Assignment.where(topic: topics).pluck(:topic_id, :assigned_to_id).to_h
topic_assignments = Assignment.where(target_id: topics.map(&:id), target_type: 'Topic').pluck(:target_id, :assigned_to_id).to_h
users = User
.where("users.id IN (?)", assignments.values.uniq)
.where("users.id IN (?)", topic_assignments.values.uniq)
.joins("join user_emails on user_emails.user_id = users.id AND user_emails.primary")
.select(UserLookup.lookup_columns)
.to_a
@ -89,7 +96,7 @@ module DiscourseAssign
users_map = users.index_by(&:id)
topics.each do |topic|
user_id = assignments[topic.id]
user_id = topic_assignments[topic.id]
topic.preload_assigned_to(users_map[user_id]) if user_id
end
@ -111,7 +118,7 @@ module DiscourseAssign
members = User
.joins("LEFT OUTER JOIN group_users g ON g.user_id = users.id")
.joins("LEFT OUTER JOIN assignments a ON a.assigned_to_id = users.id AND a.assigned_to_type = 'User'")
.joins("LEFT OUTER JOIN topics t ON t.id = a.topic_id")
.joins("LEFT OUTER JOIN topics t ON t.id = a.target_id AND a.target_type = 'Topic'")
.where("g.group_id = ? AND users.id > 0 AND t.deleted_at IS NULL", group.id)
.where("a.assigned_to_id IS NOT NULL")
.order('COUNT(users.id) DESC')
@ -127,14 +134,14 @@ module DiscourseAssign
end
group_assignment_count = Topic
.joins("JOIN assignments a ON a.topic_id = topics.id")
.joins("JOIN assignments a ON a.target_id = topics.id AND a.target_type = 'Topic'")
.where(<<~SQL, group_id: group.id)
a.assigned_to_id = :group_id AND a.assigned_to_type = 'Group'
SQL
.count
assignment_count = Topic
.joins("JOIN assignments a ON a.topic_id = topics.id")
.joins("JOIN assignments a ON a.target_id = topics.id AND a.target_type = 'Topic'")
.joins("JOIN group_users ON group_users.user_id = a.assigned_to_id ")
.where("group_users.group_id = ?", group.id)
.where("a.assigned_to_type = 'User'")

View File

@ -1,9 +1,18 @@
# frozen_string_literal: true
class Assignment < ActiveRecord::Base
VALID_TYPES = %w(topic).freeze
belongs_to :topic
belongs_to :assigned_to, polymorphic: true
belongs_to :assigned_by_user, class_name: "User"
belongs_to :target, polymorphic: true
scope :joins_with_topics, -> { joins("INNER JOIN topics ON topics.id = assignments.target_id AND assignments.target_type = 'Topic' AND topics.deleted_at IS NULL") }
def self.valid_type?(type)
VALID_TYPES.include?(type.downcase)
end
def assigned_to_user?
assigned_to_type == 'User'

View File

@ -94,7 +94,8 @@ export default Controller.extend({
data: {
username: this.get("model.username"),
group_name: this.get("model.group_name"),
topic_id: this.get("model.topic.id"),
target_id: this.get("model.topic.id"),
target_type: "Topic",
},
})
.then(() => {

View File

@ -6,7 +6,10 @@ export default Service.extend({
unassign(topicId) {
return ajax("/assign/unassign", {
type: "PUT",
data: { topic_id: topicId },
data: {
target_id: topicId,
target_type: "Topic",
},
});
},
@ -25,7 +28,8 @@ export default Service.extend({
type: "PUT",
data: {
username: user.username,
topic_id: topic.id,
target_id: topic.id,
target_type: "Topic",
},
});
},

View File

@ -0,0 +1,24 @@
# frozen_string_literal: true
class AddTargetToAssignments < ActiveRecord::Migration[6.1]
def up
add_column :assignments, :target_id, :integer
add_column :assignments, :target_type, :string
execute <<~SQL
UPDATE assignments
SET target_type = 'Topic', target_id = topic_id
WHERE target_type IS NULL
SQL
change_column :assignments, :target_id, :integer, null: false
change_column :assignments, :target_type, :string, null: false
add_index :assignments, [:target_id, :target_type], unique: true
add_index :assignments, [:assigned_to_id, :assigned_to_type, :target_id, :target_type], unique: true, name: 'unique_target_and_assigned'
end
def down
remove_columns :assignments, :target_id, :target_type
end
end

View File

@ -4,15 +4,27 @@ class CorrectlyMoveAssignmentsFromCustomFieldsToATable < ActiveRecord::Migration
def up
# An old version of 20210709101534 incorrectly imported `assignments` with
# the topic_id and assigned_to_id columns flipped. This query deletes those invalid records.
execute <<~SQL
DELETE FROM assignments USING topic_custom_fields
WHERE
assignments.assigned_to_id = topic_custom_fields.topic_id
AND assignments.topic_id = topic_custom_fields.value::integer
AND topic_custom_fields.name = 'assigned_to_id'
SQL
if column_exists?(:assignments, :assigned_to_type)
if column_exists?(:assignments, :target_id)
execute <<~SQL
INSERT INTO assignments (assigned_to_id, assigned_by_user_id, topic_id, created_at, updated_at, assigned_to_type, target_id, target_type)
SELECT
assigned_to.value::integer,
assigned_by.value::integer,
assigned_by.topic_id,
assigned_by.created_at,
assigned_by.updated_at,
'User',
assigned_by.topic_id,
'Topic'
FROM topic_custom_fields assigned_by
INNER JOIN topic_custom_fields assigned_to ON assigned_to.topic_id = assigned_by.topic_id
WHERE assigned_by.name = 'assigned_by_id'
AND assigned_to.name = 'assigned_to_id'
ORDER BY assigned_by.created_at DESC
ON CONFLICT DO NOTHING
SQL
elsif column_exists?(:assignments, :assigned_to_type)
execute <<~SQL
INSERT INTO assignments (assigned_to_id, assigned_by_user_id, topic_id, created_at, updated_at, assigned_to_type)
SELECT
@ -21,7 +33,7 @@ class CorrectlyMoveAssignmentsFromCustomFieldsToATable < ActiveRecord::Migration
assigned_by.topic_id,
assigned_by.created_at,
assigned_by.updated_at,
'User'
'User',
FROM topic_custom_fields assigned_by
INNER JOIN topic_custom_fields assigned_to ON assigned_to.topic_id = assigned_by.topic_id
WHERE assigned_by.name = 'assigned_by_id'
@ -29,6 +41,7 @@ class CorrectlyMoveAssignmentsFromCustomFieldsToATable < ActiveRecord::Migration
ORDER BY assigned_by.created_at DESC
ON CONFLICT DO NOTHING
SQL
else
execute <<~SQL
INSERT INTO assignments (assigned_to_id, assigned_by_user_id, topic_id, created_at, updated_at)

View File

@ -16,7 +16,7 @@ module Jobs
assigned_to_users = args[:assigned_to_type] == "User" ? [assigned_to] : assigned_to.users
assigned_to_users.each do |user|
TopicAssigner.publish_topic_tracking_state(topic, user.id)
Assigner.publish_topic_tracking_state(topic, user.id)
next if assigned_by == user

View File

@ -11,7 +11,7 @@ module Jobs
assigned_to_users = args[:assigned_to_type] == "User" ? [User.find(args[:assigned_to_id])] : Group.find(args[:assigned_to_id]).users
assigned_to_users.each do |user|
TopicAssigner.publish_topic_tracking_state(topic, user.id)
Assigner.publish_topic_tracking_state(topic, user.id)
Notification.where(
notification_type: Notification.types[:custom],

View File

@ -36,7 +36,7 @@ module Jobs
AND user_frequency.name = '#{PendingAssignsReminder::REMINDERS_FREQUENCY}'
INNER JOIN group_users ON assignments.assigned_to_id = group_users.user_id
INNER JOIN topics ON topics.id = assignments.topic_id AND topics.deleted_at IS NULL
INNER JOIN topics ON topics.id = assignments.target_id AND assignments.target_type = 'Topic' AND topics.deleted_at IS NULL
WHERE group_users.group_id IN (#{allowed_group_ids})
AND #{frequency} > 0

View File

@ -3,7 +3,7 @@
require 'email/sender'
require 'nokogiri'
class ::TopicAssigner
class ::Assigner
def self.backfill_auto_assign
staff_mention = User
.assign_allowed
@ -15,7 +15,7 @@ class ::TopicAssigner
SELECT p.topic_id, MAX(post_number) post_number
FROM posts p
JOIN topics t ON t.id = p.topic_id
LEFT JOIN assignments a ON a.topic_id = p.topic_id
LEFT JOIN assignments a ON a.target_id = p.topic_id AND a.target_type = 'Topic'
WHERE p.user_id IN (SELECT id FROM users WHERE moderator OR admin)
AND (#{staff_mention})
AND a.assigned_to_id IS NULL
@ -107,6 +107,7 @@ class ::TopicAssigner
end
def self.publish_topic_tracking_state(topic, user_id)
# TODO decide later if we want general or separate method to publish about post tracking
if topic.private_message?
MessageBus.publish(
"/private-messages/assigned",
@ -116,9 +117,9 @@ class ::TopicAssigner
end
end
def initialize(topic, user)
def initialize(target, user)
@assigned_by = user
@topic = topic
@target = target
end
def allowed_user_ids
@ -134,7 +135,7 @@ class ::TopicAssigner
return true if @assigned_by.id == assign_to.id
assigned_total = Assignment
.joins(:topic)
.joins_with_topics
.where(topics: { deleted_at: nil })
.where(assigned_to_id: assign_to.id)
.count
@ -150,20 +151,44 @@ class ::TopicAssigner
end
end
def can_assignee_see_topic?(assignee)
Guardian.new(assignee).can_see_topic?(@topic)
def topic_target?
@topic_target ||= @target.is_a?(Topic)
end
def post_target?
@post_target ||= @target.is_a?(Post)
end
def can_assignee_see_target?(assignee)
return Guardian.new(assignee).can_see_topic?(@target) if topic_target?
return Guardian.new(assignee).can_see_post?(@target) if post_target?
raise Discourse::InvalidAccess
end
def topic
return @topic if @topic
@topic = @target if topic_target?
@topic = @target.topic if post_target?
raise Discourse::InvalidParameters if !@topic
@topic
end
def first_post
topic.posts.where(post_number: 1).first
end
def assign(assign_to, silent: false)
forbidden_reason =
case
when assign_to.is_a?(User) && !can_assignee_see_topic?(assign_to)
@topic.private_message? ? :forbidden_assignee_not_pm_participant : :forbidden_assignee_cant_see_topic
when assign_to.is_a?(Group) && assign_to.users.any? { |user| !can_assignee_see_topic?(user) }
@topic.private_message? ? :forbidden_group_assignee_not_pm_participant : :forbidden_group_assignee_cant_see_topic
when assign_to.is_a?(User) && !can_assignee_see_target?(assign_to)
topic.private_message? ? :forbidden_assignee_not_pm_participant : :forbidden_assignee_cant_see_topic
when assign_to.is_a?(Group) && assign_to.users.any? { |user| !can_assignee_see_target?(user) }
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
when topic.assignment&.assigned_to_id == assign_to.id
assign_to.is_a?(User) ? :already_assigned : :group_already_assigned
when !can_assign_to?(assign_to)
:too_many_assigns
@ -171,28 +196,29 @@ class ::TopicAssigner
return { success: false, reason: forbidden_reason } if forbidden_reason
@topic.assignment&.destroy!
@target.assignment&.destroy!
type = assign_to.is_a?(User) ? "User" : "Group"
assignment = @topic.create_assignment!(assigned_to_id: assign_to.id, assigned_to_type: type, assigned_by_user_id: @assigned_by.id)
assignment = @target.create_assignment!(assigned_to_id: assign_to.id, assigned_to_type: type, assigned_by_user_id: @assigned_by.id, topic_id: topic.id)
first_post = @topic.posts.find_by(post_number: 1)
first_post.publish_change_to_clients!(:revised, reload_topic: true)
serializer = assignment.assigned_to_user? ? BasicUserSerializer : BasicGroupSerializer
#TODO assign notification for post
Jobs.enqueue(:assign_notification,
topic_id: @topic.id,
topic_id: topic.id,
assigned_to_id: assign_to.id,
assigned_to_type: type,
assigned_by_id: @assigned_by.id,
silent: silent)
#TODO message bus for post assignment
MessageBus.publish(
"/staff/topic-assignment",
{
type: "assigned",
topic_id: @topic.id,
topic_id: topic.id,
assigned_type: type,
assigned_to: serializer.new(assign_to, scope: Guardian.new, root: false).as_json
},
@ -202,26 +228,27 @@ class ::TopicAssigner
if assignment.assigned_to_user?
if !TopicUser.exists?(
user_id: assign_to.id,
topic_id: @topic.id,
topic_id: topic.id,
notification_level: TopicUser.notification_levels[:watching]
)
TopicUser.change(
assign_to.id,
@topic.id,
topic.id,
notification_level: TopicUser.notification_levels[:watching],
notifications_reason_id: TopicUser.notification_reasons[:plugin_changed]
)
end
if SiteSetting.assign_mailer == AssignMailer.levels[:always] || (SiteSetting.assign_mailer == AssignMailer.levels[:different_users] && @assigned_by.id != assign_to.id)
if !@topic.muted?(assign_to)
message = AssignMailer.send_assignment(assign_to.email, @topic, @assigned_by)
if !topic.muted?(assign_to)
message = AssignMailer.send_assignment(assign_to.email, topic, @assigned_by)
Email::Sender.new(message, :assign_message).send
end
end
end
if !silent
@topic.add_moderator_post(
#TODO moderator post when assigned to post
topic.add_moderator_post(
@assigned_by,
nil,
bump: false,
@ -237,8 +264,8 @@ class ::TopicAssigner
type = :assigned
payload = {
type: type,
topic_id: @topic.id,
topic_title: @topic.title,
topic_id: topic.id,
topic_title: topic.title,
assigned_to_id: assign_to.id,
assigned_to_username: assign_to.username,
assigned_by_id: @assigned_by.id,
@ -262,30 +289,30 @@ class ::TopicAssigner
end
def unassign(silent: false)
if assignment = @topic.assignment
if assignment = @target.assignment
assignment.destroy!
post = @topic.posts.where(post_number: 1).first
return if post.blank?
return if first_post.blank?
post.publish_change_to_clients!(:revised, reload_topic: true)
first_post.publish_change_to_clients!(:revised, reload_topic: true)
#TODO unassign notification for post
Jobs.enqueue(:unassign_notification,
topic_id: @topic.id,
topic_id: topic.id,
assigned_to_id: assignment.assigned_to.id,
assigned_to_type: assignment.assigned_to_type)
if assignment.assigned_to_user?
if TopicUser.exists?(
user_id: assignment.assigned_to_id,
topic: @topic,
topic: topic,
notification_level: TopicUser.notification_levels[:watching],
notifications_reason_id: TopicUser.notification_reasons[:plugin_changed]
)
TopicUser.change(
assignment.assigned_to_id,
@topic.id,
topic.id,
notification_level: TopicUser.notification_levels[:tracking],
notifications_reason_id: TopicUser.notification_reasons[:plugin_changed]
)
@ -294,9 +321,10 @@ class ::TopicAssigner
assigned_to = assignment.assigned_to
# TODO unassign post for post assignment
if SiteSetting.unassign_creates_tracking_post && !silent
post_type = SiteSetting.assigns_public ? Post.types[:small_action] : Post.types[:whisper]
@topic.add_moderator_post(
topic.add_moderator_post(
@assigned_by, nil,
bump: false,
post_type: post_type,
@ -310,8 +338,8 @@ class ::TopicAssigner
type = :unassigned
payload = {
type: type,
topic_id: @topic.id,
topic_title: @topic.title,
topic_id: topic.id,
topic_title: topic.title,
unassigned_by_id: @assigned_by.id,
unassigned_by_username: @assigned_by.username
}
@ -333,7 +361,7 @@ class ::TopicAssigner
"/staff/topic-assignment",
{
type: 'unassigned',
topic_id: @topic.id,
topic_id: topic.id,
},
user_ids: allowed_user_ids
)

View File

@ -9,7 +9,7 @@ module DiscourseAssign
username: user.username,
name: user.name,
avatar_template: user.avatar_template,
assigned_at: Assignment.where(topic_id: topic.id).pluck_first(:created_at)
assigned_at: Assignment.where(target: topic).pluck_first(:created_at)
}
end
@ -22,7 +22,7 @@ module DiscourseAssign
flair_color: group.flair_color,
flair_icon: group.flair_icon,
flair_upload_id: group.flair_upload_id,
assigned_at: Assignment.where(topic_id: topic.id).pluck_first(:created_at)
assigned_at: Assignment.where(target: topic).pluck_first(:created_at)
}
end
end

View File

@ -28,7 +28,7 @@ class PendingAssignsReminder
private
def assigned_count_for(user)
Assignment.joins(:topic).where(assigned_to_id: user.id, assigned_to_type: 'User').count
Assignment.joins_with_topics.where(assigned_to_id: user.id, assigned_to_type: 'User').count
end
def assigned_topics(user, order:)

View File

@ -33,7 +33,7 @@ after_initialize do
require File.expand_path('../jobs/regular/remind_user.rb', __FILE__)
require File.expand_path('../jobs/regular/assign_notification.rb', __FILE__)
require File.expand_path('../jobs/regular/unassign_notification.rb', __FILE__)
require 'topic_assigner'
require 'assigner'
require 'pending_assigns_reminder'
# TODO: Drop when Discourse stable 2.8.0 is released
@ -48,7 +48,7 @@ after_initialize do
reloadable_patch do |plugin|
class ::Topic
has_one :assignment, dependent: :destroy
has_one :assignment, as: :target, dependent: :destroy
end
class ::Group
@ -171,13 +171,13 @@ after_initialize do
end
on(:assign_topic) do |topic, user, assigning_user, force|
if force || !Assignment.exists?(topic: topic)
TopicAssigner.new(topic, assigning_user).assign(user)
if force || !Assignment.exists?(target: topic)
Assigner.new(topic, assigning_user).assign(user)
end
end
on(:unassign_topic) do |topic, unassigning_user|
TopicAssigner.new(topic, unassigning_user).unassign
Assigner.new(topic, unassigning_user).unassign
end
Site.preloaded_category_custom_fields << "enable_unassigned_filter"
@ -185,7 +185,7 @@ after_initialize do
BookmarkQuery.on_preload do |bookmarks, bookmark_query|
if SiteSetting.assign_enabled?
topics = bookmarks.map(&:topic)
assignments = Assignment.strict_loading.where(topic: topics).includes(:assigned_to).index_by(&:topic_id)
assignments = Assignment.strict_loading.where(topic_id: topics).includes(:assigned_to).index_by(&:topic_id)
topics.each do |topic|
assigned_to = assignments[topic.id]&.assigned_to
@ -226,7 +226,7 @@ after_initialize do
if allowed_access && results.posts.length > 0
topics = results.posts.map(&:topic)
assignments = Assignment.strict_loading.where(topic: topics).includes(:assigned_to).index_by(&:topic_id)
assignments = Assignment.strict_loading.where(target: topics).includes(:assigned_to).index_by(&:topic_id)
results.posts.each do |post|
assigned_to = assignments[post.topic.id]&.assigned_to
@ -480,17 +480,17 @@ after_initialize do
TopicsBulkAction.register_operation("assign") do
if @user.can_assign?
assign_user = User.find_by_username(@operation[:username])
topics.each do |t|
TopicAssigner.new(t, @user).assign(assign_user)
topics.each do |topic|
Assigner.new(topic, @user).assign(assign_user)
end
end
end
TopicsBulkAction.register_operation("unassign") do
if @user.can_assign?
topics.each do |t|
topics.each do |topic|
if guardian.can_assign?
TopicAssigner.new(t, @user).unassign
Assigner.new(topic, @user).unassign
end
end
end
@ -574,16 +574,16 @@ after_initialize do
# Event listeners
on(:post_created) do |post|
::TopicAssigner.auto_assign(post, force: true)
::Assigner.auto_assign(post, force: true)
end
on(:post_edited) do |post, topic_changed|
::TopicAssigner.auto_assign(post, force: true)
::Assigner.auto_assign(post, force: true)
end
on(:topic_status_updated) do |topic, status, enabled|
if SiteSetting.unassign_on_close && (status == 'closed' || status == 'autoclosed') && enabled
assigner = ::TopicAssigner.new(topic, Discourse.system_user)
assigner = ::Assigner.new(topic, Discourse.system_user)
assigner.unassign(silent: true)
end
end
@ -604,7 +604,7 @@ after_initialize do
previous_assigned_to = assigned_class.find_by(id: previous_assigned_to_id)
if previous_assigned_to
assigner = TopicAssigner.new(topic, Discourse.system_user)
assigner = Assigner.new(topic, Discourse.system_user)
assigner.assign(previous_assigned_to, silent: true)
end
@ -626,7 +626,7 @@ after_initialize do
topic.custom_fields["prev_assigned_to_type"] = topic.assignment.assigned_to_type
topic.save!
assigner = TopicAssigner.new(topic, Discourse.system_user)
assigner = Assigner.new(topic, Discourse.system_user)
assigner.unassign(silent: true)
end
@ -640,7 +640,7 @@ after_initialize do
topics = Topic.joins(:assignment).where('assignments.assigned_to_id = ?', user.id)
topics.each do |topic|
TopicAssigner.new(topic, Discourse.system_user).unassign
Assigner.new(topic, Discourse.system_user).unassign
end
end
end
@ -785,7 +785,7 @@ after_initialize do
end
assign_to = User.find_by(id: assign_to_user_id)
assign_to && TopicAssigner.new(topic, Discourse.system_user).assign(assign_to)
assign_to && Assigner.new(topic, Discourse.system_user).assign(assign_to)
end
end
end

View File

@ -23,15 +23,15 @@ describe Search do
add_to_assign_allowed_group(user)
add_to_assign_allowed_group(user2)
TopicAssigner.new(post1.topic, user).assign(user)
TopicAssigner.new(post2.topic, user).assign(user2)
TopicAssigner.new(post3.topic, user).assign(user)
Assigner.new(post1.topic, user).assign(user)
Assigner.new(post2.topic, user).assign(user2)
Assigner.new(post3.topic, user).assign(user)
end
it 'can find by status' do
expect(Search.execute('in:assigned', guardian: Guardian.new(user)).posts.length).to eq(3)
TopicAssigner.new(post3.topic, user).unassign
Assigner.new(post3.topic, user).unassign
expect(Search.execute('in:unassigned', guardian: Guardian.new(user)).posts.length).to eq(1)
expect(Search.execute("assigned:#{user.username}", guardian: Guardian.new(user)).posts.length).to eq(1)

View File

@ -173,8 +173,8 @@ describe TopicQuery do
assigned_topic = Fabricate(:post).topic
assigned_topic2 = Fabricate(:post).topic
TopicAssigner.new(assigned_topic, user).assign(user)
TopicAssigner.new(assigned_topic2, user2).assign(user2)
Assigner.new(assigned_topic, user).assign(user)
Assigner.new(assigned_topic2, user2).assign(user2)
query = TopicQuery.new(user, assigned: user.username).list_latest
expect(query.topics.length).to eq(1)
@ -185,8 +185,8 @@ describe TopicQuery do
assigned_topic = Fabricate(:post).topic
assigned_topic2 = Fabricate(:post).topic
TopicAssigner.new(assigned_topic, user).assign(user)
TopicAssigner.new(assigned_topic2, user2).assign(user2)
Assigner.new(assigned_topic, user).assign(user)
Assigner.new(assigned_topic2, user2).assign(user2)
query = TopicQuery.new(user2, assigned: "me").list_latest
expect(query.topics.length).to eq(1)
@ -197,7 +197,7 @@ describe TopicQuery do
assigned_topic = Fabricate(:post).topic
unassigned_topic = Fabricate(:topic)
TopicAssigner.new(assigned_topic, user).assign(user)
Assigner.new(assigned_topic, user).assign(user)
query = TopicQuery.new(user, assigned: "nobody").list_latest
expect(query.topics.length).to eq(1)
@ -208,7 +208,7 @@ describe TopicQuery do
assigned_topic = Fabricate(:post).topic
Fabricate(:topic)
TopicAssigner.new(assigned_topic, user).assign(user)
Assigner.new(assigned_topic, user).assign(user)
query = TopicQuery.new(user, assigned: "*").list_latest
expect(query.topics.length).to eq(1)
@ -218,7 +218,7 @@ describe TopicQuery do
def assign_to(topic, user, assignee)
topic.tap do |t|
TopicAssigner.new(t, user).assign(assignee)
Assigner.new(t, user).assign(assignee)
end
end
end

View File

@ -15,6 +15,8 @@ describe CorrectlyMoveAssignmentsFromCustomFieldsToATable do
expect(assignment.topic_id).to eq(99)
expect(assignment.assigned_to_id).to eq(50)
expect(assignment.assigned_by_user_id).to eq(60)
expect(assignment.target_id).to eq(99)
expect(assignment.target_type).to eq('Topic')
end
end

View File

@ -47,7 +47,7 @@ describe 'integration tests' do
end
it 'publishes the right message on archive and move to inbox' do
assigner = TopicAssigner.new(pm, user)
assigner = Assigner.new(pm, user)
assigner.assign(user)
assert_publish_topic_state(pm, user: user) do
@ -60,7 +60,7 @@ describe 'integration tests' do
end
it 'publishes the right message on archive and move to inbox for groups' do
assigner = TopicAssigner.new(pm, user)
assigner = Assigner.new(pm, user)
assigner.assign(group)
assert_publish_topic_state(pm, group: group) do
@ -74,7 +74,7 @@ describe 'integration tests' do
it "unassign and assign user if unassign_on_group_archive" do
SiteSetting.unassign_on_group_archive = true
assigner = TopicAssigner.new(pm, user)
assigner = Assigner.new(pm, user)
assigner.assign(user)
GroupArchivedMessage.archive!(group.id, pm.reload)
@ -90,7 +90,7 @@ describe 'integration tests' do
it "unassign and assign group if unassign_on_group_archive" do
SiteSetting.unassign_on_group_archive = true
assigner = TopicAssigner.new(pm, user)
assigner = Assigner.new(pm, user)
assigner.assign(group)
GroupArchivedMessage.archive!(group.id, pm.reload)

View File

@ -92,7 +92,7 @@ RSpec.describe Jobs::EnqueueReminders do
def assign_one_task_to(user, assigned_on: 3.months.ago, post: Fabricate(:post))
freeze_time(assigned_on) do
TopicAssigner.new(post.topic, user).assign(user)
Assigner.new(post.topic, user).assign(user)
end
end

View File

@ -2,7 +2,7 @@
require 'rails_helper'
RSpec.describe TopicAssigner do
RSpec.describe Assigner do
before { SiteSetting.assign_enabled = true }
let(:assign_allowed_group) { Group.find_by(name: 'staff') }
@ -26,8 +26,8 @@ RSpec.describe TopicAssigner do
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) { TopicAssigner.new(topic, moderator2) }
let(:assigner_self) { TopicAssigner.new(topic, moderator) }
let(:assigner) { described_class.new(topic, moderator2) }
let(:assigner_self) { described_class.new(topic, moderator) }
it "can assign and unassign correctly" do
expect_enqueued_with(job: :assign_notification) do
@ -91,14 +91,14 @@ RSpec.describe TopicAssigner do
end
it "doesn't assign system user" do
TopicAssigner.auto_assign(post)
described_class.auto_assign(post)
expect(topic.assignment).to eq(nil)
end
it "assigns first mentioned staff user after system user" do
post.update(raw: "Don't assign @system. @modi, can you add this to your list?")
TopicAssigner.auto_assign(post)
described_class.auto_assign(post)
expect(topic.assignment.assigned_to_id).to eq(moderator.id)
end
@ -127,7 +127,7 @@ RSpec.describe TopicAssigner do
another_post = Fabricate.build(:post)
assigner.assign(moderator)
second_assign = TopicAssigner.new(another_post.topic, moderator2).assign(moderator)
second_assign = described_class.new(another_post.topic, moderator2).assign(moderator)
expect(second_assign[:success]).to eq(false)
expect(second_assign[:reason]).to eq(:too_many_assigns)
@ -138,7 +138,7 @@ RSpec.describe TopicAssigner do
another_post = Fabricate(:post)
assigner.assign(moderator)
second_assign = TopicAssigner.new(another_post.topic, moderator).assign(moderator)
second_assign = described_class.new(another_post.topic, moderator).assign(moderator)
expect(second_assign[:success]).to eq(true)
end
@ -150,10 +150,10 @@ RSpec.describe TopicAssigner do
first_assign = assigner.assign(moderator)
# reached limit so stop
second_assign = TopicAssigner.new(Fabricate(:topic), moderator2).assign(moderator)
second_assign = described_class.new(Fabricate(:topic), moderator2).assign(moderator)
# self assign has a bypass
third_assign = TopicAssigner.new(another_post.topic, moderator).assign(moderator)
third_assign = described_class.new(another_post.topic, moderator).assign(moderator)
expect(first_assign[:success]).to eq(true)
expect(second_assign[:success]).to eq(false)
@ -163,28 +163,28 @@ RSpec.describe TopicAssigner do
fab!(:admin) { Fabricate(:admin) }
it 'fails to assign when the assigned user cannot view the pm' do
assign = TopicAssigner.new(pm, admin).assign(moderator)
assign = described_class.new(pm, admin).assign(moderator)
expect(assign[:success]).to eq(false)
expect(assign[:reason]).to eq(:forbidden_assignee_not_pm_participant)
end
it 'fails to assign when not all group members has access to pm' do
assign = TopicAssigner.new(pm, admin).assign(moderator.groups.first)
assign = described_class.new(pm, admin).assign(moderator.groups.first)
expect(assign[:success]).to eq(false)
expect(assign[:reason]).to eq(:forbidden_group_assignee_not_pm_participant)
end
it 'fails to assign when the assigned user cannot view the topic' do
assign = TopicAssigner.new(secure_topic, admin).assign(moderator)
assign = described_class.new(secure_topic, admin).assign(moderator)
expect(assign[:success]).to eq(false)
expect(assign[:reason]).to eq(:forbidden_assignee_cant_see_topic)
end
it 'fails to assign when the not all group members can view the topic' do
assign = TopicAssigner.new(secure_topic, admin).assign(moderator.groups.first)
assign = described_class.new(secure_topic, admin).assign(moderator.groups.first)
expect(assign[:success]).to eq(false)
expect(assign[:reason]).to eq(:forbidden_group_assignee_cant_see_topic)
@ -193,7 +193,7 @@ RSpec.describe TopicAssigner do
it "assigns the PM to the moderator when it's included in the list of allowed users" do
pm.allowed_users << moderator
assign = TopicAssigner.new(pm, admin).assign(moderator)
assign = described_class.new(pm, admin).assign(moderator)
expect(assign[:success]).to eq(true)
end
@ -201,10 +201,16 @@ RSpec.describe TopicAssigner do
it "assigns the PM to the moderator when it's a member of an allowed group" do
pm.allowed_groups << assign_allowed_group
assign = TopicAssigner.new(pm, admin).assign(moderator)
assign = described_class.new(pm, admin).assign(moderator)
expect(assign[:success]).to eq(true)
end
it 'triggers error for incorrect type' do
expect do
described_class.new(secure_category, moderator).assign(moderator)
end.to raise_error(Discourse::InvalidAccess)
end
end
context "assign_self_regex" do
@ -218,7 +224,7 @@ RSpec.describe TopicAssigner do
end
it "automatically assigns to myself" do
expect(TopicAssigner.auto_assign(reply)).to eq(success: true)
expect(described_class.auto_assign(reply)).to eq(success: true)
expect(op.topic.assignment.assigned_to_id).to eq(me.id)
expect(op.topic.assignment.assigned_by_user_id).to eq(me.id)
end
@ -242,7 +248,7 @@ RSpec.describe TopicAssigner do
MD
another_reply = Fabricate(:post, topic: op.topic, user: admin, raw: raw)
expect(TopicAssigner.auto_assign(another_reply)).to eq(nil)
expect(described_class.auto_assign(another_reply)).to eq(nil)
end
end
@ -258,7 +264,7 @@ RSpec.describe TopicAssigner do
end
it "automatically assigns to other" do
expect(TopicAssigner.auto_assign(reply)).to eq(success: true)
expect(described_class.auto_assign(reply)).to eq(success: true)
expect(op.topic.assignment.assigned_to_id).to eq(other.id)
expect(op.topic.assignment.assigned_by_user_id).to eq(me.id)
end
@ -268,7 +274,7 @@ RSpec.describe TopicAssigner do
let(:post) { Fabricate(:post) }
let(:topic) { post.topic }
let(:moderator) { Fabricate(:moderator, groups: [assign_allowed_group]) }
let(:assigner) { TopicAssigner.new(topic, moderator) }
let(:assigner) { described_class.new(topic, moderator) }
before do
SiteSetting.unassign_on_close = true
@ -306,35 +312,35 @@ RSpec.describe TopicAssigner do
it "send an email if set to 'always'" do
SiteSetting.assign_mailer = AssignMailer.levels[:always]
expect { TopicAssigner.new(topic, moderator).assign(moderator) }
expect { described_class.new(topic, moderator).assign(moderator) }
.to change { ActionMailer::Base.deliveries.size }.by(1)
end
it "doesn't send an email if assignee is a group" do
SiteSetting.assign_mailer = AssignMailer.levels[:always]
expect { TopicAssigner.new(topic, moderator).assign(assign_allowed_group) }
expect { described_class.new(topic, moderator).assign(assign_allowed_group) }
.to change { ActionMailer::Base.deliveries.size }.by(0)
end
it "doesn't send an email if the assigner and assignee are not different" do
SiteSetting.assign_mailer = AssignMailer.levels[:different_users]
expect { TopicAssigner.new(topic, moderator).assign(moderator2) }
expect { described_class.new(topic, moderator).assign(moderator2) }
.to change { ActionMailer::Base.deliveries.size }.by(1)
end
it "doesn't send an email if the assigner and assignee are not different" do
SiteSetting.assign_mailer = AssignMailer.levels[:different_users]
expect { TopicAssigner.new(topic, moderator).assign(moderator) }
expect { described_class.new(topic, moderator).assign(moderator) }
.to change { ActionMailer::Base.deliveries.size }.by(0)
end
it "doesn't send an email" do
SiteSetting.assign_mailer = AssignMailer.levels[:never]
expect { TopicAssigner.new(topic, moderator).assign(moderator2) }
expect { described_class.new(topic, moderator).assign(moderator2) }
.to change { ActionMailer::Base.deliveries.size }.by(0)
end
end

View File

@ -18,7 +18,7 @@ RSpec.describe PendingAssignsReminder do
it 'does not create a reminder if the user only has one task' do
post = Fabricate(:post)
TopicAssigner.new(post.topic, user).assign(user)
Assigner.new(post.topic, user).assign(user)
assert_reminder_not_created
end
@ -38,10 +38,10 @@ RSpec.describe PendingAssignsReminder do
@post2.topic.update_column(:fancy_title, nil)
@post3 = Fabricate(:post)
@post4 = Fabricate(:post)
TopicAssigner.new(@post1.topic, user).assign(user)
TopicAssigner.new(@post2.topic, user).assign(user)
TopicAssigner.new(@post3.topic, user).assign(user)
TopicAssigner.new(@post4.topic, user).assign(user)
Assigner.new(@post1.topic, user).assign(user)
Assigner.new(@post2.topic, user).assign(user)
Assigner.new(@post3.topic, user).assign(user)
Assigner.new(@post4.topic, user).assign(user)
@post3.topic.trash!
@post4.topic.update(category: secure_category)
end

View File

@ -14,13 +14,13 @@ describe Topic do
describe "#assigned_to" do
it "correctly points to a user" do
Assignment.create!(topic: topic, assigned_by_user: user1, assigned_to: user2)
Assignment.create!(target_id: topic.id, target_type: "Topic", topic_id: topic.id, assigned_by_user: user1, assigned_to: user2)
expect(topic.reload.assigned_to).to eq(user2)
end
it "correctly points to a group" do
Assignment.create!(topic: topic, assigned_by_user: user1, assigned_to: group)
Assignment.create!(target_id: topic.id, target_type: "Topic", topic_id: topic.id, assigned_by_user: user1, assigned_to: group)
expect(topic.reload.assigned_to).to eq(group)
end

View File

@ -17,7 +17,7 @@ describe 'plugin' do
it 'unassigns the user' do
SiteSetting.assign_allowed_on_groups = @group_a.id.to_s
TopicAssigner.new(@topic, Discourse.system_user).assign(@user)
Assigner.new(@topic, Discourse.system_user).assign(@user)
@group_a.remove(@user)
expect(Assignment.count).to eq(0)
@ -28,7 +28,7 @@ describe 'plugin' do
@group_b.add(@user)
SiteSetting.assign_allowed_on_groups = [@group_a.id.to_s, @group_b.id.to_s].join('|')
TopicAssigner.new(@topic, Discourse.system_user).assign(@user)
Assigner.new(@topic, Discourse.system_user).assign(@user)
@group_a.remove(@user)
assignment = Assignment.first

View File

@ -21,7 +21,7 @@ RSpec.describe DiscourseAssign::AssignController do
SiteSetting.assign_allowed_on_groups = ''
put '/assign/assign.json', params: {
topic_id: post.topic_id, username: user2.username
target_id: post.topic_id, target_type: 'Topic', username: user2.username
}
expect(response.status).to eq(403)
@ -29,7 +29,7 @@ RSpec.describe DiscourseAssign::AssignController do
it 'filters requests where assigne group is not allowed' do
put '/assign/assign.json', params: {
topic_id: post.topic_id, group_name: default_allowed_group.name
target_id: post.topic_id, target_type: 'Topic', group_name: default_allowed_group.name
}
expect(response.status).to eq(400)
@ -45,7 +45,7 @@ RSpec.describe DiscourseAssign::AssignController do
defaults = "#{default_allowed_group.id}|#{allowed_group.id}"
SiteSetting.assign_allowed_on_groups = defaults
TopicAssigner.new(post.topic, user).assign(user2)
Assigner.new(post.topic, user).assign(user2)
get '/assign/suggestions.json'
suggestions = JSON.parse(response.body)['suggestions'].map { |u| u['username'] }
@ -57,7 +57,7 @@ RSpec.describe DiscourseAssign::AssignController do
allowed_group = Group.find_by(name: 'everyone')
allowed_group.add(user2)
SiteSetting.assign_allowed_on_groups = default_allowed_group.id.to_s
TopicAssigner.new(post.topic, user).assign(user2)
Assigner.new(post.topic, user).assign(user2)
get '/assign/suggestions.json'
suggestions = JSON.parse(response.body)['suggestions'].map { |u| u['username'] }.sort
@ -90,7 +90,7 @@ RSpec.describe DiscourseAssign::AssignController do
it 'excludes other users from the suggestions when they already reached the max assigns limit' do
another_admin = Fabricate(:admin, groups: [default_allowed_group])
TopicAssigner.new(post.topic, user).assign(another_admin)
Assigner.new(post.topic, user).assign(another_admin)
get '/assign/suggestions.json'
suggestions = JSON.parse(response.body)['suggestions'].map { |u| u['username'] }
@ -109,7 +109,7 @@ RSpec.describe DiscourseAssign::AssignController do
it 'assigns topic to a user' do
put '/assign/assign.json', params: {
topic_id: post.topic_id, username: user2.username
target_id: post.topic_id, target_type: 'Topic', username: user2.username
}
expect(response.status).to eq(200)
@ -118,7 +118,7 @@ RSpec.describe DiscourseAssign::AssignController do
it 'assigns topic to a group' do
put '/assign/assign.json', params: {
topic_id: post.topic_id, group_name: assign_allowed_group.name
target_id: post.topic_id, target_type: 'Topic', group_name: assign_allowed_group.name
}
expect(response.status).to eq(200)
@ -127,14 +127,14 @@ RSpec.describe DiscourseAssign::AssignController do
it 'fails to assign topic to the user if its already assigned to the same user' do
put '/assign/assign.json', params: {
topic_id: post.topic_id, username: user2.username
target_id: post.topic_id, target_type: 'Topic', username: user2.username
}
expect(response.status).to eq(200)
expect(post.topic.reload.assignment.assigned_to_id).to eq(user2.id)
put '/assign/assign.json', params: {
topic_id: post.topic_id, username: user2.username
target_id: post.topic_id, target_type: 'Topic', username: user2.username
}
expect(response.status).to eq(400)
@ -147,10 +147,10 @@ RSpec.describe DiscourseAssign::AssignController do
another_post = Fabricate(:post)
max_assigns = 1
SiteSetting.max_assigned_topics = max_assigns
TopicAssigner.new(post.topic, user).assign(another_user)
Assigner.new(post.topic, user).assign(another_user)
put '/assign/assign.json', params: {
topic_id: another_post.topic_id, username: another_user.username
target_id: another_post.topic_id, target_type: 'Topic', username: another_user.username
}
expect(response.status).to eq(400)
@ -164,7 +164,7 @@ RSpec.describe DiscourseAssign::AssignController do
another_user = Fabricate(:user)
add_to_assign_allowed_group(another_user)
put '/assign/assign.json', params: {
topic_id: pm.id, username: another_user.username
target_id: pm.id, target_type: 'Topic', username: another_user.username
}
expect(response.parsed_body['error']).to eq(
@ -177,7 +177,7 @@ RSpec.describe DiscourseAssign::AssignController do
another_user = Fabricate(:user)
add_to_assign_allowed_group(another_user)
put '/assign/assign.json', params: {
topic_id: topic.id, username: another_user.username
target_id: topic.id, target_type: "Topic", username: another_user.username
}
expect(response.parsed_body['error']).to eq(
@ -197,13 +197,13 @@ RSpec.describe DiscourseAssign::AssignController do
add_to_assign_allowed_group(user2)
freeze_time 1.hour.from_now
TopicAssigner.new(post1.topic, user).assign(user)
Assigner.new(post1.topic, user).assign(user)
freeze_time 1.hour.from_now
TopicAssigner.new(post2.topic, user).assign(user2)
Assigner.new(post2.topic, user).assign(user2)
freeze_time 1.hour.from_now
TopicAssigner.new(post3.topic, user).assign(user)
Assigner.new(post3.topic, user).assign(user)
sign_in(user)
end
@ -251,9 +251,9 @@ RSpec.describe DiscourseAssign::AssignController do
add_to_assign_allowed_group(user2)
add_to_assign_allowed_group(user)
TopicAssigner.new(post1.topic, user).assign(user)
TopicAssigner.new(post2.topic, user).assign(user2)
TopicAssigner.new(post3.topic, user).assign(user)
Assigner.new(post1.topic, user).assign(user)
Assigner.new(post2.topic, user).assign(user2)
Assigner.new(post3.topic, user).assign(user)
end
it 'list members order by assignments_count' do

View File

@ -56,8 +56,8 @@ describe ListController do
before do
add_to_assign_allowed_group(user)
TopicAssigner.new(topic1, user).assign(user)
TopicAssigner.new(topic2, user).assign(user2)
Assigner.new(topic1, user).assign(user)
Assigner.new(topic2, user).assign(user2)
sign_in(user)
end
@ -81,7 +81,7 @@ describe ListController do
it 'doesnt returns deleted topics' do
sign_in(admin)
TopicAssigner.new(topic, user).assign(user)
Assigner.new(topic, user).assign(user)
delete "/t/#{topic.id}.json"
@ -114,9 +114,9 @@ describe ListController do
add_to_assign_allowed_group(user)
add_to_assign_allowed_group(user2)
TopicAssigner.new(post1.topic, user).assign(user)
TopicAssigner.new(post2.topic, user).assign(user2)
TopicAssigner.new(post3.topic, user).assign(user)
Assigner.new(post1.topic, user).assign(user)
Assigner.new(post2.topic, user).assign(user2)
Assigner.new(post3.topic, user).assign(user)
sign_in(user)
end
@ -206,9 +206,9 @@ describe ListController do
add_to_assign_allowed_group(user)
add_to_assign_allowed_group(user2)
TopicAssigner.new(post1.topic, user).assign(user)
TopicAssigner.new(post2.topic, user).assign(user2)
TopicAssigner.new(post3.topic, user).assign(user)
Assigner.new(post1.topic, user).assign(user)
Assigner.new(post2.topic, user).assign(user2)
Assigner.new(post3.topic, user).assign(user)
sign_in(user)
end
@ -260,8 +260,8 @@ describe ListController do
before do
add_to_assign_allowed_group(user)
TopicAssigner.new(post1.topic, user).assign(user)
TopicAssigner.new(post2.topic, user).assign(user2)
Assigner.new(post1.topic, user).assign(user)
Assigner.new(post2.topic, user).assign(user2)
sign_in(user)
end

View File

@ -35,7 +35,7 @@ describe FlaggedTopicSerializer do
topic.posts << Fabricate(:post)
TopicAssigner.new(topic, user).assign(user)
Assigner.new(topic, user).assign(user)
topic
end
@ -62,7 +62,7 @@ describe FlaggedTopicSerializer do
topic.posts << Fabricate(:post)
TopicAssigner.new(topic, user).assign(assign_allowed_group)
Assigner.new(topic, user).assign(assign_allowed_group)
topic
end

View File

@ -19,10 +19,10 @@ RSpec.describe GroupShowSerializer do
end
it "counts assigned users and groups" do
TopicAssigner.new(topic, user).assign(user)
Assigner.new(topic, user).assign(user)
expect(serializer.as_json[:group_show][:assignment_count]).to eq(1)
TopicAssigner.new(topic2, user).assign(group)
Assigner.new(topic2, user).assign(group)
expect(serializer.as_json[:group_show][:assignment_count]).to eq(2)
end
end

View File

@ -20,10 +20,10 @@ RSpec.describe SuggestedTopicSerializer do
end
it "adds information about assignee for users and groups" do
TopicAssigner.new(topic, user).assign(user)
Assigner.new(topic, user).assign(user)
expect(serializer.as_json[:suggested_topic][:assigned_to_user][:username]).to eq(user.username)
TopicAssigner.new(topic2, user).assign(group)
Assigner.new(topic2, user).assign(group)
expect(serializer2.as_json[:suggested_topic][:assigned_to_group][:name]).to eq(group.name)
end
end

View File

@ -25,7 +25,7 @@ RSpec.describe TopicListSerializer do
topic.posts << Fabricate(:post)
TopicAssigner.new(topic, user).assign(user)
Assigner.new(topic, user).assign(user)
topic
end