Merge branch 'migrate-pages-domains-edit-to-show' into 'master'

Migrate Pages Domain Settings from Edit to Show

See merge request gitlab-org/gitlab!20403

app/controllers/projects/pages_domains_controller.rb

@@ -8,7 +8,6 @@ class Projects::PagesDomainsController < Projects::ApplicationController
   before_action :domain, except: [:new, :create]
 
   def show
-    redirect_to edit_project_pages_domain_path(@project, @domain)
   end
 
   def new
@@ -24,17 +23,18 @@ class Projects::PagesDomainsController < Projects::ApplicationController
       flash[:alert] = 'Failed to verify domain ownership'
     end
 
-    redirect_to edit_project_pages_domain_path(@project, @domain)
+    redirect_to project_pages_domain_path(@project, @domain)
   end
 
   def edit
+    redirect_to project_pages_domain_path(@project, @domain)
   end
 
   def create
     @domain = @project.pages_domains.create(create_params)
 
     if @domain.valid?
-      redirect_to edit_project_pages_domain_path(@project, @domain)
+      redirect_to project_pages_domain_path(@project, @domain)
     else
       render 'new'
     end
@@ -46,7 +46,7 @@ class Projects::PagesDomainsController < Projects::ApplicationController
         status: :found,
         notice: 'Domain was updated'
     else
-      render 'edit'
+      render 'show'
     end
   end
 
@@ -68,7 +68,7 @@ class Projects::PagesDomainsController < Projects::ApplicationController
       flash[:alert] = @domain.errors.full_messages.join(', ')
     end
 
-    redirect_to edit_project_pages_domain_path(@project, @domain)
+    redirect_to project_pages_domain_path(@project, @domain)
   end
 
   private

app/views/projects/pages/_list.html.haml

@@ -21,11 +21,11 @@
                   %span.badge.badge-danger
                     = s_('GitLabPages|Expired')
           %div
-            = link_to s_('GitLabPages|Edit'), edit_project_pages_domain_path(@project, domain), class: "btn btn-sm btn-grouped btn-success btn-inverted"
+            = link_to s_('GitLabPages|Edit'), project_pages_domain_path(@project, domain), class: "btn btn-sm btn-grouped btn-success btn-inverted"
             = link_to s_('GitLabPages|Remove'), project_pages_domain_path(@project, domain), data: { confirm: s_('GitLabPages|Are you sure?')}, method: :delete, class: "btn btn-remove btn-sm btn-grouped"
         - if verification_enabled && domain.unverified?
           %li.list-group-item.bs-callout-warning
