Merge branch 'edit-in-patch-branch' into 'master'

Default target branch to patch-n when editing file in protected branch

Fixes #3441

See merge request !2021

13 files changed, 71 insertions, 46 deletions

diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb
index c2aaf094e68..7d0d57858e0 100644
--- a/app/controllers/projects/application_controller.rb
+++ b/app/controllers/projects/application_controller.rb
@@ -21,7 +21,7 @@ class Projects::ApplicationController < ApplicationController
     unless @repository.branch_names.include?(@ref)
       redirect_to(
         namespace_project_tree_path(@project.namespace, @project, @ref),
-        notice: "This action is not allowed unless you are on top of a branch"
+        notice: "This action is not allowed unless you are on a branch"
       )
     end
   end
diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb
index 31a33bfd237..62163682936 100644
--- a/app/controllers/projects/blob_controller.rb
+++ b/app/controllers/projects/blob_controller.rb
@@ -162,12 +162,20 @@ class Projects::BlobController < Projects::ApplicationController
   end
 
   def sanitized_new_branch_name
-    @new_branch ||= sanitize(strip_tags(params[:new_branch]))
+    sanitize(strip_tags(params[:new_branch]))
   end
 
   def editor_variables
     @current_branch = @ref
-    @new_branch = params[:new_branch].present? ? sanitized_new_branch_name : @ref
+
+    @new_branch =
+      if params[:new_branch].present?
+        sanitized_new_branch_name
+      elsif ::Gitlab::GitAccess.new(current_user, @project).can_push_to_branch?(@ref)
+        @ref
+      else
+        @repository.next_patch_branch
+      end
 
     @file_path =
       if action_name.to_s == 'create'
diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb
index 4a3d971f7c6..68e5d5be600 100644
--- a/app/helpers/blob_helper.rb
+++ b/app/helpers/blob_helper.rb
@@ -30,26 +30,24 @@ module BlobHelper
         nil
       end
 
-    if blob_viewable?(blob)
-      text = 'Edit'
-      after = options[:after] || ''
-      from_mr = options[:from_merge_request_id]
-      link_opts = {}
-      link_opts[:from_merge_request_id] = from_mr if from_mr
-      cls = 'btn btn-small'
-      if allowed_tree_edit?(project, ref)
-        link_to(text,
-                namespace_project_edit_blob_path(project.namespace, project,
-                                                 tree_join(ref, path),
-                                                 link_opts),
-                class: cls
-               )
-      else
-        content_tag :span, text, class: cls + ' disabled'
-      end + after.html_safe
-    else
-      ''
-    end
+    return unless blob && blob.text? && blob_editable?(blob)
+
+    text = 'Edit'
+    after = options[:after] || ''
+    from_mr = options[:from_merge_request_id]
+    link_opts = {}
+    link_opts[:from_merge_request_id] = from_mr if from_mr
+    cls = 'btn btn-small'
+    link_to(text,
+            namespace_project_edit_blob_path(project.namespace, project,
+                                             tree_join(ref, path),
+                                             link_opts),
+            class: cls
+           ) + after.html_safe
+  end
+
+  def blob_editable?(blob, project = @project, ref = @ref)
+    !blob.lfs_pointer? && allowed_tree_edit?(project, ref)
   end
 
   def leave_edit_message
diff --git a/app/helpers/branches_helper.rb b/app/helpers/branches_helper.rb
index d6eaa7d57bc..e39548e17e1 100644
--- a/app/helpers/branches_helper.rb
+++ b/app/helpers/branches_helper.rb
@@ -11,7 +11,7 @@ module BranchesHelper
 
   def can_push_branch?(project, branch_name)
     return false unless project.repository.branch_names.include?(branch_name)
-    
+
     ::Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(branch_name)
   end
 end
diff --git a/app/helpers/tree_helper.rb b/app/helpers/tree_helper.rb
index 6afa1aacc5b..886a1e734b5 100644
--- a/app/helpers/tree_helper.rb
+++ b/app/helpers/tree_helper.rb
@@ -46,16 +46,26 @@ module TreeHelper
     File.join(*args)
   end
 
+  def on_top_of_branch?(project = @project, ref = @ref)
+    project.repository.branch_names.include?(ref)
+  end
+
   def allowed_tree_edit?(project = nil, ref = nil)
     project ||= @project
     ref ||= @ref
-    return false unless project.repository.branch_names.include?(ref)
+    return false unless on_top_of_branch?(project, ref)
 
-    ::Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(ref)
+    can?(current_user, :push_code, project)
   end
 
-  def can_delete_or_replace?(blob)
-    allowed_tree_edit? && !blob.lfs_pointer?
+  def tree_edit_branch(project = @project, ref = @ref)
+    if allowed_tree_edit?(project, ref)
+      if can_push_branch?(project, ref)
+        ref
+      else
+        project.repository.next_patch_branch
+      end
+    end
   end
 
   def tree_breadcrumbs(tree, max_links = 2)
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 1d43307e1e7..1edec52c09e 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -329,6 +329,17 @@ class Repository
     commit(sha)
   end
 
+  def next_patch_branch
+    patch_branch_ids = self.branch_names.map do |n|
+      result = n.match(/\Apatch-([0-9]+)\z/)
+      result[1].to_i if result
+    end.compact
+
+    highest_patch_branch_id = patch_branch_ids.max || 0
+
+    "patch-#{highest_patch_branch_id + 1}"
+  end
+
   # Remove archives older than 2 hours
   def branches_sorted_by(value)
     case value
diff --git a/app/services/files/base_service.rb b/app/services/files/base_service.rb
index f50aaf2eb52..9a67b160940 100644
--- a/app/services/files/base_service.rb
+++ b/app/services/files/base_service.rb
@@ -53,7 +53,7 @@ module Files
 
       unless project.empty_repo?
         unless repository.branch_names.include?(@current_branch)
