From f30b47b3876faf9c171522e1e838e41f33f4c6b3 Mon Sep 17 00:00:00 2001 From: Rob Watson Date: Sun, 25 Feb 2018 14:06:09 +0100 Subject: [PATCH] PagesDomain: Add edit/update functionality --- .../projects/pages_domains_controller.rb | 29 ++++++--- .../projects/pages_domains/_form.html.haml | 58 +++++++++--------- .../projects/pages_domains/edit.html.haml | 9 +++ .../projects/pages_domains/new.html.haml | 5 +- .../projects/pages_domains/show.html.haml | 1 + .../unreleased/feature-edit_pages_domain.yml | 5 ++ config/routes/project.rb | 2 +- .../projects/pages_domains_controller_spec.rb | 60 +++++++++++++++++++ spec/features/projects/pages_spec.rb | 31 ++++++++++ 9 files changed, 159 insertions(+), 41 deletions(-) create mode 100644 app/views/projects/pages_domains/edit.html.haml create mode 100644 changelogs/unreleased/feature-edit_pages_domain.yml diff --git a/app/controllers/projects/pages_domains_controller.rb b/app/controllers/projects/pages_domains_controller.rb index b71f1e5fef4..4856be61e88 100644 --- a/app/controllers/projects/pages_domains_controller.rb +++ b/app/controllers/projects/pages_domains_controller.rb @@ -3,7 +3,7 @@ class Projects::PagesDomainsController < Projects::ApplicationController before_action :require_pages_enabled! before_action :authorize_update_pages!, except: [:show] - before_action :domain, only: [:show, :destroy, :verify] + before_action :domain, except: [:new, :create] def show end @@ -24,8 +24,11 @@ class Projects::PagesDomainsController < Projects::ApplicationController redirect_to project_pages_domain_path(@project, @domain) end + def edit + end + def create - @domain = @project.pages_domains.create(pages_domain_params) + @domain = @project.pages_domains.create(create_params) if @domain.valid? redirect_to project_pages_domain_path(@project, @domain) @@ -34,6 +37,16 @@ class Projects::PagesDomainsController < Projects::ApplicationController end end + def update + if @domain.update(update_params) + redirect_to project_pages_path(@project), + status: 302, + notice: 'Domain was updated' + else + render 'edit' + end + end + def destroy @domain.destroy @@ -49,12 +62,12 @@ class Projects::PagesDomainsController < Projects::ApplicationController private - def pages_domain_params - params.require(:pages_domain).permit( - :certificate, - :key, - :domain - ) + def create_params + params.require(:pages_domain).permit(:key, :certificate, :domain) + end + + def update_params + params.require(:pages_domain).permit(:key, :certificate) end def domain diff --git a/app/views/projects/pages_domains/_form.html.haml b/app/views/projects/pages_domains/_form.html.haml index ca1b41b140a..d81b07832bb 100644 --- a/app/views/projects/pages_domains/_form.html.haml +++ b/app/views/projects/pages_domains/_form.html.haml @@ -1,34 +1,30 @@ -= form_for [@project.namespace.becomes(Namespace), @project, @domain], html: { class: 'form-horizontal fieldset-form' } do |f| - - if @domain.errors.any? - #error_explanation - .alert.alert-danger - - @domain.errors.full_messages.each do |msg| - %p= msg +- if @domain.errors.any? + #error_explanation + .alert.alert-danger + - @domain.errors.full_messages.each do |msg| + %p= msg + +.form-group + = f.label :domain, class: 'control-label' do + Domain + .col-sm-10 + = f.text_field :domain, required: true, autocomplete: 'off', class: 'form-control', disabled: @domain.persisted? + +- if Gitlab.config.pages.external_https + .form-group + = f.label :certificate, class: 'control-label' do + Certificate (PEM) + .col-sm-10 + = f.text_area :certificate, rows: 5, class: 'form-control' + %span.help-inline Upload a certificate for your domain with all intermediates .form-group - = f.label :domain, class: 'control-label' do - Domain + = f.label :key, class: 'control-label' do + Key (PEM) .col-sm-10 - = f.text_field :domain, required: true, autocomplete: 'off', class: 'form-control' - - - if Gitlab.config.pages.external_https - .form-group - = f.label :certificate, class: 'control-label' do - Certificate (PEM) - .col-sm-10 - = f.text_area :certificate, rows: 5, class: 'form-control' - %span.help-inline Upload a certificate for your domain with all intermediates - - .form-group - = f.label :key, class: 'control-label' do - Key (PEM) - .col-sm-10 - = f.text_area :key, rows: 5, class: 'form-control' - %span.help-inline Upload a private key for your certificate - - else - .nothing-here-block - Support for custom certificates is disabled. - Ask your system's administrator to enable it. - - .form-actions - = f.submit 'Create New Domain', class: "btn btn-save" + = f.text_area :key, rows: 5, class: 'form-control' + %span.help-inline Upload a private key for your certificate +- else + .nothing-here-block + Support for custom certificates is disabled. + Ask your system's administrator to enable it. diff --git a/app/views/projects/pages_domains/edit.html.haml b/app/views/projects/pages_domains/edit.html.haml new file mode 100644 index 00000000000..60e7ecf6984 --- /dev/null +++ b/app/views/projects/pages_domains/edit.html.haml @@ -0,0 +1,9 @@ +- page_title @domain.domain +%h3.page_title + = @domain.domain +%hr.clearfix +%div + = form_for [@project.namespace.becomes(Namespace), @project, @domain], html: { class: 'form-horizontal fieldset-form' } do |f| + = render 'form', { f: f } + .form-actions + = f.submit 'Save Changes', class: "btn btn-save" diff --git a/app/views/projects/pages_domains/new.html.haml b/app/views/projects/pages_domains/new.html.haml index e1477c71d06..63a4392e307 100644 --- a/app/views/projects/pages_domains/new.html.haml +++ b/app/views/projects/pages_domains/new.html.haml @@ -3,4 +3,7 @@ New Pages Domain %hr.clearfix %div - = render 'form' + = form_for [@project.namespace.becomes(Namespace), @project, @domain], html: { class: 'form-horizontal fieldset-form' } do |f| + = render 'form', { f: f } + .form-actions + = f.submit 'Create New Domain', class: "btn btn-save" diff --git a/app/views/projects/pages_domains/show.html.haml b/app/views/projects/pages_domains/show.html.haml index 72e9203bdb0..bcd4e1c4b05 100644 --- a/app/views/projects/pages_domains/show.html.haml +++ b/app/views/projects/pages_domains/show.html.haml @@ -8,6 +8,7 @@ %h3.page-title Pages Domain + = link_to 'Edit', edit_project_pages_domain_path(@project, @domain), class: 'btn btn-success pull-right' .table-holder %table.table diff --git a/changelogs/unreleased/feature-edit_pages_domain.yml b/changelogs/unreleased/feature-edit_pages_domain.yml new file mode 100644 index 00000000000..bd0af53296c --- /dev/null +++ b/changelogs/unreleased/feature-edit_pages_domain.yml @@ -0,0 +1,5 @@ +--- +title: 'Pages custom domain: allow update of key/certificate' +merge_request: 17376 +author: rfwatson +type: changed diff --git a/config/routes/project.rb b/config/routes/project.rb index dd1a9c3be3b..34636285c51 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -55,7 +55,7 @@ constraints(ProjectUrlConstrainer.new) do end resource :pages, only: [:show, :destroy] do - resources :domains, only: [:show, :new, :create, :destroy], controller: 'pages_domains', constraints: { id: %r{[^/]+} } do + resources :domains, except: :index, controller: 'pages_domains', constraints: { id: %r{[^/]+} } do member do post :verify end diff --git a/spec/controllers/projects/pages_domains_controller_spec.rb b/spec/controllers/projects/pages_domains_controller_spec.rb index 2192fd5cae2..83a3799e883 100644 --- a/spec/controllers/projects/pages_domains_controller_spec.rb +++ b/spec/controllers/projects/pages_domains_controller_spec.rb @@ -53,6 +53,66 @@ describe Projects::PagesDomainsController do end end + describe 'GET edit' do + it "displays the 'edit' page" do + get(:edit, request_params.merge(id: pages_domain.domain)) + + expect(response).to have_gitlab_http_status(200) + expect(response).to render_template('edit') + end + end + + describe 'PATCH update' do + before do + controller.instance_variable_set(:@domain, pages_domain) + end + + let(:pages_domain_params) do + attributes_for(:pages_domain, :with_certificate, :with_key).slice(:key, :certificate) + end + + let(:params) do + request_params.merge(id: pages_domain.domain, pages_domain: pages_domain_params) + end + + it 'updates the domain' do + expect(pages_domain) + .to receive(:update) + .with(pages_domain_params) + .and_return(true) + + patch(:update, params) + end + + it 'redirects to the project page' do + patch(:update, params) + + expect(flash[:notice]).to eq 'Domain was updated' + expect(response).to redirect_to(project_pages_path(project)) + end + + context 'the domain is invalid' do + it 'renders the edit action' do + allow(pages_domain).to receive(:update).and_return(false) + + patch(:update, params) + + expect(response).to render_template('edit') + end + end + + context 'the parameters include the domain' do + it 'renders 400 Bad Request' do + expect(pages_domain) + .to receive(:update) + .with(hash_not_including(:domain)) + .and_return(true) + + patch(:update, params.deep_merge(pages_domain: { domain: 'abc' })) + end + end + end + describe 'POST verify' do let(:params) { request_params.merge(id: pages_domain.domain) } diff --git a/spec/features/projects/pages_spec.rb b/spec/features/projects/pages_spec.rb index a96f2c186a4..2a0d235ef04 100644 --- a/spec/features/projects/pages_spec.rb +++ b/spec/features/projects/pages_spec.rb @@ -160,6 +160,37 @@ feature 'Pages' do expect(page).to have_content('my.test.domain.com') end + + describe 'updating the certificate for an existing domain' do + let!(:domain) do + create(:pages_domain, :with_key, :with_certificate, project: project) + end + + it 'allows the certificate to be updated' do + visit project_pages_path(project) + + within('#content-body') { click_link 'Details' } + click_link 'Edit' + click_button 'Save Changes' + + expect(page).to have_content('Domain was updated') + end + + context 'when the certificate is invalid' do + it 'tells the user what the problem is' do + visit project_pages_path(project) + + within('#content-body') { click_link 'Details' } + click_link 'Edit' + fill_in 'Certificate (PEM)', with: 'invalid data' + click_button 'Save Changes' + + expect(page).to have_content('Certificate must be a valid PEM certificate') + expect(page).to have_content('Certificate misses intermediates') + expect(page).to have_content("Key doesn't match the certificate") + end + end + end end end