From fcb8114bba48cdf39a71170d1ceb18d0ae25961c Mon Sep 17 00:00:00 2001
From: Kabiru Mwenja <makabby@gmail.com>
Date: Mon, 27 Nov 2023 14:54:50 +0300
Subject: [PATCH 1/9] A11y[Op#51022] Replace New Storage "magic form" with more
 accessible ActionMenu

https://community.openproject.org/work_packages/51022
---
 .../general_info_form_component.html.erb      | 13 +++---
 .../forms/general_info_form_component.rb      |  4 +-
 .../new_storage_button_component.html.erb     | 16 +++++--
 .../admin/new_storage_button_component.rb     | 10 ++--
 .../admin/storage_list_component.html.erb     |  5 --
 .../storages/admin/storages_controller.rb     | 18 ++++++--
 .../admin/provider_type_hidden_input_form.rb  | 46 +++++++++++++++++++
 .../storages/app/models/storages/storage.rb   |  5 ++
 .../storages/admin/storages/new.html.erb      | 26 ++++++-----
 modules/storages/config/locales/en.yml        |  3 +-
 modules/storages/config/routes.rb             |  2 +-
 11 files changed, 107 insertions(+), 41 deletions(-)
 create mode 100644 modules/storages/app/forms/storages/admin/provider_type_hidden_input_form.rb

diff --git a/modules/storages/app/components/storages/admin/forms/general_info_form_component.html.erb b/modules/storages/app/components/storages/admin/forms/general_info_form_component.html.erb
index eb635a067dc9..46304fe15e69 100644
--- a/modules/storages/app/components/storages/admin/forms/general_info_form_component.html.erb
+++ b/modules/storages/app/components/storages/admin/forms/general_info_form_component.html.erb
@@ -7,13 +7,12 @@
     ) do |form|
       flex_layout do |general_info_row|
         general_info_row.with_row(mb: 3) do
-          render(
-            Storages::Admin::ProviderTypeSelectForm.new(
-              form,
-              storage:,
-              select_list_options: { caption: provider_type_select_caption, readonly: storage.new_record? }
-            )
-          )
+          concat(render(Primer::Beta::Text.new(font_weight: :bold)) { I18n.t("storages.label_file_storage") })
+          concat(render(Storages::Admin::ProviderTypeHiddenInputForm.new(form, storage:)))
+        end
+
+        general_info_row.with_row(mb: 3) do
+          render(Primer::Beta::Text.new(test_selector: 'storage-provider-configuration-instructions')) { provider_configuration_instructions }
         end
 
         general_info_row.with_row(mb: 3) do
diff --git a/modules/storages/app/components/storages/admin/forms/general_info_form_component.rb b/modules/storages/app/components/storages/admin/forms/general_info_form_component.rb
index a979d8779a18..1d48a6822462 100644
--- a/modules/storages/app/components/storages/admin/forms/general_info_form_component.rb
+++ b/modules/storages/app/components/storages/admin/forms/general_info_form_component.rb
@@ -75,9 +75,7 @@ def cancel_button_path
       end
     end
 
-    def provider_type_select_caption
-      return if storage.provider_type.blank?
-
+    def provider_configuration_instructions
       caption_for_provider_type(storage.short_provider_type)
     end
 
diff --git a/modules/storages/app/components/storages/admin/new_storage_button_component.html.erb b/modules/storages/app/components/storages/admin/new_storage_button_component.html.erb
index 1b07c83f8034..4d5d61567acf 100644
--- a/modules/storages/app/components/storages/admin/new_storage_button_component.html.erb
+++ b/modules/storages/app/components/storages/admin/new_storage_button_component.html.erb
@@ -1,6 +1,16 @@
 <%=
-  render(Primer::Beta::Button.new(**button_options)) do |button|
-    button.with_leading_visual_icon(icon: :plus)
-    label
+  render(Primer::Alpha::ActionMenu.new) do |menu|
+    menu.with_show_button(**show_button_options) do |button|
+      button.with_leading_visual_icon(icon: :plus)
+      label
+    end
+
+    ::Storages::Storage::PROVIDER_TYPES.each do |provider_type|
+      short_provider_type = ::Storages::Storage.shorten_provider_type(provider_type)
+      menu.with_item(
+        label: I18n.t("storages.provider_types.#{short_provider_type}.name"),
+        href: select_provider_admin_settings_storages_path(provider: short_provider_type)
+      )
+    end
   end
 %>
diff --git a/modules/storages/app/components/storages/admin/new_storage_button_component.rb b/modules/storages/app/components/storages/admin/new_storage_button_component.rb
index 5de250128fcb..d0f1afff7a6b 100644
--- a/modules/storages/app/components/storages/admin/new_storage_button_component.rb
+++ b/modules/storages/app/components/storages/admin/new_storage_button_component.rb
@@ -29,17 +29,13 @@
 #++
 #
 module Storages::Admin
-  class NewStorageButtonComponent < ApplicationComponent
+  class NewStorageButtonComponent < ApplicationComponent # rubocop:disable OpenProject/AddPreviewForViewComponent
     options scheme: :primary,
-            size: :medium,
-            tag: :a
+            size: :medium
 
-    def button_options
+    def show_button_options
       { scheme:,
         size:,
-        tag:,
-        role: :button,
-        href: Rails.application.routes.url_helpers.new_admin_settings_storage_path,
         aria: { label: I18n.t("storages.label_add_new_storage") } }
     end
 
diff --git a/modules/storages/app/components/storages/admin/storage_list_component.html.erb b/modules/storages/app/components/storages/admin/storage_list_component.html.erb
index df473eea3412..3f0047245f69 100644
--- a/modules/storages/app/components/storages/admin/storage_list_component.html.erb
+++ b/modules/storages/app/components/storages/admin/storage_list_component.html.erb
@@ -60,11 +60,6 @@
       component.with_visual_icon(icon: :cloud)
       component.with_heading(tag: :h2).with_content(I18n.t('storages.storage_list_blank_slate.heading'))
       component.with_description { I18n.t('storages.storage_list_blank_slate.description') }
-
-      component.with_primary_action(**Storages::Admin::NewStorageButtonComponent.new.button_options)  do |button|
-        button.with_leading_visual_icon(icon: :plus)
-        I18n.t("storages.label_storage")
-      end
     end
   end
 %>
diff --git a/modules/storages/app/controllers/storages/admin/storages_controller.rb b/modules/storages/app/controllers/storages/admin/storages_controller.rb
index e5b4055f3fca..e5e0a1e443b6 100644
--- a/modules/storages/app/controllers/storages/admin/storages_controller.rb
+++ b/modules/storages/app/controllers/storages/admin/storages_controller.rb
@@ -43,6 +43,7 @@ class Storages::Admin::StoragesController < ApplicationController
   before_action :require_admin
   before_action :find_model_object,
                 only: %i[show show_oauth_application destroy edit edit_host confirm_destroy update replace_oauth_application]