-            - details_link_start = "<a href='#{edit_project_pages_domain_path(@project, domain)}'>".html_safe
+            - details_link_start = "<a href='#{project_pages_domain_path(@project, domain)}'>".html_safe
             - details_link_end = '</a>'.html_safe
             = s_('GitLabPages|%{domain} is not verified. To learn how to verify ownership, visit your %{link_start}domain details%{link_end}.').html_safe % { domain: domain.domain,
                 link_start: details_link_start,

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

-- add_to_breadcrumbs _("Pages"), project_pages_path(@project)
-- breadcrumb_title @domain.domain
-- page_title @domain.domain
-
-- verification_enabled = Gitlab::CurrentSettings.pages_domain_verification_enabled?
-
-- if verification_enabled && @domain.unverified?
-  = content_for :flash_message do
-    .alert.alert-warning
-      .container-fluid.container-limited
-        = _("This domain is not verified. You will need to verify ownership before access is enabled.")
-
-%h3.page-title
-  = _('Pages Domain')
-= render 'projects/pages_domains/helper_text'
-%div
-  = form_for [@project.namespace.becomes(Namespace), @project, @domain], 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"
-      = link_to _('Cancel'), project_pages_path(@project), class: 'btn btn-default btn-inverse'

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

 - add_to_breadcrumbs _("Pages"), project_pages_path(@project)
 - breadcrumb_title @domain.domain
-- page_title "#{@domain.domain}", _('Pages Domains')
-- dns_record = "#{@domain.domain} CNAME #{@domain.project.pages_subdomain}.#{Settings.pages.host}."
+- page_title @domain.domain
 
 - verification_enabled = Gitlab::CurrentSettings.pages_domain_verification_enabled?
 
@@ -11,51 +10,12 @@
       .container-fluid.container-limited
         = _("This domain is not verified. You will need to verify ownership before access is enabled.")
 
-%h3.page-title.with-button
-  = link_to _('Edit'), edit_project_pages_domain_path(@project, @domain), class: 'btn btn-success float-right'
-  = _("Pages Domain")
-
-.table-holder
-  %table.table
-    %tr
-      %td
-        = _("Domain")
-      %td
-        = external_link(@domain.url, @domain.url)
-    %tr
-      %td
-        = _("DNS")
-      %td
-        .input-group
-          = text_field_tag :domain_dns, dns_record , class: "monospace js-select-on-focus form-control", readonly: true
-          .input-group-append
-            = clipboard_button(target: '#domain_dns', class: 'btn-default input-group-text d-none d-sm-block')
-        %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}"
-      %tr
-        %td
-          = _("Verification status")
-        %td
-          = form_tag verify_project_pages_domain_path(@project, @domain) do
-            .status-badge
-              - text, status = @domain.unverified? ? [_('Unverified'), 'badge-danger'] : [_('Verified'), 'badge-success']
-              .badge{ class: status }
-                = text
-              %button.btn.has-tooltip{ type: "submit", data: { container: 'body' }, title: _("Retry verification") }
-                = sprite_icon('redo')
-          .input-group
-            = text_field_tag :domain_verification, verification_record, class: "monospace js-select-on-focus form-control", readonly: true
-            .input-group-append
-              = clipboard_button(target: '#domain_verification', class: 'btn-default d-none d-sm-block')
-          %p.form-text.text-muted
-            - link_to_help = link_to(_('verify ownership'), help_page_path('user/project/pages/custom_domains_ssl_tls_certification/index.md', anchor: '4-verify-the-domains-ownership'))
-            = _("To %{link_to_help} of your domain, add the above key to a TXT record within to your DNS configuration.").html_safe % { link_to_help: link_to_help }
-
-    %tr
-      %td
-        = _("Certificate")
-      %td
-        = render 'lets_encrypt_callout', auto_ssl_available_and_enabled: false
+%h3.page-title
+  = _('Pages Domain')
+= render 'projects/pages_domains/helper_text'
+%div
+  = form_for [@project.namespace.becomes(Namespace), @project, @domain], 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"
+      = link_to _('Cancel'), project_pages_path(@project), class: 'btn btn-default btn-inverse'

locale/gitlab.pot

@@ -12315,9 +12315,6 @@ msgstr ""
 msgid "Pages Domain"
 msgstr ""
 
-msgid "Pages Domains"
-msgstr ""
-
 msgid "Pages getting started guide"
 msgstr ""
 

spec/controllers/projects/pages_domains_controller_spec.rb

@@ -32,10 +32,11 @@ describe Projects::PagesDomainsController do
       get(:show, params: request_params.merge(id: pages_domain.domain))
     end
 
-    it "redirects to the 'edit' page" do
+    it "displays to the 'show' page" do
       make_request
 
-      expect(response).to redirect_to(edit_project_pages_domain_path(project, pages_domain.domain))
+      expect(response).to have_gitlab_http_status(200)
+      expect(response).to render_template('show')
     end
 
     context 'when user is developer' do
@@ -69,16 +70,16 @@ describe Projects::PagesDomainsController do
       created_domain = PagesDomain.reorder(:id).last
 
       expect(created_domain).to be_present
-      expect(response).to redirect_to(edit_project_pages_domain_path(project, created_domain))
+      expect(response).to redirect_to(project_pages_domain_path(project, created_domain))
     end
   end
 
-  describe 'GET edit' do
-    it "displays the 'edit' page" do
-      get(:edit, params: request_params.merge(id: pages_domain.domain))
+  describe 'GET show' do
+    it "displays the 'show' page" do
+      get(:show, params: request_params.merge(id: pages_domain.domain))
 
       expect(response).to have_gitlab_http_status(200)
-      expect(response).to render_template('edit')
+      expect(response).to render_template('show')
     end
   end
 
@@ -128,10 +129,10 @@ describe Projects::PagesDomainsController do
     context 'the domain is invalid' do
       let(:pages_domain_params) { { user_provided_certificate: 'blabla' } }
 
-      it 'renders the edit action' do
+      it 'renders the show action' do
         patch(:update, params: params)
 
-        expect(response).to render_template('edit')
+        expect(response).to render_template('show')
       end
     end
 
@@ -160,7 +161,7 @@ describe Projects::PagesDomainsController do
 
       post :verify, params: params
 
