Merge branch '1447-custom-domain-500' into 'master'

Fix 500 error while navigating to the `pages_domains` 'show' page.

Closes #1447 and #1448

See merge request !993

changelogs/unreleased-ee/1447-custom-domain-500.yml

+---
+title: Fix 404/500 error while navigating to the 'show/destroy' pages.
+merge_request: 993
+author: 

config/routes/project.rb

@@ -41,7 +41,7 @@ constraints(ProjectUrlConstrainer.new) do
 
       ## EE-specific
       resource :pages, only: [:show, :destroy] do
-        resources :domains, only: [:show, :new, :create, :destroy], controller: 'pages_domains'
+        resources :domains, only: [:show, :new, :create, :destroy], controller: 'pages_domains', constraints: { id: /[^\/]+/ }
       end
       ## EE-specific
 

spec/controllers/projects/pages_domains_controller_spec.rb

+require 'spec_helper'
+
+describe Projects::PagesDomainsController do
+  let(:user)    { create(:user) }
+  let(:project) { create(:project) }
+
+  let(:request_params) do
+    {
+      namespace_id: project.namespace,
+      project_id: project
+    }
+  end
+
+  before do
+    sign_in(user)
+    project.team << [user, :master]
+  end
+
+  describe 'GET show' do
+    let!(:pages_domain)   { create(:pages_domain, project: project) }
+
+    it "displays the 'show' page" do
+      get(:show, request_params.merge(id: pages_domain.domain))
+
+      expect(response).to have_http_status(200)
+      expect(response).to render_template('show')
+    end
+  end
+
+  describe 'GET new' do
+    it "displays the 'new' page" do
+      get(:new, request_params)
+
+      expect(response).to have_http_status(200)
+      expect(response).to render_template('new')
+    end
+  end
+
+  describe 'POST create' do
+    let(:pages_domain_params) do
+      build(:pages_domain, :with_certificate, :with_key).slice(:key, :certificate, :domain)
+    end
+
+    it "creates a new pages domain" do
+      expect do
+        post(:create, request_params.merge(pages_domain: pages_domain_params))
+      end.to change { PagesDomain.count }.by(1)
+
+      expect(response).to redirect_to(namespace_project_pages_path(project.namespace, project))
+    end
+  end
+
+  describe 'DELETE destroy' do
+    let!(:pages_domain)   { create(:pages_domain, project: project) }
+
+    it "deletes the pages domain" do
+      expect do
+        delete(:destroy, request_params.merge(id: pages_domain.domain))
+      end.to change { PagesDomain.count }.by(-1)
+
+      expect(response).to redirect_to(namespace_project_pages_path(project.namespace, project))
+    end
+  end
+end

spec/routing/project_routing_spec.rb

@@ -27,35 +27,42 @@ describe 'project routing' do
   #     let(:actions)    { [:index] }
   #     let(:controller) { 'issues' }
   #   end
+  #
+  #   # Different controller name and path
+  #   it_behaves_like 'RESTful project resources' do
+  #     let(:controller) { 'pages_domains' }
+  #     let(:controller_path) { 'pages/domains' }
+  #   end
   shared_examples 'RESTful project resources' do
     let(:actions) { [:index, :create, :new, :edit, :show, :update, :destroy] }
+    let(:controller_path) { controller }
 
     it 'to #index' do
-      expect(get("/gitlab/gitlabhq/#{controller}")).to route_to("projects/#{controller}#index", namespace_id: 'gitlab', project_id: 'gitlabhq') if actions.include?(:index)
+      expect(get("/gitlab/gitlabhq/#{controller_path}")).to route_to("projects/#{controller}#index", namespace_id: 'gitlab', project_id: 'gitlabhq') if actions.include?(:index)
     end
 
     it 'to #create' do
-      expect(post("/gitlab/gitlabhq/#{controller}")).to route_to("projects/#{controller}#create", namespace_id: 'gitlab', project_id: 'gitlabhq') if actions.include?(:create)
+      expect(post("/gitlab/gitlabhq/#{controller_path}")).to route_to("projects/#{controller}#create", namespace_id: 'gitlab', project_id: 'gitlabhq') if actions.include?(:create)
     end
 
     it 'to #new' do
-      expect(get("/gitlab/gitlabhq/#{controller}/new")).to route_to("projects/#{controller}#new", namespace_id: 'gitlab', project_id: 'gitlabhq') if actions.include?(:new)
+      expect(get("/gitlab/gitlabhq/#{controller_path}/new")).to route_to("projects/#{controller}#new", namespace_id: 'gitlab', project_id: 'gitlabhq') if actions.include?(:new)
     end
 
     it 'to #edit' do
-      expect(get("/gitlab/gitlabhq/#{controller}/1/edit")).to route_to("projects/#{controller}#edit", namespace_id: 'gitlab', project_id: 'gitlabhq', id: '1') if actions.include?(:edit)
+      expect(get("/gitlab/gitlabhq/#{controller_path}/1/edit")).to route_to("projects/#{controller}#edit", namespace_id: 'gitlab', project_id: 'gitlabhq', id: '1') if actions.include?(:edit)
     end
 
     it 'to #show' do
-      expect(get("/gitlab/gitlabhq/#{controller}/1")).to route_to("projects/#{controller}#show", namespace_id: 'gitlab', project_id: 'gitlabhq', id: '1') if actions.include?(:show)
+      expect(get("/gitlab/gitlabhq/#{controller_path}/1")).to route_to("projects/#{controller}#show", namespace_id: 'gitlab', project_id: 'gitlabhq', id: '1') if actions.include?(:show)
     end
 
     it 'to #update' do
-      expect(put("/gitlab/gitlabhq/#{controller}/1")).to route_to("projects/#{controller}#update", namespace_id: 'gitlab', project_id: 'gitlabhq', id: '1') if actions.include?(:update)
+      expect(put("/gitlab/gitlabhq/#{controller_path}/1")).to route_to("projects/#{controller}#update", namespace_id: 'gitlab', project_id: 'gitlabhq', id: '1') if actions.include?(:update)
     end
 
     it 'to #destroy' do
-      expect(delete("/gitlab/gitlabhq/#{controller}/1")).to route_to("projects/#{controller}#destroy", namespace_id: 'gitlab', project_id: 'gitlabhq', id: '1') if actions.include?(:destroy)
+      expect(delete("/gitlab/gitlabhq/#{controller_path}/1")).to route_to("projects/#{controller}#destroy", namespace_id: 'gitlab', project_id: 'gitlabhq', id: '1') if actions.include?(:destroy)
     end
   end
 
@@ -539,4 +546,20 @@ describe 'project routing' do
         'projects/avatars#destroy', namespace_id: 'gitlab', project_id: 'gitlabhq')
     end
   end
+
+  describe Projects::PagesDomainsController, 'routing' do
+    it_behaves_like 'RESTful project resources' do
+      let(:actions)    { [:show, :new, :create, :destroy] }
+      let(:controller) { 'pages_domains' }
+      let(:controller_path) { 'pages/domains' }
+    end
+
+    it 'to #destroy with a valid domain name' do
+      expect(delete('/gitlab/gitlabhq/pages/domains/my.domain.com')).to route_to('projects/pages_domains#destroy', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'my.domain.com')
+    end
+
+    it 'to #show with a valid domain' do
+      expect(get('/gitlab/gitlabhq/pages/domains/my.domain.com')).to route_to('projects/pages_domains#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'my.domain.com')
+    end
+  end
 end