summaryrefslogtreecommitdiff
path: root/app
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2020-02-12 21:08:48 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2020-02-12 21:08:48 +0000
commit006e89697dd5165f355afc20fc6bb0cdfa7b381a (patch)
tree9095aeb37b2c80f3b0cc5a8dfd27baf93f05b61b /app
parent43e3dc2f95a25c600e08f65d4f1c406a1a63ed3d (diff)
downloadgitlab-ce-006e89697dd5165f355afc20fc6bb0cdfa7b381a.tar.gz
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'app')
-rw-r--r--app/assets/javascripts/helpers/avatar_helper.js6
-rw-r--r--app/controllers/boards/issues_controller.rb3
-rw-r--r--app/models/concerns/bulk_insert_safe.rb44
-rw-r--r--app/models/concerns/issuable.rb16
-rw-r--r--app/models/issue.rb6
-rw-r--r--app/models/label_link.rb1
-rw-r--r--app/models/merge_request_diff_commit.rb1
-rw-r--r--app/models/merge_request_diff_file.rb1
-rw-r--r--app/services/boards/issues/list_service.rb22
9 files changed, 92 insertions, 8 deletions
diff --git a/app/assets/javascripts/helpers/avatar_helper.js b/app/assets/javascripts/helpers/avatar_helper.js
index 35ac7b2629c..7891b44dd27 100644
--- a/app/assets/javascripts/helpers/avatar_helper.js
+++ b/app/assets/javascripts/helpers/avatar_helper.js
@@ -1,4 +1,4 @@
-import _ from 'underscore';
+import { escape } from 'lodash';
import { getFirstCharacterCapitalized } from '~/lib/utils/text_utility';
export const DEFAULT_SIZE_CLASS = 's40';
@@ -19,7 +19,7 @@ export function renderIdenticon(entity, options = {}) {
const bgClass = getIdenticonBackgroundClass(entity.id);
const title = getIdenticonTitle(entity.name);
- return `<div class="avatar identicon ${_.escape(sizeClass)} ${_.escape(bgClass)}">${_.escape(
+ return `<div class="avatar identicon ${escape(sizeClass)} ${escape(bgClass)}">${escape(
title,
)}</div>`;
}
@@ -31,5 +31,5 @@ export function renderAvatar(entity, options = {}) {
const { sizeClass = DEFAULT_SIZE_CLASS } = options;
- return `<img src="${_.escape(entity.avatar_url)}" class="avatar ${_.escape(sizeClass)}" />`;
+ return `<img src="${escape(entity.avatar_url)}" class="avatar ${escape(sizeClass)}" />`;
}
diff --git a/app/controllers/boards/issues_controller.rb b/app/controllers/boards/issues_controller.rb
index 1d6711e3c22..ed2c39e3fd3 100644
--- a/app/controllers/boards/issues_controller.rb
+++ b/app/controllers/boards/issues_controller.rb
@@ -20,6 +20,9 @@ module Boards
skip_before_action :authenticate_user!, only: [:index]
before_action :validate_id_list, only: [:bulk_move]
before_action :can_move_issues?, only: [:bulk_move]
+ before_action do
+ push_frontend_feature_flag(:board_search_optimization, board.group)
+ end
# rubocop: disable CodeReuse/ActiveRecord
def index
diff --git a/app/models/concerns/bulk_insert_safe.rb b/app/models/concerns/bulk_insert_safe.rb
new file mode 100644
index 00000000000..6d75906b21f
--- /dev/null
+++ b/app/models/concerns/bulk_insert_safe.rb
@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+
+module BulkInsertSafe
+ extend ActiveSupport::Concern
+
+ # These are the callbacks we think safe when used on models that are
+ # written to the database in bulk
+ CALLBACK_NAME_WHITELIST = Set[
+ :initialize,
+ :validate,
+ :validation,
+ :find,
+ :destroy
+ ].freeze
+
+ MethodNotAllowedError = Class.new(StandardError)
+
+ class_methods do
+ def set_callback(name, *args)
+ unless _bulk_insert_callback_allowed?(name, args)
+ raise MethodNotAllowedError.new(
+ "Not allowed to call `set_callback(#{name}, #{args})` when model extends `BulkInsertSafe`." \
+ "Callbacks that fire per each record being inserted do not work with bulk-inserts.")
+ end
+
+ super
+ end
+
+ private
+
+ def _bulk_insert_callback_allowed?(name, args)
+ _bulk_insert_whitelisted?(name) || _bulk_insert_saved_from_belongs_to?(name, args)
+ end
+
+ # belongs_to associations will install a before_save hook during class loading
+ def _bulk_insert_saved_from_belongs_to?(name, args)
+ args.first == :before && args.second.to_s.start_with?('autosave_associated_records_for_')
+ end
+
+ def _bulk_insert_whitelisted?(name)
+ CALLBACK_NAME_WHITELIST.include?(name)
+ end
+ end
+end
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index a6961df1b51..f3e03ed22d7 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -249,7 +249,7 @@ module Issuable
Gitlab::Database.nulls_last_order('highest_priority', direction))
end
- def order_labels_priority(direction = 'ASC', excluded_labels: [], extra_select_columns: [])
+ def order_labels_priority(direction = 'ASC', excluded_labels: [], extra_select_columns: [], with_cte: false)
params = {
target_type: name,
target_column: "#{table_name}.id",
@@ -265,7 +265,7 @@ module Issuable
] + extra_select_columns
select(select_columns.join(', '))
- .group(arel_table[:id])
+ .group(issue_grouping_columns(use_cte: with_cte))
.reorder(Gitlab::Database.nulls_last_order('highest_priority', direction))
end
@@ -294,6 +294,18 @@ module Issuable
grouping_columns
end
+ # Includes all table keys in group by clause when sorting
+ # preventing errors in postgres when using CTE search optimisation
+ #
+ # Returns an array of arel columns
+ def issue_grouping_columns(use_cte: false)
+ if use_cte
+ [arel_table[:state]] + attribute_names.map { |attr| arel_table[attr.to_sym] }
+ else
+ arel_table[:id]
+ end
+ end
+
def to_ability_name
model_name.singular
end
diff --git a/app/models/issue.rb b/app/models/issue.rb
index fd4a8c90386..684108b1a7a 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -172,8 +172,10 @@ class Issue < ApplicationRecord
end
end
- def self.order_by_position_and_priority
- order_labels_priority
+ # `with_cte` argument allows sorting when using CTE queries and prevents
+ # errors in postgres when using CTE search optimisation
+ def self.order_by_position_and_priority(with_cte: false)
+ order_labels_priority(with_cte: with_cte)
.reorder(Gitlab::Database.nulls_last_order('relative_position', 'ASC'),
Gitlab::Database.nulls_last_order('highest_priority', 'ASC'),
"id DESC")
diff --git a/app/models/label_link.rb b/app/models/label_link.rb
index ffc0afd8e85..5ae1e88e14e 100644
--- a/app/models/label_link.rb
+++ b/app/models/label_link.rb
@@ -1,6 +1,7 @@
# frozen_string_literal: true
class LabelLink < ApplicationRecord
+ include BulkInsertSafe
include Importable
belongs_to :target, polymorphic: true, inverse_of: :label_links # rubocop:disable Cop/PolymorphicAssociations
diff --git a/app/models/merge_request_diff_commit.rb b/app/models/merge_request_diff_commit.rb
index aa41a68f184..460b394f067 100644
--- a/app/models/merge_request_diff_commit.rb
+++ b/app/models/merge_request_diff_commit.rb
@@ -1,6 +1,7 @@
# frozen_string_literal: true
class MergeRequestDiffCommit < ApplicationRecord
+ include BulkInsertSafe
include ShaAttribute
include CachedCommit
diff --git a/app/models/merge_request_diff_file.rb b/app/models/merge_request_diff_file.rb
index 14c86ec69da..23319445a38 100644
--- a/app/models/merge_request_diff_file.rb
+++ b/app/models/merge_request_diff_file.rb
@@ -1,6 +1,7 @@
# frozen_string_literal: true
class MergeRequestDiffFile < ApplicationRecord
+ include BulkInsertSafe
include Gitlab::EncodingHelper
include DiffFile
diff --git a/app/services/boards/issues/list_service.rb b/app/services/boards/issues/list_service.rb
index a9240e1d8a0..699fa17cb65 100644
--- a/app/services/boards/issues/list_service.rb
+++ b/app/services/boards/issues/list_service.rb
@@ -10,7 +10,7 @@ module Boards
end
def execute
- fetch_issues.order_by_position_and_priority
+ fetch_issues.order_by_position_and_priority(with_cte: can_attempt_search_optimization?)
end
# rubocop: disable CodeReuse/ActiveRecord
@@ -63,6 +63,7 @@ module Boards
set_state
set_scope
set_non_archived
+ set_attempt_search_optimizations
params
end
@@ -87,6 +88,16 @@ module Boards
params[:non_archived] = parent.is_a?(Group)
end
+ def set_attempt_search_optimizations
+ return unless can_attempt_search_optimization?
+
+ if board.group_board?
+ params[:attempt_group_search_optimizations] = true
+ else
+ params[:attempt_project_search_optimizations] = true
+ end
+ end
+
# rubocop: disable CodeReuse/ActiveRecord
def board_label_ids
@board_label_ids ||= board.lists.movable.pluck(:label_id)
@@ -113,6 +124,15 @@ module Boards
.where("label_links.label_id = ?", list.label_id).limit(1))
end
# rubocop: enable CodeReuse/ActiveRecord
+
+ def board_group
+ board.group_board? ? parent : parent.group
+ end
+
+ def can_attempt_search_optimization?
+ params[:search].present? &&
+ Feature.enabled?(:board_search_optimization, board_group, default_enabled: false)
+ end
end
end
end