Add latest changes from gitlab-org/gitlab@master

app/experiments/application_experiment.rb

@@ -1,14 +1,6 @@
 # frozen_string_literal: true
 
 class ApplicationExperiment < Gitlab::Experiment # rubocop:disable Gitlab/NamespacedClass
-  def enabled?
-    return false if Feature::Definition.get(feature_flag_name).nil? # there has to be a feature flag yaml file
-    return false unless Gitlab.dev_env_or_com? # we have to be in an environment that allows experiments
-
-    # the feature flag has to be rolled out
-    Feature.get(feature_flag_name).state != :off # rubocop:disable Gitlab/AvoidFeatureGet
-  end
-
   def publish(_result = nil)
     super
 
@@ -72,12 +64,4 @@ class ApplicationExperiment < Gitlab::Experiment # rubocop:disable Gitlab/Namesp
     actor = context.try(:actor)
     actor.respond_to?(:id) ? actor : context.try(:user)
   end
-
-  def feature_flag_name
-    name.tr('/', '_')
-  end
-
-  def experiment_group?
-    Feature.enabled?(feature_flag_name, self, type: :experiment, default_enabled: :yaml)
-  end
 end

app/models/ci/namespace_mirror.rb

@@ -15,9 +15,6 @@ module Ci
     end
 
     scope :by_namespace_id, -> (namespace_id) { where(namespace_id: namespace_id) }
-    scope :namespace_id_from_traversal_ids, -> do
-      select('ci_namespace_mirrors.traversal_ids[array_length(ci_namespace_mirrors.traversal_ids, 1)] AS namespace_id')
-    end
 
     class << self
       def sync!(event)

app/models/preloaders/users_max_access_level_in_projects_preloader.rb

@@ -0,0 +1,46 @@
+# frozen_string_literal: true
+
+module Preloaders
+  # This class preloads the max access level (role) for the users within the given projects and
+  # stores the values in requests store via the ProjectTeam class.
+  class UsersMaxAccessLevelInProjectsPreloader
+    def initialize(projects:, users:)
+      @projects = projects
+      @users = users
+    end
+
+    def execute
+      return unless @projects.present? && @users.present?
+
+      access_levels.each do |(project_id, user_id), access_level|
+        project = projects_by_id[project_id]
+
+        project.team.write_member_access_for_user_id(user_id, access_level)
+      end
+    end
+
+    private
+
+    def access_levels
+      ProjectAuthorization
+        .where(project_id: project_ids, user_id: user_ids)
+        .group(:project_id, :user_id)
+        .maximum(:access_level)
+    end
+
+    # Use reselect to override the existing select to prevent
+    # the error `subquery has too many columns`
+    # NotificationsController passes in an Array so we need to check the type
+    def project_ids
+      @projects.is_a?(ActiveRecord::Relation) ? @projects.reselect(:id) : @projects
+    end
+
+    def user_ids
+      @users.is_a?(ActiveRecord::Relation) ? @users.reselect(:id) : @users
+    end
+
+    def projects_by_id
+      @projects_by_id ||= @projects.index_by(&:id)
+    end
+  end
+end

app/models/user.rb

@@ -2230,32 +2230,20 @@ class User < ApplicationRecord
   end
 
   def ci_owned_project_runners_from_group_members
-    cte_project_ids = Gitlab::SQL::CTE.new(
-      :cte_project_ids,
-      Ci::ProjectMirror
-        .select(:project_id)
-        .joins('JOIN ci_namespace_mirrors ON ci_namespace_mirrors.traversal_ids[array_length(ci_namespace_mirrors.traversal_ids, 1)] = ci_project_mirrors.namespace_id')
-        .merge(ci_namespace_mirrors_for_group_members(Gitlab::Access::MAINTAINER))
-    )
-
-    Ci::Runner
+    Ci::RunnerProject
       .select('ci_runners.*')
-      .joins(:runner_projects)
-      .where('ci_runner_projects.project_id IN (SELECT project_id FROM cte_project_ids)')
-      .with(cte_project_ids.to_arel)
+      .joins(:runner)
+      .joins('JOIN ci_project_mirrors ON ci_project_mirrors.project_id = ci_runner_projects.project_id')
+      .joins('JOIN ci_namespace_mirrors ON ci_namespace_mirrors.namespace_id = ci_project_mirrors.namespace_id')
+      .merge(ci_namespace_mirrors_for_group_members(Gitlab::Access::MAINTAINER))
   end
 
   def ci_owned_group_runners
-    cte_namespace_ids = Gitlab::SQL::CTE.new(
-      :cte_namespace_ids,
-      ci_namespace_mirrors_for_group_members(Gitlab::Access::OWNER).namespace_id_from_traversal_ids
-    )
-
-    Ci::Runner
+    Ci::RunnerNamespace
       .select('ci_runners.*')
-      .joins(:runner_namespaces)
-      .where('ci_runner_namespaces.namespace_id IN (SELECT namespace_id FROM cte_namespace_ids)')
-      .with(cte_namespace_ids.to_arel)
+      .joins(:runner)
+      .joins('JOIN ci_namespace_mirrors ON ci_namespace_mirrors.namespace_id = ci_runner_namespaces.namespace_id')
+      .merge(ci_namespace_mirrors_for_group_members(Gitlab::Access::OWNER))
   end
 
   def ci_namespace_mirrors_for_group_members(level)

config/initializers/gitlab_experiment.rb

@@ -23,9 +23,7 @@ Gitlab::Experiment.configure do |config|
   # Customize the logic of our default rollout, which shouldn't include
   # assigning the control yet -- we specifically set it to false for now.
   #
-  config.default_rollout = Gitlab::Experiment::Rollout::Percent.new(
-    include_control: false
-  )
+  config.default_rollout = Gitlab::Experiment::Rollout::Feature.new
 
   # Mount the engine and middleware at a gitlab friendly style path.
   #

doc/api/graphql/reference/index.md

@@ -9675,6 +9675,7 @@ Represents a DAST profile schedule.
 | <a id="dastprofileschedulecadence"></a>`cadence` | [`DastProfileCadence`](#dastprofilecadence) | Cadence of the DAST profile schedule. |
 | <a id="dastprofilescheduleid"></a>`id` | [`DastProfileScheduleID!`](#dastprofilescheduleid) | ID of the DAST profile schedule. |
 | <a id="dastprofileschedulenextrunat"></a>`nextRunAt` | [`Time`](#time) | Next run time of the DAST profile schedule in the given timezone. |
+| <a id="dastprofilescheduleownervalid"></a>`ownerValid` | [`Boolean`](#boolean) | Status of the current owner of the DAST profile schedule. |
 | <a id="dastprofileschedulestartsat"></a>`startsAt` | [`Time`](#time) | Start time of the DAST profile schedule in the given timezone. |
 | <a id="dastprofilescheduletimezone"></a>`timezone` | [`String`](#string) | Time zone of the start time of the DAST profile schedule. |
 

doc/user/project/settings/import_export.md

@@ -159,6 +159,8 @@ Imported users can be mapped by their public email addresses on self-managed ins
 for mapping to work correctly.
 - For contributions to be mapped correctly, users must be an existing member of the namespace,
   or they can be added as a member of the project. Otherwise, a supplementary comment is left to mention that the original author and the MRs, notes, or issues that are owned by the importer.
+- Imported users are set as [direct members](../members/index.md)
+  in the imported project.
 
 For project migration imports performed over GitLab.com groups, preserving author information is
 possible through a [professional services engagement](https://about.gitlab.com/services/migration/).

lib/gitlab/experiment/rollout/feature.rb

@@ -0,0 +1,67 @@
+# frozen_string_literal: true
+
+module Gitlab
+  class Experiment
+    module Rollout
+      class Feature < Percent
+        # For this rollout strategy to consider an experiment as enabled, we
+        # must:
+        #
+        # - have a feature flag yaml file that declares it.
+        # - be in an environment that permits it.
+        # - not have rolled out the feature flag at all (no percent of actors,
+        #   no inclusions, etc.)
+        def enabled?
+          return false if ::Feature::Definition.get(feature_flag_name).nil?
+          return false unless Gitlab.dev_env_or_com?
+
+          ::Feature.get(feature_flag_name).state != :off # rubocop:disable Gitlab/AvoidFeatureGet
+        end
+
+        # For assignment we first check to see if our feature flag is enabled
+        # for "self". This is done by calling `#flipper_id` (used behind the
+        # scenes by `Feature`). By default this is our `experiment.id` (or more
+        # specifically, the context key, which is an anonymous SHA generated
+        # using the details of an experiment.
+        #
+        # If the `Feature.enabled?` check is false, we return nil implicitly,
+        # which will assign the control. Otherwise we call super, which will
+        # assign a variant evenly, or based on our provided distribution rules.
+        def execute_assigment
+          super if ::Feature.enabled?(feature_flag_name, self, type: :experiment, default_enabled: :yaml)
+        end
+
+        # NOTE: There's a typo in the name of this method that we'll fix up.
+        alias_method :execute_assignment, :execute_assigment
+
+        # This is what's provided to the `Feature.enabled?` call that will be
+        # used to determine experiment inclusion. An experiment may provide an
+        # override for this method to make the experiment work on user, group,
+        # or projects.
+        #
+        # For example, when running an experiment on a project, you could make
+        # the experiment assignable by project (using chatops) by implementing
+        # a `flipper_id` method in the experiment:
+        #
+        # def flipper_id
+        #   context.project.flipper_id
+        # end
+        #
+        # Or even cleaner, simply delegate it:
+        #
+        # delegate :flipper_id, to: -> { context.project }
+        def flipper_id
+          return experiment.flipper_id if experiment.respond_to?(:flipper_id)
+
+          "Experiment;#{id}"
+        end
+
+        private
+
+        def feature_flag_name
+          experiment.name.tr('/', '_')
+        end
+      end
+    end
+  end
+end

lib/gitlab/graphql/project/dast_profile_connection_extension.rb

@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+module Gitlab
+  module Graphql
+    module Project
+      class DastProfileConnectionExtension < GraphQL::Schema::Field::ConnectionExtension
+        def after_resolve(value:, object:, context:, **rest)
+          preload_authorizations(context[:project_dast_profiles])
+          context[:project_dast_profiles] = nil
+          value
+        end
+
+        def preload_authorizations(dast_profiles)
+          return unless dast_profiles
+
+          projects = dast_profiles.map(&:project)
+          users = dast_profiles.filter_map { |dast_profile| dast_profile.dast_profile_schedule&.owner }
+          Preloaders::UsersMaxAccessLevelInProjectsPreloader.new(projects: projects, users: users).execute
+        end
+      end
+    end
+  end
+end

lib/gitlab/usage/metrics/names_suggestions/generator.rb

@@ -7,7 +7,7 @@ module Gitlab
         class Generator < ::Gitlab::UsageData
           class << self
             def generate(key_path)
-              uncached_data.deep_stringify_keys.dig(*key_path.split('.'))
+              data.deep_stringify_keys.dig(*key_path.split('.'))
             end
 
             def add_metric(metric, time_frame: 'none', options: {})

lib/gitlab/usage_data.rb

@@ -42,7 +42,11 @@ module Gitlab
       include Gitlab::Usage::TimeFrame
 
       def data
-        uncached_data
+        clear_memoized
+
+        with_finished_at(:recording_ce_finished_at) do
+          usage_data_metrics
+        end
       end
 
       def license_usage_data
@@ -683,14 +687,6 @@ module Gitlab
 
       private
 
-      def uncached_data
-        clear_memoized
-
-        with_finished_at(:recording_ce_finished_at) do
-          usage_data_metrics
-        end
-      end
-
       def stage_manage_events(time_period)
         if time_period.empty?
           Gitlab::Utils::UsageData::FALLBACK

lib/gitlab/usage_data_counters/known_events/common.yml

@@ -383,3 +383,8 @@
   redis_slot: geo
   aggregation: daily
   feature_flag: track_geo_proxy_events
+# Growth
+- name: users_clicking_registration_features_offer
+  category: growth
+  redis_slot: users
+  aggregation: weekly

spec/experiments/application_experiment_spec.rb

@@ -24,38 +24,6 @@ RSpec.describe ApplicationExperiment, :experiment do
     expect { experiment('namespaced/stub') { } }.not_to raise_error
   end
 
-  describe "#enabled?" do
-    before do
-      allow(application_experiment).to receive(:enabled?).and_call_original
-
-      allow(Feature::Definition).to receive(:get).and_return('_instance_')
-      allow(Gitlab).to receive(:dev_env_or_com?).and_return(true)
-      allow(Feature).to receive(:get).and_return(double(state: :on))
-    end
-
-    it "is enabled when all criteria are met" do
-      expect(application_experiment).to be_enabled
-    end
-
-    it "isn't enabled if the feature definition doesn't exist" do
-      expect(Feature::Definition).to receive(:get).with('namespaced_stub').and_return(nil)
-
-      expect(application_experiment).not_to be_enabled
-    end
-
-    it "isn't enabled if we're not in dev or dotcom environments" do
-      expect(Gitlab).to receive(:dev_env_or_com?).and_return(false)
-
-      expect(application_experiment).not_to be_enabled
-    end
-
-    it "isn't enabled if the feature flag state is :off" do
-      expect(Feature).to receive(:get).with('namespaced_stub').and_return(double(state: :off))
-
-      expect(application_experiment).not_to be_enabled
-    end
-  end
-
   describe "#publish" do
     let(:should_track) { true }
 
@@ -214,26 +182,6 @@ RSpec.describe ApplicationExperiment, :experiment do
       )
     end
 
-    it "tracks the event correctly even when using the base class" do
-      subject = Gitlab::Experiment.new(:unnamed)
-      subject.track(:action, context: [fake_context])
-
-      expect_snowplow_event(
-        category: 'unnamed',
-        action: 'action',
-        context: [
-          {
-            schema: 'iglu:com.gitlab/fake/jsonschema/0-0-0',
-            data: { data: '_data_' }
-          },
-          {
-            schema: 'iglu:com.gitlab/gitlab_experiment/jsonschema/1-0-0',
-            data: { experiment: 'unnamed', key: subject.context.key, variant: 'control' }
-          }
-        ]
-      )
-    end
-
     context "when using known context resources" do
       let(:user) { build(:user, id: non_existing_record_id) }
       let(:project) { build(:project, id: non_existing_record_id) }
@@ -347,23 +295,15 @@ RSpec.describe ApplicationExperiment, :experiment do
   end
 
   context "when resolving variants" do
-    it "uses the default value as specified in the yaml" do
-      expect(Feature).to receive(:enabled?).with('namespaced_stub', application_experiment, type: :experiment, default_enabled: :yaml)
-
-      expect(application_experiment.variant.name).to eq('control')
+    before do
+      stub_feature_flags(namespaced_stub: true)
     end
 
-    context "when rolled out to 100%" do
-      before do
-        stub_feature_flags(namespaced_stub: true)
-      end
+    it "returns an assigned name" do
+      application_experiment.variant(:variant1) {}
+      application_experiment.variant(:variant2) {}
 
-      it "returns an assigned name" do
-        application_experiment.variant(:variant1) {}
-        application_experiment.variant(:variant2) {}
-
-        expect(application_experiment.variant.name).to eq('variant2')
-      end
+      expect(application_experiment.assigned.name).to eq('variant2')
     end
   end
 

spec/lib/gitlab/experiment/rollout/feature_spec.rb

@@ -0,0 +1,75 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Experiment::Rollout::Feature, :experiment do
+  subject { described_class.new.for(subject_experiment) }
+
+  let(:subject_experiment) { experiment('namespaced/stub') }
+
+  describe "#enabled?" do
+    before do
+      allow(Feature::Definition).to receive(:get).and_return('_instance_')
+      allow(Gitlab).to receive(:dev_env_or_com?).and_return(true)
+      allow(Feature).to receive(:get).and_return(double(state: :on))
+    end
+
+    it "is enabled when all criteria are met" do
+      expect(subject).to be_enabled
+    end
+
+    it "isn't enabled if the feature definition doesn't exist" do
+      expect(Feature::Definition).to receive(:get).with('namespaced_stub').and_return(nil)
+
+      expect(subject).not_to be_enabled
+    end
+
+    it "isn't enabled if we're not in dev or dotcom environments" do
+      expect(Gitlab).to receive(:dev_env_or_com?).and_return(false)
+
+      expect(subject).not_to be_enabled
+    end
+
+    it "isn't enabled if the feature flag state is :off" do
+      expect(Feature).to receive(:get).with('namespaced_stub').and_return(double(state: :off))
+
+      expect(subject).not_to be_enabled
+    end
+  end
+
+  describe "#execute_assignment" do
+    before do
+      allow(Feature).to receive(:enabled?).with('namespaced_stub', any_args).and_return(true)
+    end
+
+    it "uses the default value as specified in the yaml" do
+      expect(Feature).to receive(:enabled?).with(
+        'namespaced_stub',
+        subject,
+        type: :experiment,
+        default_enabled: :yaml
+      ).and_return(false)
+
+      expect(subject.execute_assignment).to be_nil
+    end
+
+    it "returns an assigned name" do
+      allow(subject).to receive(:behavior_names).and_return([:variant1, :variant2])
+
+      expect(subject.execute_assignment).to eq(:variant2)
+    end
+  end
+
+  describe "#flipper_id" do
+    it "returns the expected flipper id if the experiment doesn't provide one" do
+      subject.instance_variable_set(:@experiment, double(id: '__id__'))
+      expect(subject.flipper_id).to eq('Experiment;__id__')
+    end
+
+    it "lets the experiment provide a flipper id so it can override the default" do
+      allow(subject_experiment).to receive(:flipper_id).and_return('_my_overridden_id_')
+
+      expect(subject.flipper_id).to eq('_my_overridden_id_')
+    end
+  end
+end

spec/lib/gitlab/import_export/all_models.yml

@@ -529,6 +529,7 @@ project:
 - vulnerability_feedback
 - vulnerability_identifiers
 - vulnerability_scanners
+- dast_profiles
 - dast_site_profiles
 - dast_scanner_profiles
 - dast_sites

spec/lib/gitlab/usage_data_counters/hll_redis_counter_spec.rb

@@ -49,7 +49,8 @@ RSpec.describe Gitlab::UsageDataCounters::HLLRedisCounter, :clean_gitlab_redis_s
         'secure',
         'importer',
         'network_policies',
-        'geo'
+        'geo',
+        'growth'
       )
     end
   end

spec/models/preloaders/users_max_access_level_in_projects_preloader_spec.rb

@@ -0,0 +1,51 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+RSpec.describe Preloaders::UsersMaxAccessLevelInProjectsPreloader do
+  let_it_be(:user1) { create(:user) }
+  let_it_be(:user2) { create(:user) }
+
+  let_it_be(:project_1) { create(:project) }
+  let_it_be(:project_2) { create(:project) }
+  let_it_be(:project_3) { create(:project) }
+
+  let(:projects) { [project_1, project_2, project_3] }
+  let(:users) { [user1, user2] }
+
+  before do
+    project_1.add_developer(user1)
+    project_1.add_developer(user2)
+
+    project_2.add_developer(user1)
+    project_2.add_developer(user2)
+
+    project_3.add_developer(user1)
+    project_3.add_developer(user2)
+  end
+
+  context 'preload maximum access level to avoid querying project_authorizations', :request_store do
+    it 'avoids N+1 queries', :request_store do
+      Preloaders::UsersMaxAccessLevelInProjectsPreloader.new(projects: projects, users: users).execute
+
+      expect(count_queries).to eq(0)
+    end
+
+    it 'runs N queries without preloading' do
+      query_count_without_preload = count_queries
+
+      Preloaders::UsersMaxAccessLevelInProjectsPreloader.new(projects: projects, users: users).execute
+      count_queries_with_preload = count_queries
+
+      expect(count_queries_with_preload).to be < query_count_without_preload
+    end
+  end
+
+  def count_queries
+    ActiveRecord::QueryRecorder.new do
+      projects.each do |project|
+        user1.can?(:read_project, project)
+        user2.can?(:read_project, project)
+      end
+    end.count
+  end
+end

spec/support/import_export/import_export.yml

@@ -28,4 +28,4 @@ excluded_attributes:
     - :iid
   project:
     - :id
-    - :created_at
+    - :created_at