improve commit routing

gitlabhq · Mar 20, 2015 · 3d7eb9d · 3d7eb9d
1 parent ff0b455
commit 3d7eb9d
Show file tree

Hide file tree

Showing 9 changed files with 19 additions and 15 deletions.
diff --git a/app/controllers/builds_controller.rb b/app/controllers/builds_controller.rb
@@ -14,7 +14,7 @@ def show
 
       if commit
         # Redirect to commit page
-        redirect_to project_show_commit_path(@project, sha: @build.commit.sha, ref: @build.commit.ref)
+        redirect_to project_ref_commit_path(@project, @build.commit.ref, @build.commit.sha)
         return
       end
     end

diff --git a/app/controllers/commits_controller.rb b/app/controllers/commits_controller.rb
@@ -1,15 +1,14 @@
 class CommitsController < ApplicationController
   before_filter :authenticate_user!, except: [:status]
   before_filter :project
+  before_filter :commit
   before_filter :authorize_access_project!, except: [:status]
 
   def show
-    @commit = project.commits.find_by_sha_and_ref!(params[:sha], params[:ref])
     @builds = @commit.builds
   end
 
   def status
-    @commit = project.commits.find_by(sha: params[:id])
     render json: @commit.to_json(only: [:id, :sha], methods: [:status, :coverage])
   end
 
@@ -18,4 +17,8 @@ def status
   def project
     @project ||= Project.find(params[:project_id])
   end
+
+  def commit
+    @commit ||= Project.find(params[:project_id]).commits.find_by_sha_and_ref!(params[:id], params[:ref_id])
+  end
 end
diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb
@@ -13,6 +13,6 @@ def commit_status_alert_class(commit)
   end
 
   def commit_link(commit)
-    link_to(commit.short_sha, project_show_commit_path(commit.project, sha: commit.sha, ref: commit.ref))
+    link_to(commit.short_sha, project_ref_commit_path(commit.project, commit.ref, commit.sha))
   end
 end
diff --git a/app/models/project_services/slack_message.rb b/app/models/project_services/slack_message.rb
@@ -44,7 +44,7 @@ def attachments
   def attachment_message
     out = "<#{RoutesHelper.project_url(project)}|#{project_name}>: "
     if commit.matrix?
-      out << "Commit <#{RoutesHelper.project_show_commit_url(project, sha: commit.sha, ref: commit.ref)}|\##{commit.id}> "
+      out << "Commit <#{RoutesHelper.project_ref_commit_url(project, commit.ref, commit.sha)}|\##{commit.id}> "
     else
       build = commit.builds_without_retry.first
       out << "Build <#{RoutesHelper.project_build_url(project, build)}|\##{build.id}> "

diff --git a/app/views/builds/show.html.haml b/app/views/builds/show.html.haml
@@ -9,7 +9,7 @@
         Edit Job
 
 %p
-  = link_to project_show_commit_path(@project, sha: @commit.sha, ref: @commit.ref) do
+  = link_to project_ref_commit_path(@project, @commit.ref, @commit.sha) do
     &larr; Back to project commit
 %hr
 #up-build-trace

diff --git a/app/views/commits/_commit.html.haml b/app/views/commits/_commit.html.haml
@@ -3,7 +3,7 @@
     = commit.status
 
   %td.build-link
-    = link_to project_show_commit_path(commit.project, sha: commit.sha, ref: commit.ref) do
+    = link_to project_ref_commit_path(commit.project, commit.ref, commit.sha) do
       %strong #{commit.short_sha}
 
   %td.build-message

diff --git a/config/routes.rb b/config/routes.rb
@@ -28,14 +28,15 @@
     end
 
     resource :charts, only: [:show]
-    resources :commits, only: [] do
-      member do
-        get :status
+
+    resources :refs, constraints: { ref_id: /.*/ }, only: [] do
+      resources :commits, only: [:show] do
+        member do
+          get :status
+        end
       end
     end
 
-    get '/commit/:sha/ref/*ref' => 'commits#show', as: :show_commit
-
     resources :builds, only: [:show] do
       member do
         get :cancel

diff --git a/spec/features/commits_spec.rb b/spec/features/commits_spec.rb
@@ -11,7 +11,7 @@
 
   describe "GET /:project/commits/:sha" do
     before do
-      visit project_show_commit_path(@project, sha: @commit.sha, ref: @commit.ref)
+      visit project_ref_commit_path(@project, @commit.ref, @commit.sha)
     end
 
     it { page.should have_content @commit.sha[0..7] }

diff --git a/spec/requests/commits_spec.rb b/spec/requests/commits_spec.rb
@@ -6,9 +6,9 @@
     @commit = FactoryGirl.create :commit, project: @project
   end
 
-  describe "GET /:project/commits/:id/status.json" do
+  describe "GET /:project/refs/:ref_name/commits/:id/status.json" do
     before do
-      get status_project_commit_path(@project, @commit), format: :json
+      get status_project_ref_commit_path(@project, @commit.ref, @commit.sha), format: :json
     end
 
     it { response.status.should == 200 }