Merge branch 'jj-add-new-project-readme-experiment' into 'master'

Add new project readme experiment [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!51505

Gemfile

@@ -477,7 +477,7 @@ gem 'flipper', '~> 0.17.1'
 gem 'flipper-active_record', '~> 0.17.1'
 gem 'flipper-active_support_cache_store', '~> 0.17.1'
 gem 'unleash', '~> 0.1.5'
-gem 'gitlab-experiment', '~> 0.4.8'
+gem 'gitlab-experiment', '~> 0.4.9'
 
 # Structured logging
 gem 'lograge', '~> 0.5'

Gemfile.lock

@@ -425,7 +425,7 @@ GEM
     github-markup (1.7.0)
     gitlab-chronic (0.10.5)
       numerizer (~> 0.2)
-    gitlab-experiment (0.4.8)
+    gitlab-experiment (0.4.9)
       activesupport (>= 3.0)
       scientist (~> 1.5, >= 1.5.0)
     gitlab-fog-azure-rm (1.0.0)
@@ -1371,7 +1371,7 @@ DEPENDENCIES
   gitaly (~> 13.8.0.pre.rc3)
   github-markup (~> 1.7.0)
   gitlab-chronic (~> 0.10.5)
-  gitlab-experiment (~> 0.4.8)
+  gitlab-experiment (~> 0.4.9)
   gitlab-fog-azure-rm (~> 1.0)
   gitlab-labkit (= 0.14.0)
   gitlab-license (~> 1.0)

app/controllers/projects_controller.rb

@@ -75,6 +75,7 @@ class ProjectsController < Projects::ApplicationController
     @project = ::Projects::CreateService.new(current_user, project_params(attributes: project_params_create_attributes)).execute
 
     if @project.saved?
+      experiment(:new_project_readme, actor: current_user).track(:created, property: active_new_project_tab)
       redirect_to(
         project_path(@project, custom_import_params),
         notice: _("Project '%{project_name}' was successfully created.") % { project_name: @project.name }

app/experiments/new_project_readme_experiment.rb

+# frozen_string_literal: true
+
+class NewProjectReadmeExperiment < ApplicationExperiment # rubocop:disable Gitlab/NamespacedClass
+  include Gitlab::Git::WrapsGitalyErrors
+
+  INITIAL_WRITE_LIMIT = 3
+  EXPERIMENT_START_DATE = DateTime.parse('2021/1/20')
+  MAX_ACCOUNT_AGE = 7.days
+
+  exclude { context.value[:actor].nil? }
+  exclude { context.actor.created_at < MAX_ACCOUNT_AGE.ago }
+
+  def control_behavior
+    false # we don't want the checkbox to be checked
+  end
+
+  def candidate_behavior
+    true # check the checkbox by default
+  end
+
+  def track_initial_writes(project)
+    return unless should_track? # early return if we don't need to ask for commit counts
+    return unless project.created_at > EXPERIMENT_START_DATE # early return for older projects
+    return unless (commit_count = commit_count_for(project)) < INITIAL_WRITE_LIMIT
+
+    track(:write, property: project.created_at.to_s, value: commit_count)
+  end
+
+  private
+
+  def commit_count_for(project)
+    raw_repo = project.repository&.raw_repository
+    return INITIAL_WRITE_LIMIT unless raw_repo&.root_ref
+
+    begin
+      Gitlab::GitalyClient::CommitService.new(raw_repo).commit_count(raw_repo.root_ref, {
+        all: true, # include all branches
+        max_count: INITIAL_WRITE_LIMIT # limit as an optimization
+      })
+    rescue StandardError => e
+      Gitlab::ErrorTracking.track_exception(e, experiment: name)
+      INITIAL_WRITE_LIMIT
+    end
+  end
+end

app/views/projects/_new_project_fields.html.haml

@@ -54,7 +54,8 @@
   .form-group.row.initialize-with-readme-setting
     %div{ :class => "col-sm-12" }
       .form-check
-        = check_box_tag 'project[initialize_with_readme]', '1', false, class: 'form-check-input qa-initialize-with-readme-checkbox', data: { track_label: "#{track_label}", track_event: "activate_form_input", track_property: "init_with_readme", track_value: "" }
+        - experiment(:new_project_readme, actor: current_user) do |e|
+          = check_box_tag 'project[initialize_with_readme]', '1', e.run, class: 'form-check-input qa-initialize-with-readme-checkbox', data: { track_label: "#{track_label}", track_event: "activate_form_input", track_property: "init_with_readme", track_value: "" }
         = label_tag 'project[initialize_with_readme]', class: 'form-check-label' do
           .option-title
             %strong= s_('ProjectsNew|Initialize repository with a README')

app/workers/post_receive.rb

@@ -2,6 +2,7 @@
 
 class PostReceive # rubocop:disable Scalability/IdempotentWorker
   include ApplicationWorker
+  include Gitlab::Experiment::Dsl
 
   feature_category :source_code_management
   urgency :high
@@ -121,6 +122,7 @@ class PostReceive # rubocop:disable Scalability/IdempotentWorker
   end
 
   def after_project_changes_hooks(project, user, refs, changes)
+    experiment(:new_project_readme, actor: user).track_initial_writes(project)
     repository_update_hook_data = Gitlab::DataBuilder::Repository.update(project, user, changes, refs)
     SystemHooksService.new.execute_hooks(repository_update_hook_data, :repository_update_hooks)
     Gitlab::UsageDataCounters::SourceCodeCounter.count(:pushes)

ee/config/feature_flags/experiment/new_project_readme.yml

+---
+name: new_project_readme
+rollout_issue_url: 
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/51505
+milestone: '13.8'
+type: experiment
+group: group::adoption
+default_enabled: false

spec/controllers/projects_controller_spec.rb

@@ -384,6 +384,29 @@ RSpec.describe ProjectsController do
     end
   end
 
+  describe 'POST create' do
+    let!(:project_params) do
+      {
+        path: 'foo',
+        description: 'bar',
+        namespace_id: user.namespace.id,
+        visibility_level: Gitlab::VisibilityLevel::PUBLIC
+      }
+    end
+
+    before do
+      sign_in(user)
+    end
+
+    it 'tracks a created event for the new_project_readme experiment', :experiment do
+      expect(experiment(:new_project_readme)).to track(:created, property: 'blank').on_any_instance.with_context(
+        actor: user
+      )
+
+      post :create, params: { project: project_params }
+    end
+  end
+
   describe 'POST #archive' do
     let_it_be(:group) { create(:group) }
     let_it_be(:project) { create(:project, group: group) }

spec/experiments/new_project_readme_experiment_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe NewProjectReadmeExperiment, :experiment do
+  subject { described_class.new(actor: actor) }
+
+  let(:actor) { User.new(id: 42, created_at: Time.current) }
+
+  before do
+    stub_experiments(new_project_readme: :control)
+  end
+
+  describe "exclusions" do
+    let(:threshold) { described_class::MAX_ACCOUNT_AGE }
+
+    it { is_expected.to exclude(actor: User.new(created_at: (threshold + 1.minute).ago)) }
+    it { is_expected.not_to exclude(actor: User.new(created_at: (threshold - 1.minute).ago)) }
+  end
+
+  describe "the control behavior" do
+    subject { described_class.new(actor: actor).run(:control) }
+
+    it { is_expected.to be false }
+  end
+
+  describe "the candidate behavior" do
+    subject { described_class.new(actor: actor).run(:candidate) }
+
+    it { is_expected.to be true }
+  end
+
+  context "when tracking initial writes" do
+    let!(:project) { create(:project, :repository) }
+
+    def stub_gitaly_count(count = 1)
+      allow(Gitlab::GitalyClient).to receive(:call).and_call_original
+      allow(Gitlab::GitalyClient).to receive(:call).with(anything, :commit_service, :count_commits, anything, anything)
+        .and_return(double(count: count))
+    end
+
+    before do
+      stub_gitaly_count
+    end
+
+    it "tracks an event for the first commit on a project with a repository" do
+      expect(subject).to receive(:track).with(:write, property: project.created_at.to_s, value: 1).and_call_original
+
+      subject.track_initial_writes(project)
+    end
+
+    it "tracks an event for the second commit on a project with a repository" do
+      stub_gitaly_count(2)
+
+      expect(subject).to receive(:track).with(:write, property: project.created_at.to_s, value: 2).and_call_original
+
+      subject.track_initial_writes(project)
+    end
+
+    it "doesn't track if the repository has more then 2 commits" do
+      stub_gitaly_count(3)
+
+      expect(subject).not_to receive(:track)
+
+      subject.track_initial_writes(project)
+    end
+
+    it "doesn't track when we generally shouldn't" do
+      allow(subject).to receive(:should_track?).and_return(false)
+
+      expect(subject).not_to receive(:track)
+
+      subject.track_initial_writes(project)
+    end
+
+    it "doesn't track if the project is older" do
+      expect(project).to receive(:created_at).and_return(described_class::EXPERIMENT_START_DATE - 1.minute)
+
+      expect(subject).not_to receive(:track)
+
+      subject.track_initial_writes(project)
+    end
+
+    it "handles exceptions by logging them" do
+      allow(Gitlab::GitalyClient).to receive(:call).with(anything, :commit_service, :count_commits, anything, anything)
+        .and_raise(e = StandardError.new('_message_'))
+
+      expect(Gitlab::ErrorTracking).to receive(:track_exception).with(e, experiment: 'new_project_readme')
+
+      subject.track_initial_writes(project)
+    end
+  end
+end

spec/features/projects/user_creates_project_spec.rb

@@ -8,6 +8,8 @@ RSpec.describe 'User creates a project', :js do
   before do
     sign_in(user)
     create(:personal_key, user: user)
+
+    stub_experiments(new_project_readme: :candidate)
   end
 
   it 'creates a new project' do
@@ -16,6 +18,10 @@ RSpec.describe 'User creates a project', :js do
     find('[data-qa-selector="blank_project_link"]').click
     fill_in(:project_name, with: 'Empty')
 
+    # part of the new_project_readme experiment
+    expect(page).to have_checked_field 'Initialize repository with a README'
+    uncheck 'Initialize repository with a README'
+
     page.within('#content-body') do
       click_button('Create project')
     end

spec/support/gitlab_experiment.rb

@@ -6,7 +6,7 @@ require 'gitlab/experiment/rspec'
 # This is a temporary fix until we have a larger discussion around the
 # challenges raised in https://gitlab.com/gitlab-org/gitlab/-/issues/300104
 class ApplicationExperiment < Gitlab::Experiment # rubocop:disable Gitlab/NamespacedClass
-  def initialize(*args)
+  def initialize(name = nil, variant_name = nil, **context)
     super
     Feature.persist_used!(feature_flag_name)
   end

spec/workers/post_receive_spec.rb

@@ -85,6 +85,14 @@ RSpec.describe PostReceive do
 
         perform
       end
+
+      it 'tracks an event for the new_project_readme experiment', :experiment do
+        expect_next_instance_of(NewProjectReadmeExperiment, :new_project_readme, nil, actor: empty_project.owner) do |e|
+          expect(e).to receive(:track_initial_writes).with(empty_project)
+        end
+
+        perform
+      end
     end
 
     shared_examples 'not updating remote mirrors' do