-      expect(response).to redirect_to edit_project_pages_domain_path(project, pages_domain)
+      expect(response).to redirect_to project_pages_domain_path(project, pages_domain)
       expect(flash[:notice]).to eq('Successfully verified domain ownership')
     end
 
@@ -169,7 +170,7 @@ describe Projects::PagesDomainsController do
 
       post :verify, params: params
 
-      expect(response).to redirect_to edit_project_pages_domain_path(project, pages_domain)
+      expect(response).to redirect_to project_pages_domain_path(project, pages_domain)
       expect(flash[:alert]).to eq('Failed to verify domain ownership')
     end
 
@@ -195,10 +196,10 @@ describe Projects::PagesDomainsController do
       delete(:clean_certificate, params: request_params.merge(id: pages_domain.domain))
     end
 
-    it 'redirects to edit page' do
+    it 'redirects to show page' do
       subject
 
-      expect(response).to redirect_to(edit_project_pages_domain_path(project, pages_domain))
+      expect(response).to redirect_to(project_pages_domain_path(project, pages_domain))
     end
 
     it 'removes certificate' do
@@ -230,12 +231,12 @@ describe Projects::PagesDomainsController do
         expect(pages_domain.key).to be_present
       end
 
-      it 'redirects to edit page with a flash message' do
+      it 'redirects to show page with a flash message' do
         subject
 
         expect(flash[:alert]).to include('Certificate')
         expect(flash[:alert]).to include('Key')
-        expect(response).to redirect_to(edit_project_pages_domain_path(project, pages_domain))
+        expect(response).to redirect_to(project_pages_domain_path(project, pages_domain))
       end
     end
   end

spec/features/projects/pages_lets_encrypt_spec.rb

@@ -41,7 +41,7 @@ describe "Pages with Let's Encrypt", :https_pages_enabled do
     end
 
     it 'enables auto SSL and dynamically updates the form accordingly', :js do
-      visit edit_project_pages_domain_path(project, domain)
+      visit project_pages_domain_path(project, domain)
 
       expect(domain.auto_ssl_enabled).to eq false
 
@@ -67,7 +67,7 @@ describe "Pages with Let's Encrypt", :https_pages_enabled do
     end
 
     it 'disables auto SSL and dynamically updates the form accordingly', :js do
-      visit edit_project_pages_domain_path(project, domain)
+      visit project_pages_domain_path(project, domain)
 
       expect(find("#pages_domain_auto_ssl_enabled", visible: false).value).to eq 'true'
       expect(page).not_to have_field 'Certificate (PEM)', type: 'textarea'
@@ -88,7 +88,7 @@ describe "Pages with Let's Encrypt", :https_pages_enabled do
   shared_examples 'user sees private keys only for user provided certificate' do
     shared_examples 'user do not see private key' do
       it 'user do not see private key' do
-        visit edit_project_pages_domain_path(project, domain)
+        visit project_pages_domain_path(project, domain)
 
         expect(page).not_to have_selector '.card-header', text: 'Certificate'
         expect(page).not_to have_text domain.subject
@@ -111,14 +111,14 @@ describe "Pages with Let's Encrypt", :https_pages_enabled do
       let(:domain) { create(:pages_domain, project: project, auto_ssl_enabled: false) }
 
       it 'user sees certificate subject' do
-        visit edit_project_pages_domain_path(project, domain)
+        visit project_pages_domain_path(project, domain)
 
         expect(page).to have_selector '.card-header', text: 'Certificate'
         expect(page).to have_text domain.subject
       end
 
       it 'user can delete the certificate', :js do
-        visit edit_project_pages_domain_path(project, domain)
+        visit project_pages_domain_path(project, domain)
 
         expect(page).to have_selector '.card-header', text: 'Certificate'
         expect(page).to have_text domain.subject
@@ -139,7 +139,7 @@ describe "Pages with Let's Encrypt", :https_pages_enabled do
     before do
       stub_application_setting(lets_encrypt_terms_of_service_accepted: false)
 
-      visit edit_project_pages_domain_path(project, domain)
+      visit project_pages_domain_path(project, domain)
     end
 
     it "does not render the Let's Encrypt field", :js do

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

@@ -8,6 +8,7 @@ describe 'projects/pages_domains/show' do
   before do
     assign(:project, project)
     assign(:domain, domain)
+    stub_pages_setting(external_https: true)
   end
 
   context 'when auto_ssl is enabled' do