Commit d78af2ea authored by Vladimir Shushlin's avatar Vladimir Shushlin

Extract pages domain presenter into helper method

This fixes 2 bugs:
* if there are validation errors on creation, presenter wasn't used
  this caused an error while rendering
  https://gitlab.com/gitlab-org/gitlab/-/issues/216728
* verify button wasn't working properly
  https://gitlab.com/gitlab-org/gitlab/-/issues/215632
parent 55924c93
...@@ -7,6 +7,8 @@ class Projects::PagesDomainsController < Projects::ApplicationController ...@@ -7,6 +7,8 @@ class Projects::PagesDomainsController < Projects::ApplicationController
before_action :authorize_update_pages! before_action :authorize_update_pages!
before_action :domain, except: [:new, :create] before_action :domain, except: [:new, :create]
helper_method :domain_presenter
def show def show
end end
...@@ -27,7 +29,7 @@ class Projects::PagesDomainsController < Projects::ApplicationController ...@@ -27,7 +29,7 @@ class Projects::PagesDomainsController < Projects::ApplicationController
end end
def retry_auto_ssl def retry_auto_ssl
PagesDomains::RetryAcmeOrderService.new(@domain.pages_domain).execute PagesDomains::RetryAcmeOrderService.new(@domain).execute
redirect_to project_pages_domain_path(@project, @domain) redirect_to project_pages_domain_path(@project, @domain)
end end
...@@ -88,6 +90,10 @@ class Projects::PagesDomainsController < Projects::ApplicationController ...@@ -88,6 +90,10 @@ class Projects::PagesDomainsController < Projects::ApplicationController
end end
def domain 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
end end
- auto_ssl_available = ::Gitlab::LetsEncrypt.enabled? - 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 - 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 - if auto_ssl_available
.form-group.border-section .form-group.border-section
...@@ -36,9 +36,9 @@ ...@@ -36,9 +36,9 @@
= _('Certificate') = _('Certificate')
.d-flex.justify-content-between.align-items-center.p-3 .d-flex.justify-content-between.align-items-center.p-3
%span %span
= @domain.pages_domain.subject || _('missing') = domain_presenter.pages_domain.subject || _('missing')
= link_to _('Remove'), = 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?') }, data: { confirm: _('Are you sure?') },
class: 'btn btn-remove btn-sm', class: 'btn btn-remove btn-sm',
method: :delete method: :delete
......
- verification_enabled = Gitlab::CurrentSettings.pages_domain_verification_enabled? - 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 .form-group.border-section
.row .row
...@@ -13,17 +13,17 @@ ...@@ -13,17 +13,17 @@
%p.form-text.text-muted %p.form-text.text-muted
= _("To access this domain create a new DNS record") = _("To access this domain create a new DNS record")
- if verification_enabled - 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 .form-group.border-section
.row .row
.col-sm-2 .col-sm-2
= _("Verification status") = _("Verification status")
.col-sm-10 .col-sm-10
.status-badge .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 } .badge{ class: status }
= text = 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 .input-group
= text_field_tag :domain_verification, verification_record, class: "monospace js-select-on-focus form-control", readonly: true = text_field_tag :domain_verification, verification_record, class: "monospace js-select-on-focus form-control", readonly: true
.input-group-append .input-group-append
......
- if @domain.errors.any? - if domain_presenter.errors.any?
.alert.alert-danger .alert.alert-danger
- @domain.errors.full_messages.each do |msg| - domain_presenter.errors.full_messages.each do |msg|
= msg = msg
.form-group.border-section .form-group.border-section
.row .row
- if @domain.persisted? - if domain_presenter.persisted?
.col-sm-2 .col-sm-2
= _("Domain") = _("Domain")
.col-sm-10 .col-sm-10
= external_link(@domain.url, @domain.url) = external_link(domain_presenter.url, domain_presenter.url)
- else - else
.col-sm-2 .col-sm-2
= f.label :domain, _("Domain") = f.label :domain, _("Domain")
...@@ -17,7 +17,7 @@ ...@@ -17,7 +17,7 @@
.input-group .input-group
= f.text_field :domain, required: true, autocomplete: "off", class: "form-control" = f.text_field :domain, required: true, autocomplete: "off", class: "form-control"
- if @domain.persisted? - if domain_presenter.persisted?
= render 'dns' = render 'dns'
- if Gitlab.config.pages.external_https - if Gitlab.config.pages.external_https
......
- if @domain.enabled? - if domain_presenter.enabled?
- if @domain.auto_ssl_enabled - if domain_presenter.auto_ssl_enabled
- if @domain.show_auto_ssl_failed_warning? - 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) } .form-group.border-section.js-shown-if-auto-ssl{ class: ("d-none" unless auto_ssl_available_and_enabled) }
.row .row
.col-sm-10.offset-sm-2 .col-sm-10.offset-sm-2
...@@ -9,8 +9,8 @@ ...@@ -9,8 +9,8 @@
= icon('warning', class: 'mr-2') = icon('warning', class: 'mr-2')
= _("Something went wrong while obtaining the Let's Encrypt certificate.") = _("Something went wrong while obtaining the Let's Encrypt certificate.")
.row.mx-0.mt-3 .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 = 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.certificate_gitlab_provided? - elsif !domain_presenter.certificate_gitlab_provided?
.form-group.border-section.js-shown-if-auto-ssl{ class: ("d-none" unless auto_ssl_available_and_enabled) } .form-group.border-section.js-shown-if-auto-ssl{ class: ("d-none" unless auto_ssl_available_and_enabled) }
.row .row
.col-sm-10.offset-sm-2 .col-sm-10.offset-sm-2
......
...@@ -4,7 +4,7 @@ ...@@ -4,7 +4,7 @@
= _("New Pages Domain") = _("New Pages Domain")
= render 'projects/pages_domains/helper_text' = render 'projects/pages_domains/helper_text'
%div %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 } = render 'form', { f: f }
.form-actions .form-actions
= f.submit _('Create New Domain'), class: "btn btn-success" = f.submit _('Create New Domain'), class: "btn btn-success"
......
- add_to_breadcrumbs _("Pages"), project_pages_path(@project) - add_to_breadcrumbs _("Pages"), project_pages_path(@project)
- breadcrumb_title @domain.domain - breadcrumb_title domain_presenter.domain
- page_title @domain.domain - page_title domain_presenter.domain
- verification_enabled = Gitlab::CurrentSettings.pages_domain_verification_enabled? - verification_enabled = Gitlab::CurrentSettings.pages_domain_verification_enabled?
- if verification_enabled && @domain.unverified? - if verification_enabled && domain_presenter.unverified?
= content_for :flash_message do = content_for :flash_message do
.alert.alert-warning .alert.alert-warning
.container-fluid.container-limited .container-fluid.container-limited
...@@ -14,7 +14,7 @@ ...@@ -14,7 +14,7 @@
= _('Pages Domain') = _('Pages Domain')
= render 'projects/pages_domains/helper_text' = render 'projects/pages_domains/helper_text'
%div %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 } = render 'form', { f: f }
.form-actions.d-flex.justify-content-between .form-actions.d-flex.justify-content-between
= f.submit _('Save Changes'), class: "btn btn-success" = f.submit _('Save Changes'), class: "btn btn-success"
......
---
title: Fix 500 on creating an invalid domains and verification
merge_request: 31190
author:
type: fixed
...@@ -148,16 +148,10 @@ describe Projects::PagesDomainsController do ...@@ -148,16 +148,10 @@ describe Projects::PagesDomainsController do
describe 'POST verify' do describe 'POST verify' do
let(:params) { request_params.merge(id: pages_domain.domain) } 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 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 post :verify, params: params
...@@ -166,7 +160,9 @@ describe Projects::PagesDomainsController do ...@@ -166,7 +160,9 @@ describe Projects::PagesDomainsController do
end end
it 'handles verification failure' do 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 post :verify, params: params
......
...@@ -158,6 +158,17 @@ shared_examples 'pages settings editing' do ...@@ -158,6 +158,17 @@ shared_examples 'pages settings editing' do
expect(page).to have_content('my.test.domain.com') expect(page).to have_content('my.test.domain.com')
end 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 describe 'with dns verification enabled' do
before do before do
stub_application_setting(pages_domain_verification_enabled: true) stub_application_setting(pages_domain_verification_enabled: true)
......
...@@ -7,7 +7,7 @@ describe 'projects/pages_domains/show' do ...@@ -7,7 +7,7 @@ describe 'projects/pages_domains/show' do
before do before do
assign(:project, project) assign(:project, project)
assign(:domain, domain.present) allow(view).to receive(:domain_presenter).and_return(domain.present)
stub_pages_setting(external_https: true) stub_pages_setting(external_https: true)
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment