diff options
author | Nick Thomas <nick@gitlab.com> | 2019-03-27 16:18:28 +0000 |
---|---|---|
committer | Nick Thomas <nick@gitlab.com> | 2019-03-28 16:18:23 +0000 |
commit | f086dbf66ee54e684fed4be17bce1e3537779d24 (patch) | |
tree | 2b095a22cba62bdc5c933d6b488572bca883e929 | |
parent | 013f7cd24c4359d5d704b719d85b6df7ca1fdd55 (diff) | |
download | gitlab-ce-f086dbf66ee54e684fed4be17bce1e3537779d24.tar.gz |
Add a thin encapsulation around .pluck(:id)
-rw-r--r-- | app/models/application_record.rb | 8 | ||||
-rw-r--r-- | app/services/issuable_base_service.rb | 4 | ||||
-rw-r--r-- | app/services/merge_requests/delete_non_latest_diffs_service.rb | 4 | ||||
-rw-r--r-- | app/services/milestones/promote_service.rb | 4 | ||||
-rw-r--r-- | app/services/quick_actions/interpret_service.rb | 13 |
5 files changed, 19 insertions, 14 deletions
diff --git a/app/models/application_record.rb b/app/models/application_record.rb index a3d662d8250..6976185264e 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -7,6 +7,14 @@ class ApplicationRecord < ActiveRecord::Base where(id: ids) end + def self.id_not_in(ids) + where.not(id: ids) + end + + def self.pluck_primary_key + where(nil).pluck(self.primary_key) + end + def self.safe_find_or_create_by!(*args) safe_find_or_create_by(*args).tap do |record| record.validate! unless record.persisted? diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index f35ad2a9d8b..2b2774746a2 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -76,13 +76,11 @@ class IssuableBaseService < BaseService find_or_create_label_ids end - # rubocop: disable CodeReuse/ActiveRecord def filter_labels_in_param(key) return if params[key].to_a.empty? - params[key] = available_labels.where(id: params[key]).pluck(:id) + params[key] = available_labels.id_in(params[key]).pluck_primary_key end - # rubocop: enable CodeReuse/ActiveRecord def find_or_create_label_ids labels = params.delete(:labels) diff --git a/app/services/merge_requests/delete_non_latest_diffs_service.rb b/app/services/merge_requests/delete_non_latest_diffs_service.rb index d5929446122..bdb7ec8a7c2 100644 --- a/app/services/merge_requests/delete_non_latest_diffs_service.rb +++ b/app/services/merge_requests/delete_non_latest_diffs_service.rb @@ -8,15 +8,13 @@ module MergeRequests @merge_request = merge_request end - # rubocop: disable CodeReuse/ActiveRecord def execute diffs = @merge_request.non_latest_diffs.with_files diffs.each_batch(of: BATCH_SIZE) do |relation, index| - ids = relation.pluck(:id).map { |id| [id] } + ids = relation.pluck_primary_key.map { |id| [id] } DeleteDiffFilesWorker.bulk_perform_in(index * 5.minutes, ids) end end - # rubocop: enable CodeReuse/ActiveRecord end end diff --git a/app/services/milestones/promote_service.rb b/app/services/milestones/promote_service.rb index cbe5996e8ca..f6c04a7bae2 100644 --- a/app/services/milestones/promote_service.rb +++ b/app/services/milestones/promote_service.rb @@ -26,17 +26,15 @@ module Milestones private - # rubocop: disable CodeReuse/ActiveRecord def milestone_ids_for_merge(group_milestone) # Pluck need to be used here instead of select so the array of ids # is persistent after old milestones gets deleted. @milestone_ids_for_merge ||= begin search_params = { title: group_milestone.title, project_ids: group_project_ids, state: 'all' } milestones = MilestonesFinder.new(search_params).execute - milestones.pluck(:id) + milestones.pluck_primary_key end end - # rubocop: enable CodeReuse/ActiveRecord def move_children_to_group_milestone(group_milestone) milestone_ids_for_merge(group_milestone).in_groups_of(100, false) do |milestone_ids| diff --git a/app/services/quick_actions/interpret_service.rb b/app/services/quick_actions/interpret_service.rb index 131efb8925e..cdf3583a30c 100644 --- a/app/services/quick_actions/interpret_service.rb +++ b/app/services/quick_actions/interpret_service.rb @@ -554,7 +554,7 @@ module QuickActions current_user.can?(:"update_#{issuable.to_ability_name}", issuable) && issuable.project.boards.count == 1 end - # rubocop: disable CodeReuse/ActiveRecord + command :board_move do |target_list_name| label_ids = find_label_ids(target_list_name) @@ -562,14 +562,17 @@ module QuickActions label_id = label_ids.first # Ensure this label corresponds to a list on the board - next unless Label.on_project_boards(issuable.project_id).where(id: label_id).exists? + next unless Label.on_project_boards(issuable.project_id).id_in(label_id).exists? + + @updates[:remove_label_ids] = issuable + .labels + .on_project_boards(issuable.project_id) + .id_not_in(label_id) + .pluck_primary_key - @updates[:remove_label_ids] = - issuable.labels.on_project_boards(issuable.project_id).where.not(id: label_id).pluck(:id) @updates[:add_label_ids] = [label_id] end end - # rubocop: enable CodeReuse/ActiveRecord desc 'Mark this issue as a duplicate of another issue' explanation do |duplicate_reference| |