-          raise_error("You can only create files if you are on top of a branch")
+          raise_error("You can only create or edit files when you are on a branch")
         end
 
         if @current_branch != @target_branch
diff --git a/app/views/projects/blob/_actions.html.haml b/app/views/projects/blob/_actions.html.haml
index 0e54e59e953..b1df8d19938 100644
--- a/app/views/projects/blob/_actions.html.haml
+++ b/app/views/projects/blob/_actions.html.haml
@@ -1,5 +1,4 @@
 .btn-group.tree-btn-group
-  = edit_blob_link(@project, @ref, @path)
   = link_to 'Raw', namespace_project_raw_path(@project.namespace, @project, @id),
       class: 'btn btn-sm', target: '_blank'
   -# only show normal/blame view links for text files
@@ -12,11 +11,16 @@
           class: 'btn btn-sm' unless @blob.empty?
   = link_to 'History', namespace_project_commits_path(@project.namespace, @project, @id),
       class: 'btn btn-sm'
-  - if @ref != @commit.sha
-    = link_to 'Permalink', namespace_project_blob_path(@project.namespace, @project,
-        tree_join(@commit.sha, @path)), class: 'btn btn-sm'
+  = link_to 'Permalink', namespace_project_blob_path(@project.namespace, @project,
+      tree_join(@commit.sha, @path)), class: 'btn btn-sm'
 
-- if can_delete_or_replace?(@blob)
+- if blob_editable?(@blob)
   .btn-group{ role: "group" }
+    = edit_blob_link(@project, @ref, @path)
     %button.btn.btn-default{ 'data-target' => '#modal-upload-blob', 'data-toggle' => 'modal' } Replace
     %button.btn.btn-remove{ 'data-target' => '#modal-remove-blob', 'data-toggle' => 'modal' } Delete
+- elsif !on_top_of_branch?
+  .btn-group{ role: "group" }
+    %button.btn.btn-default.disabled.has_tooltip{title: "You can only edit files when you are on a branch.", data: {container: 'body'}} Edit
+    %button.btn.btn-default.disabled.has_tooltip{title: "You can only replace files when you are on a branch.", data: {container: 'body'}} Replace
+    %button.btn.btn-remove.disabled.has_tooltip{title: "You can only delete files when you are on a branch.", data: {container: 'body'}} Delete
diff --git a/app/views/projects/blob/show.html.haml b/app/views/projects/blob/show.html.haml
index 09d6fc18e3e..3f8d11ed8c8 100644
--- a/app/views/projects/blob/show.html.haml
+++ b/app/views/projects/blob/show.html.haml
@@ -6,7 +6,7 @@
 %div#tree-holder.tree-holder
   = render 'blob', blob: @blob
 
-- if can_delete_or_replace?(@blob)
+- if blob_editable?(@blob)
   = render 'projects/blob/remove'
 
   - title = "Replace #{@blob.name}"
diff --git a/app/views/projects/tree/_tree_header.html.haml b/app/views/projects/tree/_tree_header.html.haml
index 12356dbcb6b..0e1f7076608 100644
--- a/app/views/projects/tree/_tree_header.html.haml
+++ b/app/views/projects/tree/_tree_header.html.haml
@@ -30,3 +30,7 @@
             = link_to '#modal-create-new-dir', { 'data-target' => '#modal-create-new-dir', 'data-toggle' => 'modal'} do
               = icon('folder fw')
               New directory
+  - elsif !on_top_of_branch?
+    %li
+      %span.btn.add-to-tree.disabled.has_tooltip{title: "You can only add files when you are on a branch.", data: {container: 'body'}}
+        = icon('plus')
diff --git a/app/views/shared/_new_commit_form.html.haml b/app/views/shared/_new_commit_form.html.haml
index 55aa045fd59..111219f2064 100644
--- a/app/views/shared/_new_commit_form.html.haml
+++ b/app/views/shared/_new_commit_form.html.haml
@@ -4,7 +4,7 @@
   .form-group.branch
     = label_tag 'new_branch', 'Target branch', class: 'control-label'
     .col-sm-10
-      = text_field_tag 'new_branch', @new_branch || @ref, required: true, class: "form-control js-new-branch"
+      = text_field_tag 'new_branch', @new_branch || tree_edit_branch, required: true, class: "form-control js-new-branch"
 
       .js-create-merge-request-container
         .checkbox
diff --git a/features/project/source/browse_files.feature b/features/project/source/browse_files.feature
index 37f99b37619..439787f2641 100644
--- a/features/project/source/browse_files.feature
+++ b/features/project/source/browse_files.feature
@@ -110,12 +110,6 @@ Feature: Project Source Browse Files
     Given I visit a binary file in the repo
     Then I cannot see the edit button
 
-  Scenario: If I don't have edit permission the edit link is disabled
-    Given public project "Community"
-    And I visit project "Community" source page
-    And I click on ".gitignore" file in repo
-    Then The edit button is disabled
-
   @javascript
   Scenario: I can edit and commit file
     Given I click on ".gitignore" file in repo
diff --git a/features/steps/project/source/browse_files.rb b/features/steps/project/source/browse_files.rb
index 2792174cc93..f2b95764267 100644
--- a/features/steps/project/source/browse_files.rb
+++ b/features/steps/project/source/browse_files.rb
@@ -53,10 +53,6 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
     expect(page).not_to have_link 'edit'
   end
 
-  step 'The edit button is disabled' do
-    expect(page).to have_css '.disabled', text: 'Edit'
-  end
-
   step 'I can edit code' do
     set_new_content
     expect(evaluate_script('blob.editor.getValue()')).to eq new_gitignore_content