diff --git a/app/assets/javascripts/helpers/avatar_helper.js b/app/assets/javascripts/helpers/avatar_helper.js
index 35ac7b2629c02aa8ce0b414180d22a5343751c33..7891b44dd2776c632d6f8732a93d53d3fa0bbf7e 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 `
${_.escape(
+ return `
${escape(
title,
)}
`;
}
@@ -31,5 +31,5 @@ export function renderAvatar(entity, options = {}) {
const { sizeClass = DEFAULT_SIZE_CLASS } = options;
- return `
})
`;
+ return `
})
`;
}
diff --git a/app/controllers/boards/issues_controller.rb b/app/controllers/boards/issues_controller.rb
index 1d6711e3c226145dd061927413e0d1789bc8988e..ed2c39e3fd3a8d63ded7a648047d234d9249dff3 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 0000000000000000000000000000000000000000..6d75906b21fc79244b75e91a6a1095f05933741c
--- /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 a6961df1b514a487a31a38c46fa8819ac07e258a..f3e03ed22d7f32c4fceea254b52434b07e9ac9ed 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 fd4a8c90386966eb2a873f39b0764b03f311a605..684108b1a7a986cf38b8428e1988dd1651123eb4 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 ffc0afd8e8564eae11b7bbfea84a59eaaa9480ca..5ae1e88e14eec7412da9e54952cf0dc0a903f73f 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 aa41a68f18424657e1570575ee81690354586069..460b394f06749cfcbd5ea577454ecadeb6af2e62 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 14c86ec69da560396fefc8bc55b09271b1e63ed3..23319445a38b2834a41a2f5cc4f57172720a7004 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 a9240e1d8a04dc172f0137bc8ffc83e3a5e85744..699fa17cb65a45c63522501ed241eae1efb3c028 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
diff --git a/changelogs/unreleased/loadash_helpers.yml b/changelogs/unreleased/loadash_helpers.yml
new file mode 100644
index 0000000000000000000000000000000000000000..0296ba6665f63f875b45f384f6c6b5b05a08d695
--- /dev/null
+++ b/changelogs/unreleased/loadash_helpers.yml
@@ -0,0 +1,5 @@
+---
+title: Replace underscore with lodash in /app/assets/javascripts/helpers
+merge_request: 25014
+author: rkpattnaik780
+type: changed
diff --git a/doc/development/feature_flags/controls.md b/doc/development/feature_flags/controls.md
index d711e49ee8bb213449d7331c69ac6bdbf179ebb5..922995cb9158fa4a7cd5b565befb69be075220a6 100644
--- a/doc/development/feature_flags/controls.md
+++ b/doc/development/feature_flags/controls.md
@@ -51,7 +51,7 @@ easier to measure the impact of both separately.
GitLab's feature library (using
[Flipper](https://github.com/jnunemaker/flipper), and covered in the [Feature
Flags process](process.md) guide) supports rolling out changes to a percentage of
-users. This in turn can be controlled using [GitLab Chatops](../../ci/chatops/README.md).
+time to users. This in turn can be controlled using [GitLab Chatops](../../ci/chatops/README.md).
For an up to date list of feature flag commands please see [the source
code](https://gitlab.com/gitlab-com/chatops/blob/master/lib/chatops/commands/feature.rb).
@@ -136,8 +136,13 @@ you run these 2 commands:
/chatops run feature set some_feature 25
```
-Then `some_feature` will be enabled for 25% of the users interacting with
-`gitlab-org/gitlab`, and no one else.
+Then `some_feature` will be enabled for 25% of the time the users are interacting with
+`gitlab-org/gitlab`. Note that the the feature is not enabled to 25%
+of the users, rather a simple randomization is made each time the `enabled?` is checked.
+
+NOTE: **Note:**
+**Percentage of time** rollout is not a good idea if what you want is to make sure a feature
+is always on or off to the users.
## Cleaning up
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index eb949de4f3fc2d76a84fec8749bc2f5fe529f64c..9edcf28e93e02a8036624d38c4f80f30aaca8c76 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -7773,6 +7773,9 @@ msgstr ""
msgid "Events"
msgstr ""
+msgid "Events in %{project_path}"
+msgstr ""
+
msgid "Every %{action} attempt has failed: %{job_error_message}. Please try again."
msgstr ""
@@ -14593,6 +14596,9 @@ msgstr ""
msgid "Project '%{project_name}' will be deleted on %{date}"
msgstr ""
+msgid "Project Audit Events"
+msgstr ""
+
msgid "Project Badges"
msgstr ""
diff --git a/qa/qa/specs/features/browser_ui/3_create/merge_request/create_merge_request_spec.rb b/qa/qa/specs/features/browser_ui/3_create/merge_request/create_merge_request_spec.rb
index 5baa0e90c47c03856b5916a4a2b308c32ab8cfd2..4a9901f2a84b840a655c6772518b6bddd2199181 100644
--- a/qa/qa/specs/features/browser_ui/3_create/merge_request/create_merge_request_spec.rb
+++ b/qa/qa/specs/features/browser_ui/3_create/merge_request/create_merge_request_spec.rb
@@ -14,7 +14,7 @@ module QA
@merge_request_description = '... to find them, to bring them all, and in the darkness bind them'
end
- it 'creates a basic merge request', :smoke do
+ it 'creates a basic merge request' do
Resource::MergeRequest.fabricate_via_browser_ui! do |merge_request|
merge_request.project = @project
merge_request.title = @merge_request_title
diff --git a/spec/models/concerns/bulk_insert_safe_spec.rb b/spec/models/concerns/bulk_insert_safe_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..918846807385a8ea8db18bd08f87ae150373ee11
--- /dev/null
+++ b/spec/models/concerns/bulk_insert_safe_spec.rb
@@ -0,0 +1,38 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe BulkInsertSafe do
+ class BulkInsertItem < ApplicationRecord
+ include BulkInsertSafe
+ end
+
+ module InheritedUnsafeMethods
+ extend ActiveSupport::Concern
+
+ included do
+ after_save -> { "unsafe" }
+ end
+ end
+
+ module InheritedSafeMethods
+ extend ActiveSupport::Concern
+
+ included do
+ after_initialize -> { "safe" }
+ end
+ end
+
+ it_behaves_like 'a BulkInsertSafe model', BulkInsertItem
+
+ context 'when inheriting class methods' do
+ it 'raises an error when method is not bulk-insert safe' do
+ expect { BulkInsertItem.include(InheritedUnsafeMethods) }.to(
+ raise_error(subject::MethodNotAllowedError))
+ end
+
+ it 'does not raise an error when method is bulk-insert safe' do
+ expect { BulkInsertItem.include(InheritedSafeMethods) }.not_to raise_error
+ end
+ end
+end
diff --git a/spec/models/label_link_spec.rb b/spec/models/label_link_spec.rb
index b160e72e75962598f700b8493f95124cbb67b118..0a5cb5374b045f707932c5ad95c7229f54571d2a 100644
--- a/spec/models/label_link_spec.rb
+++ b/spec/models/label_link_spec.rb
@@ -7,4 +7,6 @@ describe LabelLink do
it { is_expected.to belong_to(:label) }
it { is_expected.to belong_to(:target) }
+
+ it_behaves_like 'a BulkInsertSafe model', LabelLink
end
diff --git a/spec/models/merge_request_diff_commit_spec.rb b/spec/models/merge_request_diff_commit_spec.rb
index a74c0b2064220a819246d08c67bb030bca5b6aac..a296122ae09e2c3c635eb429849f3b3756c4b51b 100644
--- a/spec/models/merge_request_diff_commit_spec.rb
+++ b/spec/models/merge_request_diff_commit_spec.rb
@@ -6,6 +6,8 @@ describe MergeRequestDiffCommit do
let(:merge_request) { create(:merge_request) }
let(:project) { merge_request.project }
+ it_behaves_like 'a BulkInsertSafe model', MergeRequestDiffCommit
+
describe '#to_hash' do
subject { merge_request.commits.first }
diff --git a/spec/models/merge_request_diff_file_spec.rb b/spec/models/merge_request_diff_file_spec.rb
index 84f9c9d06ba4580ae5958583cc729ed032bd8e88..6ecbc5bf832146922b7ee3aaef02eb7b1b2c0ddf 100644
--- a/spec/models/merge_request_diff_file_spec.rb
+++ b/spec/models/merge_request_diff_file_spec.rb
@@ -3,6 +3,8 @@
require 'spec_helper'
describe MergeRequestDiffFile do
+ it_behaves_like 'a BulkInsertSafe model', MergeRequestDiffFile
+
describe '#diff' do
context 'when diff is not stored' do
let(:unpacked) { 'unpacked' }
diff --git a/spec/support/shared_examples/models/concerns/bulk_insert_safe_shared_examples.rb b/spec/support/shared_examples/models/concerns/bulk_insert_safe_shared_examples.rb
new file mode 100644
index 0000000000000000000000000000000000000000..0a2b6616bb4dd1a0ef58cf9c306beff499e47726
--- /dev/null
+++ b/spec/support/shared_examples/models/concerns/bulk_insert_safe_shared_examples.rb
@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+RSpec.shared_examples 'a BulkInsertSafe model' do |target_class|
+ # We consider all callbacks unsafe for bulk insertions unless we have explicitly
+ # whitelisted them (esp. anything related to :save, :create, :commit etc.)
+ let(:callback_method_blacklist) do
+ ActiveRecord::Callbacks::CALLBACKS.reject do |callback|
+ cb_name = callback.to_s.gsub(/(before_|after_|around_)/, '').to_sym
+ BulkInsertSafe::CALLBACK_NAME_WHITELIST.include?(cb_name)
+ end.to_set
+ end
+
+ context 'when calling class methods directly' do
+ it 'raises an error when method is not bulk-insert safe' do
+ callback_method_blacklist.each do |m|
+ expect { target_class.send(m, nil) }.to(
+ raise_error(BulkInsertSafe::MethodNotAllowedError),
+ "Expected call to #{m} to raise an error, but it didn't"
+ )
+ end
+ end
+
+ it 'does not raise an error when method is bulk-insert safe' do
+ BulkInsertSafe::CALLBACK_NAME_WHITELIST.each do |name|
+ expect { target_class.set_callback(name) {} }.not_to raise_error
+ end
+ end
+
+ it 'does not raise an error when the call is triggered by belongs_to' do
+ expect { target_class.belongs_to(:other_record) }.not_to raise_error
+ end
+ end
+end