Merge branch...

Merge branch '216728-actionview-template-error-undefined-method-pages_domain-for-pagesdomain-0x00007f45ea6e9330' into 'master'

Extract pages_domain_presenter into a helper fixing related bugs

See merge request gitlab-org/gitlab!31190

app/controllers/projects/pages_domains_controller.rb

@@ -7,6 +7,8 @@ class Projects::PagesDomainsController < Projects::ApplicationController
   before_action :authorize_update_pages!
   before_action :domain, except: [:new, :create]
 
+  helper_method :domain_presenter
+
   def show
   end
 
@@ -27,7 +29,7 @@ class Projects::PagesDomainsController < Projects::ApplicationController
   end
 
   def retry_auto_ssl
-    PagesDomains::RetryAcmeOrderService.new(@domain.pages_domain).execute
+    PagesDomains::RetryAcmeOrderService.new(@domain).execute
 
     redirect_to project_pages_domain_path(@project, @domain)
   end
@@ -88,6 +90,10 @@ class Projects::PagesDomainsController < Projects::ApplicationController
   end
 
   def domain
-    @domain ||= @project.pages_domains.find_by_domain!(params[:id].to_s).present(current_user: current_user)
+    @domain ||= @project.pages_domains.find_by_domain!(params[:id].to_s)
+  end
+
+  def domain_presenter
+    @domain_presenter ||= domain.present(current_user: current_user)
   end
 end

app/views/projects/pages_domains/_certificate.html.haml

 - auto_ssl_available = ::Gitlab::LetsEncrypt.enabled?
-- auto_ssl_enabled = @domain.auto_ssl_enabled?
+- auto_ssl_enabled = domain_presenter.auto_ssl_enabled?
 - auto_ssl_available_and_enabled = auto_ssl_available && auto_ssl_enabled
-- has_user_defined_certificate = @domain.certificate && @domain.certificate_user_provided?
+- has_user_defined_certificate = domain_presenter.certificate && domain_presenter.certificate_user_provided?
 
 - if auto_ssl_available
   .form-group.border-section
@@ -36,9 +36,9 @@
             = _('Certificate')
           .d-flex.justify-content-between.align-items-center.p-3
             %span
-              = @domain.pages_domain.subject || _('missing')
+              = domain_presenter.pages_domain.subject || _('missing')
             = link_to _('Remove'),
-              clean_certificate_project_pages_domain_path(@project, @domain),
+              clean_certificate_project_pages_domain_path(@project, domain_presenter),
               data: { confirm: _('Are you sure?') },
               class: 'btn btn-remove btn-sm',
               method: :delete

app/views/projects/pages_domains/_dns.html.haml

 - verification_enabled = Gitlab::CurrentSettings.pages_domain_verification_enabled?
-- dns_record = "#{@domain.domain} CNAME #{@domain.project.pages_subdomain}.#{Settings.pages.host}."
+- dns_record = "#{domain_presenter.domain} CNAME #{domain_presenter.project.pages_subdomain}.#{Settings.pages.host}."
 
 .form-group.border-section
   .row
@@ -13,17 +13,17 @@
       %p.form-text.text-muted
         = _("To access this domain create a new DNS record")
 - if verification_enabled
-  - verification_record = "#{@domain.verification_domain} TXT #{@domain.keyed_verification_code}"
+  - verification_record = "#{domain_presenter.verification_domain} TXT #{domain_presenter.keyed_verification_code}"
   .form-group.border-section
     .row
       .col-sm-2
         = _("Verification status")
       .col-sm-10
         .status-badge
-          - text, status = @domain.unverified? ? [_('Unverified'), 'badge-danger'] : [_('Verified'), 'badge-success']
+          - text, status = domain_presenter.unverified? ? [_('Unverified'), 'badge-danger'] : [_('Verified'), 'badge-success']
           .badge{ class: status }
             = text
-          = link_to sprite_icon("redo"), verify_project_pages_domain_path(@project, @domain), method: :post, class: "btn has-tooltip", title: _("Retry verification")
+          = link_to sprite_icon("redo"), verify_project_pages_domain_path(@project, domain_presenter), method: :post, class: "btn has-tooltip", title: _("Retry verification")
         .input-group
           = text_field_tag :domain_verification, verification_record, class: "monospace js-select-on-focus form-control", readonly: true
           .input-group-append

app/views/projects/pages_domains/_form.html.haml

-- if @domain.errors.any?
+- if domain_presenter.errors.any?
   .alert.alert-danger
-    - @domain.errors.full_messages.each do |msg|
+    - domain_presenter.errors.full_messages.each do |msg|
       = msg
 
 .form-group.border-section
   .row
-    - if @domain.persisted?
+    - if domain_presenter.persisted?
       .col-sm-2
         = _("Domain")
       .col-sm-10
-        = external_link(@domain.url, @domain.url)
+        = external_link(domain_presenter.url, domain_presenter.url)
     - else
       .col-sm-2
         = f.label :domain, _("Domain")
@@ -17,7 +17,7 @@
         .input-group
           = f.text_field :domain, required: true, autocomplete: "off", class: "form-control"
 
-- if @domain.persisted?
+- if domain_presenter.persisted?
   = render 'dns'
 
 - if Gitlab.config.pages.external_https

app/views/projects/pages_domains/_lets_encrypt_callout.html.haml

