Commit 38ea3939b298b02f8923a092686604235d522b95
Exists in
master
and in
4 other branches
Merge branch 'hiroponz-fix-404-json-file'
Showing
10 changed files
with
100 additions
and
150 deletions
Show diff stats
app/controllers/blame_controller.rb
| @@ -7,8 +7,6 @@ class BlameController < ProjectResourceController | @@ -7,8 +7,6 @@ class BlameController < ProjectResourceController | ||
| 7 | before_filter :authorize_code_access! | 7 | before_filter :authorize_code_access! |
| 8 | before_filter :require_non_empty_project | 8 | before_filter :require_non_empty_project |
| 9 | 9 | ||
| 10 | - before_filter :assign_ref_vars | ||
| 11 | - | ||
| 12 | def show | 10 | def show |
| 13 | @repo = @project.repo | 11 | @repo = @project.repo |
| 14 | @blame = Grit::Blob.blame(@repo, @commit.id, @path) | 12 | @blame = Grit::Blob.blame(@repo, @commit.id, @path) |
app/controllers/blob_controller.rb
| @@ -7,8 +7,6 @@ class BlobController < ProjectResourceController | @@ -7,8 +7,6 @@ class BlobController < ProjectResourceController | ||
| 7 | before_filter :authorize_code_access! | 7 | before_filter :authorize_code_access! |
| 8 | before_filter :require_non_empty_project | 8 | before_filter :require_non_empty_project |
| 9 | 9 | ||
| 10 | - before_filter :assign_ref_vars | ||
| 11 | - | ||
| 12 | def show | 10 | def show |
| 13 | if @tree.is_blob? | 11 | if @tree.is_blob? |
| 14 | send_data( | 12 | send_data( |
| @@ -0,0 +1,47 @@ | @@ -0,0 +1,47 @@ | ||
| 1 | +# Controller for edit a repository's file | ||
| 2 | +class EditTreeController < ProjectResourceController | ||
| 3 | + include ExtractsPath | ||
| 4 | + | ||
| 5 | + # Authorize | ||
| 6 | + before_filter :authorize_read_project! | ||
| 7 | + before_filter :authorize_code_access! | ||
| 8 | + before_filter :require_non_empty_project | ||
| 9 | + | ||
| 10 | + before_filter :edit_requirements, only: [:edit, :update] | ||
| 11 | + | ||
| 12 | + def show | ||
| 13 | + @last_commit = @project.repository.last_commit_for(@ref, @path).sha | ||
| 14 | + end | ||
| 15 | + | ||
| 16 | + def update | ||
| 17 | + edit_file_action = Gitlab::Satellite::EditFileAction.new(current_user, @project, @ref, @path) | ||
| 18 | + updated_successfully = edit_file_action.commit!( | ||
| 19 | + params[:content], | ||
| 20 | + params[:commit_message], | ||
| 21 | + params[:last_commit] | ||
| 22 | + ) | ||
| 23 | + | ||
| 24 | + if updated_successfully | ||
| 25 | + redirect_to project_tree_path(@project, @id), notice: "Your changes have been successfully commited" | ||
| 26 | + else | ||
| 27 | + flash[:notice] = "Your changes could not be commited, because the file has been changed" | ||
| 28 | + render :edit | ||
| 29 | + end | ||
| 30 | + end | ||
| 31 | + | ||
| 32 | + private | ||
| 33 | + | ||
| 34 | + def edit_requirements | ||
| 35 | + unless @tree.is_blob? && @tree.text? | ||
| 36 | + redirect_to project_tree_path(@project, @id), notice: "You can only edit text files" | ||
| 37 | + end | ||
| 38 | + | ||
| 39 | + allowed = if project.protected_branch? @ref | ||
| 40 | + can?(current_user, :push_code_to_protected_branches, project) | ||
| 41 | + else | ||
| 42 | + can?(current_user, :push_code, project) | ||
| 43 | + end | ||
| 44 | + | ||
| 45 | + return access_denied! unless allowed | ||
| 46 | + end | ||
| 47 | +end |
app/controllers/tree_controller.rb
| @@ -7,9 +7,6 @@ class TreeController < ProjectResourceController | @@ -7,9 +7,6 @@ class TreeController < ProjectResourceController | ||
| 7 | before_filter :authorize_code_access! | 7 | before_filter :authorize_code_access! |
| 8 | before_filter :require_non_empty_project | 8 | before_filter :require_non_empty_project |
| 9 | 9 | ||
| 10 | - before_filter :assign_ref_vars | ||
| 11 | - before_filter :edit_requirements, only: [:edit, :update] | ||
| 12 | - | ||
| 13 | def show | 10 | def show |
| 14 | @hex_path = Digest::SHA1.hexdigest(@path) | 11 | @hex_path = Digest::SHA1.hexdigest(@path) |
| 15 | @logs_path = logs_file_project_ref_path(@project, @ref, @path) | 12 | @logs_path = logs_file_project_ref_path(@project, @ref, @path) |
| @@ -20,40 +17,4 @@ class TreeController < ProjectResourceController | @@ -20,40 +17,4 @@ class TreeController < ProjectResourceController | ||
| 20 | format.js { no_cache_headers } | 17 | format.js { no_cache_headers } |
| 21 | end | 18 | end |
| 22 | end | 19 | end |
| 23 | - | ||
| 24 | - def edit | ||
| 25 | - @last_commit = @project.repository.last_commit_for(@ref, @path).sha | ||
| 26 | - end | ||
| 27 | - | ||
| 28 | - def update | ||
| 29 | - edit_file_action = Gitlab::Satellite::EditFileAction.new(current_user, @project, @ref, @path) | ||
| 30 | - updated_successfully = edit_file_action.commit!( | ||
| 31 | - params[:content], | ||
| 32 | - params[:commit_message], | ||
| 33 | - params[:last_commit] | ||
| 34 | - ) | ||
| 35 | - | ||
| 36 | - if updated_successfully | ||
| 37 | - redirect_to project_tree_path(@project, @id), notice: "Your changes have been successfully commited" | ||
| 38 | - else | ||
| 39 | - flash[:notice] = "Your changes could not be commited, because the file has been changed" | ||
| 40 | - render :edit | ||
| 41 | - end | ||
| 42 | - end | ||
| 43 | - | ||
| 44 | - private | ||
| 45 | - | ||
| 46 | - def edit_requirements | ||
| 47 | - unless @tree.is_blob? && @tree.text? | ||
| 48 | - redirect_to project_tree_path(@project, @id), notice: "You can only edit text files" | ||
| 49 | - end | ||
| 50 | - | ||
| 51 | - allowed = if project.protected_branch? @ref | ||
| 52 | - can?(current_user, :push_code_to_protected_branches, project) | ||
| 53 | - else | ||
| 54 | - can?(current_user, :push_code, project) | ||
| 55 | - end | ||
| 56 | - | ||
| 57 | - return access_denied! unless allowed | ||
| 58 | - end | ||
| 59 | end | 20 | end |
| @@ -0,0 +1,44 @@ | @@ -0,0 +1,44 @@ | ||
| 1 | +.file-editor | ||
| 2 | + = form_tag(project_edit_tree_path(@project, @id), method: :put, class: "form-horizontal") do | ||
| 3 | + .file_holder | ||
| 4 | + .file_title | ||
| 5 | + %i.icon-file | ||
| 6 | + %span.file_name | ||
| 7 | + = @tree.path | ||
| 8 | + %small | ||
| 9 | + on | ||
| 10 | + %strong= @ref | ||
| 11 | + %span.options | ||
| 12 | + .btn-group.tree-btn-group | ||
| 13 | + = link_to "Cancel", project_tree_path(@project, @id), class: "btn btn-tiny btn-cancel", confirm: "Are you sure?" | ||
| 14 | + .file_content.code | ||
| 15 | + %pre#editor= @tree.data | ||
| 16 | + | ||
| 17 | + .control-group.commit_message-group | ||
| 18 | + = label_tag 'commit_message', class: "control-label" do | ||
| 19 | + Commit message | ||
| 20 | + .controls | ||
| 21 | + = text_area_tag 'commit_message', '', placeholder: "Update #{@tree.name}", required: true, rows: 3 | ||
| 22 | + .form-actions | ||
| 23 | + = hidden_field_tag 'last_commit', @last_commit | ||
| 24 | + = hidden_field_tag 'content', '', id: :file_content | ||
| 25 | + .commit-button-annotation | ||
| 26 | + = button_tag "Commit", class: 'btn commit-btn js-commit-button' | ||
| 27 | + .message | ||
| 28 | + to branch | ||
| 29 | + %strong= @ref | ||
| 30 | + = link_to "Cancel", project_tree_path(@project, @id), class: "btn btn-cancel", confirm: "Are you sure?" | ||
| 31 | + | ||
| 32 | +:javascript | ||
| 33 | + var ace_mode = "#{@tree.language.try(:ace_mode)}"; | ||
| 34 | + var editor = ace.edit("editor"); | ||
| 35 | + if (ace_mode) { | ||
| 36 | + editor.getSession().setMode('ace/mode/' + ace_mode); | ||
| 37 | + } | ||
| 38 | + | ||
| 39 | + disableButtonIfEmptyField("#commit_message", ".js-commit-button"); | ||
| 40 | + | ||
| 41 | + $(".js-commit-button").click(function(){ | ||
| 42 | + $("#file_content").val(editor.getValue()); | ||
| 43 | + $(".file-editor form").submit(); | ||
| 44 | + }); |
app/views/tree/_blob_actions.html.haml
| 1 | .btn-group.tree-btn-group | 1 | .btn-group.tree-btn-group |
| 2 | -# only show edit link for text files | 2 | -# only show edit link for text files |
| 3 | - if @tree.text? | 3 | - if @tree.text? |
| 4 | - = link_to "edit", edit_project_tree_path(@project, @id), class: "btn btn-tiny", disabled: !allowed_tree_edit? | 4 | + = link_to "edit", project_edit_tree_path(@project, @id), class: "btn btn-tiny", disabled: !allowed_tree_edit? |
| 5 | = link_to "raw", project_blob_path(@project, @id), class: "btn btn-tiny", target: "_blank" | 5 | = link_to "raw", project_blob_path(@project, @id), class: "btn btn-tiny", target: "_blank" |
| 6 | -# only show normal/blame view links for text files | 6 | -# only show normal/blame view links for text files |
| 7 | - if @tree.text? | 7 | - if @tree.text? |
app/views/tree/edit.html.haml
| @@ -1,44 +0,0 @@ | @@ -1,44 +0,0 @@ | ||
| 1 | -.file-editor | ||
| 2 | - = form_tag(project_tree_path(@project, @id), method: :put, class: "form-horizontal") do | ||
| 3 | - .file_holder | ||
| 4 | - .file_title | ||
| 5 | - %i.icon-file | ||
| 6 | - %span.file_name | ||
| 7 | - = @tree.path | ||
| 8 | - %small | ||
| 9 | - on | ||
| 10 | - %strong= @ref | ||
| 11 | - %span.options | ||
| 12 | - .btn-group.tree-btn-group | ||
| 13 | - = link_to "Cancel", project_tree_path(@project, @id), class: "btn btn-tiny btn-cancel", confirm: "Are you sure?" | ||
| 14 | - .file_content.code | ||
| 15 | - %pre#editor= @tree.data | ||
| 16 | - | ||
| 17 | - .control-group.commit_message-group | ||
| 18 | - = label_tag 'commit_message', class: "control-label" do | ||
| 19 | - Commit message | ||
| 20 | - .controls | ||
| 21 | - = text_area_tag 'commit_message', '', placeholder: "Update #{@tree.name}", required: true, rows: 3 | ||
| 22 | - .form-actions | ||
| 23 | - = hidden_field_tag 'last_commit', @last_commit | ||
| 24 | - = hidden_field_tag 'content', '', id: :file_content | ||
| 25 | - .commit-button-annotation | ||
| 26 | - = button_tag "Commit", class: 'btn commit-btn js-commit-button' | ||
| 27 | - .message | ||
| 28 | - to branch | ||
| 29 | - %strong= @ref | ||
| 30 | - = link_to "Cancel", project_tree_path(@project, @id), class: "btn btn-cancel", confirm: "Are you sure?" | ||
| 31 | - | ||
| 32 | -:javascript | ||
| 33 | - var ace_mode = "#{@tree.language.try(:ace_mode)}"; | ||
| 34 | - var editor = ace.edit("editor"); | ||
| 35 | - if (ace_mode) { | ||
| 36 | - editor.getSession().setMode('ace/mode/' + ace_mode); | ||
| 37 | - } | ||
| 38 | - | ||
| 39 | - disableButtonIfEmptyField("#commit_message", ".js-commit-button"); | ||
| 40 | - | ||
| 41 | - $(".js-commit-button").click(function(){ | ||
| 42 | - $("#file_content").val(editor.getValue()); | ||
| 43 | - $(".file-editor form").submit(); | ||
| 44 | - }); |
config/routes.rb
| @@ -168,7 +168,8 @@ Gitlab::Application.routes.draw do | @@ -168,7 +168,8 @@ Gitlab::Application.routes.draw do | ||
| 168 | # | 168 | # |
| 169 | resources :projects, constraints: { id: /(?:[a-zA-Z.0-9_\-]+\/)?[a-zA-Z.0-9_\-]+/ }, except: [:new, :create, :index], path: "/" do | 169 | resources :projects, constraints: { id: /(?:[a-zA-Z.0-9_\-]+\/)?[a-zA-Z.0-9_\-]+/ }, except: [:new, :create, :index], path: "/" do |
| 170 | resources :blob, only: [:show], constraints: {id: /.+/} | 170 | resources :blob, only: [:show], constraints: {id: /.+/} |
| 171 | - resources :tree, only: [:show, :edit, :update], constraints: {id: /.+/} | 171 | + resources :tree, only: [:show], constraints: {id: /.+/, format: /(html|js)/ } |
| 172 | + resources :edit_tree, only: [:show, :update], constraints: {id: /.+/}, path: 'edit' | ||
| 172 | resources :commit, only: [:show], constraints: {id: /[[:alnum:]]{6,40}/} | 173 | resources :commit, only: [:show], constraints: {id: /[[:alnum:]]{6,40}/} |
| 173 | resources :commits, only: [:show], constraints: {id: /(?:[^.]|\.(?!atom$))+/, format: /atom/} | 174 | resources :commits, only: [:show], constraints: {id: /(?:[^.]|\.(?!atom$))+/, format: /atom/} |
| 174 | resources :compare, only: [:index, :create] | 175 | resources :compare, only: [:index, :create] |
lib/extracts_path.rb
| @@ -8,7 +8,7 @@ module ExtractsPath | @@ -8,7 +8,7 @@ module ExtractsPath | ||
| 8 | 8 | ||
| 9 | included do | 9 | included do |
| 10 | if respond_to?(:before_filter) | 10 | if respond_to?(:before_filter) |
| 11 | - before_filter :assign_ref_vars, only: [:show] | 11 | + before_filter :assign_ref_vars |
| 12 | end | 12 | end |
| 13 | end | 13 | end |
| 14 | 14 | ||
| @@ -33,7 +33,7 @@ module ExtractsPath | @@ -33,7 +33,7 @@ module ExtractsPath | ||
| 33 | # extract_ref("v2.0.0/README.md") | 33 | # extract_ref("v2.0.0/README.md") |
| 34 | # # => ['v2.0.0', 'README.md'] | 34 | # # => ['v2.0.0', 'README.md'] |
| 35 | # | 35 | # |
| 36 | - # extract_ref('/gitlab/vagrant/tree/master/app/models/project.rb') | 36 | + # extract_ref('master/app/models/project.rb') |
| 37 | # # => ['master', 'app/models/project.rb'] | 37 | # # => ['master', 'app/models/project.rb'] |
| 38 | # | 38 | # |
| 39 | # extract_ref('issues/1234/app/models/project.rb') | 39 | # extract_ref('issues/1234/app/models/project.rb') |
| @@ -45,22 +45,12 @@ module ExtractsPath | @@ -45,22 +45,12 @@ module ExtractsPath | ||
| 45 | # | 45 | # |
| 46 | # Returns an Array where the first value is the tree-ish and the second is the | 46 | # Returns an Array where the first value is the tree-ish and the second is the |
| 47 | # path | 47 | # path |
| 48 | - def extract_ref(input) | 48 | + def extract_ref(id) |
| 49 | pair = ['', ''] | 49 | pair = ['', ''] |
| 50 | 50 | ||
| 51 | return pair unless @project | 51 | return pair unless @project |
| 52 | 52 | ||
| 53 | - # Remove relative_url_root from path | ||
| 54 | - input.gsub!(/^#{Gitlab.config.gitlab.relative_url_root}/, "") | ||
| 55 | - # Remove project, actions and all other staff from path | ||
| 56 | - input.gsub!(/^\/#{Regexp.escape(@project.path_with_namespace)}/, "") | ||
| 57 | - input.gsub!(/^\/(tree|commits|blame|blob|refs|graph)\//, "") # remove actions | ||
| 58 | - input.gsub!(/\?.*$/, "") # remove stamps suffix | ||
| 59 | - input.gsub!(/.atom$/, "") # remove rss feed | ||
| 60 | - input.gsub!(/.json$/, "") # remove json suffix | ||
| 61 | - input.gsub!(/\/edit$/, "") # remove edit route part | ||
| 62 | - | ||
| 63 | - if input.match(/^([[:alnum:]]{40})(.+)/) | 53 | + if id.match(/^([[:alnum:]]{40})(.+)/) |
| 64 | # If the ref appears to be a SHA, we're done, just split the string | 54 | # If the ref appears to be a SHA, we're done, just split the string |
| 65 | pair = $~.captures | 55 | pair = $~.captures |
| 66 | else | 56 | else |
| @@ -68,7 +58,6 @@ module ExtractsPath | @@ -68,7 +58,6 @@ module ExtractsPath | ||
| 68 | # branches and tags | 58 | # branches and tags |
| 69 | 59 | ||
| 70 | # Append a trailing slash if we only get a ref and no file path | 60 | # Append a trailing slash if we only get a ref and no file path |
| 71 | - id = input | ||
| 72 | id += '/' unless id.ends_with?('/') | 61 | id += '/' unless id.ends_with?('/') |
| 73 | 62 | ||
| 74 | valid_refs = @project.repository.ref_names | 63 | valid_refs = @project.repository.ref_names |
| @@ -105,11 +94,9 @@ module ExtractsPath | @@ -105,11 +94,9 @@ module ExtractsPath | ||
| 105 | # Automatically renders `not_found!` if a valid tree path could not be | 94 | # Automatically renders `not_found!` if a valid tree path could not be |
| 106 | # resolved (e.g., when a user inserts an invalid path or ref). | 95 | # resolved (e.g., when a user inserts an invalid path or ref). |
| 107 | def assign_ref_vars | 96 | def assign_ref_vars |
| 108 | - path = CGI::unescape(request.fullpath.dup) | ||
| 109 | - | ||
| 110 | - @ref, @path = extract_ref(path) | 97 | + @id = params[:id] |
| 111 | 98 | ||
| 112 | - @id = File.join(@ref, @path) | 99 | + @ref, @path = extract_ref(@id) |
| 113 | 100 | ||
| 114 | # It is used "@project.repository.commits(@ref, @path, 1, 0)", | 101 | # It is used "@project.repository.commits(@ref, @path, 1, 0)", |
| 115 | # because "@project.repository.commit(@ref)" returns wrong commit when @ref is tag name. | 102 | # because "@project.repository.commit(@ref)" returns wrong commit when @ref is tag name. |
spec/lib/extracts_path_spec.rb
| @@ -54,47 +54,5 @@ describe ExtractsPath do | @@ -54,47 +54,5 @@ describe ExtractsPath do | ||
| 54 | extract_ref('stable/CHANGELOG').should == ['stable', 'CHANGELOG'] | 54 | extract_ref('stable/CHANGELOG').should == ['stable', 'CHANGELOG'] |
| 55 | end | 55 | end |
| 56 | end | 56 | end |
| 57 | - | ||
| 58 | - context "with a fullpath" do | ||
| 59 | - it "extracts a valid branch" do | ||
| 60 | - extract_ref('/gitlab/gitlab-ci/tree/foo/bar/baz/CHANGELOG').should == ['foo/bar/baz', 'CHANGELOG'] | ||
| 61 | - end | ||
| 62 | - | ||
| 63 | - it "extracts a valid tag" do | ||
| 64 | - extract_ref('/gitlab/gitlab-ci/tree/v2.0.0/CHANGELOG').should == ['v2.0.0', 'CHANGELOG'] | ||
| 65 | - end | ||
| 66 | - | ||
| 67 | - it "extracts a valid commit SHA" do | ||
| 68 | - extract_ref('/gitlab/gitlab-ci/tree/f4b14494ef6abf3d144c28e4af0c20143383e062/CHANGELOG').should == | ||
| 69 | - ['f4b14494ef6abf3d144c28e4af0c20143383e062', 'CHANGELOG'] | ||
| 70 | - end | ||
| 71 | - | ||
| 72 | - it "extracts a timestamp" do | ||
| 73 | - extract_ref('/gitlab/gitlab-ci/tree/v2.0.0/CHANGELOG?_=12354435').should == ['v2.0.0', 'CHANGELOG'] | ||
| 74 | - end | ||
| 75 | - end | ||
| 76 | - | ||
| 77 | - context "with a fullpath and a relative_url_root" do | ||
| 78 | - before do | ||
| 79 | - Gitlab.config.gitlab.stub(relative_url_root: '/relative') | ||
| 80 | - end | ||
| 81 | - | ||
| 82 | - it "extracts a valid branch with relative_url_root" do | ||
| 83 | - extract_ref('/relative/gitlab/gitlab-ci/tree/foo/bar/baz/CHANGELOG').should == ['foo/bar/baz', 'CHANGELOG'] | ||
| 84 | - end | ||
| 85 | - | ||
| 86 | - it "extracts a valid tag" do | ||
| 87 | - extract_ref('/relative/gitlab/gitlab-ci/tree/v2.0.0/CHANGELOG').should == ['v2.0.0', 'CHANGELOG'] | ||
| 88 | - end | ||
| 89 | - | ||
| 90 | - it "extracts a valid commit SHA" do | ||
| 91 | - extract_ref('/relative/gitlab/gitlab-ci/tree/f4b14494ef6abf3d144c28e4af0c20143383e062/CHANGELOG').should == | ||
| 92 | - ['f4b14494ef6abf3d144c28e4af0c20143383e062', 'CHANGELOG'] | ||
| 93 | - end | ||
| 94 | - | ||
| 95 | - it "extracts a timestamp" do | ||
| 96 | - extract_ref('/relative/gitlab/gitlab-ci/tree/v2.0.0/CHANGELOG?_=12354435').should == ['v2.0.0', 'CHANGELOG'] | ||
| 97 | - end | ||
| 98 | - end | ||
| 99 | end | 57 | end |
| 100 | end | 58 | end |