Merge branch 'jswain_promote_empty_repo_upload' into 'master'

Promote empty_repo_upload experiment

See merge request gitlab-org/gitlab!73111

app/assets/javascripts/pages/projects/show/index.js

@@ -7,7 +7,7 @@ import initInviteMembersModal from '~/invite_members/init_invite_members_modal';
 import initInviteMembersTrigger from '~/invite_members/init_invite_members_trigger';
 import leaveByUrl from '~/namespaces/leave_by_url';
 import initVueNotificationsDropdown from '~/notifications';
-import { initUploadFileTrigger } from '~/projects/upload_file_experiment';
+import { initUploadFileTrigger } from '~/projects/upload_file';
 import initReadMore from '~/read_more';
 import UserCallout from '~/user_callout';
 import Star from '../../../star';

app/assets/javascripts/projects/details/upload_button.vue

 <script>
 import { GlButton, GlModalDirective } from '@gitlab/ui';
 import UploadBlobModal from '~/repository/components/upload_blob_modal.vue';
-import { trackFileUploadEvent } from '../upload_file_experiment_tracking';
 
 const UPLOAD_BLOB_MODAL_ID = 'details-modal-upload-blob';
 
@@ -30,11 +29,6 @@ export default {
       default: '',
     },
   },
-  methods: {
-    trackOpenModal() {
-      trackFileUploadEvent('click_upload_modal_trigger');
-    },
-  },
   uploadBlobModalId: UPLOAD_BLOB_MODAL_ID,
 };
 </script>
@@ -44,7 +38,6 @@ export default {
       v-gl-modal="$options.uploadBlobModalId"
       icon="upload"
       data-testid="upload-file-button"
-      @click="trackOpenModal"
       >{{ __('Upload File') }}</gl-button
     >
     <upload-blob-modal

app/assets/javascripts/projects/upload_file_experiment.js → app/assets/javascripts/projects/upload_file.js

@@ -4,7 +4,7 @@ import createRouter from '~/repository/router';
 import UploadButton from './details/upload_button.vue';
 
 export const initUploadFileTrigger = () => {
-  const uploadFileTriggerEl = document.querySelector('.js-upload-file-experiment-trigger');
+  const uploadFileTriggerEl = document.querySelector('.js-upload-file-trigger');
 
   if (!uploadFileTriggerEl) return false;
 

app/assets/javascripts/projects/upload_file_experiment_tracking.js

-import ExperimentTracking from '~/experimentation/experiment_tracking';
-
-export const trackFileUploadEvent = (eventName) => {
-  const isEmpty = Boolean(document.querySelector('.project-home-panel.empty-project'));
-  const property = isEmpty ? 'empty' : 'nonempty';
-  const label = 'blob-upload-modal';
-  const FileUploadTracking = new ExperimentTracking('empty_repo_upload', { label, property });
-  FileUploadTracking.event(eventName);
-};

app/assets/javascripts/repository/components/delete_blob_modal.vue

@@ -116,15 +116,14 @@ export default {
         ],
       };
     },
