Merge pull request #14251 from opf/task/51022-revisit-the-create-stor…

…age-workflow

 A11y[Op#51022] Replace New Storage "magic form" with more accessible ActionMenu

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'
               },

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:))) if storage.new_record?
+        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

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
 

modules/storages/app/components/storages/admin/new_storage_button_component.html.erb

@@ -1,6 +1,17 @@
 <%=
-  render(Primer::Beta::Button.new(**button_options)) do |button|
-    button.with_leading_visual_icon(icon: :plus)
-    label
+  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
+
+    ::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
 %>

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
 

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
 %>

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,13 @@ 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 }
+      end
     end
   end
 
-  def create # rubocop:disable Metrics/AbcSize
+  def create
     service_result = Storages::Storages::CreateService
                       .new(user: current_user)
                       .call(permitted_storage_params)
@@ -104,9 +107,7 @@ def create # rubocop:disable Metrics/AbcSize
     @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
@@ -203,6 +204,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')
+      redirect_to admin_settings_storages_path
+    end
+  end
+
   def oauth_application(service_result)
     service_result.dependent_results&.first&.result
   end

modules/storages/app/components/storages/admin/select_storage_provider_component.rb → modules/storages/app/forms/storages/admin/provider_type_hidden_input_form.rb

@@ -1,5 +1,3 @@
-# frozen_string_literal: true
-
 #-- copyright
 # OpenProject is an open source project management software.
 # Copyright (C) 2012-2023 the OpenProject GmbH
@@ -27,10 +25,22 @@
 #
 # See COPYRIGHT and LICENSE files for more details.
 #++
-#
+
 module Storages::Admin
-  class SelectStorageProviderComponent < ApplicationComponent
-    include OpPrimer::ComponentHelpers
-    alias_method :storage, :model
+  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

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'