diff --git a/config/settings.yml b/config/settings.yml index a0aedb0..17bdc10 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -31,6 +31,6 @@ plugins: client: true type: group_list list_type: compact - default: 'staff' + default: '' allow_any: false refresh: true diff --git a/db/migrate/20190718144722_set_assign_allowed_on_groups_default.rb b/db/migrate/20190718144722_set_assign_allowed_on_groups_default.rb new file mode 100644 index 0000000..004402e --- /dev/null +++ b/db/migrate/20190718144722_set_assign_allowed_on_groups_default.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class SetAssignAllowedOnGroupsDefault < ActiveRecord::Migration[5.2] + def up + current_values = DB.query_single("SELECT value FROM site_settings WHERE name = 'assign_allowed_on_groups'").first + + # Dynamically sets the default value, supports older versions. + if current_values.nil? + min_version = 201_907_171_337_43 + migrated_site_setting = DB.query_single( + "SELECT schema_migrations.version FROM schema_migrations WHERE schema_migrations.version = '#{min_version}'" + ).first + default = migrated_site_setting.present? ? Group::AUTO_GROUPS[:staff] : 'staff' + + execute "INSERT INTO site_settings (name, data_type, value, created_at, updated_at) + VALUES ('assign_allowed_on_groups', #{SiteSettings::TypeSupervisor.types[:group_list]}, '#{default}', now(), now())" + end + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/jobs/scheduled/enqueue_reminders.rb b/jobs/scheduled/enqueue_reminders.rb index ed364f9..52d0aa4 100644 --- a/jobs/scheduled/enqueue_reminders.rb +++ b/jobs/scheduled/enqueue_reminders.rb @@ -16,8 +16,7 @@ module Jobs end def allowed_group_ids - allowed_groups = SiteSetting.assign_allowed_on_groups.split('|') - Group.where(name: allowed_groups).pluck(:id).join(',') + Group.assign_allowed_groups.pluck(:id).join(',') end def user_ids diff --git a/lib/topic_assigner.rb b/lib/topic_assigner.rb index 563fbed..6372e46 100644 --- a/lib/topic_assigner.rb +++ b/lib/topic_assigner.rb @@ -96,7 +96,7 @@ class ::TopicAssigner allowed_groups = SiteSetting.assign_allowed_on_groups.split('|') User.human_users - .joins(:groups).where(groups: { name: allowed_groups }) + .assign_allowed .where('username_lower IN (?)', mentions.map(&:downcase)) .first end diff --git a/plugin.rb b/plugin.rb index 8b871a5..0624616 100644 --- a/plugin.rb +++ b/plugin.rb @@ -23,6 +23,13 @@ Discourse::Application.routes.append do get "topics/messages-assigned/:username" => "list#messages_assigned", as: "topics_messages_assigned", constraints: { username: /[\w.\-]+?/ } end + +# TODO: Remove this once 2.4.0.beta3 is released. +# HACK: Checking if the file exists, this means we can assume the migration happenned +above_min_version = File.exist?( + File.expand_path('../../../db/migrate/20190717133743_migrate_group_list_site_settings.rb', __FILE__) +) + after_initialize do require File.expand_path('../jobs/scheduled/enqueue_reminders.rb', __FILE__) require File.expand_path('../jobs/regular/remind_user.rb', __FILE__) @@ -48,11 +55,19 @@ after_initialize do =end reviewable_api_enabled = defined?(Reviewable) + # TODO: Remove this once 2.4 becomes the new stable. + attribute = above_min_version ? 'id' : 'name' + + add_class_method(:group, :assign_allowed_groups) do + allowed_groups = SiteSetting.assign_allowed_on_groups.split('|') + where("groups.#{attribute} IN (?)", allowed_groups) + end + add_to_class(:user, :can_assign?) do @can_assign ||= begin allowed_groups = SiteSetting.assign_allowed_on_groups.split('|').compact - allowed_groups.present? && groups.where('name in (?)', allowed_groups).exists? ? + allowed_groups.present? && groups.where("groups.#{attribute} in (?)", allowed_groups).exists? ? :true : :false end @can_assign == :true @@ -62,21 +77,22 @@ after_initialize do add_class_method(:user, :assign_allowed) do allowed_groups = SiteSetting.assign_allowed_on_groups.split('|') - where('users.id IN ( + where("users.id IN ( SELECT user_id FROM group_users INNER JOIN groups ON group_users.group_id = groups.id - WHERE groups.name IN (?) - )', allowed_groups) + WHERE groups.#{attribute} IN (?) + )", allowed_groups) end add_model_callback(Group, :before_update) do - if name_changed? + if !above_min_version && name_changed? SiteSetting.assign_allowed_on_groups = SiteSetting.assign_allowed_on_groups.gsub(name_was, name) end end add_model_callback(Group, :before_destroy) do - new_setting = SiteSetting.assign_allowed_on_groups.gsub(/#{name}[|]?/, '') + to_remove = above_min_version ? id : name + new_setting = SiteSetting.assign_allowed_on_groups.gsub(/#{to_remove}[|]?/, '') new_setting = new_setting.chomp('|') if new_setting.ends_with?('|') SiteSetting.assign_allowed_on_groups = new_setting end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index a269856..d4cb68a 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -10,19 +10,18 @@ RSpec.describe Group do SiteSetting.assign_enabled = true end - it 'updates the site setting when the group name changes' do - SiteSetting.assign_allowed_on_groups = "#{group.name}|staff|moderators" - different_name = 'different_name' - - group.update!(name: different_name) - - expect(SiteSetting.assign_allowed_on_groups).to eq "#{different_name}|staff|moderators" + let(:above_min_version) do + min_version = 201_907_171_337_43 + migrated_site_setting = DB.query_single( + "SELECT schema_migrations.version FROM schema_migrations WHERE schema_migrations.version = '#{min_version}'" + ).first.present? end - let(:removed_group_setting) { 'staff|moderators' } + let(:removed_group_setting) { above_min_version ? '3|4' : 'staff|moderators' } + let(:group_attribute) { above_min_version ? group.id : group.name } it 'removes the group from the setting when the group gets destroyed' do - SiteSetting.assign_allowed_on_groups = "#{group.name}|staff|moderators" + SiteSetting.assign_allowed_on_groups = "#{group_attribute}|#{removed_group_setting}" group.destroy! @@ -30,7 +29,7 @@ RSpec.describe Group do end it 'removes the group from the setting when this is the last one on the list' do - SiteSetting.assign_allowed_on_groups = "staff|moderators|#{group.name}" + SiteSetting.assign_allowed_on_groups = "#{removed_group_setting}|#{group_attribute}" group.destroy! @@ -38,7 +37,8 @@ RSpec.describe Group do end it 'removes the group from the list when it is on the middle of the list' do - SiteSetting.assign_allowed_on_groups = "staff|#{group.name}|moderators" + allowed_groups = above_min_version ? "3|#{group_attribute}|4" : "staff|#{group_attribute}|moderators" + SiteSetting.assign_allowed_on_groups = allowed_groups group.destroy! diff --git a/spec/requests/assign_controller_spec.rb b/spec/requests/assign_controller_spec.rb index d4fb776..7e64a49 100644 --- a/spec/requests/assign_controller_spec.rb +++ b/spec/requests/assign_controller_spec.rb @@ -11,6 +11,13 @@ RSpec.describe DiscourseAssign::AssignController do let(:post) { Fabricate(:post) } let(:user2) { Fabricate(:active_user) } + let(:above_min_version) do + min_version = 201_907_171_337_43 + migrated_site_setting = DB.query_single( + "SELECT schema_migrations.version FROM schema_migrations WHERE schema_migrations.version = '#{min_version}'" + ).first.present? + end + describe 'only allow users from allowed groups' do before { sign_in(user2) } @@ -31,7 +38,13 @@ RSpec.describe DiscourseAssign::AssignController do allowed_group = Group.find_by(name: 'everyone') user2.groups << allowed_group user2.groups << default_allowed_group - SiteSetting.assign_allowed_on_groups = 'staff|everyone' + defaults = if above_min_version + "#{default_allowed_group.id}|#{allowed_group.id}" + else + "#{default_allowed_group.name}|#{allowed_group.name}" + end + + SiteSetting.assign_allowed_on_groups = defaults TopicAssigner.new(post.topic, user).assign(user2) get '/assign/suggestions.json' @@ -43,7 +56,7 @@ RSpec.describe DiscourseAssign::AssignController do it 'does not include users from disallowed groups' do allowed_group = Group.find_by(name: 'everyone') user2.groups << allowed_group - SiteSetting.assign_allowed_on_groups = 'staff' + SiteSetting.assign_allowed_on_groups = above_min_version ? default_allowed_group.id.to_s : default_allowed_group.name TopicAssigner.new(post.topic, user).assign(user2) get '/assign/suggestions.json'