[#55382] adapt PR suggestions

- use helper methods from OpTurbo::Streamable
- add error codes to validation result

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

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

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

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

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)

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

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

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

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

modules/storages/spec/requests/storages/admin/connection_validation_spec.rb

@@ -77,35 +77,42 @@
 
     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"
     end
 
     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