From f4f4a6b5303a0889f3fdb1bfe0bb014a6788c4d6 Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@gitlab.com>
Date: Fri, 18 Dec 2015 16:14:12 +0100
Subject: [PATCH] Fix specs and behavior for LFS files

---
 app/controllers/projects/forks_controller.rb   | 11 +++++++----
 app/controllers/projects/imports_controller.rb |  9 +++++++--
 app/controllers/projects/tree_controller.rb    |  2 +-
 app/helpers/blob_helper.rb                     | 10 ++++------
 app/views/projects/blob/_actions.html.haml     |  2 +-
 app/views/projects/diffs/_file.html.haml       |  4 ++--
 features/steps/project/source/browse_files.rb  |  6 +++---
 7 files changed, 25 insertions(+), 19 deletions(-)

diff --git a/app/controllers/projects/forks_controller.rb b/app/controllers/projects/forks_controller.rb
index 51181b8042e..1d599b6c427 100644
--- a/app/controllers/projects/forks_controller.rb
+++ b/app/controllers/projects/forks_controller.rb
@@ -13,15 +13,13 @@ class Projects::ForksController < Projects::ApplicationController
     @forked_project = ::Projects::ForkService.new(project, current_user, namespace: namespace).execute
 
     if @forked_project.saved? && @forked_project.forked?
-      continue_params[:notice] ||= "The project was successfully forked."
-
       if @forked_project.import_in_progress?
         redirect_to namespace_project_import_path(@forked_project.namespace, @forked_project, continue: continue_params)
       else
         if continue_params
           redirect_to continue_params[:to], notice: continue_params[:notice]
         else
-          redirect_to namespace_project_path(@forked_project.namespace, @forked_project)
+          redirect_to namespace_project_path(@forked_project.namespace, @forked_project), notice: "The project was successfully forked."
         end
       end
     else
@@ -32,6 +30,11 @@ class Projects::ForksController < Projects::ApplicationController
   private
 
   def continue_params
-    params[:continue].permit(:to, :notice, :notice_now)
+    continue_params = params[:continue]
+    if continue_params
+      continue_params.permit(:to, :notice, :notice_now)
+    else
+      nil
+    end
   end
 end
diff --git a/app/controllers/projects/imports_controller.rb b/app/controllers/projects/imports_controller.rb
index e9c9edd3a3c..8d8035ef5ff 100644
--- a/app/controllers/projects/imports_controller.rb
+++ b/app/controllers/projects/imports_controller.rb
@@ -28,7 +28,7 @@ class Projects::ImportsController < Projects::ApplicationController
       if continue_params
         redirect_to continue_params[:to], notice: continue_params[:notice]
       else
-        redirect_to project_path(@project)
+        redirect_to project_path(@project), notice: "The project was successfully forked."
       end
     elsif @project.import_failed?
       redirect_to new_namespace_project_import_path(@project.namespace, @project)
@@ -43,7 +43,12 @@ class Projects::ImportsController < Projects::ApplicationController
   private
 
   def continue_params
-    @continue_params ||= params[:continue].permit(:to, :notice, :notice_now)
+    continue_params = params[:continue]
+    if continue_params
+      continue_params.permit(:to, :notice, :notice_now)
+    else
+      nil
+    end
   end
 
   def require_no_repo
diff --git a/app/controllers/projects/tree_controller.rb b/app/controllers/projects/tree_controller.rb
index 4f78bde2d2d..cb3ed0f6f9c 100644
--- a/app/controllers/projects/tree_controller.rb
+++ b/app/controllers/projects/tree_controller.rb
@@ -35,7 +35,7 @@ class Projects::TreeController < Projects::ApplicationController
     return render_404 unless @commit_params.values.all?
 
     create_commit(Files::CreateDirService,  success_notice: "The directory has been successfully created.",
-                                            success_path: namespace_project_blob_path(@project.namespace, @project, File.join(@target_branch, @dir_name)),
+                                            success_path: namespace_project_tree_path(@project.namespace, @project, File.join(@target_branch, @dir_name)),
                                             failure_path: namespace_project_tree_path(@project.namespace, @project, @ref))
   end
 
diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb
index 3368e77a0eb..d31d4cde08f 100644
--- a/app/helpers/blob_helper.rb
+++ b/app/helpers/blob_helper.rb
@@ -27,7 +27,7 @@ module BlobHelper
 
     blob = project.repository.blob_at(ref, path) rescue nil
 