-    /* eslint-disable dot-notation */
     showCreateNewMrToggle() {
-      return this.canPushCode && this.form.fields['branch_name'].value !== this.originalBranch;
+      return this.canPushCode && this.form.fields.branch_name.value !== this.originalBranch;
     },
     formCompleted() {
-      return this.form.fields['commit_message'].value && this.form.fields['branch_name'].value;
+      return this.form.fields.commit_message.value && this.form.fields.branch_name.value;
     },
     showHint() {
-      const splitCommitMessageByLineBreak = this.form.fields['commit_message'].value
+      const splitCommitMessageByLineBreak = this.form.fields.commit_message.value
         .trim()
         .split('\n');
       const [firstLine, ...otherLines] = splitCommitMessageByLineBreak;
@@ -136,7 +135,7 @@ export default {
         otherLines.some((text) => text.length > COMMIT_MESSAGE_BODY_MAX_LENGTH);
 
       return (
-        !this.form.fields['commit_message'].feedback &&
+        !this.form.fields.commit_message.feedback &&
         (hasFirstLineExceedMaxLength || hasOtherLineExceedMaxLength)
       );
     },
@@ -173,9 +172,7 @@ export default {
       <input type="hidden" name="_method" value="delete" />
       <input :value="$options.csrf.token" type="hidden" name="authenticity_token" />
       <template v-if="emptyRepo">
-        <!-- Once "empty_repo_upload_experiment" is made available, will need to add class 'js-branch-name'
-          Follow-up issue: https://gitlab.com/gitlab-org/gitlab/-/issues/335721 -->
-        <input type="hidden" name="branch_name" :value="originalBranch" />
+        <input type="hidden" name="branch_name" :value="originalBranch" class="js-branch-name" />
       </template>
       <template v-else>
         <input type="hidden" name="original_branch" :value="originalBranch" />

app/assets/javascripts/repository/components/upload_blob_modal.vue

@@ -15,7 +15,6 @@ import { ContentTypeMultipartFormData } from '~/lib/utils/headers';
 import { numberToHumanSize } from '~/lib/utils/number_utils';
 import { visitUrl, joinPaths } from '~/lib/utils/url_utility';
 import { __ } from '~/locale';
-import { trackFileUploadEvent } from '~/projects/upload_file_experiment_tracking';
 import UploadDropzone from '~/vue_shared/components/upload_dropzone/upload_dropzone.vue';
 import {
   SECONDARY_OPTIONS_TEXT,
@@ -165,9 +164,6 @@ export default {
         },
       })
         .then((response) => {
-          if (!this.replacePath) {
-            trackFileUploadEvent('click_upload_modal_form_submit');
-          }
           visitUrl(response.data.filePath);
         })
         .catch(() => {

app/controllers/projects_controller.rb

@@ -336,11 +336,6 @@ class ProjectsController < Projects::ApplicationController
     if can?(current_user, :download_code, @project)
       return render 'projects/no_repo' unless @project.repository_exists?
 
-      if @project.can_current_user_push_to_default_branch?
-        property = @project.empty_repo? ? 'empty' : 'nonempty'
-        experiment(:empty_repo_upload, project: @project).track(:view_project_show, property: property)
-      end
-
       render 'projects/empty' if @project.empty_repo?
     else
       if can?(current_user, :read_wiki, @project)

app/presenters/project_presenter.rb

@@ -249,13 +249,10 @@ class ProjectPresenter < Gitlab::View::Presenter::Delegated
     strong_memoize(:upload_anchor_data) do
       next unless can_current_user_push_to_default_branch?
 
-      experiment(:empty_repo_upload, project: project) do |e|
-        e.use {}
-        e.try do
       AnchorData.new(false,
                      statistic_icon('upload') + _('Upload file'),
                      '#modal-upload-blob',
-                         'js-upload-file-experiment-trigger',
+                     'js-upload-file-trigger',
                      nil,
                      nil,
                      {
@@ -267,13 +264,6 @@ class ProjectPresenter < Gitlab::View::Presenter::Delegated
                      }
                     )
     end
-        e.run
-      end
-    end
-  end
-
-  def empty_repo_upload_experiment?
-    upload_anchor_data.present?
   end
 
   def new_file_anchor_data

app/views/projects/empty.html.haml

@@ -77,5 +77,5 @@
               git push -u origin --all
               git push -u origin --tags
 
-- if @project.empty_repo_upload_experiment?
+- if @project.upload_anchor_data.present?
   = render 'projects/blob/upload', title: _('Upload New File'), placeholder: _('Upload New File'), button_title: _('Upload file'), form_path: project_create_blob_path(@project, default_branch_name), ref: default_branch_name, method: :post

app/views/shared/_new_commit_form.html.haml

@@ -5,9 +5,7 @@
 
 - if project.empty_repo?
   - ref = local_assigns[:ref] || @ref
-  - branch_name_class = project.empty_repo_upload_experiment? ? 'js-branch-name' : nil
-
-  = hidden_field_tag 'branch_name', ref, class: branch_name_class
+  = hidden_field_tag 'branch_name', ref, class: 'js-branch-name'
 - else
   - if can?(current_user, :push_code, @project)
     .form-group.row.branch

app/workers/post_receive.rb

@@ -128,7 +128,6 @@ class PostReceive
   end
 
   def after_project_changes_hooks(project, user, refs, changes)
-    experiment(:empty_repo_upload, project: project).track_initial_write
     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)

config/feature_flags/experiment/empty_repo_upload.yml

----
-name: empty_repo_upload
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/52755
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/285296
-milestone: '13.9'
-type: experiment
-group: group::adoption
-default_enabled: false

spec/controllers/projects_controller_spec.rb

@@ -213,21 +213,6 @@ RSpec.describe ProjectsController do
 
       before do
         sign_in(user)
-
-        allow(controller).to receive(:record_experiment_user)
-      end
-
-      context 'when user can push to default branch', :experiment do
-        let(:user) { empty_project.owner }
-
-        it 'creates an "view_project_show" experiment tracking event' do
-          expect(experiment(:empty_repo_upload)).to track(
-            :view_project_show,
-            property: 'empty'
-          ).on_next_instance
-
-          get :show, params: { namespace_id: empty_project.namespace, id: empty_project }
-        end
       end
 
       User.project_views.keys.each do |project_view|

spec/experiments/empty_repo_upload_experiment_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe EmptyRepoUploadExperiment, :experiment do
-  subject { described_class.new(project: project) }
-
-  let(:project) { create(:project, :repository) }
-
-  describe '#track_initial_write' do
-    context 'when experiment is turned on' do
-      before do
-        stub_experiments(empty_repo_upload: :control)
-      end
-
-      it "tracks an event for the first commit on a project" do
-        expect(subject).to receive(:commit_count_for).with(project, max_count: described_class::INITIAL_COMMIT_COUNT, experiment: 'empty_repo_upload').and_return(1)
-
-        expect(subject).to receive(:track).with(:initial_write, project: project).and_call_original
-
-        subject.track_initial_write
-      end
-
-      it "doesn't track an event for projects with a commit count more than 1" do
-        expect(subject).to receive(:commit_count_for).and_return(2)
-
-        expect(subject).not_to receive(:track)
-
-        subject.track_initial_write
-      end
-
-      it "doesn't track if the project is older" do
-        expect(project).to receive(:created_at).and_return(described_class::TRACKING_START_DATE - 1.minute)
-
-        expect(subject).not_to receive(:track)
-
-        subject.track_initial_write
-      end
-    end
-
-    context 'when experiment is turned off' do
-      it "doesn't track when we generally shouldn't" do
-        expect(subject).not_to receive(:track)
-
-        subject.track_initial_write
-      end
-    end
-  end
-end

spec/features/projects/show/user_uploads_files_spec.rb

@@ -44,16 +44,13 @@ RSpec.describe 'Projects > Show > User uploads files' do
     end
   end
 
-  context 'when in the empty_repo_upload experiment' do
-    before do
-      stub_experiments(empty_repo_upload: :candidate)
+  context 'with an empty repo' do
+    let(:project) { create(:project, :empty_repo, creator: user) }
 
+    before do
       visit(project_path(project))
     end
 
-    context 'with an empty repo' do
-      let(:project) { create(:project, :empty_repo, creator: user) }
-
     [true, false].each do |value|
       include_examples 'uploads and commits a new text file via "upload file" button', drop: value
     end
@@ -62,9 +59,12 @@ RSpec.describe 'Projects > Show > User uploads files' do
   context 'with a nonempty repo' do
     let(:project) { create(:project, :repository, creator: user) }
 
+    before do
+      visit(project_path(project))
+    end
+
     [true, false].each do |value|
       include_examples 'uploads and commits a new text file via "upload file" button', drop: value
     end
   end
-  end
 end

spec/frontend/projects/details/upload_button_spec.js

 import { GlButton } from '@gitlab/ui';
 import { shallowMount } from '@vue/test-utils';
 import UploadButton from '~/projects/details/upload_button.vue';
-import { trackFileUploadEvent } from '~/projects/upload_file_experiment_tracking';
 import UploadBlobModal from '~/repository/components/upload_blob_modal.vue';
 
-jest.mock('~/projects/upload_file_experiment_tracking');
-
 const MODAL_ID = 'details-modal-upload-blob';
 
 describe('UploadButton', () => {
@@ -50,10 +47,6 @@ describe('UploadButton', () => {
       wrapper.find(GlButton).vm.$emit('click');
     });
 
-    it('tracks the click_upload_modal_trigger event', () => {
-      expect(trackFileUploadEvent).toHaveBeenCalledWith('click_upload_modal_trigger');
-    });
-
     it('opens the modal', () => {
       expect(glModalDirective).toHaveBeenCalledWith(MODAL_ID);
     });

spec/frontend/projects/upload_file_experiment_tracking_spec.js

-import ExperimentTracking from '~/experimentation/experiment_tracking';
-import { trackFileUploadEvent } from '~/projects/upload_file_experiment_tracking';
-
-jest.mock('~/experimentation/experiment_tracking');
-
-const eventName = 'click_upload_modal_form_submit';
-const fixture = `<a class='js-upload-file-experiment-trigger'></a><div class='project-home-panel empty-project'></div>`;
-
-beforeEach(() => {
-  document.body.innerHTML = fixture;
-});
-
-afterEach(() => {
-  document.body.innerHTML = '';
-});
-
-describe('trackFileUploadEvent', () => {
-  it('initializes ExperimentTracking with the correct tracking event', () => {
-    trackFileUploadEvent(eventName);
-
-    expect(ExperimentTracking.prototype.event).toHaveBeenCalledWith(eventName);
-  });
-
-  it('calls ExperimentTracking with the correct arguments', () => {
-    trackFileUploadEvent(eventName);
-
-    expect(ExperimentTracking).toHaveBeenCalledWith('empty_repo_upload', {
-      label: 'blob-upload-modal',
-      property: 'empty',
-    });
-  });
-
-  it('calls ExperimentTracking with the correct arguments when the project is not empty', () => {
-    document.querySelector('.empty-project').remove();
-
-    trackFileUploadEvent(eventName);
-
-    expect(ExperimentTracking).toHaveBeenCalledWith('empty_repo_upload', {
-      label: 'blob-upload-modal',
-      property: 'nonempty',
-    });
-  });
-});

spec/frontend/repository/components/upload_blob_modal_spec.js

@@ -6,11 +6,9 @@ import waitForPromises from 'helpers/wait_for_promises';
 import createFlash from '~/flash';
 import httpStatusCodes from '~/lib/utils/http_status';
 import { visitUrl } from '~/lib/utils/url_utility';
-import { trackFileUploadEvent } from '~/projects/upload_file_experiment_tracking';
 import UploadBlobModal from '~/repository/components/upload_blob_modal.vue';
 import UploadDropzone from '~/vue_shared/components/upload_dropzone/upload_dropzone.vue';
 
-jest.mock('~/projects/upload_file_experiment_tracking');
 jest.mock('~/flash');
 jest.mock('~/lib/utils/url_utility', () => ({
   visitUrl: jest.fn(),
@@ -162,10 +160,6 @@ describe('UploadBlobModal', () => {
             await waitForPromises();
           });
 
-          it('tracks the click_upload_modal_trigger event when opening the modal', () => {
-            expect(trackFileUploadEvent).toHaveBeenCalledWith('click_upload_modal_form_submit');
-          });
-
           it('redirects to the uploaded file', () => {
             expect(visitUrl).toHaveBeenCalled();
           });
@@ -185,10 +179,6 @@ describe('UploadBlobModal', () => {
             await waitForPromises();
           });
 
-          it('does not track an event', () => {
-            expect(trackFileUploadEvent).not.toHaveBeenCalled();
-          });
-
           it('creates a flash error', () => {
             expect(createFlash).toHaveBeenCalledWith({
               message: 'Error uploading file. Please try again.',

spec/presenters/project_presenter_spec.rb

@@ -567,12 +567,7 @@ RSpec.describe ProjectPresenter do
     end
 
     describe '#upload_anchor_data' do
-      context 'with empty_repo_upload enabled' do
-        before do
-          stub_experiments(empty_repo_upload: :candidate)
-        end
-
-        context 'user can push to branch' do
+      context 'when a user can push to the default branch' do
         before do
           project.add_developer(user)
         end
@@ -592,19 +587,7 @@ RSpec.describe ProjectPresenter do
         end
       end
 
-        context 'user cannot push to branch' do
-          it 'returns nil' do
-            expect(presenter.upload_anchor_data).to be_nil
-          end
-        end
-      end
-
-      context 'with empty_repo_upload disabled' do
-        before do
-          stub_experiments(empty_repo_upload: :control)
-          project.add_developer(user)
-        end
-
+      context 'when the user cannot push to default branch' do
         it 'returns nil' do
           expect(presenter.upload_anchor_data).to be_nil
         end
@@ -666,7 +649,6 @@ RSpec.describe ProjectPresenter do
       context 'for a developer' do
         before do
           project.add_developer(user)
-          stub_experiments(empty_repo_upload: :candidate)
         end
 
         it 'orders the items correctly' do
@@ -680,16 +662,6 @@ RSpec.describe ProjectPresenter do
             a_string_including('CI/CD')
           )
         end
-
-        context 'when not in the upload experiment' do
-          before do
-            stub_experiments(empty_repo_upload: :control)
-          end
-
-          it 'does not include upload button' do
-            expect(empty_repo_statistics_buttons.map(&:label)).not_to start_with(a_string_including('Upload'))
-          end
-        end
       end
     end
 
@@ -781,20 +753,4 @@ RSpec.describe ProjectPresenter do
 
     it { is_expected.to match(/code_quality_walkthrough=true.*template=Code-Quality/) }
   end
-
-  describe 'empty_repo_upload_experiment?' do
-    subject { presenter.empty_repo_upload_experiment? }
-
-    it 'returns false when upload_anchor_data is nil' do
-      allow(presenter).to receive(:upload_anchor_data).and_return(nil)
-
-      expect(subject).to be false
-    end
-
-    it 'returns true when upload_anchor_data exists' do
-      allow(presenter).to receive(:upload_anchor_data).and_return(true)
-
-      expect(subject).to be true
-    end
-  end
 end

spec/workers/post_receive_spec.rb

@@ -91,14 +91,6 @@ RSpec.describe PostReceive do
 
         perform
       end
-
-      it 'tracks an event for the empty_repo_upload experiment', :experiment do
-        expect_next_instance_of(EmptyRepoUploadExperiment) do |e|
-          expect(e).to receive(:track_initial_write)
-        end
-
-        perform
-      end
     end
 
     shared_examples 'not updating remote mirrors' do