-- if @domain.enabled?
-  - if @domain.auto_ssl_enabled
-    - if @domain.show_auto_ssl_failed_warning?
+- if domain_presenter.enabled?
+  - if domain_presenter.auto_ssl_enabled
+    - if domain_presenter.show_auto_ssl_failed_warning?
       .form-group.border-section.js-shown-if-auto-ssl{ class: ("d-none" unless auto_ssl_available_and_enabled) }
         .row
           .col-sm-10.offset-sm-2
@@ -9,8 +9,8 @@
                 = icon('warning', class: 'mr-2')
                 = _("Something went wrong while obtaining the Let's Encrypt certificate.")
               .row.mx-0.mt-3
-                = link_to s_('GitLabPagesDomains|Retry'), retry_auto_ssl_project_pages_domain_path(@project, @domain), class: "btn btn-sm btn-grouped btn-warning", method: :post
-    - elsif !@domain.certificate_gitlab_provided?
+                = link_to s_('GitLabPagesDomains|Retry'), retry_auto_ssl_project_pages_domain_path(@project, domain_presenter), class: "btn btn-sm btn-grouped btn-warning", method: :post
+    - elsif !domain_presenter.certificate_gitlab_provided?
       .form-group.border-section.js-shown-if-auto-ssl{ class: ("d-none" unless auto_ssl_available_and_enabled) }
         .row
           .col-sm-10.offset-sm-2

app/views/projects/pages_domains/new.html.haml

@@ -4,7 +4,7 @@
   = _("New Pages Domain")
 = render 'projects/pages_domains/helper_text'
 %div
-  = form_for [@project.namespace.becomes(Namespace), @project, @domain], html: { class: 'fieldset-form' } do |f|
+  = form_for [@project.namespace.becomes(Namespace), @project, domain_presenter], html: { class: 'fieldset-form' } do |f|
     = render 'form', { f: f }
     .form-actions
       = f.submit _('Create New Domain'), class: "btn btn-success"

app/views/projects/pages_domains/show.html.haml

 - add_to_breadcrumbs _("Pages"), project_pages_path(@project)
-- breadcrumb_title @domain.domain
-- page_title @domain.domain
+- breadcrumb_title domain_presenter.domain
+- page_title domain_presenter.domain
 
 - verification_enabled = Gitlab::CurrentSettings.pages_domain_verification_enabled?
 
-- if verification_enabled && @domain.unverified?
+- if verification_enabled && domain_presenter.unverified?
   = content_for :flash_message do
     .alert.alert-warning
       .container-fluid.container-limited
@@ -14,7 +14,7 @@
   = _('Pages Domain')
 = render 'projects/pages_domains/helper_text'
 %div
-  = form_for [@project.namespace.becomes(Namespace), @project, @domain], html: { class: 'fieldset-form' } do |f|
+  = form_for [@project.namespace.becomes(Namespace), @project, domain_presenter], html: { class: 'fieldset-form' } do |f|
     = render 'form', { f: f }
     .form-actions.d-flex.justify-content-between
       = f.submit _('Save Changes'), class: "btn btn-success"

changelogs/unreleased/216728-actionview-template-error-undefined-method-pages_domain-for-pagesd.yml

+---
+title: Fix 500 on creating an invalid domains and verification
+merge_request: 31190
+author:
+type: fixed

spec/controllers/projects/pages_domains_controller_spec.rb

@@ -148,16 +148,10 @@ describe Projects::PagesDomainsController do
   describe 'POST verify' do
     let(:params) { request_params.merge(id: pages_domain.domain) }
 
-    def stub_service
-      service = double(:service)
-
-      expect(VerifyPagesDomainService).to receive(:new) { service }
-
-      service
-    end
-
     it 'handles verification success' do
-      expect(stub_service).to receive(:execute).and_return(status: :success)
+      expect_next_instance_of(VerifyPagesDomainService, pages_domain) do |service|
+        expect(service).to receive(:execute).and_return(status: :success)
+      end
 
       post :verify, params: params
 
@@ -166,7 +160,9 @@ describe Projects::PagesDomainsController do
     end
 
     it 'handles verification failure' do
-      expect(stub_service).to receive(:execute).and_return(status: :failed)
+      expect_next_instance_of(VerifyPagesDomainService, pages_domain) do |service|
+        expect(service).to receive(:execute).and_return(status: :failed)
+      end
 
       post :verify, params: params
 

spec/features/projects/pages_spec.rb

@@ -158,6 +158,17 @@ shared_examples 'pages settings editing' do
           expect(page).to have_content('my.test.domain.com')
         end
 
+        it 'shows validation error if domain is duplicated' do
+          project.pages_domains.create!(domain: 'my.test.domain.com')
+
+          visit new_project_pages_domain_path(project)
+
+          fill_in 'Domain', with: 'my.test.domain.com'
+          click_button 'Create New Domain'
+
+          expect(page).to have_content('Domain has already been taken')
+        end
+
         describe 'with dns verification enabled' do
           before do
             stub_application_setting(pages_domain_verification_enabled: true)

spec/views/projects/pages_domains/show.html.haml_spec.rb

@@ -7,7 +7,7 @@ describe 'projects/pages_domains/show' do
 
   before do
     assign(:project, project)
-    assign(:domain, domain.present)
+    allow(view).to receive(:domain_presenter).and_return(domain.present)
     stub_pages_setting(external_https: true)
   end