import _ from 'underscore'; import { escape } from 'lodash';
import { getFirstCharacterCapitalized } from '~/lib/utils/text_utility'; import { getFirstCharacterCapitalized } from '~/lib/utils/text_utility';
export const DEFAULT_SIZE_CLASS = 's40'; export const DEFAULT_SIZE_CLASS = 's40';
...@@ -19,7 +19,7 @@ export function renderIdenticon(entity, options = {}) { ...@@ -19,7 +19,7 @@ export function renderIdenticon(entity, options = {}) {
const bgClass = getIdenticonBackgroundClass(entity.id); const bgClass = getIdenticonBackgroundClass(entity.id);
const title = getIdenticonTitle(entity.name); 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, title,
)}</div>`; )}</div>`;
} }
...@@ -31,5 +31,5 @@ export function renderAvatar(entity, options = {}) { ...@@ -31,5 +31,5 @@ export function renderAvatar(entity, options = {}) {
const { sizeClass = DEFAULT_SIZE_CLASS } = 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)}" />`;
} }
...@@ -20,6 +20,9 @@ module Boards ...@@ -20,6 +20,9 @@ module Boards
skip_before_action :authenticate_user!, only: [:index] skip_before_action :authenticate_user!, only: [:index]
before_action :validate_id_list, only: [:bulk_move] before_action :validate_id_list, only: [:bulk_move]
before_action :can_move_issues?, 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 # rubocop: disable CodeReuse/ActiveRecord
def index def index
... ...
......
# 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
...@@ -249,7 +249,7 @@ module Issuable ...@@ -249,7 +249,7 @@ module Issuable
Gitlab::Database.nulls_last_order('highest_priority', direction)) Gitlab::Database.nulls_last_order('highest_priority', direction))
end 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 = { params = {
target_type: name, target_type: name,
target_column: "#{table_name}.id", target_column: "#{table_name}.id",
...@@ -265,7 +265,7 @@ module Issuable ...@@ -265,7 +265,7 @@ module Issuable
] + extra_select_columns ] + extra_select_columns
select(select_columns.join(', ')) 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)) .reorder(Gitlab::Database.nulls_last_order('highest_priority', direction))
end end
...@@ -294,6 +294,18 @@ module Issuable ...@@ -294,6 +294,18 @@ module Issuable
grouping_columns grouping_columns
end 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 def to_ability_name
model_name.singular model_name.singular
end end
... ...
......
...@@ -172,8 +172,10 @@ class Issue < ApplicationRecord ...@@ -172,8 +172,10 @@ class Issue < ApplicationRecord
end end
end end
def self.order_by_position_and_priority # `with_cte` argument allows sorting when using CTE queries and prevents
order_labels_priority # 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'), .reorder(Gitlab::Database.nulls_last_order('relative_position', 'ASC'),
Gitlab::Database.nulls_last_order('highest_priority', 'ASC'), Gitlab::Database.nulls_last_order('highest_priority', 'ASC'),
"id DESC") "id DESC")
... ...
......
# frozen_string_literal: true # frozen_string_literal: true
class LabelLink < ApplicationRecord class LabelLink < ApplicationRecord
include BulkInsertSafe
include Importable include Importable
belongs_to :target, polymorphic: true, inverse_of: :label_links # rubocop:disable Cop/PolymorphicAssociations belongs_to :target, polymorphic: true, inverse_of: :label_links # rubocop:disable Cop/PolymorphicAssociations
... ...
......
# frozen_string_literal: true # frozen_string_literal: true
class MergeRequestDiffCommit < ApplicationRecord class MergeRequestDiffCommit < ApplicationRecord
include BulkInsertSafe
include ShaAttribute include ShaAttribute
include CachedCommit include CachedCommit
... ...
......
# frozen_string_literal: true # frozen_string_literal: true
class MergeRequestDiffFile < ApplicationRecord class MergeRequestDiffFile < ApplicationRecord
include BulkInsertSafe
include Gitlab::EncodingHelper include Gitlab::EncodingHelper
include DiffFile include DiffFile
... ...
......
...@@ -10,7 +10,7 @@ module Boards ...@@ -10,7 +10,7 @@ module Boards
end end
def execute def execute
fetch_issues.order_by_position_and_priority fetch_issues.order_by_position_and_priority(with_cte: can_attempt_search_optimization?)
end end
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
...@@ -63,6 +63,7 @@ module Boards ...@@ -63,6 +63,7 @@ module Boards
set_state set_state
set_scope set_scope
set_non_archived set_non_archived
set_attempt_search_optimizations
params params
end end
...@@ -87,6 +88,16 @@ module Boards ...@@ -87,6 +88,16 @@ module Boards
params[:non_archived] = parent.is_a?(Group) params[:non_archived] = parent.is_a?(Group)
end 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 # rubocop: disable CodeReuse/ActiveRecord
def board_label_ids def board_label_ids
@board_label_ids ||= board.lists.movable.pluck(:label_id) @board_label_ids ||= board.lists.movable.pluck(:label_id)
...@@ -113,6 +124,15 @@ module Boards ...@@ -113,6 +124,15 @@ module Boards
.where("label_links.label_id = ?", list.label_id).limit(1)) .where("label_links.label_id = ?", list.label_id).limit(1))
end end
# rubocop: enable CodeReuse/ActiveRecord # 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 end
end end
... ...
......
---
title: Replace underscore with lodash in /app/assets/javascripts/helpers
merge_request: 25014
author: rkpattnaik780
type: changed
...@@ -51,7 +51,7 @@ easier to measure the impact of both separately. ...@@ -51,7 +51,7 @@ easier to measure the impact of both separately.
GitLab's feature library (using GitLab's feature library (using
[Flipper](https://github.com/jnunemaker/flipper), and covered in the [Feature [Flipper](https://github.com/jnunemaker/flipper), and covered in the [Feature
Flags process](process.md) guide) supports rolling out changes to a percentage of 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 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). code](https://gitlab.com/gitlab-com/chatops/blob/master/lib/chatops/commands/feature.rb).
...@@ -136,8 +136,13 @@ you run these 2 commands: ...@@ -136,8 +136,13 @@ you run these 2 commands:
/chatops run feature set some_feature 25 /chatops run feature set some_feature 25
``` ```
Then `some_feature` will be enabled for 25% of the users interacting with Then `some_feature` will be enabled for 25% of the time the users are interacting with
`gitlab-org/gitlab`, and no one else. `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 ## Cleaning up
... ...
......
...@@ -7773,6 +7773,9 @@ msgstr "" ...@@ -7773,6 +7773,9 @@ msgstr ""
msgid "Events" msgid "Events"
msgstr "" msgstr ""
msgid "Events in %{project_path}"
msgstr ""
msgid "Every %{action} attempt has failed: %{job_error_message}. Please try again." msgid "Every %{action} attempt has failed: %{job_error_message}. Please try again."
msgstr "" msgstr ""
...@@ -14593,6 +14596,9 @@ msgstr "" ...@@ -14593,6 +14596,9 @@ msgstr ""
msgid "Project '%{project_name}' will be deleted on %{date}" msgid "Project '%{project_name}' will be deleted on %{date}"
msgstr "" msgstr ""
msgid "Project Audit Events"
msgstr ""
msgid "Project Badges" msgid "Project Badges"
msgstr "" msgstr ""
... ...
......
...@@ -14,7 +14,7 @@ module QA ...@@ -14,7 +14,7 @@ module QA
@merge_request_description = '... to find them, to bring them all, and in the darkness bind them' @merge_request_description = '... to find them, to bring them all, and in the darkness bind them'
end 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| Resource::MergeRequest.fabricate_via_browser_ui! do |merge_request|
merge_request.project = @project merge_request.project = @project
merge_request.title = @merge_request_title merge_request.title = @merge_request_title
... ...
......
# 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
...@@ -7,4 +7,6 @@ describe LabelLink do ...@@ -7,4 +7,6 @@ describe LabelLink do
it { is_expected.to belong_to(:label) } it { is_expected.to belong_to(:label) }
it { is_expected.to belong_to(:target) } it { is_expected.to belong_to(:target) }
it_behaves_like 'a BulkInsertSafe model', LabelLink
end end
...@@ -6,6 +6,8 @@ describe MergeRequestDiffCommit do ...@@ -6,6 +6,8 @@ describe MergeRequestDiffCommit do
let(:merge_request) { create(:merge_request) } let(:merge_request) { create(:merge_request) }
let(:project) { merge_request.project } let(:project) { merge_request.project }
it_behaves_like 'a BulkInsertSafe model', MergeRequestDiffCommit
describe '#to_hash' do describe '#to_hash' do
subject { merge_request.commits.first } subject { merge_request.commits.first }
... ...
......
...@@ -3,6 +3,8 @@ ...@@ -3,6 +3,8 @@
require 'spec_helper' require 'spec_helper'
describe MergeRequestDiffFile do describe MergeRequestDiffFile do
it_behaves_like 'a BulkInsertSafe model', MergeRequestDiffFile
describe '#diff' do describe '#diff' do
context 'when diff is not stored' do context 'when diff is not stored' do
let(:unpacked) { 'unpacked' } let(:unpacked) { 'unpacked' }
... ...
......
# 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