From 35144b7075d343a8b3676708da03986103ed5dd1 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Mon, 18 Jan 2021 18:10:54 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .../components/gfm_autocomplete/utils.js | 9 ++++++ ...i_testing_web_performance_widget_total.yml | 8 +++++ lib/gitlab/experimentation.rb | 31 ++----------------- .../experimentation/controller_concern.rb | 5 +-- lib/gitlab/experimentation/experiment.rb | 3 +- .../known_events/common.yml | 5 +++ .../components/gfm_autocomplete/utils_spec.js | 28 +++++++++++++++++ .../controller_concern_spec.rb | 27 ---------------- .../gitlab/experimentation/experiment_spec.rb | 3 +- spec/lib/gitlab/experimentation_spec.rb | 12 ++----- 10 files changed, 58 insertions(+), 73 deletions(-) create mode 100644 config/feature_flags/development/usage_data_i_testing_web_performance_widget_total.yml diff --git a/app/assets/javascripts/vue_shared/components/gfm_autocomplete/utils.js b/app/assets/javascripts/vue_shared/components/gfm_autocomplete/utils.js index ab0fe21cb99..809932b0f29 100644 --- a/app/assets/javascripts/vue_shared/components/gfm_autocomplete/utils.js +++ b/app/assets/javascripts/vue_shared/components/gfm_autocomplete/utils.js @@ -9,6 +9,8 @@ const memberLimit = 10; const nonWordOrInteger = /\W|^\d+$/; +export const menuItemLimit = 100; + export const GfmAutocompleteType = { Emojis: 'emojis', Issues: 'issues', @@ -31,6 +33,7 @@ export const tributeConfig = { config: { trigger: ':', lookup: (value) => value, + menuItemLimit, menuItemTemplate: ({ original }) => `${original} ${Emoji.glEmojiTag(original)}`, selectTemplate: ({ original }) => `:${original}:`, }, @@ -40,6 +43,7 @@ export const tributeConfig = { config: { trigger: '#', lookup: (value) => `${value.iid}${value.title}`, + menuItemLimit, menuItemTemplate: ({ original }) => `${original.reference || original.iid} ${escape(original.title)}`, selectTemplate: ({ original }) => original.reference || `#${original.iid}`, @@ -50,6 +54,7 @@ export const tributeConfig = { config: { trigger: '~', lookup: 'title', + menuItemLimit, menuItemTemplate: ({ original }) => ` ${escape(original.title)}`, @@ -132,6 +137,7 @@ export const tributeConfig = { config: { trigger: '!', lookup: (value) => `${value.iid}${value.title}`, + menuItemLimit, menuItemTemplate: ({ original }) => `${original.reference || original.iid} ${escape(original.title)}`, selectTemplate: ({ original }) => original.reference || `!${original.iid}`, @@ -142,6 +148,7 @@ export const tributeConfig = { config: { trigger: '%', lookup: 'title', + menuItemLimit, menuItemTemplate: ({ original }) => escape(original.title), selectTemplate: ({ original }) => `%"${escape(original.title)}"`, }, @@ -152,6 +159,7 @@ export const tributeConfig = { trigger: '/', fillAttr: 'name', lookup: (value) => `${value.name}${value.aliases.join()}`, + menuItemLimit, menuItemTemplate: ({ original }) => { const aliases = original.aliases.length ? `(or /${original.aliases.join(', /')})` @@ -180,6 +188,7 @@ export const tributeConfig = { trigger: '$', fillAttr: 'id', lookup: (value) => `${value.id}${value.title}`, + menuItemLimit, menuItemTemplate: ({ original }) => `${original.id} ${escape(original.title)}`, }, }, diff --git a/config/feature_flags/development/usage_data_i_testing_web_performance_widget_total.yml b/config/feature_flags/development/usage_data_i_testing_web_performance_widget_total.yml new file mode 100644 index 00000000000..6efb6bea040 --- /dev/null +++ b/config/feature_flags/development/usage_data_i_testing_web_performance_widget_total.yml @@ -0,0 +1,8 @@ +--- +name: usage_data_i_testing_web_performance_widget_total +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/46746 +rollout_issue_url: +milestone: '13.8' +type: development +group: group::testing +default_enabled: true diff --git a/lib/gitlab/experimentation.rb b/lib/gitlab/experimentation.rb index 9fa49cd7c70..196203211ed 100644 --- a/lib/gitlab/experimentation.rb +++ b/lib/gitlab/experimentation.rb @@ -6,7 +6,6 @@ # Experiment options: # - tracking_category (optional, used to set the category when tracking an experiment event) # - use_backwards_compatible_subject_index (optional, set this to true if you need backwards compatibility -- you likely do not need this, see note in the next paragraph.) -# - rollout_strategy: default is `:cookie` based rollout. We may also set it to `:user` based rollout # # Using the backwards-compatible subject index (use_backwards_compatible_subject_index option): # This option was added when [the calculation of experimentation_subject_index was changed](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/45733/diffs#41af4a6fa5a10c7068559ce21c5188483751d934_157_173). It is not intended to be used by new experiments, it exists merely for the segmentation integrity of in-flight experiments at the time the change was deployed. That is, we want users who were assigned to the "experimental" group or the "control" group before the change to still be in those same groups after the change. See [the original issue](https://gitlab.com/gitlab-org/gitlab/-/issues/270858) and [this related comment](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/48110#note_458223745) for more information. @@ -93,19 +92,16 @@ module Gitlab tracking_category: 'Growth::Conversion::Experiment::TrialDuringSignup' }, ci_syntax_templates: { - tracking_category: 'Growth::Activation::Experiment::CiSyntaxTemplates', - rollout_strategy: :user + tracking_category: 'Growth::Activation::Experiment::CiSyntaxTemplates' }, pipelines_empty_state: { - tracking_category: 'Growth::Activation::Experiment::PipelinesEmptyState', - rollout_strategy: :user + tracking_category: 'Growth::Activation::Experiment::PipelinesEmptyState' }, invite_members_new_dropdown: { tracking_category: 'Growth::Expansion::Experiment::InviteMembersNewDropdown' }, show_trial_status_in_sidebar: { - tracking_category: 'Growth::Conversion::Experiment::ShowTrialStatusInSidebar', - rollout_strategy: :group + tracking_category: 'Growth::Conversion::Experiment::ShowTrialStatusInSidebar' }, trial_onboarding_issues: { tracking_category: 'Growth::Conversion::Experiment::TrialOnboardingIssues' @@ -129,7 +125,6 @@ module Gitlab def in_experiment_group?(experiment_key, subject:) return false if subject.blank? return false unless active?(experiment_key) - raise 'Subject must conform to the rollout strategy' unless valid_subject_for_rollout_strategy?(experiment_key, subject) experiment = get_experiment(experiment_key) return false unless experiment @@ -137,26 +132,6 @@ module Gitlab experiment.enabled_for_index?(index_for_subject(experiment, subject)) end - def rollout_strategy(experiment_key) - experiment = get_experiment(experiment_key) - return unless experiment - - experiment.rollout_strategy - end - - def valid_subject_for_rollout_strategy?(experiment_key, subject) - case rollout_strategy(experiment_key) - when :user - subject.is_a?(User) - when :group - subject.is_a?(Group) - when :cookie - subject.nil? || subject.is_a?(String) - else - false - end - end - private def index_for_subject(experiment, subject) diff --git a/lib/gitlab/experimentation/controller_concern.rb b/lib/gitlab/experimentation/controller_concern.rb index e76f33c2250..e43f3c8c007 100644 --- a/lib/gitlab/experimentation/controller_concern.rb +++ b/lib/gitlab/experimentation/controller_concern.rb @@ -39,7 +39,6 @@ module Gitlab def experiment_enabled?(experiment_key, subject: nil) return true if forced_enabled?(experiment_key) return false if dnt_enabled? - raise "Subject must conform to the rollout strategy for #{experiment_key}" unless Experimentation.valid_subject_for_rollout_strategy?(experiment_key, subject) subject ||= fallback_experimentation_subject_index(experiment_key) @@ -66,9 +65,7 @@ module Gitlab return if dnt_enabled? return unless Experimentation.active?(experiment_key) && current_user - subject = Experimentation.rollout_strategy(experiment_key) == :cookie ? nil : current_user - - ::Experiment.add_user(experiment_key, tracking_group(experiment_key, nil, subject: subject), current_user, context) + ::Experiment.add_user(experiment_key, tracking_group(experiment_key, nil, subject: current_user), current_user, context) end def record_experiment_conversion_event(experiment_key) diff --git a/lib/gitlab/experimentation/experiment.rb b/lib/gitlab/experimentation/experiment.rb index 17dda45f5b7..36cd673a38f 100644 --- a/lib/gitlab/experimentation/experiment.rb +++ b/lib/gitlab/experimentation/experiment.rb @@ -5,13 +5,12 @@ module Gitlab class Experiment FEATURE_FLAG_SUFFIX = "_experiment_percentage" - attr_reader :key, :tracking_category, :use_backwards_compatible_subject_index, :rollout_strategy + attr_reader :key, :tracking_category, :use_backwards_compatible_subject_index def initialize(key, **params) @key = key @tracking_category = params[:tracking_category] @use_backwards_compatible_subject_index = params[:use_backwards_compatible_subject_index] - @rollout_strategy = params[:rollout_strategy] || :cookie end def active? diff --git a/lib/gitlab/usage_data_counters/known_events/common.yml b/lib/gitlab/usage_data_counters/known_events/common.yml index e478f0c75ed..4cbde0c0372 100644 --- a/lib/gitlab/usage_data_counters/known_events/common.yml +++ b/lib/gitlab/usage_data_counters/known_events/common.yml @@ -263,6 +263,11 @@ redis_slot: testing aggregation: weekly feature_flag: usage_data_i_testing_full_code_quality_report_total +- name: i_testing_web_performance_widget_total + category: testing + redis_slot: testing + aggregation: weekly + feature_flag: usage_data_i_testing_web_performance_widget_total # Project Management group - name: g_project_management_issue_title_changed category: issues_edit diff --git a/spec/frontend/vue_shared/components/gfm_autocomplete/utils_spec.js b/spec/frontend/vue_shared/components/gfm_autocomplete/utils_spec.js index 65ab4fc44a5..7ec3fbd4e3b 100644 --- a/spec/frontend/vue_shared/components/gfm_autocomplete/utils_spec.js +++ b/spec/frontend/vue_shared/components/gfm_autocomplete/utils_spec.js @@ -14,6 +14,10 @@ describe('gfm_autocomplete/utils', () => { expect(emojisConfig.lookup(emoji)).toBe(emoji); }); + it('limits the number of rendered items to 100', () => { + expect(emojisConfig.menuItemLimit).toBe(100); + }); + it('shows the emoji name and icon in the menu item', () => { expect(emojisConfig.menuItemTemplate({ original: emoji })).toMatchSnapshot(); }); @@ -47,6 +51,10 @@ describe('gfm_autocomplete/utils', () => { ); }); + it('limits the number of rendered items to 100', () => { + expect(issuesConfig.menuItemLimit).toBe(100); + }); + it('shows the reference and title in the menu item within a group context', () => { expect(issuesConfig.menuItemTemplate({ original: groupContextIssue })).toMatchSnapshot(); }); @@ -98,6 +106,10 @@ describe('gfm_autocomplete/utils', () => { expect(labelsConfig.lookup).toBe('title'); }); + it('limits the number of rendered items to 100', () => { + expect(labelsConfig.menuItemLimit).toBe(100); + }); + it('shows the title in the menu item', () => { expect(labelsConfig.menuItemTemplate({ original: label })).toMatchSnapshot(); }); @@ -291,6 +303,10 @@ describe('gfm_autocomplete/utils', () => { ); }); + it('limits the number of rendered items to 100', () => { + expect(mergeRequestsConfig.menuItemLimit).toBe(100); + }); + it('shows the reference and title in the menu item within a group context', () => { expect( mergeRequestsConfig.menuItemTemplate({ original: groupContextMergeRequest }), @@ -332,6 +348,10 @@ describe('gfm_autocomplete/utils', () => { expect(milestonesConfig.lookup).toBe('title'); }); + it('limits the number of rendered items to 100', () => { + expect(milestonesConfig.menuItemLimit).toBe(100); + }); + it('shows the title in the menu item', () => { expect(milestonesConfig.menuItemTemplate({ original: milestone })).toMatchSnapshot(); }); @@ -368,6 +388,10 @@ describe('gfm_autocomplete/utils', () => { ); }); + it('limits the number of rendered items to 100', () => { + expect(quickActionsConfig.menuItemLimit).toBe(100); + }); + it('shows the name, aliases, params and description in the menu item', () => { expect(quickActionsConfig.menuItemTemplate({ original: quickAction })).toMatchSnapshot(); }); @@ -392,6 +416,10 @@ describe('gfm_autocomplete/utils', () => { expect(snippetsConfig.lookup(snippet)).toBe(`${snippet.id}${snippet.title}`); }); + it('limits the number of rendered items to 100', () => { + expect(snippetsConfig.menuItemLimit).toBe(100); + }); + it('shows the id and title in the menu item', () => { expect(snippetsConfig.menuItemTemplate({ original: snippet })).toMatchSnapshot(); }); diff --git a/spec/lib/gitlab/experimentation/controller_concern_spec.rb b/spec/lib/gitlab/experimentation/controller_concern_spec.rb index 1cebe37bea5..c47f71c207d 100644 --- a/spec/lib/gitlab/experimentation/controller_concern_spec.rb +++ b/spec/lib/gitlab/experimentation/controller_concern_spec.rb @@ -10,10 +10,6 @@ RSpec.describe Gitlab::Experimentation::ControllerConcern, type: :controller do use_backwards_compatible_subject_index: true }, test_experiment: { - tracking_category: 'Team', - rollout_strategy: rollout_strategy - }, - my_experiment: { tracking_category: 'Team' } } @@ -24,7 +20,6 @@ RSpec.describe Gitlab::Experimentation::ControllerConcern, type: :controller do end let(:enabled_percentage) { 10 } - let(:rollout_strategy) { nil } controller(ApplicationController) do include Gitlab::Experimentation::ControllerConcern @@ -122,7 +117,6 @@ RSpec.describe Gitlab::Experimentation::ControllerConcern, type: :controller do end context 'when subject is given' do - let(:rollout_strategy) { :user } let(:user) { build(:user) } it 'uses the subject' do @@ -250,7 +244,6 @@ RSpec.describe Gitlab::Experimentation::ControllerConcern, type: :controller do it "provides the subject's hashed global_id as label" do experiment_subject = double(:subject, to_global_id: 'abc') - allow(Gitlab::Experimentation).to receive(:valid_subject_for_rollout_strategy?).and_return(true) controller.track_experiment_event(:test_experiment, 'start', 1, subject: experiment_subject) @@ -427,26 +420,6 @@ RSpec.describe Gitlab::Experimentation::ControllerConcern, type: :controller do controller.record_experiment_user(:test_experiment, context) end - - context 'with a cookie based rollout strategy' do - it 'calls tracking_group with a nil subject' do - expect(controller).to receive(:tracking_group).with(:test_experiment, nil, subject: nil).and_return(:experimental) - allow(::Experiment).to receive(:add_user).with(:test_experiment, :experimental, user, context) - - controller.record_experiment_user(:test_experiment, context) - end - end - - context 'with a user based rollout strategy' do - let(:rollout_strategy) { :user } - - it 'calls tracking_group with a user subject' do - expect(controller).to receive(:tracking_group).with(:test_experiment, nil, subject: user).and_return(:experimental) - allow(::Experiment).to receive(:add_user).with(:test_experiment, :experimental, user, context) - - controller.record_experiment_user(:test_experiment, context) - end - end end context 'the user is part of the control group' do diff --git a/spec/lib/gitlab/experimentation/experiment_spec.rb b/spec/lib/gitlab/experimentation/experiment_spec.rb index 94dbf1d7e4b..008e6699597 100644 --- a/spec/lib/gitlab/experimentation/experiment_spec.rb +++ b/spec/lib/gitlab/experimentation/experiment_spec.rb @@ -9,8 +9,7 @@ RSpec.describe Gitlab::Experimentation::Experiment do let(:params) do { tracking_category: 'Category1', - use_backwards_compatible_subject_index: true, - rollout_strategy: nil + use_backwards_compatible_subject_index: true } end diff --git a/spec/lib/gitlab/experimentation_spec.rb b/spec/lib/gitlab/experimentation_spec.rb index 71d08903532..b503960b8c7 100644 --- a/spec/lib/gitlab/experimentation_spec.rb +++ b/spec/lib/gitlab/experimentation_spec.rb @@ -102,11 +102,7 @@ RSpec.describe Gitlab::Experimentation do context 'when subject has a global_id' do let(:experiment_subject) { double(:subject, to_global_id: 'z') } - it do - allow(Gitlab::Experimentation).to receive(:valid_subject_for_rollout_strategy?).and_return(true) - - is_expected.to eq(true) - end + it { is_expected.to eq(true) } end context 'when subject is nil' do @@ -154,11 +150,7 @@ RSpec.describe Gitlab::Experimentation do context 'when subject has a global_id' do let(:experiment_subject) { double(:subject, to_global_id: 'abcd') } - it do - allow(Gitlab::Experimentation).to receive(:valid_subject_for_rollout_strategy?).and_return(true) - - is_expected.to eq(true) - end + it { is_expected.to eq(true) } end context 'when subject is nil' do