diff --git a/app/controllers/blame_controller.rb b/app/controllers/blame_controller.rb index 37d7245c..76caa4a6 100644 --- a/app/controllers/blame_controller.rb +++ b/app/controllers/blame_controller.rb @@ -7,8 +7,6 @@ class BlameController < ProjectResourceController before_filter :authorize_code_access! before_filter :require_non_empty_project - before_filter :assign_ref_vars - def show @repo = @project.repo @blame = Grit::Blob.blame(@repo, @commit.id, @path) diff --git a/app/controllers/blob_controller.rb b/app/controllers/blob_controller.rb index d4a45d95..530b72fe 100644 --- a/app/controllers/blob_controller.rb +++ b/app/controllers/blob_controller.rb @@ -7,8 +7,6 @@ class BlobController < ProjectResourceController before_filter :authorize_code_access! before_filter :require_non_empty_project - before_filter :assign_ref_vars - def show if @tree.is_blob? send_data( diff --git a/app/controllers/edit_tree_controller.rb b/app/controllers/edit_tree_controller.rb new file mode 100644 index 00000000..aae983aa --- /dev/null +++ b/app/controllers/edit_tree_controller.rb @@ -0,0 +1,47 @@ +# Controller for edit a repository's file +class EditTreeController < ProjectResourceController + include ExtractsPath + + # Authorize + before_filter :authorize_read_project! + before_filter :authorize_code_access! + before_filter :require_non_empty_project + + before_filter :edit_requirements, only: [:edit, :update] + + def show + @last_commit = @project.repository.last_commit_for(@ref, @path).sha + end + + def update + edit_file_action = Gitlab::Satellite::EditFileAction.new(current_user, @project, @ref, @path) + updated_successfully = edit_file_action.commit!( + params[:content], + params[:commit_message], + params[:last_commit] + ) + + if updated_successfully + redirect_to project_tree_path(@project, @id), notice: "Your changes have been successfully commited" + else + flash[:notice] = "Your changes could not be commited, because the file has been changed" + render :edit + end + end + + private + + def edit_requirements + unless @tree.is_blob? && @tree.text? + redirect_to project_tree_path(@project, @id), notice: "You can only edit text files" + end + + allowed = if project.protected_branch? @ref + can?(current_user, :push_code_to_protected_branches, project) + else + can?(current_user, :push_code, project) + end + + return access_denied! unless allowed + end +end diff --git a/app/controllers/tree_controller.rb b/app/controllers/tree_controller.rb index 2151bd7c..a03ea3ff 100644 --- a/app/controllers/tree_controller.rb +++ b/app/controllers/tree_controller.rb @@ -7,9 +7,6 @@ class TreeController < ProjectResourceController before_filter :authorize_code_access! before_filter :require_non_empty_project - before_filter :assign_ref_vars - before_filter :edit_requirements, only: [:edit, :update] - def show @hex_path = Digest::SHA1.hexdigest(@path) @logs_path = logs_file_project_ref_path(@project, @ref, @path) @@ -20,40 +17,4 @@ class TreeController < ProjectResourceController format.js { no_cache_headers } end end - - def edit - @last_commit = @project.repository.last_commit_for(@ref, @path).sha - end - - def update - edit_file_action = Gitlab::Satellite::EditFileAction.new(current_user, @project, @ref, @path) - updated_successfully = edit_file_action.commit!( - params[:content], - params[:commit_message], - params[:last_commit] - ) - - if updated_successfully - redirect_to project_tree_path(@project, @id), notice: "Your changes have been successfully commited" - else - flash[:notice] = "Your changes could not be commited, because the file has been changed" - render :edit - end - end - - private - - def edit_requirements - unless @tree.is_blob? && @tree.text? - redirect_to project_tree_path(@project, @id), notice: "You can only edit text files" - end - - allowed = if project.protected_branch? @ref - can?(current_user, :push_code_to_protected_branches, project) - else - can?(current_user, :push_code, project) - end - - return access_denied! unless allowed - end end diff --git a/app/views/tree/edit.html.haml b/app/views/edit_tree/show.html.haml similarity index 93% rename from app/views/tree/edit.html.haml rename to app/views/edit_tree/show.html.haml index 81918e50..211396ba 100644 --- a/app/views/tree/edit.html.haml +++ b/app/views/edit_tree/show.html.haml @@ -1,5 +1,5 @@ .file-editor - = form_tag(project_tree_path(@project, @id), method: :put, class: "form-horizontal") do + = form_tag(project_edit_tree_path(@project, @id), method: :put, class: "form-horizontal") do .file_holder .file_title %i.icon-file diff --git a/app/views/tree/_blob_actions.html.haml b/app/views/tree/_blob_actions.html.haml index 0bde968d..1d55a4ff 100644 --- a/app/views/tree/_blob_actions.html.haml +++ b/app/views/tree/_blob_actions.html.haml @@ -1,7 +1,7 @@ .btn-group.tree-btn-group -# only show edit link for text files - if @tree.text? - = link_to "edit", edit_project_tree_path(@project, @id), class: "btn btn-tiny", disabled: !allowed_tree_edit? + = link_to "edit", project_edit_tree_path(@project, @id), class: "btn btn-tiny", disabled: !allowed_tree_edit? = link_to "raw", project_blob_path(@project, @id), class: "btn btn-tiny", target: "_blank" -# only show normal/blame view links for text files - if @tree.text? diff --git a/config/routes.rb b/config/routes.rb index 0028baf8..fafaef84 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -168,7 +168,8 @@ Gitlab::Application.routes.draw do # resources :projects, constraints: { id: /(?:[a-zA-Z.0-9_\-]+\/)?[a-zA-Z.0-9_\-]+/ }, except: [:new, :create, :index], path: "/" do resources :blob, only: [:show], constraints: {id: /.+/} - resources :tree, only: [:show, :edit, :update], constraints: {id: /.+/} + resources :tree, only: [:show], constraints: {id: /.+/, format: /(html|js)/ } + resources :edit_tree, only: [:show, :update], constraints: {id: /.+/}, path: 'edit' 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/lib/extracts_path.rb b/lib/extracts_path.rb index 66b2f450..351fc2f2 100644 --- a/lib/extracts_path.rb +++ b/lib/extracts_path.rb @@ -8,7 +8,7 @@ module ExtractsPath included do if respond_to?(:before_filter) - before_filter :assign_ref_vars, only: [:show] + before_filter :assign_ref_vars end end @@ -33,7 +33,7 @@ module ExtractsPath # extract_ref("v2.0.0/README.md") # # => ['v2.0.0', 'README.md'] # - # extract_ref('/gitlab/vagrant/tree/master/app/models/project.rb') + # extract_ref('master/app/models/project.rb') # # => ['master', 'app/models/project.rb'] # # extract_ref('issues/1234/app/models/project.rb') @@ -45,22 +45,12 @@ module ExtractsPath # # Returns an Array where the first value is the tree-ish and the second is the # path - def extract_ref(input) + def extract_ref(id) pair = ['', ''] return pair unless @project - # Remove relative_url_root from path - input.gsub!(/^#{Gitlab.config.gitlab.relative_url_root}/, "") - # Remove project, actions and all other staff from path - input.gsub!(/^\/#{Regexp.escape(@project.path_with_namespace)}/, "") - input.gsub!(/^\/(tree|commits|blame|blob|refs|graph)\//, "") # remove actions - input.gsub!(/\?.*$/, "") # remove stamps suffix - input.gsub!(/.atom$/, "") # remove rss feed - input.gsub!(/.json$/, "") # remove json suffix - input.gsub!(/\/edit$/, "") # remove edit route part - - if input.match(/^([[:alnum:]]{40})(.+)/) + if id.match(/^([[:alnum:]]{40})(.+)/) # If the ref appears to be a SHA, we're done, just split the string pair = $~.captures else @@ -68,7 +58,6 @@ module ExtractsPath # branches and tags # Append a trailing slash if we only get a ref and no file path - id = input id += '/' unless id.ends_with?('/') valid_refs = @project.repository.ref_names @@ -105,11 +94,9 @@ module ExtractsPath # Automatically renders `not_found!` if a valid tree path could not be # resolved (e.g., when a user inserts an invalid path or ref). def assign_ref_vars - path = CGI::unescape(request.fullpath.dup) + @id = params[:id] - @ref, @path = extract_ref(path) - - @id = File.join(@ref, @path) + @ref, @path = extract_ref(@id) # It is used "@project.repository.commits(@ref, @path, 1, 0)", # because "@project.repository.commit(@ref)" returns wrong commit when @ref is tag name. diff --git a/spec/lib/extracts_path_spec.rb b/spec/lib/extracts_path_spec.rb index ee20ae79..aac72c63 100644 --- a/spec/lib/extracts_path_spec.rb +++ b/spec/lib/extracts_path_spec.rb @@ -54,47 +54,5 @@ describe ExtractsPath do extract_ref('stable/CHANGELOG').should == ['stable', 'CHANGELOG'] end end - - context "with a fullpath" do - it "extracts a valid branch" do - extract_ref('/gitlab/gitlab-ci/tree/foo/bar/baz/CHANGELOG').should == ['foo/bar/baz', 'CHANGELOG'] - end - - it "extracts a valid tag" do - extract_ref('/gitlab/gitlab-ci/tree/v2.0.0/CHANGELOG').should == ['v2.0.0', 'CHANGELOG'] - end - - it "extracts a valid commit SHA" do - extract_ref('/gitlab/gitlab-ci/tree/f4b14494ef6abf3d144c28e4af0c20143383e062/CHANGELOG').should == - ['f4b14494ef6abf3d144c28e4af0c20143383e062', 'CHANGELOG'] - end - - it "extracts a timestamp" do - extract_ref('/gitlab/gitlab-ci/tree/v2.0.0/CHANGELOG?_=12354435').should == ['v2.0.0', 'CHANGELOG'] - end - end - - context "with a fullpath and a relative_url_root" do - before do - Gitlab.config.gitlab.stub(relative_url_root: '/relative') - end - - it "extracts a valid branch with relative_url_root" do - extract_ref('/relative/gitlab/gitlab-ci/tree/foo/bar/baz/CHANGELOG').should == ['foo/bar/baz', 'CHANGELOG'] - end - - it "extracts a valid tag" do - extract_ref('/relative/gitlab/gitlab-ci/tree/v2.0.0/CHANGELOG').should == ['v2.0.0', 'CHANGELOG'] - end - - it "extracts a valid commit SHA" do - extract_ref('/relative/gitlab/gitlab-ci/tree/f4b14494ef6abf3d144c28e4af0c20143383e062/CHANGELOG').should == - ['f4b14494ef6abf3d144c28e4af0c20143383e062', 'CHANGELOG'] - end - - it "extracts a timestamp" do - extract_ref('/relative/gitlab/gitlab-ci/tree/v2.0.0/CHANGELOG?_=12354435').should == ['v2.0.0', 'CHANGELOG'] - end - end end end