From 40e6be604f21515fca386b73d97eda8ca294657a Mon Sep 17 00:00:00 2001 From: Justin Zeng Date: Sun, 17 Mar 2024 20:59:24 -0700 Subject: [PATCH 1/9] Add REST API to update pages settings Changelog: added MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/147227 EE: false --- app/services/pages/update_service.rb | 26 ++++++++++++ lib/api/pages.rb | 24 +++++++++++ spec/requests/api/pages_spec.rb | 47 ++++++++++++++++++++++ spec/services/pages/update_service_spec.rb | 33 +++++++++++++++ 4 files changed, 130 insertions(+) create mode 100644 app/services/pages/update_service.rb create mode 100644 spec/services/pages/update_service_spec.rb diff --git a/app/services/pages/update_service.rb b/app/services/pages/update_service.rb new file mode 100644 index 00000000000000..b661e8a5001b6c --- /dev/null +++ b/app/services/pages/update_service.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +module Pages + class UpdateService < BaseService + def execute + Project.transaction do + update_project_settings if params.key?(:pages_unique_domain_enabled) + update_project if params.key?(:pages_https_only) + end + + project + end + + private + + def update_project_settings + project_setting_attributes = { pages_unique_domain_enabled: params[:pages_unique_domain_enabled] } + project.project_setting.update!(project_setting_attributes) + end + + def update_project + project_attributes = { pages_https_only: params[:pages_https_only] } + project.update!(project_attributes) + end + end +end diff --git a/lib/api/pages.rb b/lib/api/pages.rb index 30e126b34cbbb9..d5dee94b9c05a5 100644 --- a/lib/api/pages.rb +++ b/lib/api/pages.rb @@ -31,6 +31,30 @@ class Pages < ::API::Base no_content! end + desc 'Update pages settings' do + detail 'Update page settings for a project. User must have administrative access.' + success code: 200 + failure [ + { code: 401, message: 'Unauthorized' }, + { code: 404, message: 'Not Found' } + ] + tags %w[pages] + end + params do + optional :pages_unique_domain_enabled, type: Boolean, desc: 'Whether to use unique domain' + optional :pages_https_only, type: Boolean, desc: 'Whether to force HTTPS' + end + patch ':id/pages' do + authenticated_with_can_read_all_resources! + authorize! :update_pages, user_project + + break not_found! unless user_project.pages_enabled? + + updated_project = ::Pages::UpdateService.new(user_project, current_user, params).execute + + present ::Pages::ProjectSettings.new(updated_project), with: Entities::Pages::ProjectSettings + end + desc 'Get pages settings' do detail 'Get pages URL and other settings. This feature was introduced in Gitlab 16.8' success code: 200 diff --git a/spec/requests/api/pages_spec.rb b/spec/requests/api/pages_spec.rb index 23ffeb143cbacf..c91f781477b96d 100644 --- a/spec/requests/api/pages_spec.rb +++ b/spec/requests/api/pages_spec.rb @@ -88,4 +88,51 @@ end end end + + describe 'PATCH /projects/:id/pages' do + let(:path) { "/projects/#{project.id}/pages" } + let(:params) { { pages_unique_domain_enabled: true, pages_https_only: true } } + + before do + stub_pages_setting(external_https: true) + end + + context 'when the user is authorized' do + it 'updates the pages settings and returns 200' do + patch api(path, admin, admin_mode: true), params: params + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['force_https']).to eq(true) + expect(json_response['is_unique_domain_enabled']).to eq(true) + end + end + + context 'when the user is unauthorized' do + it 'returns a 403 forbidden' do + patch api(path, user), params: params + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when pages feature is disabled' do + before do + stub_pages_setting(enabled: false) + end + + it 'returns a 404 not found' do + patch api(path, admin, admin_mode: true), params: params + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when there is no project' do + it 'returns 404 not found' do + non_existent_project_id = -1 + patch api("/projects/#{non_existent_project_id}/pages", admin, admin_mode: true), params: params + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end end diff --git a/spec/services/pages/update_service_spec.rb b/spec/services/pages/update_service_spec.rb new file mode 100644 index 00000000000000..4676237e6d0677 --- /dev/null +++ b/spec/services/pages/update_service_spec.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Pages::UpdateService, feature_category: :pages do + let_it_be(:admin) { create(:admin) } + let_it_be_with_reload(:project) { create(:project) } + let(:service) { described_class.new(project, admin, params) } + + before do + stub_pages_setting(external_https: true) + end + + context 'when updating pages settings' do + let(:params) do + { pages_unique_domain_enabled: false, pages_https_only: false } + end + + it 'updates pages_https_only setting' do + expect { service.execute } + .to change { project.reload.pages_https_only }.from(true).to(false) + end + + it 'updates pages_unique_domain_enabled setting' do + create(:project_setting, project: project, pages_unique_domain_enabled: true, + pages_unique_domain: "random-unique-domain-here") + + expect { service.execute } + .to change { project.reload.project_setting.pages_unique_domain_enabled } + .from(true).to(false) + end + end +end -- GitLab From a30ae9e4779c30f950422de6105ef8ec4847150b Mon Sep 17 00:00:00 2001 From: Justin Zeng Date: Fri, 22 Mar 2024 15:41:42 -0700 Subject: [PATCH 2/9] Add API docs and rename methods --- app/services/pages/update_service.rb | 8 ++-- doc/api/pages.md | 69 ++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 4 deletions(-) diff --git a/app/services/pages/update_service.rb b/app/services/pages/update_service.rb index b661e8a5001b6c..6006c0cd0ba785 100644 --- a/app/services/pages/update_service.rb +++ b/app/services/pages/update_service.rb @@ -4,8 +4,8 @@ module Pages class UpdateService < BaseService def execute Project.transaction do - update_project_settings if params.key?(:pages_unique_domain_enabled) - update_project if params.key?(:pages_https_only) + update_pages_unique_domain_enabled if params.key?(:pages_unique_domain_enabled) + update_pages_https_only if params.key?(:pages_https_only) end project @@ -13,12 +13,12 @@ def execute private - def update_project_settings + def update_pages_unique_domain_enabled project_setting_attributes = { pages_unique_domain_enabled: params[:pages_unique_domain_enabled] } project.project_setting.update!(project_setting_attributes) end - def update_project + def update_pages_https_only project_attributes = { pages_https_only: params[:pages_https_only] } project.update!(project_attributes) end diff --git a/doc/api/pages.md b/doc/api/pages.md index 7d8f7e99e5431d..9515f9afd15a1c 100644 --- a/doc/api/pages.md +++ b/doc/api/pages.md @@ -100,3 +100,72 @@ Example response: ] } ``` + +## Update page settings for a project + +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/147227) + +Prerequisites: + +- You must have administrator access to the instance. + +Update pages settings for the project. + +```plaintext +PATCH /projects/:id/pages +``` + +Supported attributes: + +| Attribute | Type | Required | Description | +| --------------------------------| -------------- | -------- | --------------------------------------------------------------------------------------------------------------------| +| `id` | integer/string | Yes | The ID or [URL-encoded path of the project](rest/index.md#namespaced-path-encoding) owned by the authenticated user | +| `pages_unique_domain_enabled` | boolean | No | Whether to use unique domain | +| `pages_https_only` | boolean | No | Whether to force HTTPs | + +If successful, returns [`200`](rest/index.md#status-codes) and the following +response attributes: + +| Attribute | Type | Description | +| ----------------------------------------- | ---------- | ----------------------- | +| `url` | string | URL to access this project pages. | +| `is_unique_domain_enabled` | boolean | If [unique domain](../user/project/pages/introduction.md) is enabled. | +| `force_https` | boolean | `true` if the project is set to force HTTPS. | +| `deployments[]` | array | List of current active deployments. | + +| `deployments[]` attribute | Type | Description | +| ----------------------------------------- | ---------- | ----------------------- | +| `created_at` | date | Date deployment was created. | +| `url` | string | URL for this deployment. | +| `path_prefix` | string | Path prefix of this deployment when using [multiple deployments](../user/project/pages/index.md#create-multiple-deployments). | +| `root_directory` | string | Root directory. | + +Example request: + +```shell +curl --request 'PATCH' --header "PRIVATE-TOKEN: " "https://gitlab.example.com/api/v4/projects/2/pages" +``` + +Example response: + +```json +{ + "url": "http://html-root-4160ce5f0e9a6c90ccb02755b7fc80f5a2a09ffbb1976cf80b653.pages.gdk.test:3010", + "is_unique_domain_enabled": true, + "force_https": false, + "deployments": [ + { + "created_at": "2024-01-05T18:58:14.916Z", + "url": "http://html-root-4160ce5f0e9a6c90ccb02755b7fc80f5a2a09ffbb1976cf80b653.pages.gdk.test:3010/", + "path_prefix": "", + "root_directory": null + }, + { + "created_at": "2024-01-05T18:58:46.042Z", + "url": "http://html-root-4160ce5f0e9a6c90ccb02755b7fc80f5a2a09ffbb1976cf80b653.pages.gdk.test:3010/mr3", + "path_prefix": "mr3", + "root_directory": null + } + ] +} +``` -- GitLab From 69a1e6eb5b0fc623ce6dded01b968046f677b1c8 Mon Sep 17 00:00:00 2001 From: Justin Zeng Date: Fri, 22 Mar 2024 15:47:44 -0700 Subject: [PATCH 3/9] Added sample curl patch attributes --- doc/api/pages.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/api/pages.md b/doc/api/pages.md index 9515f9afd15a1c..79fdc240bfb67a 100644 --- a/doc/api/pages.md +++ b/doc/api/pages.md @@ -143,7 +143,8 @@ response attributes: Example request: ```shell -curl --request 'PATCH' --header "PRIVATE-TOKEN: " "https://gitlab.example.com/api/v4/projects/2/pages" +curl --request PATCH --header "PRIVATE-TOKEN: " --url "https://gitlab.example.com/api/v4/projects/:id/pages" \ +--form 'pages_unique_domain_enabled=true' --form 'pages_https_only=true' ``` Example response: -- GitLab From 8448a6c8cae5817d34323f8e649adf6235f5d5cf Mon Sep 17 00:00:00 2001 From: Justin Zeng Date: Wed, 27 Mar 2024 16:58:43 -0700 Subject: [PATCH 4/9] Refactored UpdateService --- app/services/pages/update_service.rb | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/app/services/pages/update_service.rb b/app/services/pages/update_service.rb index 6006c0cd0ba785..da438f25f5ebac 100644 --- a/app/services/pages/update_service.rb +++ b/app/services/pages/update_service.rb @@ -4,8 +4,8 @@ module Pages class UpdateService < BaseService def execute Project.transaction do - update_pages_unique_domain_enabled if params.key?(:pages_unique_domain_enabled) - update_pages_https_only if params.key?(:pages_https_only) + update_pages_unique_domain_enabled! + update_pages_https_only! end project @@ -13,14 +13,16 @@ def execute private - def update_pages_unique_domain_enabled - project_setting_attributes = { pages_unique_domain_enabled: params[:pages_unique_domain_enabled] } - project.project_setting.update!(project_setting_attributes) + def update_pages_unique_domain_enabled! + return unless params.key?(:pages_unique_domain_enabled) + + project.project_setting.update!(pages_unique_domain_enabled: params[:pages_unique_domain_enabled]) end - def update_pages_https_only - project_attributes = { pages_https_only: params[:pages_https_only] } - project.update!(project_attributes) + def update_pages_https_only! + return unless params.key?(:pages_https_only) + + project.update!(pages_https_only: params[:pages_https_only]) end end end -- GitLab From e91c6f19b7f32df67133cd352c6b65589fb0d3f5 Mon Sep 17 00:00:00 2001 From: Justin Zeng Date: Thu, 28 Mar 2024 09:18:10 -0700 Subject: [PATCH 5/9] Fixed api doc --- doc/api/pages.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/doc/api/pages.md b/doc/api/pages.md index 79fdc240bfb67a..5060670dc80660 100644 --- a/doc/api/pages.md +++ b/doc/api/pages.md @@ -34,7 +34,7 @@ DELETE /projects/:id/pages curl --request 'DELETE' --header "PRIVATE-TOKEN: " "https://gitlab.example.com/api/v4/projects/2/pages" ``` -## Get pages settings for a project +## Get Pages settings for a project > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/436932) in GitLab 16.8. @@ -59,7 +59,7 @@ response attributes: | Attribute | Type | Description | | ----------------------------------------- | ---------- | ----------------------- | -| `url` | string | URL to access this project pages. | +| `url` | string | URL to access this project's Pages. | | `is_unique_domain_enabled` | boolean | If [unique domain](../user/project/pages/introduction.md) is enabled. | | `force_https` | boolean | `true` if the project is set to force HTTPS. | | `deployments[]` | array | List of current active deployments. | @@ -101,15 +101,15 @@ Example response: } ``` -## Update page settings for a project +## Update Pages settings for a project -> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/147227) +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/147227) in GitLab 16.11. Prerequisites: - You must have administrator access to the instance. -Update pages settings for the project. +Update Pages settings for the project. ```plaintext PATCH /projects/:id/pages @@ -128,7 +128,7 @@ response attributes: | Attribute | Type | Description | | ----------------------------------------- | ---------- | ----------------------- | -| `url` | string | URL to access this project pages. | +| `url` | string | URL to access this project's Pages. | | `is_unique_domain_enabled` | boolean | If [unique domain](../user/project/pages/introduction.md) is enabled. | | `force_https` | boolean | `true` if the project is set to force HTTPS. | | `deployments[]` | array | List of current active deployments. | -- GitLab From ce18df426e4a8e7561aa5d4c362e1a0a1f6fec83 Mon Sep 17 00:00:00 2001 From: Justin Zeng Date: Sat, 30 Mar 2024 18:30:02 -0700 Subject: [PATCH 6/9] Fix white space --- spec/requests/api/pages_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/requests/api/pages_spec.rb b/spec/requests/api/pages_spec.rb index c91f781477b96d..05ab9edf19f3a5 100644 --- a/spec/requests/api/pages_spec.rb +++ b/spec/requests/api/pages_spec.rb @@ -100,6 +100,7 @@ context 'when the user is authorized' do it 'updates the pages settings and returns 200' do patch api(path, admin, admin_mode: true), params: params + expect(response).to have_gitlab_http_status(:ok) expect(json_response['force_https']).to eq(true) expect(json_response['is_unique_domain_enabled']).to eq(true) -- GitLab From 455d3b3c3947728e8c899bc5ed203623cd671a8b Mon Sep 17 00:00:00 2001 From: Justin Zeng Date: Sat, 6 Apr 2024 18:56:56 -0700 Subject: [PATCH 7/9] Add authorization check in service layer --- app/services/pages/update_service.rb | 24 ++++++-- lib/api/pages.rb | 10 +++- spec/requests/api/pages_spec.rb | 41 +++++++++++-- spec/services/pages/update_service_spec.rb | 67 +++++++++++++++++----- 4 files changed, 117 insertions(+), 25 deletions(-) diff --git a/app/services/pages/update_service.rb b/app/services/pages/update_service.rb index da438f25f5ebac..e88a47ca37eb4b 100644 --- a/app/services/pages/update_service.rb +++ b/app/services/pages/update_service.rb @@ -2,13 +2,25 @@ module Pages class UpdateService < BaseService + include Gitlab::Allowable + UpdateError = Class.new(StandardError) + def execute - Project.transaction do - update_pages_unique_domain_enabled! - update_pages_https_only! + unless can_update_page_settings? + return ServiceResponse.error(message: _('The current user is not authorized to update the page settings'), + reason: :forbidden) end - project + begin + Project.transaction do + update_pages_unique_domain_enabled! + update_pages_https_only! + end + + ServiceResponse.success(payload: { project: project }) + rescue UpdateError + ServiceResponse.error(message: _('Failed to update page settings')) + end end private @@ -24,5 +36,9 @@ def update_pages_https_only! project.update!(pages_https_only: params[:pages_https_only]) end + + def can_update_page_settings? + current_user&.can_read_all_resources? && can?(current_user, :update_pages, project) + end end end diff --git a/lib/api/pages.rb b/lib/api/pages.rb index d5dee94b9c05a5..a2aa8e3b2db859 100644 --- a/lib/api/pages.rb +++ b/lib/api/pages.rb @@ -50,9 +50,15 @@ class Pages < ::API::Base break not_found! unless user_project.pages_enabled? - updated_project = ::Pages::UpdateService.new(user_project, current_user, params).execute + response = ::Pages::UpdateService.new(user_project, current_user, params).execute - present ::Pages::ProjectSettings.new(updated_project), with: Entities::Pages::ProjectSettings + if response.success? + present ::Pages::ProjectSettings.new(response.payload[:project]), with: Entities::Pages::ProjectSettings + elsif response.reason == :forbidden + forbidden!(response.message) + else + bad_request!(response.message) + end end desc 'Get pages settings' do diff --git a/spec/requests/api/pages_spec.rb b/spec/requests/api/pages_spec.rb index 05ab9edf19f3a5..e734b48678a11b 100644 --- a/spec/requests/api/pages_spec.rb +++ b/spec/requests/api/pages_spec.rb @@ -98,12 +98,32 @@ end context 'when the user is authorized' do - it 'updates the pages settings and returns 200' do - patch api(path, admin, admin_mode: true), params: params + context 'and the update succeeds' do + it 'updates the pages settings and returns 200' do + patch api(path, admin, admin_mode: true), params: params + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['force_https']).to eq(true) + expect(json_response['is_unique_domain_enabled']).to eq(true) + end + end + + context 'and update service fails' do + before do + failure_response = instance_double(ServiceResponse, success?: false, reason: :validation_error, + message: "Failed to update page settings") + + allow_next_instance_of(::Pages::UpdateService) do |instance| + allow(instance).to receive(:execute).and_return(failure_response) + end + end + + it 'returns a 400 bad request' do + patch api(path, admin, admin_mode: true), params: params - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['force_https']).to eq(true) - expect(json_response['is_unique_domain_enabled']).to eq(true) + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq("400 Bad request - Failed to update page settings") + end end end @@ -135,5 +155,16 @@ expect(response).to have_gitlab_http_status(:not_found) end end + + context 'when the parameters are invalid' do + let(:invalid_params) { { pages_unique_domain_enabled: nil, pages_https_only: "not_a_boolean" } } + + it 'returns a 400 bad request' do + patch api(path, admin, admin_mode: true), params: invalid_params + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq('pages_https_only is invalid') + end + end end end diff --git a/spec/services/pages/update_service_spec.rb b/spec/services/pages/update_service_spec.rb index 4676237e6d0677..0ea039362b4201 100644 --- a/spec/services/pages/update_service_spec.rb +++ b/spec/services/pages/update_service_spec.rb @@ -4,30 +4,69 @@ RSpec.describe Pages::UpdateService, feature_category: :pages do let_it_be(:admin) { create(:admin) } + let_it_be(:user) { create(:user) } let_it_be_with_reload(:project) { create(:project) } - let(:service) { described_class.new(project, admin, params) } + let(:params) { { pages_unique_domain_enabled: false, pages_https_only: false } } before do stub_pages_setting(external_https: true) end - context 'when updating pages settings' do - let(:params) do - { pages_unique_domain_enabled: false, pages_https_only: false } - end + describe '#execute' do + context 'with sufficient permissions' do + let(:service) { described_class.new(project, admin, params) } + + before do + allow(admin).to receive(:can_read_all_resources?).and_return(true) + allow(service).to receive(:can?).with(admin, :update_pages, project).and_return(true) + end + + context 'when updating page setting succeeds' do + it 'updates page settings' do + create(:project_setting, project: project, pages_unique_domain_enabled: true, + pages_unique_domain: "random-unique-domain-here") + + expect { service.execute } + .to change { project.reload.pages_https_only }.from(true).to(false) + .and change { project.project_setting.pages_unique_domain_enabled }.from(true).to(false) + end + + it 'returns a success response' do + result = service.execute - it 'updates pages_https_only setting' do - expect { service.execute } - .to change { project.reload.pages_https_only }.from(true).to(false) + expect(result).to be_a(ServiceResponse) + expect(result).to be_success + expect(result.payload[:project]).to eq(project) + end + end + + context 'when an update raises an UpdateError' do + before do + allow_next_instance_of(ProjectSetting) do |instance| + allow(instance).to receive(:update!).and_raise(Pages::UpdateService::UpdateError) + end + end + + it 'returns an error response' do + result = service.execute + + expect(result).to be_a(ServiceResponse) + expect(result.error?).to be(true) + expect(result.message).to eq(_('Failed to update page settings')) + end + end end - it 'updates pages_unique_domain_enabled setting' do - create(:project_setting, project: project, pages_unique_domain_enabled: true, - pages_unique_domain: "random-unique-domain-here") + context 'with insufficient permissions' do + let(:service) { described_class.new(project, user, params) } + + it 'returns a forbidden response' do + result = service.execute - expect { service.execute } - .to change { project.reload.project_setting.pages_unique_domain_enabled } - .from(true).to(false) + expect(result).to be_a(ServiceResponse) + expect(result.error?).to be(true) + expect(result.message).to eq(_('The current user is not authorized to update the page settings')) + end end end end -- GitLab From 429b1e8e72d0511e93470c88dc88436fb270e686 Mon Sep 17 00:00:00 2001 From: Justin Zeng Date: Sat, 6 Apr 2024 18:59:18 -0700 Subject: [PATCH 8/9] Translate strings --- locale/gitlab.pot | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 9735df9edd5cc6..500c20346fd5c1 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -21362,6 +21362,9 @@ msgstr "" msgid "Failed to update organization" msgstr "" +msgid "Failed to update page settings" +msgstr "" + msgid "Failed to update the Canary Ingress." msgstr "" @@ -51018,6 +51021,9 @@ msgstr "" msgid "The current user is not authorized to set pipeline schedule variables" msgstr "" +msgid "The current user is not authorized to update the page settings" +msgstr "" + msgid "The current user is not authorized to update the pipeline schedule" msgstr "" -- GitLab From 46f52f42ecb27610455966633a526e7189c2ff31 Mon Sep 17 00:00:00 2001 From: Justin Zeng Date: Wed, 10 Apr 2024 20:40:49 -0700 Subject: [PATCH 9/9] Remove UpdateError and rescue --- app/services/pages/update_service.rb | 15 +++++---------- lib/api/pages.rb | 4 +--- locale/gitlab.pot | 3 --- spec/requests/api/pages_spec.rb | 18 ------------------ spec/services/pages/update_service_spec.rb | 16 ---------------- 5 files changed, 6 insertions(+), 50 deletions(-) diff --git a/app/services/pages/update_service.rb b/app/services/pages/update_service.rb index e88a47ca37eb4b..f27b1c29442fd5 100644 --- a/app/services/pages/update_service.rb +++ b/app/services/pages/update_service.rb @@ -3,7 +3,6 @@ module Pages class UpdateService < BaseService include Gitlab::Allowable - UpdateError = Class.new(StandardError) def execute unless can_update_page_settings? @@ -11,16 +10,12 @@ def execute reason: :forbidden) end - begin - Project.transaction do - update_pages_unique_domain_enabled! - update_pages_https_only! - end - - ServiceResponse.success(payload: { project: project }) - rescue UpdateError - ServiceResponse.error(message: _('Failed to update page settings')) + Project.transaction do + update_pages_unique_domain_enabled! + update_pages_https_only! end + + ServiceResponse.success(payload: { project: project }) end private diff --git a/lib/api/pages.rb b/lib/api/pages.rb index a2aa8e3b2db859..8f038c175bd5fd 100644 --- a/lib/api/pages.rb +++ b/lib/api/pages.rb @@ -54,10 +54,8 @@ class Pages < ::API::Base if response.success? present ::Pages::ProjectSettings.new(response.payload[:project]), with: Entities::Pages::ProjectSettings - elsif response.reason == :forbidden - forbidden!(response.message) else - bad_request!(response.message) + forbidden!(response.message) end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 500c20346fd5c1..dca7a90a26785d 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -21362,9 +21362,6 @@ msgstr "" msgid "Failed to update organization" msgstr "" -msgid "Failed to update page settings" -msgstr "" - msgid "Failed to update the Canary Ingress." msgstr "" diff --git a/spec/requests/api/pages_spec.rb b/spec/requests/api/pages_spec.rb index e734b48678a11b..a4a066075a02be 100644 --- a/spec/requests/api/pages_spec.rb +++ b/spec/requests/api/pages_spec.rb @@ -107,24 +107,6 @@ expect(json_response['is_unique_domain_enabled']).to eq(true) end end - - context 'and update service fails' do - before do - failure_response = instance_double(ServiceResponse, success?: false, reason: :validation_error, - message: "Failed to update page settings") - - allow_next_instance_of(::Pages::UpdateService) do |instance| - allow(instance).to receive(:execute).and_return(failure_response) - end - end - - it 'returns a 400 bad request' do - patch api(path, admin, admin_mode: true), params: params - - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['message']).to eq("400 Bad request - Failed to update page settings") - end - end end context 'when the user is unauthorized' do diff --git a/spec/services/pages/update_service_spec.rb b/spec/services/pages/update_service_spec.rb index 0ea039362b4201..35086956124c30 100644 --- a/spec/services/pages/update_service_spec.rb +++ b/spec/services/pages/update_service_spec.rb @@ -39,22 +39,6 @@ expect(result.payload[:project]).to eq(project) end end - - context 'when an update raises an UpdateError' do - before do - allow_next_instance_of(ProjectSetting) do |instance| - allow(instance).to receive(:update!).and_raise(Pages::UpdateService::UpdateError) - end - end - - it 'returns an error response' do - result = service.execute - - expect(result).to be_a(ServiceResponse) - expect(result.error?).to be(true) - expect(result.message).to eq(_('Failed to update page settings')) - end - end end context 'with insufficient permissions' do -- GitLab