From 39c657930625ddc3ac8a921f01ffc83acadce68f Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 17 Sep 2012 13:36:13 -0400 Subject: [PATCH] Add BlameController, remove Refs#blame action --- app/controllers/blame_controller.rb | 45 +++++++++++++++++++ app/controllers/refs_controller.rb | 6 +-- app/views/{refs => blame}/_head.html.haml | 0 app/views/blame/show.html.haml | 2 +- app/views/tree/_tree_file.html.haml | 2 +- config/routes.rb | 10 +---- .../requests/gitlab_flavored_markdown_spec.rb | 2 +- spec/routing/project_routing_spec.rb | 17 +++---- 8 files changed, 57 insertions(+), 27 deletions(-) create mode 100644 app/controllers/blame_controller.rb rename app/views/{refs => blame}/_head.html.haml (100%) diff --git a/app/controllers/blame_controller.rb b/app/controllers/blame_controller.rb new file mode 100644 index 00000000000..d159a83c8cb --- /dev/null +++ b/app/controllers/blame_controller.rb @@ -0,0 +1,45 @@ +# Controller for viewing a file's blame +class BlameController < ApplicationController + # Thrown when given an invalid path + class InvalidPathError < StandardError; end + + include RefExtractor + + layout "project" + + before_filter :project + + # Authorize + before_filter :add_project_abilities + before_filter :authorize_read_project! + before_filter :authorize_code_access! + before_filter :require_non_empty_project + + before_filter :define_tree_vars + + def show + @blame = Grit::Blob.blame(@repo, @commit.id, @path) + end + + private + + def define_tree_vars + @ref, @path = extract_ref(params[:id]) + + @id = File.join(@ref, @path) + @repo = @project.repo + @commit = CommitDecorator.decorate(@project.commit(@ref)) + + @tree = Tree.new(@commit.tree, @project, @ref, @path) + @tree = TreeDecorator.new(@tree) + + raise InvalidPathError if @tree.invalid? + + @hex_path = Digest::SHA1.hexdigest(@path) + + @history_path = project_tree_path(@project, @id) + @logs_path = logs_file_project_ref_path(@project, @ref, @path) + rescue NoMethodError, InvalidPathError + not_found! + end +end diff --git a/app/controllers/refs_controller.rb b/app/controllers/refs_controller.rb index 6cdd095311e..334dfc7f0cf 100644 --- a/app/controllers/refs_controller.rb +++ b/app/controllers/refs_controller.rb @@ -9,7 +9,7 @@ class RefsController < ApplicationController before_filter :require_non_empty_project before_filter :ref - before_filter :define_tree_vars, only: [:tree, :blob, :blame, :logs_tree] + before_filter :define_tree_vars, only: [:blob, :logs_tree] before_filter :render_full_content layout "project" @@ -79,10 +79,6 @@ class RefsController < ApplicationController end end - def blame - @blame = Grit::Blob.blame(@repo, @commit.id, params[:path]) - end - protected def define_tree_vars diff --git a/app/views/refs/_head.html.haml b/app/views/blame/_head.html.haml similarity index 100% rename from app/views/refs/_head.html.haml rename to app/views/blame/_head.html.haml diff --git a/app/views/blame/show.html.haml b/app/views/blame/show.html.haml index 75b86315031..0b2f93bea2b 100644 --- a/app/views/blame/show.html.haml +++ b/app/views/blame/show.html.haml @@ -20,7 +20,7 @@ %span.options = link_to "raw", blob_project_ref_path(@project, @ref, path: params[:path]), class: "btn very_small", target: "_blank" = link_to "history", project_commits_path(@project, path: params[:path], ref: @ref), class: "btn very_small" - = link_to "source", project_tree_path(@project, tree_join(@ref, params[:path])), class: "btn very_small" + = link_to "source", project_tree_path(@project, @id), class: "btn very_small" .file_content.blame %table - @blame.each do |commit, lines| diff --git a/app/views/tree/_tree_file.html.haml b/app/views/tree/_tree_file.html.haml index ae8f1ccf7bc..052172be008 100644 --- a/app/views/tree/_tree_file.html.haml +++ b/app/views/tree/_tree_file.html.haml @@ -7,7 +7,7 @@ %span.options = link_to "raw", blob_project_ref_path(@project, @ref, path: @path), class: "btn very_small", target: "_blank" = link_to "history", project_commits_path(@project, path: params[:path], ref: @ref), class: "btn very_small" - = link_to "blame", blame_file_project_ref_path(@project, @ref, path: @path.gsub(/^\//, '')), class: "btn very_small" + = link_to "blame", project_blame_path(@project, @id), class: "btn very_small" - if file.text? - if gitlab_markdown?(name) .file_content.wiki diff --git a/config/routes.rb b/config/routes.rb index d1ed8749d95..1aa10c5d1d6 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -136,14 +136,6 @@ Gitlab::Application.routes.draw do id: /[a-zA-Z.0-9\/_\-]+/, path: /.*/ } - - # blame - get "blame/:path" => "refs#blame", - as: :blame_file, - constraints: { - id: /[a-zA-Z.0-9\/_\-]+/, - path: /.*/ - } end end @@ -204,7 +196,7 @@ Gitlab::Application.routes.draw do end # XXX: WIP - # resources :blame, only: [:show], constraints: {id: /.+/} + resources :blame, only: [:show], constraints: {id: /.+/} # resources :blob, only: [:show], constraints: {id: /.+/} # resources :raw, only: [:show], constraints: {id: /.+/} resources :tree, only: [:show], constraints: {id: /.+/} diff --git a/spec/requests/gitlab_flavored_markdown_spec.rb b/spec/requests/gitlab_flavored_markdown_spec.rb index db3f89cbe9b..2e5d4209315 100644 --- a/spec/requests/gitlab_flavored_markdown_spec.rb +++ b/spec/requests/gitlab_flavored_markdown_spec.rb @@ -69,7 +69,7 @@ describe "Gitlab Flavored Markdown" do end it "should render title in refs#blame" do - visit blame_file_project_ref_path(project, id: @branch_name, path: @test_file) + visit blame_file_project_ref_path(project, File.join(@branch_name, @test_file)) within(".blame_commit") do page.should have_link("##{issue.id}") diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index f9c99a209fc..303df02c08b 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -195,7 +195,6 @@ end # blob_project_ref GET /:project_id/:id/blob(.:format) refs#blob # logs_tree_project_ref GET /:project_id/:id/logs_tree(.:format) refs#logs_tree # logs_file_project_ref GET /:project_id/:id/logs_tree/:path(.:format) refs#logs_tree -# blame_file_project_ref GET /:project_id/:id/blame/:path(.:format) refs#blame describe RefsController, "routing" do it "to #switch" do get("/gitlabhq/switch").should route_to('refs#switch', project_id: 'gitlabhq') @@ -209,10 +208,6 @@ describe RefsController, "routing" do it "to #blob" do get("/gitlabhq/stable/blob").should route_to('refs#blob', project_id: 'gitlabhq', id: 'stable') end - - it "to #blame" do - get("/gitlabhq/stable/blame/foo/bar/baz").should route_to('refs#blame', project_id: 'gitlabhq', id: 'stable', path: 'foo/bar/baz') - end end # diffs_project_merge_request GET /:project_id/merge_requests/:id/diffs(.:format) merge_requests#diffs @@ -399,6 +394,12 @@ describe NotesController, "routing" do end end +describe BlameController, "routing" do + it "to #show" do + get("/gitlabhq/blame/master/app/models/project.rb").should route_to('blame#show', project_id: 'gitlabhq', id: 'master/app/models/project.rb') + end +end + describe TreeController, "routing" do it "to #show" do get("/gitlabhq/tree/master/app/models/project.rb").should route_to('tree#show', project_id: 'gitlabhq', id: 'master/app/models/project.rb') @@ -432,11 +433,7 @@ end # /gitlabhq/tree/master/app # /gitlabhq/tree/test/branch/name/app describe "pending routing" do - describe "/:project_id/blame/:id" do - it "routes to a ref with a path" do - get("/gitlabhq/blame/master/app/models/project.rb").should route_to('blame#show', project_id: 'gitlabhq', id: 'master/app/models/project.rb') - end - end + before { pending } describe "/:project_id/blob/:id" do it "routes to a ref with a path" do