From dc8f43fbb1541b269d049eae47db01c86855b704 Mon Sep 17 00:00:00 2001 From: Krzysztof Kotlarek Date: Thu, 14 Oct 2021 09:22:29 +1100 Subject: [PATCH] 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) --- .../discourse_assign/assign_controller.rb | 33 ++++--- app/models/assignment.rb | 9 ++ .../controllers/assign-user.js.es6 | 3 +- .../discourse/services/task-actions.js.es6 | 8 +- ...0211006223156_add_target_to_assignments.rb | 24 +++++ ...signments_from_custom_fields_to_a_table.rb | 31 ++++-- jobs/regular/assign_notification.rb | 2 +- jobs/regular/unassign_notification.rb | 2 +- jobs/scheduled/enqueue_reminders.rb | 2 +- lib/{topic_assigner.rb => assigner.rb} | 98 ++++++++++++------- lib/discourse_assign/helpers.rb | 4 +- lib/pending_assigns_reminder.rb | 2 +- plugin.rb | 36 +++---- spec/components/search_spec.rb | 8 +- spec/components/topic_query_spec.rb | 14 +-- ...ents_from_custom_fields_to_a_table_spec.rb | 2 + spec/integration/assign_spec.rb | 8 +- spec/jobs/scheduled/enqueue_reminders_spec.rb | 2 +- ...opic_assigner_spec.rb => assigner_spec.rb} | 54 +++++----- spec/lib/pending_assigns_reminder_spec.rb | 10 +- spec/models/topic_spec.rb | 4 +- spec/plugin_spec.rb | 4 +- spec/requests/assign_controller_spec.rb | 38 +++---- spec/requests/list_controller_spec.rb | 22 ++--- .../flagged_topic_serializer_spec.rb | 4 +- .../serializers/group_show_serializer_spec.rb | 4 +- .../suggested_topic_serializer_spec.rb | 4 +- .../serializers/topic_list_serializer_spec.rb | 2 +- 28 files changed, 264 insertions(+), 170 deletions(-) create mode 100644 db/migrate/20211006223156_add_target_to_assignments.rb rename lib/{topic_assigner.rb => assigner.rb} (79%) rename spec/lib/{topic_assigner_spec.rb => assigner_spec.rb} (84%) diff --git a/app/controllers/discourse_assign/assign_controller.rb b/app/controllers/discourse_assign/assign_controller.rb index 22c54e9..c41cf55 100644 --- a/app/controllers/discourse_assign/assign_controller.rb +++ b/app/controllers/discourse_assign/assign_controller.rb @@ -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'") diff --git a/app/models/assignment.rb b/app/models/assignment.rb index 9568402..ac67f17 100644 --- a/app/models/assignment.rb +++ b/app/models/assignment.rb @@ -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' diff --git a/assets/javascripts/discourse-assign/controllers/assign-user.js.es6 b/assets/javascripts/discourse-assign/controllers/assign-user.js.es6 index cf4df58..f291f94 100644 --- a/assets/javascripts/discourse-assign/controllers/assign-user.js.es6 +++ b/assets/javascripts/discourse-assign/controllers/assign-user.js.es6 @@ -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(() => { diff --git a/assets/javascripts/discourse/services/task-actions.js.es6 b/assets/javascripts/discourse/services/task-actions.js.es6 index f78279b..853865a 100644 --- a/assets/javascripts/discourse/services/task-actions.js.es6 +++ b/assets/javascripts/discourse/services/task-actions.js.es6 @@ -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", }, }); }, diff --git a/db/migrate/20211006223156_add_target_to_assignments.rb b/db/migrate/20211006223156_add_target_to_assignments.rb new file mode 100644 index 0000000..b6d45d2 --- /dev/null +++ b/db/migrate/20211006223156_add_target_to_assignments.rb @@ -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 diff --git a/db/post_migrate/20210714173022_correctly_move_assignments_from_custom_fields_to_a_table.rb b/db/post_migrate/20210714173022_correctly_move_assignments_from_custom_fields_to_a_table.rb index 1921758..616f7c5 100644 --- a/db/post_migrate/20210714173022_correctly_move_assignments_from_custom_fields_to_a_table.rb +++ b/db/post_migrate/20210714173022_correctly_move_assignments_from_custom_fields_to_a_table.rb @@ -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) diff --git a/jobs/regular/assign_notification.rb b/jobs/regular/assign_notification.rb index 36a4a78..f88798f 100644 --- a/jobs/regular/assign_notification.rb +++ b/jobs/regular/assign_notification.rb @@ -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 diff --git a/jobs/regular/unassign_notification.rb b/jobs/regular/unassign_notification.rb index 6c887d5..00fb15c 100644 --- a/jobs/regular/unassign_notification.rb +++ b/jobs/regular/unassign_notification.rb @@ -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], diff --git a/jobs/scheduled/enqueue_reminders.rb b/jobs/scheduled/enqueue_reminders.rb index 4b60bba..dc6756a 100644 --- a/jobs/scheduled/enqueue_reminders.rb +++ b/jobs/scheduled/enqueue_reminders.rb @@ -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 diff --git a/lib/topic_assigner.rb b/lib/assigner.rb similarity index 79% rename from lib/topic_assigner.rb rename to lib/assigner.rb index 1403cf9..4a711c9 100644 --- a/lib/topic_assigner.rb +++ b/lib/assigner.rb @@ -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 ) diff --git a/lib/discourse_assign/helpers.rb b/lib/discourse_assign/helpers.rb index ac379f9..831ed05 100644 --- a/lib/discourse_assign/helpers.rb +++ b/lib/discourse_assign/helpers.rb @@ -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 diff --git a/lib/pending_assigns_reminder.rb b/lib/pending_assigns_reminder.rb index 3ba7867..5d2c612 100644 --- a/lib/pending_assigns_reminder.rb +++ b/lib/pending_assigns_reminder.rb @@ -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:) diff --git a/plugin.rb b/plugin.rb index 8ed9e50..1634cb3 100644 --- a/plugin.rb +++ b/plugin.rb @@ -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 diff --git a/spec/components/search_spec.rb b/spec/components/search_spec.rb index 9ffe2fc..1309d49 100644 --- a/spec/components/search_spec.rb +++ b/spec/components/search_spec.rb @@ -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) diff --git a/spec/components/topic_query_spec.rb b/spec/components/topic_query_spec.rb index 546b884..2b00399 100644 --- a/spec/components/topic_query_spec.rb +++ b/spec/components/topic_query_spec.rb @@ -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 diff --git a/spec/db/post_migrate/20210714173022_correctly_move_assignments_from_custom_fields_to_a_table_spec.rb b/spec/db/post_migrate/20210714173022_correctly_move_assignments_from_custom_fields_to_a_table_spec.rb index 4a2acdb..b8d7279 100644 --- a/spec/db/post_migrate/20210714173022_correctly_move_assignments_from_custom_fields_to_a_table_spec.rb +++ b/spec/db/post_migrate/20210714173022_correctly_move_assignments_from_custom_fields_to_a_table_spec.rb @@ -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 diff --git a/spec/integration/assign_spec.rb b/spec/integration/assign_spec.rb index 7cc1218..6f5d99b 100644 --- a/spec/integration/assign_spec.rb +++ b/spec/integration/assign_spec.rb @@ -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) diff --git a/spec/jobs/scheduled/enqueue_reminders_spec.rb b/spec/jobs/scheduled/enqueue_reminders_spec.rb index 0fe7eec..1c5e5b5 100644 --- a/spec/jobs/scheduled/enqueue_reminders_spec.rb +++ b/spec/jobs/scheduled/enqueue_reminders_spec.rb @@ -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 diff --git a/spec/lib/topic_assigner_spec.rb b/spec/lib/assigner_spec.rb similarity index 84% rename from spec/lib/topic_assigner_spec.rb rename to spec/lib/assigner_spec.rb index 70a5943..c01737f 100644 --- a/spec/lib/topic_assigner_spec.rb +++ b/spec/lib/assigner_spec.rb @@ -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 diff --git a/spec/lib/pending_assigns_reminder_spec.rb b/spec/lib/pending_assigns_reminder_spec.rb index 99b493a..e8b56aa 100644 --- a/spec/lib/pending_assigns_reminder_spec.rb +++ b/spec/lib/pending_assigns_reminder_spec.rb @@ -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 diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index 656b290..647ec1f 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -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 diff --git a/spec/plugin_spec.rb b/spec/plugin_spec.rb index 8466e97..d2fc6dc 100644 --- a/spec/plugin_spec.rb +++ b/spec/plugin_spec.rb @@ -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 diff --git a/spec/requests/assign_controller_spec.rb b/spec/requests/assign_controller_spec.rb index 4930c1f..e563adb 100644 --- a/spec/requests/assign_controller_spec.rb +++ b/spec/requests/assign_controller_spec.rb @@ -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 diff --git a/spec/requests/list_controller_spec.rb b/spec/requests/list_controller_spec.rb index 61c17d8..a7e471a 100644 --- a/spec/requests/list_controller_spec.rb +++ b/spec/requests/list_controller_spec.rb @@ -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 diff --git a/spec/serializers/flagged_topic_serializer_spec.rb b/spec/serializers/flagged_topic_serializer_spec.rb index 48761e4..16ee23d 100644 --- a/spec/serializers/flagged_topic_serializer_spec.rb +++ b/spec/serializers/flagged_topic_serializer_spec.rb @@ -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 diff --git a/spec/serializers/group_show_serializer_spec.rb b/spec/serializers/group_show_serializer_spec.rb index 1e2aaff..d7c0f09 100644 --- a/spec/serializers/group_show_serializer_spec.rb +++ b/spec/serializers/group_show_serializer_spec.rb @@ -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 diff --git a/spec/serializers/suggested_topic_serializer_spec.rb b/spec/serializers/suggested_topic_serializer_spec.rb index a31edef..cd560ac 100644 --- a/spec/serializers/suggested_topic_serializer_spec.rb +++ b/spec/serializers/suggested_topic_serializer_spec.rb @@ -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 diff --git a/spec/serializers/topic_list_serializer_spec.rb b/spec/serializers/topic_list_serializer_spec.rb index 8e212e8..650af81 100644 --- a/spec/serializers/topic_list_serializer_spec.rb +++ b/spec/serializers/topic_list_serializer_spec.rb @@ -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