From 2adcd9a832ca7b465f012a9812e6fb9c2f0210f9 Mon Sep 17 00:00:00 2001 From: romanrizzi Date: Mon, 2 Mar 2020 10:53:39 -0300 Subject: [PATCH] DEV: Remove backward-compatibility code now that 2.4 is the new stable --- plugin.rb | 57 ++++++++++--------------- spec/models/group_spec.rb | 13 ++---- spec/requests/assign_controller_spec.rb | 18 ++------ 3 files changed, 28 insertions(+), 60 deletions(-) diff --git a/plugin.rb b/plugin.rb index ba558f1..c90df16 100644 --- a/plugin.rb +++ b/plugin.rb @@ -23,12 +23,6 @@ Discourse::Application.routes.append do get "topics/messages-assigned/:username" => "list#messages_assigned", as: "topics_messages_assigned", constraints: { username: ::RouteFormat.username } 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__) @@ -46,12 +40,9 @@ after_initialize do self.value = self.value.to_i if self.name == frequency_field end - # 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) + where(id: allowed_groups) end add_to_class(:user, :can_assign?) do @@ -59,7 +50,7 @@ after_initialize do begin return true if admin? allowed_groups = SiteSetting.assign_allowed_on_groups.split('|').compact - allowed_groups.present? && groups.where("groups.#{attribute} in (?)", allowed_groups).exists? ? + allowed_groups.present? && groups.where(id: allowed_groups).exists? ? :true : :false end @can_assign == :true @@ -72,19 +63,18 @@ after_initialize do where("users.admin OR users.id IN ( SELECT user_id FROM group_users INNER JOIN groups ON group_users.group_id = groups.id - WHERE groups.#{attribute} IN (?) + WHERE groups.id IN (?) )", allowed_groups) end add_model_callback(Group, :before_update) do - if !above_min_version && name_changed? + if 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 - to_remove = above_min_version ? id : name - new_setting = SiteSetting.assign_allowed_on_groups.gsub(/#{to_remove}[|]?/, '') + new_setting = SiteSetting.assign_allowed_on_groups.gsub(/#{id}[|]?/, '') new_setting = new_setting.chomp('|') if new_setting.ends_with?('|') SiteSetting.assign_allowed_on_groups = new_setting end @@ -268,26 +258,23 @@ after_initialize do id && id.to_i rescue nil end - # TODO: Remove this when 2.4 becomes the new stable - if self.respond_to?(:add_custom_reviewable_filter) - add_custom_reviewable_filter( - [ - :assigned_to, - Proc.new do |results, value| - results.joins(<<~SQL - INNER JOIN posts p ON p.id = target_id - INNER JOIN topics t ON t.id = p.topic_id - INNER JOIN topic_custom_fields tcf ON tcf.topic_id = t.id - INNER JOIN users u ON u.id = tcf.value::integer - SQL - ) - .where(target_type: Post.name) - .where('tcf.name = ?', TopicAssigner::ASSIGNED_TO_ID) - .where('u.username = ?', value) - end - ] - ) - end + add_custom_reviewable_filter( + [ + :assigned_to, + Proc.new do |results, value| + results.joins(<<~SQL + INNER JOIN posts p ON p.id = target_id + INNER JOIN topics t ON t.id = p.topic_id + INNER JOIN topic_custom_fields tcf ON tcf.topic_id = t.id + INNER JOIN users u ON u.id = tcf.value::integer + SQL + ) + .where(target_type: Post.name) + .where('tcf.name = ?', TopicAssigner::ASSIGNED_TO_ID) + .where('u.username = ?', value) + end + ] + ) on(:post_created) do |post| ::TopicAssigner.auto_assign(post, force: true) diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index d4cb68a..72b8a9b 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -10,15 +10,8 @@ RSpec.describe Group do SiteSetting.assign_enabled = true end - 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) { above_min_version ? '3|4' : 'staff|moderators' } - let(:group_attribute) { above_min_version ? group.id : group.name } + let(:removed_group_setting) { '3|4' } + let(:group_attribute) { group.id } it 'removes the group from the setting when the group gets destroyed' do SiteSetting.assign_allowed_on_groups = "#{group_attribute}|#{removed_group_setting}" @@ -37,7 +30,7 @@ RSpec.describe Group do end it 'removes the group from the list when it is on the middle of the list' do - allowed_groups = above_min_version ? "3|#{group_attribute}|4" : "staff|#{group_attribute}|moderators" + allowed_groups = "3|#{group_attribute}|4" 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 758b2fd..4ccd4b9 100644 --- a/spec/requests/assign_controller_spec.rb +++ b/spec/requests/assign_controller_spec.rb @@ -12,13 +12,6 @@ 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 - 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) } @@ -39,11 +32,7 @@ RSpec.describe DiscourseAssign::AssignController do allowed_group = Group.find_by(name: 'everyone') allowed_group.add(user2) - defaults = if above_min_version - "#{default_allowed_group.id}|#{allowed_group.id}" - else - "#{default_allowed_group.name}|#{allowed_group.name}" - end + defaults = "#{default_allowed_group.id}|#{allowed_group.id}" SiteSetting.assign_allowed_on_groups = defaults TopicAssigner.new(post.topic, user).assign(user2) @@ -57,7 +46,7 @@ RSpec.describe DiscourseAssign::AssignController do it 'does not include users from disallowed groups' do allowed_group = Group.find_by(name: 'everyone') allowed_group.add(user2) - SiteSetting.assign_allowed_on_groups = above_min_version ? default_allowed_group.id.to_s : default_allowed_group.name + SiteSetting.assign_allowed_on_groups = default_allowed_group.id.to_s TopicAssigner.new(post.topic, user).assign(user2) get '/assign/suggestions.json' @@ -71,8 +60,7 @@ RSpec.describe DiscourseAssign::AssignController do visible_group.add(user) invisible_group = Fabricate(:group, members_visibility_level: Group.visibility_levels[:members]) - SiteSetting.assign_allowed_on_groups = above_min_version ? "#{visible_group.id}|#{invisible_group.id}" - : "#{visible_group.name}|#{invisible_group.name}" + SiteSetting.assign_allowed_on_groups = "#{visible_group.id}|#{invisible_group.id}" get '/assign/suggestions.json' assign_allowed_on_groups = JSON.parse(response.body)['assign_allowed_on_groups']