diff options
author | Dmitriy Zaporozhets <dzaporozhets@gitlab.com> | 2015-01-27 05:45:54 +0300 |
---|---|---|
committer | Dmitriy Zaporozhets <dzaporozhets@gitlab.com> | 2015-01-27 05:45:54 +0300 |
commit | 0f52d259617c4fe82ca1377bba904b3297407dea (patch) | |
tree | 9296e6269ed0230d4acb7e98dab67c4af2bdbb9e | |
parent | 79915528d87b20321ba152d5c0f6c2c04c20483d (diff) | |
parent | ed8c3e2d738312c09e336fb8d549eea7d9cc71b9 (diff) |
Merge branch 'init-from-ui' into 'master'
File creation and editing refactored
* Refactored controllers/helpers/views related to blob resource
* make it possible to create new file in empty repository
Second step in building #1930
See merge request !1440
24 files changed, 176 insertions, 170 deletions
diff --git a/app/controllers/projects/base_tree_controller.rb b/app/controllers/projects/base_tree_controller.rb deleted file mode 100644 index a7b1b7b40e8..00000000000 --- a/app/controllers/projects/base_tree_controller.rb +++ /dev/null @@ -1,7 +0,0 @@ -class Projects::BaseTreeController < Projects::ApplicationController - include ExtractsPath - - before_filter :authorize_download_code! - before_filter :require_non_empty_project -end - diff --git a/app/controllers/projects/blame_controller.rb b/app/controllers/projects/blame_controller.rb index 367d1295f34..106f21b83e6 100644 --- a/app/controllers/projects/blame_controller.rb +++ b/app/controllers/projects/blame_controller.rb @@ -2,7 +2,7 @@ class Projects::BlameController < Projects::ApplicationController include ExtractsPath - # Authorize + before_filter :assign_ref_vars before_filter :authorize_download_code! before_filter :require_non_empty_project diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb index 2412800c493..b471d57f698 100644 --- a/app/controllers/projects/blob_controller.rb +++ b/app/controllers/projects/blob_controller.rb @@ -2,16 +2,70 @@ class Projects::BlobController < Projects::ApplicationController include ExtractsPath - # Authorize + # Raised when given an invalid file path + class InvalidPathError < StandardError; end + before_filter :authorize_download_code! - before_filter :require_non_empty_project + before_filter :require_non_empty_project, except: [:new, :create] before_filter :authorize_push_code!, only: [:destroy] + before_filter :assign_blob_vars + before_filter :commit, except: [:new, :create] + before_filter :blob, except: [:new, :create] + before_filter :from_merge_request, only: [:edit, :update] + before_filter :after_edit_path, only: [:edit, :update] + before_filter :require_branch_head, only: [:edit, :update] + + def new + commit unless @repository.empty? + end - before_filter :blob + def create + file_path = File.join(@path, File.basename(params[:file_name])) + result = Files::CreateService.new(@project, current_user, params, @ref, file_path).execute + + if result[:status] == :success + flash[:notice] = "Your changes have been successfully committed" + redirect_to project_blob_path(@project, File.join(@ref, file_path)) + else + flash[:alert] = result[:message] + render :new + end + end def show end + def edit + @last_commit = Gitlab::Git::Commit.last_for_path(@repository, @ref, @path).sha + end + + def update + result = Files::UpdateService. + new(@project, current_user, params, @ref, @path).execute + + if result[:status] == :success + flash[:notice] = "Your changes have been successfully committed" + + if from_merge_request + from_merge_request.reload_code + end + + redirect_to after_edit_path + else + flash[:alert] = result[:message] + render :edit + end + end + + def preview + @content = params[:content] + diffy = Diffy::Diff.new(@blob.data, @content, diff: '-U 3', + include_diff_info: true) + @diff_lines = Gitlab::Diff::Parser.new.parse(diffy.diff.scan(/.*\n/)) + + render layout: false + end + def destroy result = Files::DeleteService.new(@project, current_user, params, @ref, @path).execute @@ -46,10 +100,44 @@ class Projects::BlobController < Projects::ApplicationController if @blob @blob - elsif tree.entries.any? - redirect_to project_tree_path(@project, File.join(@ref, @path)) and return else + if tree = @repository.tree(@commit.id, @path) + if tree.entries.any? + redirect_to project_tree_path(@project, File.join(@ref, @path)) and return + end + end + return not_found! end end + + def commit + @commit = @repository.commit(@ref) + + return not_found! unless @commit + end + + def assign_blob_vars + @id = params[:id] + @ref, @path = extract_ref(@id) + + + rescue InvalidPathError + not_found! + end + + def after_edit_path + @after_edit_path ||= + if from_merge_request + diffs_project_merge_request_path(from_merge_request.target_project, from_merge_request) + + "#file-path-#{hexdigest(@path)}" + else + project_blob_path(@project, @id) + end + end + + def from_merge_request + # If blob edit was initiated from merge request page + @from_merge_request ||= MergeRequest.find_by(id: params[:from_merge_request_id]) + end end diff --git a/app/controllers/projects/commits_controller.rb b/app/controllers/projects/commits_controller.rb index 9476b6c0284..0a85c36a758 100644 --- a/app/controllers/projects/commits_controller.rb +++ b/app/controllers/projects/commits_controller.rb @@ -3,7 +3,7 @@ require "base64" class Projects::CommitsController < Projects::ApplicationController include ExtractsPath - # Authorize + before_filter :assign_ref_vars before_filter :authorize_download_code! before_filter :require_non_empty_project diff --git a/app/controllers/projects/edit_tree_controller.rb b/app/controllers/projects/edit_tree_controller.rb deleted file mode 100644 index 65661c80410..00000000000 --- a/app/controllers/projects/edit_tree_controller.rb +++ /dev/null @@ -1,60 +0,0 @@ -class Projects::EditTreeController < Projects::BaseTreeController - before_filter :require_branch_head - before_filter :blob - before_filter :authorize_push_code! - before_filter :from_merge_request - before_filter :after_edit_path - - def show - @last_commit = Gitlab::Git::Commit.last_for_path(@repository, @ref, @path).sha - end - - def update - result = Files::UpdateService. - new(@project, current_user, params, @ref, @path).execute - - if result[:status] == :success - flash[:notice] = "Your changes have been successfully committed" - - if from_merge_request - from_merge_request.reload_code - end - - redirect_to after_edit_path - else - flash[:alert] = result[:message] - render :show - end - end - - def preview - @content = params[:content] - - diffy = Diffy::Diff.new(@blob.data, @content, diff: '-U 3', - include_diff_info: true) - @diff_lines = Gitlab::Diff::Parser.new.parse(diffy.diff.scan(/.*\n/)) - - render layout: false - end - - private - - def blob - @blob ||= @repository.blob_at(@commit.id, @path) - end - - def after_edit_path - @after_edit_path ||= - if from_merge_request - diffs_project_merge_request_path(from_merge_request.target_project, from_merge_request) + - "#file-path-#{hexdigest(@path)}" - else - project_blob_path(@project, @id) - end - end - - def from_merge_request - # If blob edit was initiated from merge request page - @from_merge_request ||= MergeRequest.find_by(id: params[:from_merge_request_id]) - end -end diff --git a/app/controllers/projects/network_controller.rb b/app/controllers/projects/network_controller.rb index ada1aed0df7..59f2a745367 100644 --- a/app/controllers/projects/network_controller.rb +++ b/app/controllers/projects/network_controller.rb @@ -2,7 +2,7 @@ class Projects::NetworkController < Projects::ApplicationController include ExtractsPath include ApplicationHelper - # Authorize + before_filter :assign_ref_vars before_filter :authorize_download_code! before_filter :require_non_empty_project diff --git a/app/controllers/projects/new_tree_controller.rb b/app/controllers/projects/new_tree_controller.rb deleted file mode 100644 index ffba706b2f6..00000000000 --- a/app/controllers/projects/new_tree_controller.rb +++ /dev/null @@ -1,20 +0,0 @@ -class Projects::NewTreeController < Projects::BaseTreeController - before_filter :require_branch_head - before_filter :authorize_push_code! - - def show - end - - def update - file_path = File.join(@path, File.basename(params[:file_name])) - result = Files::CreateService.new(@project, current_user, params, @ref, file_path).execute - - if result[:status] == :success - flash[:notice] = "Your changes have been successfully committed" - redirect_to project_blob_path(@project, File.join(@ref, file_path)) - else - flash[:alert] = result[:message] - render :show - end - end -end diff --git a/app/controllers/projects/raw_controller.rb b/app/controllers/projects/raw_controller.rb index fdbc4c5a098..84888265dc1 100644 --- a/app/controllers/projects/raw_controller.rb +++ b/app/controllers/projects/raw_controller.rb @@ -2,7 +2,7 @@ class Projects::RawController < Projects::ApplicationController include ExtractsPath - # Authorize + before_filter :assign_ref_vars before_filter :authorize_download_code! before_filter :require_non_empty_project diff --git a/app/controllers/projects/refs_controller.rb b/app/controllers/projects/refs_controller.rb index 67665f5f601..cede0ebe0ae 100644 --- a/app/controllers/projects/refs_controller.rb +++ b/app/controllers/projects/refs_controller.rb @@ -1,7 +1,7 @@ class Projects::RefsController < Projects::ApplicationController include ExtractsPath - # Authorize + before_filter :assign_ref_vars before_filter :authorize_download_code! before_filter :require_non_empty_project diff --git a/app/controllers/projects/tree_controller.rb b/app/controllers/projects/tree_controller.rb index 4d033b36848..5b52640a4e1 100644 --- a/app/controllers/projects/tree_controller.rb +++ b/app/controllers/projects/tree_controller.rb @@ -1,7 +1,12 @@ # Controller for viewing a repository's file structure -class Projects::TreeController < Projects::BaseTreeController - def show +class Projects::TreeController < Projects::ApplicationController + include ExtractsPath + + before_filter :assign_ref_vars + before_filter :authorize_download_code! + before_filter :require_non_empty_project, except: [:new, :create] + def show if tree.entries.empty? if @repository.blob_at(@commit.id, @path) redirect_to project_blob_path(@project, File.join(@ref, @path)) and return diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb index 3a282803963..e75eebd2da9 100644 --- a/app/helpers/blob_helper.rb +++ b/app/helpers/blob_helper.rb @@ -19,4 +19,42 @@ module BlobHelper def no_highlight_files %w(credits changelog copying copyright license authors) end + + def edit_blob_link(project, ref, path, options = {}) + blob = + begin + project.repository.blob_at(ref, path) + rescue + nil + end + + if blob && blob.text? + 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, project_edit_blob_path(project, tree_join(ref, path), + link_opts), class: cls + else + content_tag :span, text, class: cls + ' disabled' + end + after.html_safe + else + '' + end + end + + def leave_edit_message + "Leave edit mode?\nAll unsaved changes will be lost." + end + + def editing_preview_title(filename) + if Gitlab::MarkdownHelper.previewable?(filename) + 'Preview' + else + 'Preview changes' + end + end end diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index de232ab4e25..9d2c99356af 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -187,7 +187,11 @@ module ProjectsHelper "Issues - " + title end elsif current_controller?(:blob) - "#{@project.path}\/#{@blob.path} at #{@ref} - " + title + if current_action?(:new) || current_action?(:create) + "New file at #{@ref}" + elsif @blob + "Edit file #{@blob.path} at #{@ref}" + end elsif current_controller?(:commits) "Commits at #{@ref} - " + title elsif current_controller?(:merge_requests) diff --git a/app/helpers/tree_helper.rb b/app/helpers/tree_helper.rb index 1d987a6ffc0..727ec3fb231 100644 --- a/app/helpers/tree_helper.rb +++ b/app/helpers/tree_helper.rb @@ -64,32 +64,6 @@ module TreeHelper ::Gitlab::GitAccess.can_push_to_branch?(current_user, project, ref) end - def edit_blob_link(project, ref, path, options = {}) - blob = - begin - project.repository.blob_at(ref, path) - rescue - nil - end - - if blob && blob.text? - 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, project_edit_tree_path(project, tree_join(ref, path), - link_opts), class: cls - else - content_tag :span, text, class: cls + ' disabled' - end + after.html_safe - else - '' - end - end - def tree_breadcrumbs(tree, max_links = 2) if @path.present? part_path = "" @@ -121,16 +95,4 @@ module TreeHelper return tree.name end end - - def leave_edit_message - "Leave edit mode?\nAll unsaved changes will be lost." - end - - def editing_preview_title(filename) - if Gitlab::MarkdownHelper.previewable?(filename) - 'Preview' - else - 'Diff' - end - end end diff --git a/app/views/projects/edit_tree/show.html.haml b/app/views/projects/blob/edit.html.haml index 7e0789853af..883845c03f5 100644 --- a/app/views/projects/edit_tree/show.html.haml +++ b/app/views/projects/blob/edit.html.haml @@ -1,11 +1,16 @@ .file-editor %ul.nav.nav-tabs.js-edit-mode %li.active - = link_to 'Edit', '#editor' + = link_to '#editor' do + %i.fa.fa-edit + Edit file + %li - = link_to editing_preview_title(@blob.name), '#preview', 'data-preview-url' => preview_project_edit_tree_path(@project, @id) + = link_to '#preview', 'data-preview-url' => project_preview_blob_path(@project, @id) do + %i.fa.fa-eye + = editing_preview_title(@blob.name) - = form_tag(project_edit_tree_path(@project, @id), method: :put, class: "form-horizontal") do + = form_tag(project_update_blob_path(@project, @id), method: :put, class: "form-horizontal") do = render 'projects/blob_editor', ref: @ref, path: @path, blob_data: @blob.data = render 'shared/commit_message_container', params: params, placeholder: "Update #{@blob.name}" diff --git a/app/views/projects/new_tree/show.html.haml b/app/views/projects/blob/new.html.haml index cf7b768694f..57e830d5c56 100644 --- a/app/views/projects/new_tree/show.html.haml +++ b/app/views/projects/blob/new.html.haml @@ -1,7 +1,7 @@ %h3.page-title New file %hr .file-editor - = form_tag(project_new_tree_path(@project, @id), method: :put, class: 'form-horizontal form-new-file') do + = form_tag(project_create_blob_path(@project, @id), method: :post, class: 'form-horizontal form-new-file') do .form-group.commit_message-group = label_tag 'file_name', class: 'control-label' do File name diff --git a/app/views/projects/edit_tree/preview.html.haml b/app/views/projects/blob/preview.html.haml index e7c3460ad78..e7c3460ad78 100644 --- a/app/views/projects/edit_tree/preview.html.haml +++ b/app/views/projects/blob/preview.html.haml diff --git a/app/views/projects/empty.html.haml b/app/views/projects/empty.html.haml index 3a42fce43e9..776a7327bc2 100644 --- a/app/views/projects/empty.html.haml +++ b/app/views/projects/empty.html.haml @@ -8,7 +8,7 @@ The repository for this project is empty %p.lead You can - = link_to '#', class: 'btn btn-new btn-lg' do + = link_to project_new_blob_path(@project, 'master'), class: 'btn btn-new btn-lg' do add a file or push it via command line. diff --git a/app/views/projects/tree/_tree.html.haml b/app/views/projects/tree/_tree.html.haml index 68ccd4d61bb..f902440b3f1 100644 --- a/app/views/projects/tree/_tree.html.haml +++ b/app/views/projects/tree/_tree.html.haml @@ -10,7 +10,7 @@ = link_to title, '#' - if current_user && can_push_branch?(@project, @ref) %li - = link_to project_new_tree_path(@project, @id), title: 'New file', id: 'new-file-link' do + = link_to project_new_blob_path(@project, @id), title: 'New file', id: 'new-file-link' do %small %i.fa.fa-plus diff --git a/config/routes.rb b/config/routes.rb index 8c3eef23260..f29b620e079 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -211,17 +211,20 @@ Gitlab::Application.routes.draw do end scope module: :projects do + # Blob routes: + get '/new/:id', to: 'blob#new', constraints: {id: /.+/}, as: 'new_blob' + post '/create/:id', to: 'blob#create', constraints: {id: /.+/}, as: 'create_blob' + get '/edit/:id', to: 'blob#edit', constraints: {id: /.+/}, as: 'edit_blob' + put '/update/:id', to: 'blob#update', constraints: {id: /.+/}, as: 'update_blob' + post '/preview/:id', to: 'blob#preview', constraints: {id: /.+/}, as: 'preview_blob' + resources :blob, only: [:show, :destroy], constraints: { id: /.+/, format: false } do get :diff, on: :member end + resources :raw, only: [:show], constraints: {id: /.+/} resources :tree, only: [:show], constraints: {id: /.+/, format: /(html|js)/ } - resources :edit_tree, only: [:show, :update], constraints: { id: /.+/ }, path: 'edit' do - # Cannot be GET to differentiate from GET paths that end in preview. - post :preview, on: :member - end resource :avatar, only: [:show, :destroy] - resources :new_tree, only: [:show, :update], constraints: {id: /.+/}, path: 'new' resources :commit, only: [:show], constraints: {id: /[[:alnum:]]{6,40}/} resources :commits, only: [:show], constraints: {id: /(?:[^.]|\.(?!atom$))+/, format: /atom/} resources :compare, only: [:index, :create] diff --git a/features/steps/project/source/browse_files.rb b/features/steps/project/source/browse_files.rb index 805e6ff0eac..1caad73654b 100644 --- a/features/steps/project/source/browse_files.rb +++ b/features/steps/project/source/browse_files.rb @@ -78,7 +78,7 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps end step 'I click link "Diff"' do - click_link 'Diff' + click_link 'Preview changes' end step 'I click on "Commit Changes"' do diff --git a/features/steps/shared/paths.rb b/features/steps/shared/paths.rb index 33ef6ccacf1..cef48c179b2 100644 --- a/features/steps/shared/paths.rb +++ b/features/steps/shared/paths.rb @@ -284,11 +284,11 @@ module SharedPaths end step 'I am on the new file page' do - current_path.should eq(project_new_tree_path(@project, root_ref)) + current_path.should eq(project_create_blob_path(@project, root_ref)) end step 'I am on the ".gitignore" edit file page' do - current_path.should eq(project_edit_tree_path( + current_path.should eq(project_edit_blob_path( @project, File.join(root_ref, '.gitignore'))) end diff --git a/lib/extracts_path.rb b/lib/extracts_path.rb index e51cb30bdd9..19215cfb7e6 100644 --- a/lib/extracts_path.rb +++ b/lib/extracts_path.rb @@ -1,17 +1,9 @@ # Module providing methods for dealing with separating a tree-ish string and a # file path string when combined in a request parameter module ExtractsPath - extend ActiveSupport::Concern - # Raised when given an invalid file path class InvalidPathError < StandardError; end - included do - if respond_to?(:before_filter) - before_filter :assign_ref_vars - end - end - # Given a string containing both a Git tree-ish, such as a branch or tag, and # a filesystem path joined by forward slashes, attempts to separate the two. # diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index 8191d1fb9c4..e36b266a1ff 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -430,21 +430,17 @@ describe Projects::TreeController, 'routing' do end end -describe Projects::EditTreeController, 'routing' do - it 'to #show' do +describe Projects::BlobController, 'routing' do + it 'to #edit' do get('/gitlab/gitlabhq/edit/master/app/models/project.rb').should( - route_to('projects/edit_tree#show', + route_to('projects/blob#edit', project_id: 'gitlab/gitlabhq', id: 'master/app/models/project.rb')) - get('/gitlab/gitlabhq/edit/master/app/models/project.rb/preview').should( - route_to('projects/edit_tree#show', - project_id: 'gitlab/gitlabhq', - id: 'master/app/models/project.rb/preview')) end it 'to #preview' do - post('/gitlab/gitlabhq/edit/master/app/models/project.rb/preview').should( - route_to('projects/edit_tree#preview', + post('/gitlab/gitlabhq/preview/master/app/models/project.rb').should( + route_to('projects/blob#preview', project_id: 'gitlab/gitlabhq', id: 'master/app/models/project.rb')) end diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 0cd822bcdae..0e60baae2c4 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -48,7 +48,7 @@ describe MergeRequests::UpdateService do end it 'should create system note about merge_request reassign' do - note = @merge_request.notes.reload.last + note = @merge_request.notes.last note.note.should include "Reassigned to \@#{user2.username}" end end |