+  before_action :ensure_valid_provider_type_selected, only: %i[select_provider]
 
   # menu_item is defined in the Redmine::MenuManager::MenuController
   # module, included from ApplicationController.
@@ -79,7 +80,7 @@ def new
   end
 
   def select_provider
-    @object = Storages::Storage.new(permitted_storage_params(:storages_storage))
+    @object = Storages::Storage.new(provider_type: @provider_type)
     service_result = ::Storages::Storages::SetAttributesService
                  .new(user: current_user,
                       model: @object,
@@ -91,11 +92,14 @@ def select_provider
     service_result.on_failure { render :new }
 
     service_result.on_success do
-      respond_to { |format| format.turbo_stream }
+      respond_to do |format|
+        format.html { render :new }
+        format.turbo_stream
+      end
     end
   end
 
-  def create # rubocop:disable Metrics/AbcSize
+  def create
     service_result = Storages::Storages::CreateService
                       .new(user: current_user)
                       .call(permitted_storage_params)
@@ -203,6 +207,14 @@ def show_local_breadcrumb
 
   private
 
+  def ensure_valid_provider_type_selected
+    short_provider_type = params[:provider]
+    if short_provider_type.blank? || (@provider_type = ::Storages::Storage::PROVIDER_TYPE_SHORT_NAMES[short_provider_type]).blank?
+      flash[:error] = I18n.t('storages.error_invalid_provider_type')
+      render :index
+    end
+  end
+
   def oauth_application(service_result)
     service_result.dependent_results&.first&.result
   end
diff --git a/modules/storages/app/forms/storages/admin/provider_type_hidden_input_form.rb b/modules/storages/app/forms/storages/admin/provider_type_hidden_input_form.rb
new file mode 100644
index 000000000000..1a03e7c7ccda
--- /dev/null
+++ b/modules/storages/app/forms/storages/admin/provider_type_hidden_input_form.rb
@@ -0,0 +1,46 @@
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2023 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 ProviderTypeHiddenInputForm < ApplicationForm
+    form do |storage_form|
+      storage_form.text_field(
+        name: :provider_type,
+        label: I18n.t('activerecord.attributes.storages/storage.provider_type'),
+        hidden: true,
+        required: true,
+        value: @storage.provider_type
+      )
+    end
+
+    def initialize(storage:)
+      super()
+      @storage = storage
+    end
+  end
+end
diff --git a/modules/storages/app/models/storages/storage.rb b/modules/storages/app/models/storages/storage.rb
index 90b8c7790333..3eb0e61ab32e 100644
--- a/modules/storages/app/models/storages/storage.rb
+++ b/modules/storages/app/models/storages/storage.rb
@@ -45,6 +45,11 @@ class Storage < ApplicationRecord
       PROVIDER_TYPE_ONE_DRIVE = 'Storages::OneDriveStorage'
     ].freeze
 
+    PROVIDER_TYPE_SHORT_NAMES = {
+      nextcloud: PROVIDER_TYPE_NEXTCLOUD,
+      one_drive: PROVIDER_TYPE_ONE_DRIVE
+    }.with_indifferent_access.freeze
+
     self.inheritance_column = :provider_type
 
     has_many :file_links, class_name: 'Storages::FileLink'
diff --git a/modules/storages/app/views/storages/admin/storages/new.html.erb b/modules/storages/app/views/storages/admin/storages/new.html.erb
index a99699aad2b7..e75bc530eeda 100644
--- a/modules/storages/app/views/storages/admin/storages/new.html.erb
+++ b/modules/storages/app/views/storages/admin/storages/new.html.erb
@@ -1,9 +1,15 @@
-<% html_title t(:label_administration), t("project_module_storages"), t('storages.label_new_file_storage') %>
-<% local_assigns[:additional_breadcrumb] = t('storages.label_new_file_storage') %>
+<!-- Standard Ruby view, please see the controller for comments -->
+<%
+  label_new_file_storage = t("storages.label_new_file_storage",
+                              provider: t("storages.provider_types.#{@storage.short_provider_type}.name"))
+%>
+
+<% html_title t(:label_administration), t("project_module_storages"), label_new_file_storage %>
+<% local_assigns[:additional_breadcrumb] = label_new_file_storage %>
 
 <%= render(Primer::OpenProject::PageHeader.new) do |header| %>
   <% header.with_title(test_selector: 'storage-name-title') do %>
-    <%= t("storages.label_new_file_storage") %>
+    <%= label_new_file_storage %>
   <% end %>
 
   <% header.with_back_button(href: admin_settings_storages_path, 'aria-label':  I18n.t("button_back")) %>
@@ -14,14 +20,12 @@
 
   <% header.with_description do %>
     <%=
-    t("storages.instructions.new_storage",
-      new_storage_link_text: render(
-        Primer::Beta::Link.new(href: ::OpenProject::Static::Links[:storage_docs][:setup][:href], target: '_blank')
-      ) { t("storages.instructions.new_storage_link_text") }
-    ).html_safe
+      t("storages.instructions.new_storage",
+        new_storage_link_text: render(
+          Primer::Beta::Link.new(href: ::OpenProject::Static::Links[:storage_docs][:setup][:href], target: '_blank')
+        ) { t("storages.instructions.new_storage_link_text") }
+      ).html_safe
     %>
-  <% end %>
-
 <% end %>
 
-<%= render(::Storages::Admin::SelectStorageProviderComponent.new(@storage)) %>
+<%= render(Storages::Admin::StorageViewComponent.new(@storage)) %>
diff --git a/modules/storages/config/locales/en.yml b/modules/storages/config/locales/en.yml
index ba82ace54899..4f88f2d97e2d 100644
--- a/modules/storages/config/locales/en.yml
+++ b/modules/storages/config/locales/en.yml
@@ -181,6 +181,7 @@ en:
     storage_list_blank_slate:
       heading: "You don't have any storages yet."
       description: "Add a storage to see them here."
+    error_invalid_provider_type: "Please select a valid provider type."
     label_active: "Active"
     label_add_new_storage: "Add new storage"
     label_delete_storage: "Delete storage"
@@ -214,7 +215,7 @@ en:
     label_project_folder: "Project folder"
     label_redirect_uri: "Redirect URI"
     label_new_storage: "New storage"
-    label_new_file_storage: "New file storage"
+    label_new_file_storage: "New %{provider} storage"
     label_edit_storage: "Edit storage"
     label_edit_storage_host: "Edit storage host"
     label_edit_storage_oauth_client: "Edit storage OAuth client"
diff --git a/modules/storages/config/routes.rb b/modules/storages/config/routes.rb
index 115b94874663..8e6e0dae7bd7 100644
--- a/modules/storages/config/routes.rb
+++ b/modules/storages/config/routes.rb
@@ -39,7 +39,7 @@
         resource :automatically_managed_project_folders, controller: '/storages/admin/automatically_managed_project_folders',
                                                          only: %i[new create edit update]
 
