From 9c08378e169d102d84ab2ea7b825059653bfd6ea Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 18 Jul 2024 10:27:48 +0300 Subject: [PATCH 01/22] [#55967] Implement file picker for the existing folders with manually managed permissions in the dialog https://community.openproject.org/work_packages/55967 --- frontend/src/app/app.module.ts | 4 +++ .../open_project/forms/dsl/input_methods.rb | 4 +++ .../forms/dsl/storage_login_button_input.rb | 31 +++++++++++++++++++ .../forms/storage_login_button.html.erb | 6 ++++ .../forms/storage_login_button.rb | 18 +++++++++++ .../project_folder_mode_form.rb | 20 ++++++++++++ 6 files changed, 83 insertions(+) create mode 100644 lib/primer/open_project/forms/dsl/storage_login_button_input.rb create mode 100644 lib/primer/open_project/forms/storage_login_button.html.erb create mode 100644 lib/primer/open_project/forms/storage_login_button.rb diff --git a/frontend/src/app/app.module.ts b/frontend/src/app/app.module.ts index 8d42dc403598..19d0f3e42ccf 100644 --- a/frontend/src/app/app.module.ts +++ b/frontend/src/app/app.module.ts @@ -173,6 +173,9 @@ import { } from 'core-app/features/work-packages/routing/wp-split-view/wp-split-view-entry.component'; import { InAppNotificationsDateAlertsUpsaleComponent } from 'core-app/features/in-app-notifications/date-alerts-upsale/ian-date-alerts-upsale.component'; import { ShareUpsaleComponent } from 'core-app/features/enterprise/share-upsale/share-upsale.component'; +import { + StorageLoginButtonComponent, +} from 'core-app/shared/components/storages/storage-login-button/storage-login-button.component'; export function initializeServices(injector:Injector) { return () => { @@ -362,6 +365,7 @@ export class OpenProjectModule { registerCustomElement('opce-attribute-help-text', AttributeHelpTextComponent, { injector }); registerCustomElement('opce-exclusion-info', OpExclusionInfoComponent, { injector }); registerCustomElement('opce-attachments', OpAttachmentsComponent, { injector }); + registerCustomElement('opce-storage-login-button', StorageLoginButtonComponent, { injector }); // TODO: These elements are now registered custom elements, but are actually single-use components. They should be removed when we move these pages to Rails. registerCustomElement('opce-new-project', NewProjectComponent, { injector }); diff --git a/lib/primer/open_project/forms/dsl/input_methods.rb b/lib/primer/open_project/forms/dsl/input_methods.rb index 5a4aa6148507..5af03a5710be 100644 --- a/lib/primer/open_project/forms/dsl/input_methods.rb +++ b/lib/primer/open_project/forms/dsl/input_methods.rb @@ -20,6 +20,10 @@ def project_autocompleter(**, &) def rich_text_area(**) add_input RichTextAreaInput.new(builder: @builder, form: @form, **) end + + def storage_login_button(**) + add_input StorageLoginButtonInput.new(builder: @builder, form: @form, **) + end end end end diff --git a/lib/primer/open_project/forms/dsl/storage_login_button_input.rb b/lib/primer/open_project/forms/dsl/storage_login_button_input.rb new file mode 100644 index 000000000000..9290e124d001 --- /dev/null +++ b/lib/primer/open_project/forms/dsl/storage_login_button_input.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module Primer + module OpenProject + module Forms + module Dsl + class StorageLoginButtonInput < Primer::Forms::Dsl::Input + attr_reader :name, :label + + def initialize(storage_login_button_options:, **system_arguments) + @storage_login_button_options = storage_login_button_options + + super(**system_arguments) + end + + def to_component + StorageLoginButton.new(input: self, storage_login_button_options: @storage_login_button_options) + end + + def type + :storage_login_button + end + + def focusable? + true + end + end + end + end + end +end diff --git a/lib/primer/open_project/forms/storage_login_button.html.erb b/lib/primer/open_project/forms/storage_login_button.html.erb new file mode 100644 index 000000000000..dfe5f3ab29cb --- /dev/null +++ b/lib/primer/open_project/forms/storage_login_button.html.erb @@ -0,0 +1,6 @@ +<%= render(FormControl.new(input: @input)) do %> + <%= angular_component_tag 'opce-storage-login-button', + data: @storage_login_button_options.delete(:data) { {} }, + inputs: @storage_login_button_options.delete(:inputs) { {} } + %> +<% end %> diff --git a/lib/primer/open_project/forms/storage_login_button.rb b/lib/primer/open_project/forms/storage_login_button.rb new file mode 100644 index 000000000000..8cbd166196cc --- /dev/null +++ b/lib/primer/open_project/forms/storage_login_button.rb @@ -0,0 +1,18 @@ +module Primer + module OpenProject + module Forms + # :nodoc: + class StorageLoginButton < Primer::Forms::BaseComponent + include AngularHelper + + delegate :builder, :form, to: :@input + + def initialize(input:, storage_login_button_options:) + super() + @input = input + @storage_login_button_options = storage_login_button_options + end + end + end + end +end diff --git a/modules/storages/app/forms/storages/admin/project_storages/project_folder_mode_form.rb b/modules/storages/app/forms/storages/admin/project_storages/project_folder_mode_form.rb index c0639dc60c58..244b8c7e62e1 100644 --- a/modules/storages/app/forms/storages/admin/project_storages/project_folder_mode_form.rb +++ b/modules/storages/app/forms/storages/admin/project_storages/project_folder_mode_form.rb @@ -30,6 +30,8 @@ module Storages module Admin module ProjectStorages class ProjectFolderModeForm < ApplicationForm + include StorageLoginHelper + form do |radio_form| radio_form.radio_button_group(name: :project_folder_mode) do |radio_group| if @project_storage.project_folder_mode_possible?("inactive") @@ -41,6 +43,24 @@ class ProjectFolderModeForm < ApplicationForm radio_group.radio_button(value: "automatic", label: I18n.t(:"storages.label_automatic_folder"), caption: I18n.t(:"storages.instructions.automatic_folder")) end + + if @project_storage.project_folder_mode_possible?("manual") + radio_group.radio_button(value: "manual", label: I18n.t(:"storages.label_manual_folder"), + caption: I18n.t(:"storages.instructions.manual_folder")) + end + end + + if @project_storage.project_folder_mode_possible?("manual") + radio_form.storage_login_button( + storage_login_button_options: { + data: { + "project-storage-form-target": "loginButton" + }, + inputs: { + input: storage_login_input(@project_storage.storage) + } + } + ) end end From 6399345e0b9152a5935e8d570159ed3d1b695061 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 18 Jul 2024 13:21:29 +0300 Subject: [PATCH 02/22] chore[Op#55967]: use the correct translations --- .../open_project/forms/dsl/input_methods.rb | 4 +- .../forms/dsl/storage_login_button_input.rb | 31 ------------- ...e_manual_project_folder_selection_input.rb | 43 +++++++++++++++++++ .../forms/storage_login_button.html.erb | 6 --- .../forms/storage_login_button.rb | 18 -------- ...e_manual_project_folder_selection.html.erb | 28 ++++++++++++ ...storage_manual_project_folder_selection.rb | 41 ++++++++++++++++++ ...add_projects_form_modal_component.html.erb | 8 ++++ .../add_projects_form_modal_component.rb | 2 + .../project_folder_mode_form.rb | 13 ++++-- 10 files changed, 133 insertions(+), 61 deletions(-) delete mode 100644 lib/primer/open_project/forms/dsl/storage_login_button_input.rb create mode 100644 lib/primer/open_project/forms/dsl/storage_manual_project_folder_selection_input.rb delete mode 100644 lib/primer/open_project/forms/storage_login_button.html.erb delete mode 100644 lib/primer/open_project/forms/storage_login_button.rb create mode 100644 lib/primer/open_project/forms/storage_manual_project_folder_selection.html.erb create mode 100644 lib/primer/open_project/forms/storage_manual_project_folder_selection.rb diff --git a/lib/primer/open_project/forms/dsl/input_methods.rb b/lib/primer/open_project/forms/dsl/input_methods.rb index 5af03a5710be..8b9a74cf3d9a 100644 --- a/lib/primer/open_project/forms/dsl/input_methods.rb +++ b/lib/primer/open_project/forms/dsl/input_methods.rb @@ -21,8 +21,8 @@ def rich_text_area(**) add_input RichTextAreaInput.new(builder: @builder, form: @form, **) end - def storage_login_button(**) - add_input StorageLoginButtonInput.new(builder: @builder, form: @form, **) + def storage_manual_project_folder_selection(**) + add_input StorageManualProjectFolderSelectionInput.new(builder: @builder, form: @form, **) end end end diff --git a/lib/primer/open_project/forms/dsl/storage_login_button_input.rb b/lib/primer/open_project/forms/dsl/storage_login_button_input.rb deleted file mode 100644 index 9290e124d001..000000000000 --- a/lib/primer/open_project/forms/dsl/storage_login_button_input.rb +++ /dev/null @@ -1,31 +0,0 @@ -# frozen_string_literal: true - -module Primer - module OpenProject - module Forms - module Dsl - class StorageLoginButtonInput < Primer::Forms::Dsl::Input - attr_reader :name, :label - - def initialize(storage_login_button_options:, **system_arguments) - @storage_login_button_options = storage_login_button_options - - super(**system_arguments) - end - - def to_component - StorageLoginButton.new(input: self, storage_login_button_options: @storage_login_button_options) - end - - def type - :storage_login_button - end - - def focusable? - true - end - end - end - end - end -end diff --git a/lib/primer/open_project/forms/dsl/storage_manual_project_folder_selection_input.rb b/lib/primer/open_project/forms/dsl/storage_manual_project_folder_selection_input.rb new file mode 100644 index 000000000000..60316cadc597 --- /dev/null +++ b/lib/primer/open_project/forms/dsl/storage_manual_project_folder_selection_input.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +module Primer + module OpenProject + module Forms + module Dsl + class StorageManualProjectFolderSelectionInput < Primer::Forms::Dsl::Input + attr_reader :name, :label + + def initialize(name:, label:, project_storage:, storage_login_button_options:, last_project_folders: {}, + wrapper_data_attributes: {}, **system_arguments) + @name = name + @label = label + @project_storage = project_storage + @last_project_folders = last_project_folders + @wrapper_data_attributes = wrapper_data_attributes + @storage_login_button_options = storage_login_button_options + + super(**system_arguments) + end + + def to_component + StorageManualProjectFolderSelection.new( + input: self, + project_storage: @project_storage, + last_project_folders: @last_project_folders, + storage_login_button_options: @storage_login_button_options, + wrapper_data_attributes: @wrapper_data_attributes + ) + end + + def type + :storage_login_button + end + + def focusable? + true + end + end + end + end + end +end diff --git a/lib/primer/open_project/forms/storage_login_button.html.erb b/lib/primer/open_project/forms/storage_login_button.html.erb deleted file mode 100644 index dfe5f3ab29cb..000000000000 --- a/lib/primer/open_project/forms/storage_login_button.html.erb +++ /dev/null @@ -1,6 +0,0 @@ -<%= render(FormControl.new(input: @input)) do %> - <%= angular_component_tag 'opce-storage-login-button', - data: @storage_login_button_options.delete(:data) { {} }, - inputs: @storage_login_button_options.delete(:inputs) { {} } - %> -<% end %> diff --git a/lib/primer/open_project/forms/storage_login_button.rb b/lib/primer/open_project/forms/storage_login_button.rb deleted file mode 100644 index 8cbd166196cc..000000000000 --- a/lib/primer/open_project/forms/storage_login_button.rb +++ /dev/null @@ -1,18 +0,0 @@ -module Primer - module OpenProject - module Forms - # :nodoc: - class StorageLoginButton < Primer::Forms::BaseComponent - include AngularHelper - - delegate :builder, :form, to: :@input - - def initialize(input:, storage_login_button_options:) - super() - @input = input - @storage_login_button_options = storage_login_button_options - end - end - end - end -end diff --git a/lib/primer/open_project/forms/storage_manual_project_folder_selection.html.erb b/lib/primer/open_project/forms/storage_manual_project_folder_selection.html.erb new file mode 100644 index 000000000000..7165d4472d48 --- /dev/null +++ b/lib/primer/open_project/forms/storage_manual_project_folder_selection.html.erb @@ -0,0 +1,28 @@ +<%= render(Primer::BaseComponent.new(tag: :div, display: :none, data: @wrapper_data_attributes)) do %> + + <%= render(FormControl.new(input: @input)) do %> + <% if storage_oauth_access_granted? %> + <%= + render(Primer::Beta::Button.new( + scheme: :default, + data: { + 'project-storage-form-target': "selectProjectFolderButton", + 'action': "project-storage-form#selectProjectFolder" + } + )) do |button| + button.with_leading_visual_icon(icon: 'file-directory') + I18n.t(:"storages.buttons.select_folder") + end + %> + <% else %> + <%= angular_component_tag 'opce-storage-login-button', + data: @storage_login_button_options.delete(:data) { {} }, + inputs: @storage_login_button_options.delete(:inputs) { {} } + %> + <% end %> + <% end %> + + <%= + render(Primer::BaseComponent.new(tag: :span, data: { "project-storage-form-target": "selectedFolderText"})) { "..." } + %> +<% end %> diff --git a/lib/primer/open_project/forms/storage_manual_project_folder_selection.rb b/lib/primer/open_project/forms/storage_manual_project_folder_selection.rb new file mode 100644 index 000000000000..ef6b50f678d2 --- /dev/null +++ b/lib/primer/open_project/forms/storage_manual_project_folder_selection.rb @@ -0,0 +1,41 @@ +module Primer + module OpenProject + module Forms + # :nodoc: + class StorageManualProjectFolderSelection < Primer::Forms::BaseComponent + include AngularHelper + + delegate :builder, :form, to: :@input + + def initialize(input:, project_storage:, storage_login_button_options:, last_project_folders: {}, + wrapper_data_attributes: {}) + super() + @input = input + @project_storage = project_storage + @last_project_folders = last_project_folders + @storage_login_button_options = storage_login_button_options + @wrapper_data_attributes = derive_wrapper_data_attributes(wrapper_data_attributes) + end + + private + + def derive_wrapper_data_attributes(options) + options.reverse_merge( + "application-target": "dynamic", + controller: "project-storage-form", + "project-storage-form-folder-mode-value": @project_storage.project_folder_mode, + "project-storage-form-placeholder-folder-name-value": I18n.t(:"storages.label_no_selected_folder"), + "project-storage-form-not-logged-in-validation-value": I18n.t(:"storages.instructions.not_logged_into_storage"), + "project-storage-form-last-project-folders-value": @last_project_folders, + "project-storage-form-target": "projectFolderSection" + ) + end + + def storage_oauth_access_granted? + OAuthClientToken + .exists?(user: User.current, oauth_client: @project_storage.storage.oauth_client) + end + end + end + end +end diff --git a/modules/storages/app/components/storages/admin/storages/add_projects_form_modal_component.html.erb b/modules/storages/app/components/storages/admin/storages/add_projects_form_modal_component.html.erb index 5bc9e3c84210..086d0a4f2945 100644 --- a/modules/storages/app/components/storages/admin/storages/add_projects_form_modal_component.html.erb +++ b/modules/storages/app/components/storages/admin/storages/add_projects_form_modal_component.html.erb @@ -27,6 +27,14 @@ See COPYRIGHT and LICENSE files for more details. ++#%> +<% helpers.content_controller 'project-storage-form', + dynamic: true, + 'project-storage-form-folder-mode-value': @project_storage.project_folder_mode, + 'project-storage-form-placeholder-folder-name-value': I18n.t(:"storages.label_no_selected_folder"), + 'project-storage-form-not-logged-in-validation-value': I18n.t(:"storages.instructions.not_logged_into_storage"), + 'project-storage-form-last-project-folders-value': @last_project_folders +%> + <%= component_wrapper do primer_form_with( diff --git a/modules/storages/app/components/storages/admin/storages/add_projects_form_modal_component.rb b/modules/storages/app/components/storages/admin/storages/add_projects_form_modal_component.rb index 5809fb5c4304..c8ab831a2c68 100644 --- a/modules/storages/app/components/storages/admin/storages/add_projects_form_modal_component.rb +++ b/modules/storages/app/components/storages/admin/storages/add_projects_form_modal_component.rb @@ -32,9 +32,11 @@ module Storages class AddProjectsFormModalComponent < ApplicationComponent include OpPrimer::ComponentHelpers include OpTurbo::Streamable + include StimulusHelper def initialize(project_storage:, **) @project_storage = project_storage + @last_project_folders = {} super(@project_storage, **) end diff --git a/modules/storages/app/forms/storages/admin/project_storages/project_folder_mode_form.rb b/modules/storages/app/forms/storages/admin/project_storages/project_folder_mode_form.rb index 244b8c7e62e1..cd1f117b290c 100644 --- a/modules/storages/app/forms/storages/admin/project_storages/project_folder_mode_form.rb +++ b/modules/storages/app/forms/storages/admin/project_storages/project_folder_mode_form.rb @@ -45,13 +45,17 @@ class ProjectFolderModeForm < ApplicationForm end if @project_storage.project_folder_mode_possible?("manual") - radio_group.radio_button(value: "manual", label: I18n.t(:"storages.label_manual_folder"), - caption: I18n.t(:"storages.instructions.manual_folder")) + radio_group.radio_button(value: "manual", label: I18n.t(:"storages.label_existing_manual_folder"), + caption: I18n.t(:"storages.instructions.existing_manual_folder")) end end if @project_storage.project_folder_mode_possible?("manual") - radio_form.storage_login_button( + radio_form.storage_manual_project_folder_selection( + name: :project_folder, + label: nil, + project_storage: @project_storage, + last_project_folders: @last_project_folders, storage_login_button_options: { data: { "project-storage-form-target": "loginButton" @@ -64,9 +68,10 @@ class ProjectFolderModeForm < ApplicationForm end end - def initialize(project_storage:) + def initialize(project_storage:, last_project_folders: {}) super() @project_storage = project_storage + @last_project_folders = last_project_folders end end end From 8ea0e002383ade00119070ae2f05b6e57d8fc4d2 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Fri, 19 Jul 2024 17:19:54 +0300 Subject: [PATCH 03/22] feat[Op#55967]: replace FormControl component with BaseComponent --- ...e_manual_project_folder_selection_input.rb | 11 +++++--- ...e_manual_project_folder_selection.html.erb | 25 +++++++++++-------- ...storage_manual_project_folder_selection.rb | 22 ++++++---------- ...add_projects_form_modal_component.html.erb | 20 ++++++++------- .../project_folder_mode_form.rb | 20 ++++++++++++--- 5 files changed, 57 insertions(+), 41 deletions(-) diff --git a/lib/primer/open_project/forms/dsl/storage_manual_project_folder_selection_input.rb b/lib/primer/open_project/forms/dsl/storage_manual_project_folder_selection_input.rb index 60316cadc597..ff667cb89c4c 100644 --- a/lib/primer/open_project/forms/dsl/storage_manual_project_folder_selection_input.rb +++ b/lib/primer/open_project/forms/dsl/storage_manual_project_folder_selection_input.rb @@ -7,14 +7,17 @@ module Dsl class StorageManualProjectFolderSelectionInput < Primer::Forms::Dsl::Input attr_reader :name, :label - def initialize(name:, label:, project_storage:, storage_login_button_options:, last_project_folders: {}, - wrapper_data_attributes: {}, **system_arguments) + def initialize(name:, label:, project_storage:, last_project_folders: {}, storage_login_button_options: {}, + select_folder_button_options: {}, **system_arguments) @name = name @label = label @project_storage = project_storage @last_project_folders = last_project_folders - @wrapper_data_attributes = wrapper_data_attributes @storage_login_button_options = storage_login_button_options + @select_folder_button_options = select_folder_button_options + + # # See: https://github.com/opf/primer_view_components/blob/main/lib/primer/forms/dsl/input.rb#L87C50-L87C60 + # system_arguments[:direction] = :row super(**system_arguments) end @@ -25,7 +28,7 @@ def to_component project_storage: @project_storage, last_project_folders: @last_project_folders, storage_login_button_options: @storage_login_button_options, - wrapper_data_attributes: @wrapper_data_attributes + select_folder_button_options: @select_folder_button_options ) end diff --git a/lib/primer/open_project/forms/storage_manual_project_folder_selection.html.erb b/lib/primer/open_project/forms/storage_manual_project_folder_selection.html.erb index 7165d4472d48..5e7bb8d0101b 100644 --- a/lib/primer/open_project/forms/storage_manual_project_folder_selection.html.erb +++ b/lib/primer/open_project/forms/storage_manual_project_folder_selection.html.erb @@ -1,19 +1,26 @@ -<%= render(Primer::BaseComponent.new(tag: :div, display: :none, data: @wrapper_data_attributes)) do %> - - <%= render(FormControl.new(input: @input)) do %> +<%= render(Primer::BaseComponent.new(tag: :div, data: @wrapper_data_attributes)) do %> + <%= render(Primer::BaseComponent.new(tag: :div, display: :inline_flex, direction: :row, align_items: :center)) do %> <% if storage_oauth_access_granted? %> <%= render(Primer::Beta::Button.new( scheme: :default, - data: { - 'project-storage-form-target': "selectProjectFolderButton", - 'action': "project-storage-form#selectProjectFolder" - } + display: :inline_block, + data: @select_folder_button_options.delete(:data) { {} } )) do |button| button.with_leading_visual_icon(icon: 'file-directory') I18n.t(:"storages.buttons.select_folder") end %> + + <%= + render( + Primer::Beta::Text.new( + font_weight: :bold, + data: @selected_folder_label_options.delete(:data) { {} }, + pl: 2 + ) + ) { I18n.t(:"storages.label_no_selected_folder") } + %> <% else %> <%= angular_component_tag 'opce-storage-login-button', data: @storage_login_button_options.delete(:data) { {} }, @@ -21,8 +28,4 @@ %> <% end %> <% end %> - - <%= - render(Primer::BaseComponent.new(tag: :span, data: { "project-storage-form-target": "selectedFolderText"})) { "..." } - %> <% end %> diff --git a/lib/primer/open_project/forms/storage_manual_project_folder_selection.rb b/lib/primer/open_project/forms/storage_manual_project_folder_selection.rb index ef6b50f678d2..3da0982a0869 100644 --- a/lib/primer/open_project/forms/storage_manual_project_folder_selection.rb +++ b/lib/primer/open_project/forms/storage_manual_project_folder_selection.rb @@ -7,30 +7,24 @@ class StorageManualProjectFolderSelection < Primer::Forms::BaseComponent delegate :builder, :form, to: :@input - def initialize(input:, project_storage:, storage_login_button_options:, last_project_folders: {}, + def initialize(input:, project_storage:, last_project_folders: {}, + storage_login_button_options: {}, select_folder_button_options: {}, wrapper_data_attributes: {}) super() @input = input + @project_storage = project_storage @last_project_folders = last_project_folders + @storage_login_button_options = storage_login_button_options - @wrapper_data_attributes = derive_wrapper_data_attributes(wrapper_data_attributes) + @selected_folder_label_options = select_folder_button_options.delete(:selected_folder_label_options) { {} } + @select_folder_button_options = select_folder_button_options + + @wrapper_data_attributes = wrapper_data_attributes end private - def derive_wrapper_data_attributes(options) - options.reverse_merge( - "application-target": "dynamic", - controller: "project-storage-form", - "project-storage-form-folder-mode-value": @project_storage.project_folder_mode, - "project-storage-form-placeholder-folder-name-value": I18n.t(:"storages.label_no_selected_folder"), - "project-storage-form-not-logged-in-validation-value": I18n.t(:"storages.instructions.not_logged_into_storage"), - "project-storage-form-last-project-folders-value": @last_project_folders, - "project-storage-form-target": "projectFolderSection" - ) - end - def storage_oauth_access_granted? OAuthClientToken .exists?(user: User.current, oauth_client: @project_storage.storage.oauth_client) diff --git a/modules/storages/app/components/storages/admin/storages/add_projects_form_modal_component.html.erb b/modules/storages/app/components/storages/admin/storages/add_projects_form_modal_component.html.erb index 086d0a4f2945..d443ea8660d6 100644 --- a/modules/storages/app/components/storages/admin/storages/add_projects_form_modal_component.html.erb +++ b/modules/storages/app/components/storages/admin/storages/add_projects_form_modal_component.html.erb @@ -27,14 +27,6 @@ See COPYRIGHT and LICENSE files for more details. ++#%> -<% helpers.content_controller 'project-storage-form', - dynamic: true, - 'project-storage-form-folder-mode-value': @project_storage.project_folder_mode, - 'project-storage-form-placeholder-folder-name-value': I18n.t(:"storages.label_no_selected_folder"), - 'project-storage-form-not-logged-in-validation-value': I18n.t(:"storages.instructions.not_logged_into_storage"), - 'project-storage-form-last-project-folders-value': @last_project_folders -%> - <%= component_wrapper do primer_form_with( @@ -66,7 +58,17 @@ See COPYRIGHT and LICENSE files for more details. render(Primer::Beta::Text.new) { t(:"storages.help_texts.project_folder_bulk") } end - flex.with_row do + flex.with_row( + data: { + "application-target": "dynamic", + controller: "storages--project-folder-mode-form", + "storages--project-folder-mode-form-folder-mode-value": @project_storage.project_folder_mode, + "storages--project-folder-mode-form-placeholder-folder-name-value": I18n.t(:"storages.label_no_selected_folder"), + "storages--project-folder-mode-form-not-logged-in-validation-value": I18n.t(:"storages.instructions.not_logged_into_storage"), + "storages--project-folder-mode-form-last-project-folders-value": @last_project_folders, + "storages--project-folder-mode-form-target": "projectFolderSection" + } + ) do render(Storages::Admin::ProjectStorages::ProjectFolderModeForm.new(form, project_storage:)) end end diff --git a/modules/storages/app/forms/storages/admin/project_storages/project_folder_mode_form.rb b/modules/storages/app/forms/storages/admin/project_storages/project_folder_mode_form.rb index cd1f117b290c..d24c53644848 100644 --- a/modules/storages/app/forms/storages/admin/project_storages/project_folder_mode_form.rb +++ b/modules/storages/app/forms/storages/admin/project_storages/project_folder_mode_form.rb @@ -36,17 +36,20 @@ class ProjectFolderModeForm < ApplicationForm radio_form.radio_button_group(name: :project_folder_mode) do |radio_group| if @project_storage.project_folder_mode_possible?("inactive") radio_group.radio_button(value: "inactive", label: I18n.t(:"storages.label_no_specific_folder"), - caption: I18n.t(:"storages.instructions.no_specific_folder")) + caption: I18n.t(:"storages.instructions.no_specific_folder", + data: { action: "project-storage-form#updateForm" })) end if @project_storage.project_folder_mode_possible?("automatic") radio_group.radio_button(value: "automatic", label: I18n.t(:"storages.label_automatic_folder"), - caption: I18n.t(:"storages.instructions.automatic_folder")) + caption: I18n.t(:"storages.instructions.automatic_folder", + data: { action: "project-storage-form#updateForm" })) end if @project_storage.project_folder_mode_possible?("manual") radio_group.radio_button(value: "manual", label: I18n.t(:"storages.label_existing_manual_folder"), - caption: I18n.t(:"storages.instructions.existing_manual_folder")) + caption: I18n.t(:"storages.instructions.existing_manual_folder", + data: { action: "project-storage-form#updateForm" })) end end @@ -63,6 +66,17 @@ class ProjectFolderModeForm < ApplicationForm inputs: { input: storage_login_input(@project_storage.storage) } + }, + select_folder_button_options: { + data: { + "storages--project-folder-mode-form-target": "selectProjectFolderButton", + action: "storages--project-folder-mode-form#selectProjectFolder" + }, + selected_folder_label_options: { + data: { + "storages--project-folder-mode-form-target": "selectedFolderText" + } + } } ) end From ebfe1552de786806ee8f7504c93cbecbe8fa5bcf Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Fri, 19 Jul 2024 17:34:08 +0300 Subject: [PATCH 04/22] chore[Op#55967]: overriding `FormControl` flex settings not possible --- .../forms/dsl/storage_manual_project_folder_selection_input.rb | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/primer/open_project/forms/dsl/storage_manual_project_folder_selection_input.rb b/lib/primer/open_project/forms/dsl/storage_manual_project_folder_selection_input.rb index ff667cb89c4c..fc9fed313416 100644 --- a/lib/primer/open_project/forms/dsl/storage_manual_project_folder_selection_input.rb +++ b/lib/primer/open_project/forms/dsl/storage_manual_project_folder_selection_input.rb @@ -16,9 +16,6 @@ def initialize(name:, label:, project_storage:, last_project_folders: {}, storag @storage_login_button_options = storage_login_button_options @select_folder_button_options = select_folder_button_options - # # See: https://github.com/opf/primer_view_components/blob/main/lib/primer/forms/dsl/input.rb#L87C50-L87C60 - # system_arguments[:direction] = :row - super(**system_arguments) end From 89848562794eeb84e610422b9ca6998bafe79bc0 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Fri, 19 Jul 2024 18:32:24 +0300 Subject: [PATCH 05/22] feat[Op#55967]: begin project folder from JS Controller --- .../project-folder-mode-form.controller.ts | 73 +++++++++++++++++++ ...e_manual_project_folder_selection_input.rb | 8 +- ...e_manual_project_folder_selection.html.erb | 2 +- ...storage_manual_project_folder_selection.rb | 6 +- ...add_projects_form_modal_component.html.erb | 6 +- .../project_folder_mode_form.rb | 22 ++++-- 6 files changed, 97 insertions(+), 20 deletions(-) create mode 100644 frontend/src/stimulus/controllers/dynamic/storages/project-folder-mode-form.controller.ts diff --git a/frontend/src/stimulus/controllers/dynamic/storages/project-folder-mode-form.controller.ts b/frontend/src/stimulus/controllers/dynamic/storages/project-folder-mode-form.controller.ts new file mode 100644 index 000000000000..577c2237fa0a --- /dev/null +++ b/frontend/src/stimulus/controllers/dynamic/storages/project-folder-mode-form.controller.ts @@ -0,0 +1,73 @@ +/* + * -- 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 ProjectFolderModeForm extends Controller { + static targets = [ + 'projectFolderSection', + 'selectProjectFolderButton', + 'selectedFolderLabel', + 'storageLoginButton', + ]; + + static values = { + projectFolderMode: String, + }; + + declare readonly projectFolderSectionTarget:HTMLElement; + declare readonly selectProjectFolderButtonTarget:HTMLElement; + declare readonly selectedFolderLabelTarget:HTMLElement; + declare readonly storageLoginButtonTarget:HTMLElement; + + declare readonly hasProjectFolderSectionTarget:boolean; + + declare projectFolderModeValue:string; + + connect():void { + // On first load if projectFolderModeValue is manual, show the projectFolderSection + this.toggleFolderDisplay(this.projectFolderModeValue); + } + + updateForm(evt:InputEvent):void { + const projectFolderMode = (evt.target as HTMLInputElement).value; + + this.toggleFolderDisplay(projectFolderMode); + } + + private toggleFolderDisplay(value:string):void { + // If the manual radio button is selected, show the manual folder selection section + if (this.hasProjectFolderSectionTarget && value === 'manual') { + this.projectFolderSectionTarget.classList.remove('d-none'); + } else { + this.projectFolderSectionTarget.classList.add('d-none'); + } + } +} diff --git a/lib/primer/open_project/forms/dsl/storage_manual_project_folder_selection_input.rb b/lib/primer/open_project/forms/dsl/storage_manual_project_folder_selection_input.rb index fc9fed313416..115271847a88 100644 --- a/lib/primer/open_project/forms/dsl/storage_manual_project_folder_selection_input.rb +++ b/lib/primer/open_project/forms/dsl/storage_manual_project_folder_selection_input.rb @@ -8,13 +8,14 @@ class StorageManualProjectFolderSelectionInput < Primer::Forms::Dsl::Input attr_reader :name, :label def initialize(name:, label:, project_storage:, last_project_folders: {}, storage_login_button_options: {}, - select_folder_button_options: {}, **system_arguments) + select_folder_button_options: {}, wrapper_arguments: {}, **system_arguments) @name = name @label = label @project_storage = project_storage @last_project_folders = last_project_folders @storage_login_button_options = storage_login_button_options @select_folder_button_options = select_folder_button_options + @wrapper_arguments = wrapper_arguments super(**system_arguments) end @@ -25,12 +26,13 @@ def to_component project_storage: @project_storage, last_project_folders: @last_project_folders, storage_login_button_options: @storage_login_button_options, - select_folder_button_options: @select_folder_button_options + select_folder_button_options: @select_folder_button_options, + wrapper_arguments: @wrapper_arguments ) end def type - :storage_login_button + :storage_manual_project_folder_selection end def focusable? diff --git a/lib/primer/open_project/forms/storage_manual_project_folder_selection.html.erb b/lib/primer/open_project/forms/storage_manual_project_folder_selection.html.erb index 5e7bb8d0101b..d4334c10240a 100644 --- a/lib/primer/open_project/forms/storage_manual_project_folder_selection.html.erb +++ b/lib/primer/open_project/forms/storage_manual_project_folder_selection.html.erb @@ -1,4 +1,4 @@ -<%= render(Primer::BaseComponent.new(tag: :div, data: @wrapper_data_attributes)) do %> +<%= render(Primer::BaseComponent.new(tag: :div, data: @wrapper_data_attributes, classes: @wrapper_classes)) do %> <%= render(Primer::BaseComponent.new(tag: :div, display: :inline_flex, direction: :row, align_items: :center)) do %> <% if storage_oauth_access_granted? %> <%= diff --git a/lib/primer/open_project/forms/storage_manual_project_folder_selection.rb b/lib/primer/open_project/forms/storage_manual_project_folder_selection.rb index 3da0982a0869..1a8ceb9c6ee1 100644 --- a/lib/primer/open_project/forms/storage_manual_project_folder_selection.rb +++ b/lib/primer/open_project/forms/storage_manual_project_folder_selection.rb @@ -8,8 +8,7 @@ class StorageManualProjectFolderSelection < Primer::Forms::BaseComponent delegate :builder, :form, to: :@input def initialize(input:, project_storage:, last_project_folders: {}, - storage_login_button_options: {}, select_folder_button_options: {}, - wrapper_data_attributes: {}) + storage_login_button_options: {}, select_folder_button_options: {}, wrapper_arguments: {}) super() @input = input @@ -20,7 +19,8 @@ def initialize(input:, project_storage:, last_project_folders: {}, @selected_folder_label_options = select_folder_button_options.delete(:selected_folder_label_options) { {} } @select_folder_button_options = select_folder_button_options - @wrapper_data_attributes = wrapper_data_attributes + @wrapper_data_attributes = wrapper_arguments.delete(:data) { {} } + @wrapper_classes = wrapper_arguments.delete(:classes) { [] } end private diff --git a/modules/storages/app/components/storages/admin/storages/add_projects_form_modal_component.html.erb b/modules/storages/app/components/storages/admin/storages/add_projects_form_modal_component.html.erb index d443ea8660d6..f7b7e2a9ed0a 100644 --- a/modules/storages/app/components/storages/admin/storages/add_projects_form_modal_component.html.erb +++ b/modules/storages/app/components/storages/admin/storages/add_projects_form_modal_component.html.erb @@ -62,11 +62,7 @@ See COPYRIGHT and LICENSE files for more details. data: { "application-target": "dynamic", controller: "storages--project-folder-mode-form", - "storages--project-folder-mode-form-folder-mode-value": @project_storage.project_folder_mode, - "storages--project-folder-mode-form-placeholder-folder-name-value": I18n.t(:"storages.label_no_selected_folder"), - "storages--project-folder-mode-form-not-logged-in-validation-value": I18n.t(:"storages.instructions.not_logged_into_storage"), - "storages--project-folder-mode-form-last-project-folders-value": @last_project_folders, - "storages--project-folder-mode-form-target": "projectFolderSection" + 'storages--project-folder-mode-form-project-folder-mode-value': @project_storage.project_folder_mode } ) do render(Storages::Admin::ProjectStorages::ProjectFolderModeForm.new(form, project_storage:)) diff --git a/modules/storages/app/forms/storages/admin/project_storages/project_folder_mode_form.rb b/modules/storages/app/forms/storages/admin/project_storages/project_folder_mode_form.rb index d24c53644848..9f54d08ad7f8 100644 --- a/modules/storages/app/forms/storages/admin/project_storages/project_folder_mode_form.rb +++ b/modules/storages/app/forms/storages/admin/project_storages/project_folder_mode_form.rb @@ -36,20 +36,20 @@ class ProjectFolderModeForm < ApplicationForm radio_form.radio_button_group(name: :project_folder_mode) do |radio_group| if @project_storage.project_folder_mode_possible?("inactive") radio_group.radio_button(value: "inactive", label: I18n.t(:"storages.label_no_specific_folder"), - caption: I18n.t(:"storages.instructions.no_specific_folder", - data: { action: "project-storage-form#updateForm" })) + caption: I18n.t(:"storages.instructions.no_specific_folder"), + data: { action: "storages--project-folder-mode-form#updateForm" }) end if @project_storage.project_folder_mode_possible?("automatic") radio_group.radio_button(value: "automatic", label: I18n.t(:"storages.label_automatic_folder"), - caption: I18n.t(:"storages.instructions.automatic_folder", - data: { action: "project-storage-form#updateForm" })) + caption: I18n.t(:"storages.instructions.automatic_folder"), + data: { action: "storages--project-folder-mode-form#updateForm" }) end if @project_storage.project_folder_mode_possible?("manual") radio_group.radio_button(value: "manual", label: I18n.t(:"storages.label_existing_manual_folder"), - caption: I18n.t(:"storages.instructions.existing_manual_folder", - data: { action: "project-storage-form#updateForm" })) + caption: I18n.t(:"storages.instructions.existing_manual_folder"), + data: { action: "storages--project-folder-mode-form#updateForm" }) end end @@ -61,7 +61,7 @@ class ProjectFolderModeForm < ApplicationForm last_project_folders: @last_project_folders, storage_login_button_options: { data: { - "project-storage-form-target": "loginButton" + "storages--project-folder-mode-form-target": "storageLoginButton" }, inputs: { input: storage_login_input(@project_storage.storage) @@ -74,9 +74,15 @@ class ProjectFolderModeForm < ApplicationForm }, selected_folder_label_options: { data: { - "storages--project-folder-mode-form-target": "selectedFolderText" + "storages--project-folder-mode-form-target": "selectedFolderLabel" } } + }, + wrapper_arguments: { + data: { + "storages--project-folder-mode-form-target": "projectFolderSection" + }, + classes: ["d-none"] } ) end From 3c0fc8bb9423f0a524835a6601390c5c8d51680d Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Mon, 22 Jul 2024 10:25:21 +0300 Subject: [PATCH 06/22] feat[Op#55967]: register `opce-location-picker-modal` --- frontend/src/app/app.module.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/frontend/src/app/app.module.ts b/frontend/src/app/app.module.ts index 19d0f3e42ccf..bf937e791d76 100644 --- a/frontend/src/app/app.module.ts +++ b/frontend/src/app/app.module.ts @@ -176,6 +176,9 @@ import { ShareUpsaleComponent } from 'core-app/features/enterprise/share-upsale/ import { StorageLoginButtonComponent, } from 'core-app/shared/components/storages/storage-login-button/storage-login-button.component'; +import { + LocationPickerModalComponent, +} from 'core-app/shared/components/storages/location-picker-modal/location-picker-modal.component'; export function initializeServices(injector:Injector) { return () => { @@ -366,6 +369,7 @@ export class OpenProjectModule { registerCustomElement('opce-exclusion-info', OpExclusionInfoComponent, { injector }); registerCustomElement('opce-attachments', OpAttachmentsComponent, { injector }); registerCustomElement('opce-storage-login-button', StorageLoginButtonComponent, { injector }); + registerCustomElement('opce-location-picker-modal', LocationPickerModalComponent, { injector }); // TODO: These elements are now registered custom elements, but are actually single-use components. They should be removed when we move these pages to Rails. registerCustomElement('opce-new-project', NewProjectComponent, { injector }); From 77f63864365c96efa44d438776b31c5f83ded0f2 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Mon, 22 Jul 2024 15:25:51 +0300 Subject: [PATCH 07/22] WIP[Op#55967]: get modal to render for first time --- .../project-storage-form.controller.ts | 12 +- .../project-folder-mode-form.controller.ts | 143 +++++++++++++++++- ...add_projects_form_modal_component.html.erb | 5 +- .../project_folder_mode_form.rb | 24 +++ 4 files changed, 168 insertions(+), 16 deletions(-) diff --git a/frontend/src/stimulus/controllers/dynamic/project-storage-form.controller.ts b/frontend/src/stimulus/controllers/dynamic/project-storage-form.controller.ts index 4032b2fb8665..e2baad5f1dd7 100644 --- a/frontend/src/stimulus/controllers/dynamic/project-storage-form.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/project-storage-form.controller.ts @@ -68,29 +68,19 @@ export default class ProjectStorageFormController extends Controller { }; declare folderModeValue:string; - declare placeholderFolderNameValue:string; - declare notLoggedInValidationValue:string; - declare lastProjectFoldersValue:{ manual:string; automatic:string }; declare readonly storageTarget:HTMLElement; - declare readonly selectProjectFolderButtonTarget:HTMLButtonElement; - declare readonly loginButtonTarget:HTMLButtonElement; - declare readonly projectFolderSectionTarget:HTMLElement; - declare readonly projectFolderIdInputTarget:HTMLInputElement; - declare readonly projectFolderIdValidationTarget:HTMLSpanElement; - - declare readonly hasProjectFolderIdValidationTarget:boolean; - declare readonly selectedFolderTextTarget:HTMLSpanElement; + declare readonly hasProjectFolderIdValidationTarget:boolean; declare readonly hasProjectFolderSectionTarget:boolean; connect():void { diff --git a/frontend/src/stimulus/controllers/dynamic/storages/project-folder-mode-form.controller.ts b/frontend/src/stimulus/controllers/dynamic/storages/project-folder-mode-form.controller.ts index 577c2237fa0a..83e00307a4d2 100644 --- a/frontend/src/stimulus/controllers/dynamic/storages/project-folder-mode-form.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/storages/project-folder-mode-form.controller.ts @@ -29,37 +29,153 @@ */ import { Controller } from '@hotwired/stimulus'; +import { + combineLatest, + from, + Observable, + of, +} from 'rxjs'; +import { + filter, + map, + switchMap, +} from 'rxjs/operators'; + +import { IStorageFile } from 'core-app/core/state/storage-files/storage-file.model'; +import { IStorage } from 'core-app/core/state/storages/storage.model'; +import { OpModalService } from 'core-app/shared/components/modal/modal.service'; +import { + LocationPickerModalComponent, +} from 'core-app/shared/components/storages/location-picker-modal/location-picker-modal.component'; +import { storageConnected } from 'core-app/shared/components/storages/storages-constants.const'; export default class ProjectFolderModeForm extends Controller { static targets = [ 'projectFolderSection', + 'projectFolderIdInput', 'selectProjectFolderButton', 'selectedFolderLabel', 'storageLoginButton', + 'storage', ]; static values = { projectFolderMode: String, + placeholderFolderName: String, + notLoggedInValidation: String, + lastProjectFolders: Object, }; declare readonly projectFolderSectionTarget:HTMLElement; declare readonly selectProjectFolderButtonTarget:HTMLElement; declare readonly selectedFolderLabelTarget:HTMLElement; declare readonly storageLoginButtonTarget:HTMLElement; + declare readonly storageTarget:HTMLElement; + declare readonly projectFolderIdInputTarget:HTMLInputElement; declare readonly hasProjectFolderSectionTarget:boolean; + declare readonly hasStorageLoginButtonTarget:boolean; + declare readonly hasProjectFolderButtonTarget:boolean; declare projectFolderModeValue:string; + declare placeholderFolderNameValue:string; + declare notLoggedInValidationValue:string; + declare lastProjectFoldersValue:{ manual:string; automatic:string }; connect():void { - // On first load if projectFolderModeValue is manual, show the projectFolderSection - this.toggleFolderDisplay(this.projectFolderModeValue); + combineLatest([ + this.fetchStorageAuthorizationState(), + this.fetchProjectFolder(), + ]).subscribe(([isConnected, projectFolder]) => { + if (isConnected) { + this.selectedFolderLabelTarget.innerText = projectFolder === null + ? this.placeholderFolderNameValue + : projectFolder.name; + } else { + this.selectedFolderLabelTarget.innerText = this.notLoggedInValidationValue; + } + + this.toggleFolderDisplay(this.projectFolderModeValue); + this.setProjectFolderModeQueryParam(this.projectFolderModeValue); + }); + } + + selectProjectFolder(_evt:Event):void { + const locals = { + projectFolderHref: this.projectFolderHref, + storage: this.storage, + }; + + this.modalService + .pipe( + switchMap((service) => service.show(LocationPickerModalComponent, 'global', locals)), + switchMap((modal) => modal.closingEvent), + filter((modal) => modal.submitted), + ) + .subscribe((modal) => { + this.selectedFolderLabelTarget.innerText = modal.location.name; + this.projectFolderIdInputTarget.value = modal.location.id as string; + }); } updateForm(evt:InputEvent):void { - const projectFolderMode = (evt.target as HTMLInputElement).value; + const mode = (evt.target as HTMLInputElement).value; + const { manual, automatic } = this.lastProjectFoldersValue; + + switch (mode) { + case 'manual': + this.projectFolderIdInputTarget.value = manual ?? ''; + + this.fetchProjectFolder().subscribe((projectFolder) => { + this.selectedFolderLabelTarget.innerText = projectFolder === null + ? this.placeholderFolderNameValue + : projectFolder.name; + }); - this.toggleFolderDisplay(projectFolderMode); + break; + case 'automatic': + this.projectFolderIdInputTarget.value = automatic ?? ''; + break; + default: + this.projectFolderIdInputTarget.value = ''; + } + + this.projectFolderModeValue = mode; + this.toggleFolderDisplay(mode); + this.setProjectFolderModeQueryParam(mode); + } + + private fetchStorageAuthorizationState():Observable { + return from(fetch(this.storage._links.self.href) + .then((data) => data.json())) + .pipe( + map((storage:IStorage) => storage._links.authorizationState.href === storageConnected), + ); + } + + private fetchProjectFolder():Observable { + const href = this.projectFolderHref; + if (href === null) { + return of(null); + } + + return from(fetch(href).then((data) => data.json())) + .pipe( + map((file) => { + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + if (file._type === 'StorageFile') { + return file as IStorageFile; + } + + return null; + }), + ); + } + + private setProjectFolderModeQueryParam(mode:string) { + const url = new URL(window.location.href); + url.searchParams.set('storages_project_storage[project_folder_mode]', mode); + window.history.replaceState(window.history.state, '', url); } private toggleFolderDisplay(value:string):void { @@ -70,4 +186,23 @@ export default class ProjectFolderModeForm extends Controller { this.projectFolderSectionTarget.classList.add('d-none'); } } + + private get projectFolderHref():string|null { + const projectFolderId = this.projectFolderIdInputTarget.value; + + if (projectFolderId.length === 0) { + return null; + } + + return `${this.storage._links.self.href}/files/${projectFolderId}`; + } + + private get modalService():Observable { + return from(window.OpenProject.getPluginContext()) + .pipe(map((pluginContext) => pluginContext.services.opModalService)); + } + + private get storage():IStorage { + return JSON.parse(this.storageTarget.dataset.storage as string) as IStorage; + } } diff --git a/modules/storages/app/components/storages/admin/storages/add_projects_form_modal_component.html.erb b/modules/storages/app/components/storages/admin/storages/add_projects_form_modal_component.html.erb index f7b7e2a9ed0a..2ef1aa7fbea8 100644 --- a/modules/storages/app/components/storages/admin/storages/add_projects_form_modal_component.html.erb +++ b/modules/storages/app/components/storages/admin/storages/add_projects_form_modal_component.html.erb @@ -62,7 +62,10 @@ See COPYRIGHT and LICENSE files for more details. data: { "application-target": "dynamic", controller: "storages--project-folder-mode-form", - 'storages--project-folder-mode-form-project-folder-mode-value': @project_storage.project_folder_mode + 'storages--project-folder-mode-form-project-folder-mode-value': @project_storage.project_folder_mode, + 'storages--project-folder-mode-form-placeholder-folder-name-value': t(:"storages.label_no_selected_folder"), + 'storages--project-folder-mode-form-not-logged-in-validation-value': t(:"storages.instructions.not_logged_into_storage"), + 'storages--project-folder-mode-form-last-project-folders-value': @last_project_folders } ) do render(Storages::Admin::ProjectStorages::ProjectFolderModeForm.new(form, project_storage:)) diff --git a/modules/storages/app/forms/storages/admin/project_storages/project_folder_mode_form.rb b/modules/storages/app/forms/storages/admin/project_storages/project_folder_mode_form.rb index 9f54d08ad7f8..e4f13c74bf60 100644 --- a/modules/storages/app/forms/storages/admin/project_storages/project_folder_mode_form.rb +++ b/modules/storages/app/forms/storages/admin/project_storages/project_folder_mode_form.rb @@ -31,6 +31,7 @@ module Admin module ProjectStorages class ProjectFolderModeForm < ApplicationForm include StorageLoginHelper + include APIV3Helper form do |radio_form| radio_form.radio_button_group(name: :project_folder_mode) do |radio_group| @@ -53,6 +54,29 @@ class ProjectFolderModeForm < ApplicationForm end end + radio_form.hidden( + name: :storage, + value: @project_storage.storage_id, + data: { + "storages--project-folder-mode-form-target": "storage", + storage: { + name: @project_storage.storage.name, + id: @project_storage.storage.id, + _links: { + self: { href: api_v3_paths.storage(@project_storage.storage.id) }, + type: { href: API::V3::Storages::URN_STORAGE_TYPE_NEXTCLOUD } + } + } + } + ) + + radio_form.hidden( + name: :project_folder_id, + data: { + "storages--project-folder-mode-form-target": "projectFolderIdInput" + } + ) + if @project_storage.project_folder_mode_possible?("manual") radio_form.storage_manual_project_folder_selection( name: :project_folder, From 8e56d10d509e8ab79c35fce23e289865fa972278 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 25 Jul 2024 15:09:12 +0300 Subject: [PATCH 08/22] feat[Op#55967]: Add a custom target to modal component Allow a custom target for the benefit of nested dialog which overlays from new stacking context. The custom target can be placed anywhere. Co-authored-by: Andreas Pfohl --- frontend/src/app/app.module.ts | 2 + .../modal/custom-modal-overlay.component.ts | 51 +++++++++++++++++++ .../modal/modal-overlay.component.ts | 11 +++- .../shared/components/modal/modal.module.ts | 3 ++ .../shared/components/modal/modal.service.ts | 7 ++- .../modal/portal-outlet-target.enum.ts | 32 ++++++++++++ .../project-folder-mode-form.controller.ts | 3 +- ...add_projects_form_modal_component.html.erb | 4 ++ .../add_projects_form_modal_component.rb | 1 + 9 files changed, 110 insertions(+), 4 deletions(-) create mode 100644 frontend/src/app/shared/components/modal/custom-modal-overlay.component.ts create mode 100644 frontend/src/app/shared/components/modal/portal-outlet-target.enum.ts diff --git a/frontend/src/app/app.module.ts b/frontend/src/app/app.module.ts index bf937e791d76..468f2de121fc 100644 --- a/frontend/src/app/app.module.ts +++ b/frontend/src/app/app.module.ts @@ -179,6 +179,7 @@ import { import { LocationPickerModalComponent, } from 'core-app/shared/components/storages/location-picker-modal/location-picker-modal.component'; +import { OpCustomModalOverlayComponent } from 'core-app/shared/components/modal/custom-modal-overlay.component'; export function initializeServices(injector:Injector) { return () => { @@ -370,6 +371,7 @@ export class OpenProjectModule { registerCustomElement('opce-attachments', OpAttachmentsComponent, { injector }); registerCustomElement('opce-storage-login-button', StorageLoginButtonComponent, { injector }); registerCustomElement('opce-location-picker-modal', LocationPickerModalComponent, { injector }); + registerCustomElement('opce-custom-modal-overlay', OpCustomModalOverlayComponent, { injector }); // TODO: These elements are now registered custom elements, but are actually single-use components. They should be removed when we move these pages to Rails. registerCustomElement('opce-new-project', NewProjectComponent, { injector }); diff --git a/frontend/src/app/shared/components/modal/custom-modal-overlay.component.ts b/frontend/src/app/shared/components/modal/custom-modal-overlay.component.ts new file mode 100644 index 000000000000..a919192fced3 --- /dev/null +++ b/frontend/src/app/shared/components/modal/custom-modal-overlay.component.ts @@ -0,0 +1,51 @@ +// -- copyright +// OpenProject is an open source project management software. +// Copyright (C) 2012-2024 the OpenProject GmbH +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +// +// See COPYRIGHT and LICENSE files for more details. +//++ + +import { + ChangeDetectionStrategy, + Component, +} from '@angular/core'; + +import { ModalData } from 'core-app/shared/components/modal/modal.service'; +import { OpModalOverlayComponent } from 'core-app/shared/components/modal/modal-overlay.component'; +import { PortalOutletTarget } from 'core-app/shared/components/modal/portal-outlet-target.enum'; + +export const opCustomModalOverlaySelector = 'op-custom-modal-overlay'; + +@Component({ + selector: opCustomModalOverlaySelector, + templateUrl: './modal-overlay.component.html', + changeDetection: ChangeDetectionStrategy.OnPush, +}) +export class OpCustomModalOverlayComponent extends OpModalOverlayComponent { + protected isDefaultTarget(modalData:ModalData | null):boolean { + if (modalData === null) return true; + + return modalData.target === PortalOutletTarget.Custom; + } +} diff --git a/frontend/src/app/shared/components/modal/modal-overlay.component.ts b/frontend/src/app/shared/components/modal/modal-overlay.component.ts index 5fdd4dd9077a..d7e1c0c857cf 100644 --- a/frontend/src/app/shared/components/modal/modal-overlay.component.ts +++ b/frontend/src/app/shared/components/modal/modal-overlay.component.ts @@ -39,11 +39,12 @@ import { ComponentPortal, } from '@angular/cdk/portal'; import { combineLatest } from 'rxjs'; -import { distinctUntilChanged } from 'rxjs/operators'; +import { distinctUntilChanged, filter } from 'rxjs/operators'; import { I18nService } from 'core-app/core/i18n/i18n.service'; import { OpModalComponent } from 'core-app/shared/components/modal/modal.component'; import { ModalData, OpModalService } from 'core-app/shared/components/modal/modal.service'; +import { PortalOutletTarget } from 'core-app/shared/components/modal/portal-outlet-target.enum'; export const opModalOverlaySelector = 'op-modal-overlay'; @@ -84,7 +85,7 @@ export class OpModalOverlayComponent { combineLatest([ this.activeModalInstance$, // multiple 'closing' events in a row are squashed - this.activeModalData$.pipe(distinctUntilChanged()), + this.activeModalData$.pipe(distinctUntilChanged(), filter(this.isDefaultTarget.bind(this))), ]) .subscribe(([instance, data]) => { if (instance === null && data === null) { @@ -103,6 +104,12 @@ export class OpModalOverlayComponent { }); } + protected isDefaultTarget(modalData:ModalData | null):boolean { + if (modalData === null) return true; + + return modalData.target === PortalOutletTarget.Default; + } + public close($event:MouseEvent, includeChildClicks = true):void { if (!includeChildClicks && $event.currentTarget !== $event.target) { return; diff --git a/frontend/src/app/shared/components/modal/modal.module.ts b/frontend/src/app/shared/components/modal/modal.module.ts index 36012dedd726..ebae80a4730a 100644 --- a/frontend/src/app/shared/components/modal/modal.module.ts +++ b/frontend/src/app/shared/components/modal/modal.module.ts @@ -7,6 +7,7 @@ import { OpModalWrapperAugmentService } from './modal-wrapper-augment.service'; import { OpModalBannerComponent } from 'core-app/shared/components/modal/modal-banner/modal-banner.component'; import { OpModalOverlayComponent } from 'core-app/shared/components/modal/modal-overlay.component'; import { CommonModule } from '@angular/common'; +import { OpCustomModalOverlayComponent } from 'core-app/shared/components/modal/custom-modal-overlay.component'; @NgModule({ imports: [ @@ -18,6 +19,7 @@ import { CommonModule } from '@angular/common'; ], exports: [ OpModalOverlayComponent, + OpCustomModalOverlayComponent, OpModalBannerComponent, ], providers: [ @@ -26,6 +28,7 @@ import { CommonModule } from '@angular/common'; declarations: [ OpModalBannerComponent, OpModalOverlayComponent, + OpCustomModalOverlayComponent, ], }) export class OpenprojectModalModule { } diff --git a/frontend/src/app/shared/components/modal/modal.service.ts b/frontend/src/app/shared/components/modal/modal.service.ts index 436fe1944811..c495e3d8a3dc 100644 --- a/frontend/src/app/shared/components/modal/modal.service.ts +++ b/frontend/src/app/shared/components/modal/modal.service.ts @@ -26,16 +26,17 @@ // See COPYRIGHT and LICENSE files for more details. //++ +import { ComponentType, PortalInjector } from '@angular/cdk/portal'; import { Injectable, InjectionToken, Injector, } from '@angular/core'; -import { ComponentType, PortalInjector } from '@angular/cdk/portal'; import { BehaviorSubject, Observable } from 'rxjs'; import { filter, take } from 'rxjs/operators'; import { OpModalComponent } from 'core-app/shared/components/modal/modal.component'; +import { PortalOutletTarget } from 'core-app/shared/components/modal/portal-outlet-target.enum'; export const OpModalLocalsToken = new InjectionToken('OP_MODAL_LOCALS'); @@ -44,6 +45,7 @@ export interface ModalData { injector:Injector; notFullscreen:boolean; mobileTopPosition:boolean; + target:PortalOutletTarget; } @Injectable({ providedIn: 'root' }) @@ -74,6 +76,7 @@ export class OpModalService { * @param locals A map to be injected via token into the component. * @param notFullscreen * @param mobileTopPosition + * @param target An optional target override for the modal portal outlet */ public show( modal:ComponentType, @@ -81,6 +84,7 @@ export class OpModalService { locals:Record = {}, notFullscreen = false, mobileTopPosition = false, + target = PortalOutletTarget.Default, ):Observable { this.close(); @@ -94,6 +98,7 @@ export class OpModalService { injector: this.injectorFor(injector, locals), notFullscreen, mobileTopPosition, + target, }); return this.activeModalInstance$ diff --git a/frontend/src/app/shared/components/modal/portal-outlet-target.enum.ts b/frontend/src/app/shared/components/modal/portal-outlet-target.enum.ts new file mode 100644 index 000000000000..35fe06e77d92 --- /dev/null +++ b/frontend/src/app/shared/components/modal/portal-outlet-target.enum.ts @@ -0,0 +1,32 @@ +// -- copyright +// OpenProject is an open source project management software. +// Copyright (C) 2012-2024 the OpenProject GmbH +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +// +// See COPYRIGHT and LICENSE files for more details. +//++ + +export enum PortalOutletTarget { + Default = 1, // targets 'op-modal-overlay' in base.html + Custom, // targets 'op-custom-modal-overlay' (can be anywhere) +} diff --git a/frontend/src/stimulus/controllers/dynamic/storages/project-folder-mode-form.controller.ts b/frontend/src/stimulus/controllers/dynamic/storages/project-folder-mode-form.controller.ts index 83e00307a4d2..588cd136ffa5 100644 --- a/frontend/src/stimulus/controllers/dynamic/storages/project-folder-mode-form.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/storages/project-folder-mode-form.controller.ts @@ -44,6 +44,7 @@ import { import { IStorageFile } from 'core-app/core/state/storage-files/storage-file.model'; import { IStorage } from 'core-app/core/state/storages/storage.model'; import { OpModalService } from 'core-app/shared/components/modal/modal.service'; +import { PortalOutletTarget } from 'core-app/shared/components/modal/portal-outlet-target.enum'; import { LocationPickerModalComponent, } from 'core-app/shared/components/storages/location-picker-modal/location-picker-modal.component'; @@ -108,7 +109,7 @@ export default class ProjectFolderModeForm extends Controller { this.modalService .pipe( - switchMap((service) => service.show(LocationPickerModalComponent, 'global', locals)), + switchMap((service) => service.show(LocationPickerModalComponent, 'global', locals, false, false, PortalOutletTarget.Custom)), switchMap((modal) => modal.closingEvent), filter((modal) => modal.submitted), ) diff --git a/modules/storages/app/components/storages/admin/storages/add_projects_form_modal_component.html.erb b/modules/storages/app/components/storages/admin/storages/add_projects_form_modal_component.html.erb index 2ef1aa7fbea8..7acc10220a33 100644 --- a/modules/storages/app/components/storages/admin/storages/add_projects_form_modal_component.html.erb +++ b/modules/storages/app/components/storages/admin/storages/add_projects_form_modal_component.html.erb @@ -42,6 +42,10 @@ See COPYRIGHT and LICENSE files for more details. aria: { label: title } )) do flex_layout do |flex| + flex.with_row do + angular_component_tag 'opce-custom-modal-overlay' + end + flex.with_row(mb: 3) do render(Storages::Admin::Storages::AddProjectsAutocompleterForm.new(form, project_storage:)) end diff --git a/modules/storages/app/components/storages/admin/storages/add_projects_form_modal_component.rb b/modules/storages/app/components/storages/admin/storages/add_projects_form_modal_component.rb index c8ab831a2c68..9093db2cd38b 100644 --- a/modules/storages/app/components/storages/admin/storages/add_projects_form_modal_component.rb +++ b/modules/storages/app/components/storages/admin/storages/add_projects_form_modal_component.rb @@ -33,6 +33,7 @@ class AddProjectsFormModalComponent < ApplicationComponent include OpPrimer::ComponentHelpers include OpTurbo::Streamable include StimulusHelper + include AngularHelper def initialize(project_storage:, **) @project_storage = project_storage From 3e787f66d43bfc4b8cb14afb5192a3c7cd1b5a60 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 25 Jul 2024 15:11:56 +0300 Subject: [PATCH 09/22] chore[Op#55967]: remove location picker custom element --- frontend/src/app/app.module.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/frontend/src/app/app.module.ts b/frontend/src/app/app.module.ts index 468f2de121fc..86d3af6284ed 100644 --- a/frontend/src/app/app.module.ts +++ b/frontend/src/app/app.module.ts @@ -176,9 +176,6 @@ import { ShareUpsaleComponent } from 'core-app/features/enterprise/share-upsale/ import { StorageLoginButtonComponent, } from 'core-app/shared/components/storages/storage-login-button/storage-login-button.component'; -import { - LocationPickerModalComponent, -} from 'core-app/shared/components/storages/location-picker-modal/location-picker-modal.component'; import { OpCustomModalOverlayComponent } from 'core-app/shared/components/modal/custom-modal-overlay.component'; export function initializeServices(injector:Injector) { @@ -370,7 +367,6 @@ export class OpenProjectModule { registerCustomElement('opce-exclusion-info', OpExclusionInfoComponent, { injector }); registerCustomElement('opce-attachments', OpAttachmentsComponent, { injector }); registerCustomElement('opce-storage-login-button', StorageLoginButtonComponent, { injector }); - registerCustomElement('opce-location-picker-modal', LocationPickerModalComponent, { injector }); registerCustomElement('opce-custom-modal-overlay', OpCustomModalOverlayComponent, { injector }); // TODO: These elements are now registered custom elements, but are actually single-use components. They should be removed when we move these pages to Rails. From d95a6cf8c9200b881106fb38ab879e1057d56361 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Mon, 29 Jul 2024 16:40:14 +0300 Subject: [PATCH 10/22] chore[Op#55967]: extend `ProjectStorageFormController` --- .../project-storage-form.controller.ts | 14 +- .../project-folder-mode-form.controller.ts | 145 ++---------------- .../project_folder_mode_form.rb | 4 +- 3 files changed, 19 insertions(+), 144 deletions(-) diff --git a/frontend/src/stimulus/controllers/dynamic/project-storage-form.controller.ts b/frontend/src/stimulus/controllers/dynamic/project-storage-form.controller.ts index e2baad5f1dd7..e1a332f45c0f 100644 --- a/frontend/src/stimulus/controllers/dynamic/project-storage-form.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/project-storage-form.controller.ts @@ -153,16 +153,16 @@ export default class ProjectStorageFormController extends Controller { this.setProjectFolderModeQueryParam(mode); } - private get modalService():Observable { + protected get modalService():Observable { return from(window.OpenProject.getPluginContext()) .pipe(map((pluginContext) => pluginContext.services.opModalService)); } - private get storage():IStorage { + protected get storage():IStorage { return JSON.parse(this.storageTarget.dataset.storage as string) as IStorage; } - private get projectFolderHref():string|null { + protected get projectFolderHref():string|null { const projectFolderId = this.projectFolderIdInputTarget.value; if (projectFolderId.length === 0) { @@ -172,7 +172,7 @@ export default class ProjectStorageFormController extends Controller { return `${this.storage._links.self.href}/files/${projectFolderId}`; } - private fetchStorageAuthorizationState():Observable { + protected fetchStorageAuthorizationState():Observable { return from(fetch(this.storage._links.self.href) .then((data) => data.json())) .pipe( @@ -180,7 +180,7 @@ export default class ProjectStorageFormController extends Controller { ); } - private fetchProjectFolder():Observable { + protected fetchProjectFolder():Observable { const href = this.projectFolderHref; if (href === null) { return of(null); @@ -199,13 +199,13 @@ export default class ProjectStorageFormController extends Controller { ); } - private setProjectFolderModeQueryParam(mode:string) { + protected setProjectFolderModeQueryParam(mode:string) { const url = new URL(window.location.href); url.searchParams.set('storages_project_storage[project_folder_mode]', mode); window.history.replaceState(window.history.state, '', url); } - private toggleFolderDisplay(value:string):void { + protected toggleFolderDisplay(value:string):void { // If the manual radio button is selected, show the manual folder selection section if (this.hasProjectFolderSectionTarget && value === 'manual') { this.projectFolderSectionTarget.style.display = 'flex'; diff --git a/frontend/src/stimulus/controllers/dynamic/storages/project-folder-mode-form.controller.ts b/frontend/src/stimulus/controllers/dynamic/storages/project-folder-mode-form.controller.ts index 588cd136ffa5..d26e9e7b5cec 100644 --- a/frontend/src/stimulus/controllers/dynamic/storages/project-folder-mode-form.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/storages/project-folder-mode-form.controller.ts @@ -28,76 +28,30 @@ * ++ */ -import { Controller } from '@hotwired/stimulus'; -import { - combineLatest, - from, - Observable, - of, -} from 'rxjs'; -import { - filter, - map, - switchMap, -} from 'rxjs/operators'; - -import { IStorageFile } from 'core-app/core/state/storage-files/storage-file.model'; -import { IStorage } from 'core-app/core/state/storages/storage.model'; -import { OpModalService } from 'core-app/shared/components/modal/modal.service'; import { PortalOutletTarget } from 'core-app/shared/components/modal/portal-outlet-target.enum'; import { LocationPickerModalComponent, } from 'core-app/shared/components/storages/location-picker-modal/location-picker-modal.component'; -import { storageConnected } from 'core-app/shared/components/storages/storages-constants.const'; - -export default class ProjectFolderModeForm extends Controller { - static targets = [ - 'projectFolderSection', - 'projectFolderIdInput', - 'selectProjectFolderButton', - 'selectedFolderLabel', - 'storageLoginButton', - 'storage', - ]; - - static values = { - projectFolderMode: String, - placeholderFolderName: String, - notLoggedInValidation: String, - lastProjectFolders: Object, - }; - - declare readonly projectFolderSectionTarget:HTMLElement; - declare readonly selectProjectFolderButtonTarget:HTMLElement; - declare readonly selectedFolderLabelTarget:HTMLElement; - declare readonly storageLoginButtonTarget:HTMLElement; - declare readonly storageTarget:HTMLElement; - declare readonly projectFolderIdInputTarget:HTMLInputElement; - - declare readonly hasProjectFolderSectionTarget:boolean; - declare readonly hasStorageLoginButtonTarget:boolean; - declare readonly hasProjectFolderButtonTarget:boolean; - - declare projectFolderModeValue:string; - declare placeholderFolderNameValue:string; - declare notLoggedInValidationValue:string; - declare lastProjectFoldersValue:{ manual:string; automatic:string }; +import { combineLatest } from 'rxjs'; +import { filter, switchMap } from 'rxjs/operators'; +import ProjectStorageFormController from '../project-storage-form.controller'; +export default class ProjectFolderModeForm extends ProjectStorageFormController { connect():void { combineLatest([ this.fetchStorageAuthorizationState(), this.fetchProjectFolder(), ]).subscribe(([isConnected, projectFolder]) => { if (isConnected) { - this.selectedFolderLabelTarget.innerText = projectFolder === null + this.selectedFolderTextTarget.innerText = projectFolder === null ? this.placeholderFolderNameValue : projectFolder.name; } else { - this.selectedFolderLabelTarget.innerText = this.notLoggedInValidationValue; + this.selectedFolderTextTarget.innerText = this.notLoggedInValidationValue; } - this.toggleFolderDisplay(this.projectFolderModeValue); - this.setProjectFolderModeQueryParam(this.projectFolderModeValue); + this.toggleFolderDisplay(this.folderModeValue); + this.setProjectFolderModeQueryParam(this.folderModeValue); }); } @@ -114,72 +68,12 @@ export default class ProjectFolderModeForm extends Controller { filter((modal) => modal.submitted), ) .subscribe((modal) => { - this.selectedFolderLabelTarget.innerText = modal.location.name; + this.selectedFolderTextTarget.innerText = modal.location.name; this.projectFolderIdInputTarget.value = modal.location.id as string; }); } - updateForm(evt:InputEvent):void { - const mode = (evt.target as HTMLInputElement).value; - const { manual, automatic } = this.lastProjectFoldersValue; - - switch (mode) { - case 'manual': - this.projectFolderIdInputTarget.value = manual ?? ''; - - this.fetchProjectFolder().subscribe((projectFolder) => { - this.selectedFolderLabelTarget.innerText = projectFolder === null - ? this.placeholderFolderNameValue - : projectFolder.name; - }); - - break; - case 'automatic': - this.projectFolderIdInputTarget.value = automatic ?? ''; - break; - default: - this.projectFolderIdInputTarget.value = ''; - } - - this.projectFolderModeValue = mode; - this.toggleFolderDisplay(mode); - this.setProjectFolderModeQueryParam(mode); - } - - private fetchStorageAuthorizationState():Observable { - return from(fetch(this.storage._links.self.href) - .then((data) => data.json())) - .pipe( - map((storage:IStorage) => storage._links.authorizationState.href === storageConnected), - ); - } - - private fetchProjectFolder():Observable { - const href = this.projectFolderHref; - if (href === null) { - return of(null); - } - - return from(fetch(href).then((data) => data.json())) - .pipe( - map((file) => { - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - if (file._type === 'StorageFile') { - return file as IStorageFile; - } - - return null; - }), - ); - } - - private setProjectFolderModeQueryParam(mode:string) { - const url = new URL(window.location.href); - url.searchParams.set('storages_project_storage[project_folder_mode]', mode); - window.history.replaceState(window.history.state, '', url); - } - - private toggleFolderDisplay(value:string):void { + protected toggleFolderDisplay(value:string):void { // If the manual radio button is selected, show the manual folder selection section if (this.hasProjectFolderSectionTarget && value === 'manual') { this.projectFolderSectionTarget.classList.remove('d-none'); @@ -187,23 +81,4 @@ export default class ProjectFolderModeForm extends Controller { this.projectFolderSectionTarget.classList.add('d-none'); } } - - private get projectFolderHref():string|null { - const projectFolderId = this.projectFolderIdInputTarget.value; - - if (projectFolderId.length === 0) { - return null; - } - - return `${this.storage._links.self.href}/files/${projectFolderId}`; - } - - private get modalService():Observable { - return from(window.OpenProject.getPluginContext()) - .pipe(map((pluginContext) => pluginContext.services.opModalService)); - } - - private get storage():IStorage { - return JSON.parse(this.storageTarget.dataset.storage as string) as IStorage; - } } diff --git a/modules/storages/app/forms/storages/admin/project_storages/project_folder_mode_form.rb b/modules/storages/app/forms/storages/admin/project_storages/project_folder_mode_form.rb index e4f13c74bf60..2a7fb24581d4 100644 --- a/modules/storages/app/forms/storages/admin/project_storages/project_folder_mode_form.rb +++ b/modules/storages/app/forms/storages/admin/project_storages/project_folder_mode_form.rb @@ -85,7 +85,7 @@ class ProjectFolderModeForm < ApplicationForm last_project_folders: @last_project_folders, storage_login_button_options: { data: { - "storages--project-folder-mode-form-target": "storageLoginButton" + "storages--project-folder-mode-form-target": "loginButton" }, inputs: { input: storage_login_input(@project_storage.storage) @@ -98,7 +98,7 @@ class ProjectFolderModeForm < ApplicationForm }, selected_folder_label_options: { data: { - "storages--project-folder-mode-form-target": "selectedFolderLabel" + "storages--project-folder-mode-form-target": "selectedFolderText" } } }, From 2d3c65dce4254935732458e7ce2c9f928c5761a7 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 30 Jul 2024 09:58:53 +0300 Subject: [PATCH 11/22] chore[Op#55967]: rework service to accept params instead --- .../storages/project_storages_controller.rb | 3 +- .../project_storages/bulk_create_service.rb | 29 +++++----- .../bulk_create_service_spec.rb | 53 +++++++++++-------- 3 files changed, 47 insertions(+), 38 deletions(-) diff --git a/modules/storages/app/controllers/storages/admin/storages/project_storages_controller.rb b/modules/storages/app/controllers/storages/admin/storages/project_storages_controller.rb index 181495260827..1a9d778de718 100644 --- a/modules/storages/app/controllers/storages/admin/storages/project_storages_controller.rb +++ b/modules/storages/app/controllers/storages/admin/storages/project_storages_controller.rb @@ -58,9 +58,8 @@ def new def create create_service = ::Storages::ProjectStorages::BulkCreateService .new(user: current_user, projects: @projects, storage: @storage, - project_folder_mode: params.to_unsafe_h[:storages_project_storage][:project_folder_mode], include_sub_projects: include_sub_projects?) - .call + .call(params.to_unsafe_h[:storages_project_storage]) create_service.on_success { update_project_list_via_turbo_stream(url_for_action: :index) } diff --git a/modules/storages/app/services/storages/project_storages/bulk_create_service.rb b/modules/storages/app/services/storages/project_storages/bulk_create_service.rb index a5f40eb27793..123a407842fe 100644 --- a/modules/storages/app/services/storages/project_storages/bulk_create_service.rb +++ b/modules/storages/app/services/storages/project_storages/bulk_create_service.rb @@ -30,21 +30,20 @@ module Storages::ProjectStorages class BulkCreateService < ::BaseServices::BaseCallable - def initialize(user:, projects:, storage:, project_folder_mode:, include_sub_projects: false) + def initialize(user:, projects:, storage:, include_sub_projects: false) super() @user = user @projects = projects @storage = storage - @project_folder_mode = project_folder_mode @include_sub_projects = include_sub_projects end - def perform + def perform(params = {}) service_call = validate_permissions - service_call = validate_contract(service_call, incoming_activations_ids) if service_call.success? + service_call = validate_contract(service_call, incoming_activations_ids, params) if service_call.success? service_call = perform_bulk_create(service_call) if service_call.success? - service_call = create_last_project_folders(service_call) if service_call.success? - broadcast_project_storages_created if service_call.success? + service_call = create_last_project_folders(service_call, params) if service_call.success? + broadcast_project_storages_created(params) if service_call.success? service_call end @@ -61,9 +60,11 @@ def validate_permissions end end - def validate_contract(service_call, project_ids) + def validate_contract(service_call, project_ids, params) + project_folder_params = params.slice(:project_folder_mode, :project_folder_id) + set_attributes_results = project_ids.map do |id| - set_attributes(project_id: id, storage_id: @storage.id, project_folder_mode: @project_folder_mode) + set_attributes(project_id: id, storage_id: @storage.id, **project_folder_params) end if (failures = set_attributes_results.select(&:failure?)).any? @@ -78,9 +79,7 @@ def validate_contract(service_call, project_ids) def perform_bulk_create(service_call) bulk_insertion = ::Storages::ProjectStorage.insert_all( - service_call.result.map do |model| - model.attributes.slice("project_id", "storage_id", "project_folder_mode", "creator_id") - end, + service_call.result.map { |model| model.attributes.compact }, unique_by: %i[project_id storage_id], returning: %w[id] ) @@ -89,8 +88,8 @@ def perform_bulk_create(service_call) service_call end - def create_last_project_folders(service_call) - return service_call if @project_folder_mode.to_sym == :inactive + def create_last_project_folders(service_call, params) + return service_call if params[:project_folder_mode].to_sym == :inactive last_project_folders = ::Storages::LastProjectFolders::BulkCreateService .new(user: @user, project_storages: service_call.result) @@ -100,10 +99,10 @@ def create_last_project_folders(service_call) service_call end - def broadcast_project_storages_created + def broadcast_project_storages_created(params) OpenProject::Notifications.send( OpenProject::Events::PROJECT_STORAGE_CREATED, - project_folder_mode: @project_folder_mode, + project_folder_mode: params[:project_folder_mode], storage: @storage ) end diff --git a/modules/storages/spec/services/storages/project_storages/bulk_create_service_spec.rb b/modules/storages/spec/services/storages/project_storages/bulk_create_service_spec.rb index 51d92ebee20d..09053e405fbb 100644 --- a/modules/storages/spec/services/storages/project_storages/bulk_create_service_spec.rb +++ b/modules/storages/spec/services/storages/project_storages/bulk_create_service_spec.rb @@ -40,10 +40,10 @@ context "with a single project" do let(:project) { create(:project) } let(:project_folder_mode) { "inactive" } - let(:instance) { described_class.new(user:, projects: [project], storage:, project_folder_mode:) } + let(:instance) { described_class.new(user:, projects: [project], storage:) } it "activates the storage for the given project", :aggregate_failures do - expect { instance.call } + expect { instance.call(project_folder_mode:) } .to change(Storages::ProjectStorage, :count).by(1) project_storage = Storages::ProjectStorage.last @@ -68,9 +68,9 @@ it "activates the storage for the project and sub-projects", :aggregate_failures do create_service = described_class.new(user:, projects: projects.map(&:reload), storage:, - project_folder_mode:, include_sub_projects: true) + include_sub_projects: true) - expect { create_service.call } + expect { create_service.call(project_folder_mode:) } .to change(Storages::ProjectStorage, :count).by(4) .and change(Storages::LastProjectFolder, :count).by(4) @@ -90,9 +90,9 @@ it "activates the storage for the project and sub-projects" do create_service = described_class.new(user:, projects: [project.reload, subproject], storage:, - project_folder_mode:, include_sub_projects: true) + include_sub_projects: true) - expect { create_service.call } + expect { create_service.call(project_folder_mode:) } .to change(Storages::ProjectStorage, :count).by(2) .and change(Storages::LastProjectFolder, :count).by(2) @@ -110,9 +110,9 @@ let(:project) { create(:project) } it "activates the storage only once" do - create_service = described_class.new(user:, projects: [project, project], storage:, project_folder_mode:) + create_service = described_class.new(user:, projects: [project, project], storage:) - expect { create_service.call } + expect { create_service.call(project_folder_mode:) } .to change(Storages::ProjectStorage, :count).by(1) .and change(Storages::LastProjectFolder, :count).by(1) @@ -142,9 +142,9 @@ let(:project_folder_mode) { "inactive" } it "activates the storage" do - create_service = described_class.new(user:, projects: [project], storage:, project_folder_mode:) + create_service = described_class.new(user:, projects: [project], storage:) - expect { create_service.call } + expect { create_service.call(project_folder_mode:) } .to change(Storages::ProjectStorage, :count).by(1) project_storage = Storages::ProjectStorage.last @@ -168,10 +168,10 @@ end let(:project) { create(:project) } let(:project_folder_mode) { "inactive" } - let(:instance) { described_class.new(user:, projects: [project], storage:, project_folder_mode:) } + let(:instance) { described_class.new(user:, projects: [project], storage:) } it "does not create any project storages" do - expect { instance.call }.not_to change(Storages::ProjectStorage, :count) + expect { instance.call(project_folder_mode:) }.not_to change(Storages::ProjectStorage, :count) expect(instance.call).to be_failure expect(OpenProject::Notifications).not_to have_received(:send) .with(OpenProject::Events::PROJECT_STORAGE_CREATED, project_folder_mode:, storage:) @@ -179,10 +179,10 @@ end context "with empty projects" do - let(:instance) { described_class.new(user:, projects: [], storage:, project_folder_mode:) } + let(:instance) { described_class.new(user:, projects: [], storage:) } it "does not create any project storages" do - service_result = instance.call + service_result = instance.call(project_folder_mode:) expect(service_result).to be_failure expect(service_result.errors).to eq("not found") expect(OpenProject::Notifications).not_to have_received(:send) @@ -194,10 +194,9 @@ let(:active_project) { create(:project) } it "only creates the project storage for the active project", :aggregate_failures do - create_service = described_class.new(user:, projects: [archived_project, active_project], storage:, - project_folder_mode:) + create_service = described_class.new(user:, projects: [archived_project, active_project], storage:) - expect { create_service.call } + expect { create_service.call(project_folder_mode:) } .to change(Storages::ProjectStorage, :count).by(1) .and change(Storages::LastProjectFolder, :count).by(1) @@ -214,13 +213,25 @@ context "with broken contract" do let(:storage) { create(:nextcloud_storage, :as_not_automatically_managed) } let(:project) { create(:project) } - let(:project_folder_mode) { "automatic" } it "does not create any records" do - create_service = described_class.new(user:, projects: [project], storage:, project_folder_mode:) + create_service = described_class.new(user:, projects: [project], storage:) + result = nil + + aggregate_failures "automatic mode cannot be used with non-automatically managed storage" do + expect { result = create_service.call(project_folder_mode: "automatic") } + .not_to change(Storages::ProjectStorage, :count) + expect(result).to be_failure + expect(result.errors.map(&:full_messages).flatten) + .to eq(["Project folder mode is not available for this storage."]) + end - expect { create_service.call }.not_to change(Storages::ProjectStorage, :count) - expect(create_service.call).to be_failure + aggregate_failures "manual mode requires a project folder id" do + expect { result = create_service.call(project_folder_mode: "manual") } + .not_to change(Storages::ProjectStorage, :count) + expect(result).to be_failure + expect(result.errors.map(&:full_messages).flatten).to eq(["Project folder can't be blank."]) + end end end end From c5fe7912bdd2b65fd5b5bb5d304926a0c99811e7 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 30 Jul 2024 11:14:48 +0300 Subject: [PATCH 12/22] chore[Op#55967]: extract display fn to reduce duplication --- .../project-storage-form.controller.ts | 31 ++++++++++--------- .../project-folder-mode-form.controller.ts | 30 +++++++----------- 2 files changed, 28 insertions(+), 33 deletions(-) diff --git a/frontend/src/stimulus/controllers/dynamic/project-storage-form.controller.ts b/frontend/src/stimulus/controllers/dynamic/project-storage-form.controller.ts index e1a332f45c0f..b78be7209a33 100644 --- a/frontend/src/stimulus/controllers/dynamic/project-storage-form.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/project-storage-form.controller.ts @@ -41,13 +41,13 @@ import { switchMap, } from 'rxjs/operators'; +import { IStorageFile } from 'core-app/core/state/storage-files/storage-file.model'; import { IStorage } from 'core-app/core/state/storages/storage.model'; import { OpModalService } from 'core-app/shared/components/modal/modal.service'; -import { IStorageFile } from 'core-app/core/state/storage-files/storage-file.model'; -import { storageConnected } from 'core-app/shared/components/storages/storages-constants.const'; import { LocationPickerModalComponent, } from 'core-app/shared/components/storages/location-picker-modal/location-picker-modal.component'; +import { storageConnected } from 'core-app/shared/components/storages/storages-constants.const'; export default class ProjectStorageFormController extends Controller { static targets = [ @@ -88,18 +88,7 @@ export default class ProjectStorageFormController extends Controller { this.fetchStorageAuthorizationState(), this.fetchProjectFolder(), ]).subscribe(([isConnected, projectFolder]) => { - if (isConnected) { - this.selectProjectFolderButtonTarget.style.display = 'inline-block'; - this.loginButtonTarget.style.display = 'none'; - this.selectedFolderTextTarget.innerText = projectFolder === null - ? this.placeholderFolderNameValue - : projectFolder.name; - } else { - this.selectProjectFolderButtonTarget.style.display = 'none'; - this.loginButtonTarget.style.display = 'inline-block'; - this.selectedFolderTextTarget.innerText = this.notLoggedInValidationValue; - } - + this.displayFolderSelectionOrLoginButton(isConnected, projectFolder); this.toggleFolderDisplay(this.folderModeValue); this.setProjectFolderModeQueryParam(this.folderModeValue); }); @@ -199,6 +188,20 @@ export default class ProjectStorageFormController extends Controller { ); } + protected displayFolderSelectionOrLoginButton(isConnected:boolean, projectFolder:IStorageFile|null):void { + if (isConnected) { + this.selectProjectFolderButtonTarget.style.display = 'inline-block'; + this.loginButtonTarget.style.display = 'none'; + this.selectedFolderTextTarget.innerText = projectFolder === null + ? this.placeholderFolderNameValue + : projectFolder.name; + } else { + this.selectProjectFolderButtonTarget.style.display = 'none'; + this.loginButtonTarget.style.display = 'inline-block'; + this.selectedFolderTextTarget.innerText = this.notLoggedInValidationValue; + } + } + protected setProjectFolderModeQueryParam(mode:string) { const url = new URL(window.location.href); url.searchParams.set('storages_project_storage[project_folder_mode]', mode); diff --git a/frontend/src/stimulus/controllers/dynamic/storages/project-folder-mode-form.controller.ts b/frontend/src/stimulus/controllers/dynamic/storages/project-folder-mode-form.controller.ts index d26e9e7b5cec..9c6d09ffbe9d 100644 --- a/frontend/src/stimulus/controllers/dynamic/storages/project-folder-mode-form.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/storages/project-folder-mode-form.controller.ts @@ -28,33 +28,15 @@ * ++ */ +import { IStorageFile } from 'core-app/core/state/storage-files/storage-file.model'; import { PortalOutletTarget } from 'core-app/shared/components/modal/portal-outlet-target.enum'; import { LocationPickerModalComponent, } from 'core-app/shared/components/storages/location-picker-modal/location-picker-modal.component'; -import { combineLatest } from 'rxjs'; import { filter, switchMap } from 'rxjs/operators'; import ProjectStorageFormController from '../project-storage-form.controller'; export default class ProjectFolderModeForm extends ProjectStorageFormController { - connect():void { - combineLatest([ - this.fetchStorageAuthorizationState(), - this.fetchProjectFolder(), - ]).subscribe(([isConnected, projectFolder]) => { - if (isConnected) { - this.selectedFolderTextTarget.innerText = projectFolder === null - ? this.placeholderFolderNameValue - : projectFolder.name; - } else { - this.selectedFolderTextTarget.innerText = this.notLoggedInValidationValue; - } - - this.toggleFolderDisplay(this.folderModeValue); - this.setProjectFolderModeQueryParam(this.folderModeValue); - }); - } - selectProjectFolder(_evt:Event):void { const locals = { projectFolderHref: this.projectFolderHref, @@ -73,6 +55,16 @@ export default class ProjectFolderModeForm extends ProjectStorageFormController }); } + protected displayFolderSelectionOrLoginButton(isConnected:boolean, projectFolder:IStorageFile|null):void { + if (isConnected) { + this.selectedFolderTextTarget.innerText = projectFolder === null + ? this.placeholderFolderNameValue + : projectFolder.name; + } else { + this.selectedFolderTextTarget.innerText = this.notLoggedInValidationValue; + } + } + protected toggleFolderDisplay(value:string):void { // If the manual radio button is selected, show the manual folder selection section if (this.hasProjectFolderSectionTarget && value === 'manual') { From c0be3696ecdb2edb0f9e5ed78a5b059ede52fed3 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 30 Jul 2024 12:23:45 +0300 Subject: [PATCH 13/22] chore[Op#55967]: further extract outlettarget to reduce duplication --- .../project-storage-form.controller.ts | 7 +++++- .../project-folder-mode-form.controller.ts | 24 +++---------------- 2 files changed, 9 insertions(+), 22 deletions(-) diff --git a/frontend/src/stimulus/controllers/dynamic/project-storage-form.controller.ts b/frontend/src/stimulus/controllers/dynamic/project-storage-form.controller.ts index b78be7209a33..a1dc85e6a661 100644 --- a/frontend/src/stimulus/controllers/dynamic/project-storage-form.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/project-storage-form.controller.ts @@ -44,6 +44,7 @@ import { import { IStorageFile } from 'core-app/core/state/storage-files/storage-file.model'; import { IStorage } from 'core-app/core/state/storages/storage.model'; import { OpModalService } from 'core-app/shared/components/modal/modal.service'; +import { PortalOutletTarget } from 'core-app/shared/components/modal/portal-outlet-target.enum'; import { LocationPickerModalComponent, } from 'core-app/shared/components/storages/location-picker-modal/location-picker-modal.component'; @@ -102,7 +103,7 @@ export default class ProjectStorageFormController extends Controller { this.modalService .pipe( - switchMap((service) => service.show(LocationPickerModalComponent, 'global', locals)), + switchMap((service) => service.show(LocationPickerModalComponent, 'global', locals, false, false, this.OutletTarget)), switchMap((modal) => modal.closingEvent), filter((modal) => modal.submitted), ) @@ -142,6 +143,10 @@ export default class ProjectStorageFormController extends Controller { this.setProjectFolderModeQueryParam(mode); } + protected get OutletTarget():PortalOutletTarget { + return PortalOutletTarget.Default; + } + protected get modalService():Observable { return from(window.OpenProject.getPluginContext()) .pipe(map((pluginContext) => pluginContext.services.opModalService)); diff --git a/frontend/src/stimulus/controllers/dynamic/storages/project-folder-mode-form.controller.ts b/frontend/src/stimulus/controllers/dynamic/storages/project-folder-mode-form.controller.ts index 9c6d09ffbe9d..e6f1bd755219 100644 --- a/frontend/src/stimulus/controllers/dynamic/storages/project-folder-mode-form.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/storages/project-folder-mode-form.controller.ts @@ -30,29 +30,11 @@ import { IStorageFile } from 'core-app/core/state/storage-files/storage-file.model'; import { PortalOutletTarget } from 'core-app/shared/components/modal/portal-outlet-target.enum'; -import { - LocationPickerModalComponent, -} from 'core-app/shared/components/storages/location-picker-modal/location-picker-modal.component'; -import { filter, switchMap } from 'rxjs/operators'; import ProjectStorageFormController from '../project-storage-form.controller'; -export default class ProjectFolderModeForm extends ProjectStorageFormController { - selectProjectFolder(_evt:Event):void { - const locals = { - projectFolderHref: this.projectFolderHref, - storage: this.storage, - }; - - this.modalService - .pipe( - switchMap((service) => service.show(LocationPickerModalComponent, 'global', locals, false, false, PortalOutletTarget.Custom)), - switchMap((modal) => modal.closingEvent), - filter((modal) => modal.submitted), - ) - .subscribe((modal) => { - this.selectedFolderTextTarget.innerText = modal.location.name; - this.projectFolderIdInputTarget.value = modal.location.id as string; - }); +export default class ProjectFolderModeFormController extends ProjectStorageFormController { + protected get OutletTarget():PortalOutletTarget { + return PortalOutletTarget.Custom; } protected displayFolderSelectionOrLoginButton(isConnected:boolean, projectFolder:IStorageFile|null):void { From e1951b0daea7a1428dd9c834576cb4ca34686898 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 31 Jul 2024 13:59:47 +0300 Subject: [PATCH 14/22] tests[Op#55967]: add feature tests for manual file selection --- .../storages/admin/project_storages_spec.rb | 106 ++++++++++++++---- .../support/components/file_picker_dialog.rb | 4 +- 2 files changed, 89 insertions(+), 21 deletions(-) diff --git a/modules/storages/spec/features/storages/admin/project_storages_spec.rb b/modules/storages/spec/features/storages/admin/project_storages_spec.rb index e2b69d372d74..b0fa9beb93d9 100644 --- a/modules/storages/spec/features/storages/admin/project_storages_spec.rb +++ b/modules/storages/spec/features/storages/admin/project_storages_spec.rb @@ -33,33 +33,19 @@ RSpec.describe "Admin lists project mappings for a storage", :js, + :webmock, :with_cuprite, with_flag: { enable_storage_for_multiple_projects: true } do shared_let(:admin) { create(:admin, preferences: { time_zone: "Etc/UTC" }) } shared_let(:non_admin) { create(:user) } shared_let(:project) { create(:project, name: "My active Project") } - shared_let(:archived_project) do - create(:project, - active: false, - name: "My archived Project") - end - shared_let(:storage) do - create(:nextcloud_storage, - :as_automatically_managed, - name: "My Nextcloud Storage") - end - shared_let(:project_storage) do - create(:project_storage, - project:, - storage:, - project_folder_mode: "automatic") - end + shared_let(:archived_project) { create(:project, active: false, name: "My archived Project") } + shared_let(:storage) { create(:nextcloud_storage, :as_automatically_managed, name: "My Nextcloud Storage") } + shared_let(:project_storage) { create(:project_storage, project:, storage:, project_folder_mode: "automatic") } + shared_let(:archived_project_project_storage) do - create(:project_storage, - project: archived_project, - storage:, - project_folder_mode: "inactive") + create(:project_storage, project: archived_project, storage:, project_folder_mode: "inactive") end let(:project_storages_index_page) { Pages::Admin::Storages::ProjectStorages::Index.new } @@ -190,6 +176,86 @@ end end + describe "Linking a project to a storage with a manually managed folder" do + let(:oauth_application) { create(:oauth_application) } + let(:storage) { create(:nextcloud_storage, :as_automatically_managed, oauth_application:) } + let(:oauth_client) { create(:oauth_client, integration: storage) } + let(:oauth_client_token) { create(:oauth_client_token, oauth_client:, user: admin) } + let(:location_picker) { Components::FilePickerDialog.new } + + let(:root_xml_response) { build(:webdav_data) } + let(:folder1_xml_response) { build(:webdav_data_folder) } + let(:folder1_fileinfo_response) do + { + ocs: { + data: { + status: "OK", + statuscode: 200, + id: 11, + name: "Folder1", + path: "files/Folder1", + mtime: 1682509719, + ctime: 0 + } + } + } + end + + before do + oauth_client_token + + stub_request(:propfind, "#{storage.host}/remote.php/dav/files/#{oauth_client_token.origin_user_id}") + .to_return(status: 207, body: root_xml_response, headers: {}) + stub_request(:propfind, "#{storage.host}/remote.php/dav/files/#{oauth_client_token.origin_user_id}/Folder1") + .to_return(status: 207, body: folder1_xml_response, headers: {}) + stub_request(:get, "#{storage.host}/ocs/v1.php/apps/integration_openproject/fileinfo/11") + .to_return(status: 200, body: folder1_fileinfo_response.to_json, headers: {}) + stub_request(:get, "#{storage.host}/ocs/v1.php/cloud/user").to_return(status: 200, body: "{}") + stub_request( + :delete, + "#{storage.host}/remote.php/dav/files/OpenProject/OpenProject/Project%20name%20without%20sequence%20(#{project.id})" + ).to_return(status: 200, body: "", headers: {}) + end + + it "allows linking a project to a storage" do + project = create(:project) + subproject = create(:project, parent: project) + click_on "Add projects" + + within("dialog") do + autocompleter = page.find(".op-project-autocompleter") + autocompleter.fill_in with: project.name + + expect(page).to have_no_text(archived_project.name) + + find(".ng-option-label", text: project.name).click + check "Include sub-projects" + + expect(page.find_by_id("storages_project_storage_project_folder_mode_automatic")).to be_checked + + choose "Existing folder with manually managed permissions" + expect(page).to have_text("No selected folder") + click_on "Select folder" + + location_picker.expect_open + using_wait_time(20) do + location_picker.wait_for_folder_loaded + location_picker.enter_folder("Folder1") + location_picker.wait_for_folder_loaded + end + location_picker.confirm + + # Add storage + expect(page).to have_text("Folder1") + + click_on "Add" + end + + expect(page).to have_text(project.name) + expect(page).to have_text(subproject.name) + end + end + describe "Removal of a project from a storage" do it "shows a warning dialog that can be aborted" do expect(page).to have_text(project.name) diff --git a/modules/storages/spec/support/components/file_picker_dialog.rb b/modules/storages/spec/support/components/file_picker_dialog.rb index f256c10b9c04..2b3504e8e3f3 100644 --- a/modules/storages/spec/support/components/file_picker_dialog.rb +++ b/modules/storages/spec/support/components/file_picker_dialog.rb @@ -55,7 +55,9 @@ def confirm_button_state(selection_count:) def wait_for_folder_loaded page.within(container) do - expect(page).to have_no_css('[data-test-selector="op-file-list--loading-indicator"]') + RSpec::Wait.with_wait(timeout: 10, delay: 1) do + RSpec::Wait.wait_for(page).to have_no_css('[data-test-selector="op-file-list--loading-indicator"]') + end end end From f38f91038cd28a4d83f3c4827cafbfede8c10a1a Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 6 Aug 2024 12:15:46 +0300 Subject: [PATCH 15/22] fix[Op#55967]: Remove redundant errors from bulk service failure result --- .../storages/project_storages/bulk_create_service.rb | 3 +-- modules/storages/config/locales/en.yml | 2 ++ .../storages/project_storages/bulk_create_service_spec.rb | 6 +++--- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/modules/storages/app/services/storages/project_storages/bulk_create_service.rb b/modules/storages/app/services/storages/project_storages/bulk_create_service.rb index 123a407842fe..3b392009c488 100644 --- a/modules/storages/app/services/storages/project_storages/bulk_create_service.rb +++ b/modules/storages/app/services/storages/project_storages/bulk_create_service.rb @@ -68,8 +68,7 @@ def validate_contract(service_call, project_ids, params) end if (failures = set_attributes_results.select(&:failure?)).any? - service_call.success = false - service_call.errors = failures.map(&:errors) + service_call = failures.first else service_call.result = set_attributes_results.map(&:result) end diff --git a/modules/storages/config/locales/en.yml b/modules/storages/config/locales/en.yml index 6a9ad4c66259..5ece01da04ab 100644 --- a/modules/storages/config/locales/en.yml +++ b/modules/storages/config/locales/en.yml @@ -26,6 +26,8 @@ en: mode_unavailable: is not available for this storage. project_ids: blank: Please select a project. + project_folder_id: + blank: Please select a folder. storages/storage: attributes: host: diff --git a/modules/storages/spec/services/storages/project_storages/bulk_create_service_spec.rb b/modules/storages/spec/services/storages/project_storages/bulk_create_service_spec.rb index 09053e405fbb..e2f141b5c861 100644 --- a/modules/storages/spec/services/storages/project_storages/bulk_create_service_spec.rb +++ b/modules/storages/spec/services/storages/project_storages/bulk_create_service_spec.rb @@ -222,15 +222,15 @@ expect { result = create_service.call(project_folder_mode: "automatic") } .not_to change(Storages::ProjectStorage, :count) expect(result).to be_failure - expect(result.errors.map(&:full_messages).flatten) - .to eq(["Project folder mode is not available for this storage."]) + expect(result.errors.full_messages.to_sentence) + .to eq("Project folder mode is not available for this storage.") end aggregate_failures "manual mode requires a project folder id" do expect { result = create_service.call(project_folder_mode: "manual") } .not_to change(Storages::ProjectStorage, :count) expect(result).to be_failure - expect(result.errors.map(&:full_messages).flatten).to eq(["Project folder can't be blank."]) + expect(result.errors.messages).to eq({ project_folder_id: ["Please select a folder."] }) end end end From df7956252809897bd3fb12fa45001d709b8776c6 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 6 Aug 2024 13:54:11 +0300 Subject: [PATCH 16/22] tests[Op#55967]: group manual linking tests together --- .../storages/admin/project_storages_spec.rb | 144 +++++++++++------- 1 file changed, 86 insertions(+), 58 deletions(-) diff --git a/modules/storages/spec/features/storages/admin/project_storages_spec.rb b/modules/storages/spec/features/storages/admin/project_storages_spec.rb index b0fa9beb93d9..98ec3990964e 100644 --- a/modules/storages/spec/features/storages/admin/project_storages_spec.rb +++ b/modules/storages/spec/features/storages/admin/project_storages_spec.rb @@ -180,79 +180,107 @@ let(:oauth_application) { create(:oauth_application) } let(:storage) { create(:nextcloud_storage, :as_automatically_managed, oauth_application:) } let(:oauth_client) { create(:oauth_client, integration: storage) } - let(:oauth_client_token) { create(:oauth_client_token, oauth_client:, user: admin) } - let(:location_picker) { Components::FilePickerDialog.new } - - let(:root_xml_response) { build(:webdav_data) } - let(:folder1_xml_response) { build(:webdav_data_folder) } - let(:folder1_fileinfo_response) do - { - ocs: { - data: { - status: "OK", - statuscode: 200, - id: 11, - name: "Folder1", - path: "files/Folder1", - mtime: 1682509719, - ctime: 0 + + context "when the user has granted OAuth access" do + let(:oauth_client_token) { create(:oauth_client_token, oauth_client:, user: admin) } + let(:location_picker) { Components::FilePickerDialog.new } + + let(:root_xml_response) { build(:webdav_data) } + let(:folder1_xml_response) { build(:webdav_data_folder) } + let(:folder1_fileinfo_response) do + { + ocs: { + data: { + status: "OK", + statuscode: 200, + id: 11, + name: "Folder1", + path: "files/Folder1", + mtime: 1682509719, + ctime: 0 + } } } - } - end + end - before do - oauth_client_token - - stub_request(:propfind, "#{storage.host}/remote.php/dav/files/#{oauth_client_token.origin_user_id}") - .to_return(status: 207, body: root_xml_response, headers: {}) - stub_request(:propfind, "#{storage.host}/remote.php/dav/files/#{oauth_client_token.origin_user_id}/Folder1") - .to_return(status: 207, body: folder1_xml_response, headers: {}) - stub_request(:get, "#{storage.host}/ocs/v1.php/apps/integration_openproject/fileinfo/11") - .to_return(status: 200, body: folder1_fileinfo_response.to_json, headers: {}) - stub_request(:get, "#{storage.host}/ocs/v1.php/cloud/user").to_return(status: 200, body: "{}") - stub_request( - :delete, - "#{storage.host}/remote.php/dav/files/OpenProject/OpenProject/Project%20name%20without%20sequence%20(#{project.id})" - ).to_return(status: 200, body: "", headers: {}) - end + before do + oauth_client_token + + stub_request(:propfind, "#{storage.host}/remote.php/dav/files/#{oauth_client_token.origin_user_id}") + .to_return(status: 207, body: root_xml_response, headers: {}) + stub_request(:propfind, "#{storage.host}/remote.php/dav/files/#{oauth_client_token.origin_user_id}/Folder1") + .to_return(status: 207, body: folder1_xml_response, headers: {}) + stub_request(:get, "#{storage.host}/ocs/v1.php/apps/integration_openproject/fileinfo/11") + .to_return(status: 200, body: folder1_fileinfo_response.to_json, headers: {}) + stub_request(:get, "#{storage.host}/ocs/v1.php/cloud/user").to_return(status: 200, body: "{}") + stub_request( + :delete, + "#{storage.host}/remote.php/dav/files/OpenProject/OpenProject/Project%20name%20without%20sequence%20(#{project.id})" + ).to_return(status: 200, body: "", headers: {}) + end + + it "allows linking a project to a storage" do + project = create(:project) + subproject = create(:project, parent: project) + click_on "Add projects" - it "allows linking a project to a storage" do - project = create(:project) - subproject = create(:project, parent: project) - click_on "Add projects" + within("dialog") do + autocompleter = page.find(".op-project-autocompleter") + autocompleter.fill_in with: project.name - within("dialog") do - autocompleter = page.find(".op-project-autocompleter") - autocompleter.fill_in with: project.name + expect(page).to have_no_text(archived_project.name) - expect(page).to have_no_text(archived_project.name) + find(".ng-option-label", text: project.name).click + check "Include sub-projects" - find(".ng-option-label", text: project.name).click - check "Include sub-projects" + expect(page.find_by_id("storages_project_storage_project_folder_mode_automatic")).to be_checked - expect(page.find_by_id("storages_project_storage_project_folder_mode_automatic")).to be_checked + choose "Existing folder with manually managed permissions" + expect(page).to have_text("No selected folder") + # TODO: add case where user submits without selecting a folder + click_on "Select folder" - choose "Existing folder with manually managed permissions" - expect(page).to have_text("No selected folder") - click_on "Select folder" + location_picker.expect_open + using_wait_time(20) do + location_picker.wait_for_folder_loaded + location_picker.enter_folder("Folder1") + location_picker.wait_for_folder_loaded + end + location_picker.confirm - location_picker.expect_open - using_wait_time(20) do - location_picker.wait_for_folder_loaded - location_picker.enter_folder("Folder1") - location_picker.wait_for_folder_loaded + # Add storage + expect(page).to have_text("Folder1") + + click_on "Add" end - location_picker.confirm - # Add storage - expect(page).to have_text("Folder1") + expect(page).to have_text(project.name) + expect(page).to have_text(subproject.name) + end + end - click_on "Add" + context "when the user has not granted oauth access" do + let(:nonce) { "57a17c3f-b2ed-446e-9dd8-651ba3aec37d" } + let(:redirect_uri) do + "#{CGI.escape(OpenProject::Application.root_url)}/oauth_clients/#{storage.oauth_client.client_id}/callback" end - expect(page).to have_text(project.name) - expect(page).to have_text(subproject.name) + before do + allow(SecureRandom).to receive(:uuid).and_call_original.ordered + allow(SecureRandom).to receive(:uuid).and_return(nonce).ordered + end + + it "show a storage login button" do + click_on "Add projects" + + within("dialog") do + expect(page).to have_button("Nextcloud login") + click_on("Nextcloud login") + wait_for(page).to have_current_path("/index.php/apps/oauth2/authorize" \ + "?client_id=#{storage.oauth_client.client_id}&" \ + "redirect_uri=#{redirect_uri}&response_type=code&state=#{nonce}") + end + end end end From 207d811fbe4abbcf34eb9acc8d8b40ac2e908b83 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 6 Aug 2024 13:58:41 +0300 Subject: [PATCH 17/22] feat[Op#55967]: render validation message when no folder is selected --- ...add_projects_form_modal_component.html.erb | 2 +- .../storages/project_storages_controller.rb | 8 +++--- .../project_folder_mode_form.rb | 20 +++++++++++++-- .../storages/admin/project_storages_spec.rb | 25 ++++++++++++++++++- 4 files changed, 47 insertions(+), 8 deletions(-) diff --git a/modules/storages/app/components/storages/admin/storages/add_projects_form_modal_component.html.erb b/modules/storages/app/components/storages/admin/storages/add_projects_form_modal_component.html.erb index 7acc10220a33..a44507d96818 100644 --- a/modules/storages/app/components/storages/admin/storages/add_projects_form_modal_component.html.erb +++ b/modules/storages/app/components/storages/admin/storages/add_projects_form_modal_component.html.erb @@ -66,7 +66,7 @@ See COPYRIGHT and LICENSE files for more details. data: { "application-target": "dynamic", controller: "storages--project-folder-mode-form", - 'storages--project-folder-mode-form-project-folder-mode-value': @project_storage.project_folder_mode, + 'storages--project-folder-mode-form-folder-mode-value': @project_storage.project_folder_mode, 'storages--project-folder-mode-form-placeholder-folder-name-value': t(:"storages.label_no_selected_folder"), 'storages--project-folder-mode-form-not-logged-in-validation-value': t(:"storages.instructions.not_logged_into_storage"), 'storages--project-folder-mode-form-last-project-folders-value': @last_project_folders diff --git a/modules/storages/app/controllers/storages/admin/storages/project_storages_controller.rb b/modules/storages/app/controllers/storages/admin/storages/project_storages_controller.rb index 1a9d778de718..9e80adaae129 100644 --- a/modules/storages/app/controllers/storages/admin/storages/project_storages_controller.rb +++ b/modules/storages/app/controllers/storages/admin/storages/project_storages_controller.rb @@ -64,10 +64,10 @@ def create create_service.on_success { update_project_list_via_turbo_stream(url_for_action: :index) } create_service.on_failure do - update_flash_message_via_turbo_stream( - message: join_flash_messages(create_service.errors), - full: true, dismiss_scheme: :hide, scheme: :danger - ) + project_storage = create_service.result + project_storage.errors.merge!(create_service.errors) + component = Storages::Admin::Storages::AddProjectsFormModalComponent.new(project_storage:) + update_via_turbo_stream(component:, status: :bad_request) end respond_with_turbo_streams(status: create_service.success? ? :ok : :unprocessable_entity) diff --git a/modules/storages/app/forms/storages/admin/project_storages/project_folder_mode_form.rb b/modules/storages/app/forms/storages/admin/project_storages/project_folder_mode_form.rb index 2a7fb24581d4..77fd0031a898 100644 --- a/modules/storages/app/forms/storages/admin/project_storages/project_folder_mode_form.rb +++ b/modules/storages/app/forms/storages/admin/project_storages/project_folder_mode_form.rb @@ -34,7 +34,7 @@ class ProjectFolderModeForm < ApplicationForm include APIV3Helper form do |radio_form| - radio_form.radio_button_group(name: :project_folder_mode) do |radio_group| + radio_form.radio_button_group(name: :project_folder_mode, validation_message:) do |radio_group| if @project_storage.project_folder_mode_possible?("inactive") radio_group.radio_button(value: "inactive", label: I18n.t(:"storages.label_no_specific_folder"), caption: I18n.t(:"storages.instructions.no_specific_folder"), @@ -106,7 +106,7 @@ class ProjectFolderModeForm < ApplicationForm data: { "storages--project-folder-mode-form-target": "projectFolderSection" }, - classes: ["d-none"] + classes: project_folder_selection_classes } ) end @@ -117,6 +117,22 @@ def initialize(project_storage:, last_project_folders: {}) @project_storage = project_storage @last_project_folders = last_project_folders end + + private + + def validation_message + @project_storage + .errors + .messages_for(:project_folder_id) + .to_sentence + .presence + end + + def project_folder_selection_classes + [].tap do |classes| + classes << "d-none" unless @project_storage.errors.include?(:project_folder_id) + end + end end end end diff --git a/modules/storages/spec/features/storages/admin/project_storages_spec.rb b/modules/storages/spec/features/storages/admin/project_storages_spec.rb index 98ec3990964e..a8ff896b44c1 100644 --- a/modules/storages/spec/features/storages/admin/project_storages_spec.rb +++ b/modules/storages/spec/features/storages/admin/project_storages_spec.rb @@ -237,7 +237,6 @@ choose "Existing folder with manually managed permissions" expect(page).to have_text("No selected folder") - # TODO: add case where user submits without selecting a folder click_on "Select folder" location_picker.expect_open @@ -257,6 +256,30 @@ expect(page).to have_text(project.name) expect(page).to have_text(subproject.name) end + + context "when the user does not select a folder" do + it "shows an error message" do + project = create(:project) + click_on "Add projects" + + within("dialog") do + autocompleter = page.find(".op-project-autocompleter") + autocompleter.fill_in with: project.name + + find(".ng-option-label", text: project.name).click + check "Include sub-projects" + + choose "Existing folder with manually managed permissions" + expect(page).to have_text("No selected folder") + + click_on "Add" + + expect(page).to have_text("Please select a folder.") + expect(page.find_by_id("storages_project_storage_project_folder_mode_manual")).to be_checked + expect(page).to have_text("No selected folder") + end + end + end end context "when the user has not granted oauth access" do From b034ea4d7008b97fbe1466715587465d36fff1cd Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 6 Aug 2024 15:48:47 +0300 Subject: [PATCH 18/22] tests[Op#55967]: choose manual selection before assertions --- .../spec/features/storages/admin/project_storages_spec.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/modules/storages/spec/features/storages/admin/project_storages_spec.rb b/modules/storages/spec/features/storages/admin/project_storages_spec.rb index a8ff896b44c1..e7ef37415c74 100644 --- a/modules/storages/spec/features/storages/admin/project_storages_spec.rb +++ b/modules/storages/spec/features/storages/admin/project_storages_spec.rb @@ -31,11 +31,14 @@ require "spec_helper" require_module_spec_helper +# We decrease the notification polling interval because some portions of the JS code rely on something triggering +# the Angular change detection. This is usually done by the notification polling, but we don't want to wait RSpec.describe "Admin lists project mappings for a storage", :js, :webmock, :with_cuprite, - with_flag: { enable_storage_for_multiple_projects: true } do + with_flag: { enable_storage_for_multiple_projects: true }, + with_settings: { notifications_polling_interval: 1_000 } do shared_let(:admin) { create(:admin, preferences: { time_zone: "Etc/UTC" }) } shared_let(:non_admin) { create(:user) } @@ -228,8 +231,6 @@ autocompleter = page.find(".op-project-autocompleter") autocompleter.fill_in with: project.name - expect(page).to have_no_text(archived_project.name) - find(".ng-option-label", text: project.name).click check "Include sub-projects" @@ -297,6 +298,7 @@ click_on "Add projects" within("dialog") do + choose "Existing folder with manually managed permissions" expect(page).to have_button("Nextcloud login") click_on("Nextcloud login") wait_for(page).to have_current_path("/index.php/apps/oauth2/authorize" \ From e31436f7d5433240e18a9b1524fcfbf051e886e4 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 6 Aug 2024 16:24:50 +0300 Subject: [PATCH 19/22] chore[Op#55967]: normalize i18n & rubocop --- .../storages/admin/storages/project_storages_controller.rb | 2 +- modules/storages/config/locales/en.yml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/storages/app/controllers/storages/admin/storages/project_storages_controller.rb b/modules/storages/app/controllers/storages/admin/storages/project_storages_controller.rb index 9e80adaae129..8b667704ab44 100644 --- a/modules/storages/app/controllers/storages/admin/storages/project_storages_controller.rb +++ b/modules/storages/app/controllers/storages/admin/storages/project_storages_controller.rb @@ -55,7 +55,7 @@ def new respond_with_dialog Storages::Admin::Storages::AddProjectsModalComponent.new(project_storage: @project_storage) end - def create + def create # rubocop:disable Metrics/AbcSize create_service = ::Storages::ProjectStorages::BulkCreateService .new(user: current_user, projects: @projects, storage: @storage, include_sub_projects: include_sub_projects?) diff --git a/modules/storages/config/locales/en.yml b/modules/storages/config/locales/en.yml index 5ece01da04ab..d0ca24fac6fe 100644 --- a/modules/storages/config/locales/en.yml +++ b/modules/storages/config/locales/en.yml @@ -22,12 +22,12 @@ en: only_numeric_or_uuid: can only be numeric or uuid. storages/project_storage: attributes: + project_folder_id: + blank: Please select a folder. project_folder_mode: mode_unavailable: is not available for this storage. project_ids: blank: Please select a project. - project_folder_id: - blank: Please select a folder. storages/storage: attributes: host: From ffff1a0f1b37e53c985e547eae0da833cb32b3e7 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 6 Aug 2024 17:22:35 +0300 Subject: [PATCH 20/22] test[Op#55967]: fixup tests --- .../storages/admin/project_storages_spec.rb | 30 ++++++------------- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/modules/storages/spec/features/storages/admin/project_storages_spec.rb b/modules/storages/spec/features/storages/admin/project_storages_spec.rb index e7ef37415c74..2db9ff004b39 100644 --- a/modules/storages/spec/features/storages/admin/project_storages_spec.rb +++ b/modules/storages/spec/features/storages/admin/project_storages_spec.rb @@ -44,7 +44,7 @@ shared_let(:project) { create(:project, name: "My active Project") } shared_let(:archived_project) { create(:project, active: false, name: "My archived Project") } - shared_let(:storage) { create(:nextcloud_storage, :as_automatically_managed, name: "My Nextcloud Storage") } + shared_let(:storage) { create(:nextcloud_storage_with_complete_configuration, name: "My Nextcloud Storage") } shared_let(:project_storage) { create(:project_storage, project:, storage:, project_folder_mode: "automatic") } shared_let(:archived_project_project_storage) do @@ -89,8 +89,6 @@ before do login_as(admin) storage.update!(host: "https://example.com") - create(:oauth_application, integration: storage) - create(:oauth_client, integration: storage) visit admin_settings_storage_project_storages_path(storage) end @@ -180,12 +178,8 @@ end describe "Linking a project to a storage with a manually managed folder" do - let(:oauth_application) { create(:oauth_application) } - let(:storage) { create(:nextcloud_storage, :as_automatically_managed, oauth_application:) } - let(:oauth_client) { create(:oauth_client, integration: storage) } - context "when the user has granted OAuth access" do - let(:oauth_client_token) { create(:oauth_client_token, oauth_client:, user: admin) } + let(:oauth_client_token) { create(:oauth_client_token, oauth_client: storage.oauth_client, user: admin) } let(:location_picker) { Components::FilePickerDialog.new } let(:root_xml_response) { build(:webdav_data) } @@ -237,7 +231,7 @@ expect(page.find_by_id("storages_project_storage_project_folder_mode_automatic")).to be_checked choose "Existing folder with manually managed permissions" - expect(page).to have_text("No selected folder") + wait_for(page).to have_text("No selected folder") click_on "Select folder" location_picker.expect_open @@ -271,7 +265,7 @@ check "Include sub-projects" choose "Existing folder with manually managed permissions" - expect(page).to have_text("No selected folder") + wait_for(page).to have_text("No selected folder") click_on "Add" @@ -284,14 +278,8 @@ end context "when the user has not granted oauth access" do - let(:nonce) { "57a17c3f-b2ed-446e-9dd8-651ba3aec37d" } - let(:redirect_uri) do - "#{CGI.escape(OpenProject::Application.root_url)}/oauth_clients/#{storage.oauth_client.client_id}/callback" - end - before do - allow(SecureRandom).to receive(:uuid).and_call_original.ordered - allow(SecureRandom).to receive(:uuid).and_return(nonce).ordered + OAuthClientToken.where(user: admin, oauth_client: storage.oauth_client).destroy_all end it "show a storage login button" do @@ -299,11 +287,11 @@ within("dialog") do choose "Existing folder with manually managed permissions" - expect(page).to have_button("Nextcloud login") + wait_for(page).to have_button("Nextcloud login") click_on("Nextcloud login") - wait_for(page).to have_current_path("/index.php/apps/oauth2/authorize" \ - "?client_id=#{storage.oauth_client.client_id}&" \ - "redirect_uri=#{redirect_uri}&response_type=code&state=#{nonce}") + wait_for(page).to have_current_path( + %r{/index.php/apps/oauth2/authorize\?client_id=.*&redirect_uri=.*&response_type=code&state=.*} + ) end end end From 710e28592d87430f6a832077aef005933109fb6c Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 6 Aug 2024 18:14:06 +0300 Subject: [PATCH 21/22] test[Op#55967]: undo wait time increments --- .../storages/spec/support/components/file_picker_dialog.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/modules/storages/spec/support/components/file_picker_dialog.rb b/modules/storages/spec/support/components/file_picker_dialog.rb index 2b3504e8e3f3..f256c10b9c04 100644 --- a/modules/storages/spec/support/components/file_picker_dialog.rb +++ b/modules/storages/spec/support/components/file_picker_dialog.rb @@ -55,9 +55,7 @@ def confirm_button_state(selection_count:) def wait_for_folder_loaded page.within(container) do - RSpec::Wait.with_wait(timeout: 10, delay: 1) do - RSpec::Wait.wait_for(page).to have_no_css('[data-test-selector="op-file-list--loading-indicator"]') - end + expect(page).to have_no_css('[data-test-selector="op-file-list--loading-indicator"]') end end From 2cff9fb7d3fbf53488738623825ac55fd6add33a Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 7 Aug 2024 15:27:58 +0300 Subject: [PATCH 22/22] fix[Op#55967]: correct JS style override anti-pattern https://github.com/opf/openproject/pull/16181#discussion_r1706486015 --- .../dynamic/project-storage-form.controller.ts | 14 +++++++------- .../project-folder-mode-form.controller.ts | 9 --------- .../project_settings/_project_folder_form.html.erb | 4 +--- 3 files changed, 8 insertions(+), 19 deletions(-) diff --git a/frontend/src/stimulus/controllers/dynamic/project-storage-form.controller.ts b/frontend/src/stimulus/controllers/dynamic/project-storage-form.controller.ts index a1dc85e6a661..2e1385657e51 100644 --- a/frontend/src/stimulus/controllers/dynamic/project-storage-form.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/project-storage-form.controller.ts @@ -195,14 +195,14 @@ export default class ProjectStorageFormController extends Controller { protected displayFolderSelectionOrLoginButton(isConnected:boolean, projectFolder:IStorageFile|null):void { if (isConnected) { - this.selectProjectFolderButtonTarget.style.display = 'inline-block'; - this.loginButtonTarget.style.display = 'none'; + this.selectProjectFolderButtonTarget.classList.remove('d-none'); + this.loginButtonTarget.classList.add('d-none'); this.selectedFolderTextTarget.innerText = projectFolder === null ? this.placeholderFolderNameValue : projectFolder.name; } else { - this.selectProjectFolderButtonTarget.style.display = 'none'; - this.loginButtonTarget.style.display = 'inline-block'; + this.selectProjectFolderButtonTarget.classList.add('d-none'); + this.loginButtonTarget.classList.remove('d-none'); this.selectedFolderTextTarget.innerText = this.notLoggedInValidationValue; } } @@ -213,12 +213,12 @@ export default class ProjectStorageFormController extends Controller { window.history.replaceState(window.history.state, '', url); } - protected toggleFolderDisplay(value:string):void { + private toggleFolderDisplay(value:string):void { // If the manual radio button is selected, show the manual folder selection section if (this.hasProjectFolderSectionTarget && value === 'manual') { - this.projectFolderSectionTarget.style.display = 'flex'; + this.projectFolderSectionTarget.classList.remove('d-none'); } else { - this.projectFolderSectionTarget.style.display = 'none'; + this.projectFolderSectionTarget.classList.add('d-none'); } } } diff --git a/frontend/src/stimulus/controllers/dynamic/storages/project-folder-mode-form.controller.ts b/frontend/src/stimulus/controllers/dynamic/storages/project-folder-mode-form.controller.ts index e6f1bd755219..ce5cdd449038 100644 --- a/frontend/src/stimulus/controllers/dynamic/storages/project-folder-mode-form.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/storages/project-folder-mode-form.controller.ts @@ -46,13 +46,4 @@ export default class ProjectFolderModeFormController extends ProjectStorageFormC this.selectedFolderTextTarget.innerText = this.notLoggedInValidationValue; } } - - protected toggleFolderDisplay(value:string):void { - // If the manual radio button is selected, show the manual folder selection section - if (this.hasProjectFolderSectionTarget && value === 'manual') { - this.projectFolderSectionTarget.classList.remove('d-none'); - } else { - this.projectFolderSectionTarget.classList.add('d-none'); - } - } } diff --git a/modules/storages/app/views/storages/project_settings/_project_folder_form.html.erb b/modules/storages/app/views/storages/project_settings/_project_folder_form.html.erb index 7341efc4f5f7..d667e379aadc 100644 --- a/modules/storages/app/views/storages/project_settings/_project_folder_form.html.erb +++ b/modules/storages/app/views/storages/project_settings/_project_folder_form.html.erb @@ -121,8 +121,7 @@ See COPYRIGHT and LICENSE files for more details. <% end %>