-    return unless blob && blob_text_editable?(blob)
+    return unless blob && blob_text_viewable?(blob)
 
     from_mr = options[:from_merge_request_id]
     link_opts = {}
@@ -63,6 +63,8 @@ module BlobHelper
 
     if !on_top_of_branch?
       button_tag label, class: "btn btn-#{btn_class} disabled has_tooltip", title: "You can only #{action} files when you are on a branch", data: { container: 'body' }
+    elsif blob.lfs_pointer?
+      button_tag label, class: "btn btn-#{btn_class} disabled has_tooltip", title: "It is not possible to #{action} files that are stored in LFS using the web interface", data: { container: 'body' }
     elsif can_edit_blob?(blob)
       button_tag label, class: "btn btn-#{btn_class}", 'data-target' => "#modal-#{modal_type}-blob", 'data-toggle' => 'modal'
     elsif can?(current_user, :fork_project, project)
@@ -102,10 +104,6 @@ module BlobHelper
     )
   end
 
-  def blob_text_editable?(blob)
-    blob.text? && !blob.lfs_pointer?
-  end
-
   def can_edit_blob?(blob, project = @project, ref = @ref)
     !blob.lfs_pointer? && can_edit_tree?(project, ref)
   end
@@ -130,7 +128,7 @@ module BlobHelper
     icon("#{file_type_icon_class('file', mode, name)} fw")
   end
 
-  def blob_viewable?(blob)
+  def blob_text_viewable?(blob)
     blob && blob.text? && !blob.lfs_pointer?
   end
 
diff --git a/app/views/projects/blob/_actions.html.haml b/app/views/projects/blob/_actions.html.haml
index caefd911a2a..cdac50f7a8d 100644
--- a/app/views/projects/blob/_actions.html.haml
+++ b/app/views/projects/blob/_actions.html.haml
@@ -2,7 +2,7 @@
   = 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
-  - if blob_viewable?(@blob)
+  - if blob_text_viewable?(@blob)
     - if current_page? namespace_project_blame_path(@project.namespace, @project, @id)
       = link_to 'Normal View', namespace_project_blob_path(@project.namespace, @project, @id),
           class: 'btn btn-sm'
diff --git a/app/views/projects/diffs/_file.html.haml b/app/views/projects/diffs/_file.html.haml
index 9c6d7b46429..517f6aef7c5 100644
--- a/app/views/projects/diffs/_file.html.haml
+++ b/app/views/projects/diffs/_file.html.haml
@@ -24,7 +24,7 @@
             = "#{diff_file.diff.a_mode} → #{diff_file.diff.b_mode}"
 
       .diff-controls
-        - if blob_viewable?(blob)
+        - if blob_text_viewable?(blob)
           = link_to '#', class: 'js-toggle-diff-comments btn btn-sm active has_tooltip', title: "Toggle comments for this file" do
             %i.fa.fa-comments
           &nbsp;
@@ -40,7 +40,7 @@
   .diff-content.diff-wrap-lines
     -# Skipp all non non-supported blobs
     - return unless blob.respond_to?('text?')
-    - if blob_viewable?(blob)
+    - if blob_text_viewable?(blob)
       - if diff_view == 'parallel'
         = render "projects/diffs/parallel_view", diff_file: diff_file, project: project, blob: blob, index: i
       - else
diff --git a/features/steps/project/source/browse_files.rb b/features/steps/project/source/browse_files.rb
index da8a07c2b4a..4962b68b8f2 100644
--- a/features/steps/project/source/browse_files.rb
+++ b/features/steps/project/source/browse_files.rb
@@ -253,7 +253,7 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
 
   step 'I am redirected to the root directory' do
     expect(current_path).to eq(
-      namespace_project_tree_path(@project.namespace, @project, 'master/'))
+      namespace_project_tree_path(@project.namespace, @project, 'master'))
   end
 
   step "I don't see the permalink link" do
@@ -332,8 +332,8 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
     expect(page).to have_content 'Permalink'
     expect(page).not_to have_content 'Edit'
     expect(page).not_to have_content 'Blame'
-    expect(page).not_to have_content 'Delete'
-    expect(page).not_to have_content 'Replace'
+    expect(page).to have_content 'Delete'
+    expect(page).to have_content 'Replace'
   end
 
   private
-- 
GitLab