From f4f4a6b5303a0889f3fdb1bfe0bb014a6788c4d6 Mon Sep 17 00:00:00 2001 From: Douwe Maan 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   @@ -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