Commit e38e2ce29a2d0ce731bafd5012eae8d21de05d4b
Exists in
master
and in
4 other branches
Merge pull request #3154 from hiroponz/fix-routing-error
Fix routing error
Showing
5 changed files
with
21 additions
and
15 deletions
Show diff stats
config/routes.rb
| @@ -165,7 +165,7 @@ Gitlab::Application.routes.draw do | @@ -165,7 +165,7 @@ Gitlab::Application.routes.draw do | ||
| 165 | # | 165 | # |
| 166 | # Project Area | 166 | # Project Area |
| 167 | # | 167 | # |
| 168 | - resources :projects, constraints: { id: /[a-zA-Z.0-9_\-\/]+/ }, except: [:new, :create, :index], path: "/" do | 168 | + resources :projects, constraints: { id: /(?:[a-zA-Z.0-9_\-]+\/)?[a-zA-Z.0-9_\-]+/ }, except: [:new, :create, :index], path: "/" do |
| 169 | member do | 169 | member do |
| 170 | get "wall" | 170 | get "wall" |
| 171 | get "files" | 171 | get "files" |
| @@ -174,10 +174,10 @@ Gitlab::Application.routes.draw do | @@ -174,10 +174,10 @@ Gitlab::Application.routes.draw do | ||
| 174 | resources :blob, only: [:show], constraints: {id: /.+/} | 174 | resources :blob, only: [:show], constraints: {id: /.+/} |
| 175 | resources :tree, only: [:show, :edit, :update], constraints: {id: /.+/} | 175 | resources :tree, only: [:show, :edit, :update], constraints: {id: /.+/} |
| 176 | resources :commit, only: [:show], constraints: {id: /[[:alnum:]]{6,40}/} | 176 | resources :commit, only: [:show], constraints: {id: /[[:alnum:]]{6,40}/} |
| 177 | - resources :commits, only: [:show], constraints: {id: /.+/} | 177 | + resources :commits, only: [:show], constraints: {id: /(?:[^.]|\.(?!atom$))+/, format: /atom/} |
| 178 | resources :compare, only: [:index, :create] | 178 | resources :compare, only: [:index, :create] |
| 179 | resources :blame, only: [:show], constraints: {id: /.+/} | 179 | resources :blame, only: [:show], constraints: {id: /.+/} |
| 180 | - resources :graph, only: [:show], constraints: {id: /.+/} | 180 | + resources :graph, only: [:show], constraints: {id: /(?:[^.]|\.(?!json$))+/, format: /json/} |
| 181 | match "/compare/:from...:to" => "compare#show", as: "compare", | 181 | match "/compare/:from...:to" => "compare#show", as: "compare", |
| 182 | :via => [:get, :post], constraints: {from: /.+/, to: /.+/} | 182 | :via => [:get, :post], constraints: {from: /.+/, to: /.+/} |
| 183 | 183 |
features/steps/project/project_network_graph.rb
| @@ -27,6 +27,7 @@ class ProjectNetworkGraph < Spinach::FeatureSteps | @@ -27,6 +27,7 @@ class ProjectNetworkGraph < Spinach::FeatureSteps | ||
| 27 | 27 | ||
| 28 | And 'I switch ref to "stable"' do | 28 | And 'I switch ref to "stable"' do |
| 29 | page.select 'stable', :from => 'ref' | 29 | page.select 'stable', :from => 'ref' |
| 30 | + sleep 2 | ||
| 30 | end | 31 | end |
| 31 | 32 | ||
| 32 | And 'page should select "stable" in select box' do | 33 | And 'page should select "stable" in select box' do |
| @@ -44,6 +45,7 @@ class ProjectNetworkGraph < Spinach::FeatureSteps | @@ -44,6 +45,7 @@ class ProjectNetworkGraph < Spinach::FeatureSteps | ||
| 44 | fill_in 'q', :with => '98d6492' | 45 | fill_in 'q', :with => '98d6492' |
| 45 | find('button').click | 46 | find('button').click |
| 46 | end | 47 | end |
| 48 | + sleep 2 | ||
| 47 | end | 49 | end |
| 48 | 50 | ||
| 49 | And 'page should have "v2.1.0" on graph' do | 51 | And 'page should have "v2.1.0" on graph' do |
lib/extracts_path.rb
| @@ -105,12 +105,6 @@ module ExtractsPath | @@ -105,12 +105,6 @@ module ExtractsPath | ||
| 105 | # Automatically renders `not_found!` if a valid tree path could not be | 105 | # 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). | 106 | # resolved (e.g., when a user inserts an invalid path or ref). |
| 107 | def assign_ref_vars | 107 | def assign_ref_vars |
| 108 | - # Handle formats embedded in the id | ||
| 109 | - if params[:id].ends_with?('.atom') | ||
| 110 | - params[:id].gsub!(/\.atom$/, '') | ||
| 111 | - request.format = :atom | ||
| 112 | - end | ||
| 113 | - | ||
| 114 | path = CGI::unescape(request.fullpath.dup) | 108 | path = CGI::unescape(request.fullpath.dup) |
| 115 | 109 | ||
| 116 | @ref, @path = extract_ref(path) | 110 | @ref, @path = extract_ref(path) |
spec/controllers/commits_controller_spec.rb
| @@ -13,7 +13,7 @@ describe CommitsController do | @@ -13,7 +13,7 @@ describe CommitsController do | ||
| 13 | describe "GET show" do | 13 | describe "GET show" do |
| 14 | context "as atom feed" do | 14 | context "as atom feed" do |
| 15 | it "should render as atom" do | 15 | it "should render as atom" do |
| 16 | - get :show, project_id: project.path, id: "master.atom" | 16 | + get :show, project_id: project.path, id: "master", format: "atom" |
| 17 | response.should be_success | 17 | response.should be_success |
| 18 | response.content_type.should == 'application/atom+xml' | 18 | response.content_type.should == 'application/atom+xml' |
| 19 | end | 19 | end |
spec/routing/project_routing_spec.rb
| @@ -56,7 +56,6 @@ end | @@ -56,7 +56,6 @@ end | ||
| 56 | # projects POST /projects(.:format) projects#create | 56 | # projects POST /projects(.:format) projects#create |
| 57 | # new_project GET /projects/new(.:format) projects#new | 57 | # new_project GET /projects/new(.:format) projects#new |
| 58 | # wall_project GET /:id/wall(.:format) projects#wall | 58 | # wall_project GET /:id/wall(.:format) projects#wall |
| 59 | -# graph_project GET /:id/graph(.:format) projects#graph | ||
| 60 | # files_project GET /:id/files(.:format) projects#files | 59 | # files_project GET /:id/files(.:format) projects#files |
| 61 | # edit_project GET /:id/edit(.:format) projects#edit | 60 | # edit_project GET /:id/edit(.:format) projects#edit |
| 62 | # project GET /:id(.:format) projects#show | 61 | # project GET /:id(.:format) projects#show |
| @@ -75,10 +74,6 @@ describe ProjectsController, "routing" do | @@ -75,10 +74,6 @@ describe ProjectsController, "routing" do | ||
| 75 | get("/gitlabhq/wall").should route_to('projects#wall', id: 'gitlabhq') | 74 | get("/gitlabhq/wall").should route_to('projects#wall', id: 'gitlabhq') |
| 76 | end | 75 | end |
| 77 | 76 | ||
| 78 | - it "to #graph" do | ||
| 79 | - get("/gitlabhq/graph/master").should route_to('graph#show', project_id: 'gitlabhq', id: 'master') | ||
| 80 | - end | ||
| 81 | - | ||
| 82 | it "to #files" do | 77 | it "to #files" do |
| 83 | get("/gitlabhq/files").should route_to('projects#files', id: 'gitlabhq') | 78 | get("/gitlabhq/files").should route_to('projects#files', id: 'gitlabhq') |
| 84 | end | 79 | end |
| @@ -202,6 +197,7 @@ describe RefsController, "routing" do | @@ -202,6 +197,7 @@ describe RefsController, "routing" do | ||
| 202 | it "to #logs_tree" do | 197 | it "to #logs_tree" do |
| 203 | get("/gitlabhq/refs/stable/logs_tree").should route_to('refs#logs_tree', project_id: 'gitlabhq', id: 'stable') | 198 | get("/gitlabhq/refs/stable/logs_tree").should route_to('refs#logs_tree', project_id: 'gitlabhq', id: 'stable') |
| 204 | get("/gitlabhq/refs/stable/logs_tree/foo/bar/baz").should route_to('refs#logs_tree', project_id: 'gitlabhq', id: 'stable', path: 'foo/bar/baz') | 199 | get("/gitlabhq/refs/stable/logs_tree/foo/bar/baz").should route_to('refs#logs_tree', project_id: 'gitlabhq', id: 'stable', path: 'foo/bar/baz') |
| 200 | + get("/gitlab/gitlabhq/refs/stable/logs_tree/files.scss").should route_to('refs#logs_tree', project_id: 'gitlab/gitlabhq', id: 'stable', path: 'files.scss') | ||
| 205 | end | 201 | end |
| 206 | end | 202 | end |
| 207 | 203 | ||
| @@ -301,6 +297,10 @@ describe CommitsController, "routing" do | @@ -301,6 +297,10 @@ describe CommitsController, "routing" do | ||
| 301 | let(:actions) { [:show] } | 297 | let(:actions) { [:show] } |
| 302 | let(:controller) { 'commits' } | 298 | let(:controller) { 'commits' } |
| 303 | end | 299 | end |
| 300 | + | ||
| 301 | + it "to #show" do | ||
| 302 | + get("/gitlab/gitlabhq/commits/master.atom").should route_to('commits#show', project_id: 'gitlab/gitlabhq', id: "master", format: "atom") | ||
| 303 | + end | ||
| 304 | end | 304 | end |
| 305 | 305 | ||
| 306 | # project_team_members GET /:project_id/team_members(.:format) team_members#index | 306 | # project_team_members GET /:project_id/team_members(.:format) team_members#index |
| @@ -385,6 +385,7 @@ end | @@ -385,6 +385,7 @@ end | ||
| 385 | describe BlameController, "routing" do | 385 | describe BlameController, "routing" do |
| 386 | it "to #show" do | 386 | it "to #show" do |
| 387 | get("/gitlabhq/blame/master/app/models/project.rb").should route_to('blame#show', project_id: 'gitlabhq', id: 'master/app/models/project.rb') | 387 | get("/gitlabhq/blame/master/app/models/project.rb").should route_to('blame#show', project_id: 'gitlabhq', id: 'master/app/models/project.rb') |
| 388 | + get("/gitlab/gitlabhq/blame/master/files.scss").should route_to('blame#show', project_id: 'gitlab/gitlabhq', id: 'master/files.scss') | ||
| 388 | end | 389 | end |
| 389 | end | 390 | end |
| 390 | 391 | ||
| @@ -393,6 +394,7 @@ describe BlobController, "routing" do | @@ -393,6 +394,7 @@ describe BlobController, "routing" do | ||
| 393 | it "to #show" do | 394 | it "to #show" do |
| 394 | get("/gitlabhq/blob/master/app/models/project.rb").should route_to('blob#show', project_id: 'gitlabhq', id: 'master/app/models/project.rb') | 395 | get("/gitlabhq/blob/master/app/models/project.rb").should route_to('blob#show', project_id: 'gitlabhq', id: 'master/app/models/project.rb') |
| 395 | get("/gitlabhq/blob/master/app/models/compare.rb").should route_to('blob#show', project_id: 'gitlabhq', id: 'master/app/models/compare.rb') | 396 | get("/gitlabhq/blob/master/app/models/compare.rb").should route_to('blob#show', project_id: 'gitlabhq', id: 'master/app/models/compare.rb') |
| 397 | + get("/gitlab/gitlabhq/blob/master/files.scss").should route_to('blob#show', project_id: 'gitlab/gitlabhq', id: 'master/files.scss') | ||
| 396 | end | 398 | end |
| 397 | end | 399 | end |
| 398 | 400 | ||
| @@ -400,6 +402,7 @@ end | @@ -400,6 +402,7 @@ end | ||
| 400 | describe TreeController, "routing" do | 402 | describe TreeController, "routing" do |
| 401 | it "to #show" do | 403 | it "to #show" do |
| 402 | get("/gitlabhq/tree/master/app/models/project.rb").should route_to('tree#show', project_id: 'gitlabhq', id: 'master/app/models/project.rb') | 404 | get("/gitlabhq/tree/master/app/models/project.rb").should route_to('tree#show', project_id: 'gitlabhq', id: 'master/app/models/project.rb') |
| 405 | + get("/gitlab/gitlabhq/tree/master/files.scss").should route_to('tree#show', project_id: 'gitlab/gitlabhq', id: 'master/files.scss') | ||
| 403 | end | 406 | end |
| 404 | end | 407 | end |
| 405 | 408 | ||
| @@ -420,3 +423,10 @@ describe CompareController, "routing" do | @@ -420,3 +423,10 @@ describe CompareController, "routing" do | ||
| 420 | get("/gitlabhq/compare/issue/1234...stable").should route_to('compare#show', project_id: 'gitlabhq', from: 'issue/1234', to: 'stable') | 423 | get("/gitlabhq/compare/issue/1234...stable").should route_to('compare#show', project_id: 'gitlabhq', from: 'issue/1234', to: 'stable') |
| 421 | end | 424 | end |
| 422 | end | 425 | end |
| 426 | + | ||
| 427 | +describe GraphController, "routing" do | ||
| 428 | + it "to #show" do | ||
| 429 | + get("/gitlabhq/graph/master").should route_to('graph#show', project_id: 'gitlabhq', id: 'master') | ||
| 430 | + get("/gitlabhq/graph/master.json").should route_to('graph#show', project_id: 'gitlabhq', id: 'master', format: "json") | ||
| 431 | + end | ||
| 432 | +end |