Add BlameController, remove Refs#blame action

8 files changed, 57 insertions, 27 deletions

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
index 2a6052a1122..2a6052a1122 100644
--- a/app/views/refs/_head.html.haml
+++ b/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