Merge branch 'add-directory-feature' into 'master'

Add directory feature button

This MR adds the "Add directory" feature as described in #2557 and splits out the "Create file" and "Upload file" in a dropdown.

### Screenshots

![image](https://gitlab.com/gitlab-org/gitlab-ce/uploads/098b77026508eef222e0f93ec78f34ec/image.png)

![image](https://gitlab.com/gitlab-org/gitlab-ce/uploads/93588b0280ffa2bd44603193feac5bdf/image.png)

![image](https://gitlab.com/gitlab-org/gitlab-ce/uploads/9094a63d41d7cb33e103390545c9c146/image.png)

Added branch capability to upload file form:

![image](https://gitlab.com/gitlab-org/gitlab-ce/uploads/9b3c012a3f2333a5db0e2ceb8e6711ec/image.png)

"Upload file" is no longer accessible from the "Create file" page. Users can now select a target branch in upload file as well.
   
Closes #2799: Fixes a bug where file modes were overwritten after a commit
    
Closes https://github.com/gitlabhq/gitlabhq/issues/8253: Existing files can no longer be overwritten in the "Create file" section.
    
Closes #2557 

See merge request !1453

CHANGELOG

 Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.1.0 (unreleased)
+  - Add support for creating directories from Files page (Stan Hu)
   - Fix bug where transferring a project would result in stale commit links (Stan Hu)
   - Include full path of source and target branch names in New Merge Request page (Stan Hu)
   - Add user preference to view activities as default dashboard (Stan Hu)

Gemfile

@@ -47,7 +47,7 @@ gem "browser", '~> 1.0.0'
 
 # Extracting information from a git repository
 # Provide access to Gitlab::Git library
-gem "gitlab_git", '~> 7.2.17'
+gem "gitlab_git", '~> 7.2.18'
 
 # LDAP Auth
 # GitLab fork with several improvements to original library. For full list of changes

Gemfile.lock

@@ -279,7 +279,7 @@ GEM
       mime-types (~> 1.19)
     gitlab_emoji (0.1.1)
       gemojione (~> 2.0)
-    gitlab_git (7.2.17)
+    gitlab_git (7.2.18)
       activesupport (~> 4.0)
       charlock_holmes (~> 0.6)
       gitlab-linguist (~> 3.0)
@@ -836,7 +836,7 @@ DEPENDENCIES
   gitlab-flowdock-git-hook (~> 1.0.1)
   gitlab-linguist (~> 3.0.1)
   gitlab_emoji (~> 0.1)
-  gitlab_git (~> 7.2.17)
+  gitlab_git (~> 7.2.18)
   gitlab_meta (= 7.0)
   gitlab_omniauth-ldap (~> 1.2.1)
   gollum-lib (~> 4.0.2)

app/assets/javascripts/blob/blob_file_dropzone.js.coffee

@@ -47,6 +47,7 @@ class @BlobFileDropzone
           return
 
         this.on 'sending', (file, xhr, formData) ->
+          formData.append('new_branch', form.find('#new_branch').val())
           formData.append('commit_message', form.find('#commit_message').val())
           return
 

app/controllers/projects/blob_controller.rb

@@ -8,7 +8,7 @@ class Projects::BlobController < Projects::ApplicationController
 
   before_action :require_non_empty_project, except: [:new, :create]
   before_action :authorize_download_code!
-  before_action :authorize_push_code!, only: [:destroy]
+  before_action :authorize_push_code!, only: [:destroy, :create]
   before_action :assign_blob_vars
   before_action :commit, except: [:new, :create]
   before_action :blob, except: [:new, :create]
@@ -25,7 +25,7 @@ class Projects::BlobController < Projects::ApplicationController
     result = Files::CreateService.new(@project, current_user, @commit_params).execute
 
     if result[:status] == :success
-      flash[:notice] = "Your changes have been successfully committed"
+      flash[:notice] = "The changes have been successfully committed"
       respond_to do |format|
         format.html { redirect_to namespace_project_blob_path(@project.namespace, @project, File.join(@target_branch, @file_path)) }
         format.json { render json: { message: "success", filePath: namespace_project_blob_path(@project.namespace, @project, File.join(@target_branch, @file_path)) } }
@@ -34,7 +34,7 @@ class Projects::BlobController < Projects::ApplicationController
       flash[:alert] = result[:message]
       respond_to do |format|
         format.html { render :new }
-        format.json { render json: { message: "failed", filePath: namespace_project_new_blob_path(@project.namespace, @project, @id) } }
+        format.json { render json: { message: "failed", filePath: namespace_project_blob_path(@project.namespace, @project, @id) } }
       end
     end
   end
@@ -154,7 +154,7 @@ class Projects::BlobController < Projects::ApplicationController
 
   def editor_variables
     @current_branch = @ref
-    @target_branch = (sanitized_new_branch_name || @ref)
+    @target_branch = params[:new_branch].present? ? sanitized_new_branch_name : @ref
 
     @file_path =
       if action_name.to_s == 'create'

app/controllers/projects/tree_controller.rb

 # Controller for viewing a repository's file structure
 class Projects::TreeController < Projects::ApplicationController
   include ExtractsPath
+  include ActionView::Helpers::SanitizeHelper
 
   before_action :require_non_empty_project, except: [:new, :create]
   before_action :assign_ref_vars
+  before_action :assign_dir_vars, only: [:create_dir]
   before_action :authorize_download_code!
+  before_action :authorize_push_code!, only: [:create_dir]
 
   def show
     return not_found! unless @repository.commit(@ref)
@@ -26,4 +29,38 @@ class Projects::TreeController < Projects::ApplicationController
       format.js { no_cache_headers }
     end
   end
+
+  def create_dir
+    return not_found! unless @commit_params.values.all?
+
+    begin
+      result = Files::CreateDirService.new(@project, current_user, @commit_params).execute
+      message = result[:message]
+    rescue => e
+      message = e.to_s
+    end
+
+    if result && result[:status] == :success
+      flash[:notice] = "The directory has been successfully created"
+      respond_to do |format|
+        format.html { redirect_to namespace_project_blob_path(@project.namespace, @project, File.join(@new_branch, @dir_name)) }
+      end
+    else
+      flash[:alert] = message
+      respond_to do |format|
+        format.html { redirect_to namespace_project_blob_path(@project.namespace, @project, @new_branch) }
+      end
+    end
+  end
+
+  def assign_dir_vars
+    @new_branch = params[:new_branch].present? ? sanitize(strip_tags(params[:new_branch])) : @ref
+    @dir_name = File.join(@path, params[:dir_name])
+    @commit_params = {
+      file_path: @dir_name,
+      current_branch: @ref,
+      target_branch: @new_branch,
+      commit_message: params[:commit_message],
+    }
+  end
 end

app/models/repository.rb

@@ -373,11 +373,25 @@ class Repository
     @root_ref ||= raw_repository.root_ref
   end
 
-  def commit_file(user, path, content, message, branch)
+  def commit_dir(user, path, message, branch)
     commit_with_hooks(user, branch) do |ref|
-      path[0] = '' if path[0] == '/'
+      committer = user_to_committer(user)
+      options = {}
+      options[:committer] = committer
+      options[:author] = committer
+
+      options[:commit] = {
+        message: message,
+        branch: ref,
+      }
 
-      committer = user_to_comitter(user)
+      raw_repository.mkdir(path, options)
+    end
+  end
+
+  def commit_file(user, path, content, message, branch, update)
+    commit_with_hooks(user, branch) do |ref|
+      committer = user_to_committer(user)
       options = {}
       options[:committer] = committer
       options[:author] = committer
@@ -388,7 +402,8 @@ class Repository
 
       options[:file] = {
         content: content,
-        path: path
+        path: path,
+        update: update
       }
 
       Gitlab::Git::Blob.commit(raw_repository, options)
@@ -397,9 +412,7 @@ class Repository
 
   def remove_file(user, path, message, branch)
     commit_with_hooks(user, branch) do |ref|
-      path[0] = '' if path[0] == '/'
-
-      committer = user_to_comitter(user)
+      committer = user_to_committer(user)
       options = {}
       options[:committer] = committer
       options[:author] = committer
@@ -416,7 +429,7 @@ class Repository
     end
   end
 
-  def user_to_comitter(user)
+  def user_to_committer(user)
     {
       email: user.email,
       name: user.name,

app/services/files/create_dir_service.rb

+require_relative "base_service"
+
+module Files
+  class CreateDirService < Files::BaseService
+    def commit
+      repository.commit_dir(current_user, @file_path, @commit_message, @target_branch)
+    end
+  end
+end

app/services/files/create_service.rb

@@ -3,7 +3,7 @@ require_relative "base_service"
 module Files
   class CreateService < Files::BaseService
     def commit
-      repository.commit_file(current_user, @file_path, @file_content, @commit_message, @target_branch)
+      repository.commit_file(current_user, @file_path, @file_content, @commit_message, @target_branch, false)
     end
 
     def validate

app/services/files/update_service.rb

@@ -3,7 +3,7 @@ require_relative "base_service"
 module Files
   class UpdateService < Files::BaseService
     def commit
-      repository.commit_file(current_user, @file_path, @file_content, @commit_message, @target_branch)
+      repository.commit_file(current_user, @file_path, @file_content, @commit_message, @target_branch, true)
     end
   end
 end

app/services/merge_requests/merge_service.rb

@@ -29,7 +29,7 @@ module MergeRequests
     private
 
     def commit
-      committer = repository.user_to_comitter(current_user)
+      committer = repository.user_to_committer(current_user)
 
       options = {
         message: commit_message,

app/views/projects/blob/_new_dir.html.haml

+#modal-create-new-dir.modal
+  .modal-dialog
+    .modal-content
+      .modal-header
+        %a.close{href: "#", "data-dismiss" => "modal"} ×
+        %h3.page-title Create New Directory
+      .modal-body
+        = form_tag namespace_project_create_dir_path(@project.namespace, @project, @id), method: :post, remote: false, id: 'dir-create-form', class: 'form-horizontal' do
+          .form-group
+            = label_tag :dir_name, 'Directory Name', class: 'control-label'
+            .col-sm-10
+              = text_field_tag :dir_name, params[:dir_name], placeholder: "Directory name", required: true, class: 'form-control'
+          = render 'shared/commit_message_container', params: params, placeholder: ''
+          - unless @project.empty_repo?
+            .form-group
+              = label_tag :branch_name, 'Branch', class: 'control-label'
+              .col-sm-10
+                = text_field_tag 'new_branch', @ref, class: "form-control"
+          .form-group
+            .col-sm-offset-2.col-sm-10
+              = submit_tag "Create directory", class: 'btn btn-primary btn-create'
+              = link_to "Cancel", '#', class: "btn btn-cancel", "data-dismiss" => "modal"
+
+:coffeescript
+  disableButtonIfAnyEmptyField($("#dir-create-form"), ".form-control", ".btn-create");

app/views/projects/blob/_upload.html.haml

@@ -4,9 +4,6 @@
       .modal-header
         %a.close{href: "#", "data-dismiss" => "modal"} ×
         %h3.page-title #{title}
-        %p.light
-          From branch
-          %strong= @ref
       .modal-body
         = form_tag form_path, method: method, class: 'blob-file-upload-form-js form-horizontal' do
           .dropzone
@@ -18,6 +15,12 @@
           .dropzone-alerts{class: "alert alert-danger data", style: "display:none"}
           = render 'shared/commit_message_container', params: params,
             placeholder: placeholder
+          - unless @project.empty_repo?
+            .form-group.branch
+              = label_tag 'branch', class: 'control-label' do
+                Branch
+              .col-sm-10
+                = text_field_tag 'new_branch', @ref, class: "form-control"
           .form-group
             .col-sm-offset-2.col-sm-10
               = button_tag button_title, class: 'btn btn-small btn-primary btn-upload-file', id: 'submit-all'

app/views/projects/blob/new.html.haml

@@ -2,12 +2,7 @@
 = render "header_title"
 
 .gray-content-block.top-block
-  Create a new file or
-  = link_to 'upload', '#modal-upload-blob',
-    { class: 'upload-link', 'data-target' => '#modal-upload-blob', 'data-toggle' => 'modal'}
-  an existing one
-
-= render 'projects/blob/upload', title: 'Upload', placeholder: 'Upload new file', button_title: 'Upload file', form_path: namespace_project_create_blob_path(@project.namespace, @project, @id), method: :post
+  Create a new file
 
 .file-editor
   = form_tag(namespace_project_create_blob_path(@project.namespace, @project, @id), method: :post, class: 'form-horizontal form-new-file js-requires-input') do

app/views/projects/tree/_tree.html.haml

@@ -8,11 +8,25 @@
         = link_to truncate(title, length: 40), namespace_project_tree_path(@project.namespace, @project, path)
       - else
         = link_to title, '#'
-  - if current_user && can_push_branch?(@project, @ref)
+  - if allowed_tree_edit?
     %li
-      = link_to namespace_project_new_blob_path(@project.namespace, @project, @id), title: 'New file', id: 'new-file-link' do
-        %small
-          %i.fa.fa-plus
+      %span.dropdown
+        %a.dropdown-toggle.btn.btn-xs.add-to-tree{href: '#', "data-toggle" => "dropdown"}
+          = icon('plus')
+        %ul.dropdown-menu
+          %li
+            = link_to namespace_project_new_blob_path(@project.namespace, @project, @id), title: 'Create file', id: 'new-file-link' do
+              = icon('pencil fw')
+              Create file
+          %li
+            = link_to '#modal-upload-blob', { 'data-target' => '#modal-upload-blob', 'data-toggle' => 'modal'} do
+              = icon('file fw')
+              Upload file
+          %li.divider
+          %li
+            = link_to '#modal-create-new-dir', { 'data-target' => '#modal-create-new-dir', 'data-toggle' => 'modal'} do
+              = icon('folder fw')
+              New directory
 
 %div#tree-content-holder.tree-content-holder.prepend-top-20
   %table#tree-slider{class: "table_#{@hex_path} tree-table" }
@@ -46,6 +60,10 @@
 
 %div.tree_progress
 
+- if allowed_tree_edit?
+  = render 'projects/blob/upload', title: 'Upload', placeholder: 'Upload new file', button_title: 'Upload file', form_path: namespace_project_create_blob_path(@project.namespace, @project, @id), method: :post
+  = render 'projects/blob/new_dir'
+
 :javascript
   // Load last commit log for each file in tree
   $('#tree-slider').waitForImages(function() {

config/routes.rb

@@ -463,6 +463,15 @@ Gitlab::Application.routes.draw do
           )
         end
 
+        scope do
+          post(
+              '/create_dir/*id',
+              to: 'tree#create_dir',
+              constraints: { id: /.+/ },
+              as: 'create_dir'
+          )
+        end
+
         scope do
           get(
             '/blame/*id',

features/project/source/browse_files.feature

@@ -21,12 +21,12 @@ Feature: Project Source Browse Files
     Then I should see raw file content
 
   Scenario: I can create file
-    Given I click on "new file" link in repo
+    Given I click on "New file" link in repo
     Then I can see new file page
 
   @javascript
   Scenario: I can create and commit file
-    Given I click on "new file" link in repo
+    Given I click on "New file" link in repo
     And I edit code
     And I fill the new file name
     And I fill the commit message
@@ -36,14 +36,13 @@ Feature: Project Source Browse Files
 
   @javascript
   Scenario: I can upload file and commit
-    Given I click on "new file" link in repo
-    Then I can see new file page
-    And I can see "upload an existing one"
-    And I click on "upload"
+    Given I click on "Upload file" link in repo
     And I upload a new text file
     And I fill the upload file commit message
+    And I fill the new branch name
     And I click on "Upload file"
     Then I can see the new text file
+    And I am redirected to the uploaded file on new branch
     And I can see the new commit message
 
   @javascript
@@ -59,7 +58,7 @@ Feature: Project Source Browse Files
 
   @javascript
   Scenario: I can create and commit file and specify new branch
-    Given I click on "new file" link in repo
+    Given I click on "New file" link in repo
     And I edit code
     And I fill the new file name
     And I fill the commit message
@@ -83,7 +82,7 @@ Feature: Project Source Browse Files
 
   @javascript
   Scenario: If I enter an illegal file name I see an error message
-    Given I click on "new file" link in repo
+    Given I click on "New file" link in repo
     And I fill the new file name with an illegal name
     And I edit code
     And I fill the commit message
@@ -138,6 +137,24 @@ Feature: Project Source Browse Files
     Then I am on the ".gitignore" edit file page
     And I see a commit error message
 
+  @javascript
+  Scenario: I can create directory in repo
+    When I click on "New directory" link in repo
+    And I fill the new directory name
+    And I fill the commit message
+    And I fill the new branch name
+    And I click on "Create directory"
+    Then I am redirected to the new directory
+
+  @javascript
+  Scenario: I attempt to create an existing directory
+    When I click on "New directory" link in repo
+    And I fill an existing directory name
+    And I fill the commit message
+    And I click on "Create directory"
+    Then I see "Unable to create directory"
+    And I am redirected to the root directory
+
   @javascript
   Scenario: I can see editing preview
     Given I click on ".gitignore" file in repo

features/steps/project/source/browse_files.rb

@@ -71,7 +71,7 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
   end
 
   step 'I fill the new branch name' do
-    fill_in :new_branch, with: 'new_branch_name'
+    fill_in :new_branch, with: 'new_branch_name', visible: true
   end
 
   step 'I fill the new file name with an illegal name' do
@@ -90,6 +90,10 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
     click_button 'Commit Changes'
   end
 
+  step 'I click on "Create directory"' do
+    click_button 'Create directory'
+  end
+
   step 'I click on "Remove"' do
     click_button 'Remove'
   end
@@ -110,21 +114,32 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
     expect(page).to have_css '.line_holder.new'
   end
 
-  step 'I click on "new file" link in repo' do
-    click_link 'new-file-link'
+  step 'I click on "New file" link in repo' do
+    find('.add-to-tree').click
+    click_link 'Create file'
   end
 
-  step 'I can see new file page' do
-    expect(page).to have_content "new file"
-    expect(page).to have_content "Commit message"
+  step 'I click on "Upload file" link in repo' do
+    find('.add-to-tree').click
+    click_link 'Upload file'
+  end
+
+  step 'I click on "New directory" link in repo' do
+    find('.add-to-tree').click
+    click_link 'New directory'
+  end
+
+  step 'I fill the new directory name' do
+    fill_in :dir_name, with: new_dir_name
   end
 
-  step 'I can see "upload an existing one"' do
-    expect(page).to have_content "upload an existing one"
+  step 'I fill an existing directory name' do
+    fill_in :dir_name, with: 'files'
   end
 
-  step 'I click on "upload"' do
-    click_link 'upload'
+  step 'I can see new file page' do
+    expect(page).to have_content "new file"
+    expect(page).to have_content "Commit message"
   end
 
   step 'I click on "Upload file"' do
@@ -228,10 +243,30 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
       @project.namespace, @project, 'new_branch_name/' + new_file_name))
   end
 
+  step 'I am redirected to the uploaded file on new branch' do
+    expect(current_path).to eq(namespace_project_blob_path(
+      @project.namespace, @project,
+      'new_branch_name/' + File.basename(test_text_file)))
+  end
+
+  step 'I am redirected to the new directory' do
+    expect(current_path).to eq(namespace_project_tree_path(
+      @project.namespace, @project, 'new_branch_name/' + new_dir_name))
+  end
+
+  step 'I am redirected to the root directory' do
+    expect(current_path).to eq(namespace_project_tree_path(
+      @project.namespace, @project, 'master/'))
+  end
+
   step "I don't see the permalink link" do
     expect(page).not_to have_link('permalink')
   end
 
+  step 'I see "Unable to create directory"' do
+    expect(page).to have_content('Directory already exists')
+  end
+
   step 'I see a commit error message' do
     expect(page).to have_content('Your changes could not be committed')
   end
@@ -287,6 +322,12 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
     'not_a_file.md'
   end
 
+  # Constant value that is a valid directory and
+  # not a directory present at root of the seed repository.
+  def new_dir_name
+    'new_dir/subdir'
+  end
+
   def drop_in_dropzone(file_path)
     # Generate a fake input selector
     page.execute_script <<-JS

spec/controllers/projects/tree_controller_spec.rb

@@ -88,4 +88,40 @@ describe Projects::TreeController do
       end
     end
   end
+
+  describe '#create_dir' do
+    render_views
+
+    before do
+      post(:create_dir,
+           namespace_id: project.namespace.to_param,
+           project_id: project.to_param,
+           id: 'master',
+           dir_name: path,
+           new_branch: target_branch,
+           commit_message: 'Test commit message')
+    end
+
+    context 'successful creation' do
+      let(:path) { 'files/new_dir'}
+      let(:target_branch) { 'master-test'}
+
+      it 'redirects to the new directory' do
+        expect(subject).
+            to redirect_to("/#{project.path_with_namespace}/blob/#{target_branch}/#{path}")
+        expect(flash[:notice]).to eq('The directory has been successfully created')
+      end
+    end
+
+    context 'unsuccessful creation' do
+      let(:path) { 'README.md' }
+      let(:target_branch) { 'master'}
+
+      it 'does not allow overwriting of existing files' do
+        expect(subject).
+            to redirect_to("/#{project.path_with_namespace}/blob/master")
+        expect(flash[:alert]).to eq('Directory already exists as a file')
+      end
+    end
+  end
 end