-        post :select_provider, on: :collection
+        get :select_provider, on: :collection
 
         member do
           get :show_oauth_application

From d7fb626e6432ff3e5d3934fbb9c10445f273d518 Mon Sep 17 00:00:00 2001
From: Kabiru Mwenja <makabby@gmail.com>
Date: Mon, 27 Nov 2023 15:00:13 +0300
Subject: [PATCH 2/9] A11y[Op#51022]: Cleanup unused files

---
 .../select-provider-form.controller.ts        | 58 ----------------
 ...select_storage_provider_component.html.erb | 66 ------------------
 .../select_storage_provider_component.rb      | 36 ----------
 .../storages/admin/storages_controller.rb     |  1 -
 .../admin/provider_type_select_form.rb        | 69 -------------------
 .../storages/select_provider.turbo_stream.erb | 34 ---------
 6 files changed, 264 deletions(-)
 delete mode 100644 frontend/src/stimulus/controllers/dynamic/storages/select-provider-form.controller.ts
 delete mode 100644 modules/storages/app/components/storages/admin/select_storage_provider_component.html.erb
 delete mode 100644 modules/storages/app/components/storages/admin/select_storage_provider_component.rb
 delete mode 100644 modules/storages/app/forms/storages/admin/provider_type_select_form.rb
 delete mode 100644 modules/storages/app/views/storages/admin/storages/select_provider.turbo_stream.erb

diff --git a/frontend/src/stimulus/controllers/dynamic/storages/select-provider-form.controller.ts b/frontend/src/stimulus/controllers/dynamic/storages/select-provider-form.controller.ts
deleted file mode 100644
index 83447ea62a84..000000000000
--- a/frontend/src/stimulus/controllers/dynamic/storages/select-provider-form.controller.ts
+++ /dev/null
@@ -1,58 +0,0 @@
-/*
- * -- copyright
- * OpenProject is an open source project management software.
- * Copyright (C) 2023 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.
- * ++
- */
-
-import { Controller } from '@hotwired/stimulus';
-
-export default class SelectProviderFormController extends Controller {
-  static targets = [
-    'providerForm',
-    'providerSelect',
-  ];
-
-  declare readonly providerFormTarget:HTMLFormElement;
-  declare readonly providerSelectTarget:HTMLSelectElement;
-
-  connect():void {
-    // On first load if providerTypeValue is already selected, show the provider form
-    this.fetchProviderTypeForm(this.providerSelectTarget.value);
-  }
-
-  public showProviderForm(evt:Event):void {
-    this.fetchProviderTypeForm((evt.target as HTMLSelectElement).value);
-  }
-
-  fetchProviderTypeForm(providerTypeValue:string):void {
-    if (providerTypeValue === '') {
-      return;
-    }
-
-    this.providerFormTarget.requestSubmit();
-  }
-}
diff --git a/modules/storages/app/components/storages/admin/select_storage_provider_component.html.erb b/modules/storages/app/components/storages/admin/select_storage_provider_component.html.erb
deleted file mode 100644
index ebe760e5d2cb..000000000000
--- a/modules/storages/app/components/storages/admin/select_storage_provider_component.html.erb
+++ /dev/null
@@ -1,66 +0,0 @@
-<%=
-  render(OpTurbo::FrameComponent.new(id: :select_storage_provider)) do
-    render(Primer::Beta::BorderBox.new) do |component|
-      component.with_header(color: :default) do
-        render(Primer::Beta::Text.new(font_weight: :semibold)) { I18n.t('storages.file_storage_view.general_information') }
-      end
-
-      component.with_row(scheme: :default) do
-        render(
-          Primer::Beta::Text.new(
-            tag: :div,
-            test_selector: 'storage-select-provider-form',
-            data: {
-              controller: "storages--select-provider-form",
-              application_target: "dynamic"
-            }
-          )
-        ) do
-          primer_form_with(
-            model: storage,
-            url: select_provider_admin_settings_storages_path,
-            data: {
-              storages__select_provider_form_target: 'providerForm',
-            }
-          ) do |form|
-            flex_layout do |general_info_row|
-              general_info_row.with_row(mb: 3) do
-                render(Storages::Admin::ProviderTypeSelectForm.new(
-                  form,
-                  storage:,
-                  select_list_options: {
-                    data: {
-                      action: 'change->storages--select-provider-form#showProviderForm',
-                      storages__select_provider_form_target: 'providerSelect'
-                    },
-                    include_blank: I18n.t('storages.label_select_provider')
-                  }
-                )
-
-                )
-              end
-
-              general_info_row.with_row do
-                render(
-                  Storages::Admin::SubmitOrCancelForm.new(
-                    form,
-                    storage:,
-                    submit_button_options: {
-                      disabled: true,
-                      test_selector: 'storage-select-provider-submit-button'
-                    },
-                    cancel_button_options: {
-                      href: admin_settings_storages_path,
-                      test_selector: 'storage-select-provider-cancel-button',
-                      target: '_top'
-                    }
-                  )
-                )
-              end
-            end
-          end
-        end
-      end
-    end
-  end
-%>
diff --git a/modules/storages/app/components/storages/admin/select_storage_provider_component.rb b/modules/storages/app/components/storages/admin/select_storage_provider_component.rb
deleted file mode 100644
index 3061a77acc67..000000000000
--- a/modules/storages/app/components/storages/admin/select_storage_provider_component.rb
+++ /dev/null
@@ -1,36 +0,0 @@
-# frozen_string_literal: true
-
-#-- copyright
-# OpenProject is an open source project management software.
-# Copyright (C) 2012-2023 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 SelectStorageProviderComponent < ApplicationComponent
-    include OpPrimer::ComponentHelpers
-    alias_method :storage, :model
-  end
-end
diff --git a/modules/storages/app/controllers/storages/admin/storages_controller.rb b/modules/storages/app/controllers/storages/admin/storages_controller.rb
index e5e0a1e443b6..c248bfa5cc60 100644
--- a/modules/storages/app/controllers/storages/admin/storages_controller.rb
+++ b/modules/storages/app/controllers/storages/admin/storages_controller.rb
@@ -94,7 +94,6 @@ def select_provider
     service_result.on_success do
       respond_to do |format|
         format.html { render :new }
-        format.turbo_stream
       end
     end
   end
diff --git a/modules/storages/app/forms/storages/admin/provider_type_select_form.rb b/modules/storages/app/forms/storages/admin/provider_type_select_form.rb
deleted file mode 100644
index 69a3749cc47e..000000000000
--- a/modules/storages/app/forms/storages/admin/provider_type_select_form.rb
+++ /dev/null
@@ -1,69 +0,0 @@
-#-- copyright
-# OpenProject is an open source project management software.
-# Copyright (C) 2012-2023 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 ProviderTypeSelectForm < ApplicationForm
-    form do |storage_form|
-      storage_form.select_list(**@select_list_options) do |storage_provider_list|
-        if @storage.provider_type.present?
-          storage_provider_list.option(
-            label: I18n.t("storages.provider_types.#{@storage.short_provider_type}.name"),
-            value: @storage.provider_type
-          )
-        else
-          ::Storages::Storage::PROVIDER_TYPES.each do |provider_type|
-            storage_provider_list.option(
-              label: I18n.t("storages.provider_types.#{::Storages::Storage.shorten_provider_type(provider_type)}.name"),
-              value: provider_type
-            )
-          end
-        end
-      end
-    end
-
-    def initialize(storage:, select_list_options: {})
-      super()
-      @storage = storage
-      @select_list_options = default_select_list_options(storage).merge(select_list_options)
-    end
-
-    private
-
-    def default_select_list_options(storage)
-      {
-        name: :provider_type,
-        label: I18n.t('activerecord.attributes.storages/storage.provider_type'),
-        caption: nil,
-        include_blank: false,
-        required: true,
-        disabled: storage.persisted?,
-        input_width: :small
-      }
-    end
-  end
-end
diff --git a/modules/storages/app/views/storages/admin/storages/select_provider.turbo_stream.erb b/modules/storages/app/views/storages/admin/storages/select_provider.turbo_stream.erb
deleted file mode 100644
index c55a50808861..000000000000
--- a/modules/storages/app/views/storages/admin/storages/select_provider.turbo_stream.erb
+++ /dev/null
@@ -1,34 +0,0 @@
-<%#-- copyright
-OpenProject is an open source project management software.
-Copyright (C) 2012-2023 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 :select_storage_provider do %>
-  <%=
-    render(Storages::Admin::StorageViewComponent.new(@storage))
-  %>
-<% end %>

From 42b23b2875cc96a775efd7a93d1dd841f56b8a09 Mon Sep 17 00:00:00 2001
From: Kabiru Mwenja <makabby@gmail.com>
Date: Mon, 27 Nov 2023 15:15:07 +0300
Subject: [PATCH 3/9] docs[Op#51022]: Render setup documentation by provider
 type

Related to: https://github.com/opf/openproject/pull/14140
---
 lib/open_project/static/links.rb                            | 6 ++++++
 .../storages/app/views/storages/admin/storages/new.html.erb | 6 +++---
 modules/storages/config/locales/en.yml                      | 4 ++--
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/lib/open_project/static/links.rb b/lib/open_project/static/links.rb
index 126c12fe79c6..da3d8a904ed8 100644
--- a/lib/open_project/static/links.rb
+++ b/lib/open_project/static/links.rb
@@ -257,8 +257,14 @@ def static_links
             },
             storage_docs: {
               setup: {
+                href: 'https://www.openproject.org/docs/system-admin-guide/integrations/storage/'
+              },
+              nextcloud_setup: {
                 href: 'https://www.openproject.org/docs/system-admin-guide/integrations/nextcloud/'
               },
+              one_drive_setup: {
+                href: 'https://www.openproject.org/docs/system-admin-guide/integrations/one-drive/'
+              },
               nextcloud_oauth_application: {
                 href: 'https://apps.nextcloud.com/apps/integration_openproject'
               },
diff --git a/modules/storages/app/views/storages/admin/storages/new.html.erb b/modules/storages/app/views/storages/admin/storages/new.html.erb
index e75bc530eeda..0ec295bc478f 100644
--- a/modules/storages/app/views/storages/admin/storages/new.html.erb
+++ b/modules/storages/app/views/storages/admin/storages/new.html.erb
@@ -21,9 +21,9 @@
   <% header.with_description do %>
     <%=
       t("storages.instructions.new_storage",
-        new_storage_link_text: render(
-          Primer::Beta::Link.new(href: ::OpenProject::Static::Links[:storage_docs][:setup][:href], target: '_blank')
-        ) { t("storages.instructions.new_storage_link_text") }
+        file_storage_docs_link: render(
+          Primer::Beta::Link.new(href: ::OpenProject::Static::Links[:storage_docs][:"#{@storage.short_provider_type}_setup"][:href], target: '_blank')
+        ) { t("storages.instructions.file_storage_docs_link", provider_name: I18n.t("storages.provider_types.#{@storage.short_provider_type}.name")) }
       ).html_safe
     %>
 <% end %>
diff --git a/modules/storages/config/locales/en.yml b/modules/storages/config/locales/en.yml
index 4f88f2d97e2d..171979bb5344 100644
--- a/modules/storages/config/locales/en.yml
+++ b/modules/storages/config/locales/en.yml
@@ -118,8 +118,8 @@ en:
       managed_project_folders_application_password: >
         Copy this value from:
       managed_project_folders_application_password_caption: "Enable automatic managed folders by copying this value from: %{provider_type_link}."
-      new_storage: "Read our %{new_storage_link_text} for more information about this setup."
-      new_storage_link_text: "file storage documentation"
+      new_storage: "Read our documentation on %{file_storage_docs_link} integration for more information."
+      file_storage_docs_link: "setting up a %{provider_name} file storage"
       no_storage_set_up: "There are no file storages set up yet."
       no_specific_folder: "By default, each user will start at their own home folder when they upload a file."
       automatic_folder: "This will automatically create a root folder for this project and manage the access permissions for each project member."

From 43b9b889ea80960cb622adee0ea86ee6e7da783e Mon Sep 17 00:00:00 2001
From: Kabiru Mwenja <makabby@gmail.com>
Date: Mon, 27 Nov 2023 17:34:34 +0300
Subject: [PATCH 4/9] test[Op#51022]: Update admin storages feature specs

---
 .../new_storage_button_component.html.erb     |  2 +-
 .../storages/admin/storages_controller.rb     |  4 +-
 .../storages/admin/storages/edit.html.erb     |  2 +-
 .../storages/admin/storages/new.html.erb      |  4 +-
 .../admin/storages/new.turbo_stream.erb       |  4 +-
 .../spec/features/admin_storages_spec.rb      | 49 ++++++++++++-------
 6 files changed, 38 insertions(+), 27 deletions(-)

diff --git a/modules/storages/app/components/storages/admin/new_storage_button_component.html.erb b/modules/storages/app/components/storages/admin/new_storage_button_component.html.erb
index 4d5d61567acf..e37e9caa770d 100644
--- a/modules/storages/app/components/storages/admin/new_storage_button_component.html.erb
+++ b/modules/storages/app/components/storages/admin/new_storage_button_component.html.erb
@@ -1,5 +1,5 @@
 <%=
-  render(Primer::Alpha::ActionMenu.new) do |menu|
+  render(Primer::Alpha::ActionMenu.new(test_selector: 'storages-select-provider-action-menu')) do |menu|
     menu.with_show_button(**show_button_options) do |button|
       button.with_leading_visual_icon(icon: :plus)
       label
diff --git a/modules/storages/app/controllers/storages/admin/storages_controller.rb b/modules/storages/app/controllers/storages/admin/storages_controller.rb
index c248bfa5cc60..7018e180e317 100644
--- a/modules/storages/app/controllers/storages/admin/storages_controller.rb
+++ b/modules/storages/app/controllers/storages/admin/storages_controller.rb
@@ -107,9 +107,7 @@ def create
     @oauth_application = oauth_application(service_result)
 
     service_result.on_failure do
-      respond_to do |format|
-        format.turbo_stream { render :select_provider }
-      end
+      render :new
     end
 
     service_result.on_success do
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 b06bc99be6e3..c02a6204ec2f 100644
--- a/modules/storages/app/views/storages/admin/storages/edit.html.erb
+++ b/modules/storages/app/views/storages/admin/storages/edit.html.erb
@@ -31,7 +31,7 @@ See COPYRIGHT and LICENSE files for more details.
 <% local_assigns[:additional_breadcrumb] = @storage.name %>
 
 <%= render(Primer::OpenProject::PageHeader.new) do |header| %>
-  <% header.with_title(test_selector: 'storage-name-title') do %>
+  <% header.with_title(test_selector: 'storage-new-page-header--title') do %>
     <%= render OpTurbo::FrameComponent.new(@storage, context: :edit_storage_header) do %>
       <%= @storage.name %>
     <% end %>
diff --git a/modules/storages/app/views/storages/admin/storages/new.html.erb b/modules/storages/app/views/storages/admin/storages/new.html.erb
index 0ec295bc478f..01a52515d0fb 100644
--- a/modules/storages/app/views/storages/admin/storages/new.html.erb
+++ b/modules/storages/app/views/storages/admin/storages/new.html.erb
@@ -8,7 +8,7 @@
 <% local_assigns[:additional_breadcrumb] = label_new_file_storage %>
 
 <%= render(Primer::OpenProject::PageHeader.new) do |header| %>
-  <% header.with_title(test_selector: 'storage-name-title') do %>
+  <% header.with_title(test_selector: 'storage-new-page-header--title') do %>
     <%= label_new_file_storage %>
   <% end %>
 
@@ -18,7 +18,7 @@
     <%= t(:project_module_storages) %>
   <% end %>
 
-  <% header.with_description do %>
+  <% header.with_description(test_selector: 'storage-new-page-header--description') do %>
     <%=
       t("storages.instructions.new_storage",
         file_storage_docs_link: render(
diff --git a/modules/storages/app/views/storages/admin/storages/new.turbo_stream.erb b/modules/storages/app/views/storages/admin/storages/new.turbo_stream.erb
index 036c0a4897ce..ca3c5aa7af46 100644
--- a/modules/storages/app/views/storages/admin/storages/new.turbo_stream.erb
+++ b/modules/storages/app/views/storages/admin/storages/new.turbo_stream.erb
@@ -27,8 +27,8 @@ See COPYRIGHT and LICENSE files for more details.
 
 ++#%>
 
-<%= turbo_stream.replace :select_storage_provider do %>
+<%= turbo_stream.replace :storage_general_info_section do %>
   <%=
-    render(::Storages::Admin::SelectStorageProviderComponent.new(@storage))
+    render(::Storages::Admin::StorageViewComponent.new(@storage))
   %>
 <% end %>
diff --git a/modules/storages/spec/features/admin_storages_spec.rb b/modules/storages/spec/features/admin_storages_spec.rb
index 97529e136edc..be240e931499 100644
--- a/modules/storages/spec/features/admin_storages_spec.rb
+++ b/modules/storages/spec/features/admin_storages_spec.rb
@@ -126,16 +126,23 @@
       it 'renders a Nextcloud specific multi-step form', :webmock do
         visit admin_settings_storages_path
 
-        within('.blankslate') { click_link("Storage") }
-        expect(page).to have_current_path(new_admin_settings_storage_path)
+        within('.PageHeader') { click_button("Storage") }
+        within_test_selector('storages-select-provider-action-menu') { click_link('Nextcloud') }
+
+        expect(page).to have_current_path(select_provider_admin_settings_storages_path(provider: 'nextcloud'))
 
         aggregate_failures 'Select provider view' do
-          # General information
-          expect(page).to have_select('storages_storage[provider_type]', with_options: %w[Nextcloud OneDrive/SharePoint])
-          expect(find_test_selector('storage-select-provider-submit-button')).to be_disabled
+          # Page Header
+          expect(page).to have_test_selector('storage-new-page-header--title', text: 'New Nextcloud storage')
+          expect(page).to have_test_selector('storage-new-page-header--description',
+                                             text: "Read our documentation on setting up a Nextcloud file storage " \
+                                                   "integration for more information.")
 
-          # Select Nextcloud
-          select('Nextcloud', from: 'storages_storage[provider_type]')
+          # General information
+          expect(page).to have_test_selector('storage-provider-configuration-instructions',
+                                             text: "Please make sure you have administration privileges in your " \
+                                                   "Nextcloud instance and the application “Integration OpenProject” " \
+                                                   "is installed before doing the setup.")
 
           # OAuth application
           expect(page).to have_test_selector('storage-openproject-oauth-label', text: 'OpenProject OAuth')
@@ -255,16 +262,22 @@
       it 'renders a One Drive specific multi-step form', :webmock do
         visit admin_settings_storages_path
 
-        within('.PageHeader') { click_link("Storage") }
-        expect(page).to have_current_path(new_admin_settings_storage_path)
+        within('.PageHeader') { click_button("Storage") }
+        within_test_selector('storages-select-provider-action-menu') { click_link('OneDrive/SharePoint') }
+
+        expect(page).to have_current_path(select_provider_admin_settings_storages_path(provider: 'one_drive'))
 
         aggregate_failures 'Select provider view' do
-          # General information
-          expect(page).to have_select('storages_storage[provider_type]', with_options: %w[Nextcloud OneDrive/SharePoint])
-          expect(find_test_selector('storage-select-provider-submit-button')).to be_disabled
+          # Page Header
+          expect(page).to have_test_selector('storage-new-page-header--title', text: 'New OneDrive/SharePoint storage')
+          expect(page).to have_test_selector('storage-new-page-header--description',
+                                             text: "Read our documentation on setting up a OneDrive/SharePoint " \
+                                                   "file storage integration for more information.")
 
-          # Select OneDrive
-          select('OneDrive/SharePoint', from: 'storages_storage[provider_type]')
+          # General information
+          expect(page).to have_test_selector('storage-provider-configuration-instructions',
+                                             text: "Please make sure you have administration privileges in the " \
+                                                   "Azure application before doing the setup.")
 
           # OAuth client
           wait_for(page).to have_test_selector('storage-oauth-client-label', text: 'Azure OAuth')
@@ -354,7 +367,7 @@
       it 'renders an edit view', :webmock do
         visit edit_admin_settings_storage_path(storage)
 
-        expect(page).to have_test_selector('storage-name-title', text: storage.name.capitalize)
+        expect(page).to have_test_selector('storage-new-page-header--title', text: storage.name.capitalize)
 
         aggregate_failures 'Storage edit view' do
           # General information
@@ -393,7 +406,7 @@
             click_button 'Save and continue'
           end
 
-          expect(page).to have_test_selector('storage-name-title', text: 'My Nextcloud')
+          expect(page).to have_test_selector('storage-new-page-header--title', text: 'My Nextcloud')
           expect(page).to have_test_selector('storage-description', text: "Nextcloud - My Nextcloud - #{storage.host}")
 
           # Update a storage - unhappy path
@@ -502,7 +515,7 @@
       it 'renders an edit view', :webmock do
         visit edit_admin_settings_storage_path(storage)
 
-        expect(page).to have_test_selector('storage-name-title', text: 'Test Drive')
+        expect(page).to have_test_selector('storage-new-page-header--title', text: 'Test Drive')
 
         aggregate_failures 'Storage edit view' do
           # General information
@@ -528,7 +541,7 @@
             click_button 'Save and continue'
           end
 
-          expect(page).to have_test_selector('storage-name-title', text: 'My OneDrive')
+          expect(page).to have_test_selector('storage-new-page-header--title', text: 'My OneDrive')
           expect(page).to have_test_selector('storage-description', text: 'OneDrive/SharePoint - My OneDrive')
 
           # Update a storage - unhappy path

From 9d0dbebfd1dd832f65de994216ed1bb096637bd7 Mon Sep 17 00:00:00 2001
From: Kabiru Mwenja <makabby@gmail.com>
Date: Tue, 28 Nov 2023 13:47:18 +0300
Subject: [PATCH 5/9] chore[Op#51022]: update edit page with provider type

---
 .../general_info_form_component.html.erb      |  2 +-
 .../storages/admin/storages/edit.html.erb     | 43 +++++++++++--------
 .../storages/admin/storages/new.html.erb      |  1 +
 .../admin/storages/update.turbo_stream.erb    | 10 ++++-
 .../spec/features/admin_storages_spec.rb      | 16 +++----
 5 files changed, 42 insertions(+), 30 deletions(-)

diff --git a/modules/storages/app/components/storages/admin/forms/general_info_form_component.html.erb b/modules/storages/app/components/storages/admin/forms/general_info_form_component.html.erb
index 46304fe15e69..f7a3f16cfdeb 100644
--- a/modules/storages/app/components/storages/admin/forms/general_info_form_component.html.erb
+++ b/modules/storages/app/components/storages/admin/forms/general_info_form_component.html.erb
@@ -8,7 +8,7 @@
       flex_layout do |general_info_row|
         general_info_row.with_row(mb: 3) do
           concat(render(Primer::Beta::Text.new(font_weight: :bold)) { I18n.t("storages.label_file_storage") })
-          concat(render(Storages::Admin::ProviderTypeHiddenInputForm.new(form, storage:)))
+          concat(render(Storages::Admin::ProviderTypeHiddenInputForm.new(form, storage:))) if storage.new_record?
         end
 
         general_info_row.with_row(mb: 3) do
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 c02a6204ec2f..e46746b17de4 100644
--- a/modules/storages/app/views/storages/admin/storages/edit.html.erb
+++ b/modules/storages/app/views/storages/admin/storages/edit.html.erb
@@ -27,13 +27,20 @@ See COPYRIGHT and LICENSE files for more details.
 
 ++#%>
 
+<%
+  label_storage_provider_part = render(Primer::Beta::Text.new(tag: :span, font_weight: :light, color: :muted)) do
+    "(#{I18n.t("storages.provider_types.#{@storage.short_provider_type}.name")})"
+  end
+  label_storage_name_with_provider_label = "#{@storage.name} #{label_storage_provider_part}".html_safe
+%>
+
 <% html_title t(:label_administration), t("project_module_storages"), t('label_edit_x', x: @storage.name) %>
 <% local_assigns[:additional_breadcrumb] = @storage.name %>
 
 <%= render(Primer::OpenProject::PageHeader.new) do |header| %>
   <% header.with_title(test_selector: 'storage-new-page-header--title') do %>
     <%= render OpTurbo::FrameComponent.new(@storage, context: :edit_storage_header) do %>
-      <%= @storage.name %>
+      <%= label_storage_name_with_provider_label %>
     <% end %>
   <% end %>
 
@@ -45,24 +52,24 @@ See COPYRIGHT and LICENSE files for more details.
 
   <% header.with_actions do %>
     <%=
-    primer_form_with(
-      model: @storage,
-      url: confirm_destroy_admin_settings_storage_path(@storage),
-      method: :get
-        ) do |_form|
-          render(
-            Primer::Beta::Button.new(
-              scheme: :danger,
-              size: :medium,
-              type: :submit,
-              aria: { label: I18n.t("storages.label_delete_storage") },
-          test_selector: 'storage-delete-button'
-        )
-      ) do |button|
-        button.with_leading_visual_icon(icon: :trash)
-        I18n.t('button_delete')
+      primer_form_with(
+        model: @storage,
+        url: confirm_destroy_admin_settings_storage_path(@storage),
+        method: :get
+      ) do |_form|
+        render(
+          Primer::Beta::Button.new(
+            scheme: :danger,
+            size: :medium,
+            type: :submit,
+            aria: { label: I18n.t("storages.label_delete_storage") },
+            test_selector: 'storage-delete-button'
+          )
+        ) do |button|
+          button.with_leading_visual_icon(icon: :trash)
+          I18n.t('button_delete')
+        end
       end
-    end
     %>
   <% end %>
 <% end %>
diff --git a/modules/storages/app/views/storages/admin/storages/new.html.erb b/modules/storages/app/views/storages/admin/storages/new.html.erb
index 01a52515d0fb..b84293fe2aee 100644
--- a/modules/storages/app/views/storages/admin/storages/new.html.erb
+++ b/modules/storages/app/views/storages/admin/storages/new.html.erb
@@ -26,6 +26,7 @@
         ) { t("storages.instructions.file_storage_docs_link", provider_name: I18n.t("storages.provider_types.#{@storage.short_provider_type}.name")) }
       ).html_safe
     %>
+  <% end %>
 <% end %>
 
 <%= render(Storages::Admin::StorageViewComponent.new(@storage)) %>
diff --git a/modules/storages/app/views/storages/admin/storages/update.turbo_stream.erb b/modules/storages/app/views/storages/admin/storages/update.turbo_stream.erb
index 6d886c17da5f..4fd990299510 100644
--- a/modules/storages/app/views/storages/admin/storages/update.turbo_stream.erb
+++ b/modules/storages/app/views/storages/admin/storages/update.turbo_stream.erb
@@ -1,6 +1,14 @@
+<%
+  label_storage_provider_part = render(Primer::Beta::Text.new(tag: :span, font_weight: :light, color: :muted)) do
+    "(#{I18n.t("storages.provider_types.#{@storage.short_provider_type}.name")})"
+  end
+  label_storage_name_with_provider_label = "#{@storage.name} #{label_storage_provider_part}".html_safe
+%>
+
 <%= turbo_stream.update dom_id(@storage, :edit_storage_header) do %>
-  <%= @storage.name %>
+  <%= label_storage_name_with_provider_label %>
 <% end %>
+
 <%= turbo_stream.update :full_breadcrumbs do %>
   <%
     breadcrumb_paths(
diff --git a/modules/storages/spec/features/admin_storages_spec.rb b/modules/storages/spec/features/admin_storages_spec.rb
index be240e931499..56aa2e876b44 100644
--- a/modules/storages/spec/features/admin_storages_spec.rb
+++ b/modules/storages/spec/features/admin_storages_spec.rb
@@ -329,7 +329,7 @@
     end
   end
 
-  describe 'File storage edit view' do
+  describe 'Edit file storage' do
     it 'renders a danger zone for deletion' do
       storage = create(:nextcloud_storage, name: "Foo Nextcloud")
       visit edit_admin_settings_storage_path(storage)
@@ -353,7 +353,7 @@
     end
 
     context 'with Nextcloud Storage' do
-      let(:storage) { create(:nextcloud_storage, :as_automatically_managed) }
+      let(:storage) { create(:nextcloud_storage, :as_automatically_managed, name: 'Cloud Storage') }
       let(:oauth_application) { create(:oauth_application, integration: storage) }
       let(:oauth_client) { create(:oauth_client, integration: storage) }
       let(:secret) { 'awesome_secret' }
@@ -367,7 +367,7 @@
       it 'renders an edit view', :webmock do
         visit edit_admin_settings_storage_path(storage)
 
-        expect(page).to have_test_selector('storage-new-page-header--title', text: storage.name.capitalize)
+        expect(page).to have_test_selector('storage-new-page-header--title', text: "Cloud Storage (Nextcloud)")
 
         aggregate_failures 'Storage edit view' do
           # General information
@@ -400,13 +400,11 @@
           # Update a storage - happy path
           find_test_selector('storage-edit-host-button').click
           within_test_selector('storage-general-info-form') do
-            expect(page).to have_css('#storages_nextcloud_storage_provider_type[disabled]')
-
             fill_in 'storages_nextcloud_storage_name', with: 'My Nextcloud'
             click_button 'Save and continue'
           end
 
-          expect(page).to have_test_selector('storage-new-page-header--title', text: 'My Nextcloud')
+          expect(page).to have_test_selector('storage-new-page-header--title', text: 'My Nextcloud (Nextcloud)')
           expect(page).to have_test_selector('storage-description', text: "Nextcloud - My Nextcloud - #{storage.host}")
 
           # Update a storage - unhappy path
@@ -515,7 +513,7 @@
       it 'renders an edit view', :webmock do
         visit edit_admin_settings_storage_path(storage)
 
-        expect(page).to have_test_selector('storage-new-page-header--title', text: 'Test Drive')
+        expect(page).to have_test_selector('storage-new-page-header--title', text: 'Test Drive (OneDrive/SharePoint)')
 
         aggregate_failures 'Storage edit view' do
           # General information
@@ -535,13 +533,11 @@
           # Update a storage - happy path
           find_test_selector('storage-edit-host-button').click
           within_test_selector('storage-general-info-form') do
-            expect(page).to have_css('#storages_one_drive_storage_provider_type[disabled]')
-
             fill_in 'storages_one_drive_storage_name', with: 'My OneDrive'
             click_button 'Save and continue'
           end
 
-          expect(page).to have_test_selector('storage-new-page-header--title', text: 'My OneDrive')
+          expect(page).to have_test_selector('storage-new-page-header--title', text: 'My OneDrive (OneDrive/SharePoint)')
           expect(page).to have_test_selector('storage-description', text: 'OneDrive/SharePoint - My OneDrive')
 
           # Update a storage - unhappy path

From 4923ec28195d18be58d0e8f504474c15696cf4d7 Mon Sep 17 00:00:00 2001
From: Kabiru Mwenja <makabby@gmail.com>
Date: Tue, 28 Nov 2023 14:34:04 +0300
Subject: [PATCH 6/9] chore[Op#51022]: Add triangle down for new storage button

---
 .../storages/admin/new_storage_button_component.html.erb         | 1 +
 1 file changed, 1 insertion(+)

diff --git a/modules/storages/app/components/storages/admin/new_storage_button_component.html.erb b/modules/storages/app/components/storages/admin/new_storage_button_component.html.erb
index e37e9caa770d..99a88c0282ef 100644
--- a/modules/storages/app/components/storages/admin/new_storage_button_component.html.erb
+++ b/modules/storages/app/components/storages/admin/new_storage_button_component.html.erb
@@ -2,6 +2,7 @@
   render(Primer::Alpha::ActionMenu.new(test_selector: 'storages-select-provider-action-menu')) do |menu|
     menu.with_show_button(**show_button_options) do |button|
       button.with_leading_visual_icon(icon: :plus)
+      button.with_trailing_action_icon(icon: :"triangle-down")
       label
     end
 

From 38bbbc141a0ca278a9b948bc0f0d88807338429f Mon Sep 17 00:00:00 2001
From: Kabiru Mwenja <makabby@gmail.com>
Date: Tue, 28 Nov 2023 14:42:16 +0300
Subject: [PATCH 7/9] fix[Op#51022]: mark nested link as html safe

---
 modules/storages/app/views/storages/admin/storages/new.html.erb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/modules/storages/app/views/storages/admin/storages/new.html.erb b/modules/storages/app/views/storages/admin/storages/new.html.erb
index b84293fe2aee..43ed4563378a 100644
--- a/modules/storages/app/views/storages/admin/storages/new.html.erb
+++ b/modules/storages/app/views/storages/admin/storages/new.html.erb
@@ -23,7 +23,7 @@
       t("storages.instructions.new_storage",
         file_storage_docs_link: render(
           Primer::Beta::Link.new(href: ::OpenProject::Static::Links[:storage_docs][:"#{@storage.short_provider_type}_setup"][:href], target: '_blank')
-        ) { t("storages.instructions.file_storage_docs_link", provider_name: I18n.t("storages.provider_types.#{@storage.short_provider_type}.name")) }
+        ) { t("storages.instructions.file_storage_docs_link", provider_name: I18n.t("storages.provider_types.#{@storage.short_provider_type}.name")).html_safe }
       ).html_safe
     %>
   <% end %>

From 47de181a0bcd854830b38ca9def339182e2a4324 Mon Sep 17 00:00:00 2001
From: Kabiru Mwenja <makabby@gmail.com>
Date: Tue, 28 Nov 2023 15:01:52 +0300
Subject: [PATCH 8/9] fix[Op#51022]: Handle invalid provider selection

---
 .../storages/admin/storages_controller.rb     |  2 +-
 modules/storages/config/locales/en.yml        |  2 +-
 .../spec/features/admin_storages_spec.rb      | 30 +++++++++++++++----
 3 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/modules/storages/app/controllers/storages/admin/storages_controller.rb b/modules/storages/app/controllers/storages/admin/storages_controller.rb
index 7018e180e317..6ad24737f62c 100644
--- a/modules/storages/app/controllers/storages/admin/storages_controller.rb
+++ b/modules/storages/app/controllers/storages/admin/storages_controller.rb
@@ -208,7 +208,7 @@ def ensure_valid_provider_type_selected
     short_provider_type = params[:provider]
     if short_provider_type.blank? || (@provider_type = ::Storages::Storage::PROVIDER_TYPE_SHORT_NAMES[short_provider_type]).blank?
       flash[:error] = I18n.t('storages.error_invalid_provider_type')
-      render :index
+      redirect_to admin_settings_storages_path
     end
   end
 
diff --git a/modules/storages/config/locales/en.yml b/modules/storages/config/locales/en.yml
index 171979bb5344..4b01882fe80b 100644
--- a/modules/storages/config/locales/en.yml
+++ b/modules/storages/config/locales/en.yml
@@ -181,7 +181,7 @@ en:
     storage_list_blank_slate:
       heading: "You don't have any storages yet."
       description: "Add a storage to see them here."
-    error_invalid_provider_type: "Please select a valid provider type."
+    error_invalid_provider_type: "Please select a valid storage provider."
     label_active: "Active"
     label_add_new_storage: "Add new storage"
     label_delete_storage: "Delete storage"
diff --git a/modules/storages/spec/features/admin_storages_spec.rb b/modules/storages/spec/features/admin_storages_spec.rb
index 56aa2e876b44..a13f6b2bc96f 100644
--- a/modules/storages/spec/features/admin_storages_spec.rb
+++ b/modules/storages/spec/features/admin_storages_spec.rb
@@ -34,7 +34,6 @@
 RSpec.describe 'Admin storages',
                :js,
                :storage_server_helpers do
-
   let(:admin) { create(:admin) }
 
   before { login_as admin }
@@ -61,8 +60,7 @@
 
         expect(page).to have_test_selector('storage-name', text: complete_storage.name)
         expect(page).to have_test_selector('storage-name', text: incomplete_storage.name)
-        expect(page).to have_css("a[role='button'][aria-label='Add new storage'][href='#{new_admin_settings_storage_path}']",
-                                 text: 'Storage')
+        expect(page).to have_css("button[aria-label='Add new storage']", text: 'Storage')
 
         within "li#storages_nextcloud_storage_#{complete_storage.id}" do
           expect(page).not_to have_test_selector('label-incomplete')
@@ -104,13 +102,13 @@
       it 'renders a blank slate' do
         visit admin_settings_storages_path
 
+        # Show Add storage button
+        expect(page).to have_css("button[aria-label='Add new storage']", text: 'Storage')
+
         # Show empty storages list
         expect(page).to have_title('File storages')
         expect(page.find('.PageHeader-title')).to have_text('File storages')
         expect(page).to have_text("You don't have any storages yet.")
-        # Show Add storage buttons
-        expect(page).to have_css("a[role='button'][aria-label='Add new storage'][href='#{new_admin_settings_storage_path}']",
-                                 text: 'Storage').twice
       end
     end
   end
@@ -327,6 +325,26 @@
         end
       end
     end
+
+    describe 'Select provider page' do
+      context 'when navigating directly to the page' do
+        it 'redirects you back to the index page' do
+          visit select_provider_admin_settings_storages_path
+
+          expect(page).to have_current_path(admin_settings_storages_path)
+          wait_for(page).to have_text("Please select a valid storage provider.")
+        end
+      end
+
+      context 'when navigating to the page with an invalid provider' do
+        it 'redirects you back to the index page' do
+          visit select_provider_admin_settings_storages_path(provider: 'foobar')
+
+          expect(page).to have_current_path(admin_settings_storages_path)
+          wait_for(page).to have_text("Please select a valid storage provider.")
+        end
+      end
+    end
   end
 
   describe 'Edit file storage' do

From cbac21c0af83f3648e936c0245f470a1e86178f4 Mon Sep 17 00:00:00 2001
From: Kabiru Mwenja <makabby@gmail.com>
Date: Wed, 29 Nov 2023 15:43:04 +0300
Subject: [PATCH 9/9] fix[Op#51022]: Do not split translations for I18n
 benefits

Co-authored-by: Henriette Darge <h.darge@openproject.com>
---
 .../storages/app/views/storages/admin/storages/new.html.erb  | 5 ++---
 modules/storages/config/locales/en.yml                       | 3 +--
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/modules/storages/app/views/storages/admin/storages/new.html.erb b/modules/storages/app/views/storages/admin/storages/new.html.erb
index 43ed4563378a..1cee2eedb9f7 100644
--- a/modules/storages/app/views/storages/admin/storages/new.html.erb
+++ b/modules/storages/app/views/storages/admin/storages/new.html.erb
@@ -21,9 +21,8 @@
   <% header.with_description(test_selector: 'storage-new-page-header--description') do %>
     <%=
       t("storages.instructions.new_storage",
-        file_storage_docs_link: render(
-          Primer::Beta::Link.new(href: ::OpenProject::Static::Links[:storage_docs][:"#{@storage.short_provider_type}_setup"][:href], target: '_blank')
-        ) { t("storages.instructions.file_storage_docs_link", provider_name: I18n.t("storages.provider_types.#{@storage.short_provider_type}.name")).html_safe }
+        provider_link: ::OpenProject::Static::Links[:storage_docs][:"#{@storage.short_provider_type}_setup"][:href].html_safe,
+        provider_name: I18n.t("storages.provider_types.#{@storage.short_provider_type}.name")
       ).html_safe
     %>
   <% end %>
diff --git a/modules/storages/config/locales/en.yml b/modules/storages/config/locales/en.yml
index 4b01882fe80b..04b9875408a4 100644
--- a/modules/storages/config/locales/en.yml
+++ b/modules/storages/config/locales/en.yml
@@ -118,8 +118,7 @@ en:
       managed_project_folders_application_password: >
         Copy this value from:
       managed_project_folders_application_password_caption: "Enable automatic managed folders by copying this value from: %{provider_type_link}."
-      new_storage: "Read our documentation on %{file_storage_docs_link} integration for more information."
-      file_storage_docs_link: "setting up a %{provider_name} file storage"
+      new_storage: "Read our documentation on <a target='_blank' href='%{provider_link}'>setting up a %{provider_name} file storage</a> integration for more information."
       no_storage_set_up: "There are no file storages set up yet."
       no_specific_folder: "By default, each user will start at their own home folder when they upload a file."
       automatic_folder: "This will automatically create a root folder for this project and manage the access permissions for each project member."