diff --git a/app/controllers/discourse_assign/assign_controller.rb b/app/controllers/discourse_assign/assign_controller.rb index 41b13e3..4bd5e12 100644 --- a/app/controllers/discourse_assign/assign_controller.rb +++ b/app/controllers/discourse_assign/assign_controller.rb @@ -175,46 +175,40 @@ module DiscourseAssign end def user_menu_assigns - assign_notifications = Notification.unread_type( - current_user, - Notification.types[:assigned], - user_menu_limit - ) + assign_notifications = + Notification.unread_type(current_user, Notification.types[:assigned], user_menu_limit) if assign_notifications.size < user_menu_limit opts = {} - ignored_assignment_ids = assign_notifications.filter_map do |notification| - notification.data_hash[:assignment_id] - end + ignored_assignment_ids = + assign_notifications.filter_map { |notification| notification.data_hash[:assignment_id] } opts[:ignored_assignment_ids] = ignored_assignment_ids if ignored_assignment_ids.present? - assigns_list = TopicQuery.new( - current_user, - per_page: user_menu_limit - assign_notifications.size - ).list_messages_assigned(current_user, ignored_assignment_ids: ignored_assignment_ids) + assigns_list = + TopicQuery.new( + current_user, + per_page: user_menu_limit - assign_notifications.size, + ).list_messages_assigned(current_user, ignored_assignment_ids: ignored_assignment_ids) end if assign_notifications.present? - serialized_notifications = ActiveModel::ArraySerializer.new( - assign_notifications, - each_serializer: NotificationSerializer, - scope: guardian - ) + serialized_notifications = + ActiveModel::ArraySerializer.new( + assign_notifications, + each_serializer: NotificationSerializer, + scope: guardian, + ) end if assigns_list - serialized_assigns = serialize_data( - assigns_list, - TopicListSerializer, - scope: guardian, - root: false - )[:topics] + serialized_assigns = + serialize_data(assigns_list, TopicListSerializer, scope: guardian, root: false)[:topics] end render json: { - notifications: serialized_notifications || [], - topics: serialized_assigns || [], - } + notifications: serialized_notifications || [], + topics: serialized_assigns || [], + } end private diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index cd30f66..a81b959 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -126,6 +126,9 @@ en: min_recently_assigned_days: label: Min recently assigned days description: Defaults to 14 days. + skip_new_users_for_days: + label: Skip new users for days + description: Defaults to 0 days (users can be assigned immediately after signing up). max_recently_assigned_days: label: Max recently assigned days description: First attempt to exclude users assigned in the last `n` days. If no user left, fallbacks to `min_recently_assigned_days`. Defaults to 180 days. diff --git a/lib/random_assign_utils.rb b/lib/random_assign_utils.rb index c7b9296..fc0edaa 100644 --- a/lib/random_assign_utils.rb +++ b/lib/random_assign_utils.rb @@ -38,6 +38,7 @@ class RandomAssignUtils raise_error(automation, "Group(#{group_id}) not found") end + assignable_user_ids = User.assign_allowed.pluck(:id) users_on_holiday = Set.new( User.where( @@ -45,11 +46,15 @@ class RandomAssignUtils ).pluck(:id), ) + group_users = group.group_users.joins(:user) + if skip_new_users_for_days = fields.dig("skip_new_users_for_days", "value").presence + group_users = group_users.where("users.created_at < ?", skip_new_users_for_days.to_i.days.ago) + end + group_users_ids = - group - .group_users - .joins(:user) + group_users .pluck("users.id") + .filter { |user_id| assignable_user_ids.include?(user_id) } .reject { |user_id| users_on_holiday.include?(user_id) } if group_users_ids.empty? diff --git a/plugin.rb b/plugin.rb index cf481fa..d88c9d7 100644 --- a/plugin.rb +++ b/plugin.rb @@ -400,9 +400,7 @@ after_initialize do SQL where_args = { user_id: user.id } - if ignored_assignment_ids.present? - where_args[:ignored_assignment_ids] = ignored_assignment_ids - end + where_args[:ignored_assignment_ids] = ignored_assignment_ids if ignored_assignment_ids.present? list = list.where("topics.id IN (#{topic_ids_sql})", **where_args).includes(:allowed_users) create_list(:assigned, { unordered: true }, list) @@ -978,6 +976,7 @@ after_initialize do field :minimum_time_between_assignments, component: :text field :max_recently_assigned_days, component: :text field :min_recently_assigned_days, component: :text + field :skip_new_users_for_days, component: :text field :in_working_hours, component: :boolean field :post_template, component: :post diff --git a/spec/lib/random_assign_utils_spec.rb b/spec/lib/random_assign_utils_spec.rb index 17a51d0..63b8e05 100644 --- a/spec/lib/random_assign_utils_spec.rb +++ b/spec/lib/random_assign_utils_spec.rb @@ -44,7 +44,7 @@ describe RandomAssignUtils do automation, ) expect(topic_1.posts.first.raw).to match( - "Attempted randomly assign a member of `@#{group_1.name}`, but no one was available.", + I18n.t("discourse_automation.scriptables.random_assign.no_one", group: group_1.name), ) end end @@ -73,7 +73,33 @@ describe RandomAssignUtils do automation, ) expect(topic_1.posts.first.raw).to match( - "Attempted randomly assign a member of `@#{group_1.name}`, but no one was available.", + I18n.t("discourse_automation.scriptables.random_assign.no_one", group: group_1.name), + ) + end + end + + context "no users can be assigned because none are members of assign_allowed_on_groups groups" do + fab!(:topic_1) { Fabricate(:topic) } + fab!(:group_1) { Fabricate(:group) } + fab!(:user_1) { Fabricate(:user) } + + before { group_1.add(user_1) } + + it "creates post on the topic" do + described_class.automation_script!( + {}, + { + "assignees_group" => { + "value" => group_1.id, + }, + "assigned_topic" => { + "value" => topic_1.id, + }, + }, + automation, + ) + expect(topic_1.posts.first.raw).to match( + I18n.t("discourse_automation.scriptables.random_assign.no_one", group: group_1.name), ) end end @@ -158,7 +184,7 @@ describe RandomAssignUtils do automation, ) expect(topic_1.posts.first.raw).to match( - "Attempted randomly assign a member of `@#{group_1.name}`, but no one was available.", + I18n.t("discourse_automation.scriptables.random_assign.no_one", group: group_1.name), ) end end @@ -250,6 +276,59 @@ describe RandomAssignUtils do end end end + + context "skip_new_users_for_days is set" do + fab!(:topic_1) { Fabricate(:topic) } + fab!(:post_1) { Fabricate(:post, topic: topic_1) } + fab!(:group_1) { Fabricate(:group) } + fab!(:user_1) { Fabricate(:user) } + + before do + SiteSetting.assign_allowed_on_groups = "#{group_1.id}" + group_1.add(user_1) + end + + it "creates post on the topic if all users are new" do + described_class.automation_script!( + {}, + { + "assignees_group" => { + "value" => group_1.id, + }, + "assigned_topic" => { + "value" => topic_1.id, + }, + "skip_new_users_for_days" => { + "value" => "10", + }, + }, + automation, + ) + expect(topic_1.posts.last.raw).to match( + I18n.t("discourse_automation.scriptables.random_assign.no_one", group: group_1.name), + ) + end + + it "assign topic if all users are old" do + described_class.automation_script!( + {}, + { + "assignees_group" => { + "value" => group_1.id, + }, + "assigned_topic" => { + "value" => topic_1.id, + }, + "skip_new_users_for_days" => { + "value" => "0", + }, + }, + automation, + ) + + expect(topic_1.assignment).not_to eq(nil) + end + end end describe ".recently_assigned_users_ids" do diff --git a/spec/requests/assign_controller_spec.rb b/spec/requests/assign_controller_spec.rb index 74556ee..5b64209 100644 --- a/spec/requests/assign_controller_spec.rb +++ b/spec/requests/assign_controller_spec.rb @@ -401,7 +401,9 @@ RSpec.describe DiscourseAssign::AssignController do fab!(:read_assigned_post) { Fabricate(:post, topic: Fabricate(:post).topic) } fab!(:read_assigned_post_in_same_topic) { Fabricate(:post, topic: Fabricate(:post).topic) } - fab!(:unread_assigned_post_in_same_topic) { Fabricate(:post, topic: read_assigned_post_in_same_topic.topic) } + fab!(:unread_assigned_post_in_same_topic) do + Fabricate(:post, topic: read_assigned_post_in_same_topic.topic) + end fab!(:another_user_unread_assigned_topic) { Fabricate(:post).topic } fab!(:another_user_read_assigned_topic) { Fabricate(:post).topic } @@ -416,9 +418,7 @@ RSpec.describe DiscourseAssign::AssignController do read_assigned_post, unread_assigned_post_in_same_topic, read_assigned_post_in_same_topic, - ].each do |target| - Assigner.new(target, normal_admin).assign(user) - end + ].each { |target| Assigner.new(target, normal_admin).assign(user) } Notification .where( @@ -428,25 +428,23 @@ RSpec.describe DiscourseAssign::AssignController do topic_id: [ read_assigned_topic.id, read_assigned_post.topic.id, - read_assigned_post_in_same_topic.topic.id - ] + read_assigned_post_in_same_topic.topic.id, + ], ) .where.not( topic_id: read_assigned_post_in_same_topic.topic.id, - post_number: unread_assigned_post_in_same_topic.post_number + post_number: unread_assigned_post_in_same_topic.post_number, ) .update_all(read: true) Assigner.new(another_user_read_assigned_topic, normal_admin).assign(user2) Assigner.new(another_user_unread_assigned_topic, normal_admin).assign(user2) - Notification - .where( - notification_type: Notification.types[:assigned], - read: false, - user_id: user2.id, - topic_id: another_user_read_assigned_topic, - ) - .update_all(read: true) + Notification.where( + notification_type: Notification.types[:assigned], + read: false, + user_id: user2.id, + topic_id: another_user_read_assigned_topic, + ).update_all(read: true) end context "when logged out" do @@ -457,9 +455,7 @@ RSpec.describe DiscourseAssign::AssignController do end context "when logged in" do - before do - sign_in(user) - end + before { sign_in(user) } it "responds with 403 if the current user can't assign" do user.update!(admin: false) @@ -479,11 +475,16 @@ RSpec.describe DiscourseAssign::AssignController do expect(response.status).to eq(200) notifications = response.parsed_body["notifications"] - expect(notifications.map { |n| [n["topic_id"], n["post_number"]] }).to eq([ - [unread_assigned_topic.id, 1], - [unread_assigned_post.topic.id, unread_assigned_post.post_number], - [unread_assigned_post_in_same_topic.topic.id, unread_assigned_post_in_same_topic.post_number] - ]) + expect(notifications.map { |n| [n["topic_id"], n["post_number"]] }).to eq( + [ + [unread_assigned_topic.id, 1], + [unread_assigned_post.topic.id, unread_assigned_post.post_number], + [ + unread_assigned_post_in_same_topic.topic.id, + unread_assigned_post_in_same_topic.post_number, + ], + ], + ) end it "responds with an array of assigned topics that are not associated with any of the unread assigned notifications" do @@ -491,11 +492,13 @@ RSpec.describe DiscourseAssign::AssignController do expect(response.status).to eq(200) topics = response.parsed_body["topics"] - expect(topics.map { |t| t["id"] }).to eq([ - read_assigned_post_in_same_topic.topic.id, - read_assigned_post.topic.id, - read_assigned_topic.id, - ]) + expect(topics.map { |t| t["id"] }).to eq( + [ + read_assigned_post_in_same_topic.topic.id, + read_assigned_post.topic.id, + read_assigned_topic.id, + ], + ) end it "fills up the remaining of the UsersController::USER_MENU_LIST_LIMIT limit with assigned topics" do