From 71dbd861b38b19b720778928fd66f019ca5ec2e2 Mon Sep 17 00:00:00 2001 From: Eric Schubert Date: Fri, 7 Jun 2024 12:39:26 +0200 Subject: [PATCH 1/9] [#55382] add connection validation section - https://community.openproject.org/work_packages/55382 - added new controller for connection validation - added new component to sidebar --- Gemfile | 1 + Gemfile.lock | 5 + .../connection_validation_component.html.erb | 68 ++++++++ .../connection_validation_component.rb | 45 ++++++ .../storages/admin/sidebar_component.html.erb | 1 + .../admin/connection_validation_controller.rb | 148 ++++++++++++++++++ .../models/storages/connection_validation.rb | 33 ++++ .../validate_connection.turbo_stream.erb | 32 ++++ modules/storages/config/locales/en.yml | 10 ++ modules/storages/config/routes.rb | 11 +- 10 files changed, 352 insertions(+), 2 deletions(-) create mode 100644 modules/storages/app/components/storages/admin/sidebar/connection_validation_component.html.erb create mode 100644 modules/storages/app/components/storages/admin/sidebar/connection_validation_component.rb create mode 100644 modules/storages/app/controllers/storages/admin/connection_validation_controller.rb create mode 100644 modules/storages/app/models/storages/connection_validation.rb create mode 100644 modules/storages/app/views/storages/admin/connection_validation/validate_connection.turbo_stream.erb diff --git a/Gemfile b/Gemfile index 35b9e21a8add..ea8c8d5f4dd0 100644 --- a/Gemfile +++ b/Gemfile @@ -210,6 +210,7 @@ gem "validate_url" # Storages support code gem "dry-container" +gem "dry-monads" # ActiveRecord extension which adds typecasting to store accessors gem "store_attribute", "~> 1.0" diff --git a/Gemfile.lock b/Gemfile.lock index ba216ee21a8b..fa7d48ddd91a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -476,6 +476,10 @@ GEM concurrent-ruby (~> 1.0) dry-core (~> 1.0, < 2) zeitwerk (~> 2.6) + dry-monads (1.6.0) + concurrent-ruby (~> 1.0) + dry-core (~> 1.0, < 2) + zeitwerk (~> 2.6) dry-types (1.7.2) bigdecimal (~> 3.0) concurrent-ruby (~> 1.0) @@ -1195,6 +1199,7 @@ DEPENDENCIES doorkeeper (~> 5.7.0) dotenv-rails dry-container + dry-monads email_validator (~> 2.2.3) equivalent-xml (~> 0.6) erb_lint diff --git a/modules/storages/app/components/storages/admin/sidebar/connection_validation_component.html.erb b/modules/storages/app/components/storages/admin/sidebar/connection_validation_component.html.erb new file mode 100644 index 000000000000..d5e13502875f --- /dev/null +++ b/modules/storages/app/components/storages/admin/sidebar/connection_validation_component.html.erb @@ -0,0 +1,68 @@ +<%#-- 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. + +++#%> + +<%= + component_wrapper do + flex_layout do |container| + container.with_row do + flex_layout do |heading| + heading.with_row do + render(Primer::Beta::Heading.new(tag: :h4)) { I18n.t("storages.connection_validation.title") } + end + end + end + + container.with_row(mt: 2) do + primer_form_with( + model: @storage, + url: validate_connection_admin_settings_storage_connection_validation_path(@storage), + method: :post, + data: { turbo: true } + ) do + flex_layout do |form| + form.with_row do + render(OpTurbo::FrameComponent.new(id: :connection_validation_result)) + end + + form.with_row(mt: 2) do + render(Primer::Beta::Button.new( + scheme: :default, + block: :true, + type: :submit, + )) do |button| + button.with_leading_visual_icon(icon: "tasklist") + I18n.t("storages.connection_validation.action") + end + end + end + end + end + end + end +%> diff --git a/modules/storages/app/components/storages/admin/sidebar/connection_validation_component.rb b/modules/storages/app/components/storages/admin/sidebar/connection_validation_component.rb new file mode 100644 index 000000000000..f8f0176d52f6 --- /dev/null +++ b/modules/storages/app/components/storages/admin/sidebar/connection_validation_component.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +#-- 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. +#++ + +module Storages + module Admin + module Sidebar + class ConnectionValidationComponent < ApplicationComponent # rubocop:disable OpenProject/AddPreviewForViewComponent + include OpTurbo::Streamable + include OpPrimer::ComponentHelpers + + def initialize(storage:) + super(storage) + @storage = storage + end + end + end + end +end 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 3b19f1e32f1b..720b0e4c00cb 100644 --- a/modules/storages/app/components/storages/admin/sidebar_component.html.erb +++ b/modules/storages/app/components/storages/admin/sidebar_component.html.erb @@ -1,6 +1,7 @@ <%= component_wrapper do render(Primer::OpenProject::BorderGrid.new) do |border_grid| + border_grid.with_row { render(Storages::Admin::Sidebar::ConnectionValidationComponent.new(storage: @storage)) } border_grid.with_row { render(Storages::Admin::Sidebar::HealthStatusComponent.new(storage: @storage)) } 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 new file mode 100644 index 000000000000..b18dcf1c6485 --- /dev/null +++ b/modules/storages/app/controllers/storages/admin/connection_validation_controller.rb @@ -0,0 +1,148 @@ +# frozen_string_literal: true + +#-- 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. +#++ + +module Storages + module Admin + class ConnectionValidationController < ApplicationController + include OpTurbo::ComponentStream + include Dry::Monads[:maybe] + + using Peripherals::ServiceResultRefinements + + layout "admin" + + before_action :require_admin + + model_object OneDriveStorage + + before_action :find_model_object, only: %i[validate_connection] + + def validate_connection + @result = maybe_is_not_configured + .or { drive_id_wrong } + .or { tenant_id_wrong } + .or { client_id_wrong } + .or { client_secret_wrong } + .or { request_failed_with_unknown_error } + .value_or(ConnectionValidation.new(icon: "check-circle", + scheme: :success, + description: I18n.t("storages.connection_validation.success"))) + + respond_to do |format| + format.turbo_stream + end + end + + private + + def query + @query ||= Peripherals::Registry + .resolve("#{@storage.short_provider_type}.queries.files") + .call(storage: @storage, auth_strategy:, folder: root_folder) + end + + def maybe_is_not_configured + return None() if @storage.configured? + + Some(ConnectionValidation.new(icon: :alert, + scheme: :warning, + description: I18n.t("storages.connection_validation.not_configured"))) + end + + def drive_id_wrong + return None() if query.result != :not_found + + Some(ConnectionValidation.new(icon: :skip, + scheme: :danger, + description: I18n.t("storages.connection_validation.drive_id_wrong"))) + end + + def tenant_id_wrong + return None() if query.result != :unauthorized + + payload = JSON.parse(query.error_payload) + return None() if payload["error"] != "invalid_request" + + tenant_id_string = "Tenant '#{@storage.tenant_id}' not found." + return None() unless payload["error_description"].include?(tenant_id_string) + + Some(ConnectionValidation.new(icon: :skip, + scheme: :danger, + description: I18n.t("storages.connection_validation.tenant_id_wrong"))) + end + + def client_id_wrong + return None() if query.result != :unauthorized + + payload = JSON.parse(query.error_payload) + return None() if payload["error"] != "unauthorized_client" + + Some(ConnectionValidation.new(icon: :skip, + scheme: :danger, + description: I18n.t("storages.connection_validation.client_id_wrong"))) + end + + def client_secret_wrong + return None() if query.result != :unauthorized + + payload = JSON.parse(query.error_payload) + return None() if payload["error"] != "invalid_client" + + Some(ConnectionValidation.new(icon: :skip, + scheme: :danger, + description: I18n.t("storages.connection_validation.client_secret_wrong"))) + end + + def request_failed_with_unknown_error + return None() if query.success? + + Rails.logger.error("Connection validation failed with unknown error:\n\t" \ + "status: #{query.result}\n\tresponse: #{query.error_payload}") + + Some(ConnectionValidation.new(icon: :skip, + scheme: :danger, + description: I18n.t("storages.connection_validation.unknown_error"))) + end + + def find_model_object(object_id = :storage_id) + super + @storage = @object + end + + def root_folder + Peripherals::ParentFolder.new("/") + end + + def auth_strategy + Peripherals::Registry.resolve("#{@storage.short_provider_type}.authentication.userless").call + end + end + end +end diff --git a/modules/storages/app/models/storages/connection_validation.rb b/modules/storages/app/models/storages/connection_validation.rb new file mode 100644 index 000000000000..9840279040e5 --- /dev/null +++ b/modules/storages/app/models/storages/connection_validation.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +#-- 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. +#++ + +module Storages + ConnectionValidation = Data.define(:icon, :scheme, :description) +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 new file mode 100644 index 000000000000..8c6bf8749e2e --- /dev/null +++ b/modules/storages/app/views/storages/admin/connection_validation/validate_connection.turbo_stream.erb @@ -0,0 +1,32 @@ +<%#-- 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(Primer::Alpha::Banner.new(icon: @result.icon, scheme: @result.scheme)) { @result.description } %> +<% end %> diff --git a/modules/storages/config/locales/en.yml b/modules/storages/config/locales/en.yml index 8e0acddddc4b..e92dd48a1f70 100644 --- a/modules/storages/config/locales/en.yml +++ b/modules/storages/config/locales/en.yml @@ -116,6 +116,16 @@ en: subscribe: Subscribe title: Email notifications unsubscribe: Unsubscribe + connection_validation: + title: Connection validation + action: Validate connection + not_configured: The connection could not be validated. Please finish configuration first. + drive_id_wrong: The configured drive id could not be found. Please check the configuration. + tenant_id_wrong: The configured directory (tenant) id is invalid. Please check the configuration. + client_id_wrong: The configured OAuth 2 client id is invalid. Please check the configuration. + client_secret_wrong: The configured OAuth 2 client secret is invalid. Please check the configuration. + unknown_error: The connection could not be validated. An unknown error occurred. Please check the server logs for further information. + success: The connection works as expected. help_texts: project_folder: The project folder is the default folder for file uploads for this project. Users can nevertheless still upload files to other locations. instructions: diff --git a/modules/storages/config/routes.rb b/modules/storages/config/routes.rb index 30617b2ca52b..04b2d533977b 100644 --- a/modules/storages/config/routes.rb +++ b/modules/storages/config/routes.rb @@ -38,11 +38,18 @@ post :finish_setup end - resource :automatically_managed_project_folders, controller: "/storages/admin/automatically_managed_project_folders", - only: %i[new create edit update] + resource :automatically_managed_project_folders, + controller: "/storages/admin/automatically_managed_project_folders", + only: %i[new create edit update] resource :access_management, controller: "/storages/admin/access_management", only: %i[new create edit update] + resource :connection_validation, + controller: "/storages/admin/connection_validation", + only: [] do + post :validate_connection, on: :member + end + get :select_provider, on: :collection member do From fbc5d269af726bf03be766e83da4930f115e1946 Mon Sep 17 00:00:00 2001 From: Eric Schubert Date: Fri, 7 Jun 2024 12:44:10 +0200 Subject: [PATCH 2/9] [#55382] show connection validation only for one drive --- .../app/components/storages/admin/sidebar_component.html.erb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 720b0e4c00cb..b4f92cfad5f8 100644 --- a/modules/storages/app/components/storages/admin/sidebar_component.html.erb +++ b/modules/storages/app/components/storages/admin/sidebar_component.html.erb @@ -1,7 +1,10 @@ <%= component_wrapper do render(Primer::OpenProject::BorderGrid.new) do |border_grid| - border_grid.with_row { render(Storages::Admin::Sidebar::ConnectionValidationComponent.new(storage: @storage)) } + if @storage.provider_type_one_drive? + border_grid.with_row { render(Storages::Admin::Sidebar::ConnectionValidationComponent.new(storage: @storage)) } + end + border_grid.with_row { render(Storages::Admin::Sidebar::HealthStatusComponent.new(storage: @storage)) } border_grid.with_row { render(Storages::Admin::Sidebar::HealthNotificationsComponent.new(storage: @storage)) } end From d7e9d6f495fa688a31f60534d3b5c5e74c08f383 Mon Sep 17 00:00:00 2001 From: Eric Schubert Date: Fri, 7 Jun 2024 14:00:53 +0200 Subject: [PATCH 3/9] [#55382] validate drive content (only ampf) - enable sidebar for all one drive storages to show the connection validation --- .../storages/admin/sidebar_component.html.erb | 6 +++-- .../admin/connection_validation_controller.rb | 23 +++++++++++++++++++ .../storages/admin/storages/edit.html.erb | 3 ++- modules/storages/config/locales/en.yml | 1 + 4 files changed, 30 insertions(+), 3 deletions(-) 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 b4f92cfad5f8..879910a31322 100644 --- a/modules/storages/app/components/storages/admin/sidebar_component.html.erb +++ b/modules/storages/app/components/storages/admin/sidebar_component.html.erb @@ -5,8 +5,10 @@ border_grid.with_row { render(Storages::Admin::Sidebar::ConnectionValidationComponent.new(storage: @storage)) } end - border_grid.with_row { render(Storages::Admin::Sidebar::HealthStatusComponent.new(storage: @storage)) } - border_grid.with_row { render(Storages::Admin::Sidebar::HealthNotificationsComponent.new(storage: @storage)) } + if @storage.automatic_management_enabled? + border_grid.with_row { render(Storages::Admin::Sidebar::HealthStatusComponent.new(storage: @storage)) } + border_grid.with_row { render(Storages::Admin::Sidebar::HealthNotificationsComponent.new(storage: @storage)) } + end end 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 b18dcf1c6485..28de15ce1e20 100644 --- a/modules/storages/app/controllers/storages/admin/connection_validation_controller.rb +++ b/modules/storages/app/controllers/storages/admin/connection_validation_controller.rb @@ -44,6 +44,7 @@ class ConnectionValidationController < ApplicationController before_action :find_model_object, only: %i[validate_connection] + # rubocop:disable Metrics/AbcSize def validate_connection @result = maybe_is_not_configured .or { drive_id_wrong } @@ -51,6 +52,7 @@ def validate_connection .or { client_id_wrong } .or { client_secret_wrong } .or { request_failed_with_unknown_error } + .or { drive_with_unexpected_content } .value_or(ConnectionValidation.new(icon: "check-circle", scheme: :success, description: I18n.t("storages.connection_validation.success"))) @@ -60,6 +62,8 @@ def validate_connection end end + # rubocop:enable Metrics/AbcSize + private def query @@ -120,6 +124,25 @@ def client_secret_wrong description: I18n.t("storages.connection_validation.client_secret_wrong"))) end + # rubocop:disable Metrics/AbcSize + def drive_with_unexpected_content + return None() if query.failure? + return None() unless @storage.automatic_management_enabled? + + expected_folder_ids = @storage.project_storages + .where(project_folder_mode: "automatic") + .map(&:project_folder_id) + + unexpected_files = query.result.files.reject { |file| expected_folder_ids.include?(file.id) } + return None() if unexpected_files.empty? + + Some(ConnectionValidation.new(icon: :alert, + scheme: :warning, + description: I18n.t("storages.connection_validation.unexpected_content"))) + end + + # rubocop:enable Metrics/AbcSize + def request_failed_with_unknown_error return None() if query.success? diff --git a/modules/storages/app/views/storages/admin/storages/edit.html.erb b/modules/storages/app/views/storages/admin/storages/edit.html.erb index ecda5f36208a..b5d320e8349a 100644 --- a/modules/storages/app/views/storages/admin/storages/edit.html.erb +++ b/modules/storages/app/views/storages/admin/storages/edit.html.erb @@ -64,7 +64,8 @@ See COPYRIGHT and LICENSE files for more details. <% end %> <% end %> -<% if @storage.automatic_management_enabled? %> +<% display_sidebar = @storage.provider_type_one_drive? || @storage.automatic_management_enabled? %> +<% if display_sidebar %> <%= render(Primer::Alpha::Layout.new(stacking_breakpoint: :lg)) do |component| %> <% component.with_main() do %> <%= render(::Storages::Admin::StorageViewComponent.new(@storage)) %> diff --git a/modules/storages/config/locales/en.yml b/modules/storages/config/locales/en.yml index e92dd48a1f70..8c61a1d5aeeb 100644 --- a/modules/storages/config/locales/en.yml +++ b/modules/storages/config/locales/en.yml @@ -125,6 +125,7 @@ en: client_id_wrong: The configured OAuth 2 client id is invalid. Please check the configuration. client_secret_wrong: The configured OAuth 2 client secret is invalid. Please check the configuration. unknown_error: The connection could not be validated. An unknown error occurred. Please check the server logs for further information. + unexpected_content: Unexpected content found in the drive. success: The connection works as expected. help_texts: project_folder: The project folder is the default folder for file uploads for this project. Users can nevertheless still upload files to other locations. From 26233a005b453844e7ace921a8605eea82073311 Mon Sep 17 00:00:00 2001 From: Eric Schubert Date: Wed, 12 Jun 2024 13:26:23 +0200 Subject: [PATCH 4/9] [#55382] extracted validator to own class --- .../one_drive_connection_validator.rb | 154 ++++++++++++++++++ .../admin/connection_validation_controller.rb | 115 +------------ .../admin/connection_validation_spec.rb | 57 +++++++ 3 files changed, 214 insertions(+), 112 deletions(-) create mode 100644 modules/storages/app/common/storages/peripherals/one_drive_connection_validator.rb create mode 100644 modules/storages/spec/requests/storages/admin/connection_validation_spec.rb 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 new file mode 100644 index 000000000000..01a7c9e27881 --- /dev/null +++ b/modules/storages/app/common/storages/peripherals/one_drive_connection_validator.rb @@ -0,0 +1,154 @@ +# frozen_string_literal:true + +#-- 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. +#++ + +module Storages + module Peripherals + class OneDriveConnectionValidator + include Dry::Monads[:maybe] + + using ServiceResultRefinements + + def initialize(storage:) + @storage = storage + end + + def validate + maybe_is_not_configured + .or { tenant_id_wrong } + .or { client_id_wrong } + .or { client_secret_wrong } + .or { drive_id_wrong } + .or { request_failed_with_unknown_error } + .or { drive_with_unexpected_content } + .value_or(ConnectionValidation.new(icon: "check-circle", + scheme: :success, + description: I18n.t("storages.connection_validation.success"))) + end + + private + + def query + @query ||= Peripherals::Registry + .resolve("#{@storage.short_provider_type}.queries.files") + .call(storage: @storage, auth_strategy:, folder: root_folder) + end + + def maybe_is_not_configured + return None() if @storage.configured? + + Some(ConnectionValidation.new(icon: :alert, + scheme: :warning, + description: I18n.t("storages.connection_validation.not_configured"))) + end + + def drive_id_wrong + return None() if query.result != :not_found + + Some(ConnectionValidation.new(icon: :skip, + scheme: :danger, + description: I18n.t("storages.connection_validation.drive_id_wrong"))) + end + + def tenant_id_wrong + return None() if query.result != :unauthorized + + payload = JSON.parse(query.error_payload) + return None() if payload["error"] != "invalid_request" + + tenant_id_string = "Tenant '#{@storage.tenant_id}' not found." + return None() unless payload["error_description"].include?(tenant_id_string) + + Some(ConnectionValidation.new(icon: :skip, + scheme: :danger, + description: I18n.t("storages.connection_validation.tenant_id_wrong"))) + end + + def client_id_wrong + return None() if query.result != :unauthorized + + payload = JSON.parse(query.error_payload) + return None() if payload["error"] != "unauthorized_client" + + Some(ConnectionValidation.new(icon: :skip, + scheme: :danger, + description: I18n.t("storages.connection_validation.client_id_wrong"))) + end + + def client_secret_wrong + return None() if query.result != :unauthorized + + payload = JSON.parse(query.error_payload) + return None() if payload["error"] != "invalid_client" + + Some(ConnectionValidation.new(icon: :skip, + scheme: :danger, + description: I18n.t("storages.connection_validation.client_secret_wrong"))) + end + + # rubocop:disable Metrics/AbcSize + def drive_with_unexpected_content + return None() if query.failure? + return None() unless @storage.automatic_management_enabled? + + expected_folder_ids = @storage.project_storages + .where(project_folder_mode: "automatic") + .map(&:project_folder_id) + + unexpected_files = query.result.files.reject { |file| expected_folder_ids.include?(file.id) } + return None() if unexpected_files.empty? + + Some(ConnectionValidation.new(icon: :alert, + scheme: :warning, + description: I18n.t("storages.connection_validation.unexpected_content"))) + end + + # rubocop:enable Metrics/AbcSize + + def request_failed_with_unknown_error + return None() if query.success? + + Rails.logger.error("Connection validation failed with unknown error:\n\t" \ + "status: #{query.result}\n\tresponse: #{query.error_payload}") + + Some(ConnectionValidation.new(icon: :skip, + scheme: :danger, + description: I18n.t("storages.connection_validation.unknown_error"))) + end + + def root_folder + Peripherals::ParentFolder.new("/") + end + + def auth_strategy + Peripherals::Registry.resolve("#{@storage.short_provider_type}.authentication.userless").call + end + end + end +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 28de15ce1e20..123a95a9407a 100644 --- a/modules/storages/app/controllers/storages/admin/connection_validation_controller.rb +++ b/modules/storages/app/controllers/storages/admin/connection_validation_controller.rb @@ -32,9 +32,6 @@ module Storages module Admin class ConnectionValidationController < ApplicationController include OpTurbo::ComponentStream - include Dry::Monads[:maybe] - - using Peripherals::ServiceResultRefinements layout "admin" @@ -44,128 +41,22 @@ class ConnectionValidationController < ApplicationController before_action :find_model_object, only: %i[validate_connection] - # rubocop:disable Metrics/AbcSize def validate_connection - @result = maybe_is_not_configured - .or { drive_id_wrong } - .or { tenant_id_wrong } - .or { client_id_wrong } - .or { client_secret_wrong } - .or { request_failed_with_unknown_error } - .or { drive_with_unexpected_content } - .value_or(ConnectionValidation.new(icon: "check-circle", - scheme: :success, - description: I18n.t("storages.connection_validation.success"))) + @result = Peripherals::OneDriveConnectionValidator + .new(storage: @storage) + .validate respond_to do |format| format.turbo_stream end end - # rubocop:enable Metrics/AbcSize - private - def query - @query ||= Peripherals::Registry - .resolve("#{@storage.short_provider_type}.queries.files") - .call(storage: @storage, auth_strategy:, folder: root_folder) - end - - def maybe_is_not_configured - return None() if @storage.configured? - - Some(ConnectionValidation.new(icon: :alert, - scheme: :warning, - description: I18n.t("storages.connection_validation.not_configured"))) - end - - def drive_id_wrong - return None() if query.result != :not_found - - Some(ConnectionValidation.new(icon: :skip, - scheme: :danger, - description: I18n.t("storages.connection_validation.drive_id_wrong"))) - end - - def tenant_id_wrong - return None() if query.result != :unauthorized - - payload = JSON.parse(query.error_payload) - return None() if payload["error"] != "invalid_request" - - tenant_id_string = "Tenant '#{@storage.tenant_id}' not found." - return None() unless payload["error_description"].include?(tenant_id_string) - - Some(ConnectionValidation.new(icon: :skip, - scheme: :danger, - description: I18n.t("storages.connection_validation.tenant_id_wrong"))) - end - - def client_id_wrong - return None() if query.result != :unauthorized - - payload = JSON.parse(query.error_payload) - return None() if payload["error"] != "unauthorized_client" - - Some(ConnectionValidation.new(icon: :skip, - scheme: :danger, - description: I18n.t("storages.connection_validation.client_id_wrong"))) - end - - def client_secret_wrong - return None() if query.result != :unauthorized - - payload = JSON.parse(query.error_payload) - return None() if payload["error"] != "invalid_client" - - Some(ConnectionValidation.new(icon: :skip, - scheme: :danger, - description: I18n.t("storages.connection_validation.client_secret_wrong"))) - end - - # rubocop:disable Metrics/AbcSize - def drive_with_unexpected_content - return None() if query.failure? - return None() unless @storage.automatic_management_enabled? - - expected_folder_ids = @storage.project_storages - .where(project_folder_mode: "automatic") - .map(&:project_folder_id) - - unexpected_files = query.result.files.reject { |file| expected_folder_ids.include?(file.id) } - return None() if unexpected_files.empty? - - Some(ConnectionValidation.new(icon: :alert, - scheme: :warning, - description: I18n.t("storages.connection_validation.unexpected_content"))) - end - - # rubocop:enable Metrics/AbcSize - - def request_failed_with_unknown_error - return None() if query.success? - - Rails.logger.error("Connection validation failed with unknown error:\n\t" \ - "status: #{query.result}\n\tresponse: #{query.error_payload}") - - Some(ConnectionValidation.new(icon: :skip, - scheme: :danger, - description: I18n.t("storages.connection_validation.unknown_error"))) - end - def find_model_object(object_id = :storage_id) super @storage = @object end - - def root_folder - Peripherals::ParentFolder.new("/") - end - - def auth_strategy - Peripherals::Registry.resolve("#{@storage.short_provider_type}.authentication.userless").call - end end 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 new file mode 100644 index 000000000000..46a62bbd487e --- /dev/null +++ b/modules/storages/spec/requests/storages/admin/connection_validation_spec.rb @@ -0,0 +1,57 @@ +#-- 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. +#++ + +require "spec_helper" + +RSpec.describe "connection validation", :skip_csrf do + describe "POST /admin/settings/storages/:id/connection_validation/validate_connection" do + let(:storage) { create(:one_drive_storage) } + let(:user) { create(:admin) } + let(:validator) do + double = instance_double(Storages::Peripherals::OneDriveConnectionValidator) + allow(double).to receive_messages(validate: validation_result) + double + end + let(:validation_result) do + Storages::ConnectionValidation.new(icon: "check-circle", scheme: :default, description: "Successful!") + end + + current_user { user } + + before do + allow(Storages::Peripherals::OneDriveConnectionValidator).to receive(:new).and_return(validator) + end + + it "returns a connection validation result" do + response = post validate_connection_admin_settings_storage_connection_validation_path(storage.id, format: :turbo_stream) + expect(response.status).to eq(200) + + doc = Nokogiri::HTML(response.body) + end + end +end From c2a122bfbd9ca874dbed6858c68d28c23aa6d7c7 Mon Sep 17 00:00:00 2001 From: Eric Schubert Date: Wed, 19 Jun 2024 14:11:11 +0200 Subject: [PATCH 5/9] [#55382] restructured connection validation in sidebar - have one health status section, containing both AMPF and connection validation --- .../one_drive_connection_validator.rb | 46 +++++----- .../sidebar/health_status_component.html.erb | 61 ++++++++----- .../admin/sidebar/health_status_component.rb | 88 ++++++++----------- ...b => validation_result_component.html.erb} | 47 ++++------ ...nent.rb => validation_result_component.rb} | 24 +++-- .../storages/admin/sidebar_component.html.erb | 7 +- .../models/storages/connection_validation.rb | 2 +- .../validate_connection.turbo_stream.erb | 2 +- modules/storages/config/locales/en.yml | 28 +++--- 9 files changed, 152 insertions(+), 153 deletions(-) rename modules/storages/app/components/storages/admin/sidebar/{connection_validation_component.html.erb => validation_result_component.html.erb} (54%) rename modules/storages/app/components/storages/admin/sidebar/{connection_validation_component.rb => validation_result_component.rb} (65%) 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 01a7c9e27881..86b6c6feb0d4 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,9 +47,7 @@ def validate .or { drive_id_wrong } .or { request_failed_with_unknown_error } .or { drive_with_unexpected_content } - .value_or(ConnectionValidation.new(icon: "check-circle", - scheme: :success, - description: I18n.t("storages.connection_validation.success"))) + .value_or(ConnectionValidation.new(type: :healthy, timestamp: Time.current, description: nil)) end private @@ -63,17 +61,17 @@ def query def maybe_is_not_configured return None() if @storage.configured? - Some(ConnectionValidation.new(icon: :alert, - scheme: :warning, - description: I18n.t("storages.connection_validation.not_configured"))) + Some(ConnectionValidation.new(type: :none, + timestamp: Time.current, + description: I18n.t("storages.health.connection_validation.not_configured"))) end def drive_id_wrong return None() if query.result != :not_found - Some(ConnectionValidation.new(icon: :skip, - scheme: :danger, - description: I18n.t("storages.connection_validation.drive_id_wrong"))) + Some(ConnectionValidation.new(type: :error, + timestamp: Time.current, + description: I18n.t("storages.health.connection_validation.drive_id_wrong"))) end def tenant_id_wrong @@ -85,9 +83,9 @@ def tenant_id_wrong tenant_id_string = "Tenant '#{@storage.tenant_id}' not found." return None() unless payload["error_description"].include?(tenant_id_string) - Some(ConnectionValidation.new(icon: :skip, - scheme: :danger, - description: I18n.t("storages.connection_validation.tenant_id_wrong"))) + Some(ConnectionValidation.new(type: :error, + timestamp: Time.current, + description: I18n.t("storages.health.connection_validation.tenant_id_wrong"))) end def client_id_wrong @@ -96,9 +94,9 @@ def client_id_wrong payload = JSON.parse(query.error_payload) return None() if payload["error"] != "unauthorized_client" - Some(ConnectionValidation.new(icon: :skip, - scheme: :danger, - description: I18n.t("storages.connection_validation.client_id_wrong"))) + Some(ConnectionValidation.new(type: :error, + timestamp: Time.current, + description: I18n.t("storages.health.connection_validation.client_id_wrong"))) end def client_secret_wrong @@ -107,9 +105,9 @@ def client_secret_wrong payload = JSON.parse(query.error_payload) return None() if payload["error"] != "invalid_client" - Some(ConnectionValidation.new(icon: :skip, - scheme: :danger, - description: I18n.t("storages.connection_validation.client_secret_wrong"))) + Some(ConnectionValidation.new(type: :error, + timestamp: Time.current, + description: I18n.t("storages.health.connection_validation.client_secret_wrong"))) end # rubocop:disable Metrics/AbcSize @@ -124,9 +122,9 @@ def drive_with_unexpected_content unexpected_files = query.result.files.reject { |file| expected_folder_ids.include?(file.id) } return None() if unexpected_files.empty? - Some(ConnectionValidation.new(icon: :alert, - scheme: :warning, - description: I18n.t("storages.connection_validation.unexpected_content"))) + Some(ConnectionValidation.new(type: :warning, + timestamp: Time.current, + description: I18n.t("storages.health.connection_validation.unexpected_content"))) end # rubocop:enable Metrics/AbcSize @@ -137,9 +135,9 @@ def request_failed_with_unknown_error Rails.logger.error("Connection validation failed with unknown error:\n\t" \ "status: #{query.result}\n\tresponse: #{query.error_payload}") - Some(ConnectionValidation.new(icon: :skip, - scheme: :danger, - description: I18n.t("storages.connection_validation.unknown_error"))) + Some(ConnectionValidation.new(type: :error, + timestamp: Time.current, + description: I18n.t("storages.health.connection_validation.unknown_error"))) end def root_folder 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 013a12725891..64e2b2e94f07 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 @@ -31,34 +31,55 @@ See COPYRIGHT and LICENSE files for more details. component_wrapper do flex_layout do |health_status_container| health_status_container.with_row do - flex_layout do |heading| - heading.with_row do - render(Primer::Beta::Heading.new(tag: :h4)) { I18n.t("storages.health.title") } + render(Primer::Beta::Heading.new(tag: :h4)) { I18n.t("storages.health.title") } + end + + 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 + end - heading.with_row(mt: 2) do - render(Primer::Beta::Text.new(font_weight: :bold)) { I18n.t("storages.health.subtitle") } + health_status_container.with_row(mt: 2) do + primer_form_with( + model: @storage, + url: validate_connection_admin_settings_storage_connection_validation_path(@storage), + method: :post, + data: { turbo: true } + ) do + render(Primer::Beta::Button.new( + scheme: :link, + color: :default, + font_weight: :bold, + type: :submit, + )) do |button| + button.with_leading_visual_icon(icon: "meter") + I18n.t("storages.health.connection_validation.action") + end end end end - health_status_container.with_row(mt: 2) do - flex_layout do |health_status_label| - health_status_label.with_row do - concat(render(Primer::Beta::Text.new(pr: 2, test_selector: "storage-health-checked-at")) do - I18n.t('storages.health.checked', datetime: helpers.format_time(@storage.health_checked_at)) - end) + if @storage.automatic_management_enabled? + health_status_container.with_row(mt: 2) do + render(Primer::Beta::Text.new(font_weight: :bold)) { I18n.t("storages.health.project_folders.subtitle") } + end - concat(render(Primer::Beta::Label.new(scheme: health_status_indicator[:scheme], test_selector: "storage-health-status")) do - health_status_indicator[:label] - end) - end + health_status_container.with_row(mt: 2) do + concat(render(Primer::Beta::Text.new(pr: 2, test_selector: "storage-health-checked-at")) do + I18n.t('storages.health.checked', datetime: helpers.format_time(@storage.health_checked_at)) + end) + + concat(render(Primer::Beta::Label.new(scheme: health_status_indicator[:scheme], test_selector: "storage-health-status")) do + health_status_indicator[:label] + end) + end - if @storage.health_unhealthy? - health_status_label.with_row(mt: 2) do - render(Primer::Beta::Text.new(color: :muted, test_selector: "storage-health-error")) do - formatted_health_reason - end + if @storage.health_unhealthy? + health_status_container.with_row(mt: 2) do + render(Primer::Beta::Text.new(color: :muted, test_selector: "storage-health-error")) do + formatted_health_reason end end end 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 42e05f958ca9..ff6e386ac24f 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 @@ -1,62 +1,44 @@ # frozen_string_literal: true -#-- 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. -#++ -# -module Storages::Admin - class Sidebar::HealthStatusComponent < ApplicationComponent # rubocop:disable OpenProject/AddPreviewForViewComponent - include ApplicationHelper - include OpTurbo::Streamable - include OpPrimer::ComponentHelpers +module Storages + module Admin + module Sidebar + class HealthStatusComponent < ApplicationComponent # rubocop:disable OpenProject/AddPreviewForViewComponent + include ApplicationHelper + include OpTurbo::Streamable + include OpPrimer::ComponentHelpers - def initialize(storage:) - super(storage) - @storage = storage - end + def initialize(storage:) + super(storage) + @storage = storage + end - private + private - def health_status_indicator - case @storage.health_status - when "healthy" - { scheme: :success, label: I18n.t("storages.health.label_healthy") } - when "unhealthy" - { scheme: :danger, label: I18n.t("storages.health.label_error") } - else - { scheme: :attention, label: I18n.t("storages.health.label_pending") } - end - end + def health_status_indicator + case @storage.health_status + when "healthy" + { scheme: :success, label: I18n.t("storages.health.label_healthy") } + when "unhealthy" + { scheme: :danger, label: I18n.t("storages.health.label_error") } + else + { scheme: :attention, label: I18n.t("storages.health.label_pending") } + end + end - # This method returns the health identifier, description and the time since when the error occurs in a - # formatted manner. e.g. "Not found: Outbound request destination not found since 12/07/2023 03:45 PM" - def formatted_health_reason - "#{@storage.health_reason_identifier.tr('_', ' ').strip.capitalize}: #{@storage.health_reason_description} " + - I18n.t("storages.health.since", datetime: helpers.format_time(@storage.health_changed_at)) + # This method returns the health identifier, description and the time since when the error occurs in a + # formatted manner. e.g. "Not found: Outbound request destination not found since 12/07/2023 03:45 PM" + def formatted_health_reason + "#{@storage.health_reason_identifier.tr('_', ' ').strip.capitalize}: #{@storage.health_reason_description} " + + I18n.t("storages.health.since", datetime: helpers.format_time(@storage.health_changed_at)) + end + + def validation_result_placeholder + ConnectionValidation.new(type: :none, + timestamp: Time.current, + description: I18n.t("storages.health.connection_validation.placeholder")) + end + end end end end diff --git a/modules/storages/app/components/storages/admin/sidebar/connection_validation_component.html.erb b/modules/storages/app/components/storages/admin/sidebar/validation_result_component.html.erb similarity index 54% rename from modules/storages/app/components/storages/admin/sidebar/connection_validation_component.html.erb rename to modules/storages/app/components/storages/admin/sidebar/validation_result_component.html.erb index d5e13502875f..16c3869be12f 100644 --- a/modules/storages/app/components/storages/admin/sidebar/connection_validation_component.html.erb +++ b/modules/storages/app/components/storages/admin/sidebar/validation_result_component.html.erb @@ -28,40 +28,25 @@ See COPYRIGHT and LICENSE files for more details. ++#%> <%= - component_wrapper do - flex_layout do |container| - container.with_row do - flex_layout do |heading| - heading.with_row do - render(Primer::Beta::Heading.new(tag: :h4)) { I18n.t("storages.connection_validation.title") } - end - end - end + flex_layout do |container| + container.with_row do + render(Primer::Beta::Text.new(font_weight: :bold)) { I18n.t("storages.health.connection_validation.subtitle") } + end + if @result.type != :none container.with_row(mt: 2) do - primer_form_with( - model: @storage, - url: validate_connection_admin_settings_storage_connection_validation_path(@storage), - method: :post, - data: { turbo: true } - ) do - flex_layout do |form| - form.with_row do - render(OpTurbo::FrameComponent.new(id: :connection_validation_result)) - end + status = status_indicator + + concat(render(Primer::Beta::Text.new(pr: 2)) 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 - form.with_row(mt: 2) do - render(Primer::Beta::Button.new( - scheme: :default, - block: :true, - type: :submit, - )) do |button| - button.with_leading_visual_icon(icon: "tasklist") - I18n.t("storages.connection_validation.action") - end - end - end - end + if @result.description.present? + container.with_row(mt: 2) do + render(Primer::Beta::Text.new(color: :muted)) { @result.description } end end end diff --git a/modules/storages/app/components/storages/admin/sidebar/connection_validation_component.rb b/modules/storages/app/components/storages/admin/sidebar/validation_result_component.rb similarity index 65% rename from modules/storages/app/components/storages/admin/sidebar/connection_validation_component.rb rename to modules/storages/app/components/storages/admin/sidebar/validation_result_component.rb index f8f0176d52f6..4c745fba7f58 100644 --- a/modules/storages/app/components/storages/admin/sidebar/connection_validation_component.rb +++ b/modules/storages/app/components/storages/admin/sidebar/validation_result_component.rb @@ -31,13 +31,27 @@ module Storages module Admin module Sidebar - class ConnectionValidationComponent < ApplicationComponent # rubocop:disable OpenProject/AddPreviewForViewComponent - include OpTurbo::Streamable + class ValidationResultComponent < ApplicationComponent # rubocop:disable OpenProject/AddPreviewForViewComponent include OpPrimer::ComponentHelpers - def initialize(storage:) - super(storage) - @storage = storage + def initialize(result:) + super(result) + @result = result + end + + private + + def status_indicator + case @result.type + when :healthy + { scheme: :success, label: I18n.t("storages.health.label_healthy") } + when :warning + { scheme: :attention, label: I18n.t("storages.health.label_warning") } + when :error + { scheme: :danger, label: I18n.t("storages.health.label_error") } + else + raise ArgumentError, "Unknown validation result type for status indicator: #{@result.type}" + end end end end 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 879910a31322..69515e44110e 100644 --- a/modules/storages/app/components/storages/admin/sidebar_component.html.erb +++ b/modules/storages/app/components/storages/admin/sidebar_component.html.erb @@ -1,12 +1,9 @@ <%= component_wrapper do render(Primer::OpenProject::BorderGrid.new) do |border_grid| - if @storage.provider_type_one_drive? - border_grid.with_row { render(Storages::Admin::Sidebar::ConnectionValidationComponent.new(storage: @storage)) } - end - + 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::HealthStatusComponent.new(storage: @storage)) } border_grid.with_row { render(Storages::Admin::Sidebar::HealthNotificationsComponent.new(storage: @storage)) } end end diff --git a/modules/storages/app/models/storages/connection_validation.rb b/modules/storages/app/models/storages/connection_validation.rb index 9840279040e5..6b2845089ada 100644 --- a/modules/storages/app/models/storages/connection_validation.rb +++ b/modules/storages/app/models/storages/connection_validation.rb @@ -29,5 +29,5 @@ #++ module Storages - ConnectionValidation = Data.define(:icon, :scheme, :description) + ConnectionValidation = Data.define(:type, :description, :timestamp) 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 index 8c6bf8749e2e..f42b7b4d08a7 100644 --- 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 @@ -28,5 +28,5 @@ See COPYRIGHT and LICENSE files for more details. ++#%> <%= turbo_stream.update :connection_validation_result do %> - <%= render(Primer::Alpha::Banner.new(icon: @result.icon, scheme: @result.scheme)) { @result.description } %> + <%= render(Storages::Admin::Sidebar::ValidationResultComponent.new(result: @result)) %> <% end %> diff --git a/modules/storages/config/locales/en.yml b/modules/storages/config/locales/en.yml index 8c61a1d5aeeb..cd43dcf15bf1 100644 --- a/modules/storages/config/locales/en.yml +++ b/modules/storages/config/locales/en.yml @@ -102,13 +102,26 @@ en: redirect_uri: Redirect URI storage_provider: Storage provider health: + title: Health status checked: Last checked %{datetime} label_error: Error + label_warning: Warning label_healthy: Healthy label_pending: Pending since: since %{datetime} - subtitle: Automatically managed project folders - title: Health status + project_folders: + subtitle: Automatically managed project folders + connection_validation: + subtitle: Connection validation + placeholder: Check your connection against the server. + action: Recheck connection + not_configured: The connection could not be validated. Please finish configuration first. + drive_id_wrong: The configured drive id could not be found. Please check the configuration. + tenant_id_wrong: The configured directory (tenant) id is invalid. Please check the configuration. + client_id_wrong: The configured OAuth 2 client id is invalid. Please check the configuration. + client_secret_wrong: The configured OAuth 2 client secret is invalid. Please check the configuration. + unknown_error: The connection could not be validated. An unknown error occurred. Please check the server logs for further information. + unexpected_content: Unexpected content found in the drive. health_email_notifications: description_subscribed: All administrators receive health status email notifications for this storage. description_unsubscribed: Health status email notifications for this storage have been turned off for all administrators. @@ -116,17 +129,6 @@ en: subscribe: Subscribe title: Email notifications unsubscribe: Unsubscribe - connection_validation: - title: Connection validation - action: Validate connection - not_configured: The connection could not be validated. Please finish configuration first. - drive_id_wrong: The configured drive id could not be found. Please check the configuration. - tenant_id_wrong: The configured directory (tenant) id is invalid. Please check the configuration. - client_id_wrong: The configured OAuth 2 client id is invalid. Please check the configuration. - client_secret_wrong: The configured OAuth 2 client secret is invalid. Please check the configuration. - unknown_error: The connection could not be validated. An unknown error occurred. Please check the server logs for further information. - unexpected_content: Unexpected content found in the drive. - success: The connection works as expected. help_texts: project_folder: The project folder is the default folder for file uploads for this project. Users can nevertheless still upload files to other locations. instructions: From 7559d74af130fa7c755302314cbe346226833769 Mon Sep 17 00:00:00 2001 From: Eric Schubert Date: Wed, 19 Jun 2024 15:30:46 +0200 Subject: [PATCH 6/9] [#55382] readded removed copyright --- .../admin/sidebar/health_status_component.rb | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) 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 ff6e386ac24f..53f563c199d2 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 @@ -1,5 +1,33 @@ # frozen_string_literal: true +#-- 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. +#++ + module Storages module Admin module Sidebar From a45a4c132cd22dd09f3ae4679c390df1decd0343 Mon Sep 17 00:00:00 2001 From: Eric Schubert Date: Thu, 20 Jun 2024 14:58:23 +0200 Subject: [PATCH 7/9] [#55382] added unit tests for controller and validator --- .../validation_result_component.html.erb | 10 +- .../one_drive_connection_validator_spec.rb | 164 ++++++++++++++++++ .../admin/connection_validation_spec.rb | 93 +++++++++- 3 files changed, 257 insertions(+), 10 deletions(-) create mode 100644 modules/storages/spec/common/storages/peripherals/one_drive_connection_validator_spec.rb 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 16c3869be12f..383618b5dff3 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 @@ -30,14 +30,16 @@ See COPYRIGHT and LICENSE files for more details. <%= flex_layout do |container| container.with_row do - render(Primer::Beta::Text.new(font_weight: :bold)) { I18n.t("storages.health.connection_validation.subtitle") } + render(Primer::Beta::Text.new(font_weight: :bold, test_selector: "validation-result--subtitle")) do + I18n.t("storages.health.connection_validation.subtitle") + end end if @result.type != :none container.with_row(mt: 2) do status = status_indicator - concat(render(Primer::Beta::Text.new(pr: 2)) do + 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] }) @@ -46,7 +48,9 @@ See COPYRIGHT and LICENSE files for more details. if @result.description.present? container.with_row(mt: 2) do - render(Primer::Beta::Text.new(color: :muted)) { @result.description } + render(Primer::Beta::Text.new(color: :muted, test_selector: "validation-result--description")) do + @result.description + end end end 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 new file mode 100644 index 000000000000..37744b344cef --- /dev/null +++ b/modules/storages/spec/common/storages/peripherals/one_drive_connection_validator_spec.rb @@ -0,0 +1,164 @@ +#-- 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. +#++ + +require "spec_helper" +require_module_spec_helper + +RSpec.describe Storages::Peripherals::OneDriveConnectionValidator do + let(:storage) { create(:one_drive_storage, oauth_client: create(:oauth_client)) } + + before do + Storages::Peripherals::Registry.stub("#{storage.short_provider_type}.queries.files", ->(_) { response }) + end + + subject { described_class.new(storage:).validate } + + context "if storage is not yet configured" do + let(:storage) { create(:one_drive_storage) } + + it "returns a validation failure" do + expect(subject.type).to eq(:none) + expect(subject.description).to eq("The connection could not be validated. Please finish configuration first.") + end + end + + context "if the storage's tenant id could not be found" do + let(:error_payload) do + { + error: "invalid_request", + error_description: "There is an error. Tenant '#{storage.tenant_id}' not found. This is VERY bad." + }.to_json + end + let(:response) { build_failure(code: :unauthorized, payload: error_payload) } + + it "returns a validation failure" do + expect(subject.type).to eq(:error) + expect(subject.description) + .to eq("The configured directory (tenant) id is invalid. Please check the configuration.") + end + end + + context "if the storage's client id could not be found" do + let(:error_payload) { { error: "unauthorized_client" }.to_json } + let(:response) { build_failure(code: :unauthorized, payload: error_payload) } + + it "returns a validation failure" do + expect(subject.type).to eq(:error) + expect(subject.description).to eq("The configured OAuth 2 client id is invalid. Please check the configuration.") + end + end + + context "if the storage's client secret is wrong" do + let(:error_payload) { { error: "invalid_client" }.to_json } + let(:response) { build_failure(code: :unauthorized, payload: error_payload) } + + it "returns a validation failure" do + expect(subject.type).to eq(:error) + expect(subject.description) + .to eq("The configured OAuth 2 client secret is invalid. Please check the configuration.") + end + end + + context "if the storage's drive id could not be found" do + let(:response) { build_failure(code: :not_found, payload: nil) } + + it "returns a validation failure" do + expect(subject.type).to eq(:error) + expect(subject.description).to eq("The configured drive id could not be found. Please check the configuration.") + end + end + + context "if the request fails with an unknown error" do + let(:response) { build_failure(code: :error, payload: nil) } + + before do + allow(Rails.logger).to receive(:error) + end + + it "returns a validation failure" do + expect(subject.type).to eq(:error) + expect(subject.description) + .to eq("The connection could not be validated. An unknown error occurred. " \ + "Please check the server logs for further information.") + end + + it "logs the error message" do + described_class.new(storage:).validate + expect(Rails.logger).to have_received(:error) + end + end + + context "if the request returns unexpected files" do + let(:storage) { create(:one_drive_storage, :as_automatically_managed, oauth_client: create(:oauth_client)) } + let(:project_folder_id) { "1337" } + let(:project_storage) do + create(:project_storage, + :as_automatically_managed, + project_folder_id:, + storage:, + project: create(:project)) + end + let(:files_result) do + Storages::StorageFiles.new( + [ + Storages::StorageFile.new(id: project_folder_id, name: "I am your father"), + Storages::StorageFile.new(id: "noooooooooo", name: "testimony_of_luke_skywalker.md") + ], + Storages::StorageFile.new(id: "root", name: "root"), + [] + ) + end + let(:response) { ServiceResult.success(result: files_result) } + + before do + project_storage + end + + it "returns a validation failure" do + expect(subject.type).to eq(:warning) + expect(subject.description).to eq("Unexpected content found in the drive.") + end + end + + context "if everything was fine" do + let(:response) { ServiceResult.success } + + it "returns a validation success" do + expect(subject.type).to eq(:healthy) + expect(subject.description).to be_nil + end + end + + private + + def build_failure(code:, payload:) + data = Storages::StorageErrorData.new(source: "query", payload:) + error = Storages::StorageError.new(code:, data:) + ServiceResult.failure(result: code, errors: error) + 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 46a62bbd487e..10fc607eb805 100644 --- a/modules/storages/spec/requests/storages/admin/connection_validation_spec.rb +++ b/modules/storages/spec/requests/storages/admin/connection_validation_spec.rb @@ -37,9 +37,6 @@ allow(double).to receive_messages(validate: validation_result) double end - let(:validation_result) do - Storages::ConnectionValidation.new(icon: "check-circle", scheme: :default, description: "Successful!") - end current_user { user } @@ -47,11 +44,93 @@ allow(Storages::Peripherals::OneDriveConnectionValidator).to receive(:new).and_return(validator) end - it "returns a connection validation result" do - response = post validate_connection_admin_settings_storage_connection_validation_path(storage.id, format: :turbo_stream) - expect(response.status).to eq(200) + subject do + post validate_connection_admin_settings_storage_connection_validation_path(storage.id, format: :turbo_stream) + end + + shared_examples_for "a validation result template" do |show_timestamp:, label:, description:| + it "returns a turbo update template" do + expect(subject.status).to eq(200) + + doc = Nokogiri::HTML(subject.body) + expect(doc.xpath(xpath_for_subtitle).text).to eq("Connection validation") + + if show_timestamp + expect(doc.xpath(xpath_for_timestamp)).not_to be_empty + else + expect(doc.xpath(xpath_for_timestamp)).to be_empty + end + + if label.present? + expect(doc.xpath(xpath_for_label).text).to eq(label) + else + expect(doc.xpath(xpath_for_label).text).to be_empty + end + + if description.present? + expect(doc.xpath(xpath_for_description).text).to eq(description) + else + expect(doc.xpath(xpath_for_description).text).to be_empty + end + end + end + + 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") + end + + it_behaves_like "a validation result template", show_timestamp: false, label: nil, description: "not configured" + end + + context "if validator returns an error" do + let(:validation_result) do + Storages::ConnectionValidation.new(type: :error, timestamp: Time.current, description: "An error occurred") + end + + it_behaves_like "a validation result template", + show_timestamp: true, label: "Error", description: "An error occurred" + end + + context "if validator returns a warning" do + let(:validation_result) do + Storages::ConnectionValidation.new(type: :warning, + 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..." + end + + context "if validator returns a success" do + let(:validation_result) do + Storages::ConnectionValidation.new(type: :healthy, timestamp: Time.current, description: nil) + end - doc = Nokogiri::HTML(response.body) + it_behaves_like "a validation result template", + show_timestamp: true, label: "Healthy", description: nil end end + + 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']" + end + + def xpath_for_timestamp + "//turbo-stream[@target='connection_validation_result']/template/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')]" + end + + def xpath_for_description + "//turbo-stream[@target='connection_validation_result']/template/div/div/span[@data-test-selector='validation-result--description']" + end + + # rubocop:enable Layout/LineLength end From 633ed0ea7cdffb1f03440c48ff73a118da53d8d5 Mon Sep 17 00:00:00 2001 From: Eric Schubert Date: Thu, 20 Jun 2024 15:41:05 +0200 Subject: [PATCH 8/9] [#55382] normalize i18n files --- modules/storages/config/locales/en.yml | 28 +++++++++++++------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/modules/storages/config/locales/en.yml b/modules/storages/config/locales/en.yml index cd43dcf15bf1..a127c89109ec 100644 --- a/modules/storages/config/locales/en.yml +++ b/modules/storages/config/locales/en.yml @@ -102,26 +102,26 @@ en: redirect_uri: Redirect URI storage_provider: Storage provider health: - title: Health status checked: Last checked %{datetime} - label_error: Error - label_warning: Warning - label_healthy: Healthy - label_pending: Pending - since: since %{datetime} - project_folders: - subtitle: Automatically managed project folders connection_validation: - subtitle: Connection validation - placeholder: Check your connection against the server. action: Recheck connection - not_configured: The connection could not be validated. Please finish configuration first. - drive_id_wrong: The configured drive id could not be found. Please check the configuration. - tenant_id_wrong: The configured directory (tenant) id is invalid. Please check the configuration. client_id_wrong: The configured OAuth 2 client id is invalid. Please check the configuration. client_secret_wrong: The configured OAuth 2 client secret is invalid. Please check the configuration. - unknown_error: The connection could not be validated. An unknown error occurred. Please check the server logs for further information. + drive_id_wrong: The configured drive id could not be found. Please check the configuration. + not_configured: The connection could not be validated. Please finish configuration first. + placeholder: Check your connection against the server. + subtitle: Connection validation + tenant_id_wrong: The configured directory (tenant) id is invalid. Please check the configuration. unexpected_content: Unexpected content found in the drive. + unknown_error: The connection could not be validated. An unknown error occurred. Please check the server logs for further information. + label_error: Error + label_healthy: Healthy + label_pending: Pending + label_warning: Warning + project_folders: + subtitle: Automatically managed project folders + since: since %{datetime} + title: Health status health_email_notifications: description_subscribed: All administrators receive health status email notifications for this storage. description_unsubscribed: Health status email notifications for this storage have been turned off for all administrators. From aa732f7930828c14f00245bf95720f2e84830fe1 Mon Sep 17 00:00:00 2001 From: Eric Schubert Date: Tue, 25 Jun 2024 15:40:58 +0200 Subject: [PATCH 9/9] [#55382] adapt PR suggestions - use helper methods from OpTurbo::Streamable - add error codes to validation result --- .../one_drive_connection_validator.rb | 12 +++++- .../sidebar/health_status_component.html.erb | 4 +- .../admin/sidebar/health_status_component.rb | 1 + .../validation_result_component.html.erb | 38 ++++++++++--------- .../sidebar/validation_result_component.rb | 1 + .../storages/admin/sidebar_component.html.erb | 2 +- .../admin/connection_validation_controller.rb | 6 +-- .../models/storages/connection_validation.rb | 10 ++++- .../validate_connection.turbo_stream.erb | 32 ---------------- .../one_drive_connection_validator_spec.rb | 8 ++++ .../admin/connection_validation_spec.rb | 28 ++++++++------ 11 files changed, 72 insertions(+), 70 deletions(-) delete mode 100644 modules/storages/app/views/storages/admin/connection_validation/validate_connection.turbo_stream.erb 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