Merge branch 'vij-fix-inconsistent-branch-names' into 'master'

Modify editor patch branch name generation

See merge request gitlab-org/gitlab!46044

app/assets/javascripts/ide/stores/modules/commit/getters.js

@@ -14,6 +14,8 @@ const createTranslatedTextForFiles = (files, text) => {
 export const discardDraftButtonDisabled = state =>
   state.commitMessage === '' || state.submitCommitLoading;
 
+// Note: If changing the structure of the placeholder branch name, please also
+// update #patch_branch_name in app/helpers/tree_helper.rb
 export const placeholderBranchName = (state, _, rootState) =>
   `${gon.current_username}-${rootState.currentBranchId}-patch-${`${new Date().getTime()}`.substr(
     -BRANCH_SUFFIX_COUNT,

app/helpers/time_helper.rb

@@ -32,4 +32,8 @@ module TimeHelper
       "%02d:%02d:%02d" % [hours, minutes, seconds]
     end
   end
+
+  def time_in_milliseconds
+    (Time.now.to_f * 1000).to_i
+  end
 end

app/helpers/tree_helper.rb

@@ -75,11 +75,27 @@ module TreeHelper
     if user_access(project).can_push_to_branch?(ref)
       ref
     else
-      project = tree_edit_project(project)
-      project.repository.next_branch('patch')
+      patch_branch_name(ref)
     end
   end
 
+  # Generate a patch branch name that should look like:
+  # `username-branchname-patch-epoch`
+  # where `epoch` is the last 5 digits of the time since epoch (in
+  # milliseconds)
+  #
+  # Note: this correlates with how the WebIDE formats the branch name
+  # and if this implementation changes, so should the `placeholderBranchName`
+  # definition in app/assets/javascripts/ide/stores/modules/commit/getters.js
+  def patch_branch_name(ref)
+    return unless current_user
+
+    username = current_user.username
+    epoch = time_in_milliseconds.to_s.last(5)
+
+    "#{username}-#{ref}-patch-#{epoch}"
+  end
+
   def tree_edit_project(project = @project)
     if can?(current_user, :push_code, project)
       project

changelogs/unreleased/vij-fix-inconsistent-branch-names.yml

+---
+title: Fix single file editor patch branch name
+merge_request: 46044
+author:
+type: fixed

spec/features/projects/blobs/edit_spec.rb

@@ -179,12 +179,14 @@ RSpec.describe 'Editing file blob', :js do
       end
 
       context 'with protected branch' do
-        before do
-          visit project_edit_blob_path(project, tree_join(protected_branch, file_path))
-        end
-
         it 'shows blob editor with patch branch' do
-          expect(find('.js-branch-name').value).to eq('patch-1')
+          freeze_time do
+            visit project_edit_blob_path(project, tree_join(protected_branch, file_path))
+
+            epoch = Time.now.strftime('%s%L').last(5)
+
+            expect(find('.js-branch-name').value).to eq "#{user.username}-protected-branch-patch-#{epoch}"
+          end
         end
       end
     end

spec/helpers/time_helper_spec.rb

@@ -37,4 +37,14 @@ RSpec.describe TimeHelper do
       it { expect(duration_in_numbers(duration)).to eq formatted_string }
     end
   end
+
+  describe "#time_in_milliseconds" do
+    it "returns the time in milliseconds" do
+      freeze_time do
+        time = (Time.now.to_f * 1000).to_i
+
+        expect(time_in_milliseconds).to eq time
+      end
+    end
+  end
 end

spec/helpers/tree_helper_spec.rb

@@ -7,6 +7,8 @@ RSpec.describe TreeHelper do
   let(:repository) { project.repository }
   let(:sha) { 'c1c67abbaf91f624347bb3ae96eabe3a1b742478' }
 
+  let_it_be(:user) { create(:user) }
+
   def create_file(filename)
     project.repository.create_file(
       project.creator,
@@ -219,7 +221,6 @@ RSpec.describe TreeHelper do
     context 'user does not have write access but a personal fork exists' do
       include ProjectForksHelper
 
-      let_it_be(:user) { create(:user) }
       let(:forked_project) { create(:project, :repository, namespace: user.namespace) }
 
       before do
@@ -277,8 +278,6 @@ RSpec.describe TreeHelper do
     end
 
     context 'user has write access' do
-      let_it_be(:user) { create(:user) }
-
       before do
         project.add_developer(user)
 
@@ -314,8 +313,6 @@ RSpec.describe TreeHelper do
     end
 
     context 'gitpod feature is enabled' do
-      let_it_be(:user) { create(:user) }
-
       before do
         stub_feature_flags(gitpod: true)
         allow(Gitlab::CurrentSettings)
@@ -358,4 +355,28 @@ RSpec.describe TreeHelper do
       end
     end
   end
+
+  describe '.patch_branch_name' do
+    before do
+      allow(helper).to receive(:current_user).and_return(user)
+    end
+
+    subject { helper.patch_branch_name('master') }
+
+    it 'returns a patch branch name' do
+      freeze_time do
+        epoch = Time.now.strftime('%s%L').last(5)
+
+        expect(subject).to eq "#{user.username}-master-patch-#{epoch}"
+      end
+    end
+
+    context 'without a current_user' do
+      let(:user) { nil }
+
+      it 'returns nil' do
+        expect(subject).to be nil
+      end
+    end
+  end
 end