diff --git a/modules/storages/app/common/storages/peripherals/one_drive_connection_validator.rb b/modules/storages/app/common/storages/peripherals/one_drive_connection_validator.rb index 86b6c6feb0d4..0d53bc9cb960 100644 --- a/modules/storages/app/common/storages/peripherals/one_drive_connection_validator.rb +++ b/modules/storages/app/common/storages/peripherals/one_drive_connection_validator.rb @@ -47,7 +47,10 @@ def validate .or { drive_id_wrong } .or { request_failed_with_unknown_error } .or { drive_with_unexpected_content } - .value_or(ConnectionValidation.new(type: :healthy, timestamp: Time.current, description: nil)) + .value_or(ConnectionValidation.new(type: :healthy, + error_code: :none, + timestamp: Time.current, + description: nil)) end private @@ -62,6 +65,7 @@ def maybe_is_not_configured return None() if @storage.configured? Some(ConnectionValidation.new(type: :none, + error_code: :wrn_not_configured, timestamp: Time.current, description: I18n.t("storages.health.connection_validation.not_configured"))) end @@ -70,6 +74,7 @@ def drive_id_wrong return None() if query.result != :not_found Some(ConnectionValidation.new(type: :error, + error_code: :err_drive_invalid, timestamp: Time.current, description: I18n.t("storages.health.connection_validation.drive_id_wrong"))) end @@ -84,6 +89,7 @@ def tenant_id_wrong return None() unless payload["error_description"].include?(tenant_id_string) Some(ConnectionValidation.new(type: :error, + error_code: :err_tenant_invalid, timestamp: Time.current, description: I18n.t("storages.health.connection_validation.tenant_id_wrong"))) end @@ -95,6 +101,7 @@ def client_id_wrong return None() if payload["error"] != "unauthorized_client" Some(ConnectionValidation.new(type: :error, + error_code: :err_client_invalid, timestamp: Time.current, description: I18n.t("storages.health.connection_validation.client_id_wrong"))) end @@ -106,6 +113,7 @@ def client_secret_wrong return None() if payload["error"] != "invalid_client" Some(ConnectionValidation.new(type: :error, + error_code: :err_client_invalid, timestamp: Time.current, description: I18n.t("storages.health.connection_validation.client_secret_wrong"))) end @@ -123,6 +131,7 @@ def drive_with_unexpected_content return None() if unexpected_files.empty? Some(ConnectionValidation.new(type: :warning, + error_code: :wrn_unexpected_content, timestamp: Time.current, description: I18n.t("storages.health.connection_validation.unexpected_content"))) end @@ -136,6 +145,7 @@ def request_failed_with_unknown_error "status: #{query.result}\n\tresponse: #{query.error_payload}") Some(ConnectionValidation.new(type: :error, + error_code: :err_unknown, timestamp: Time.current, description: I18n.t("storages.health.connection_validation.unknown_error"))) end diff --git a/modules/storages/app/components/storages/admin/sidebar/health_status_component.html.erb b/modules/storages/app/components/storages/admin/sidebar/health_status_component.html.erb index 64e2b2e94f07..81159ee25382 100644 --- a/modules/storages/app/components/storages/admin/sidebar/health_status_component.html.erb +++ b/modules/storages/app/components/storages/admin/sidebar/health_status_component.html.erb @@ -36,9 +36,7 @@ See COPYRIGHT and LICENSE files for more details. if @storage.provider_type_one_drive? health_status_container.with_row(mt: 2) do - render(OpTurbo::FrameComponent.new(id: :connection_validation_result)) do - render(Storages::Admin::Sidebar::ValidationResultComponent.new(result: validation_result_placeholder)) - end + render(Storages::Admin::Sidebar::ValidationResultComponent.new(result: validation_result_placeholder)) end health_status_container.with_row(mt: 2) do diff --git a/modules/storages/app/components/storages/admin/sidebar/health_status_component.rb b/modules/storages/app/components/storages/admin/sidebar/health_status_component.rb index 53f563c199d2..97a3383375a9 100644 --- a/modules/storages/app/components/storages/admin/sidebar/health_status_component.rb +++ b/modules/storages/app/components/storages/admin/sidebar/health_status_component.rb @@ -63,6 +63,7 @@ def formatted_health_reason def validation_result_placeholder ConnectionValidation.new(type: :none, + error_code: :none, timestamp: Time.current, description: I18n.t("storages.health.connection_validation.placeholder")) end diff --git a/modules/storages/app/components/storages/admin/sidebar/validation_result_component.html.erb b/modules/storages/app/components/storages/admin/sidebar/validation_result_component.html.erb index 383618b5dff3..b50ef2442045 100644 --- a/modules/storages/app/components/storages/admin/sidebar/validation_result_component.html.erb +++ b/modules/storages/app/components/storages/admin/sidebar/validation_result_component.html.erb @@ -28,28 +28,32 @@ See COPYRIGHT and LICENSE files for more details. ++#%> <%= - flex_layout do |container| - container.with_row do - render(Primer::Beta::Text.new(font_weight: :bold, test_selector: "validation-result--subtitle")) do - I18n.t("storages.health.connection_validation.subtitle") + component_wrapper do + flex_layout do |container| + container.with_row do + render(Primer::Beta::Text.new(font_weight: :bold, test_selector: "validation-result--subtitle")) do + I18n.t("storages.health.connection_validation.subtitle") + end end - end - if @result.type != :none - container.with_row(mt: 2) do - status = status_indicator + if @result.validation_result_exists? + container.with_row(mt: 2) do + status = status_indicator - concat(render(Primer::Beta::Text.new(pr: 2, test_selector: "validation-result--timestamp")) do - I18n.t('storages.health.checked', datetime: helpers.format_time(@result.timestamp)) - end) - concat(render(Primer::Beta::Label.new(scheme: status[:scheme])) { status[:label] }) + concat(render(Primer::Beta::Text.new(pr: 2, test_selector: "validation-result--timestamp")) do + I18n.t('storages.health.checked', datetime: helpers.format_time(@result.timestamp)) + end) + concat(render(Primer::Beta::Label.new(scheme: status[:scheme])) { status[:label] }) + end end - end - if @result.description.present? - container.with_row(mt: 2) do - render(Primer::Beta::Text.new(color: :muted, test_selector: "validation-result--description")) do - @result.description + if @result.description.present? + prefix = @result.error_code? ? "#{@result.error_code.upcase}: " : "" + + container.with_row(mt: 2) do + render(Primer::Beta::Text.new(color: :muted, test_selector: "validation-result--description")) do + prefix + @result.description + end end end end diff --git a/modules/storages/app/components/storages/admin/sidebar/validation_result_component.rb b/modules/storages/app/components/storages/admin/sidebar/validation_result_component.rb index 4c745fba7f58..f145051b2929 100644 --- a/modules/storages/app/components/storages/admin/sidebar/validation_result_component.rb +++ b/modules/storages/app/components/storages/admin/sidebar/validation_result_component.rb @@ -33,6 +33,7 @@ module Admin module Sidebar class ValidationResultComponent < ApplicationComponent # rubocop:disable OpenProject/AddPreviewForViewComponent include OpPrimer::ComponentHelpers + include OpTurbo::Streamable def initialize(result:) super(result) diff --git a/modules/storages/app/components/storages/admin/sidebar_component.html.erb b/modules/storages/app/components/storages/admin/sidebar_component.html.erb index 69515e44110e..918d19b50396 100644 --- a/modules/storages/app/components/storages/admin/sidebar_component.html.erb +++ b/modules/storages/app/components/storages/admin/sidebar_component.html.erb @@ -2,7 +2,7 @@ component_wrapper do render(Primer::OpenProject::BorderGrid.new) do |border_grid| border_grid.with_row { render(Storages::Admin::Sidebar::HealthStatusComponent.new(storage: @storage)) } - + if @storage.automatic_management_enabled? border_grid.with_row { render(Storages::Admin::Sidebar::HealthNotificationsComponent.new(storage: @storage)) } end diff --git a/modules/storages/app/controllers/storages/admin/connection_validation_controller.rb b/modules/storages/app/controllers/storages/admin/connection_validation_controller.rb index 123a95a9407a..fd758c0ea165 100644 --- a/modules/storages/app/controllers/storages/admin/connection_validation_controller.rb +++ b/modules/storages/app/controllers/storages/admin/connection_validation_controller.rb @@ -45,10 +45,8 @@ def validate_connection @result = Peripherals::OneDriveConnectionValidator .new(storage: @storage) .validate - - respond_to do |format| - format.turbo_stream - end + update_via_turbo_stream(component: Sidebar::ValidationResultComponent.new(result: @result)) + respond_to_with_turbo_streams end private diff --git a/modules/storages/app/models/storages/connection_validation.rb b/modules/storages/app/models/storages/connection_validation.rb index 6b2845089ada..a9ad82f087d8 100644 --- a/modules/storages/app/models/storages/connection_validation.rb +++ b/modules/storages/app/models/storages/connection_validation.rb @@ -29,5 +29,13 @@ #++ module Storages - ConnectionValidation = Data.define(:type, :description, :timestamp) + ConnectionValidation = Data.define(:type, :error_code, :description, :timestamp) do + def validation_result_exists? + type.present? && type != :none + end + + def error_code? + error_code.present? && error_code != :none + end + end end diff --git a/modules/storages/app/views/storages/admin/connection_validation/validate_connection.turbo_stream.erb b/modules/storages/app/views/storages/admin/connection_validation/validate_connection.turbo_stream.erb deleted file mode 100644 index f42b7b4d08a7..000000000000 --- a/modules/storages/app/views/storages/admin/connection_validation/validate_connection.turbo_stream.erb +++ /dev/null @@ -1,32 +0,0 @@ -<%#-- copyright -OpenProject is an open source project management software. -Copyright (C) 2012-2024 the OpenProject GmbH - -This program is free software; you can redistribute it and/or -modify it under the terms of the GNU General Public License version 3. - -OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: -Copyright (C) 2006-2013 Jean-Philippe Lang -Copyright (C) 2010-2013 the ChiliProject Team - -This program is free software; you can redistribute it and/or -modify it under the terms of the GNU General Public License -as published by the Free Software Foundation; either version 2 -of the License, or (at your option) any later version. - -This program is distributed in the hope that it will be useful, -but WITHOUT ANY WARRANTY; without even the implied warranty of -MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -GNU General Public License for more details. - -You should have received a copy of the GNU General Public License -along with this program; if not, write to the Free Software -Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - -See COPYRIGHT and LICENSE files for more details. - -++#%> - -<%= turbo_stream.update :connection_validation_result do %> - <%= render(Storages::Admin::Sidebar::ValidationResultComponent.new(result: @result)) %> -<% end %> diff --git a/modules/storages/spec/common/storages/peripherals/one_drive_connection_validator_spec.rb b/modules/storages/spec/common/storages/peripherals/one_drive_connection_validator_spec.rb index 37744b344cef..6aed3e2cf694 100644 --- a/modules/storages/spec/common/storages/peripherals/one_drive_connection_validator_spec.rb +++ b/modules/storages/spec/common/storages/peripherals/one_drive_connection_validator_spec.rb @@ -43,6 +43,7 @@ it "returns a validation failure" do expect(subject.type).to eq(:none) + expect(subject.error_code).to eq(:wrn_not_configured) expect(subject.description).to eq("The connection could not be validated. Please finish configuration first.") end end @@ -58,6 +59,7 @@ it "returns a validation failure" do expect(subject.type).to eq(:error) + expect(subject.error_code).to eq(:err_tenant_invalid) expect(subject.description) .to eq("The configured directory (tenant) id is invalid. Please check the configuration.") end @@ -69,6 +71,7 @@ it "returns a validation failure" do expect(subject.type).to eq(:error) + expect(subject.error_code).to eq(:err_client_invalid) expect(subject.description).to eq("The configured OAuth 2 client id is invalid. Please check the configuration.") end end @@ -79,6 +82,7 @@ it "returns a validation failure" do expect(subject.type).to eq(:error) + expect(subject.error_code).to eq(:err_client_invalid) expect(subject.description) .to eq("The configured OAuth 2 client secret is invalid. Please check the configuration.") end @@ -89,6 +93,7 @@ it "returns a validation failure" do expect(subject.type).to eq(:error) + expect(subject.error_code).to eq(:err_drive_invalid) expect(subject.description).to eq("The configured drive id could not be found. Please check the configuration.") end end @@ -102,6 +107,7 @@ it "returns a validation failure" do expect(subject.type).to eq(:error) + expect(subject.error_code).to eq(:err_unknown) expect(subject.description) .to eq("The connection could not be validated. An unknown error occurred. " \ "Please check the server logs for further information.") @@ -141,6 +147,7 @@ it "returns a validation failure" do expect(subject.type).to eq(:warning) + expect(subject.error_code).to eq(:wrn_unexpected_content) expect(subject.description).to eq("Unexpected content found in the drive.") end end @@ -150,6 +157,7 @@ it "returns a validation success" do expect(subject.type).to eq(:healthy) + expect(subject.error_code).to eq(:none) expect(subject.description).to be_nil end end diff --git a/modules/storages/spec/requests/storages/admin/connection_validation_spec.rb b/modules/storages/spec/requests/storages/admin/connection_validation_spec.rb index 10fc607eb805..b3b0cfe7b9de 100644 --- a/modules/storages/spec/requests/storages/admin/connection_validation_spec.rb +++ b/modules/storages/spec/requests/storages/admin/connection_validation_spec.rb @@ -77,7 +77,10 @@ context "if the a validation result of type :none (no validation executed) is returned" do let(:validation_result) do - Storages::ConnectionValidation.new(type: :none, timestamp: Time.current, description: "not configured") + Storages::ConnectionValidation.new(type: :none, + error_code: :none, + timestamp: Time.current, + description: "not configured") end it_behaves_like "a validation result template", show_timestamp: false, label: nil, description: "not configured" @@ -85,27 +88,31 @@ context "if validator returns an error" do let(:validation_result) do - Storages::ConnectionValidation.new(type: :error, timestamp: Time.current, description: "An error occurred") + Storages::ConnectionValidation.new(type: :error, + error_code: :my_err, + timestamp: Time.current, + description: "An error occurred") end it_behaves_like "a validation result template", - show_timestamp: true, label: "Error", description: "An error occurred" + show_timestamp: true, label: "Error", description: "MY_ERR: An error occurred" end context "if validator returns a warning" do let(:validation_result) do Storages::ConnectionValidation.new(type: :warning, + error_code: :my_wrn, timestamp: Time.current, description: "There is something weird...") end it_behaves_like "a validation result template", - show_timestamp: true, label: "Warning", description: "There is something weird..." + show_timestamp: true, label: "Warning", description: "MY_WRN: There is something weird..." end context "if validator returns a success" do let(:validation_result) do - Storages::ConnectionValidation.new(type: :healthy, timestamp: Time.current, description: nil) + Storages::ConnectionValidation.new(type: :healthy, error_code: :none, timestamp: Time.current, description: nil) end it_behaves_like "a validation result template", @@ -115,22 +122,21 @@ private - # rubocop:disable Layout/LineLength def xpath_for_subtitle - "//turbo-stream[@target='connection_validation_result']/template/div/div/span[@data-test-selector='validation-result--subtitle']" + "#{xpath_for_turbo_target}/div/div/span[@data-test-selector='validation-result--subtitle']" end def xpath_for_timestamp - "//turbo-stream[@target='connection_validation_result']/template/div/div/span[@data-test-selector='validation-result--timestamp']" + "#{xpath_for_turbo_target}/div/div/span[@data-test-selector='validation-result--timestamp']" end def xpath_for_label - "//turbo-stream[@target='connection_validation_result']/template/div/div/span[contains(@class, 'Label')]" + "#{xpath_for_turbo_target}/div/div/span[contains(@class, 'Label')]" end def xpath_for_description - "//turbo-stream[@target='connection_validation_result']/template/div/div/span[@data-test-selector='validation-result--description']" + "#{xpath_for_turbo_target}/div/div/span[@data-test-selector='validation-result--description']" end - # rubocop:enable Layout/LineLength + def xpath_for_turbo_target = "//turbo-stream[@target='storages-admin-sidebar-validation-result-component']/template" end