From ad89ddfc917701112ebe97caf12af5944ddd7524 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Mon, 30 Oct 2023 18:35:50 +0300 Subject: [PATCH 01/56] chore[Op#50737]: Break up `StorageProviderForm` for easier composition --- .../general_info_form_component.html.erb | 10 ++++- .../admin/provider_host_input_form.rb | 44 +++++++++++++++++++ .../admin/provider_name_input_form.rb | 40 +++++++++++++++++ ...r_form.rb => provider_type_select_form.rb} | 23 ++-------- 4 files changed, 97 insertions(+), 20 deletions(-) create mode 100644 modules/storages/app/forms/storages/admin/provider_host_input_form.rb create mode 100644 modules/storages/app/forms/storages/admin/provider_name_input_form.rb rename modules/storages/app/forms/storages/admin/{storage_provider_form.rb => provider_type_select_form.rb} (78%) 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 0b5db97d24ea..36935e62bf8f 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,7 +7,15 @@ ) do |form| flex_layout do |general_info_row| general_info_row.with_row(mb: 3) do - render(Storages::Admin::StorageProviderForm.new(form, storage:)) + render(Storages::Admin::ProviderTypeSelectForm.new(form, storage:)) + end + + general_info_row.with_row(mb: 3) do + render(Storages::Admin::ProviderNameInputForm.new(form)) + end + + general_info_row.with_row(mb: 3) do + render(Storages::Admin::ProviderHostInputForm.new(form)) end general_info_row.with_row do diff --git a/modules/storages/app/forms/storages/admin/provider_host_input_form.rb b/modules/storages/app/forms/storages/admin/provider_host_input_form.rb new file mode 100644 index 000000000000..fb49ed372a78 --- /dev/null +++ b/modules/storages/app/forms/storages/admin/provider_host_input_form.rb @@ -0,0 +1,44 @@ +#-- 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 ProviderHostInputForm < ApplicationForm + form do |storage_form| + storage_form.text_field( + name: :host, + label: I18n.t('activerecord.attributes.storages/storage.host'), + visually_hide_label: false, + required: true, + type: :url, + pattern: ".{1,255}", + placeholder: "https://my-file-storage.com", + caption: I18n.t('storages.instructions.host') + ) + end + end +end diff --git a/modules/storages/app/forms/storages/admin/provider_name_input_form.rb b/modules/storages/app/forms/storages/admin/provider_name_input_form.rb new file mode 100644 index 000000000000..f8ba9f368872 --- /dev/null +++ b/modules/storages/app/forms/storages/admin/provider_name_input_form.rb @@ -0,0 +1,40 @@ +#-- 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 ProviderNameInputForm < ApplicationForm + form do |storage_form| + storage_form.text_field( + name: :name, + label: I18n.t('activerecord.attributes.storages/storage.name'), + required: true, + caption: I18n.t('storages.instructions.name') + ) + end + end +end diff --git a/modules/storages/app/forms/storages/admin/storage_provider_form.rb b/modules/storages/app/forms/storages/admin/provider_type_select_form.rb similarity index 78% rename from modules/storages/app/forms/storages/admin/storage_provider_form.rb rename to modules/storages/app/forms/storages/admin/provider_type_select_form.rb index 068004abeb4b..c58be9f1b83c 100644 --- a/modules/storages/app/forms/storages/admin/storage_provider_form.rb +++ b/modules/storages/app/forms/storages/admin/provider_type_select_form.rb @@ -27,14 +27,14 @@ #++ module Storages::Admin - class StorageProviderForm < ApplicationForm - form do |storage_provider_form| - storage_provider_form.select_list( + class ProviderTypeSelectForm < ApplicationForm + form do |storage_form| + storage_form.select_list( name: :provider_type, label: I18n.t('activerecord.attributes.storages/storage.provider_type'), caption: I18n.t('storages.instructions.provider_type', type_link_text: I18n.t('storages.instructions.type_link_text')), - include_blank: false, + include_blank: @storage.new_record?, required: true, disabled: @storage.persisted? ) do |storage_provider_list| @@ -52,21 +52,6 @@ class StorageProviderForm < ApplicationForm end end end - - storage_provider_form.text_field( - name: :name, - label: I18n.t('activerecord.attributes.storages/storage.name'), - required: true, - caption: I18n.t('storages.instructions.name') - ) - - storage_provider_form.text_field( - name: :host, - label: I18n.t('activerecord.attributes.storages/storage.host'), - visually_hide_label: false, - required: true, - caption: I18n.t('storages.instructions.host') - ) end def initialize(storage:) From b64417ecaf40588fed3dffed06166d4055bc190b Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Mon, 30 Oct 2023 18:38:34 +0300 Subject: [PATCH 02/56] feat[Op#50737]: Expand SaveOrCancel form options --- .../forms/general_info_form_component.html.erb | 9 ++++++++- .../forms/storages/admin/save_or_cancel_form.rb | 14 +++++++++++--- 2 files changed, 19 insertions(+), 4 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 36935e62bf8f..a11d1c1ecc03 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 @@ -19,7 +19,14 @@ end general_info_row.with_row do - render(Storages::Admin::SaveOrCancelForm.new(form, storage:)) + render( + Storages::Admin::SaveOrCancelForm.new( + form, + storage:, + cancel_button_path:, + cancel_button_should_break_from_frame: + ) + ) end end end diff --git a/modules/storages/app/forms/storages/admin/save_or_cancel_form.rb b/modules/storages/app/forms/storages/admin/save_or_cancel_form.rb index 47d724deffc7..242db77fc64f 100644 --- a/modules/storages/app/forms/storages/admin/save_or_cancel_form.rb +++ b/modules/storages/app/forms/storages/admin/save_or_cancel_form.rb @@ -36,15 +36,23 @@ class SaveOrCancelForm < ApplicationForm button_group.button(name: :cancel, scheme: :default, tag: :a, - href: Rails.application.routes.url_helpers.edit_admin_settings_storage_path(@storage), - label: I18n.t('button_cancel')) + href: @cancel_button_path, + label: I18n.t('button_cancel'), + target: @cancel_button_target) end end - def initialize(storage:, submit_label: I18n.t('storages.buttons.save_and_continue')) + def initialize( + storage:, + cancel_button_path: nil, + cancel_button_should_break_from_frame: false, + submit_label: I18n.t('storages.buttons.save_and_continue') + ) super() @storage = storage @submit_label = submit_label + @cancel_button_path = cancel_button_path || Rails.application.routes.url_helpers.edit_admin_settings_storage_path(storage) + @cancel_button_target = '_top' if cancel_button_should_break_from_frame end end end From 7c68bfb87168ffa499734c464d20ee8f3c0c1026 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Mon, 30 Oct 2023 18:48:14 +0300 Subject: [PATCH 03/56] feat[Op#50737]: Expand GeneralInfo form options --- .../general_info_form_component.html.erb | 12 +++++----- .../forms/general_info_form_component.rb | 22 +++++++++++++++++++ .../admin/storages/edit_host.html.erb | 8 ++++++- 3 files changed, 36 insertions(+), 6 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 a11d1c1ecc03..360cb8dff9aa 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 @@ -1,9 +1,9 @@ <%= render(Primer::Beta::Text.new(tag: :div, test_selector: 'storage-general-info-form')) do primer_form_with( - model: storage, - url: admin_settings_storage_path(storage), - method: :patch + model:, + url: form_url, + method: form_method ) do |form| flex_layout do |general_info_row| general_info_row.with_row(mb: 3) do @@ -14,8 +14,10 @@ render(Storages::Admin::ProviderNameInputForm.new(form)) end - general_info_row.with_row(mb: 3) do - render(Storages::Admin::ProviderHostInputForm.new(form)) + if render_host? + general_info_row.with_row(mb: 3) do + render(Storages::Admin::ProviderHostInputForm.new(form)) + end end general_info_row.with_row 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 cd8de8524c66..9031a49a7140 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 @@ -32,5 +32,27 @@ module Storages::Admin::Forms class GeneralInfoFormComponent < ApplicationComponent include OpPrimer::ComponentHelpers alias_method :storage, :model + + options form_method: :post, + render_host: true, + cancel_button_should_break_from_frame: false, + cancel_button_path: Rails.application.routes.url_helpers.admin_settings_storages_path + + alias_method :render_host?, :render_host + + def form_url + options[:form_url] || default_form_url + end + + private + + def default_form_url + case form_method + when :get, :post + Rails.application.routes.url_helpers.admin_settings_storages_path + when :patch, :put + Rails.application.routes.url_helpers.admin_settings_storage_path(storage) + end + end end end diff --git a/modules/storages/app/views/storages/admin/storages/edit_host.html.erb b/modules/storages/app/views/storages/admin/storages/edit_host.html.erb index 6c566f6fdb98..f101a26d0385 100644 --- a/modules/storages/app/views/storages/admin/storages/edit_host.html.erb +++ b/modules/storages/app/views/storages/admin/storages/edit_host.html.erb @@ -1,5 +1,11 @@ <%= render(OpTurbo::FrameComponent.new(@storage, context: :storage_view_general_info)) do - render(Storages::Admin::Forms::GeneralInfoFormComponent.new(@storage)) + render( + Storages::Admin::Forms::GeneralInfoFormComponent.new( + @storage, + form_method: :patch, + cancel_button_path: edit_admin_settings_storage_path(@storage) + ) + ) end %> From c8506a02c3d8293f3f9d5b52a4b1313235a1fa6c Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 31 Oct 2023 12:34:26 +0300 Subject: [PATCH 04/56] fix[Op#50737]: update cancel button path --- ...cally_managed_project_folders_form_component.html.erb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.html.erb b/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.html.erb index 21698f170998..f6a74c65eff2 100644 --- a/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.html.erb +++ b/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.html.erb @@ -28,7 +28,14 @@ end project_folders_form.with_row do - render(Storages::Admin::SaveOrCancelForm.new(form, storage:, submit_label: I18n.t("storages.buttons.done_complete_setup"))) + render( + Storages::Admin::SaveOrCancelForm.new( + form, + storage:, + submit_label: I18n.t("storages.buttons.done_complete_setup"), + cancel_button_path: edit_admin_settings_storage_path(storage) + ) + ) end end end From dad0c2420f316974ac3f0916b43e3cc2ba7c462c Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 31 Oct 2023 19:00:47 +0300 Subject: [PATCH 05/56] feat[Op#50737]: Expand FrameComponent to allow explicit id with context --- app/components/op_turbo/frame_component.rb | 12 ++++++++++++ spec/components/op_turbo/frame_component_spec.rb | 16 ++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/app/components/op_turbo/frame_component.rb b/app/components/op_turbo/frame_component.rb index 6f3876a84ada..4db96dd27a07 100644 --- a/app/components/op_turbo/frame_component.rb +++ b/app/components/op_turbo/frame_component.rb @@ -29,7 +29,19 @@ module OpTurbo class FrameComponent < ApplicationComponent def turbo_frame_id + optional_custom_id || turbo_frame_id_from_model + end + + private + + def turbo_frame_id_from_model ActionView::RecordIdentifier.dom_id(model, options[:context]) end + + def optional_custom_id + return if options[:id].blank? + + options.values_at(:context, :id).compact.join("_") + end end end diff --git a/spec/components/op_turbo/frame_component_spec.rb b/spec/components/op_turbo/frame_component_spec.rb index 9a056f968102..6cbd030a1df6 100644 --- a/spec/components/op_turbo/frame_component_spec.rb +++ b/spec/components/op_turbo/frame_component_spec.rb @@ -47,5 +47,21 @@ expect(component.turbo_frame_id).to eq('storages_nextcloud_storage_1') end end + + context 'with `id:` option' do + it 'returns the turbo frame id' do + component = described_class.new(id: 'test_id') + + expect(component.turbo_frame_id).to eq('test_id') + end + end + + context 'with `id:` and `context:` option' do + it 'returns the turbo frame id' do + component = described_class.new(id: 'test_id', context: :general_info) + + expect(component.turbo_frame_id).to eq('general_info_test_id') + end + end end end From 1022308f9a67cf5688a98c34bd24d564e052ae32 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 31 Oct 2023 19:07:06 +0300 Subject: [PATCH 06/56] feat[Op#50737]: Let SaveOrCancel component extend sys args directly --- ...ed_project_folders_form_component.html.erb | 8 +++- .../general_info_form_component.html.erb | 9 +++- .../forms/general_info_form_component.rb | 2 + .../oauth_client_form_component.html.erb | 10 ++++- .../storages/admin/save_or_cancel_form.rb | 44 +++++++++++-------- 5 files changed, 50 insertions(+), 23 deletions(-) diff --git a/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.html.erb b/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.html.erb index f6a74c65eff2..b92da09d10a7 100644 --- a/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.html.erb +++ b/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.html.erb @@ -32,8 +32,12 @@ Storages::Admin::SaveOrCancelForm.new( form, storage:, - submit_label: I18n.t("storages.buttons.done_complete_setup"), - cancel_button_path: edit_admin_settings_storage_path(storage) + submit_button_options: { + label: I18n.t("storages.buttons.done_complete_setup") + }, + cancel_button_options: { + href: edit_admin_settings_storage_path(storage) + } ) ) end 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 360cb8dff9aa..03fdf8fd656f 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 @@ -25,8 +25,13 @@ Storages::Admin::SaveOrCancelForm.new( form, storage:, - cancel_button_path:, - cancel_button_should_break_from_frame: + submit_button_options: { + disabled: submit_button_disabled?, + }, + cancel_button_options: { + href: cancel_button_path, + target: '_top' + } ) ) end 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 9031a49a7140..1b29c699a815 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 @@ -35,10 +35,12 @@ class GeneralInfoFormComponent < ApplicationComponent options form_method: :post, render_host: true, + submit_button_disabled: false, cancel_button_should_break_from_frame: false, cancel_button_path: Rails.application.routes.url_helpers.admin_settings_storages_path alias_method :render_host?, :render_host + alias_method :submit_button_disabled?, :submit_button_disabled def form_url options[:form_url] || default_form_url diff --git a/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb b/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb index 190820e5413e..3a8054699fff 100644 --- a/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb +++ b/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb @@ -18,7 +18,15 @@ end oauth_client_row.with_row do - render(Storages::Admin::SaveOrCancelForm.new(form, storage:)) + render( + Storages::Admin::SaveOrCancelForm.new( + form, + storage:, + cancel_button_options: { + href: edit_admin_settings_storage_path(storage) + } + ) + ) end end end diff --git a/modules/storages/app/forms/storages/admin/save_or_cancel_form.rb b/modules/storages/app/forms/storages/admin/save_or_cancel_form.rb index 242db77fc64f..2b4f52d412fc 100644 --- a/modules/storages/app/forms/storages/admin/save_or_cancel_form.rb +++ b/modules/storages/app/forms/storages/admin/save_or_cancel_form.rb @@ -30,29 +30,37 @@ module Storages::Admin class SaveOrCancelForm < ApplicationForm form do |buttons| buttons.group(layout: :horizontal) do |button_group| - button_group.submit(name: :submit, - scheme: :primary, - label: @submit_label) - button_group.button(name: :cancel, - scheme: :default, - tag: :a, - href: @cancel_button_path, - label: I18n.t('button_cancel'), - target: @cancel_button_target) + button_group.submit(**@submit_button_options) + button_group.button(**@cancel_button_options) end end - def initialize( - storage:, - cancel_button_path: nil, - cancel_button_should_break_from_frame: false, - submit_label: I18n.t('storages.buttons.save_and_continue') - ) + def initialize(storage:, submit_button_options: {}, cancel_button_options: {}) super() @storage = storage - @submit_label = submit_label - @cancel_button_path = cancel_button_path || Rails.application.routes.url_helpers.edit_admin_settings_storage_path(storage) - @cancel_button_target = '_top' if cancel_button_should_break_from_frame + @submit_button_options = default_submit_button_options.merge(submit_button_options) + @cancel_button_options = default_cancel_button_options.merge(cancel_button_options) + end + + private + + def default_submit_button_options + { + name: :submit, + scheme: :primary, + label: I18n.t('storages.buttons.save_and_continue'), + disabled: false + } + end + + def default_cancel_button_options + { + name: :cancel, + scheme: :default, + tag: :a, + href: Rails.application.routes.url_helpers.admin_settings_storages_path, + label: I18n.t('button_cancel') + } end end end From 532747c285eb9468dd4418c54dc44c2d57cd4ea5 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 31 Oct 2023 19:08:05 +0300 Subject: [PATCH 07/56] feat[Op#50737]: Let ProviderTypeSelect component extend sys args directly --- .../admin/provider_type_select_form.rb | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) 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 index c58be9f1b83c..37e80c9a2e04 100644 --- a/modules/storages/app/forms/storages/admin/provider_type_select_form.rb +++ b/modules/storages/app/forms/storages/admin/provider_type_select_form.rb @@ -29,15 +29,7 @@ module Storages::Admin class ProviderTypeSelectForm < ApplicationForm form do |storage_form| - storage_form.select_list( - name: :provider_type, - label: I18n.t('activerecord.attributes.storages/storage.provider_type'), - caption: I18n.t('storages.instructions.provider_type', - type_link_text: I18n.t('storages.instructions.type_link_text')), - include_blank: @storage.new_record?, - required: true, - disabled: @storage.persisted? - ) do |storage_provider_list| + storage_form.select_list(**@select_list_options) do |storage_provider_list| if @storage.persisted? storage_provider_list.option( label: I18n.t("storages.provider_types.#{@storage.short_provider_type}.name"), @@ -54,9 +46,24 @@ class ProviderTypeSelectForm < ApplicationForm end end - def initialize(storage:) + 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: I18n.t('storages.instructions.provider_type', + type_link_text: I18n.t('storages.instructions.type_link_text')), + include_blank: storage.new_record?, + required: true, + disabled: storage.persisted? + } end end end From 3a21a94da1144f0c5411331b17d5bf512346c7cc Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 31 Oct 2023 19:10:50 +0300 Subject: [PATCH 08/56] chore[Op#50737]: Rename `SaveOrCancelForm` to `SubmitOrCancelForm` --- ...utomatically_managed_project_folders_form_component.html.erb | 2 +- .../storages/admin/forms/general_info_form_component.html.erb | 2 +- .../storages/admin/forms/oauth_client_form_component.html.erb | 2 +- .../admin/{save_or_cancel_form.rb => submit_or_cancel_form.rb} | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) rename modules/storages/app/forms/storages/admin/{save_or_cancel_form.rb => submit_or_cancel_form.rb} (98%) diff --git a/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.html.erb b/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.html.erb index b92da09d10a7..96a24726bfde 100644 --- a/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.html.erb +++ b/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.html.erb @@ -29,7 +29,7 @@ project_folders_form.with_row do render( - Storages::Admin::SaveOrCancelForm.new( + Storages::Admin::SubmitOrCancelForm.new( form, storage:, submit_button_options: { 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 03fdf8fd656f..d7a7926c6822 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 @@ -22,7 +22,7 @@ general_info_row.with_row do render( - Storages::Admin::SaveOrCancelForm.new( + Storages::Admin::SubmitOrCancelForm.new( form, storage:, submit_button_options: { diff --git a/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb b/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb index 3a8054699fff..00fb3978fb1e 100644 --- a/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb +++ b/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb @@ -19,7 +19,7 @@ oauth_client_row.with_row do render( - Storages::Admin::SaveOrCancelForm.new( + Storages::Admin::SubmitOrCancelForm.new( form, storage:, cancel_button_options: { diff --git a/modules/storages/app/forms/storages/admin/save_or_cancel_form.rb b/modules/storages/app/forms/storages/admin/submit_or_cancel_form.rb similarity index 98% rename from modules/storages/app/forms/storages/admin/save_or_cancel_form.rb rename to modules/storages/app/forms/storages/admin/submit_or_cancel_form.rb index 2b4f52d412fc..0bc365e2bceb 100644 --- a/modules/storages/app/forms/storages/admin/save_or_cancel_form.rb +++ b/modules/storages/app/forms/storages/admin/submit_or_cancel_form.rb @@ -27,7 +27,7 @@ #++ module Storages::Admin - class SaveOrCancelForm < ApplicationForm + class SubmitOrCancelForm < ApplicationForm form do |buttons| buttons.group(layout: :horizontal) do |button_group| button_group.submit(**@submit_button_options) From e8d2d6c96aef50ea06d043999ba7b009d382e080 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 31 Oct 2023 20:23:30 +0300 Subject: [PATCH 09/56] chore[Op#50737]: Rename `StorageGeneralInfo` cmpt to just `GeneralInfo` for consistency --- ...fo_component.html.erb => general_info_component.html.erb} | 0 ...e_general_info_component.rb => general_info_component.rb} | 5 +++-- .../app/views/storages/admin/storages/edit_host.html.erb | 2 +- .../views/storages/admin/storages/update.turbo_stream.erb | 5 +++-- 4 files changed, 7 insertions(+), 5 deletions(-) rename modules/storages/app/components/storages/admin/{storage_general_info_component.html.erb => general_info_component.html.erb} (100%) rename modules/storages/app/components/storages/admin/{storage_general_info_component.rb => general_info_component.rb} (95%) diff --git a/modules/storages/app/components/storages/admin/storage_general_info_component.html.erb b/modules/storages/app/components/storages/admin/general_info_component.html.erb similarity index 100% rename from modules/storages/app/components/storages/admin/storage_general_info_component.html.erb rename to modules/storages/app/components/storages/admin/general_info_component.html.erb diff --git a/modules/storages/app/components/storages/admin/storage_general_info_component.rb b/modules/storages/app/components/storages/admin/general_info_component.rb similarity index 95% rename from modules/storages/app/components/storages/admin/storage_general_info_component.rb rename to modules/storages/app/components/storages/admin/general_info_component.rb index f7f45b866a03..ad3dc0c61c0d 100644 --- a/modules/storages/app/components/storages/admin/storage_general_info_component.rb +++ b/modules/storages/app/components/storages/admin/general_info_component.rb @@ -29,9 +29,10 @@ #++ # module Storages::Admin - class StorageGeneralInfoComponent < ApplicationComponent + class GeneralInfoComponent < ApplicationComponent include OpPrimer::ComponentHelpers - alias_method :storage, :model include StorageViewInformation + + alias_method :storage, :model end end diff --git a/modules/storages/app/views/storages/admin/storages/edit_host.html.erb b/modules/storages/app/views/storages/admin/storages/edit_host.html.erb index f101a26d0385..26ed8b6a282c 100644 --- a/modules/storages/app/views/storages/admin/storages/edit_host.html.erb +++ b/modules/storages/app/views/storages/admin/storages/edit_host.html.erb @@ -1,5 +1,5 @@ <%= - render(OpTurbo::FrameComponent.new(@storage, context: :storage_view_general_info)) do + render(OpTurbo::FrameComponent.new(@storage, context: :general_info)) do render( Storages::Admin::Forms::GeneralInfoFormComponent.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 6da3cda73937..66f69f80ae95 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,7 @@ -<%= turbo_stream.update dom_id(@storage, :storage_view_general_info) do %> - <%= render(::Storages::Admin::StorageGeneralInfoComponent.new(@storage)) %> +<%= turbo_stream.update dom_id(@storage, :general_info) do %> + <%= render(::Storages::Admin::GeneralInfoComponent.new(@storage)) %> <% end %> + <%= turbo_stream.update dom_id(@storage, :edit_storage_header) do %> <%= @storage.name %> <% end %> From 32131007805677fa31c133e29b34b2435c67079a Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 31 Oct 2023 20:28:43 +0300 Subject: [PATCH 10/56] chore[Op#50737]: Extract button options to methods --- .../admin/forms/general_info_form_component.html.erb | 9 ++------- .../admin/forms/general_info_form_component.rb | 11 ++++++++++- 2 files changed, 12 insertions(+), 8 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 d7a7926c6822..dcfb25f133ee 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 @@ -25,13 +25,8 @@ Storages::Admin::SubmitOrCancelForm.new( form, storage:, - submit_button_options: { - disabled: submit_button_disabled?, - }, - cancel_button_options: { - href: cancel_button_path, - target: '_top' - } + submit_button_options:, + cancel_button_options: ) ) end 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 1b29c699a815..3a68fae1a7a9 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 @@ -40,12 +40,21 @@ class GeneralInfoFormComponent < ApplicationComponent cancel_button_path: Rails.application.routes.url_helpers.admin_settings_storages_path alias_method :render_host?, :render_host - alias_method :submit_button_disabled?, :submit_button_disabled def form_url options[:form_url] || default_form_url end + def submit_button_options + { disabled: submit_button_disabled } + end + + def cancel_button_options + { href: cancel_button_path }.tap do |options_hash| + options_hash[:target] = '_top' if cancel_button_should_break_from_frame + end + end + private def default_form_url From cfbb336f6608c288b6a3a74d32ebad3034c83ce4 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 31 Oct 2023 20:47:46 +0300 Subject: [PATCH 11/56] feat[Op#50737]: Allow optional skip_provider_type_strategy validation for select provider form --- .../storages/storages/base_contract.rb | 3 +- .../storages/shared_contract_examples.rb | 29 ++++++++++++++++++- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/modules/storages/app/contracts/storages/storages/base_contract.rb b/modules/storages/app/contracts/storages/storages/base_contract.rb index 2209cdc0c16f..d5ac7880ef85 100644 --- a/modules/storages/app/contracts/storages/storages/base_contract.rb +++ b/modules/storages/app/contracts/storages/storages/base_contract.rb @@ -47,7 +47,8 @@ class BaseContract < ::BaseContract attribute :provider_fields - validate :provider_type_strategy, unless: -> { errors.include?(:provider_type) } + validate :provider_type_strategy, + unless: -> { errors.include?(:provider_type) || @options.delete(:skip_provider_type_strategy) } private diff --git a/modules/storages/spec/contracts/storages/storages/shared_contract_examples.rb b/modules/storages/spec/contracts/storages/storages/shared_contract_examples.rb index bfdd78817e51..0c1c126ea009 100644 --- a/modules/storages/spec/contracts/storages/storages/shared_contract_examples.rb +++ b/modules/storages/spec/contracts/storages/storages/shared_contract_examples.rb @@ -89,7 +89,7 @@ it_behaves_like 'storage contract' end -RSpec.shared_examples_for 'nextcloud storage contract', :storage_server_helpers, webmock: true do +RSpec.shared_examples_for 'nextcloud storage contract', :storage_server_helpers, :webmock do include_context 'ModelContract shared context' # Only admins have the right to create/delete storages. @@ -289,5 +289,32 @@ it_behaves_like 'contract is invalid', username: :present, password: :present end + + describe 'provider_type_strategy' do + before do + allow(contract).to receive(:provider_type_strategy) + end + + context 'without `skip_provider_type_strategy` option' do + it 'validates the provider type contract' do + contract.validate + + expect(contract).to have_received(:provider_type_strategy) + end + end + + context 'with `skip_provider_type_strategy` option' do + let(:contract) do + described_class.new(storage, build_stubbed(:admin), + options: { skip_provider_type_strategy: true }) + end + + it 'does not validate the provider type' do + contract.validate + + expect(contract).not_to have_received(:provider_type_strategy) + end + end + end end end From 268ba1710e366d48330ed62d08b286d094bcb306 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 31 Oct 2023 20:58:51 +0300 Subject: [PATCH 12/56] feat[Op#50737]: Blueprint for select storage provider form --- ...select_storage_provider_component.html.erb | 69 +++++++++++++++++++ .../select_storage_provider_component.rb | 36 ++++++++++ .../storages/admin/storages_controller.rb | 21 +++++- .../storages/admin/storages/new.html.erb | 34 ++++++--- .../storages/storage_view_component.html.erb | 37 ++++++++++ modules/storages/config/locales/en.yml | 1 + modules/storages/config/routes.rb | 2 + 7 files changed, 188 insertions(+), 12 deletions(-) create mode 100644 modules/storages/app/components/storages/admin/select_storage_provider_component.html.erb create mode 100644 modules/storages/app/components/storages/admin/select_storage_provider_component.rb create mode 100644 modules/storages/app/views/storages/admin/storages/storage_view_component.html.erb 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 new file mode 100644 index 000000000000..9438d9bffca3 --- /dev/null +++ b/modules/storages/app/components/storages/admin/select_storage_provider_component.html.erb @@ -0,0 +1,69 @@ +<%= + render(OpTurbo::FrameComponent.new(id: :storages_storage, context: :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_provider_type_value: storage.provider_type, + 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: { + # FIXME: Auto submit on provider select + # data: { action: 'change->storages--select-provider-form#showProviderForm' }, + } + ) + ) + end + + general_info_row.with_row(mb: 3) do + render(Storages::Admin::ProviderNameInputForm.new(form)) + end + + general_info_row.with_row do + render( + Storages::Admin::SubmitOrCancelForm.new( + form, + storage:, + submit_button_options: { + # FIXME: change to disabled: true, should Auto submit on provider select + disabled: false, + 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 new file mode 100644 index 000000000000..3061a77acc67 --- /dev/null +++ b/modules/storages/app/components/storages/admin/select_storage_provider_component.rb @@ -0,0 +1,36 @@ +# 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 02a7e13aad70..8af1367e97e2 100644 --- a/modules/storages/app/controllers/storages/admin/storages_controller.rb +++ b/modules/storages/app/controllers/storages/admin/storages_controller.rb @@ -65,14 +65,29 @@ def new # Set default parameters using a "service". # See also: storages/services/storages/storages/set_attributes_services.rb # That service inherits from ::BaseServices::SetAttributes + model_class = OpenProject::FeatureDecisions.storage_primer_design_active? ? Storages::Storage : Storages::NextcloudStorage @storage = ::Storages::Storages::SetAttributesService .new(user: current_user, - model: Storages::NextcloudStorage.new, + model: model_class.new, contract_class: EmptyContract) .call .result end + def select_provider + @object = Storages::Storage.new(permitted_storage_params(:storages_storage)) + service_result = ::Storages::Storages::SetAttributesService + .new(user: current_user, + model: @object, + contract_class: Storages::Storages::BaseContract, + contract_options: { skip_provider_type_strategy: true }) + .call + @storage = service_result.result + + service_result.on_failure { render :new } + service_result.on_success { render :storage_view_component } + end + # rubocop:disable Metrics/AbcSize def create service_result = Storages::Storages::CreateService.new(user: current_user).call(permitted_storage_params) @@ -194,9 +209,9 @@ def oauth_application(service_result) # Called by create and update above in order to check if the # update parameters are correctly set. - def permitted_storage_params + def permitted_storage_params(model_parameter_name = storage_provider_parameter_name) params - .require(storage_provider_parameter_name) + .require(model_parameter_name) .permit('name', 'provider_type', 'host', 'oauth_client_id', 'oauth_client_secret', 'tenant_id', 'drive_id') 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 b575b763bc41..d8aad3ee0a97 100644 --- a/modules/storages/app/views/storages/admin/storages/new.html.erb +++ b/modules/storages/app/views/storages/admin/storages/new.html.erb @@ -1,14 +1,30 @@ <% html_title t(:label_administration), t("project_module_storages"), t('storages.label_new_storage') %> <% local_assigns[:additional_breadcrumb] = t('storages.label_new_storage') %> -<%= toolbar title: t("storages.label_new_storage") %> - -<%= labelled_tabular_form_for @storage, url: admin_settings_storages_path(@storage), as: :storages_storage do |f| -%> - <%= render partial: 'new', locals: { f: f } %> - <% if @storage.oauth_client %> - <%= styled_button_tag t(:button_save), class: "-highlight -with-icon icon-checkmark" %> - <% else %> - <%= styled_button_tag t("storages.buttons.save_and_continue_setup"), class: "-highlight -with-icon icon-checkmark" %> + +<% if OpenProject::FeatureDecisions.storage_primer_design_active? %> + <%= render(Primer::OpenProject::PageHeader.new) do |header| %> + <% header.with_title(test_selector: 'storage-name-title') do %> + <%= t("storages.label_new_storage") %> + <% end %> + + <% header.with_description do %> + <%= t("storages.instructions.new_storage") %> + <% end %> + + <% end %> + + <%= render(::Storages::Admin::SelectStorageProviderComponent.new(@storage)) %> +<% else %> + <%= toolbar title: t("storages.label_new_storage") %> + + <%= labelled_tabular_form_for @storage, url: admin_settings_storages_path(@storage), as: :storages_storage do |f| -%> + <%= render partial: 'new', locals: { f: f } %> + <% if @storage.oauth_client %> + <%= styled_button_tag t(:button_save), class: "-highlight -with-icon icon-checkmark" %> + <% else %> + <%= styled_button_tag t("storages.buttons.save_and_continue_setup"), class: "-highlight -with-icon icon-checkmark" %> + <% end %> + <%= link_to t(:button_cancel), admin_settings_storages_path, class: 'button -with-icon icon-close' %> <% end %> - <%= link_to t(:button_cancel), admin_settings_storages_path, class: 'button -with-icon icon-close' %> <% end %> diff --git a/modules/storages/app/views/storages/admin/storages/storage_view_component.html.erb b/modules/storages/app/views/storages/admin/storages/storage_view_component.html.erb new file mode 100644 index 000000000000..468a2a0e2f1f --- /dev/null +++ b/modules/storages/app/views/storages/admin/storages/storage_view_component.html.erb @@ -0,0 +1,37 @@ +<%#-- 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. + +++#%> + +<% html_title t(:label_administration), t("project_module_storages"), t('storages.label_new_storage') %> +<% local_assigns[:additional_breadcrumb] = t('storages.label_new_storage') %> + +<%= + render(OpTurbo::FrameComponent.new(id: :storages_storage, context: :select_storage_provider)) do + render(Storages::Admin::StorageViewComponent.new(@storage)) + end +%> diff --git a/modules/storages/config/locales/en.yml b/modules/storages/config/locales/en.yml index 6f335165cbf7..666e74624541 100644 --- a/modules/storages/config/locales/en.yml +++ b/modules/storages/config/locales/en.yml @@ -116,6 +116,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 file storage documentation for more information about this setup." 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." diff --git a/modules/storages/config/routes.rb b/modules/storages/config/routes.rb index 7d9c98d2978c..ac2a2e71d660 100644 --- a/modules/storages/config/routes.rb +++ b/modules/storages/config/routes.rb @@ -36,6 +36,8 @@ resource :automatically_managed_project_folders, controller: '/storages/admin/automatically_managed_project_folders', only: %i[new edit update] + post :select_provider, on: :collection + member do get '/edit_host' => '/storages/admin/storages#edit_host' delete '/replace_oauth_application' => '/storages/admin/storages#replace_oauth_application' From e70db5a043e32ba1534748ef453ba4e10f4a3de2 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 31 Oct 2023 21:20:50 +0300 Subject: [PATCH 13/56] chore[Op#50737]: Use a turbo stream to reduce HTML sent back, no need to render the full layout --- .../admin/select_storage_provider_component.html.erb | 2 +- .../controllers/storages/admin/storages_controller.rb | 5 ++++- ...nent.html.erb => select_provider.turbo_stream.erb} | 11 +++-------- 3 files changed, 8 insertions(+), 10 deletions(-) rename modules/storages/app/views/storages/admin/storages/{storage_view_component.html.erb => select_provider.turbo_stream.erb} (76%) 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 index 9438d9bffca3..b42418b31ee1 100644 --- 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 @@ -1,5 +1,5 @@ <%= - render(OpTurbo::FrameComponent.new(id: :storages_storage, context: :select_storage_provider)) do + 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') } diff --git a/modules/storages/app/controllers/storages/admin/storages_controller.rb b/modules/storages/app/controllers/storages/admin/storages_controller.rb index 8af1367e97e2..42f22d155167 100644 --- a/modules/storages/app/controllers/storages/admin/storages_controller.rb +++ b/modules/storages/app/controllers/storages/admin/storages_controller.rb @@ -85,7 +85,10 @@ def select_provider @storage = service_result.result service_result.on_failure { render :new } - service_result.on_success { render :storage_view_component } + + service_result.on_success do + respond_to { |format| format.turbo_stream } + end end # rubocop:disable Metrics/AbcSize diff --git a/modules/storages/app/views/storages/admin/storages/storage_view_component.html.erb b/modules/storages/app/views/storages/admin/storages/select_provider.turbo_stream.erb similarity index 76% rename from modules/storages/app/views/storages/admin/storages/storage_view_component.html.erb rename to modules/storages/app/views/storages/admin/storages/select_provider.turbo_stream.erb index 468a2a0e2f1f..19efd61e332a 100644 --- a/modules/storages/app/views/storages/admin/storages/storage_view_component.html.erb +++ b/modules/storages/app/views/storages/admin/storages/select_provider.turbo_stream.erb @@ -27,11 +27,6 @@ See COPYRIGHT and LICENSE files for more details. ++#%> -<% html_title t(:label_administration), t("project_module_storages"), t('storages.label_new_storage') %> -<% local_assigns[:additional_breadcrumb] = t('storages.label_new_storage') %> - -<%= - render(OpTurbo::FrameComponent.new(id: :storages_storage, context: :select_storage_provider)) do - render(Storages::Admin::StorageViewComponent.new(@storage)) - end -%> +<%= turbo_stream.update :select_storage_provider do %> + <%= render(Storages::Admin::StorageViewComponent.new(@storage)) %> +<% end %> From 7d447e5ba376b72942af9ca2067878c27b4286e8 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 1 Nov 2023 11:52:33 +0300 Subject: [PATCH 14/56] feat[Op#50737]: Decompose the StorageViewComponent --- .../admin/general_info_component.html.erb | 26 ++-- ...cation_credentials_copy_component.html.erb | 4 +- ..._application_credentials_copy_component.rb | 8 ++ .../oauth_application_info_component.html.erb | 40 ++++++ .../admin/oauth_application_info_component.rb | 44 +++++++ .../oauth_client_info_component.html.erb | 32 +++++ .../admin/oauth_client_info_component.rb | 44 +++++++ .../admin/storage_view_component.html.erb | 120 ++++++------------ .../storages/admin/storage_view_component.rb | 12 +- .../admin/storage_view_information.rb | 4 + .../storages/admin/storages_controller.rb | 48 ++++--- .../admin/oauth_clients/edit.html.erb | 2 +- .../edit.html.erb | 2 +- .../admin/storages/edit_host.html.erb | 2 +- .../admin/storages/new_oauth_client.html.erb | 2 +- .../storages/select_provider.turbo_stream.erb | 4 +- .../storages/show_oauth_application.html.erb | 2 +- .../show_oauth_application.turbo_stream.erb | 56 ++++++++ modules/storages/config/routes.rb | 5 +- 19 files changed, 339 insertions(+), 118 deletions(-) create mode 100644 modules/storages/app/components/storages/admin/oauth_application_info_component.html.erb create mode 100644 modules/storages/app/components/storages/admin/oauth_application_info_component.rb create mode 100644 modules/storages/app/components/storages/admin/oauth_client_info_component.html.erb create mode 100644 modules/storages/app/components/storages/admin/oauth_client_info_component.rb create mode 100644 modules/storages/app/views/storages/admin/storages/show_oauth_application.turbo_stream.erb diff --git a/modules/storages/app/components/storages/admin/general_info_component.html.erb b/modules/storages/app/components/storages/admin/general_info_component.html.erb index fed831a65d93..3a0d97e41729 100644 --- a/modules/storages/app/components/storages/admin/general_info_component.html.erb +++ b/modules/storages/app/components/storages/admin/general_info_component.html.erb @@ -9,19 +9,21 @@ render(Primer::Beta::Truncate.new(font_weight: :light)) { storage_description } end - grid.with_area(:"icon-button", tag: :div, color: :subtle) do - flex_layout(justify_content: :flex_end) do |icons_container| - icons_container.with_column do - render( - Primer::Beta::IconButton.new( - icon: :pencil, - tag: :a, - scheme: :invisible, - href: edit_host_admin_settings_storage_path(storage), - aria: { label: I18n.t('storages.label_edit_storage_host') } , - test_selector: 'storage-edit-host-button' + if editable_storage? + grid.with_area(:"icon-button", tag: :div, color: :subtle) do + flex_layout(justify_content: :flex_end) do |icons_container| + icons_container.with_column do + render( + Primer::Beta::IconButton.new( + icon: :pencil, + tag: :a, + scheme: :invisible, + href: edit_host_admin_settings_storage_path(storage), + aria: { label: I18n.t('storages.label_edit_storage_host') } , + test_selector: 'storage-edit-host-button' + ) ) - ) + end end end end diff --git a/modules/storages/app/components/storages/admin/oauth_application_credentials_copy_component.html.erb b/modules/storages/app/components/storages/admin/oauth_application_credentials_copy_component.html.erb index a30a2f73e5cb..b49ea02251ad 100644 --- a/modules/storages/app/components/storages/admin/oauth_application_credentials_copy_component.html.erb +++ b/modules/storages/app/components/storages/admin/oauth_application_credentials_copy_component.html.erb @@ -32,8 +32,8 @@ end credentials_row.with_row do - concat(render(Primer::Beta::Button.new(scheme: :primary, tag: :a, href: edit_admin_settings_storage_path(storage))) { I18n.t('storages.buttons.done_continue') }) - concat(render(Primer::Beta::Button.new(scheme: :default, tag: :a, href: edit_admin_settings_storage_path(storage))) { I18n.t('button_cancel') }) + concat(render(Primer::Beta::Button.new(scheme: :primary, tag: :a, href: submit_button_path)) { I18n.t('storages.buttons.done_continue') }) + concat(render(Primer::Beta::Button.new(scheme: :default, tag: :a, href: cancel_button_path)) { I18n.t('button_cancel') }) end end end diff --git a/modules/storages/app/components/storages/admin/oauth_application_credentials_copy_component.rb b/modules/storages/app/components/storages/admin/oauth_application_credentials_copy_component.rb index 8f5a9a932937..8f79a52b4569 100644 --- a/modules/storages/app/components/storages/admin/oauth_application_credentials_copy_component.rb +++ b/modules/storages/app/components/storages/admin/oauth_application_credentials_copy_component.rb @@ -49,5 +49,13 @@ def oauth_application_details_link ) ) { I18n.t('storages.instructions.oauth_application_details_link_text') } end + + def submit_button_path + options[:submit_button_path] || edit_admin_settings_storage_path(storage) + end + + def cancel_button_path + options[:cancel_button_path] || edit_admin_settings_storage_path(storage) + end end end diff --git a/modules/storages/app/components/storages/admin/oauth_application_info_component.html.erb b/modules/storages/app/components/storages/admin/oauth_application_info_component.html.erb new file mode 100644 index 000000000000..3112f3eae9cc --- /dev/null +++ b/modules/storages/app/components/storages/admin/oauth_application_info_component.html.erb @@ -0,0 +1,40 @@ +<%= + grid_layout('op-storage-view--row', tag: :div, align_items: :center) do |grid| + grid.with_area(:item, tag: :div, mr: 3) do + concat(render(Primer::Beta::Text.new(font_weight: :semibold, mr: 1, test_selector: 'storage-openproject-oauth-label')) { I18n.t('storages.file_storage_view.openproject_oauth') }) + concat(configuration_check_label_for(:openproject_oauth_application_configured)) + end + + grid.with_area(:description, tag: :div, color: :subtle, test_selector: 'storage-openproject-oauth-application-description') do + render(Primer::Beta::Truncate.new(font_weight: :light)) { openproject_oauth_client_description } + end + + if editable_storage? + grid.with_area(:"icon-button", tag: :div, color: :subtle, test_selector: 'storage-replace-openproject-oauth-application') do + flex_layout(justify_content: :flex_end) do |icons_container| + icons_container.with_column do + primer_form_with( + model: storage, + url: replace_oauth_application_admin_settings_storage_path(storage), + method: :delete + ) do |_form| + render( + Primer::Beta::IconButton.new( + icon: :sync, + scheme: :invisible, + type: :submit, + aria: { + label: I18n.t("storages.buttons.replace_provider_type_oauth", + provider_type: I18n.t("storages.provider_types.#{storage.short_provider_type}.name")) + }, + data: { confirm: I18n.t("storages.confirm_replace_oauth_application") }, + test_selector: 'storage-replace-openproject-oauth-application-button' + ) + ) + end + end + end + end + end + end +%> diff --git a/modules/storages/app/components/storages/admin/oauth_application_info_component.rb b/modules/storages/app/components/storages/admin/oauth_application_info_component.rb new file mode 100644 index 000000000000..13caa32fb210 --- /dev/null +++ b/modules/storages/app/components/storages/admin/oauth_application_info_component.rb @@ -0,0 +1,44 @@ +# 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 OAuthApplicationInfoComponent < ApplicationComponent + include OpPrimer::ComponentHelpers + include StorageViewInformation + + attr_reader :storage + alias_method :oauth_application, :model + + def initialize(oauth_application:, storage:, **options) + super(oauth_application, **options) + @storage = storage + end + end +end diff --git a/modules/storages/app/components/storages/admin/oauth_client_info_component.html.erb b/modules/storages/app/components/storages/admin/oauth_client_info_component.html.erb new file mode 100644 index 000000000000..37fa98e4e8d7 --- /dev/null +++ b/modules/storages/app/components/storages/admin/oauth_client_info_component.html.erb @@ -0,0 +1,32 @@ +<%= + grid_layout('op-storage-view--row', tag: :div, align_items: :center) do |grid| + grid.with_area(:item, tag: :div, mr: 3) do + concat(render(Primer::Beta::Text.new(font_weight: :semibold, mr: 1, test_selector: 'storage-oauth-client-label')) { I18n.t('storages.file_storage_view.nextcloud_oauth') }) + concat(configuration_check_label_for(:storage_oauth_client_configured)) + end + + grid.with_area(:description, tag: :div, color: :subtle, test_selector: 'storage-oauth-client-id-description') do + render(Primer::Beta::Truncate.new(font_weight: :light)) { provider_oauth_client_description } + end + + if editable_storage? + grid.with_area(:"icon-button", tag: :div, color: :subtle) do + flex_layout(justify_content: :flex_end) do |icons_container| + icons_container.with_column do + render( + Primer::Beta::IconButton.new( + icon: :sync, + tag: :a, + href: new_admin_settings_storage_oauth_client_path(storage), + scheme: :invisible, + aria: { label: I18n.t("storages.label_edit_storage_oauth_client") }, + data: { confirm: I18n.t("storages.confirm_replace_oauth_client") }, + test_selector: 'storage-edit-oauth-client-button' + ) + ) + end + end + end + end + end +%> diff --git a/modules/storages/app/components/storages/admin/oauth_client_info_component.rb b/modules/storages/app/components/storages/admin/oauth_client_info_component.rb new file mode 100644 index 000000000000..27f358d5e567 --- /dev/null +++ b/modules/storages/app/components/storages/admin/oauth_client_info_component.rb @@ -0,0 +1,44 @@ +# 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 OAuthClientInfoComponent < ApplicationComponent + include OpPrimer::ComponentHelpers + include StorageViewInformation + + attr_reader :storage + alias_method :oauth_client, :model + + def initialize(oauth_client:, storage:, **options) + super(oauth_client, **options) + @storage = storage + end + end +end diff --git a/modules/storages/app/components/storages/admin/storage_view_component.html.erb b/modules/storages/app/components/storages/admin/storage_view_component.html.erb index 5e633236fc0e..6607adf752f7 100644 --- a/modules/storages/app/components/storages/admin/storage_view_component.html.erb +++ b/modules/storages/app/components/storages/admin/storage_view_component.html.erb @@ -5,8 +5,17 @@ end component.with_row(scheme: :default) do - render(OpTurbo::FrameComponent.new(storage, context: :storage_view_general_info)) do - render(Storages::Admin::StorageGeneralInfoComponent.new(storage)) + render(OpTurbo::FrameComponent.new(id: :storage_general_info_section)) do + if storage.new_record? + render( + Storages::Admin::Forms::GeneralInfoFormComponent.new( + storage, + cancel_button_should_break_from_frame: true # Return to index page on Cancel + ) + ) + else + render(Storages::Admin::GeneralInfoComponent.new(storage)) + end end end @@ -19,76 +28,25 @@ end component.with_row(scheme: :default) do - render(OpTurbo::FrameComponent.new(storage, context: :openproject_oauth)) do - grid_layout('op-storage-view--row', tag: :div, align_items: :center) do |grid| - grid.with_area(:item, tag: :div, mr: 3) do - concat(render(Primer::Beta::Text.new(font_weight: :semibold, mr: 1, test_selector: 'storage-openproject-oauth-label')) { I18n.t('storages.file_storage_view.openproject_oauth') }) - concat(configuration_check_label_for(:openproject_oauth_application_configured)) - end - - grid.with_area(:description, tag: :div, color: :subtle, test_selector: 'storage-openproject-oauth-application-description') do - render(Primer::Beta::Truncate.new(font_weight: :light)) { openproject_oauth_client_description } - end - - grid.with_area(:"icon-button", tag: :div, color: :subtle, test_selector: 'storage-replace-openproject-oauth-application') do - flex_layout(justify_content: :flex_end) do |icons_container| - icons_container.with_column do - primer_form_with( - model: storage, - url: replace_oauth_application_admin_settings_storage_path(storage), - method: :delete - ) do |_form| - render( - Primer::Beta::IconButton.new( - icon: :sync, - scheme: :invisible, - type: :submit, - aria: { - label: I18n.t("storages.buttons.replace_provider_type_oauth", - provider_type: I18n.t("storages.provider_types.#{storage.short_provider_type}.name")) - }, - data: { confirm: I18n.t("storages.confirm_replace_oauth_application") }, - test_selector: 'storage-replace-openproject-oauth-application-button' - ) - ) - end - end - end - end + render(OpTurbo::FrameComponent.new(id: :storage_openproject_oauth_section)) do + if storage.new_record? || openproject_oauth_application_section_closed? + render(Storages::Admin::OAuthApplicationInfoComponent.new(oauth_application:, storage:)) + else + render( + Storages::Admin::OAuthApplicationCredentialsCopyComponent.new( + oauth_application:, + storage:, + submit_button_path: show_oauth_application_admin_settings_storage_path(storage), + cancel_button_path: admin_settings_storages_path + ) + ) end end end component.with_row(scheme: :default) do - render(OpTurbo::FrameComponent.new(storage, context: :oauth_client)) do - grid_layout('op-storage-view--row', tag: :div, align_items: :center) do |grid| - grid.with_area(:item, tag: :div, mr: 3) do - concat(render(Primer::Beta::Text.new(font_weight: :semibold, mr: 1, test_selector: 'storage-oauth-client-label')) { I18n.t('storages.file_storage_view.nextcloud_oauth') }) - concat(configuration_check_label_for(:storage_oauth_client_configured)) - end - - grid.with_area(:description, tag: :div, color: :subtle, test_selector: 'storage-oauth-client-id-description') do - render(Primer::Beta::Truncate.new(font_weight: :light)) { provider_oauth_client_description } - end - - grid.with_area(:"icon-button", tag: :div, color: :subtle) do - flex_layout(justify_content: :flex_end) do |icons_container| - icons_container.with_column do - render( - Primer::Beta::IconButton.new( - icon: :sync, - tag: :a, - href: new_admin_settings_storage_oauth_client_path(storage), - scheme: :invisible, - aria: { label: I18n.t("storages.label_edit_storage_oauth_client") }, - data: { confirm: I18n.t("storages.confirm_replace_oauth_client") }, - test_selector: 'storage-edit-oauth-client-button' - ) - ) - end - end - end - end + render(OpTurbo::FrameComponent.new(id: :storage_oauth_client_section)) do + render(Storages::Admin::OAuthClientInfoComponent.new(oauth_client: storage.oauth_client, storage:)) end end @@ -101,7 +59,7 @@ end component.with_row(scheme: :default) do - render(OpTurbo::FrameComponent.new(storage, context: :automatically_managed_project_folders)) do + render(OpTurbo::FrameComponent.new(id: :automatically_managed_project_folders_section)) do grid_layout('op-storage-view--row', tag: :div, align_items: :center) do |grid| grid.with_area(:item, tag: :div, mr: 3) do concat(render(Primer::Beta::Text.new(font_weight: :semibold, mr: 1, test_selector: 'storage-managed-project-folders-label')) { I18n.t('storages.file_storage_view.automatically_managed_folders') }) @@ -112,19 +70,21 @@ render(Primer::Beta::Truncate.new(font_weight: :light)) { I18n.t('storages.page_titles.managed_project_folders.subtitle_short') } end - grid.with_area(:"icon-button", tag: :div, color: :subtle) do - flex_layout(justify_content: :flex_end) do |icons_container| - icons_container.with_column do - render( - Primer::Beta::IconButton.new( - icon: :pencil, - tag: :a, - href: new_admin_settings_storage_automatically_managed_project_folders_path(storage), - scheme: :invisible, - aria: { label: I18n.t('storages.label_edit_storage_automatically_managed_folders') }, - test_selector: 'storage-edit-automatically-managed-project-folders-button' + if editable_storage? + grid.with_area(:"icon-button", tag: :div, color: :subtle) do + flex_layout(justify_content: :flex_end) do |icons_container| + icons_container.with_column do + render( + Primer::Beta::IconButton.new( + icon: :pencil, + tag: :a, + href: new_admin_settings_storage_automatically_managed_project_folders_path(storage), + scheme: :invisible, + aria: { label: I18n.t('storages.label_edit_storage_automatically_managed_folders') }, + test_selector: 'storage-edit-automatically-managed-project-folders-button' + ) ) - ) + end end end end diff --git a/modules/storages/app/components/storages/admin/storage_view_component.rb b/modules/storages/app/components/storages/admin/storage_view_component.rb index af50ab1c83ee..ba01ef49f06a 100644 --- a/modules/storages/app/components/storages/admin/storage_view_component.rb +++ b/modules/storages/app/components/storages/admin/storage_view_component.rb @@ -31,7 +31,17 @@ module Storages::Admin class StorageViewComponent < ApplicationComponent include OpPrimer::ComponentHelpers - alias_method :storage, :model include StorageViewInformation + + options openproject_oauth_application_section_open: false + + alias_method :storage, :model + alias_method :openproject_oauth_application_section_open?, :openproject_oauth_application_section_open + + delegate :oauth_application, to: :model + + def openproject_oauth_application_section_closed? + !openproject_oauth_application_section_open? + end end end diff --git a/modules/storages/app/components/storages/admin/storage_view_information.rb b/modules/storages/app/components/storages/admin/storage_view_information.rb index 4fd088acdb63..109b89a81557 100644 --- a/modules/storages/app/components/storages/admin/storage_view_information.rb +++ b/modules/storages/app/components/storages/admin/storage_view_information.rb @@ -4,6 +4,10 @@ module Storages::Admin module StorageViewInformation private + def editable_storage? + storage.persisted? + end + def storage_description [storage.short_provider_type.capitalize, storage.name, diff --git a/modules/storages/app/controllers/storages/admin/storages_controller.rb b/modules/storages/app/controllers/storages/admin/storages_controller.rb index 42f22d155167..a7cbc2398890 100644 --- a/modules/storages/app/controllers/storages/admin/storages_controller.rb +++ b/modules/storages/app/controllers/storages/admin/storages_controller.rb @@ -41,7 +41,8 @@ class Storages::Admin::StoragesController < ApplicationController # Before executing any action below: Make sure the current user is an admin # and set the @ variable to the object referenced in the URL. before_action :require_admin - before_action :find_model_object, only: %i[show destroy edit edit_host update replace_oauth_application] + before_action :find_model_object, + only: %i[show show_oauth_application destroy edit edit_host update replace_oauth_application] before_action :prepare_update_params, only: %i[update] # menu_item is defined in the Redmine::MenuManager::MenuController @@ -91,34 +92,51 @@ def select_provider end end - # rubocop:disable Metrics/AbcSize - def create - service_result = Storages::Storages::CreateService.new(user: current_user).call(permitted_storage_params) + def create # rubocop:disable Metrics/AbcSize + service_result = Storages::Storages::CreateService + .new(user: current_user) + .call(permitted_storage_params) @storage = service_result.result @oauth_application = oauth_application(service_result) service_result.on_failure do - render :new + if OpenProject::FeatureDecisions.storage_primer_design_active? + respond_to do |format| + format.turbo_stream { render :select_provider } + end + else + render :new + end end service_result.on_success do - case @storage.provider_type - when ::Storages::Storage::PROVIDER_TYPE_ONE_DRIVE - flash[:notice] = I18n.t(:notice_successful_create) - render '/storages/admin/storages/one_drive/edit' - when ::Storages::Storage::PROVIDER_TYPE_NEXTCLOUD - if @oauth_application.present? - flash.now[:notice] = I18n.t(:notice_successful_create) - render :show_oauth_application + if OpenProject::FeatureDecisions.storage_primer_design_active? + respond_to do |format| + format.turbo_stream { render :show_oauth_application } end else - raise "Unknown provider type: #{storage_params['provider_type']}" + case @storage.provider_type + when ::Storages::Storage::PROVIDER_TYPE_ONE_DRIVE + flash.now[:notice] = I18n.t(:notice_successful_create) + render '/storages/admin/storages/one_drive/edit' + when ::Storages::Storage::PROVIDER_TYPE_NEXTCLOUD + if @oauth_application.present? + flash.now[:notice] = I18n.t(:notice_successful_create) + render :show_oauth_application + end + else + raise "Unknown provider type: #{storage_params['provider_type']}" + end end end end - # rubocop:enable Metrics/AbcSize + def show_oauth_application + @oauth_application = @storage.oauth_application + + respond_to { |format| format.turbo_stream } + end # Edit page is very similar to new page, except that we don't need to set # default attribute values because the object already exists; diff --git a/modules/storages/app/views/storages/admin/oauth_clients/edit.html.erb b/modules/storages/app/views/storages/admin/oauth_clients/edit.html.erb index 5c5f1cd484a3..77001bf54a83 100644 --- a/modules/storages/app/views/storages/admin/oauth_clients/edit.html.erb +++ b/modules/storages/app/views/storages/admin/oauth_clients/edit.html.erb @@ -1,5 +1,5 @@ <%= - render(OpTurbo::FrameComponent.new(@storage, context: :oauth_client)) do + render(OpTurbo::FrameComponent.new(id: :storage_oauth_client_section)) do render(Storages::Admin::Forms::OAuthClientFormComponent.new(@storage)) end %> diff --git a/modules/storages/app/views/storages/admin/storages/automatically_managed_project_folders/edit.html.erb b/modules/storages/app/views/storages/admin/storages/automatically_managed_project_folders/edit.html.erb index 5efe03a9cbf2..f23b463f7d5a 100644 --- a/modules/storages/app/views/storages/admin/storages/automatically_managed_project_folders/edit.html.erb +++ b/modules/storages/app/views/storages/admin/storages/automatically_managed_project_folders/edit.html.erb @@ -31,7 +31,7 @@ See COPYRIGHT and LICENSE files for more details. <% if OpenProject::FeatureDecisions.storage_primer_design_active? %> <%= - render(OpTurbo::FrameComponent.new(@storage, context: :automatically_managed_project_folders)) do + render(OpTurbo::FrameComponent.new(id: :automatically_managed_project_folders_section)) do render(Storages::Admin::Forms::AutomaticallyManagedProjectFoldersFormComponent.new(@storage)) end %> diff --git a/modules/storages/app/views/storages/admin/storages/edit_host.html.erb b/modules/storages/app/views/storages/admin/storages/edit_host.html.erb index 26ed8b6a282c..84ca93a57406 100644 --- a/modules/storages/app/views/storages/admin/storages/edit_host.html.erb +++ b/modules/storages/app/views/storages/admin/storages/edit_host.html.erb @@ -1,5 +1,5 @@ <%= - render(OpTurbo::FrameComponent.new(@storage, context: :general_info)) do + render(OpTurbo::FrameComponent.new(id: :storage_general_info_section)) do render( Storages::Admin::Forms::GeneralInfoFormComponent.new( @storage, diff --git a/modules/storages/app/views/storages/admin/storages/new_oauth_client.html.erb b/modules/storages/app/views/storages/admin/storages/new_oauth_client.html.erb index a01eb5eeac35..2d2513fabe92 100644 --- a/modules/storages/app/views/storages/admin/storages/new_oauth_client.html.erb +++ b/modules/storages/app/views/storages/admin/storages/new_oauth_client.html.erb @@ -31,7 +31,7 @@ See COPYRIGHT and LICENSE files for more details. <% if OpenProject::FeatureDecisions.storage_primer_design_active? %> <%= - render(OpTurbo::FrameComponent.new(@storage, context: :oauth_client)) do + render(OpTurbo::FrameComponent.new(id: :storage_oauth_client_section)) do render(Storages::Admin::Forms::OAuthClientFormComponent.new(oauth_client: @oauth_client, storage: @storage)) 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 index 19efd61e332a..c55a50808861 100644 --- 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 @@ -28,5 +28,7 @@ See COPYRIGHT and LICENSE files for more details. ++#%> <%= turbo_stream.update :select_storage_provider do %> - <%= render(Storages::Admin::StorageViewComponent.new(@storage)) %> + <%= + render(Storages::Admin::StorageViewComponent.new(@storage)) + %> <% end %> diff --git a/modules/storages/app/views/storages/admin/storages/show_oauth_application.html.erb b/modules/storages/app/views/storages/admin/storages/show_oauth_application.html.erb index 3d546019ee1f..c7dcc2ba3a76 100644 --- a/modules/storages/app/views/storages/admin/storages/show_oauth_application.html.erb +++ b/modules/storages/app/views/storages/admin/storages/show_oauth_application.html.erb @@ -1,5 +1,5 @@ <% if OpenProject::FeatureDecisions.storage_primer_design_active? %> - <%= render(OpTurbo::FrameComponent.new(@storage, context: :openproject_oauth)) do %> + <%= render(OpTurbo::FrameComponent.new(id: :storage_openproject_oauth_section)) do %> <%= render(Storages::Admin::OAuthApplicationCredentialsCopyComponent.new(oauth_application: @oauth_application, storage: @storage)) %> <% end %> <% else %> diff --git a/modules/storages/app/views/storages/admin/storages/show_oauth_application.turbo_stream.erb b/modules/storages/app/views/storages/admin/storages/show_oauth_application.turbo_stream.erb new file mode 100644 index 000000000000..787e4cea8d49 --- /dev/null +++ b/modules/storages/app/views/storages/admin/storages/show_oauth_application.turbo_stream.erb @@ -0,0 +1,56 @@ +<%#-- 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 :storage_general_info_section do %> + <%= render(Storages::Admin::GeneralInfoComponent.new(@storage)) %> +<% end %> + +<%= turbo_stream.update :storage_openproject_oauth_section do %> + <%= + render( + Storages::Admin::OAuthApplicationInfoComponent.new( + oauth_application: @oauth_application, + storage: @storage, + ) + ) + %> +<% end %> + +<% if @storage.provider_type_nextcloud? %> + <%= turbo_stream.update :storage_oauth_client_section do %> + <%= + render( + Storages::Admin::Forms::OAuthClientFormComponent.new( + oauth_client: @storage.build_oauth_client, + storage: @storage + ) + ) + %> + <% end %> +<% end %> diff --git a/modules/storages/config/routes.rb b/modules/storages/config/routes.rb index ac2a2e71d660..0909bd8e9a60 100644 --- a/modules/storages/config/routes.rb +++ b/modules/storages/config/routes.rb @@ -39,8 +39,9 @@ post :select_provider, on: :collection member do - get '/edit_host' => '/storages/admin/storages#edit_host' - delete '/replace_oauth_application' => '/storages/admin/storages#replace_oauth_application' + get :show_oauth_application + get :edit_host + delete :replace_oauth_application end end end From a3010f4ca1d3da1f49e6b92781f1de399575c252 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 1 Nov 2023 15:51:44 +0300 Subject: [PATCH 15/56] chore[Op#50737]: Rename OAuthCredentialsInfo component for consistency --- ....html.erb => oauth_application_info_copy_component.html.erb} | 0 ...py_component.rb => oauth_application_info_copy_component.rb} | 2 +- .../components/storages/admin/storage_view_component.html.erb | 2 +- .../storages/admin/storages/show_oauth_application.html.erb | 2 +- .../admin/storages/show_oauth_application.turbo_stream.erb | 2 +- 5 files changed, 4 insertions(+), 4 deletions(-) rename modules/storages/app/components/storages/admin/{oauth_application_credentials_copy_component.html.erb => oauth_application_info_copy_component.html.erb} (100%) rename modules/storages/app/components/storages/admin/{oauth_application_credentials_copy_component.rb => oauth_application_info_copy_component.rb} (96%) diff --git a/modules/storages/app/components/storages/admin/oauth_application_credentials_copy_component.html.erb b/modules/storages/app/components/storages/admin/oauth_application_info_copy_component.html.erb similarity index 100% rename from modules/storages/app/components/storages/admin/oauth_application_credentials_copy_component.html.erb rename to modules/storages/app/components/storages/admin/oauth_application_info_copy_component.html.erb diff --git a/modules/storages/app/components/storages/admin/oauth_application_credentials_copy_component.rb b/modules/storages/app/components/storages/admin/oauth_application_info_copy_component.rb similarity index 96% rename from modules/storages/app/components/storages/admin/oauth_application_credentials_copy_component.rb rename to modules/storages/app/components/storages/admin/oauth_application_info_copy_component.rb index 8f79a52b4569..b84cb1257949 100644 --- a/modules/storages/app/components/storages/admin/oauth_application_credentials_copy_component.rb +++ b/modules/storages/app/components/storages/admin/oauth_application_info_copy_component.rb @@ -29,7 +29,7 @@ #++ # module Storages::Admin - class OAuthApplicationCredentialsCopyComponent < ApplicationComponent + class OAuthApplicationInfoCopyComponent < ApplicationComponent include OpPrimer::ComponentHelpers attr_reader :storage diff --git a/modules/storages/app/components/storages/admin/storage_view_component.html.erb b/modules/storages/app/components/storages/admin/storage_view_component.html.erb index 6607adf752f7..12c29cc91909 100644 --- a/modules/storages/app/components/storages/admin/storage_view_component.html.erb +++ b/modules/storages/app/components/storages/admin/storage_view_component.html.erb @@ -33,7 +33,7 @@ render(Storages::Admin::OAuthApplicationInfoComponent.new(oauth_application:, storage:)) else render( - Storages::Admin::OAuthApplicationCredentialsCopyComponent.new( + Storages::Admin::OAuthApplicationInfoCopyComponent.new( oauth_application:, storage:, submit_button_path: show_oauth_application_admin_settings_storage_path(storage), diff --git a/modules/storages/app/views/storages/admin/storages/show_oauth_application.html.erb b/modules/storages/app/views/storages/admin/storages/show_oauth_application.html.erb index c7dcc2ba3a76..1d12b121b1a4 100644 --- a/modules/storages/app/views/storages/admin/storages/show_oauth_application.html.erb +++ b/modules/storages/app/views/storages/admin/storages/show_oauth_application.html.erb @@ -1,6 +1,6 @@ <% if OpenProject::FeatureDecisions.storage_primer_design_active? %> <%= render(OpTurbo::FrameComponent.new(id: :storage_openproject_oauth_section)) do %> - <%= render(Storages::Admin::OAuthApplicationCredentialsCopyComponent.new(oauth_application: @oauth_application, storage: @storage)) %> + <%= render(Storages::Admin::OAuthApplicationInfoCopyComponent.new(oauth_application: @oauth_application, storage: @storage)) %> <% end %> <% else %> <% html_title t(:label_administration), t("project_module_storages"), @storage.name, "#{t("storages.provider_types.#{@storage.short_provider_type}.name")} #{t("storages.label_oauth_application_details")}" %> diff --git a/modules/storages/app/views/storages/admin/storages/show_oauth_application.turbo_stream.erb b/modules/storages/app/views/storages/admin/storages/show_oauth_application.turbo_stream.erb index 787e4cea8d49..b610c4bc5df9 100644 --- a/modules/storages/app/views/storages/admin/storages/show_oauth_application.turbo_stream.erb +++ b/modules/storages/app/views/storages/admin/storages/show_oauth_application.turbo_stream.erb @@ -34,7 +34,7 @@ See COPYRIGHT and LICENSE files for more details. <%= turbo_stream.update :storage_openproject_oauth_section do %> <%= render( - Storages::Admin::OAuthApplicationInfoComponent.new( + Storages::Admin::OAuthApplicationInfoCopyComponent.new( oauth_application: @oauth_application, storage: @storage, ) From f258197a497a4f0461893e666fb27289560b3889 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 1 Nov 2023 19:40:48 +0300 Subject: [PATCH 16/56] chore[Op#50737]: Amend typo --- modules/storages/config/locales/en.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/storages/config/locales/en.yml b/modules/storages/config/locales/en.yml index 666e74624541..378a636f249e 100644 --- a/modules/storages/config/locales/en.yml +++ b/modules/storages/config/locales/en.yml @@ -153,7 +153,7 @@ en: Users can nevertheless still upload files to other locations. configuration_checks: incomplete: "The setup of this storage is incomplete." - oauth_client_incomplete: "Allow OpenProject to access %{provider} data using an OAuth." + oauth_client_incomplete: "Allow OpenProject to access %{provider} data using OAuth." delete_warning: storage: > Are you sure you want to delete this storage? This will also delete the storage from all projects where it is used. From 43d460a387325e60ee8029ae14b6c8d6859ca5b3 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 1 Nov 2023 19:41:44 +0300 Subject: [PATCH 17/56] feat[Op#50737]: Continue to decompose the StorageViewComponent --- ...ed_project_folders_info_component.html.erb | 31 +++++++++++++ ..._managed_project_folders_info_component.rb | 38 ++++++++++++++++ ...ed_project_folders_form_component.html.erb | 6 +-- ..._managed_project_folders_form_component.rb | 10 +++++ ...h_application_info_copy_component.html.erb | 4 +- .../oauth_application_info_copy_component.rb | 20 +++++++++ .../admin/storage_view_component.html.erb | 30 +------------ ...ally_managed_project_folders_controller.rb | 26 ++++++++--- .../admin/oauth_clients_controller.rb | 23 +++++++--- .../oauth_clients/create.turbo_stream.erb | 34 +++++++++++++++ .../admin/oauth_clients/new.turbo_stream.erb | 43 +++++++++++++++++++ .../_form.html.erb | 32 ++++++++++++++ .../show_oauth_application.turbo_stream.erb | 21 +++------ modules/storages/config/routes.rb | 2 +- 14 files changed, 259 insertions(+), 61 deletions(-) create mode 100644 modules/storages/app/components/storages/admin/automatically_managed_project_folders_info_component.html.erb create mode 100644 modules/storages/app/components/storages/admin/automatically_managed_project_folders_info_component.rb create mode 100644 modules/storages/app/views/storages/admin/oauth_clients/create.turbo_stream.erb create mode 100644 modules/storages/app/views/storages/admin/oauth_clients/new.turbo_stream.erb create mode 100644 modules/storages/app/views/storages/admin/storages/automatically_managed_project_folders/_form.html.erb diff --git a/modules/storages/app/components/storages/admin/automatically_managed_project_folders_info_component.html.erb b/modules/storages/app/components/storages/admin/automatically_managed_project_folders_info_component.html.erb new file mode 100644 index 000000000000..ae298466de33 --- /dev/null +++ b/modules/storages/app/components/storages/admin/automatically_managed_project_folders_info_component.html.erb @@ -0,0 +1,31 @@ +<%= + grid_layout('op-storage-view--row', tag: :div, align_items: :center) do |grid| + grid.with_area(:item, tag: :div, mr: 3) do + concat(render(Primer::Beta::Text.new(font_weight: :semibold, mr: 1, test_selector: 'storage-managed-project-folders-label')) { I18n.t('storages.file_storage_view.automatically_managed_folders') }) + concat(automatically_managed_project_folders_status_label) + end + + grid.with_area(:description, tag: :div, color: :subtle, test_selector: 'storage-automatically-managed-project-folders-description') do + render(Primer::Beta::Truncate.new(font_weight: :light)) { I18n.t('storages.page_titles.managed_project_folders.subtitle_short') } + end + + if editable_storage? + grid.with_area(:"icon-button", tag: :div, color: :subtle) do + flex_layout(justify_content: :flex_end) do |icons_container| + icons_container.with_column do + render( + Primer::Beta::IconButton.new( + icon: :pencil, + tag: :a, + href: new_admin_settings_storage_automatically_managed_project_folders_path(storage), + scheme: :invisible, + aria: { label: I18n.t('storages.label_edit_storage_automatically_managed_folders') }, + test_selector: 'storage-edit-automatically-managed-project-folders-button' + ) + ) + end + end + end + end + end +%> diff --git a/modules/storages/app/components/storages/admin/automatically_managed_project_folders_info_component.rb b/modules/storages/app/components/storages/admin/automatically_managed_project_folders_info_component.rb new file mode 100644 index 000000000000..6ac63c8ab742 --- /dev/null +++ b/modules/storages/app/components/storages/admin/automatically_managed_project_folders_info_component.rb @@ -0,0 +1,38 @@ +# 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 AutomaticallyManagedProjectFoldersInfoComponent < ApplicationComponent + include OpPrimer::ComponentHelpers + include StorageViewInformation + + alias_method :storage, :model + end +end diff --git a/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.html.erb b/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.html.erb index 96a24726bfde..a69713a023c6 100644 --- a/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.html.erb +++ b/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.html.erb @@ -1,9 +1,9 @@ <%= render(Primer::Beta::Text.new(tag: :div, test_selector: 'storage-automatically-managed-project-folders-form')) do primer_form_with( - model: storage, - url: admin_settings_storage_automatically_managed_project_folders_path(storage), - method: :patch, + model:, + url: form_url, + method: form_method, data: { controller: "storages--automatically-managed-project-folders-form", 'application-target': "dynamic", diff --git a/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.rb b/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.rb index f962f4819aaf..571824be92c2 100644 --- a/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.rb +++ b/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.rb @@ -33,8 +33,18 @@ class AutomaticallyManagedProjectFoldersFormComponent < ApplicationComponent include OpPrimer::ComponentHelpers alias_method :storage, :model + options form_method: :patch + + def form_url + options[:form_url] || default_form_url + end + private + def default_form_url + admin_settings_storage_automatically_managed_project_folders_path(storage) + end + def storage_provider_credentials_copy_instructions "#{I18n.t('storages.instructions.copy_from')}: #{provider_credentials_instructions_link}".html_safe end diff --git a/modules/storages/app/components/storages/admin/oauth_application_info_copy_component.html.erb b/modules/storages/app/components/storages/admin/oauth_application_info_copy_component.html.erb index b49ea02251ad..69adf695cb78 100644 --- a/modules/storages/app/components/storages/admin/oauth_application_info_copy_component.html.erb +++ b/modules/storages/app/components/storages/admin/oauth_application_info_copy_component.html.erb @@ -32,8 +32,8 @@ end credentials_row.with_row do - concat(render(Primer::Beta::Button.new(scheme: :primary, tag: :a, href: submit_button_path)) { I18n.t('storages.buttons.done_continue') }) - concat(render(Primer::Beta::Button.new(scheme: :default, tag: :a, href: cancel_button_path)) { I18n.t('button_cancel') }) + concat(render(Primer::Beta::Button.new(**submit_button_options)) { I18n.t('storages.buttons.done_continue') }) + concat(render(Primer::Beta::Button.new(**cancel_button_options)) { I18n.t('button_cancel') }) end end end diff --git a/modules/storages/app/components/storages/admin/oauth_application_info_copy_component.rb b/modules/storages/app/components/storages/admin/oauth_application_info_copy_component.rb index b84cb1257949..7833cab7b193 100644 --- a/modules/storages/app/components/storages/admin/oauth_application_info_copy_component.rb +++ b/modules/storages/app/components/storages/admin/oauth_application_info_copy_component.rb @@ -35,6 +35,8 @@ class OAuthApplicationInfoCopyComponent < ApplicationComponent attr_reader :storage alias_method :oauth_application, :model + options cancel_button_should_break_from_frame: false + def initialize(oauth_application:, storage:, **options) super(oauth_application, **options) @storage = storage @@ -50,6 +52,24 @@ def oauth_application_details_link ) { I18n.t('storages.instructions.oauth_application_details_link_text') } end + def submit_button_options + { + scheme: :primary, + tag: :a, + href: submit_button_path + }.merge(options.fetch(:submit_button_options, {})) + end + + def cancel_button_options + { scheme: :default, + tag: :a, + href: cancel_button_path }.tap do |options_hash| + options_hash[:target] = '_top' if cancel_button_should_break_from_frame + end + end + + private + def submit_button_path options[:submit_button_path] || edit_admin_settings_storage_path(storage) end diff --git a/modules/storages/app/components/storages/admin/storage_view_component.html.erb b/modules/storages/app/components/storages/admin/storage_view_component.html.erb index 12c29cc91909..135b96f38091 100644 --- a/modules/storages/app/components/storages/admin/storage_view_component.html.erb +++ b/modules/storages/app/components/storages/admin/storage_view_component.html.erb @@ -60,35 +60,7 @@ component.with_row(scheme: :default) do render(OpTurbo::FrameComponent.new(id: :automatically_managed_project_folders_section)) do - grid_layout('op-storage-view--row', tag: :div, align_items: :center) do |grid| - grid.with_area(:item, tag: :div, mr: 3) do - concat(render(Primer::Beta::Text.new(font_weight: :semibold, mr: 1, test_selector: 'storage-managed-project-folders-label')) { I18n.t('storages.file_storage_view.automatically_managed_folders') }) - concat(automatically_managed_project_folders_status_label) - end - - grid.with_area(:description, tag: :div, color: :subtle, test_selector: 'storage-automatically-managed-project-folders-description') do - render(Primer::Beta::Truncate.new(font_weight: :light)) { I18n.t('storages.page_titles.managed_project_folders.subtitle_short') } - end - - if editable_storage? - grid.with_area(:"icon-button", tag: :div, color: :subtle) do - flex_layout(justify_content: :flex_end) do |icons_container| - icons_container.with_column do - render( - Primer::Beta::IconButton.new( - icon: :pencil, - tag: :a, - href: new_admin_settings_storage_automatically_managed_project_folders_path(storage), - scheme: :invisible, - aria: { label: I18n.t('storages.label_edit_storage_automatically_managed_folders') }, - test_selector: 'storage-edit-automatically-managed-project-folders-button' - ) - ) - end - end - end - end - end + render(Storages::Admin::AutomaticallyManagedProjectFoldersInfoComponent.new(storage)) end end end diff --git a/modules/storages/app/controllers/storages/admin/automatically_managed_project_folders_controller.rb b/modules/storages/app/controllers/storages/admin/automatically_managed_project_folders_controller.rb index fa33ed893659..84e6153ad2c7 100644 --- a/modules/storages/app/controllers/storages/admin/automatically_managed_project_folders_controller.rb +++ b/modules/storages/app/controllers/storages/admin/automatically_managed_project_folders_controller.rb @@ -41,7 +41,7 @@ class Storages::Admin::AutomaticallyManagedProjectFoldersController < Applicatio # specify which model #find_model_object should look up model_object Storages::NextcloudStorage - before_action :find_model_object, only: %i[new edit update] + before_action :find_model_object, only: %i[new create edit update] # menu_item is defined in the Redmine::MenuManager::MenuController # module, included from ApplicationController. @@ -66,6 +66,18 @@ def new render '/storages/admin/storages/automatically_managed_project_folders/edit' end + def create + service_result = call_update_service + + if service_result.success? + redirect_to edit_admin_settings_storage_path(@storage) + else + respond_to do |format| + format.turbo_stream { render partial: '/storages/admin/storages/automatically_managed_project_folders/form' } + end + end + end + # Renders an edit page (allowing the user to change automatically_managed bool and password). # Used by: The StoragesController#edit, when user wants to update application credentials. # Called by: Global app/config/routes.rb to serve Web page @@ -77,10 +89,7 @@ def edit # See also: create above # Called by: Global app/config/routes.rb to serve Web page def update - service_result = ::Storages::Storages::UpdateService - .new(user: current_user, - model: @storage) - .call(permitted_storage_params_with_defaults) + service_result = call_update_service if service_result.success? flash[:notice] = I18n.t(:notice_successful_update) @@ -113,6 +122,13 @@ def find_model_object(object_id = :storage_id) @storage = @object end + def call_update_service + ::Storages::Storages::UpdateService + .new(user: current_user, + model: @storage) + .call(permitted_storage_params_with_defaults) + end + def permitted_storage_params_with_defaults permitted_storage_params.tap do |permitted_params| # If a checkbox is unchecked when its form is submitted, neither the name nor the value is submitted to the server. diff --git a/modules/storages/app/controllers/storages/admin/oauth_clients_controller.rb b/modules/storages/app/controllers/storages/admin/oauth_clients_controller.rb index 2bb256f6610c..0c28e1bcae74 100644 --- a/modules/storages/app/controllers/storages/admin/oauth_clients_controller.rb +++ b/modules/storages/app/controllers/storages/admin/oauth_clients_controller.rb @@ -46,12 +46,17 @@ class Storages::Admin::OAuthClientsController < ApplicationController # Show the admin page to create a new OAuthClient object. def new - @oauth_client = ::OAuthClients::SetAttributesService.new(user: User.current, - model: OAuthClient.new, - contract_class: EmptyContract) - .call - .result - render '/storages/admin/storages/new_oauth_client' + @oauth_client = ::OAuthClients::SetAttributesService + .new(user: User.current, + model: OAuthClient.new, + contract_class: EmptyContract) + .call + .result + + respond_to do |format| + format.html { render '/storages/admin/storages/new_oauth_client' } + format.turbo_stream + end end # Actually create a OAuthClient object. @@ -61,6 +66,7 @@ def create # rubocop:disable Metrics/AbcSize service_result = ::OAuthClients::CreateService.new(user: User.current) .call(oauth_client_params.merge(integration: @storage)) @oauth_client = service_result.result + @storage = @storage.reload service_result.on_failure do render '/storages/admin/storages/new_oauth_client' @@ -71,7 +77,10 @@ def create # rubocop:disable Metrics/AbcSize if @storage.provider_type_nextcloud? && @storage.automatic_management_unspecified? if OpenProject::FeatureDecisions.storage_primer_design_active? - redirect_to edit_admin_settings_storage_path(@storage) + respond_to do |format| + format.html { redirect_to edit_admin_settings_storage_path(@storage) } + format.turbo_stream + end else redirect_to new_admin_settings_storage_automatically_managed_project_folders_path(@storage) end diff --git a/modules/storages/app/views/storages/admin/oauth_clients/create.turbo_stream.erb b/modules/storages/app/views/storages/admin/oauth_clients/create.turbo_stream.erb new file mode 100644 index 000000000000..d6deb913f629 --- /dev/null +++ b/modules/storages/app/views/storages/admin/oauth_clients/create.turbo_stream.erb @@ -0,0 +1,34 @@ +<%#-- 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 :storage_oauth_client_section do %> + <%= render(Storages::Admin::OAuthClientInfoComponent.new(oauth_client: @oauth_client, storage: @storage)) %> +<% end %> + +<%= render partial: 'form' %> diff --git a/modules/storages/app/views/storages/admin/oauth_clients/new.turbo_stream.erb b/modules/storages/app/views/storages/admin/oauth_clients/new.turbo_stream.erb new file mode 100644 index 000000000000..0b9c1ba7c6f8 --- /dev/null +++ b/modules/storages/app/views/storages/admin/oauth_clients/new.turbo_stream.erb @@ -0,0 +1,43 @@ +<%#-- 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 :storage_openproject_oauth_section do %> + <%= + render( + Storages::Admin::OAuthApplicationInfoComponent.new( + oauth_application: @oauth_application, + storage: @storage, + ) + ) + %> +<% end %> + +<%= turbo_stream.update :storage_oauth_client_section do %> + <%= render(Storages::Admin::Forms::OAuthClientFormComponent.new(oauth_client: @oauth_client, storage: @storage)) %> +<% end %> diff --git a/modules/storages/app/views/storages/admin/storages/automatically_managed_project_folders/_form.html.erb b/modules/storages/app/views/storages/admin/storages/automatically_managed_project_folders/_form.html.erb new file mode 100644 index 000000000000..0bcde24a4cc1 --- /dev/null +++ b/modules/storages/app/views/storages/admin/storages/automatically_managed_project_folders/_form.html.erb @@ -0,0 +1,32 @@ +<%#-- 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 :automatically_managed_project_folders_section do %> + <%= render(Storages::Admin::Forms::AutomaticallyManagedProjectFoldersFormComponent.new(@storage, form_method: :post)) %> +<% end %> diff --git a/modules/storages/app/views/storages/admin/storages/show_oauth_application.turbo_stream.erb b/modules/storages/app/views/storages/admin/storages/show_oauth_application.turbo_stream.erb index b610c4bc5df9..9bc710790bb6 100644 --- a/modules/storages/app/views/storages/admin/storages/show_oauth_application.turbo_stream.erb +++ b/modules/storages/app/views/storages/admin/storages/show_oauth_application.turbo_stream.erb @@ -27,7 +27,7 @@ See COPYRIGHT and LICENSE files for more details. ++#%> -<%= turbo_stream.update :storage_general_info_section do %> +<%= turbo_stream.update :storage_general_info_section do %> <%= render(Storages::Admin::GeneralInfoComponent.new(@storage)) %> <% end %> @@ -37,20 +37,13 @@ See COPYRIGHT and LICENSE files for more details. Storages::Admin::OAuthApplicationInfoCopyComponent.new( oauth_application: @oauth_application, storage: @storage, + submit_button_options: { + data: { turbo_stream: true } + }, + submit_button_path: new_admin_settings_storage_oauth_client_path(@storage), + cancel_button_path: admin_settings_storages_path, + cancel_button_should_break_from_frame: true ) ) %> <% end %> - -<% if @storage.provider_type_nextcloud? %> - <%= turbo_stream.update :storage_oauth_client_section do %> - <%= - render( - Storages::Admin::Forms::OAuthClientFormComponent.new( - oauth_client: @storage.build_oauth_client, - storage: @storage - ) - ) - %> - <% end %> -<% end %> diff --git a/modules/storages/config/routes.rb b/modules/storages/config/routes.rb index 0909bd8e9a60..49d29a485943 100644 --- a/modules/storages/config/routes.rb +++ b/modules/storages/config/routes.rb @@ -34,7 +34,7 @@ resources :storages, controller: '/storages/admin/storages', except: [:show] do resource :oauth_client, controller: '/storages/admin/oauth_clients', only: %i[new create] resource :automatically_managed_project_folders, controller: '/storages/admin/automatically_managed_project_folders', - only: %i[new edit update] + only: %i[new create edit update] post :select_provider, on: :collection From 3ca07caa9eb4b3b607a5eb6762d44e767105a7fd Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 2 Nov 2023 10:52:08 +0300 Subject: [PATCH 18/56] fix[Op#50737]: update automatically managed form for turbo stream --- ...utomatically_managed_project_folders_form_component.rb | 8 +++++++- .../automatically_managed_project_folders_controller.rb | 4 +--- .../create.turbo_stream.erb} | 2 +- .../storages/admin/oauth_clients/create.turbo_stream.erb | 4 +++- 4 files changed, 12 insertions(+), 6 deletions(-) rename modules/storages/app/views/storages/admin/{storages/automatically_managed_project_folders/_form.html.erb => automatically_managed_project_folders/create.turbo_stream.erb} (95%) diff --git a/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.rb b/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.rb index 571824be92c2..e8108b22f1b0 100644 --- a/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.rb +++ b/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.rb @@ -33,7 +33,9 @@ class AutomaticallyManagedProjectFoldersFormComponent < ApplicationComponent include OpPrimer::ComponentHelpers alias_method :storage, :model - options form_method: :patch + def form_method + options[:form_method] || default_form_method + end def form_url options[:form_url] || default_form_url @@ -41,6 +43,10 @@ def form_url private + def default_form_method + storage.automatic_management_unspecified? ? :post : :patch + end + def default_form_url admin_settings_storage_automatically_managed_project_folders_path(storage) end diff --git a/modules/storages/app/controllers/storages/admin/automatically_managed_project_folders_controller.rb b/modules/storages/app/controllers/storages/admin/automatically_managed_project_folders_controller.rb index 84e6153ad2c7..a85bf2f032f9 100644 --- a/modules/storages/app/controllers/storages/admin/automatically_managed_project_folders_controller.rb +++ b/modules/storages/app/controllers/storages/admin/automatically_managed_project_folders_controller.rb @@ -72,9 +72,7 @@ def create if service_result.success? redirect_to edit_admin_settings_storage_path(@storage) else - respond_to do |format| - format.turbo_stream { render partial: '/storages/admin/storages/automatically_managed_project_folders/form' } - end + respond_to { |format| format.turbo_stream } end end diff --git a/modules/storages/app/views/storages/admin/storages/automatically_managed_project_folders/_form.html.erb b/modules/storages/app/views/storages/admin/automatically_managed_project_folders/create.turbo_stream.erb similarity index 95% rename from modules/storages/app/views/storages/admin/storages/automatically_managed_project_folders/_form.html.erb rename to modules/storages/app/views/storages/admin/automatically_managed_project_folders/create.turbo_stream.erb index 0bcde24a4cc1..3a4acabc9b4c 100644 --- a/modules/storages/app/views/storages/admin/storages/automatically_managed_project_folders/_form.html.erb +++ b/modules/storages/app/views/storages/admin/automatically_managed_project_folders/create.turbo_stream.erb @@ -28,5 +28,5 @@ See COPYRIGHT and LICENSE files for more details. ++#%> <%= turbo_stream.update :automatically_managed_project_folders_section do %> - <%= render(Storages::Admin::Forms::AutomaticallyManagedProjectFoldersFormComponent.new(@storage, form_method: :post)) %> + <%= render(Storages::Admin::Forms::AutomaticallyManagedProjectFoldersFormComponent.new(@storage)) %> <% end %> diff --git a/modules/storages/app/views/storages/admin/oauth_clients/create.turbo_stream.erb b/modules/storages/app/views/storages/admin/oauth_clients/create.turbo_stream.erb index d6deb913f629..7058cb5711bb 100644 --- a/modules/storages/app/views/storages/admin/oauth_clients/create.turbo_stream.erb +++ b/modules/storages/app/views/storages/admin/oauth_clients/create.turbo_stream.erb @@ -31,4 +31,6 @@ See COPYRIGHT and LICENSE files for more details. <%= render(Storages::Admin::OAuthClientInfoComponent.new(oauth_client: @oauth_client, storage: @storage)) %> <% end %> -<%= render partial: 'form' %> +<%= turbo_stream.update :automatically_managed_project_folders_section do %> + <%= render(Storages::Admin::Forms::AutomaticallyManagedProjectFoldersFormComponent.new(@storage)) %> +<% end %> From 45001c239875ca3c9c1adfd27a0eb615ba845726 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 2 Nov 2023 11:24:38 +0300 Subject: [PATCH 19/56] fix[Op#50737]: update general info update to turbo streams --- .../admin/general_info_component.html.erb | 3 +- .../storages/admin/storages_controller.rb | 15 ++++--- .../admin/storages/edit_host.html.erb | 11 ----- .../admin/storages/edit_host.turbo_stream.erb | 40 +++++++++++++++++++ .../admin/storages/update.turbo_stream.erb | 8 ++-- 5 files changed, 56 insertions(+), 21 deletions(-) delete mode 100644 modules/storages/app/views/storages/admin/storages/edit_host.html.erb create mode 100644 modules/storages/app/views/storages/admin/storages/edit_host.turbo_stream.erb diff --git a/modules/storages/app/components/storages/admin/general_info_component.html.erb b/modules/storages/app/components/storages/admin/general_info_component.html.erb index 3a0d97e41729..0b3752c92cec 100644 --- a/modules/storages/app/components/storages/admin/general_info_component.html.erb +++ b/modules/storages/app/components/storages/admin/general_info_component.html.erb @@ -20,7 +20,8 @@ scheme: :invisible, href: edit_host_admin_settings_storage_path(storage), aria: { label: I18n.t('storages.label_edit_storage_host') } , - test_selector: 'storage-edit-host-button' + test_selector: 'storage-edit-host-button', + data: { turbo_stream: true } ) ) end diff --git a/modules/storages/app/controllers/storages/admin/storages_controller.rb b/modules/storages/app/controllers/storages/admin/storages_controller.rb index a7cbc2398890..565ed5e2bbe3 100644 --- a/modules/storages/app/controllers/storages/admin/storages_controller.rb +++ b/modules/storages/app/controllers/storages/admin/storages_controller.rb @@ -142,12 +142,15 @@ def show_oauth_application # default attribute values because the object already exists; # Called by: Global app/config/routes.rb to serve Web page def edit; end - def edit_host; end + + def edit_host + respond_to { |format| format.turbo_stream } + end # Update is similar to create above # See also: create above # Called by: Global app/config/routes.rb to serve Web page - def update + def update # rubocop:disable Metrics/AbcSize service_result = ::Storages::Storages::UpdateService .new(user: current_user, model: @storage) .call(@storage_params) @@ -155,14 +158,16 @@ def update if service_result.success? flash[:notice] = I18n.t(:notice_successful_update) + respond_to do |format| format.html { redirect_to edit_admin_settings_storage_path(@storage) } format.turbo_stream end - elsif OpenProject::FeatureDecisions.storage_primer_design_active? - render :edit_host else - render :edit + respond_to do |format| + format.html { render :edit } + format.turbo_stream { render :edit_host } + end end end diff --git a/modules/storages/app/views/storages/admin/storages/edit_host.html.erb b/modules/storages/app/views/storages/admin/storages/edit_host.html.erb deleted file mode 100644 index 84ca93a57406..000000000000 --- a/modules/storages/app/views/storages/admin/storages/edit_host.html.erb +++ /dev/null @@ -1,11 +0,0 @@ -<%= - render(OpTurbo::FrameComponent.new(id: :storage_general_info_section)) do - render( - Storages::Admin::Forms::GeneralInfoFormComponent.new( - @storage, - form_method: :patch, - cancel_button_path: edit_admin_settings_storage_path(@storage) - ) - ) - end -%> diff --git a/modules/storages/app/views/storages/admin/storages/edit_host.turbo_stream.erb b/modules/storages/app/views/storages/admin/storages/edit_host.turbo_stream.erb new file mode 100644 index 000000000000..3e37f5e0dc52 --- /dev/null +++ b/modules/storages/app/views/storages/admin/storages/edit_host.turbo_stream.erb @@ -0,0 +1,40 @@ +<%#-- 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 :storage_general_info_section do %> + <%= + render( + Storages::Admin::Forms::GeneralInfoFormComponent.new( + @storage, + form_method: :patch, + cancel_button_path: edit_admin_settings_storage_path(@storage) + ) + ) + %> +<% end %> 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 66f69f80ae95..25bcfabe222c 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,7 +1,7 @@ -<%= turbo_stream.update dom_id(@storage, :general_info) do %> - <%= render(::Storages::Admin::GeneralInfoComponent.new(@storage)) %> -<% end %> - <%= turbo_stream.update dom_id(@storage, :edit_storage_header) do %> <%= @storage.name %> <% end %> + +<%= turbo_stream.update :storage_general_info_section do %> + <%= render(::Storages::Admin::GeneralInfoComponent.new(@storage)) %> +<% end %> From d598a5c572637a677c3fcceb5b77c904ca85ad8a Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 2 Nov 2023 13:51:52 +0300 Subject: [PATCH 20/56] fix[Op#50737]: amend oauth client render policy --- .../admin/forms/oauth_client_form_component.html.erb | 2 +- .../storages/admin/forms/oauth_client_form_component.rb | 4 ++++ .../app/controllers/storages/admin/storages_controller.rb | 2 -- .../views/storages/admin/oauth_clients/new.turbo_stream.erb | 6 ++++-- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb b/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb index 00fb3978fb1e..a4a76c47af84 100644 --- a/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb +++ b/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb @@ -23,7 +23,7 @@ form, storage:, cancel_button_options: { - href: edit_admin_settings_storage_path(storage) + href: cancel_button_path } ) ) diff --git a/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.rb b/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.rb index a7fa4df6bf03..b8ab502027cf 100644 --- a/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.rb +++ b/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.rb @@ -40,6 +40,10 @@ def initialize(oauth_client:, storage:, **options) @storage = storage end + def cancel_button_path + storage.persisted? ? edit_admin_settings_storage_path(storage) : admin_settings_storages_path + end + private def storage_provider_credentials_copy_instructions diff --git a/modules/storages/app/controllers/storages/admin/storages_controller.rb b/modules/storages/app/controllers/storages/admin/storages_controller.rb index 565ed5e2bbe3..3ae95aca18d5 100644 --- a/modules/storages/app/controllers/storages/admin/storages_controller.rb +++ b/modules/storages/app/controllers/storages/admin/storages_controller.rb @@ -193,8 +193,6 @@ def replace_oauth_application if service_result.success? flash[:notice] = I18n.t('storages.notice_oauth_application_replaced') render :show_oauth_application - elsif OpenProject::FeatureDecisions.storage_primer_design_active? - render :show_oauth_application else render :edit end diff --git a/modules/storages/app/views/storages/admin/oauth_clients/new.turbo_stream.erb b/modules/storages/app/views/storages/admin/oauth_clients/new.turbo_stream.erb index 0b9c1ba7c6f8..c29aafa2e050 100644 --- a/modules/storages/app/views/storages/admin/oauth_clients/new.turbo_stream.erb +++ b/modules/storages/app/views/storages/admin/oauth_clients/new.turbo_stream.erb @@ -38,6 +38,8 @@ See COPYRIGHT and LICENSE files for more details. %> <% end %> -<%= turbo_stream.update :storage_oauth_client_section do %> - <%= render(Storages::Admin::Forms::OAuthClientFormComponent.new(oauth_client: @oauth_client, storage: @storage)) %> +<% if @storage.provider_type_nextcloud? && @storage.oauth_client.blank? %> + <%= turbo_stream.update :storage_oauth_client_section do %> + <%= render(Storages::Admin::Forms::OAuthClientFormComponent.new(oauth_client: @oauth_client, storage: @storage)) %> + <% end %> <% end %> From 5ab04a54d028d70e15f5974146da2d5a73b9178d Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 2 Nov 2023 17:16:02 +0300 Subject: [PATCH 21/56] fix[Op#50737]: redirect to list page upon automatic management completion --- ...ed_project_folders_form_component.html.erb | 8 ++----- ..._managed_project_folders_form_component.rb | 22 ++++++++++++++++++- ...ally_managed_project_folders_controller.rb | 3 ++- 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.html.erb b/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.html.erb index a69713a023c6..82d30866618a 100644 --- a/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.html.erb +++ b/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.html.erb @@ -32,12 +32,8 @@ Storages::Admin::SubmitOrCancelForm.new( form, storage:, - submit_button_options: { - label: I18n.t("storages.buttons.done_complete_setup") - }, - cancel_button_options: { - href: edit_admin_settings_storage_path(storage) - } + submit_button_options:, + cancel_button_options: ) ) end diff --git a/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.rb b/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.rb index e8108b22f1b0..ff87ef2b36d9 100644 --- a/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.rb +++ b/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.rb @@ -41,10 +41,30 @@ def form_url options[:form_url] || default_form_url end + def submit_button_options + { label: I18n.t("storages.buttons.done_complete_setup") }.tap do |options_hash| + # For create action, break from Turbo Frame and follow full page redirect + options_hash[:data] = { turbo: false } if new_record? + end + end + + def cancel_button_options + { href: edit_admin_settings_storage_path(storage) }.tap do |options_hash| + if new_record? + options_hash[:href] = admin_settings_storages_path + options_hash[:target] = '_top' + end + end + end + private def default_form_method - storage.automatic_management_unspecified? ? :post : :patch + new_record? ? :post : :patch + end + + def new_record? + storage.automatic_management_unspecified? end def default_form_url diff --git a/modules/storages/app/controllers/storages/admin/automatically_managed_project_folders_controller.rb b/modules/storages/app/controllers/storages/admin/automatically_managed_project_folders_controller.rb index a85bf2f032f9..99751e67006d 100644 --- a/modules/storages/app/controllers/storages/admin/automatically_managed_project_folders_controller.rb +++ b/modules/storages/app/controllers/storages/admin/automatically_managed_project_folders_controller.rb @@ -70,7 +70,8 @@ def create service_result = call_update_service if service_result.success? - redirect_to edit_admin_settings_storage_path(@storage) + flash[:notice] = I18n.t(:notice_successful_create) + redirect_to admin_settings_storages_path else respond_to { |format| format.turbo_stream } end From 60f9fad0703759682f23e65b519ec174cd82d6f0 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Fri, 3 Nov 2023 16:40:45 +0300 Subject: [PATCH 22/56] chore[Op#50737]: Update success message --- .../admin/automatically_managed_project_folders_controller.rb | 2 +- modules/storages/config/locales/en.yml | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/storages/app/controllers/storages/admin/automatically_managed_project_folders_controller.rb b/modules/storages/app/controllers/storages/admin/automatically_managed_project_folders_controller.rb index 99751e67006d..8185af5f51c4 100644 --- a/modules/storages/app/controllers/storages/admin/automatically_managed_project_folders_controller.rb +++ b/modules/storages/app/controllers/storages/admin/automatically_managed_project_folders_controller.rb @@ -70,7 +70,7 @@ def create service_result = call_update_service if service_result.success? - flash[:notice] = I18n.t(:notice_successful_create) + flash[:notice] = I18n.t(:'storages.notice_successful_storage_connection') redirect_to admin_settings_storages_path else respond_to { |format| format.turbo_stream } diff --git a/modules/storages/config/locales/en.yml b/modules/storages/config/locales/en.yml index 378a636f249e..b81c11034307 100644 --- a/modules/storages/config/locales/en.yml +++ b/modules/storages/config/locales/en.yml @@ -230,3 +230,4 @@ en: oauth_client_details_missing: "To complete the setup, please add OAuth client credentials from your storage." automatically_managed_project_folder_missing: "To complete the setup, please configure automatically managed project folders for your storage." notice_oauth_application_replaced: "The OpenProject OAuth application was successfully replaced." + notice_successful_storage_connection: "Storage connected successfully! Remember to activate the module and the specific storage in the project settings of each desired project to use it." From 2fc9940fbca75e3d666413e048d2334cc8ec5f2f Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Fri, 3 Nov 2023 18:07:16 +0300 Subject: [PATCH 23/56] fix[Op#50737]: mend edit vs new forms for NC automatic management settings --- ...ed_project_folders_info_component.html.erb | 5 +-- ..._managed_project_folders_info_component.rb | 8 +++++ ..._managed_project_folders_form_component.rb | 6 +++- ...ally_managed_project_folders_controller.rb | 18 ++++++++--- .../admin/oauth_clients_controller.rb | 7 ++++ .../edit.turbo_stream.erb | 32 +++++++++++++++++++ 6 files changed, 69 insertions(+), 7 deletions(-) create mode 100644 modules/storages/app/views/storages/admin/automatically_managed_project_folders/edit.turbo_stream.erb diff --git a/modules/storages/app/components/storages/admin/automatically_managed_project_folders_info_component.html.erb b/modules/storages/app/components/storages/admin/automatically_managed_project_folders_info_component.html.erb index ae298466de33..a417d0fda011 100644 --- a/modules/storages/app/components/storages/admin/automatically_managed_project_folders_info_component.html.erb +++ b/modules/storages/app/components/storages/admin/automatically_managed_project_folders_info_component.html.erb @@ -17,10 +17,11 @@ Primer::Beta::IconButton.new( icon: :pencil, tag: :a, - href: new_admin_settings_storage_automatically_managed_project_folders_path(storage), + href: edit_button_path, scheme: :invisible, aria: { label: I18n.t('storages.label_edit_storage_automatically_managed_folders') }, - test_selector: 'storage-edit-automatically-managed-project-folders-button' + test_selector: 'storage-edit-automatically-managed-project-folders-button', + data: { turbo_stream: true } ) ) end diff --git a/modules/storages/app/components/storages/admin/automatically_managed_project_folders_info_component.rb b/modules/storages/app/components/storages/admin/automatically_managed_project_folders_info_component.rb index 6ac63c8ab742..cc1e83256b2b 100644 --- a/modules/storages/app/components/storages/admin/automatically_managed_project_folders_info_component.rb +++ b/modules/storages/app/components/storages/admin/automatically_managed_project_folders_info_component.rb @@ -34,5 +34,13 @@ class AutomaticallyManagedProjectFoldersInfoComponent < ApplicationComponent include StorageViewInformation alias_method :storage, :model + + def edit_button_path + if storage.automatic_management_unspecified? + new_admin_settings_storage_automatically_managed_project_folders_path(storage) + else + edit_admin_settings_storage_automatically_managed_project_folders_path(storage) + end + end end end diff --git a/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.rb b/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.rb index ff87ef2b36d9..841aac433853 100644 --- a/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.rb +++ b/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.rb @@ -64,7 +64,11 @@ def default_form_method end def new_record? - storage.automatic_management_unspecified? + if (provider_fields_changes_to_be_saved = storage.provider_fields_change_to_be_saved) + provider_fields_changes_to_be_saved.first["automatically_managed"].nil? + else + storage.automatic_management_unspecified? + end end def default_form_url diff --git a/modules/storages/app/controllers/storages/admin/automatically_managed_project_folders_controller.rb b/modules/storages/app/controllers/storages/admin/automatically_managed_project_folders_controller.rb index 8185af5f51c4..08c6ccabbbcd 100644 --- a/modules/storages/app/controllers/storages/admin/automatically_managed_project_folders_controller.rb +++ b/modules/storages/app/controllers/storages/admin/automatically_managed_project_folders_controller.rb @@ -60,10 +60,14 @@ def new @storage = ::Storages::Storages::SetNextcloudProviderFieldsAttributesService .new(user: current_user, model: @object, - contract_class: ::Storages::Storages::BaseContract) + contract_class: EmptyContract) .call .result - render '/storages/admin/storages/automatically_managed_project_folders/edit' + + respond_to do |format| + format.html { render '/storages/admin/storages/automatically_managed_project_folders/edit' } + format.turbo_stream { render :edit } + end end def create @@ -73,7 +77,10 @@ def create flash[:notice] = I18n.t(:'storages.notice_successful_storage_connection') redirect_to admin_settings_storages_path else - respond_to { |format| format.turbo_stream } + respond_to do |format| + format.html { render '/storages/admin/storages/automatically_managed_project_folders/edit' } + format.turbo_stream + end end end @@ -81,7 +88,10 @@ def create # Used by: The StoragesController#edit, when user wants to update application credentials. # Called by: Global app/config/routes.rb to serve Web page def edit - render '/storages/admin/storages/automatically_managed_project_folders/edit' + respond_to do |format| + format.html { render '/storages/admin/storages/automatically_managed_project_folders/edit' } + format.turbo_stream + end end # Update is similar to create above diff --git a/modules/storages/app/controllers/storages/admin/oauth_clients_controller.rb b/modules/storages/app/controllers/storages/admin/oauth_clients_controller.rb index 0c28e1bcae74..c92bea8cfeb6 100644 --- a/modules/storages/app/controllers/storages/admin/oauth_clients_controller.rb +++ b/modules/storages/app/controllers/storages/admin/oauth_clients_controller.rb @@ -77,6 +77,13 @@ def create # rubocop:disable Metrics/AbcSize if @storage.provider_type_nextcloud? && @storage.automatic_management_unspecified? if OpenProject::FeatureDecisions.storage_primer_design_active? + @storage = ::Storages::Storages::SetNextcloudProviderFieldsAttributesService + .new(user: current_user, + model: @storage, + contract_class: EmptyContract) + .call + .result + respond_to do |format| format.html { redirect_to edit_admin_settings_storage_path(@storage) } format.turbo_stream diff --git a/modules/storages/app/views/storages/admin/automatically_managed_project_folders/edit.turbo_stream.erb b/modules/storages/app/views/storages/admin/automatically_managed_project_folders/edit.turbo_stream.erb new file mode 100644 index 000000000000..3a4acabc9b4c --- /dev/null +++ b/modules/storages/app/views/storages/admin/automatically_managed_project_folders/edit.turbo_stream.erb @@ -0,0 +1,32 @@ +<%#-- 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 :automatically_managed_project_folders_section do %> + <%= render(Storages::Admin::Forms::AutomaticallyManagedProjectFoldersFormComponent.new(@storage)) %> +<% end %> From 78fd5a7924160497ae517f5012050f452bc5f61a Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Fri, 3 Nov 2023 18:14:28 +0300 Subject: [PATCH 24/56] fix[Op#50737]: Remove name from select provider form, add prompt --- .../admin/select_storage_provider_component.html.erb | 5 +---- modules/storages/config/locales/en.yml | 1 + 2 files changed, 2 insertions(+), 4 deletions(-) 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 index b42418b31ee1..9cc8e8557f4d 100644 --- 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 @@ -33,15 +33,12 @@ select_list_options: { # FIXME: Auto submit on provider select # data: { action: 'change->storages--select-provider-form#showProviderForm' }, + include_blank: I18n.t('storages.label_select_provider'), } ) ) end - general_info_row.with_row(mb: 3) do - render(Storages::Admin::ProviderNameInputForm.new(form)) - end - general_info_row.with_row do render( Storages::Admin::SubmitOrCancelForm.new( diff --git a/modules/storages/config/locales/en.yml b/modules/storages/config/locales/en.yml index b81c11034307..dca67012f55e 100644 --- a/modules/storages/config/locales/en.yml +++ b/modules/storages/config/locales/en.yml @@ -207,6 +207,7 @@ en: label_no_specific_folder: "No specific folder" label_automatic_folder: "New folder with automatically managed permissions" label_no_selected_folder: "No selected folder" + label_select_provider: "Select provider" label_storage: "Storage" label_storages: "Storages" label_status: "Status" From 98cec86c0a7712fba6813715a2af95fa2adf249b Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Fri, 3 Nov 2023 21:20:53 +0300 Subject: [PATCH 25/56] feat[Op#50737]: Introduce OneDrive general info form --- .../general_info_form_component.html.erb | 12 +++- .../forms/general_info_form_component.rb | 3 - .../admin/storage_view_component.html.erb | 62 ++++++++++--------- .../storages/admin/storages_controller.rb | 7 +-- .../admin/provider_drive_id_input_form.rb | 42 +++++++++++++ .../admin/provider_name_input_form.rb | 3 +- .../admin/provider_tenant_id_input_form.rb | 42 +++++++++++++ 7 files changed, 131 insertions(+), 40 deletions(-) create mode 100644 modules/storages/app/forms/storages/admin/provider_drive_id_input_form.rb create mode 100644 modules/storages/app/forms/storages/admin/provider_tenant_id_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 dcfb25f133ee..14d38c476955 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 @@ -14,12 +14,22 @@ render(Storages::Admin::ProviderNameInputForm.new(form)) end - if render_host? + if storage.provider_type_nextcloud? general_info_row.with_row(mb: 3) do render(Storages::Admin::ProviderHostInputForm.new(form)) end end + if storage.provider_type_one_drive? + general_info_row.with_row(mb: 3) do + render(Storages::Admin::ProviderTenantIdInputForm.new(form)) + end + + general_info_row.with_row(mb: 3) do + render(Storages::Admin::ProviderDriveIdInputForm.new(form)) + end + end + general_info_row.with_row do render( Storages::Admin::SubmitOrCancelForm.new( 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 3a68fae1a7a9..7644214c9ad1 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 @@ -34,13 +34,10 @@ class GeneralInfoFormComponent < ApplicationComponent alias_method :storage, :model options form_method: :post, - render_host: true, submit_button_disabled: false, cancel_button_should_break_from_frame: false, cancel_button_path: Rails.application.routes.url_helpers.admin_settings_storages_path - alias_method :render_host?, :render_host - def form_url options[:form_url] || default_form_url end diff --git a/modules/storages/app/components/storages/admin/storage_view_component.html.erb b/modules/storages/app/components/storages/admin/storage_view_component.html.erb index 135b96f38091..cf405e618d71 100644 --- a/modules/storages/app/components/storages/admin/storage_view_component.html.erb +++ b/modules/storages/app/components/storages/admin/storage_view_component.html.erb @@ -19,48 +19,50 @@ end end - component.with_row(scheme: :neutral) do - grid_layout('op-storage-view--row', tag: :div, align_items: :center) do |grid| - grid.with_area(:item, tag: :div, mr: 3) do - render(Primer::Beta::Text.new(font_weight: :semibold, mr: 1)) { I18n.t('storages.file_storage_view.oauth_applications') } + if storage.provider_type_nextcloud? + component.with_row(scheme: :neutral) do + grid_layout('op-storage-view--row', tag: :div, align_items: :center) do |grid| + grid.with_area(:item, tag: :div, mr: 3) do + render(Primer::Beta::Text.new(font_weight: :semibold, mr: 1)) { I18n.t('storages.file_storage_view.oauth_applications') } + end end end - end - component.with_row(scheme: :default) do - render(OpTurbo::FrameComponent.new(id: :storage_openproject_oauth_section)) do - if storage.new_record? || openproject_oauth_application_section_closed? - render(Storages::Admin::OAuthApplicationInfoComponent.new(oauth_application:, storage:)) - else - render( - Storages::Admin::OAuthApplicationInfoCopyComponent.new( - oauth_application:, - storage:, - submit_button_path: show_oauth_application_admin_settings_storage_path(storage), - cancel_button_path: admin_settings_storages_path + component.with_row(scheme: :default) do + render(OpTurbo::FrameComponent.new(id: :storage_openproject_oauth_section)) do + if storage.new_record? || openproject_oauth_application_section_closed? + render(Storages::Admin::OAuthApplicationInfoComponent.new(oauth_application:, storage:)) + else + render( + Storages::Admin::OAuthApplicationInfoCopyComponent.new( + oauth_application:, + storage:, + submit_button_path: show_oauth_application_admin_settings_storage_path(storage), + cancel_button_path: admin_settings_storages_path + ) ) - ) + end end end - end - component.with_row(scheme: :default) do - render(OpTurbo::FrameComponent.new(id: :storage_oauth_client_section)) do - render(Storages::Admin::OAuthClientInfoComponent.new(oauth_client: storage.oauth_client, storage:)) + component.with_row(scheme: :default) do + render(OpTurbo::FrameComponent.new(id: :storage_oauth_client_section)) do + render(Storages::Admin::OAuthClientInfoComponent.new(oauth_client: storage.oauth_client, storage:)) + end end - end - component.with_row(scheme: :neutral) do - grid_layout('op-storage-view--row', tag: :div, align_items: :center) do |grid| - grid.with_area(:item, tag: :div) do - render(Primer::Beta::Text.new(font_weight: :semibold, mr: 1)) { I18n.t('storages.file_storage_view.project_folders') } + component.with_row(scheme: :neutral) do + grid_layout('op-storage-view--row', tag: :div, align_items: :center) do |grid| + grid.with_area(:item, tag: :div) do + render(Primer::Beta::Text.new(font_weight: :semibold, mr: 1)) { I18n.t('storages.file_storage_view.project_folders') } + end end end - end - component.with_row(scheme: :default) do - render(OpTurbo::FrameComponent.new(id: :automatically_managed_project_folders_section)) do - render(Storages::Admin::AutomaticallyManagedProjectFoldersInfoComponent.new(storage)) + component.with_row(scheme: :default) do + render(OpTurbo::FrameComponent.new(id: :automatically_managed_project_folders_section)) do + render(Storages::Admin::AutomaticallyManagedProjectFoldersInfoComponent.new(storage)) + end 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 3ae95aca18d5..c2c125f94420 100644 --- a/modules/storages/app/controllers/storages/admin/storages_controller.rb +++ b/modules/storages/app/controllers/storages/admin/storages_controller.rb @@ -239,14 +239,11 @@ def permitted_storage_params(model_parameter_name = storage_provider_parameter_n .permit('name', 'provider_type', 'host', 'oauth_client_id', 'oauth_client_secret', 'tenant_id', 'drive_id') end - # TODO: Work out how to retrieve the storage provider resource name as it's based on the provider type - # PrimerForms implements Rails `form_with` which doesn't support overriding the form name as we would with - # `form_for`. - # See: https://github.com/opf/primer_view_components/blob/79fb58474771bd06946554f8325cd0b1bdd6dd31/app/helpers/primer/form_helper.rb#L7 - # def storage_provider_parameter_name if params.key?(:storages_nextcloud_storage) :storages_nextcloud_storage + elsif params.key?(:storages_one_drive_storage) + :storages_one_drive_storage else :storages_storage end diff --git a/modules/storages/app/forms/storages/admin/provider_drive_id_input_form.rb b/modules/storages/app/forms/storages/admin/provider_drive_id_input_form.rb new file mode 100644 index 000000000000..126fc7f90a23 --- /dev/null +++ b/modules/storages/app/forms/storages/admin/provider_drive_id_input_form.rb @@ -0,0 +1,42 @@ +#-- 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 ProviderDriveIdInputForm < ApplicationForm + form do |storage_form| + storage_form.text_field( + name: :drive_id, + label: Storages::Admin::LABEL_DRIVE_ID, + visually_hide_label: false, + required: true, + caption: I18n.t("storages.instructions.one_drive.drive_id"), + placeholder: I18n.t("storages.instructions.one_drive.drive_id_placeholder") + ) + end + end +end diff --git a/modules/storages/app/forms/storages/admin/provider_name_input_form.rb b/modules/storages/app/forms/storages/admin/provider_name_input_form.rb index f8ba9f368872..a963517e713d 100644 --- a/modules/storages/app/forms/storages/admin/provider_name_input_form.rb +++ b/modules/storages/app/forms/storages/admin/provider_name_input_form.rb @@ -33,7 +33,8 @@ class ProviderNameInputForm < ApplicationForm name: :name, label: I18n.t('activerecord.attributes.storages/storage.name'), required: true, - caption: I18n.t('storages.instructions.name') + caption: I18n.t('storages.instructions.name'), + placeholder: I18n.t("storages.label_file_storage") ) end end diff --git a/modules/storages/app/forms/storages/admin/provider_tenant_id_input_form.rb b/modules/storages/app/forms/storages/admin/provider_tenant_id_input_form.rb new file mode 100644 index 000000000000..91296346b197 --- /dev/null +++ b/modules/storages/app/forms/storages/admin/provider_tenant_id_input_form.rb @@ -0,0 +1,42 @@ +#-- 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 ProviderTenantIdInputForm < ApplicationForm + form do |storage_form| + storage_form.text_field( + name: :tenant_id, + label: I18n.t('activerecord.attributes.storages/storage.tenant'), + visually_hide_label: false, + required: true, + caption: I18n.t("storages.instructions.one_drive.tenant_id"), + placeholder: I18n.t("storages.instructions.one_drive.tenant_id_placeholder") + ) + end + end +end From 444f1bef254d122f98f1c0313a3d766387d08ef4 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Fri, 3 Nov 2023 21:21:43 +0300 Subject: [PATCH 26/56] chore[Op#50737]: Update storage header and title to "New file storage" --- .../app/views/storages/admin/storages/new.html.erb | 6 +++--- modules/storages/config/locales/en.yml | 11 +++++++---- 2 files changed, 10 insertions(+), 7 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 d8aad3ee0a97..ed8f2b0d041a 100644 --- a/modules/storages/app/views/storages/admin/storages/new.html.erb +++ b/modules/storages/app/views/storages/admin/storages/new.html.erb @@ -1,11 +1,11 @@ -<% html_title t(:label_administration), t("project_module_storages"), t('storages.label_new_storage') %> -<% local_assigns[:additional_breadcrumb] = t('storages.label_new_storage') %> +<% 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') %> <% if OpenProject::FeatureDecisions.storage_primer_design_active? %> <%= render(Primer::OpenProject::PageHeader.new) do |header| %> <% header.with_title(test_selector: 'storage-name-title') do %> - <%= t("storages.label_new_storage") %> + <%= t("storages.label_new_file_storage") %> <% end %> <% header.with_description do %> diff --git a/modules/storages/config/locales/en.yml b/modules/storages/config/locales/en.yml index dca67012f55e..842850af44a3 100644 --- a/modules/storages/config/locales/en.yml +++ b/modules/storages/config/locales/en.yml @@ -30,6 +30,7 @@ en: creator: "Creator" provider_type: "Provider type" host: "Host" + tenant: "Tenant" storages/file_link: origin_id: "Origin Id" @@ -143,10 +144,10 @@ en: Copy the client secret from the Azure portal. For a newly created application the secret first needs to be created manually. For authorization of web applications a secret is mandatory. missing_client_id_for_redirect_uri: "Client ID missing to provide redirect URI." - tenant_id: > - Please insert the tenant ID you got from your SharePoint administrator. - drive_id: > - The drive ID can be obtained by you SharePoint administrator. + tenant_id: "Please copy the tenant from your Azure application." + tenant_id_placeholder: "Name or UUID" + drive_id: "Please copy the drive ID from your Azure application." + drive_id_placeholder: "UUID or triple ID" help_texts: project_folder: > The project folder is the default folder for file uploads for this project. @@ -176,6 +177,7 @@ en: label_provider: "Provider" label_file_link: "File link" label_file_links: "File links" + label_file_storage: "File storage" label_creation_time: "Creation time" label_connected: "Connected" label_incomplete: "Incomplete" @@ -199,6 +201,7 @@ en: label_provider_type: "Provider type" label_project_folder: "Project folder" label_new_storage: "New storage" + label_new_file_storage: "New file storage" label_edit_storage: "Edit storage" label_edit_storage_host: "Edit storage host" label_edit_storage_oauth_client: "Edit storage OAuth client" From 57e43711472435007cf3961af49e5891417ae81b Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Mon, 6 Nov 2023 18:32:53 +0300 Subject: [PATCH 27/56] fix[Op#50737]: Remove unused form closing method check --- .../storages/admin/storage_view_component.html.erb | 2 +- .../components/storages/admin/storage_view_component.rb | 7 ------- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/modules/storages/app/components/storages/admin/storage_view_component.html.erb b/modules/storages/app/components/storages/admin/storage_view_component.html.erb index cf405e618d71..5f256409e4c5 100644 --- a/modules/storages/app/components/storages/admin/storage_view_component.html.erb +++ b/modules/storages/app/components/storages/admin/storage_view_component.html.erb @@ -30,7 +30,7 @@ component.with_row(scheme: :default) do render(OpTurbo::FrameComponent.new(id: :storage_openproject_oauth_section)) do - if storage.new_record? || openproject_oauth_application_section_closed? + if storage.new_record? render(Storages::Admin::OAuthApplicationInfoComponent.new(oauth_application:, storage:)) else render( diff --git a/modules/storages/app/components/storages/admin/storage_view_component.rb b/modules/storages/app/components/storages/admin/storage_view_component.rb index ba01ef49f06a..e60cb3cad44a 100644 --- a/modules/storages/app/components/storages/admin/storage_view_component.rb +++ b/modules/storages/app/components/storages/admin/storage_view_component.rb @@ -33,15 +33,8 @@ class StorageViewComponent < ApplicationComponent include OpPrimer::ComponentHelpers include StorageViewInformation - options openproject_oauth_application_section_open: false - alias_method :storage, :model - alias_method :openproject_oauth_application_section_open?, :openproject_oauth_application_section_open delegate :oauth_application, to: :model - - def openproject_oauth_application_section_closed? - !openproject_oauth_application_section_open? - end end end From 048d1fd06fd4f0f11eb3c3aad86b71c5034309bf Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 7 Nov 2023 12:10:46 +0300 Subject: [PATCH 28/56] chore[Op#50737]: fix controller name --- .../dynamic/storages/oauth-client-form.controller.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/stimulus/controllers/dynamic/storages/oauth-client-form.controller.ts b/frontend/src/stimulus/controllers/dynamic/storages/oauth-client-form.controller.ts index 3bd334907638..d203493adcbd 100644 --- a/frontend/src/stimulus/controllers/dynamic/storages/oauth-client-form.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/storages/oauth-client-form.controller.ts @@ -30,7 +30,7 @@ import { Controller } from '@hotwired/stimulus'; -export default class StorageFormController extends Controller { +export default class OAuthClientFormController extends Controller { static targets = [ 'clientId', 'redirectUri', From ba09d4145d4eab488d86730eab0512776e66a953 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 7 Nov 2023 12:11:22 +0300 Subject: [PATCH 29/56] chore[Op#50737]: rm unused edit template --- .../app/views/storages/admin/oauth_clients/edit.html.erb | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 modules/storages/app/views/storages/admin/oauth_clients/edit.html.erb diff --git a/modules/storages/app/views/storages/admin/oauth_clients/edit.html.erb b/modules/storages/app/views/storages/admin/oauth_clients/edit.html.erb deleted file mode 100644 index 77001bf54a83..000000000000 --- a/modules/storages/app/views/storages/admin/oauth_clients/edit.html.erb +++ /dev/null @@ -1,5 +0,0 @@ -<%= - render(OpTurbo::FrameComponent.new(id: :storage_oauth_client_section)) do - render(Storages::Admin::Forms::OAuthClientFormComponent.new(@storage)) - end -%> From c97191540e22d2065f8091bdc3f90aa1f8db2593 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 7 Nov 2023 12:12:18 +0300 Subject: [PATCH 30/56] feat[Op#50737]: Extend clipboard copy component to allow input group options --- .../open_project/common/clipboard_copy_component.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/components/open_project/common/clipboard_copy_component.rb b/app/components/open_project/common/clipboard_copy_component.rb index c00b75183337..c186345acf98 100644 --- a/app/components/open_project/common/clipboard_copy_component.rb +++ b/app/components/open_project/common/clipboard_copy_component.rb @@ -34,7 +34,8 @@ class ClipboardCopyComponent < ApplicationComponent options visually_hide_label: true, readonly: true, - required: false + required: false, + input_group_options: {} def text_field_options { name: options[:name], @@ -44,13 +45,13 @@ def text_field_options value: value_to_copy, inset: true, readonly:, - required: } + required: }.merge(input_group_options) end def clipboard_copy_options { value: value_to_copy, aria: { label: clipboard_copy_aria_label }, - classes: clipboard_copy_classes } + classes: clipboard_copy_classes }.merge(input_group_options) end private From 9795321d99dfefec54335a22c6eac21a5e592cdd Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 7 Nov 2023 12:15:47 +0300 Subject: [PATCH 31/56] feat[Op#50737]: Extend OAuthClient components to account for OneDrive storage --- .../storages/oauth-client-form.controller.ts | 22 +++++++ .../oauth_client_form_component.html.erb | 51 +++++++++++++++- .../forms/oauth_client_form_component.rb | 18 +++++- .../oauth_client_info_component.html.erb | 6 +- .../admin/storage_view_component.html.erb | 16 +++++ .../admin/storage_view_information.rb | 2 +- .../storages/admin/storages_controller.rb | 4 +- .../forms/storages/admin/oauth_client_form.rb | 29 +++++++--- .../admin/storages/create.turbo_stream.erb | 58 +++++++++++++++++++ modules/storages/config/locales/en.yml | 8 ++- 10 files changed, 196 insertions(+), 18 deletions(-) create mode 100644 modules/storages/app/views/storages/admin/storages/create.turbo_stream.erb diff --git a/frontend/src/stimulus/controllers/dynamic/storages/oauth-client-form.controller.ts b/frontend/src/stimulus/controllers/dynamic/storages/oauth-client-form.controller.ts index d203493adcbd..e1e82369c31a 100644 --- a/frontend/src/stimulus/controllers/dynamic/storages/oauth-client-form.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/storages/oauth-client-form.controller.ts @@ -33,15 +33,22 @@ import { Controller } from '@hotwired/stimulus'; export default class OAuthClientFormController extends Controller { static targets = [ 'clientId', + 'clientSecret', 'redirectUri', + 'submitButton', ]; static values = { clientIdMissing: String, }; + declare readonly hasClientIdTarget:boolean; + declare readonly hasClientSecretTarget:boolean; + declare readonly hasSubmitButtonTarget:boolean; declare readonly clientIdTarget:HTMLInputElement; + declare readonly clientSecretTarget:HTMLInputElement; declare readonly redirectUriTarget:HTMLInputElement; + declare readonly submitButtonTarget:HTMLInputElement; declare readonly clientIdMissingValue:string; @@ -51,6 +58,21 @@ export default class OAuthClientFormController extends Controller { }); this.redirectUriTarget.value = this.redirectUri(); + this.toggleSubmitButtonDisabled(); + } + + public toggleSubmitButtonDisabled():void { + const targetsConfigured = this.hasClientIdTarget && this.hasClientSecretTarget && this.hasSubmitButtonTarget; + + if (!targetsConfigured) { + return; + } + + if (this.clientIdTarget.value === '' || this.clientSecretTarget.value === '') { + this.submitButtonTarget.disabled = true; + } else { + this.submitButtonTarget.disabled = false; + } } private redirectUri():string { diff --git a/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb b/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb index a4a76c47af84..65105c6de02a 100644 --- a/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb +++ b/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb @@ -2,7 +2,12 @@ render(Primer::Beta::Text.new(tag: :div, test_selector: 'storage-oauth-client-form')) do primer_form_with( model: oauth_client, - url: admin_settings_storage_oauth_client_path(storage) + url: admin_settings_storage_oauth_client_path(storage), + data: { + controller: 'storages--oauth-client-form', + application_target: 'dynamic', + 'storages--oauth-client-form-client-id-missing-value': t(:"storages.instructions.#{storage.short_provider_type}.missing_client_id_for_redirect_uri") + } ) do |form| flex_layout do |oauth_client_row| oauth_client_row.with_row(mb: 3) do @@ -14,7 +19,43 @@ end oauth_client_row.with_row(mb: 3) do - render(Storages::Admin::OAuthClientForm.new(form, storage:)) + render( + Storages::Admin::OAuthClientForm.new( + form, + storage:, + client_id_input_options: { + data: { + 'storages--oauth-client-form-target': 'clientId', + action: 'input->storages--oauth-client-form#toggleSubmitButtonDisabled' + } + }, + client_secret_input_options: { + data: { + 'storages--oauth-client-form-target': 'clientSecret', + action: 'input->storages--oauth-client-form#toggleSubmitButtonDisabled' + } + } + ) + ) + end + + if storage.provider_type_one_drive? + oauth_client_row.with_row(mb: 3) do + render( + OpenProject::Common::ClipboardCopyComponent.new( + name: :oauth_client_redirect_uri, + visually_hide_label: false, + value_to_copy: redirect_uri_or_instructions, + label: I18n.t('storages.label_redirect_uri'), + required: true, + input_group_options: { + data: { + 'storages--oauth-client-form-target': 'redirectUri' + } + }, + ) + ) + end end oauth_client_row.with_row do @@ -22,6 +63,12 @@ Storages::Admin::SubmitOrCancelForm.new( form, storage:, + submit_button_options: { + disabled: submit_button_disabled?, + data: { + 'storages--oauth-client-form-target': 'submitButton' + } + }, cancel_button_options: { href: cancel_button_path } diff --git a/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.rb b/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.rb index b8ab502027cf..682c6a3bc870 100644 --- a/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.rb +++ b/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.rb @@ -44,7 +44,17 @@ def cancel_button_path storage.persisted? ? edit_admin_settings_storage_path(storage) : admin_settings_storages_path end - private + def submit_button_disabled? + !storage_oauth_client_configured? + end + + def redirect_uri_or_instructions + if storage_oauth_client_configured? + oauth_client.redirect_uri + else + I18n.t("storages.instructions.one_drive.missing_client_id_for_redirect_uri") + end + end def storage_provider_credentials_copy_instructions "#{I18n.t('storages.instructions.copy_from')}: #{provider_credentials_instructions_link}".html_safe @@ -59,5 +69,11 @@ def provider_credentials_instructions_link ) ) { I18n.t("storages.instructions.#{@storage.short_provider_type}.integration") } end + + private + + def storage_oauth_client_configured? + oauth_client.present? && oauth_client.client_id.present? && oauth_client.client_secret.present? + end end end diff --git a/modules/storages/app/components/storages/admin/oauth_client_info_component.html.erb b/modules/storages/app/components/storages/admin/oauth_client_info_component.html.erb index 37fa98e4e8d7..90251d0be5b8 100644 --- a/modules/storages/app/components/storages/admin/oauth_client_info_component.html.erb +++ b/modules/storages/app/components/storages/admin/oauth_client_info_component.html.erb @@ -1,7 +1,11 @@ <%= grid_layout('op-storage-view--row', tag: :div, align_items: :center) do |grid| grid.with_area(:item, tag: :div, mr: 3) do - concat(render(Primer::Beta::Text.new(font_weight: :semibold, mr: 1, test_selector: 'storage-oauth-client-label')) { I18n.t('storages.file_storage_view.nextcloud_oauth') }) + concat( + render( + Primer::Beta::Text.new(font_weight: :semibold, mr: 1, test_selector: 'storage-oauth-client-label') + ) { I18n.t("storages.file_storage_view.#{storage.short_provider_type}_oauth") } + ) concat(configuration_check_label_for(:storage_oauth_client_configured)) end diff --git a/modules/storages/app/components/storages/admin/storage_view_component.html.erb b/modules/storages/app/components/storages/admin/storage_view_component.html.erb index 5f256409e4c5..088a1b9f099e 100644 --- a/modules/storages/app/components/storages/admin/storage_view_component.html.erb +++ b/modules/storages/app/components/storages/admin/storage_view_component.html.erb @@ -65,5 +65,21 @@ end end end + + if storage.provider_type_one_drive? + component.with_row(scheme: :neutral) do + grid_layout('op-storage-view--row', tag: :div, align_items: :center) do |grid| + grid.with_area(:item, tag: :div, mr: 3) do + render(Primer::Beta::Text.new(font_weight: :semibold, mr: 1)) { I18n.t('storages.file_storage_view.oauth_applications') } + end + end + end + + component.with_row(scheme: :default) do + render(OpTurbo::FrameComponent.new(id: :storage_oauth_client_section)) do + render(Storages::Admin::OAuthClientInfoComponent.new(oauth_client: storage.oauth_client, storage:)) + end + end + end end %> diff --git a/modules/storages/app/components/storages/admin/storage_view_information.rb b/modules/storages/app/components/storages/admin/storage_view_information.rb index 109b89a81557..2b2669d0f4db 100644 --- a/modules/storages/app/components/storages/admin/storage_view_information.rb +++ b/modules/storages/app/components/storages/admin/storage_view_information.rb @@ -48,7 +48,7 @@ def provider_oauth_client_description if storage.oauth_client "#{I18n.t('storages.label_oauth_client_id')}: #{storage.oauth_client.client_id}" else - I18n.t('storages.configuration_checks.oauth_client_incomplete', provider: storage.short_provider_type.capitalize) + I18n.t("storages.configuration_checks.oauth_client_incomplete.#{storage.short_provider_type}") 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 7fa853d82afb..6cab7d6845c0 100644 --- a/modules/storages/app/controllers/storages/admin/storages_controller.rb +++ b/modules/storages/app/controllers/storages/admin/storages_controller.rb @@ -111,9 +111,7 @@ def create # rubocop:disable Metrics/AbcSize service_result.on_success do if OpenProject::FeatureDecisions.storage_primer_design_active? - respond_to do |format| - format.turbo_stream { render :show_oauth_application } - end + respond_to { |format| format.turbo_stream } else case @storage.provider_type when ::Storages::Storage::PROVIDER_TYPE_ONE_DRIVE diff --git a/modules/storages/app/forms/storages/admin/oauth_client_form.rb b/modules/storages/app/forms/storages/admin/oauth_client_form.rb index 8bad45364cdc..53568462b5c9 100644 --- a/modules/storages/app/forms/storages/admin/oauth_client_form.rb +++ b/modules/storages/app/forms/storages/admin/oauth_client_form.rb @@ -29,22 +29,35 @@ module Storages::Admin class OAuthClientForm < ApplicationForm form do |oauth_client_form| - oauth_client_form.text_field(name: :client_id, - label: label_client_id, - required: true) - - oauth_client_form.text_field(name: :client_secret, - label: label_client_secret, - required: true) + oauth_client_form.text_field(**@client_id_input_options) + oauth_client_form.text_field(**@client_secret_input_options) end - def initialize(storage:) + def initialize(storage:, client_id_input_options: {}, client_secret_input_options: {}) super() @storage = storage + @client_id_input_options = default_client_id_input_options.merge(client_id_input_options) + @client_secret_input_options = default_client_secret_input_options.merge(client_secret_input_options) end private + def default_client_id_input_options + { + name: :client_id, + label: label_client_id, + required: true + } + end + + def default_client_secret_input_options + { + name: :client_secret, + label: label_client_secret, + required: true + } + end + def label_client_id [label_provider_name, I18n.t('storages.label_oauth_client_id')].join(' ') end diff --git a/modules/storages/app/views/storages/admin/storages/create.turbo_stream.erb b/modules/storages/app/views/storages/admin/storages/create.turbo_stream.erb new file mode 100644 index 000000000000..f15990f2c5d6 --- /dev/null +++ b/modules/storages/app/views/storages/admin/storages/create.turbo_stream.erb @@ -0,0 +1,58 @@ +<%#-- 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 :storage_general_info_section do %> + <%= render(Storages::Admin::GeneralInfoComponent.new(@storage)) %> +<% end %> + + +<% if @storage.provider_type_nextcloud? %> + <%= turbo_stream.update :storage_openproject_oauth_section do %> + <%= + render( + Storages::Admin::OAuthApplicationInfoCopyComponent.new( + oauth_application: @oauth_application, + storage: @storage, + submit_button_options: { + data: { turbo_stream: true } + }, + submit_button_path: new_admin_settings_storage_oauth_client_path(@storage), + cancel_button_path: admin_settings_storages_path, + cancel_button_should_break_from_frame: true + ) + ) + %> + <% end %> +<% end %> + +<% if @storage.provider_type_one_drive? %> + <%= turbo_stream.update :storage_oauth_client_section do %> + <%= render(Storages::Admin::Forms::OAuthClientFormComponent.new(oauth_client: @storage.build_oauth_client, storage: @storage)) %> + <% end %> +<% end %> diff --git a/modules/storages/config/locales/en.yml b/modules/storages/config/locales/en.yml index 842850af44a3..51af55ffb94f 100644 --- a/modules/storages/config/locales/en.yml +++ b/modules/storages/config/locales/en.yml @@ -89,6 +89,7 @@ en: project_folders: "Project folders" storage_provider: "Storage provider" openproject_oauth: "OpenProject OAuth" + one_drive_oauth: "Azure OAuth" nextcloud_oauth: "Nextcloud OAuth" automatically_managed_folders: "Automatically managed folders" page_titles: @@ -143,7 +144,7 @@ en: oauth_client_secret: > Copy the client secret from the Azure portal. For a newly created application the secret first needs to be created manually. For authorization of web applications a secret is mandatory. - missing_client_id_for_redirect_uri: "Client ID missing to provide redirect URI." + missing_client_id_for_redirect_uri: "Please fill the OAuth values to generate a URI" tenant_id: "Please copy the tenant from your Azure application." tenant_id_placeholder: "Name or UUID" drive_id: "Please copy the drive ID from your Azure application." @@ -154,7 +155,9 @@ en: Users can nevertheless still upload files to other locations. configuration_checks: incomplete: "The setup of this storage is incomplete." - oauth_client_incomplete: "Allow OpenProject to access %{provider} data using OAuth." + oauth_client_incomplete: + nextcloud: "Allow OpenProject to access Nextcloud data using OAuth." + one_drive: "Allow OpenProject to access Azure data using OAuth to connect OneDrive/Sharepoint." delete_warning: storage: > Are you sure you want to delete this storage? This will also delete the storage from all projects where it is used. @@ -200,6 +203,7 @@ en: label_information: "Additional information" label_provider_type: "Provider type" label_project_folder: "Project folder" + label_redirect_uri: "Redirect URI" label_new_storage: "New storage" label_new_file_storage: "New file storage" label_edit_storage: "Edit storage" From 5a5a372689be570dfbd88139cb2430b08daa7096 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 7 Nov 2023 13:05:00 +0300 Subject: [PATCH 32/56] fix[Op#50737]: Read cancel button path from method --- .../storages/admin/forms/general_info_form_component.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) 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 7644214c9ad1..c1c63751ee52 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 @@ -35,8 +35,7 @@ class GeneralInfoFormComponent < ApplicationComponent options form_method: :post, submit_button_disabled: false, - cancel_button_should_break_from_frame: false, - cancel_button_path: Rails.application.routes.url_helpers.admin_settings_storages_path + cancel_button_should_break_from_frame: false def form_url options[:form_url] || default_form_url @@ -62,5 +61,9 @@ def default_form_url Rails.application.routes.url_helpers.admin_settings_storage_path(storage) end end + + def cancel_button_path + options[:cancel_button_path] || Rails.application.routes.url_helpers.admin_settings_storages_path + end end end From 111d68bf43440c95e8a5e4441dfaf1551f5dd80d Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 7 Nov 2023 15:30:34 +0300 Subject: [PATCH 33/56] fix[Op#50737]: configuration check inclusive of one drive --- .../storages/admin/general_info_component.html.erb | 2 +- .../storages/admin/storage_view_information.rb | 14 +++++++++++--- .../app/models/storages/one_drive_storage.rb | 6 +++++- modules/storages/config/locales/en.yml | 2 +- modules/storages/spec/factories/storage_factory.rb | 3 +++ .../storages/spec/models/one_drive_storage_spec.rb | 4 +++- 6 files changed, 24 insertions(+), 7 deletions(-) diff --git a/modules/storages/app/components/storages/admin/general_info_component.html.erb b/modules/storages/app/components/storages/admin/general_info_component.html.erb index 0b3752c92cec..5fbcd3219dc6 100644 --- a/modules/storages/app/components/storages/admin/general_info_component.html.erb +++ b/modules/storages/app/components/storages/admin/general_info_component.html.erb @@ -2,7 +2,7 @@ grid_layout('op-storage-view--row', tag: :div, align_items: :center) do |grid| grid.with_area(:item, tag: :div, mr: 3) do concat(render(Primer::Beta::Text.new(font_weight: :semibold, mr: 1, test_selector: 'storage-provider-label')) { I18n.t('storages.file_storage_view.storage_provider') }) - concat(configuration_check_label_for(:host_name_configured)) + concat(configuration_check_label) end grid.with_area(:description, tag: :div, color: :subtle, test_selector: 'storage-description') do diff --git a/modules/storages/app/components/storages/admin/storage_view_information.rb b/modules/storages/app/components/storages/admin/storage_view_information.rb index 2b2669d0f4db..fefbd2365c90 100644 --- a/modules/storages/app/components/storages/admin/storage_view_information.rb +++ b/modules/storages/app/components/storages/admin/storage_view_information.rb @@ -14,9 +14,17 @@ def storage_description storage.host].compact.join(' - ') end - def configuration_check_label_for(config) - if storage.configuration_checks[config.to_sym] - status_label(I18n.t('storages.label_connected'), scheme: :success, test_selector: "label-#{config}-status") + def configuration_check_label + if storage.provider_type_nextcloud? + configuration_check_label_for(:host_name_configured) + elsif storage.provider_type_one_drive? + configuration_check_label_for(:host_name_configured, :storage_tenant_drive_configured) + end + end + + def configuration_check_label_for(*configs) + if storage.configuration_checks.slice(configs.map(&:to_sym)).values.all? + status_label(I18n.t('storages.label_completed'), scheme: :success, test_selector: "label-#{config}-status") else status_label(I18n.t('storages.label_incomplete'), scheme: :attention, test_selector: "label-#{config}-status") end diff --git a/modules/storages/app/models/storages/one_drive_storage.rb b/modules/storages/app/models/storages/one_drive_storage.rb index 1d2e116cc328..d6c6e0ccc6bd 100644 --- a/modules/storages/app/models/storages/one_drive_storage.rb +++ b/modules/storages/app/models/storages/one_drive_storage.rb @@ -36,7 +36,11 @@ class OneDriveStorage < Storage using ::Storages::Peripherals::ServiceResultRefinements def configuration_checks - { storage_oauth_client_configured: oauth_client.present? } + { + storage_oauth_client_configured: oauth_client.present?, + storage_tenant_drive_configured: tenant_id.present? && drive_id.present?, + host_name_configured: name.present? + } end def oauth_configuration diff --git a/modules/storages/config/locales/en.yml b/modules/storages/config/locales/en.yml index 51af55ffb94f..fdad9b5a8e84 100644 --- a/modules/storages/config/locales/en.yml +++ b/modules/storages/config/locales/en.yml @@ -182,7 +182,7 @@ en: label_file_links: "File links" label_file_storage: "File storage" label_creation_time: "Creation time" - label_connected: "Connected" + label_completed: "Completed" label_incomplete: "Incomplete" label_name: "Name" label_host: "Host URL" diff --git a/modules/storages/spec/factories/storage_factory.rb b/modules/storages/spec/factories/storage_factory.rb index 7f10d56c559f..7e1b1e445a89 100644 --- a/modules/storages/spec/factories/storage_factory.rb +++ b/modules/storages/spec/factories/storage_factory.rb @@ -38,6 +38,7 @@ trait :with_oauth_client do oauth_client { build(:oauth_client) } end + # rubocop:enable FactoryBot/FactoryAssociationWithStrategy trait :as_generic do provider_type { 'Storages::Storage' } @@ -109,6 +110,8 @@ parent: :storage, class: '::Storages::OneDriveStorage' do host { nil } + tenant_id { SecureRandom.uuid } + drive_id { SecureRandom.uuid } end factory :sharepoint_dev_drive_storage, diff --git a/modules/storages/spec/models/one_drive_storage_spec.rb b/modules/storages/spec/models/one_drive_storage_spec.rb index 47d4f45980b8..417994ff1dfe 100644 --- a/modules/storages/spec/models/one_drive_storage_spec.rb +++ b/modules/storages/spec/models/one_drive_storage_spec.rb @@ -50,7 +50,9 @@ aggregate_failures 'configuration_checks' do expect(storage.configuration_checks) - .to eq(storage_oauth_client_configured: true) + .to eq(host_name_configured: true, + storage_oauth_client_configured: true, + storage_tenant_drive_configured: true) end end end From 2d59cfb42df80fb863c85828d70f28faa397a002 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 7 Nov 2023 15:32:39 +0300 Subject: [PATCH 34/56] Revert "fix[Op#50737]: Remove unused form closing method check" This reverts commit 57e43711472435007cf3961af49e5891417ae81b. --- .../storages/admin/storage_view_component.html.erb | 2 +- .../components/storages/admin/storage_view_component.rb | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/modules/storages/app/components/storages/admin/storage_view_component.html.erb b/modules/storages/app/components/storages/admin/storage_view_component.html.erb index 088a1b9f099e..552a71467cfc 100644 --- a/modules/storages/app/components/storages/admin/storage_view_component.html.erb +++ b/modules/storages/app/components/storages/admin/storage_view_component.html.erb @@ -30,7 +30,7 @@ component.with_row(scheme: :default) do render(OpTurbo::FrameComponent.new(id: :storage_openproject_oauth_section)) do - if storage.new_record? + if storage.new_record? || openproject_oauth_application_section_closed? render(Storages::Admin::OAuthApplicationInfoComponent.new(oauth_application:, storage:)) else render( diff --git a/modules/storages/app/components/storages/admin/storage_view_component.rb b/modules/storages/app/components/storages/admin/storage_view_component.rb index e60cb3cad44a..ba01ef49f06a 100644 --- a/modules/storages/app/components/storages/admin/storage_view_component.rb +++ b/modules/storages/app/components/storages/admin/storage_view_component.rb @@ -33,8 +33,15 @@ class StorageViewComponent < ApplicationComponent include OpPrimer::ComponentHelpers include StorageViewInformation + options openproject_oauth_application_section_open: false + alias_method :storage, :model + alias_method :openproject_oauth_application_section_open?, :openproject_oauth_application_section_open delegate :oauth_application, to: :model + + def openproject_oauth_application_section_closed? + !openproject_oauth_application_section_open? + end end end From 1e6d388a43ca08d22ab706a0ab9d1e388d226bc5 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 7 Nov 2023 15:56:00 +0300 Subject: [PATCH 35/56] fix[Op#50737]: configuration check label --- .../components/storages/admin/storage_view_information.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/storages/app/components/storages/admin/storage_view_information.rb b/modules/storages/app/components/storages/admin/storage_view_information.rb index fefbd2365c90..f00175b2b485 100644 --- a/modules/storages/app/components/storages/admin/storage_view_information.rb +++ b/modules/storages/app/components/storages/admin/storage_view_information.rb @@ -23,10 +23,10 @@ def configuration_check_label end def configuration_check_label_for(*configs) - if storage.configuration_checks.slice(configs.map(&:to_sym)).values.all? - status_label(I18n.t('storages.label_completed'), scheme: :success, test_selector: "label-#{config}-status") + if storage.configuration_checks.slice(*configs.map(&:to_sym)).values.all? + status_label(I18n.t('storages.label_completed'), scheme: :success, test_selector: "label-completed-status") else - status_label(I18n.t('storages.label_incomplete'), scheme: :attention, test_selector: "label-#{config}-status") + status_label(I18n.t('storages.label_incomplete'), scheme: :attention, test_selector: "label-incomplete-status") end end From 4844215b2a7fc179f5436f96d1bd165cb5460051 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 8 Nov 2023 14:13:34 +0300 Subject: [PATCH 36/56] chore[Op#50737]: make label names consistent --- .../app/components/storages/admin/storage_view_information.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/storages/app/components/storages/admin/storage_view_information.rb b/modules/storages/app/components/storages/admin/storage_view_information.rb index f00175b2b485..5dd591e3d9a0 100644 --- a/modules/storages/app/components/storages/admin/storage_view_information.rb +++ b/modules/storages/app/components/storages/admin/storage_view_information.rb @@ -24,9 +24,9 @@ def configuration_check_label def configuration_check_label_for(*configs) if storage.configuration_checks.slice(*configs.map(&:to_sym)).values.all? - status_label(I18n.t('storages.label_completed'), scheme: :success, test_selector: "label-completed-status") + status_label(I18n.t('storages.label_completed'), scheme: :success, test_selector: 'label-completed') else - status_label(I18n.t('storages.label_incomplete'), scheme: :attention, test_selector: "label-incomplete-status") + status_label(I18n.t('storages.label_incomplete'), scheme: :attention, test_selector: 'label-incomplete') end end From 17d113bc00f6617a4adb1bd9cd0c32841164a96c Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 8 Nov 2023 14:14:07 +0300 Subject: [PATCH 37/56] fix[Op#50737]: scope storage page titles by feature flag --- .../app/views/storages/admin/storages/new.html.erb | 8 +++++--- 1 file changed, 5 insertions(+), 3 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 ed8f2b0d041a..a198fca81316 100644 --- a/modules/storages/app/views/storages/admin/storages/new.html.erb +++ b/modules/storages/app/views/storages/admin/storages/new.html.erb @@ -1,8 +1,8 @@ -<% 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') %> - <% if OpenProject::FeatureDecisions.storage_primer_design_active? %> + <% 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') %> + <%= render(Primer::OpenProject::PageHeader.new) do |header| %> <% header.with_title(test_selector: 'storage-name-title') do %> <%= t("storages.label_new_file_storage") %> @@ -16,6 +16,8 @@ <%= render(::Storages::Admin::SelectStorageProviderComponent.new(@storage)) %> <% else %> + <% html_title t(:label_administration), t("project_module_storages"), t('storages.label_new_storage') %> + <% local_assigns[:additional_breadcrumb] = t('storages.label_new_storage') %> <%= toolbar title: t("storages.label_new_storage") %> <%= labelled_tabular_form_for @storage, url: admin_settings_storages_path(@storage), as: :storages_storage do |f| -%> From bdf3b2ba1a7c9f01e0f5065c3af3f02722aa3e4e Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 8 Nov 2023 15:33:48 +0300 Subject: [PATCH 38/56] tests[Op#50737]: Multiple fixes to correct edit vs new storage flow --- ..._managed_project_folders_form_component.rb | 4 +- .../oauth_client_form_component.html.erb | 5 +- .../application_password_input.rb | 4 +- .../spec/features/admin_storages_spec.rb | 65 +++++++++++-------- 4 files changed, 46 insertions(+), 32 deletions(-) diff --git a/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.rb b/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.rb index 841aac433853..926ef6d186e8 100644 --- a/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.rb +++ b/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.rb @@ -64,8 +64,8 @@ def default_form_method end def new_record? - if (provider_fields_changes_to_be_saved = storage.provider_fields_change_to_be_saved) - provider_fields_changes_to_be_saved.first["automatically_managed"].nil? + if storage.provider_fields_changed? + storage.provider_fields_change.first['automatically_managed'].nil? else storage.automatic_management_unspecified? end diff --git a/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb b/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb index 65105c6de02a..0455696d0273 100644 --- a/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb +++ b/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb @@ -6,7 +6,7 @@ data: { controller: 'storages--oauth-client-form', application_target: 'dynamic', - 'storages--oauth-client-form-client-id-missing-value': t(:"storages.instructions.#{storage.short_provider_type}.missing_client_id_for_redirect_uri") + 'storages--oauth-client-form-client-id-missing-value': t(:"storages.instructions.one_drive.missing_client_id_for_redirect_uri") } ) do |form| flex_layout do |oauth_client_row| @@ -67,7 +67,8 @@ disabled: submit_button_disabled?, data: { 'storages--oauth-client-form-target': 'submitButton' - } + }, + test_selector: 'storage-oauth-client-submit-button' }, cancel_button_options: { href: cancel_button_path diff --git a/modules/storages/app/forms/storages/admin/managed_project_folders/application_password_input.rb b/modules/storages/app/forms/storages/admin/managed_project_folders/application_password_input.rb index 0e0bde9dd148..a00d3432ad6f 100644 --- a/modules/storages/app/forms/storages/admin/managed_project_folders/application_password_input.rb +++ b/modules/storages/app/forms/storages/admin/managed_project_folders/application_password_input.rb @@ -33,7 +33,9 @@ class ApplicationPasswordInput < ApplicationForm name: :password, label: I18n.t(:'storages.label_managed_project_folders.application_password'), required: true, - caption: application_password_caption + caption: application_password_caption, + value: nil, # IMPORTANT: We don't want to show the password in the form + placeholder: @storage.password.present? ? "••••••••••••••••" : nil ) end diff --git a/modules/storages/spec/features/admin_storages_spec.rb b/modules/storages/spec/features/admin_storages_spec.rb index 88950db51650..61995c6504ac 100644 --- a/modules/storages/spec/features/admin_storages_spec.rb +++ b/modules/storages/spec/features/admin_storages_spec.rb @@ -90,23 +90,50 @@ end describe 'File storage edit view', with_flag: { storage_primer_design: true } do - let(:storage) { create(:nextcloud_storage) } + let(:storage) { create(:nextcloud_storage, :as_automatically_managed) } let(:oauth_application) { create(:oauth_application, integration: storage) } + let(:oauth_client) { create(:oauth_client, integration: storage) } - before { oauth_application } + before do + oauth_application + oauth_client + end - it 'renders the edit view', :webmock do + 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) - aggregate_failures 'General information' do + aggregate_failures 'Storage edit view' do + # General information expect(page).to have_test_selector('storage-provider-label', text: 'Storage provider') - expect(page).to have_test_selector('label-host_name_configured-status', text: 'Connected') + expect(page).to have_test_selector('label-completed', text: 'Completed') expect(page).to have_test_selector('storage-description', text: [storage.short_provider_type.capitalize, storage.name, storage.host].join(' - ')) + # OAuth application + expect(page).to have_test_selector('storage-openproject-oauth-label', text: 'OpenProject OAuth') + expect(page).to have_test_selector('label-completed', text: 'Completed') + expect(page).to have_test_selector('storage-openproject-oauth-application-description', + text: "OAuth Client ID: #{oauth_application.uid}") + + # OAuth client + expect(page).to have_test_selector('storage-oauth-client-label', text: 'Nextcloud OAuth') + expect(page).to have_test_selector('label-completed', text: 'Completed') + expect(page).to have_test_selector('storage-oauth-client-id-description', + text: "OAuth Client ID: #{oauth_client.client_id}") + + # Automatically managed project folders + expect(page).to have_test_selector('storage-managed-project-folders-label', + text: 'Automatically managed folders') + + expect(page).to have_test_selector('label-managed-project-folders-status', text: 'Active') + expect(page).to have_test_selector('storage-automatically-managed-project-folders-description', + text: 'Let OpenProject create folders per project automatically.') + end + + aggregate_failures 'General information' do # Update a storage - happy path find_test_selector('storage-edit-host-button').click within_test_selector('storage-general-info-form') do @@ -137,7 +164,7 @@ aggregate_failures 'OAuth application' do expect(page).to have_test_selector('storage-openproject-oauth-label', text: 'OpenProject OAuth') - expect(page).to have_test_selector('label-openproject_oauth_application_configured-status', text: 'Connected') + expect(page).to have_test_selector('label-completed', text: 'Completed') expect(page).to have_test_selector('storage-openproject-oauth-application-description', text: "OAuth Client ID: #{oauth_application.uid}") @@ -163,44 +190,29 @@ end aggregate_failures 'Nextcloud OAuth' do - expect(page).to have_test_selector('storage-oauth-client-label', text: 'Nextcloud OAuth') - expect(page).to have_test_selector('label-storage_oauth_client_configured-status', text: 'Incomplete') - expect(page).to have_test_selector('storage-oauth-client-id-description', - text: "Allow OpenProject to access Nextcloud data using an OAuth.") - accept_confirm do find_test_selector('storage-edit-oauth-client-button').click end within_test_selector('storage-oauth-client-form') do + # With null values, submit button should be disabled expect(page).to have_css('#oauth_client_client_id', value: '') expect(page).to have_css('#oauth_client_client_secret', value: '') - - # Unhappy path - Attempt to submit null values - click_button 'Save and continue' - # TODO: This should be "Client ID can't be blank." but primer assumes the label is "Client" - expect(page).to have_text("Client can't be blank.") - expect(page).to have_text("Client secret can't be blank.") + expect(find_test_selector('storage-oauth-client-submit-button')).to be_disabled # Happy path - Submit valid values fill_in 'oauth_client_client_id', with: '1234567890' fill_in 'oauth_client_client_secret', with: '0987654321' + expect(find_test_selector('storage-oauth-client-submit-button')).not_to be_disabled click_button 'Save and continue' end - expect(page).to have_test_selector('label-storage_oauth_client_configured-status', text: 'Connected') + expect(page).to have_test_selector('label-completed', text: 'Completed') expect(page).to have_test_selector('storage-oauth-client-id-description', text: "OAuth Client ID: 1234567890") end aggregate_failures 'Automatically managed project folders' do - expect(page).to have_test_selector('storage-managed-project-folders-label', - text: 'Automatically managed folders') - expect(page).to have_test_selector('label-managed-project-folders-status', - text: 'Incomplete') - expect(page).to have_test_selector('storage-automatically-managed-project-folders-description', - text: 'Let OpenProject create folders per project automatically.') - find_test_selector('storage-edit-automatically-managed-project-folders-button').click within_test_selector('storage-automatically-managed-project-folders-form') do @@ -234,8 +246,7 @@ click_button('Done, complete setup') end - expect(page).to have_test_selector('label-managed-project-folders-status', - text: 'Active') + expect(page).to have_test_selector('label-managed-project-folders-status', text: 'Active') end end From 5e890cc79c3b05bf85025d7342043a44ff963d0b Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 8 Nov 2023 20:41:29 +0300 Subject: [PATCH 39/56] fix[Op#50737]: name label by section configuration check --- .../storages/admin/storage_view_information.rb | 4 ++-- modules/storages/spec/features/admin_storages_spec.rb | 11 +++-------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/modules/storages/app/components/storages/admin/storage_view_information.rb b/modules/storages/app/components/storages/admin/storage_view_information.rb index 5dd591e3d9a0..029d9d80eaed 100644 --- a/modules/storages/app/components/storages/admin/storage_view_information.rb +++ b/modules/storages/app/components/storages/admin/storage_view_information.rb @@ -24,9 +24,9 @@ def configuration_check_label def configuration_check_label_for(*configs) if storage.configuration_checks.slice(*configs.map(&:to_sym)).values.all? - status_label(I18n.t('storages.label_completed'), scheme: :success, test_selector: 'label-completed') + status_label(I18n.t('storages.label_completed'), scheme: :success, test_selector: "label-#{configs.join('-')}-status") else - status_label(I18n.t('storages.label_incomplete'), scheme: :attention, test_selector: 'label-incomplete') + status_label(I18n.t('storages.label_incomplete'), scheme: :attention, test_selector: "label-#{configs.join('-')}-status") end end diff --git a/modules/storages/spec/features/admin_storages_spec.rb b/modules/storages/spec/features/admin_storages_spec.rb index 61995c6504ac..816c76aa0206 100644 --- a/modules/storages/spec/features/admin_storages_spec.rb +++ b/modules/storages/spec/features/admin_storages_spec.rb @@ -107,20 +107,20 @@ aggregate_failures 'Storage edit view' do # General information expect(page).to have_test_selector('storage-provider-label', text: 'Storage provider') - expect(page).to have_test_selector('label-completed', text: 'Completed') + expect(page).to have_test_selector('label-host_name_configured-status', text: 'Completed') expect(page).to have_test_selector('storage-description', text: [storage.short_provider_type.capitalize, storage.name, storage.host].join(' - ')) # OAuth application expect(page).to have_test_selector('storage-openproject-oauth-label', text: 'OpenProject OAuth') - expect(page).to have_test_selector('label-completed', text: 'Completed') + expect(page).to have_test_selector('label-openproject_oauth_application_configured-status', text: 'Completed') expect(page).to have_test_selector('storage-openproject-oauth-application-description', text: "OAuth Client ID: #{oauth_application.uid}") # OAuth client expect(page).to have_test_selector('storage-oauth-client-label', text: 'Nextcloud OAuth') - expect(page).to have_test_selector('label-completed', text: 'Completed') + expect(page).to have_test_selector('label-storage_oauth_client_configured-status', text: 'Completed') expect(page).to have_test_selector('storage-oauth-client-id-description', text: "OAuth Client ID: #{oauth_client.client_id}") @@ -163,11 +163,6 @@ end aggregate_failures 'OAuth application' do - expect(page).to have_test_selector('storage-openproject-oauth-label', text: 'OpenProject OAuth') - expect(page).to have_test_selector('label-completed', text: 'Completed') - expect(page).to have_test_selector('storage-openproject-oauth-application-description', - text: "OAuth Client ID: #{oauth_application.uid}") - accept_confirm do find_test_selector('storage-replace-openproject-oauth-application-button').click end From 5397cc56f899d0dabd6a2a7093ae17803b3f64f8 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 8 Nov 2023 20:43:04 +0300 Subject: [PATCH 40/56] fix[Op#50737]: control automatically managed section via options --- ...utomatically_managed_project_folders_form_component.rb | 3 ++- .../storages/admin/storage_view_component.html.erb | 6 +++++- .../components/storages/admin/storage_view_component.rb | 8 +++++++- .../automatically_managed_project_folders/edit.html.erb | 4 +--- modules/storages/spec/features/admin_storages_spec.rb | 7 +++---- 5 files changed, 18 insertions(+), 10 deletions(-) diff --git a/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.rb b/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.rb index 926ef6d186e8..0736e2de889e 100644 --- a/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.rb +++ b/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.rb @@ -65,7 +65,8 @@ def default_form_method def new_record? if storage.provider_fields_changed? - storage.provider_fields_change.first['automatically_managed'].nil? + previous_configuration = storage.provider_fields_change.first + previous_configuration.values_at('automatically_managed', 'password').compact.empty? else storage.automatic_management_unspecified? end diff --git a/modules/storages/app/components/storages/admin/storage_view_component.html.erb b/modules/storages/app/components/storages/admin/storage_view_component.html.erb index 552a71467cfc..7a71b34d5cf0 100644 --- a/modules/storages/app/components/storages/admin/storage_view_component.html.erb +++ b/modules/storages/app/components/storages/admin/storage_view_component.html.erb @@ -61,7 +61,11 @@ component.with_row(scheme: :default) do render(OpTurbo::FrameComponent.new(id: :automatically_managed_project_folders_section)) do - render(Storages::Admin::AutomaticallyManagedProjectFoldersInfoComponent.new(storage)) + if automatically_managed_project_folders_section_closed? + render(Storages::Admin::AutomaticallyManagedProjectFoldersInfoComponent.new(storage)) + else + render(Storages::Admin::Forms::AutomaticallyManagedProjectFoldersFormComponent.new(storage)) + end end end end diff --git a/modules/storages/app/components/storages/admin/storage_view_component.rb b/modules/storages/app/components/storages/admin/storage_view_component.rb index ba01ef49f06a..650d075cb424 100644 --- a/modules/storages/app/components/storages/admin/storage_view_component.rb +++ b/modules/storages/app/components/storages/admin/storage_view_component.rb @@ -33,15 +33,21 @@ class StorageViewComponent < ApplicationComponent include OpPrimer::ComponentHelpers include StorageViewInformation - options openproject_oauth_application_section_open: false + options openproject_oauth_application_section_open: false, + automatically_managed_project_folders_section_open: false alias_method :storage, :model alias_method :openproject_oauth_application_section_open?, :openproject_oauth_application_section_open + alias_method :automatically_managed_project_folders_section_open?, :automatically_managed_project_folders_section_open delegate :oauth_application, to: :model def openproject_oauth_application_section_closed? !openproject_oauth_application_section_open? end + + def automatically_managed_project_folders_section_closed? + !automatically_managed_project_folders_section_open? + end end end diff --git a/modules/storages/app/views/storages/admin/storages/automatically_managed_project_folders/edit.html.erb b/modules/storages/app/views/storages/admin/storages/automatically_managed_project_folders/edit.html.erb index f23b463f7d5a..a2d761da78b0 100644 --- a/modules/storages/app/views/storages/admin/storages/automatically_managed_project_folders/edit.html.erb +++ b/modules/storages/app/views/storages/admin/storages/automatically_managed_project_folders/edit.html.erb @@ -31,9 +31,7 @@ See COPYRIGHT and LICENSE files for more details. <% if OpenProject::FeatureDecisions.storage_primer_design_active? %> <%= - render(OpTurbo::FrameComponent.new(id: :automatically_managed_project_folders_section)) do - render(Storages::Admin::Forms::AutomaticallyManagedProjectFoldersFormComponent.new(@storage)) - end + render(Storages::Admin::StorageViewComponent.new(@storage, automatically_managed_project_folders_section_open: true)) %> <% else %> <% local_assigns[:additional_breadcrumb] = [ diff --git a/modules/storages/spec/features/admin_storages_spec.rb b/modules/storages/spec/features/admin_storages_spec.rb index 816c76aa0206..971c06c247f0 100644 --- a/modules/storages/spec/features/admin_storages_spec.rb +++ b/modules/storages/spec/features/admin_storages_spec.rb @@ -184,7 +184,7 @@ end end - aggregate_failures 'Nextcloud OAuth' do + aggregate_failures 'OAuth Client' do accept_confirm do find_test_selector('storage-edit-oauth-client-button').click end @@ -202,9 +202,8 @@ click_button 'Save and continue' end - expect(page).to have_test_selector('label-completed', text: 'Completed') - expect(page).to have_test_selector('storage-oauth-client-id-description', - text: "OAuth Client ID: 1234567890") + expect(page).to have_test_selector('label-storage_oauth_client_configured-status', text: 'Completed') + expect(page).to have_test_selector('storage-oauth-client-id-description', text: "OAuth Client ID: 1234567890") end aggregate_failures 'Automatically managed project folders' do From 2f7383e3a0fca9c14879acc2df3dd1d977ab5e02 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 9 Nov 2023 15:40:01 +0300 Subject: [PATCH 41/56] fix[Op#50737]: Mend OAuthClient form for one drive, introduce feature tests --- ..._managed_project_folders_form_component.rb | 7 +- .../oauth_client_form_component.html.erb | 5 +- .../forms/oauth_client_form_component.rb | 29 +- .../admin/storage_view_information.rb | 2 +- .../admin/oauth_clients_controller.rb | 75 +++- .../app/models/storages/nextcloud_storage.rb | 9 + .../oauth_clients/create.turbo_stream.erb | 6 +- .../oauth_clients/update.turbo_stream.erb | 32 ++ modules/storages/config/routes.rb | 5 +- .../spec/features/admin_storages_spec.rb | 348 +++++++++++------- .../spec/models/nextcloud_storage_spec.rb | 25 ++ 11 files changed, 371 insertions(+), 172 deletions(-) create mode 100644 modules/storages/app/views/storages/admin/oauth_clients/update.turbo_stream.erb diff --git a/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.rb b/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.rb index 0736e2de889e..153d95abbbb2 100644 --- a/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.rb +++ b/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.rb @@ -64,12 +64,7 @@ def default_form_method end def new_record? - if storage.provider_fields_changed? - previous_configuration = storage.provider_fields_change.first - previous_configuration.values_at('automatically_managed', 'password').compact.empty? - else - storage.automatic_management_unspecified? - end + storage.automatic_management_new_record? end def default_form_url diff --git a/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb b/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb index 0455696d0273..1c3897c11b4c 100644 --- a/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb +++ b/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb @@ -3,6 +3,7 @@ primer_form_with( model: oauth_client, url: admin_settings_storage_oauth_client_path(storage), + method: form_method, data: { controller: 'storages--oauth-client-form', application_target: 'dynamic', @@ -66,8 +67,8 @@ submit_button_options: { disabled: submit_button_disabled?, data: { - 'storages--oauth-client-form-target': 'submitButton' - }, + 'storages--oauth-client-form-target': 'submitButton', + }.merge(submit_button_data_options), test_selector: 'storage-oauth-client-submit-button' }, cancel_button_options: { diff --git a/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.rb b/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.rb index 682c6a3bc870..65634cd99f06 100644 --- a/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.rb +++ b/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.rb @@ -40,16 +40,27 @@ def initialize(oauth_client:, storage:, **options) @storage = storage end + def form_method + options[:form_method] || default_form_method + end + def cancel_button_path storage.persisted? ? edit_admin_settings_storage_path(storage) : admin_settings_storages_path end def submit_button_disabled? - !storage_oauth_client_configured? + !oauth_client_configured? + end + + def submit_button_data_options + {}.tap do |data| + # For One Drive create action, break from Turbo Frame and follow full page redirect + data[:turbo] = false if one_drive_first_time_configuration? + end end def redirect_uri_or_instructions - if storage_oauth_client_configured? + if oauth_client_configured? oauth_client.redirect_uri else I18n.t("storages.instructions.one_drive.missing_client_id_for_redirect_uri") @@ -72,7 +83,19 @@ def provider_credentials_instructions_link private - def storage_oauth_client_configured? + def one_drive_first_time_configuration? + storage.provider_type_one_drive? && first_time_configuration? + end + + def first_time_configuration? + storage.oauth_client.blank? || storage.oauth_client.new_record? + end + + def default_form_method + first_time_configuration? ? :post : :patch + end + + def oauth_client_configured? oauth_client.present? && oauth_client.client_id.present? && oauth_client.client_secret.present? end end diff --git a/modules/storages/app/components/storages/admin/storage_view_information.rb b/modules/storages/app/components/storages/admin/storage_view_information.rb index 029d9d80eaed..1c673f8189f8 100644 --- a/modules/storages/app/components/storages/admin/storage_view_information.rb +++ b/modules/storages/app/components/storages/admin/storage_view_information.rb @@ -9,7 +9,7 @@ def editable_storage? end def storage_description - [storage.short_provider_type.capitalize, + [I18n.t("storages.provider_types.#{storage.short_provider_type}.name"), storage.name, storage.host].compact.join(' - ') end diff --git a/modules/storages/app/controllers/storages/admin/oauth_clients_controller.rb b/modules/storages/app/controllers/storages/admin/oauth_clients_controller.rb index c92bea8cfeb6..02c05533ce22 100644 --- a/modules/storages/app/controllers/storages/admin/oauth_clients_controller.rb +++ b/modules/storages/app/controllers/storages/admin/oauth_clients_controller.rb @@ -37,7 +37,7 @@ class Storages::Admin::OAuthClientsController < ApplicationController before_action :require_admin before_action :find_storage - before_action :delete_current_oauth_client, only: %i[create] + before_action :delete_current_oauth_client, only: %i[create update] # menu_item is defined in the Redmine::MenuManager::MenuController # module, included from ApplicationController. @@ -62,37 +62,52 @@ def new # Actually create a OAuthClient object. # Use service pattern to create a new OAuthClient # Called by: Global app/config/routes.rb to serve Web page - def create # rubocop:disable Metrics/AbcSize - service_result = ::OAuthClients::CreateService.new(user: User.current) - .call(oauth_client_params.merge(integration: @storage)) - @oauth_client = service_result.result - @storage = @storage.reload + def create # rubocop:disable Metrics/AbcSize, Metrics/PerceivedComplexity + call_oauth_clients_create_service service_result.on_failure do render '/storages/admin/storages/new_oauth_client' end service_result.on_success do - flash[:notice] = I18n.t(:notice_successful_create) - - if @storage.provider_type_nextcloud? && @storage.automatic_management_unspecified? - if OpenProject::FeatureDecisions.storage_primer_design_active? - @storage = ::Storages::Storages::SetNextcloudProviderFieldsAttributesService - .new(user: current_user, - model: @storage, - contract_class: EmptyContract) - .call - .result + if OpenProject::FeatureDecisions.storage_primer_design_active? + if @storage.provider_type_nextcloud? + prepare_storage_for_automatic_management_form respond_to do |format| - format.html { redirect_to edit_admin_settings_storage_path(@storage) } - format.turbo_stream + format.turbo_stream { render :create } end + elsif @storage.provider_type_one_drive? + flash[:notice] = I18n.t(:'storages.notice_successful_storage_connection') + redirect_to admin_settings_storages_path else - redirect_to new_admin_settings_storage_automatically_managed_project_folders_path(@storage) + raise "Unsupported provider type: #{@storage.short_provider_type}" end else - redirect_to edit_admin_settings_storage_path(@storage) + flash[:notice] = I18n.t(:notice_successful_create) + + if @storage.provider_type_nextcloud? && @storage.automatic_management_unspecified? + prepare_storage_for_automatic_management_form + redirect_to new_admin_settings_storage_automatically_managed_project_folders_path(@storage) + else + redirect_to edit_admin_settings_storage_path(@storage) + end + end + end + end + + def update + call_oauth_clients_create_service + + service_result.on_failure do + respond_to do |format| + format.turbo_stream { render :new } + end + end + + service_result.on_success do + respond_to do |format| + format.turbo_stream { render :update } end end end @@ -112,6 +127,25 @@ def show_local_breadcrumb private + attr_reader :service_result + + def call_oauth_clients_create_service + @service_result = ::OAuthClients::CreateService + .new(user: User.current) + .call(oauth_client_params.merge(integration: @storage)) + @oauth_client = service_result.result + @storage = @storage.reload + end + + def prepare_storage_for_automatic_management_form + return unless @storage.automatic_management_unspecified? + + @storage = ::Storages::Storages::SetNextcloudProviderFieldsAttributesService + .new(user: current_user, model: @storage, contract_class: EmptyContract) + .call + .result + end + # Called by create and update above in order to check if the # update parameters are correctly set. def oauth_client_params @@ -125,6 +159,7 @@ def find_storage end def delete_current_oauth_client + @previous_oauth_client_present = @storage.oauth_client.present? ::OAuthClients::DeleteService.new(user: User.current, model: @storage.oauth_client).call if @storage.oauth_client end end diff --git a/modules/storages/app/models/storages/nextcloud_storage.rb b/modules/storages/app/models/storages/nextcloud_storage.rb index 06451b2beb78..95f73c2d5169 100644 --- a/modules/storages/app/models/storages/nextcloud_storage.rb +++ b/modules/storages/app/models/storages/nextcloud_storage.rb @@ -60,6 +60,15 @@ def oauth_configuration Peripherals::OAuthConfigurations::NextcloudConfiguration.new(self) end + def automatic_management_new_record? + if provider_fields_changed? + previous_configuration = provider_fields_change.first + previous_configuration.values_at('automatically_managed', 'password').compact.empty? + else + automatic_management_unspecified? + end + end + def automatic_management_unspecified? automatically_managed.nil? end diff --git a/modules/storages/app/views/storages/admin/oauth_clients/create.turbo_stream.erb b/modules/storages/app/views/storages/admin/oauth_clients/create.turbo_stream.erb index 7058cb5711bb..3fd98e27146c 100644 --- a/modules/storages/app/views/storages/admin/oauth_clients/create.turbo_stream.erb +++ b/modules/storages/app/views/storages/admin/oauth_clients/create.turbo_stream.erb @@ -31,6 +31,8 @@ See COPYRIGHT and LICENSE files for more details. <%= render(Storages::Admin::OAuthClientInfoComponent.new(oauth_client: @oauth_client, storage: @storage)) %> <% end %> -<%= turbo_stream.update :automatically_managed_project_folders_section do %> - <%= render(Storages::Admin::Forms::AutomaticallyManagedProjectFoldersFormComponent.new(@storage)) %> +<% if @storage.provider_type_nextcloud? && @storage.automatic_management_new_record? %> + <%= turbo_stream.update :automatically_managed_project_folders_section do %> + <%= render(Storages::Admin::Forms::AutomaticallyManagedProjectFoldersFormComponent.new(@storage)) %> + <% end %> <% end %> diff --git a/modules/storages/app/views/storages/admin/oauth_clients/update.turbo_stream.erb b/modules/storages/app/views/storages/admin/oauth_clients/update.turbo_stream.erb new file mode 100644 index 000000000000..9a27fd06284a --- /dev/null +++ b/modules/storages/app/views/storages/admin/oauth_clients/update.turbo_stream.erb @@ -0,0 +1,32 @@ +<%#-- 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 :storage_oauth_client_section do %> + <%= render(Storages::Admin::OAuthClientInfoComponent.new(oauth_client: @oauth_client, storage: @storage)) %> +<% end %> diff --git a/modules/storages/config/routes.rb b/modules/storages/config/routes.rb index 49d29a485943..f221bdef73fc 100644 --- a/modules/storages/config/routes.rb +++ b/modules/storages/config/routes.rb @@ -32,7 +32,10 @@ namespace :admin do namespace :settings do resources :storages, controller: '/storages/admin/storages', except: [:show] do - resource :oauth_client, controller: '/storages/admin/oauth_clients', only: %i[new create] + resource :oauth_client, controller: '/storages/admin/oauth_clients', only: %i[new create] do + patch :update, on: :member + end + resource :automatically_managed_project_folders, controller: '/storages/admin/automatically_managed_project_folders', only: %i[new create edit update] diff --git a/modules/storages/spec/features/admin_storages_spec.rb b/modules/storages/spec/features/admin_storages_spec.rb index 971c06c247f0..868dd8ea1664 100644 --- a/modules/storages/spec/features/admin_storages_spec.rb +++ b/modules/storages/spec/features/admin_storages_spec.rb @@ -90,173 +90,247 @@ end describe 'File storage edit view', with_flag: { storage_primer_design: true } do - let(:storage) { create(:nextcloud_storage, :as_automatically_managed) } - let(:oauth_application) { create(:oauth_application, integration: storage) } - let(:oauth_client) { create(:oauth_client, integration: storage) } + it 'renders a delete button' do + storage = create(:nextcloud_storage, name: "Foo Nextcloud") + visit edit_admin_settings_storage_path(storage) + + storage_delete_button = find_test_selector('storage-delete-button') + expect(storage_delete_button).to have_text('Delete') - before do - oauth_application - oauth_client + accept_confirm do + storage_delete_button.click + end + + expect(page).to have_current_path(admin_settings_storages_path) + expect(page).not_to have_text("Foo Nextcloud") end - it 'renders an edit view', :webmock do - visit edit_admin_settings_storage_path(storage) + context 'with Nextcloud Storage' do + let(:storage) { create(:nextcloud_storage, :as_automatically_managed) } + let(:oauth_application) { create(:oauth_application, integration: storage) } + let(:oauth_client) { create(:oauth_client, integration: storage) } - expect(page).to have_test_selector('storage-name-title', text: storage.name.capitalize) - - aggregate_failures 'Storage edit view' do - # General information - expect(page).to have_test_selector('storage-provider-label', text: 'Storage provider') - expect(page).to have_test_selector('label-host_name_configured-status', text: 'Completed') - expect(page).to have_test_selector('storage-description', text: [storage.short_provider_type.capitalize, - storage.name, - storage.host].join(' - ')) - - # OAuth application - expect(page).to have_test_selector('storage-openproject-oauth-label', text: 'OpenProject OAuth') - expect(page).to have_test_selector('label-openproject_oauth_application_configured-status', text: 'Completed') - expect(page).to have_test_selector('storage-openproject-oauth-application-description', - text: "OAuth Client ID: #{oauth_application.uid}") - - # OAuth client - expect(page).to have_test_selector('storage-oauth-client-label', text: 'Nextcloud OAuth') - expect(page).to have_test_selector('label-storage_oauth_client_configured-status', text: 'Completed') - expect(page).to have_test_selector('storage-oauth-client-id-description', - text: "OAuth Client ID: #{oauth_client.client_id}") - - # Automatically managed project folders - expect(page).to have_test_selector('storage-managed-project-folders-label', - text: 'Automatically managed folders') - - expect(page).to have_test_selector('label-managed-project-folders-status', text: 'Active') - expect(page).to have_test_selector('storage-automatically-managed-project-folders-description', - text: 'Let OpenProject create folders per project automatically.') + before do + oauth_application + oauth_client end - aggregate_failures 'General information' do - # 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]') + it 'renders an edit view', :webmock do + visit edit_admin_settings_storage_path(storage) - fill_in 'storages_nextcloud_storage_name', with: 'My Nextcloud' - click_button 'Save and continue' - end + expect(page).to have_test_selector('storage-name-title', text: storage.name.capitalize) - expect(page).to have_test_selector('storage-name-title', text: 'My Nextcloud') - expect(page).to have_test_selector('storage-description', text: [storage.short_provider_type.capitalize, - 'My Nextcloud', - storage.host].join(' - ')) + aggregate_failures 'Storage edit view' do + # General information + expect(page).to have_test_selector('storage-provider-label', text: 'Storage provider') + expect(page).to have_test_selector('label-host_name_configured-status', text: 'Completed') + expect(page).to have_test_selector('storage-description', text: "Nextcloud - #{storage.name} - #{storage.host}") - # Update a storage - unhappy path - find_test_selector('storage-edit-host-button').click - within_test_selector('storage-general-info-form') do - fill_in 'storages_nextcloud_storage_name', with: nil - fill_in 'storages_nextcloud_storage_host', with: nil - click_button 'Save and continue' + # OAuth application + expect(page).to have_test_selector('storage-openproject-oauth-label', text: 'OpenProject OAuth') + expect(page).to have_test_selector('label-openproject_oauth_application_configured-status', text: 'Completed') + expect(page).to have_test_selector('storage-openproject-oauth-application-description', + text: "OAuth Client ID: #{oauth_application.uid}") - expect(page).to have_text("Name can't be blank.") - expect(page).to have_text("Host is not a valid URL.") + # OAuth client + expect(page).to have_test_selector('storage-oauth-client-label', text: 'Nextcloud OAuth') + expect(page).to have_test_selector('label-storage_oauth_client_configured-status', text: 'Completed') + expect(page).to have_test_selector('storage-oauth-client-id-description', + text: "OAuth Client ID: #{oauth_client.client_id}") - click_link 'Cancel' - end - end + # Automatically managed project folders + expect(page).to have_test_selector('storage-managed-project-folders-label', + text: 'Automatically managed folders') - aggregate_failures 'OAuth application' do - accept_confirm do - find_test_selector('storage-replace-openproject-oauth-application-button').click + expect(page).to have_test_selector('label-managed-project-folders-status', text: 'Active') + expect(page).to have_test_selector('storage-automatically-managed-project-folders-description', + text: 'Let OpenProject create folders per project automatically.') end - within_test_selector('storage-openproject-oauth-application-form') do - warning_section = find_test_selector('storage-openproject_oauth_application_warning') - expect(warning_section).to have_text('The client secret value will not be accessible again after you close ' \ - 'this window. Please copy these values into the Nextcloud ' \ - 'OpenProject Integration settings.') - expect(warning_section).to have_link('Nextcloud OpenProject Integration settings', - href: "#{storage.host}/settings/admin/openproject") + aggregate_failures 'General information' do + # 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_css('#openproject_oauth_application_uid', - value: storage.reload.oauth_application.uid) - expect(page).to have_css('#openproject_oauth_application_secret', - value: storage.reload.oauth_application.secret) + expect(page).to have_test_selector('storage-name-title', text: 'My Nextcloud') + expect(page).to have_test_selector('storage-description', text: "Nextcloud - My Nextcloud - #{storage.host}") - click_link 'Done, continue' + # Update a storage - unhappy path + find_test_selector('storage-edit-host-button').click + within_test_selector('storage-general-info-form') do + fill_in 'storages_nextcloud_storage_name', with: nil + fill_in 'storages_nextcloud_storage_host', with: nil + click_button 'Save and continue' + + expect(page).to have_text("Name can't be blank.") + expect(page).to have_text("Host is not a valid URL.") + + click_link 'Cancel' + end end - end - aggregate_failures 'OAuth Client' do - accept_confirm do - find_test_selector('storage-edit-oauth-client-button').click + aggregate_failures 'OAuth application' do + accept_confirm do + find_test_selector('storage-replace-openproject-oauth-application-button').click + end + + within_test_selector('storage-openproject-oauth-application-form') do + warning_section = find_test_selector('storage-openproject_oauth_application_warning') + expect(warning_section).to have_text('The client secret value will not be accessible again after you close ' \ + 'this window. Please copy these values into the Nextcloud ' \ + 'OpenProject Integration settings.') + expect(warning_section).to have_link('Nextcloud OpenProject Integration settings', + href: "#{storage.host}/settings/admin/openproject") + + expect(page).to have_css('#openproject_oauth_application_uid', + value: storage.reload.oauth_application.uid) + expect(page).to have_css('#openproject_oauth_application_secret', + value: storage.reload.oauth_application.secret) + + click_link 'Done, continue' + end end - within_test_selector('storage-oauth-client-form') do - # With null values, submit button should be disabled - expect(page).to have_css('#oauth_client_client_id', value: '') - expect(page).to have_css('#oauth_client_client_secret', value: '') - expect(find_test_selector('storage-oauth-client-submit-button')).to be_disabled - - # Happy path - Submit valid values - fill_in 'oauth_client_client_id', with: '1234567890' - fill_in 'oauth_client_client_secret', with: '0987654321' - expect(find_test_selector('storage-oauth-client-submit-button')).not_to be_disabled - click_button 'Save and continue' + aggregate_failures 'OAuth Client' do + accept_confirm do + find_test_selector('storage-edit-oauth-client-button').click + end + + within_test_selector('storage-oauth-client-form') do + # With null values, submit button should be disabled + expect(page).to have_css('#oauth_client_client_id', value: '') + expect(page).to have_css('#oauth_client_client_secret', value: '') + expect(find_test_selector('storage-oauth-client-submit-button')).to be_disabled + + # Happy path - Submit valid values + fill_in 'oauth_client_client_id', with: '1234567890' + fill_in 'oauth_client_client_secret', with: '0987654321' + expect(find_test_selector('storage-oauth-client-submit-button')).not_to be_disabled + click_button 'Save and continue' + end + + expect(page).to have_test_selector('label-storage_oauth_client_configured-status', text: 'Completed') + expect(page).to have_test_selector('storage-oauth-client-id-description', text: "OAuth Client ID: 1234567890") end - expect(page).to have_test_selector('label-storage_oauth_client_configured-status', text: 'Completed') - expect(page).to have_test_selector('storage-oauth-client-id-description', text: "OAuth Client ID: 1234567890") + aggregate_failures 'Automatically managed project folders' do + find_test_selector('storage-edit-automatically-managed-project-folders-button').click + + within_test_selector('storage-automatically-managed-project-folders-form') do + automatically_managed_switch = page.find('[name="storages_nextcloud_storage[automatically_managed]"]') + application_password_input = page.find_by_id('storages_nextcloud_storage_password') + expect(automatically_managed_switch).to be_checked + expect(application_password_input.value).to be_empty + + # Clicking submit with application password empty should show an error + click_button('Done, complete setup') + expect(page).to have_text("Password can't be blank.") + + # Test the error path for an invalid storage password. + # Mock a valid response (=401) for example.com, so the password validation should fail + mock_nextcloud_application_credentials_validation(storage.host, password: "1234567890", + response_code: 401) + automatically_managed_switch = page.find('[name="storages_nextcloud_storage[automatically_managed]"]') + expect(automatically_managed_switch).to be_checked + fill_in 'storages_nextcloud_storage_password', with: "1234567890" + # Clicking submit with application password empty should show an error + click_button('Done, complete setup') + expect(page).to have_text("Password is not valid.") + + # Test the happy path for a valid storage password. + # Mock a valid response (=200) for example.com, so the password validation should succeed + # Fill in application password and submit + mock_nextcloud_application_credentials_validation(storage.host, password: "1234567890") + automatically_managed_switch = page.find('[name="storages_nextcloud_storage[automatically_managed]"]') + expect(automatically_managed_switch).to be_checked + fill_in 'storages_nextcloud_storage_password', with: "1234567890" + click_button('Done, complete setup') + end + + expect(page).to have_test_selector('label-managed-project-folders-status', text: 'Active') + end end + end - aggregate_failures 'Automatically managed project folders' do - find_test_selector('storage-edit-automatically-managed-project-folders-button').click - - within_test_selector('storage-automatically-managed-project-folders-form') do - automatically_managed_switch = page.find('[name="storages_nextcloud_storage[automatically_managed]"]') - application_password_input = page.find_by_id('storages_nextcloud_storage_password') - expect(automatically_managed_switch).to be_checked - expect(application_password_input.value).to be_empty - - # Clicking submit with application password empty should show an error - click_button('Done, complete setup') - expect(page).to have_text("Password can't be blank.") - - # Test the error path for an invalid storage password. - # Mock a valid response (=401) for example.com, so the password validation should fail - mock_nextcloud_application_credentials_validation(storage.host, password: "1234567890", - response_code: 401) - automatically_managed_switch = page.find('[name="storages_nextcloud_storage[automatically_managed]"]') - expect(automatically_managed_switch).to be_checked - fill_in 'storages_nextcloud_storage_password', with: "1234567890" - # Clicking submit with application password empty should show an error - click_button('Done, complete setup') - expect(page).to have_text("Password is not valid.") - - # Test the happy path for a valid storage password. - # Mock a valid response (=200) for example.com, so the password validation should succeed - # Fill in application password and submit - mock_nextcloud_application_credentials_validation(storage.host, password: "1234567890") - automatically_managed_switch = page.find('[name="storages_nextcloud_storage[automatically_managed]"]') - expect(automatically_managed_switch).to be_checked - fill_in 'storages_nextcloud_storage_password', with: "1234567890" - click_button('Done, complete setup') + context 'with OneDrive Storage' do + let(:storage) { create(:one_drive_storage, name: 'Test Drive') } + let(:oauth_client) { create(:oauth_client, integration: storage) } + + before { oauth_client } + + 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') + + aggregate_failures 'Storage edit view' do + # General information + expect(page).to have_test_selector('storage-provider-label', text: 'Storage provider') + expect(page).to have_test_selector('label-host_name_configured-storage_tenant_drive_configured-status', + text: 'Completed') + expect(page).to have_test_selector('storage-description', text: 'OneDrive/SharePoint - Test Drive') + + # OAuth client + expect(page).to have_test_selector('storage-oauth-client-label', text: 'Azure OAuth') + expect(page).to have_test_selector('label-storage_oauth_client_configured-status', text: 'Completed') + expect(page).to have_test_selector('storage-oauth-client-id-description', + text: "OAuth Client ID: #{oauth_client.client_id}") end - expect(page).to have_test_selector('label-managed-project-folders-status', text: 'Active') - end - end + aggregate_failures 'General information' do + # 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]') - it 'renders a delete button' do - storage = create(:nextcloud_storage, name: "Foo Nextcloud") - visit edit_admin_settings_storage_path(storage) + fill_in 'storages_one_drive_storage_name', with: 'My OneDrive' + click_button 'Save and continue' + end - storage_delete_button = find_test_selector('storage-delete-button') - expect(storage_delete_button).to have_text('Delete') + expect(page).to have_test_selector('storage-name-title', text: 'My OneDrive') + expect(page).to have_test_selector('storage-description', text: 'OneDrive/SharePoint - My OneDrive') - accept_confirm do - storage_delete_button.click - end + # Update a storage - unhappy path + find_test_selector('storage-edit-host-button').click + within_test_selector('storage-general-info-form') do + fill_in 'storages_one_drive_storage_name', with: nil + fill_in 'storages_one_drive_storage_drive_id', with: nil + click_button 'Save and continue' - expect(page).to have_current_path(admin_settings_storages_path) - expect(page).not_to have_text("Foo Nextcloud") + expect(page).to have_text("Name can't be blank.") + expect(page).to have_text("Drive can't be blank.") + + click_link 'Cancel' + end + end + + aggregate_failures 'OAuth Client' do + accept_confirm do + find_test_selector('storage-edit-oauth-client-button').click + end + + within_test_selector('storage-oauth-client-form') do + # With null values, submit button should be disabled + expect(page).to have_css('#oauth_client_client_id', value: '') + expect(page).to have_css('#oauth_client_client_secret', value: '') + expect(find_test_selector('storage-oauth-client-submit-button')).to be_disabled + + # Happy path - Submit valid values + fill_in 'oauth_client_client_id', with: '1234567890' + fill_in 'oauth_client_client_secret', with: '0987654321' + expect(find_test_selector('storage-oauth-client-submit-button')).not_to be_disabled + click_button 'Save and continue' + end + + expect(page).to have_test_selector('label-storage_oauth_client_configured-status', text: 'Completed') + expect(page).to have_test_selector('storage-oauth-client-id-description', text: "OAuth Client ID: 1234567890") + end + end end end diff --git a/modules/storages/spec/models/nextcloud_storage_spec.rb b/modules/storages/spec/models/nextcloud_storage_spec.rb index 25a03819a16f..7996b84c89fc 100644 --- a/modules/storages/spec/models/nextcloud_storage_spec.rb +++ b/modules/storages/spec/models/nextcloud_storage_spec.rb @@ -180,6 +180,31 @@ it_behaves_like 'a stored boolean attribute', :automatically_managed end + describe '#automatic_management_new_record?' do + context 'when automatic management has just been specified but not yet persisted' do + let(:storage) { build_stubbed(:nextcloud_storage, provider_fields: {}) } + + before { storage.automatically_managed = false } + + it { expect(storage).to be_provider_fields_changed } + it { expect(storage).to be_automatic_management_new_record } + end + + context 'when automatic management was already specified' do + let(:storage) { build_stubbed(:nextcloud_storage, :as_not_automatically_managed) } + + it { expect(storage).not_to be_provider_fields_changed } + it { expect(storage).not_to be_automatic_management_new_record } + end + + context 'when automatic management is unspecified' do + let(:storage) { build_stubbed(:nextcloud_storage, provider_fields: {}) } + + it { expect(storage).not_to be_provider_fields_changed } + it { expect(storage).to be_automatic_management_new_record } + end + end + describe '#automatic_management_unspecified?' do context 'when automatically_managed is nil' do let(:storage) { build(:nextcloud_storage, automatically_managed: nil) } From 55bd6e6a6524e3f7de9b77691cbca9bb2508274a Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Fri, 10 Nov 2023 17:42:20 +0300 Subject: [PATCH 42/56] tests[Op#50737]: Add New File storage feature specs --- .../admin/oauth_clients_controller.rb | 1 - .../spec/features/admin_storages_spec.rb | 188 ++++++++++++++++++ 2 files changed, 188 insertions(+), 1 deletion(-) diff --git a/modules/storages/app/controllers/storages/admin/oauth_clients_controller.rb b/modules/storages/app/controllers/storages/admin/oauth_clients_controller.rb index 02c05533ce22..9dd1a2088f70 100644 --- a/modules/storages/app/controllers/storages/admin/oauth_clients_controller.rb +++ b/modules/storages/app/controllers/storages/admin/oauth_clients_controller.rb @@ -159,7 +159,6 @@ def find_storage end def delete_current_oauth_client - @previous_oauth_client_present = @storage.oauth_client.present? ::OAuthClients::DeleteService.new(user: User.current, model: @storage.oauth_client).call if @storage.oauth_client end end diff --git a/modules/storages/spec/features/admin_storages_spec.rb b/modules/storages/spec/features/admin_storages_spec.rb index 868dd8ea1664..37fa86bb4cb6 100644 --- a/modules/storages/spec/features/admin_storages_spec.rb +++ b/modules/storages/spec/features/admin_storages_spec.rb @@ -89,6 +89,194 @@ end end + describe 'New file storage', with_flag: { storage_primer_design: true } do + context 'with Nextcloud Storage' do + 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) + + aggregate_failures 'Select provider view' do + # General information + expect(page).to have_select('storages_storage[provider_type]', with_options: %w[Nextcloud OneDrive/SharePoint]) + + # Select Nextcloud + select('Nextcloud', from: 'storages_storage[provider_type]') + click_button('Save and continue') + + # OAuth application + expect(page).to have_test_selector('storage-openproject-oauth-label', text: 'OpenProject OAuth') + expect(page).to have_test_selector('label-openproject_oauth_application_configured-status', text: 'Incomplete') + + # OAuth client + expect(page).to have_test_selector('storage-oauth-client-label', text: 'Nextcloud OAuth') + expect(page).to have_test_selector('label-storage_oauth_client_configured-status', text: 'Incomplete') + expect(page).to have_test_selector('storage-oauth-client-id-description', + text: "Allow OpenProject to access Nextcloud data using OAuth.") + + # Automatically managed project folders + expect(page).to have_test_selector('storage-managed-project-folders-label', + text: 'Automatically managed folders') + expect(page).to have_test_selector('label-managed-project-folders-status', text: 'Incomplete') + expect(page).to have_test_selector('storage-automatically-managed-project-folders-description', + text: 'Let OpenProject create folders per project automatically.') + end + + aggregate_failures 'General information' do + within_test_selector('storage-general-info-form') do + fill_in 'storages_nextcloud_storage_name', with: 'My Nextcloud' + click_button 'Save and continue' + + expect(page).to have_text("Host is not a valid URL.") + + mock_server_capabilities_response("https://example.com") + mock_server_config_check_response("https://example.com") + fill_in 'storages_nextcloud_storage_host', with: 'https://example.com' + click_button 'Save and continue' + end + + expect(page).to have_test_selector('label-host_name_configured-status', text: 'Completed') + expect(page).to have_test_selector('storage-description', text: "Nextcloud - My Nextcloud - https://example.com") + end + + aggregate_failures 'OAuth application' do + within_test_selector('storage-openproject-oauth-application-form') do + warning_section = find_test_selector('storage-openproject_oauth_application_warning') + expect(warning_section).to have_text('The client secret value will not be accessible again after you close ' \ + 'this window. Please copy these values into the Nextcloud ' \ + 'OpenProject Integration settings.') + expect(warning_section).to have_link('Nextcloud OpenProject Integration settings', + href: "https://example.com/settings/admin/openproject") + + storage = Storages::NextcloudStorage.find_by(host: 'https://example.com') + expect(page).to have_css('#openproject_oauth_application_uid', + value: storage.reload.oauth_application.uid) + expect(page).to have_css('#openproject_oauth_application_secret', + value: storage.reload.oauth_application.secret) + + click_link 'Done, continue' + end + end + + aggregate_failures 'OAuth Client' do + within_test_selector('storage-oauth-client-form') do + # With null values, submit button should be disabled + expect(page).to have_css('#oauth_client_client_id', value: '') + expect(page).to have_css('#oauth_client_client_secret', value: '') + expect(find_test_selector('storage-oauth-client-submit-button')).to be_disabled + + # Happy path - Submit valid values + fill_in 'oauth_client_client_id', with: '1234567890' + fill_in 'oauth_client_client_secret', with: '0987654321' + expect(find_test_selector('storage-oauth-client-submit-button')).not_to be_disabled + click_button 'Save and continue' + end + + expect(page).to have_test_selector('label-storage_oauth_client_configured-status', text: 'Completed') + expect(page).to have_test_selector('storage-oauth-client-id-description', text: "OAuth Client ID: 1234567890") + end + + aggregate_failures 'Automatically managed project folders' do + within_test_selector('storage-automatically-managed-project-folders-form') do + automatically_managed_switch = page.find('[name="storages_nextcloud_storage[automatically_managed]"]') + application_password_input = page.find_by_id('storages_nextcloud_storage_password') + expect(automatically_managed_switch).to be_checked + expect(application_password_input.value).to be_empty + + # Clicking submit with application password empty should show an error + click_button('Done, complete setup') + expect(page).to have_text("Password can't be blank.") + + # Test the error path for an invalid storage password. + # Mock a valid response (=401) for example.com, so the password validation should fail + mock_nextcloud_application_credentials_validation('https://example.com', password: "1234567890", + response_code: 401) + automatically_managed_switch = page.find('[name="storages_nextcloud_storage[automatically_managed]"]') + expect(automatically_managed_switch).to be_checked + fill_in 'storages_nextcloud_storage_password', with: "1234567890" + # Clicking submit with application password empty should show an error + click_button('Done, complete setup') + expect(page).to have_text("Password is not valid.") + + # Test the happy path for a valid storage password. + # Mock a valid response (=200) for example.com, so the password validation should succeed + # Fill in application password and submit + mock_nextcloud_application_credentials_validation('https://example.com', password: "1234567890") + automatically_managed_switch = page.find('[name="storages_nextcloud_storage[automatically_managed]"]') + expect(automatically_managed_switch).to be_checked + fill_in 'storages_nextcloud_storage_password', with: "1234567890" + click_button('Done, complete setup') + end + + expect(page).to have_current_path(admin_settings_storages_path) + expect(page).to have_text("Storage connected successfully! Remember to activate the module and the specific " \ + "storage in the project settings of each desired project to use it.") + end + end + end + + context 'with OneDrive Storage' do + 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) + + aggregate_failures 'Select provider view' do + # General information + expect(page).to have_select('storages_storage[provider_type]', with_options: %w[Nextcloud OneDrive/SharePoint]) + + # Select OneDrive + select('OneDrive/SharePoint', from: 'storages_storage[provider_type]') + click_button('Save and continue') + + # OAuth client + expect(page).to have_test_selector('storage-oauth-client-label', text: 'Azure OAuth') + expect(page).to have_test_selector('label-storage_oauth_client_configured-status', text: 'Incomplete') + expect(page).to have_test_selector('storage-oauth-client-id-description', + text: "Allow OpenProject to access Azure data using OAuth " \ + "to connect OneDrive/Sharepoint.") + end + + aggregate_failures 'General information' do + within_test_selector('storage-general-info-form') do + fill_in 'storages_one_drive_storage_name', with: 'My OneDrive' + click_button 'Save and continue' + + expect(page).to have_text("Drive can't be blank.") + + fill_in 'storages_one_drive_storage_drive_id', with: '1234567890' + click_button 'Save and continue' + end + + expect(page).to have_test_selector('label-host_name_configured-storage_tenant_drive_configured-status', + text: 'Completed') + expect(page).to have_test_selector('storage-description', text: 'OneDrive/SharePoint - My OneDrive') + end + + aggregate_failures 'OAuth Client' do + within_test_selector('storage-oauth-client-form') do + # With null values, submit button should be disabled + expect(page).to have_css('#oauth_client_client_id', value: '') + expect(page).to have_css('#oauth_client_client_secret', value: '') + expect(find_test_selector('storage-oauth-client-submit-button')).to be_disabled + + # Happy path - Submit valid values + fill_in 'oauth_client_client_id', with: '1234567890' + fill_in 'oauth_client_client_secret', with: '0987654321' + expect(find_test_selector('storage-oauth-client-submit-button')).not_to be_disabled + click_button 'Save and continue' + end + + expect(page).to have_current_path(admin_settings_storages_path) + expect(page).to have_text("Storage connected successfully! Remember to activate the module and the specific " \ + "storage in the project settings of each desired project to use it.") + end + end + end + end + describe 'File storage edit view', with_flag: { storage_primer_design: true } do it 'renders a delete button' do storage = create(:nextcloud_storage, name: "Foo Nextcloud") From 113dae482aaee9fd7b34ec25cab87a188996a60c Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Sun, 12 Nov 2023 10:38:08 +0300 Subject: [PATCH 43/56] feat[Op#50737]: Auto retrieve provider form on select change --- .../select-provider-form.controller.ts | 58 +++++++++++++++++++ .../forms/general_info_form_component.rb | 24 +++++--- ...select_storage_provider_component.html.erb | 26 ++++----- .../admin/storage_view_component.html.erb | 7 +-- .../storages/admin/storages_controller.rb | 5 ++ .../admin/provider_type_select_form.rb | 4 +- .../admin/storages/new.turbo_stream.erb | 34 +++++++++++ .../spec/features/admin_storages_spec.rb | 26 +++++---- 8 files changed, 144 insertions(+), 40 deletions(-) create mode 100644 frontend/src/stimulus/controllers/dynamic/storages/select-provider-form.controller.ts create mode 100644 modules/storages/app/views/storages/admin/storages/new.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 new file mode 100644 index 000000000000..83447ea62a84 --- /dev/null +++ b/frontend/src/stimulus/controllers/dynamic/storages/select-provider-form.controller.ts @@ -0,0 +1,58 @@ +/* + * -- 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/forms/general_info_form_component.rb b/modules/storages/app/components/storages/admin/forms/general_info_form_component.rb index c1c63751ee52..499d2db43864 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 @@ -34,8 +34,7 @@ class GeneralInfoFormComponent < ApplicationComponent alias_method :storage, :model options form_method: :post, - submit_button_disabled: false, - cancel_button_should_break_from_frame: false + submit_button_disabled: false def form_url options[:form_url] || default_form_url @@ -46,9 +45,10 @@ def submit_button_options end def cancel_button_options - { href: cancel_button_path }.tap do |options_hash| - options_hash[:target] = '_top' if cancel_button_should_break_from_frame - end + { + href: cancel_button_path, + data: { turbo_stream: true } + } end private @@ -56,14 +56,22 @@ def cancel_button_options def default_form_url case form_method when :get, :post - Rails.application.routes.url_helpers.admin_settings_storages_path + admin_settings_storages_path when :patch, :put - Rails.application.routes.url_helpers.admin_settings_storage_path(storage) + admin_settings_storage_path(storage) end end def cancel_button_path - options[:cancel_button_path] || Rails.application.routes.url_helpers.admin_settings_storages_path + options.fetch(:cancel_button_path) do + if storage.new_record? + new_admin_settings_storage_path + elsif storage.persisted? + edit_admin_settings_storage_path(storage) + else + admin_settings_storages_path + end + end end end end 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 index 9cc8e8557f4d..ebe760e5d2cb 100644 --- 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 @@ -20,22 +20,23 @@ model: storage, url: select_provider_admin_settings_storages_path, data: { - storages__select_provider_form_provider_type_value: storage.provider_type, 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: { - # FIXME: Auto submit on provider select - # data: { action: 'change->storages--select-provider-form#showProviderForm' }, - include_blank: I18n.t('storages.label_select_provider'), - } - ) + 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 @@ -45,8 +46,7 @@ form, storage:, submit_button_options: { - # FIXME: change to disabled: true, should Auto submit on provider select - disabled: false, + disabled: true, test_selector: 'storage-select-provider-submit-button' }, cancel_button_options: { diff --git a/modules/storages/app/components/storages/admin/storage_view_component.html.erb b/modules/storages/app/components/storages/admin/storage_view_component.html.erb index 7a71b34d5cf0..6b3b34319da0 100644 --- a/modules/storages/app/components/storages/admin/storage_view_component.html.erb +++ b/modules/storages/app/components/storages/admin/storage_view_component.html.erb @@ -7,12 +7,7 @@ component.with_row(scheme: :default) do render(OpTurbo::FrameComponent.new(id: :storage_general_info_section)) do if storage.new_record? - render( - Storages::Admin::Forms::GeneralInfoFormComponent.new( - storage, - cancel_button_should_break_from_frame: true # Return to index page on Cancel - ) - ) + render(Storages::Admin::Forms::GeneralInfoFormComponent.new(storage)) else render(Storages::Admin::GeneralInfoComponent.new(storage)) end diff --git a/modules/storages/app/controllers/storages/admin/storages_controller.rb b/modules/storages/app/controllers/storages/admin/storages_controller.rb index 6cab7d6845c0..de1d75325290 100644 --- a/modules/storages/app/controllers/storages/admin/storages_controller.rb +++ b/modules/storages/app/controllers/storages/admin/storages_controller.rb @@ -72,6 +72,11 @@ def new contract_class: EmptyContract) .call .result + + respond_to do |format| + format.html + format.turbo_stream + end end def select_provider 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 index 37e80c9a2e04..1f9a9f489a98 100644 --- a/modules/storages/app/forms/storages/admin/provider_type_select_form.rb +++ b/modules/storages/app/forms/storages/admin/provider_type_select_form.rb @@ -30,7 +30,7 @@ module Storages::Admin class ProviderTypeSelectForm < ApplicationForm form do |storage_form| storage_form.select_list(**@select_list_options) do |storage_provider_list| - if @storage.persisted? + if @storage.provider_type.present? storage_provider_list.option( label: I18n.t("storages.provider_types.#{@storage.short_provider_type}.name"), value: @storage.provider_type @@ -60,7 +60,7 @@ def default_select_list_options(storage) label: I18n.t('activerecord.attributes.storages/storage.provider_type'), caption: I18n.t('storages.instructions.provider_type', type_link_text: I18n.t('storages.instructions.type_link_text')), - include_blank: storage.new_record?, + include_blank: false, required: true, disabled: storage.persisted? } 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 new file mode 100644 index 000000000000..036c0a4897ce --- /dev/null +++ b/modules/storages/app/views/storages/admin/storages/new.turbo_stream.erb @@ -0,0 +1,34 @@ +<%#-- 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.replace :select_storage_provider do %> + <%= + render(::Storages::Admin::SelectStorageProviderComponent.new(@storage)) + %> +<% end %> diff --git a/modules/storages/spec/features/admin_storages_spec.rb b/modules/storages/spec/features/admin_storages_spec.rb index 37fa86bb4cb6..f924f42117a9 100644 --- a/modules/storages/spec/features/admin_storages_spec.rb +++ b/modules/storages/spec/features/admin_storages_spec.rb @@ -100,14 +100,16 @@ 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 # Select Nextcloud select('Nextcloud', from: 'storages_storage[provider_type]') - click_button('Save and continue') # OAuth application - expect(page).to have_test_selector('storage-openproject-oauth-label', text: 'OpenProject OAuth') - expect(page).to have_test_selector('label-openproject_oauth_application_configured-status', text: 'Incomplete') + wait_for do + expect(page).to have_test_selector('storage-openproject-oauth-label', text: 'OpenProject OAuth') + expect(page).to have_test_selector('label-openproject_oauth_application_configured-status', text: 'Incomplete') + end # OAuth client expect(page).to have_test_selector('storage-oauth-client-label', text: 'Nextcloud OAuth') @@ -125,7 +127,7 @@ aggregate_failures 'General information' do within_test_selector('storage-general-info-form') do - fill_in 'storages_nextcloud_storage_name', with: 'My Nextcloud' + fill_in 'storages_nextcloud_storage_name', with: 'My Nextcloud', fill_options: { clear: :backspace } click_button 'Save and continue' expect(page).to have_text("Host is not a valid URL.") @@ -226,22 +228,24 @@ 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 # Select OneDrive select('OneDrive/SharePoint', from: 'storages_storage[provider_type]') - click_button('Save and continue') # OAuth client - expect(page).to have_test_selector('storage-oauth-client-label', text: 'Azure OAuth') - expect(page).to have_test_selector('label-storage_oauth_client_configured-status', text: 'Incomplete') - expect(page).to have_test_selector('storage-oauth-client-id-description', - text: "Allow OpenProject to access Azure data using OAuth " \ - "to connect OneDrive/Sharepoint.") + wait_for do + expect(page).to have_test_selector('storage-oauth-client-label', text: 'Azure OAuth') + expect(page).to have_test_selector('label-storage_oauth_client_configured-status', text: 'Incomplete') + expect(page).to have_test_selector('storage-oauth-client-id-description', + text: "Allow OpenProject to access Azure data using OAuth " \ + "to connect OneDrive/Sharepoint.") + end end aggregate_failures 'General information' do within_test_selector('storage-general-info-form') do - fill_in 'storages_one_drive_storage_name', with: 'My OneDrive' + fill_in 'storages_one_drive_storage_name', with: 'My OneDrive', fill_options: { clear: :backspace } click_button 'Save and continue' expect(page).to have_text("Drive can't be blank.") From 6c12920deb66c0d7ab5208eb1f2a2235769c9b35 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Sun, 12 Nov 2023 13:35:34 +0300 Subject: [PATCH 44/56] fix[Op#50737]: Switch select caption by provider type --- .../general_info_form_component.html.erb | 8 ++++++- .../forms/general_info_form_component.rb | 24 +++++++++++++++++++ .../admin/provider_type_select_form.rb | 3 +-- modules/storages/config/locales/en.yml | 5 +++- 4 files changed, 36 insertions(+), 4 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 14d38c476955..dce5a61399da 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,7 +7,13 @@ ) do |form| flex_layout do |general_info_row| general_info_row.with_row(mb: 3) do - render(Storages::Admin::ProviderTypeSelectForm.new(form, storage:)) + render( + Storages::Admin::ProviderTypeSelectForm.new( + form, + storage:, + select_list_options: { caption: provider_type_select_caption } + ) + ) 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 499d2db43864..972bd896b9b8 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 @@ -73,5 +73,29 @@ def cancel_button_path end end end + + def provider_type_select_caption + return if storage.provider_type.blank? + + if storage.provider_type_nextcloud? + caption_for_provider_type(:nextcloud, "https://apps.nextcloud.com/apps/integration_openproject") + elsif storage.provider_type_one_drive? + caption_for_provider_type(:one_drive, "https://docs.microsoft.com/en-us/graph/auth-register-app-v2") + end + end + + def caption_for_provider_type(provider_type, href) + I18n.t( + "storages.instructions.#{provider_type}.provider_configuration", + application_link_text: application_link_text_for( + href, + I18n.t("storages.instructions.#{provider_type}.application_link_text") + ) + ).html_safe + end + + def application_link_text_for(href, link_text) + render(Primer::Beta::Link.new(href:, target: '_blank')) { link_text } + 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 index 1f9a9f489a98..167135024829 100644 --- a/modules/storages/app/forms/storages/admin/provider_type_select_form.rb +++ b/modules/storages/app/forms/storages/admin/provider_type_select_form.rb @@ -58,8 +58,7 @@ def default_select_list_options(storage) { name: :provider_type, label: I18n.t('activerecord.attributes.storages/storage.provider_type'), - caption: I18n.t('storages.instructions.provider_type', - type_link_text: I18n.t('storages.instructions.type_link_text')), + caption: nil, include_blank: false, required: true, disabled: storage.persisted? diff --git a/modules/storages/config/locales/en.yml b/modules/storages/config/locales/en.yml index fdad9b5a8e84..3e4309c674fa 100644 --- a/modules/storages/config/locales/en.yml +++ b/modules/storages/config/locales/en.yml @@ -110,7 +110,6 @@ en: title: "Members connection status" subtitle: "Check the connection status for the storage %{storage_name_link} of all project members." instructions: - provider_type: "Please make sure you have administration privileges in your Nextcloud instance and the application %{type_link_text} is installed before doing the setup." type: "Please make sure you have administration privileges in your Nextcloud instance and have the following application installed before doing the setup:" type_link_text: "“Integration OpenProject”" name: "Give your storage a name so that users can differentiate between multiple storages." @@ -136,8 +135,12 @@ en: oauth_application_details_link_text: "Nextcloud OpenProject Integration settings" copy_from: "Copy this value from" nextcloud: + provider_configuration: "Please make sure you have administration privileges in your Nextcloud instance and the %{application_link_text} is installed before doing the setup." + application_link_text: "application “Integration OpenProject”" integration: "Nextcloud Administration / OpenProject" one_drive: + provider_configuration: "Please make sure you have administration privileges in the %{application_link_text} before doing the setup." + application_link_text: "Azure application" integration: "OneDrive/SharePoint" oauth_client_id: > Copy the client id from the Azure portal. This is needed to generate the redirect URI. From b0cd23f4b3b849d9c19d1bd339425f1848969856 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Sun, 12 Nov 2023 14:10:07 +0300 Subject: [PATCH 45/56] chore[Op#50737]: define provider docs links --- lib/open_project/static/links.rb | 6 ++++++ .../admin/forms/general_info_form_component.rb | 10 +++------- .../app/views/storages/admin/storages/new.html.erb | 8 +++++++- modules/storages/config/locales/en.yml | 3 ++- 4 files changed, 18 insertions(+), 9 deletions(-) diff --git a/lib/open_project/static/links.rb b/lib/open_project/static/links.rb index 2c1596131ce1..126c12fe79c6 100644 --- a/lib/open_project/static/links.rb +++ b/lib/open_project/static/links.rb @@ -258,6 +258,12 @@ def static_links storage_docs: { setup: { href: 'https://www.openproject.org/docs/system-admin-guide/integrations/nextcloud/' + }, + nextcloud_oauth_application: { + href: 'https://apps.nextcloud.com/apps/integration_openproject' + }, + one_drive_oauth_application: { + href: 'https://docs.microsoft.com/en-us/graph/auth-register-app-v2' } }, ical_docs: { 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 972bd896b9b8..c6778138ef87 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 @@ -77,18 +77,14 @@ def cancel_button_path def provider_type_select_caption return if storage.provider_type.blank? - if storage.provider_type_nextcloud? - caption_for_provider_type(:nextcloud, "https://apps.nextcloud.com/apps/integration_openproject") - elsif storage.provider_type_one_drive? - caption_for_provider_type(:one_drive, "https://docs.microsoft.com/en-us/graph/auth-register-app-v2") - end + caption_for_provider_type(storage.short_provider_type) end - def caption_for_provider_type(provider_type, href) + def caption_for_provider_type(provider_type) I18n.t( "storages.instructions.#{provider_type}.provider_configuration", application_link_text: application_link_text_for( - href, + ::OpenProject::Static::Links[:storage_docs][:"#{provider_type}_oauth_application"][:href], I18n.t("storages.instructions.#{provider_type}.application_link_text") ) ).html_safe 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 a198fca81316..2de44f42eb4b 100644 --- a/modules/storages/app/views/storages/admin/storages/new.html.erb +++ b/modules/storages/app/views/storages/admin/storages/new.html.erb @@ -9,7 +9,13 @@ <% end %> <% header.with_description do %> - <%= t("storages.instructions.new_storage") %> + <%= + 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 %> diff --git a/modules/storages/config/locales/en.yml b/modules/storages/config/locales/en.yml index 3e4309c674fa..cd1eb3e8ed7d 100644 --- a/modules/storages/config/locales/en.yml +++ b/modules/storages/config/locales/en.yml @@ -117,7 +117,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 file storage documentation for more information about this setup." + new_storage: "Read our %{new_storage_link_text} for more information about this setup." + new_storage_link_text: "file storage documentation" 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 fc88cdeeda3c4528bd5fadaac7c01561ea3055be Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Sun, 12 Nov 2023 15:13:38 +0300 Subject: [PATCH 46/56] fix[Op#50737]: Fix clipboard copy by setting HTML value on clientID input --- .../storages/oauth-client-form.controller.ts | 16 ++++++++++------ .../forms/oauth_client_form_component.html.erb | 3 ++- .../storages/one_drive/_oauth_client.html.erb | 3 ++- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/frontend/src/stimulus/controllers/dynamic/storages/oauth-client-form.controller.ts b/frontend/src/stimulus/controllers/dynamic/storages/oauth-client-form.controller.ts index e1e82369c31a..74953341175e 100644 --- a/frontend/src/stimulus/controllers/dynamic/storages/oauth-client-form.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/storages/oauth-client-form.controller.ts @@ -47,17 +47,13 @@ export default class OAuthClientFormController extends Controller { declare readonly hasSubmitButtonTarget:boolean; declare readonly clientIdTarget:HTMLInputElement; declare readonly clientSecretTarget:HTMLInputElement; - declare readonly redirectUriTarget:HTMLInputElement; + declare readonly redirectUriTargets:HTMLInputElement[]; declare readonly submitButtonTarget:HTMLInputElement; declare readonly clientIdMissingValue:string; connect():void { - this.clientIdTarget.addEventListener('input', () => { - this.redirectUriTarget.value = this.redirectUri(); - }); - - this.redirectUriTarget.value = this.redirectUri(); + this.setRedirectUriValue(); this.toggleSubmitButtonDisabled(); } @@ -75,6 +71,14 @@ export default class OAuthClientFormController extends Controller { } } + public setRedirectUriValue():void { + const newValue = this.redirectUri(); + this.redirectUriTargets.forEach((target) => { + target.value = newValue; + target.setAttribute('value', newValue); + }); + } + private redirectUri():string { if (this.clientIdTarget.value === '') { return this.clientIdMissingValue; diff --git a/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb b/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb index 1c3897c11b4c..d929d467b1f3 100644 --- a/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb +++ b/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb @@ -27,7 +27,8 @@ client_id_input_options: { data: { 'storages--oauth-client-form-target': 'clientId', - action: 'input->storages--oauth-client-form#toggleSubmitButtonDisabled' + action: 'input->storages--oauth-client-form#toggleSubmitButtonDisabled '\ + 'input->storages--oauth-client-form#setRedirectUriValue' } }, client_secret_input_options: { diff --git a/modules/storages/app/views/storages/admin/storages/one_drive/_oauth_client.html.erb b/modules/storages/app/views/storages/admin/storages/one_drive/_oauth_client.html.erb index 5ab621612776..f007793e9628 100644 --- a/modules/storages/app/views/storages/admin/storages/one_drive/_oauth_client.html.erb +++ b/modules/storages/app/views/storages/admin/storages/one_drive/_oauth_client.html.erb @@ -39,7 +39,8 @@ See COPYRIGHT and LICENSE files for more details. size: 40, container_class: '-wide', data: { - 'storages--oauth-client-form-target': "clientId" + 'storages--oauth-client-form-target': "clientId", + action: 'input->storages--oauth-client-form#setRedirectUriValue' } %> <%= t("storages.instructions.#{@storage.short_provider_type}.oauth_client_id") %> From 79497e0896a9a1ce9d591d0c610d93609ff493ce Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Mon, 13 Nov 2023 09:29:45 +0300 Subject: [PATCH 47/56] fix[Op#50737]: Use RSpec wait correctly, as a drop in replacement for expect --- .../spec/features/admin_storages_spec.rb | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/modules/storages/spec/features/admin_storages_spec.rb b/modules/storages/spec/features/admin_storages_spec.rb index f924f42117a9..679e6f8cba2a 100644 --- a/modules/storages/spec/features/admin_storages_spec.rb +++ b/modules/storages/spec/features/admin_storages_spec.rb @@ -106,13 +106,11 @@ select('Nextcloud', from: 'storages_storage[provider_type]') # OAuth application - wait_for do - expect(page).to have_test_selector('storage-openproject-oauth-label', text: 'OpenProject OAuth') - expect(page).to have_test_selector('label-openproject_oauth_application_configured-status', text: 'Incomplete') - end + expect(page).to have_test_selector('storage-openproject-oauth-label', text: 'OpenProject OAuth') + expect(page).to have_test_selector('label-openproject_oauth_application_configured-status', text: 'Incomplete') # OAuth client - expect(page).to have_test_selector('storage-oauth-client-label', text: 'Nextcloud OAuth') + wait_for(page).to have_test_selector('storage-oauth-client-label', text: 'Nextcloud OAuth') expect(page).to have_test_selector('label-storage_oauth_client_configured-status', text: 'Incomplete') expect(page).to have_test_selector('storage-oauth-client-id-description', text: "Allow OpenProject to access Nextcloud data using OAuth.") @@ -234,13 +232,11 @@ select('OneDrive/SharePoint', from: 'storages_storage[provider_type]') # OAuth client - wait_for do - expect(page).to have_test_selector('storage-oauth-client-label', text: 'Azure OAuth') - expect(page).to have_test_selector('label-storage_oauth_client_configured-status', text: 'Incomplete') - expect(page).to have_test_selector('storage-oauth-client-id-description', - text: "Allow OpenProject to access Azure data using OAuth " \ - "to connect OneDrive/Sharepoint.") - end + wait_for(page).to have_test_selector('storage-oauth-client-label', text: 'Azure OAuth') + expect(page).to have_test_selector('label-storage_oauth_client_configured-status', text: 'Incomplete') + expect(page).to have_test_selector('storage-oauth-client-id-description', + text: "Allow OpenProject to access Azure data using OAuth " \ + "to connect OneDrive/Sharepoint.") end aggregate_failures 'General information' do From 46562362cc3da56b1d288e919693aa5a8386e4e6 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Mon, 13 Nov 2023 14:18:33 +0300 Subject: [PATCH 48/56] chore[Op#50737]: render oauth clients instructions by provider --- .../oauth_client_form_component.html.erb | 2 +- .../forms/oauth_client_form_component.rb | 24 ++++++++++--------- modules/storages/config/locales/en.yml | 2 ++ .../spec/features/admin_storages_spec.rb | 11 +++++++-- 4 files changed, 25 insertions(+), 14 deletions(-) diff --git a/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb b/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb index d929d467b1f3..0f23cfbd8dd8 100644 --- a/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb +++ b/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb @@ -16,7 +16,7 @@ end oauth_client_row.with_row(mb: 3) do - render(Primer::Beta::Text.new(font_weight: :light)) { storage_provider_credentials_copy_instructions } + render(Primer::Beta::Text.new(font_weight: :light, test_selector: 'storage-provider-credentials-instructions')) { storage_provider_credentials_instructions } end oauth_client_row.with_row(mb: 3) do diff --git a/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.rb b/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.rb index 65634cd99f06..1a5e0db536dd 100644 --- a/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.rb +++ b/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.rb @@ -67,21 +67,23 @@ def redirect_uri_or_instructions end end - def storage_provider_credentials_copy_instructions - "#{I18n.t('storages.instructions.copy_from')}: #{provider_credentials_instructions_link}".html_safe + def storage_provider_credentials_instructions + I18n.t("storages.instructions.#{storage.short_provider_type}.oauth_configuration", + application_link_text: send(:"#{storage.short_provider_type}_integration_link")).html_safe end - def provider_credentials_instructions_link - render( - Primer::Beta::Link.new( - href: Storages::Peripherals::StorageInteraction::Nextcloud::Util.join_uri_path(storage.host, - 'settings/admin/openproject'), - target: '_blank' - ) - ) { I18n.t("storages.instructions.#{@storage.short_provider_type}.integration") } + private + + def one_drive_integration_link(target: '_blank') + href = ::OpenProject::Static::Links[:storage_docs][:one_drive_oauth_application][:href] + render(Primer::Beta::Link.new(href:, target:)) { I18n.t('storages.instructions.one_drive.application_link_text') } end - private + def nextcloud_integration_link(target: '_blank') + href = Storages::Peripherals::StorageInteraction::Nextcloud::Util + .join_uri_path(storage.host, 'settings/admin/openproject') + render(Primer::Beta::Link.new(href:, target:)) { I18n.t('storages.instructions.nextcloud.integration') } + end def one_drive_first_time_configuration? storage.provider_type_one_drive? && first_time_configuration? diff --git a/modules/storages/config/locales/en.yml b/modules/storages/config/locales/en.yml index cd1eb3e8ed7d..c621e73e665f 100644 --- a/modules/storages/config/locales/en.yml +++ b/modules/storages/config/locales/en.yml @@ -137,10 +137,12 @@ en: copy_from: "Copy this value from" nextcloud: provider_configuration: "Please make sure you have administration privileges in your Nextcloud instance and the %{application_link_text} is installed before doing the setup." + oauth_configuration: "Copy these values from %{application_link_text}." application_link_text: "application “Integration OpenProject”" integration: "Nextcloud Administration / OpenProject" one_drive: provider_configuration: "Please make sure you have administration privileges in the %{application_link_text} before doing the setup." + oauth_configuration: "Copy these values from the %{application_link_text}. After that, copy the redirect URI back to the %{application_link_text}." application_link_text: "Azure application" integration: "OneDrive/SharePoint" oauth_client_id: > diff --git a/modules/storages/spec/features/admin_storages_spec.rb b/modules/storages/spec/features/admin_storages_spec.rb index 679e6f8cba2a..47d4c5a96464 100644 --- a/modules/storages/spec/features/admin_storages_spec.rb +++ b/modules/storages/spec/features/admin_storages_spec.rb @@ -161,6 +161,9 @@ aggregate_failures 'OAuth Client' do within_test_selector('storage-oauth-client-form') do + expect(page).to have_test_selector('storage-provider-credentials-instructions', + text: 'Copy these values from Nextcloud Administration / OpenProject.') + # With null values, submit button should be disabled expect(page).to have_css('#oauth_client_client_id', value: '') expect(page).to have_css('#oauth_client_client_secret', value: '') @@ -250,13 +253,17 @@ click_button 'Save and continue' end - expect(page).to have_test_selector('label-host_name_configured-storage_tenant_drive_configured-status', - text: 'Completed') + wait_for(page).to have_test_selector('label-host_name_configured-storage_tenant_drive_configured-status', + text: 'Completed') expect(page).to have_test_selector('storage-description', text: 'OneDrive/SharePoint - My OneDrive') end aggregate_failures 'OAuth Client' do within_test_selector('storage-oauth-client-form') do + expect(page).to have_test_selector('storage-provider-credentials-instructions', + text: 'Copy these values from the Azure application. ' \ + 'After that, copy the redirect URI back to the Azure application.') + # With null values, submit button should be disabled expect(page).to have_css('#oauth_client_client_id', value: '') expect(page).to have_css('#oauth_client_client_secret', value: '') From 31035fe9921a50d26153d59574a6dda0645585f4 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 15 Nov 2023 17:10:41 +0300 Subject: [PATCH 49/56] fix[Op#50737]: Borderbox headers should have the fg-muted color --- .../storages/admin/storage_list_component.html.erb | 2 +- .../storages/admin/storage_view_component.html.erb | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) 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 53d43573d2d0..6af245efce51 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 @@ -1,7 +1,7 @@ <%= if storages.present? render(Primer::Beta::BorderBox.new) do |component| - component.with_header(color: :subtle) do + component.with_header(color: :muted) do grid_layout('op-storage-list--header', tag: :div, align_items: :center) do |grid| grid.with_area(:name, tag: :div, mr: 3) do render(Primer::Beta::Text.new(font_weight: :semibold)) { I18n.t('storages.label_name') } diff --git a/modules/storages/app/components/storages/admin/storage_view_component.html.erb b/modules/storages/app/components/storages/admin/storage_view_component.html.erb index 6b3b34319da0..f9b8d59c2a73 100644 --- a/modules/storages/app/components/storages/admin/storage_view_component.html.erb +++ b/modules/storages/app/components/storages/admin/storage_view_component.html.erb @@ -1,6 +1,6 @@ <%= render(Primer::Beta::BorderBox.new) do |component| - component.with_header(color: :default) do + component.with_header(color: :muted) do render(Primer::Beta::Text.new(font_weight: :semibold)) { I18n.t('storages.file_storage_view.general_information') } end @@ -15,7 +15,7 @@ end if storage.provider_type_nextcloud? - component.with_row(scheme: :neutral) do + component.with_row(scheme: :neutral, color: :muted) do grid_layout('op-storage-view--row', tag: :div, align_items: :center) do |grid| grid.with_area(:item, tag: :div, mr: 3) do render(Primer::Beta::Text.new(font_weight: :semibold, mr: 1)) { I18n.t('storages.file_storage_view.oauth_applications') } @@ -46,7 +46,7 @@ end end - component.with_row(scheme: :neutral) do + component.with_row(scheme: :neutral, color: :muted) do grid_layout('op-storage-view--row', tag: :div, align_items: :center) do |grid| grid.with_area(:item, tag: :div) do render(Primer::Beta::Text.new(font_weight: :semibold, mr: 1)) { I18n.t('storages.file_storage_view.project_folders') } @@ -66,7 +66,7 @@ end if storage.provider_type_one_drive? - component.with_row(scheme: :neutral) do + component.with_row(scheme: :neutral, color: :muted) do grid_layout('op-storage-view--row', tag: :div, align_items: :center) do |grid| grid.with_area(:item, tag: :div, mr: 3) do render(Primer::Beta::Text.new(font_weight: :semibold, mr: 1)) { I18n.t('storages.file_storage_view.oauth_applications') } From 7a57ce0ddbf55e4961af2934372b970920d5aa6d Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 15 Nov 2023 17:36:10 +0300 Subject: [PATCH 50/56] fix[Op#50737]: icon buttons should have the fg-muted color --- ...utomatically_managed_project_folders_info_component.html.erb | 2 +- .../components/storages/admin/general_info_component.html.erb | 2 +- .../storages/admin/oauth_application_info_component.html.erb | 2 +- .../storages/admin/oauth_client_info_component.html.erb | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/modules/storages/app/components/storages/admin/automatically_managed_project_folders_info_component.html.erb b/modules/storages/app/components/storages/admin/automatically_managed_project_folders_info_component.html.erb index a417d0fda011..62e5be15d346 100644 --- a/modules/storages/app/components/storages/admin/automatically_managed_project_folders_info_component.html.erb +++ b/modules/storages/app/components/storages/admin/automatically_managed_project_folders_info_component.html.erb @@ -10,7 +10,7 @@ end if editable_storage? - grid.with_area(:"icon-button", tag: :div, color: :subtle) do + grid.with_area(:"icon-button", tag: :div, color: :muted) do flex_layout(justify_content: :flex_end) do |icons_container| icons_container.with_column do render( diff --git a/modules/storages/app/components/storages/admin/general_info_component.html.erb b/modules/storages/app/components/storages/admin/general_info_component.html.erb index 5fbcd3219dc6..21dca880259b 100644 --- a/modules/storages/app/components/storages/admin/general_info_component.html.erb +++ b/modules/storages/app/components/storages/admin/general_info_component.html.erb @@ -10,7 +10,7 @@ end if editable_storage? - grid.with_area(:"icon-button", tag: :div, color: :subtle) do + grid.with_area(:"icon-button", tag: :div, color: :muted) do flex_layout(justify_content: :flex_end) do |icons_container| icons_container.with_column do render( diff --git a/modules/storages/app/components/storages/admin/oauth_application_info_component.html.erb b/modules/storages/app/components/storages/admin/oauth_application_info_component.html.erb index 3112f3eae9cc..b3028fef29e0 100644 --- a/modules/storages/app/components/storages/admin/oauth_application_info_component.html.erb +++ b/modules/storages/app/components/storages/admin/oauth_application_info_component.html.erb @@ -10,7 +10,7 @@ end if editable_storage? - grid.with_area(:"icon-button", tag: :div, color: :subtle, test_selector: 'storage-replace-openproject-oauth-application') do + grid.with_area(:"icon-button", tag: :div, color: :muted, test_selector: 'storage-replace-openproject-oauth-application') do flex_layout(justify_content: :flex_end) do |icons_container| icons_container.with_column do primer_form_with( diff --git a/modules/storages/app/components/storages/admin/oauth_client_info_component.html.erb b/modules/storages/app/components/storages/admin/oauth_client_info_component.html.erb index 90251d0be5b8..e855d6da3ecd 100644 --- a/modules/storages/app/components/storages/admin/oauth_client_info_component.html.erb +++ b/modules/storages/app/components/storages/admin/oauth_client_info_component.html.erb @@ -14,7 +14,7 @@ end if editable_storage? - grid.with_area(:"icon-button", tag: :div, color: :subtle) do + grid.with_area(:"icon-button", tag: :div, color: :muted) do flex_layout(justify_content: :flex_end) do |icons_container| icons_container.with_column do render( From 9b84ff73e4950e31cc6d4a85c87c729fae6bebb4 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 15 Nov 2023 17:47:34 +0300 Subject: [PATCH 51/56] fix[Op#50737]: Match font weights with design spec --- ...omatically_managed_project_folders_form_component.html.erb | 4 ++-- .../storages/admin/forms/oauth_client_form_component.html.erb | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.html.erb b/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.html.erb index 82d30866618a..ceed94efda44 100644 --- a/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.html.erb +++ b/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.html.erb @@ -12,11 +12,11 @@ ) do |form| flex_layout do |project_folders_form| project_folders_form.with_row(mb: 3) do - render(Primer::Beta::Text.new(font_weight: :semibold)) { I18n.t(:'storages.label_managed_project_folders.automatically_managed_folders') } + render(Primer::Beta::Text.new(font_weight: :bold)) { I18n.t(:'storages.label_managed_project_folders.automatically_managed_folders') } end project_folders_form.with_row(mb: 3) do - render(Primer::Beta::Text.new(font_weight: :light)) { I18n.t("storages.page_titles.managed_project_folders.subtitle") } + render(Primer::Beta::Text.new) { I18n.t("storages.page_titles.managed_project_folders.subtitle") } end project_folders_form.with_row(mb: 3) do diff --git a/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb b/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb index 0f23cfbd8dd8..3423ebf17e96 100644 --- a/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb +++ b/modules/storages/app/components/storages/admin/forms/oauth_client_form_component.html.erb @@ -12,11 +12,11 @@ ) do |form| flex_layout do |oauth_client_row| oauth_client_row.with_row(mb: 3) do - render(Primer::Beta::Text.new(font_weight: :semibold)) { I18n.t("storages.file_storage_view.#{storage.short_provider_type}_oauth") } + render(Primer::Beta::Text.new(font_weight: :bold)) { I18n.t("storages.file_storage_view.#{storage.short_provider_type}_oauth") } end oauth_client_row.with_row(mb: 3) do - render(Primer::Beta::Text.new(font_weight: :light, test_selector: 'storage-provider-credentials-instructions')) { storage_provider_credentials_instructions } + render(Primer::Beta::Text.new(test_selector: 'storage-provider-credentials-instructions')) { storage_provider_credentials_instructions } end oauth_client_row.with_row(mb: 3) do From 13c9df8f581996dfedaed2a0340589f7fa9ac11c Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 15 Nov 2023 17:56:03 +0300 Subject: [PATCH 52/56] fix[Op#50737]: Remove Cancel button from OAuthApplicationInfoCopy component --- .../oauth_application_info_copy_component.html.erb | 1 - .../admin/oauth_application_info_copy_component.rb | 14 -------------- .../storages/admin/storage_view_component.html.erb | 3 +-- .../admin/storages/create.turbo_stream.erb | 4 +--- .../show_oauth_application.turbo_stream.erb | 4 +--- 5 files changed, 3 insertions(+), 23 deletions(-) diff --git a/modules/storages/app/components/storages/admin/oauth_application_info_copy_component.html.erb b/modules/storages/app/components/storages/admin/oauth_application_info_copy_component.html.erb index 69adf695cb78..6cf35ab5e472 100644 --- a/modules/storages/app/components/storages/admin/oauth_application_info_copy_component.html.erb +++ b/modules/storages/app/components/storages/admin/oauth_application_info_copy_component.html.erb @@ -33,7 +33,6 @@ credentials_row.with_row do concat(render(Primer::Beta::Button.new(**submit_button_options)) { I18n.t('storages.buttons.done_continue') }) - concat(render(Primer::Beta::Button.new(**cancel_button_options)) { I18n.t('button_cancel') }) end end end diff --git a/modules/storages/app/components/storages/admin/oauth_application_info_copy_component.rb b/modules/storages/app/components/storages/admin/oauth_application_info_copy_component.rb index 7833cab7b193..2bf6f2432b48 100644 --- a/modules/storages/app/components/storages/admin/oauth_application_info_copy_component.rb +++ b/modules/storages/app/components/storages/admin/oauth_application_info_copy_component.rb @@ -35,8 +35,6 @@ class OAuthApplicationInfoCopyComponent < ApplicationComponent attr_reader :storage alias_method :oauth_application, :model - options cancel_button_should_break_from_frame: false - def initialize(oauth_application:, storage:, **options) super(oauth_application, **options) @storage = storage @@ -60,22 +58,10 @@ def submit_button_options }.merge(options.fetch(:submit_button_options, {})) end - def cancel_button_options - { scheme: :default, - tag: :a, - href: cancel_button_path }.tap do |options_hash| - options_hash[:target] = '_top' if cancel_button_should_break_from_frame - end - end - private def submit_button_path options[:submit_button_path] || edit_admin_settings_storage_path(storage) end - - def cancel_button_path - options[:cancel_button_path] || edit_admin_settings_storage_path(storage) - end end end diff --git a/modules/storages/app/components/storages/admin/storage_view_component.html.erb b/modules/storages/app/components/storages/admin/storage_view_component.html.erb index f9b8d59c2a73..2219a21e0747 100644 --- a/modules/storages/app/components/storages/admin/storage_view_component.html.erb +++ b/modules/storages/app/components/storages/admin/storage_view_component.html.erb @@ -32,8 +32,7 @@ Storages::Admin::OAuthApplicationInfoCopyComponent.new( oauth_application:, storage:, - submit_button_path: show_oauth_application_admin_settings_storage_path(storage), - cancel_button_path: admin_settings_storages_path + submit_button_path: show_oauth_application_admin_settings_storage_path(storage) ) ) end diff --git a/modules/storages/app/views/storages/admin/storages/create.turbo_stream.erb b/modules/storages/app/views/storages/admin/storages/create.turbo_stream.erb index f15990f2c5d6..155a9b7fd5c2 100644 --- a/modules/storages/app/views/storages/admin/storages/create.turbo_stream.erb +++ b/modules/storages/app/views/storages/admin/storages/create.turbo_stream.erb @@ -42,9 +42,7 @@ See COPYRIGHT and LICENSE files for more details. submit_button_options: { data: { turbo_stream: true } }, - submit_button_path: new_admin_settings_storage_oauth_client_path(@storage), - cancel_button_path: admin_settings_storages_path, - cancel_button_should_break_from_frame: true + submit_button_path: new_admin_settings_storage_oauth_client_path(@storage) ) ) %> diff --git a/modules/storages/app/views/storages/admin/storages/show_oauth_application.turbo_stream.erb b/modules/storages/app/views/storages/admin/storages/show_oauth_application.turbo_stream.erb index 9bc710790bb6..95531304160f 100644 --- a/modules/storages/app/views/storages/admin/storages/show_oauth_application.turbo_stream.erb +++ b/modules/storages/app/views/storages/admin/storages/show_oauth_application.turbo_stream.erb @@ -40,9 +40,7 @@ See COPYRIGHT and LICENSE files for more details. submit_button_options: { data: { turbo_stream: true } }, - submit_button_path: new_admin_settings_storage_oauth_client_path(@storage), - cancel_button_path: admin_settings_storages_path, - cancel_button_should_break_from_frame: true + submit_button_path: new_admin_settings_storage_oauth_client_path(@storage) ) ) %> From 8051a18d3c1324a50d42d5fa71e045a3c7150eed Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 15 Nov 2023 18:14:26 +0300 Subject: [PATCH 53/56] fix[Op#50737]: Mark provider type select as readonly after storage selection --- .../storages/admin/forms/general_info_form_component.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 dce5a61399da..eb635a067dc9 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 @@ -11,7 +11,7 @@ Storages::Admin::ProviderTypeSelectForm.new( form, storage:, - select_list_options: { caption: provider_type_select_caption } + select_list_options: { caption: provider_type_select_caption, readonly: storage.new_record? } ) ) end From fb10c2b83f6c54708edbf6d8ff64dbb3113d5822 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 15 Nov 2023 18:15:08 +0300 Subject: [PATCH 54/56] tests[Op#50737]: wait for flash message after storage creation --- modules/storages/spec/features/admin_storages_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/storages/spec/features/admin_storages_spec.rb b/modules/storages/spec/features/admin_storages_spec.rb index 47d4c5a96464..68035f8403ad 100644 --- a/modules/storages/spec/features/admin_storages_spec.rb +++ b/modules/storages/spec/features/admin_storages_spec.rb @@ -277,8 +277,8 @@ end expect(page).to have_current_path(admin_settings_storages_path) - expect(page).to have_text("Storage connected successfully! Remember to activate the module and the specific " \ - "storage in the project settings of each desired project to use it.") + wait_for(page).to have_text("Storage connected successfully! Remember to activate the module and the specific " \ + "storage in the project settings of each desired project to use it.") end end end From ad79c9845f979f24f46decac16ed92af1fc2c599 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 16 Nov 2023 16:37:56 +0300 Subject: [PATCH 55/56] fix[Op#50737]: More font fixes to match design --- ...omatically_managed_project_folders_info_component.html.erb | 4 ++-- .../components/storages/admin/general_info_component.html.erb | 4 ++-- .../storages/admin/oauth_application_info_component.html.erb | 4 ++-- .../storages/admin/oauth_client_info_component.html.erb | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/modules/storages/app/components/storages/admin/automatically_managed_project_folders_info_component.html.erb b/modules/storages/app/components/storages/admin/automatically_managed_project_folders_info_component.html.erb index 62e5be15d346..a6e32e1a44b8 100644 --- a/modules/storages/app/components/storages/admin/automatically_managed_project_folders_info_component.html.erb +++ b/modules/storages/app/components/storages/admin/automatically_managed_project_folders_info_component.html.erb @@ -1,12 +1,12 @@ <%= grid_layout('op-storage-view--row', tag: :div, align_items: :center) do |grid| grid.with_area(:item, tag: :div, mr: 3) do - concat(render(Primer::Beta::Text.new(font_weight: :semibold, mr: 1, test_selector: 'storage-managed-project-folders-label')) { I18n.t('storages.file_storage_view.automatically_managed_folders') }) + concat(render(Primer::Beta::Text.new(font_weight: :bold, mr: 1, test_selector: 'storage-managed-project-folders-label')) { I18n.t('storages.file_storage_view.automatically_managed_folders') }) concat(automatically_managed_project_folders_status_label) end grid.with_area(:description, tag: :div, color: :subtle, test_selector: 'storage-automatically-managed-project-folders-description') do - render(Primer::Beta::Truncate.new(font_weight: :light)) { I18n.t('storages.page_titles.managed_project_folders.subtitle_short') } + render(Primer::Beta::Truncate.new) { I18n.t('storages.page_titles.managed_project_folders.subtitle_short') } end if editable_storage? diff --git a/modules/storages/app/components/storages/admin/general_info_component.html.erb b/modules/storages/app/components/storages/admin/general_info_component.html.erb index 21dca880259b..59036d5d399b 100644 --- a/modules/storages/app/components/storages/admin/general_info_component.html.erb +++ b/modules/storages/app/components/storages/admin/general_info_component.html.erb @@ -1,12 +1,12 @@ <%= grid_layout('op-storage-view--row', tag: :div, align_items: :center) do |grid| grid.with_area(:item, tag: :div, mr: 3) do - concat(render(Primer::Beta::Text.new(font_weight: :semibold, mr: 1, test_selector: 'storage-provider-label')) { I18n.t('storages.file_storage_view.storage_provider') }) + concat(render(Primer::Beta::Text.new(font_weight: :bold, mr: 1, test_selector: 'storage-provider-label')) { I18n.t('storages.file_storage_view.storage_provider') }) concat(configuration_check_label) end grid.with_area(:description, tag: :div, color: :subtle, test_selector: 'storage-description') do - render(Primer::Beta::Truncate.new(font_weight: :light)) { storage_description } + render(Primer::Beta::Truncate.new) { storage_description } end if editable_storage? diff --git a/modules/storages/app/components/storages/admin/oauth_application_info_component.html.erb b/modules/storages/app/components/storages/admin/oauth_application_info_component.html.erb index b3028fef29e0..28ff70c482ca 100644 --- a/modules/storages/app/components/storages/admin/oauth_application_info_component.html.erb +++ b/modules/storages/app/components/storages/admin/oauth_application_info_component.html.erb @@ -1,12 +1,12 @@ <%= grid_layout('op-storage-view--row', tag: :div, align_items: :center) do |grid| grid.with_area(:item, tag: :div, mr: 3) do - concat(render(Primer::Beta::Text.new(font_weight: :semibold, mr: 1, test_selector: 'storage-openproject-oauth-label')) { I18n.t('storages.file_storage_view.openproject_oauth') }) + concat(render(Primer::Beta::Text.new(font_weight: :bold, mr: 1, test_selector: 'storage-openproject-oauth-label')) { I18n.t('storages.file_storage_view.openproject_oauth') }) concat(configuration_check_label_for(:openproject_oauth_application_configured)) end grid.with_area(:description, tag: :div, color: :subtle, test_selector: 'storage-openproject-oauth-application-description') do - render(Primer::Beta::Truncate.new(font_weight: :light)) { openproject_oauth_client_description } + render(Primer::Beta::Truncate.new) { openproject_oauth_client_description } end if editable_storage? diff --git a/modules/storages/app/components/storages/admin/oauth_client_info_component.html.erb b/modules/storages/app/components/storages/admin/oauth_client_info_component.html.erb index e855d6da3ecd..7f1163bf0cb0 100644 --- a/modules/storages/app/components/storages/admin/oauth_client_info_component.html.erb +++ b/modules/storages/app/components/storages/admin/oauth_client_info_component.html.erb @@ -3,14 +3,14 @@ grid.with_area(:item, tag: :div, mr: 3) do concat( render( - Primer::Beta::Text.new(font_weight: :semibold, mr: 1, test_selector: 'storage-oauth-client-label') + Primer::Beta::Text.new(font_weight: :bold, mr: 1, test_selector: 'storage-oauth-client-label') ) { I18n.t("storages.file_storage_view.#{storage.short_provider_type}_oauth") } ) concat(configuration_check_label_for(:storage_oauth_client_configured)) end grid.with_area(:description, tag: :div, color: :subtle, test_selector: 'storage-oauth-client-id-description') do - render(Primer::Beta::Truncate.new(font_weight: :light)) { provider_oauth_client_description } + render(Primer::Beta::Truncate.new) { provider_oauth_client_description } end if editable_storage? From 317a3fe0005f1a9a912db54bb741f5d7f79e23f3 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 16 Nov 2023 17:33:55 +0300 Subject: [PATCH 56/56] fix[Op#50737]: Make "Cancel" button routing more consistent 1. In "General information" section, cancel redirects back to the index page 2. All other "Cancel" buttons stay on the same page, but hide the forms --- ...ally_managed_project_folders_form_component.rb | 7 +------ .../admin/forms/general_info_form_component.rb | 15 ++++++++------- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.rb b/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.rb index 153d95abbbb2..7ad495021f52 100644 --- a/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.rb +++ b/modules/storages/app/components/storages/admin/forms/automatically_managed_project_folders_form_component.rb @@ -49,12 +49,7 @@ def submit_button_options end def cancel_button_options - { href: edit_admin_settings_storage_path(storage) }.tap do |options_hash| - if new_record? - options_hash[:href] = admin_settings_storages_path - options_hash[:target] = '_top' - end - end + { href: edit_admin_settings_storage_path(storage) } end private 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 c6778138ef87..a979d8779a18 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 @@ -45,10 +45,13 @@ def submit_button_options end def cancel_button_options - { - href: cancel_button_path, - data: { turbo_stream: true } - } + { href: cancel_button_path, + data: { turbo_stream: true } }.tap do |options_hash| + if storage.new_record? + options_hash[:data][:turbo_stream] = false + options_hash[:target] = '_top' # Break out of Turbo Frame, follow full page redirect + end + end end private @@ -64,9 +67,7 @@ def default_form_url def cancel_button_path options.fetch(:cancel_button_path) do - if storage.new_record? - new_admin_settings_storage_path - elsif storage.persisted? + if storage.persisted? edit_admin_settings_storage_path(storage) else admin_settings_storages_path