[#43111] Fix design and layout for storage creation workflow

https://community.openproject.org/work_packages/43111

config/locales/en.yml

@@ -2458,7 +2458,8 @@ en:
     storage:
       not_available: "Disk storage consumption is not available for this repository."
       update_timeout: "Keep the last required disk space information for a repository for N minutes.\nAs counting the required disk space of a repository may be costly, increase this value to reduce performance impact."
-      oauth_application_details: "Please copy this values into the Nextcloud OpenProject Integration settings. The client secret value will not be accessible again after you close this window."
+      oauth_application_details: "The client secret value will not be accessible again after you close this window. Please copy these values into the Nextcloud OpenProject Integration settings:"
+      oauth_application_details_link_text: "Go to settings page"
     subversion:
       existing_title: "Existing Subversion repository"
       existing_introduction: "If you have an existing Subversion repository, you can link it with OpenProject to access it from within the application."

modules/storages/app/services/storages/storages/set_attributes_service.rb

@@ -33,8 +33,8 @@ module Storages::Storages
 
     def set_default_attributes(_params)
       storage.creator ||= user
-      storage.name ||= I18n.t('storages.provider_types.nextcloud')
       storage.provider_type = Storages::Storage::PROVIDER_TYPE_NEXTCLOUD
+      storage.name ||= I18n.t("storages.provider_types.#{storage.provider_type}.default_name")
     end
 
     private

modules/storages/app/views/storages/admin/storages/_form.html.erb

@@ -35,7 +35,7 @@ See COPYRIGHT and LICENSE files for more details.
     <legend class="form--fieldset-legend"><%= I18n.t(:label_general) %></legend>
     <div class="form--field -required">
       <%= f.select :provider_type,
-                   ::Storages::Storage::PROVIDER_TYPES.map { |provider_type| [I18n.t("storages.provider_types.#{provider_type}"), provider_type] },
+                   ::Storages::Storage::PROVIDER_TYPES.map { |provider_type| [I18n.t("storages.provider_types.#{provider_type}.name"), provider_type] },
                    {
                      selected: ::Storages::Storage::PROVIDER_TYPE_NEXTCLOUD,
                      container_class: '-slim'
@@ -43,9 +43,13 @@ See COPYRIGHT and LICENSE files for more details.
                    {
                      disabled: @object.persisted? || ::Storages::Storage::PROVIDER_TYPES.count == 1
                    } %>
+      <span class="form--field-instructions"><%= t('storages.instructions.type') %> <%= t('storages.instructions.type_link_text') %></span>
     </div>
     <div class="form--field -required">
-      <%= f.text_field :name, required: true, container_class: '-slim' %>
+      <%= f.text_field :name,
+                       required: true,
+                       placeholder: I18n.t('storages.provider_types.nextcloud.name_placeholder'),
+                       container_class: '-slim' %>
       <span class="form--field-instructions"><%= t('storages.instructions.name') %></span>
     </div>
     <div class="form--field -required">

modules/storages/app/views/storages/admin/storages/edit.html.erb

@@ -44,23 +44,23 @@
 
 <section class="form--section">
   <fieldset class="form--fieldset">
-    <legend class="form--fieldset-legend"><%= t("storages.provider_types.#{@object.provider_type}") %> <%= t(:'storages.label_oauth_client_details') %></legend>
+    <legend class="form--fieldset-legend"><%= t("storages.provider_types.#{@object.provider_type}.name") %> <%= t(:'storages.label_oauth_client_details') %></legend>
     <% if @object.oauth_client %>
       <div class="attributes-key-value">
-        <div class="attributes-key-value--key"><%= t("storages.provider_types.#{@object.provider_type}") %> <%= t(:'storages.label_oauth_client_id') %></div>
+        <div class="attributes-key-value--key"><%= t("storages.provider_types.#{@object.provider_type}.name") %> <%= t(:'storages.label_oauth_client_id') %></div>
         <div class="attributes-key-value--value-container">
           <div class="attributes-key-value--value -text">
             <span><%= @object.oauth_client.client_id %></span>
           </div>
         </div>
-        <div class="attributes-key-value--key"><%= t("storages.provider_types.#{@object.provider_type}") %> <%= t(:'storages.label_oauth_client_secret') %></div>
+        <div class="attributes-key-value--key"><%= t("storages.provider_types.#{@object.provider_type}.name") %> <%= t(:'storages.label_oauth_client_secret') %></div>
         <div class="attributes-key-value--value-container">
           <div class="attributes-key-value--value -text">
             <span><%= short_secret(@object.oauth_client.client_secret) %></span>
           </div>
         </div>
       </div>
-      <%= link_to(t("storages.buttons.replace_provider_type_oauth", provider_type: t("storages.provider_types.#{@object.provider_type}")),
+      <%= link_to(t("storages.buttons.replace_provider_type_oauth", provider_type: t("storages.provider_types.#{@object.provider_type}.name")),
                   new_admin_settings_storage_oauth_client_path(@object),
                   data: { confirm: t(:'storages.confirm_replace_oauth_client')},
                   class: 'button -with-icon icon-reload' ) %>

modules/storages/app/views/storages/admin/storages/new.html.erb

@@ -8,6 +8,8 @@
   <% if @object.oauth_client %>
     <%= styled_button_tag t(:button_save), class: "-highlight -with-icon icon-checkmark" %>
   <% else %>
-    <%= styled_button_tag t(:button_continue), class: "-highlight -with-icon icon-arrow-right3" %>
+    <%= styled_button_tag t("storages.buttons.save_and_continue_setup"), class: "-highlight -with-icon icon-checkmark" %>
   <% end %>
+  <%= link_to t("storages.buttons.cancel"), admin_settings_storages_path, class: 'button -with-icon icon-close' %>
+  <%#= styled_button_tag t("storages.buttons.cancel"), class: "-with-icon icon-close" %>
 <% end %>

modules/storages/app/views/storages/admin/storages/new_oauth_client.html.erb

@@ -1,7 +1,7 @@
 <!-- Standard Ruby view, please see the controller for comments -->
-<% html_title t(:label_administration), t("project_module_storages"), @storage.name, "#{t("storages.provider_types.#{@storage.provider_type}")} #{t("storages.label_oauth_client_details")}" %>
+<% html_title t(:label_administration), t("project_module_storages"), @storage.name, "#{t("storages.provider_types.#{@storage.provider_type}.name")} #{t("storages.label_oauth_client_details")}" %>
-<% local_assigns[:additional_breadcrumb] = "#{t("storages.provider_types.#{@storage.provider_type}")} #{t("storages.label_oauth_client_details")}" %>
+<% local_assigns[:additional_breadcrumb] = "#{t("storages.provider_types.#{@storage.provider_type}.name")} #{t("storages.label_oauth_client_details")}" %>
-<%= toolbar title: "#{t("storages.provider_types.#{@storage.provider_type}")} #{t("storages.label_oauth_client_details")}" %>
+<%= toolbar title: "#{t("storages.provider_types.#{@storage.provider_type}.name")} #{t("storages.label_oauth_client_details")}" %>
 
 <%= labelled_tabular_form_for @oauth_client, url: admin_settings_storage_oauth_client_path do |f| -%>
   <div class="form--field -required">
@@ -11,10 +11,10 @@
                      size: 40,
                      container_class: '-wide' %>
     <span class="form--field-instructions">
-      <%= t("storages.instructions.#{@storage.provider_type}.oauth_client_id") %>
+      <%= "#{t("storages.instructions.copy_from")}: " %>
-      <%= link_to "#{t("storages.provider_types.#{@storage.provider_type}")} / #{t("storages.instructions.#{@storage.provider_type}.administration")} / #{t("storages.instructions.#{@storage.provider_type}.oauth2_clients")}",
+      <%= link_to "#{t("storages.instructions.#{@storage.provider_type}.integration")}",
                   URI::join(@storage.host, "settings/admin/openproject").to_s,
-                  target: "blank" %>
+                  target: "_blank" %>
     </span>
   </div>
   <div class="form--field -required">
@@ -24,15 +24,15 @@
                      size: 40,
                      container_class: '-wide' %>
     <span class="form--field-instructions">
-      <%= t("storages.instructions.#{@storage.provider_type}.oauth_client_secret") %>
+      <%= "#{t("storages.instructions.copy_from")}: " %>
-      <%= link_to "#{t("storages.provider_types.#{@storage.provider_type}")} / #{t("storages.instructions.#{@storage.provider_type}.administration")} / #{t("storages.instructions.#{@storage.provider_type}.oauth2_clients")}",
+      <%= link_to "#{t("storages.instructions.#{@storage.provider_type}.integration")}",
                   URI::join(@storage.host, "settings/admin/openproject").to_s,
-                  target: "blank" %>
+                  target: "blank"%>
     </span>
   </div>
   <% if @storage.oauth_client %>
     <%= styled_button_tag t(:button_replace), class: "-highlight -with-icon icon-checkmark" %>
   <% else %>
-    <%= styled_button_tag t(:button_save), class: "-highlight -with-icon icon-checkmark" %>
+    <%= styled_button_tag t("storages.buttons.save_and_complete_setup"), class: "-highlight -with-icon icon-checkmark" %>
   <% end %>
 <% end %>

modules/storages/app/views/storages/admin/storages/show.html.erb

@@ -120,18 +120,18 @@ See COPYRIGHT and LICENSE files for more details.
 <div class="attributes-group">
   <div class="attributes-group--header">
     <div class="attributes-group--header-container">
-      <h3 class="attributes-group--header-text"><%= t("storages.provider_types.#{@object.provider_type}") %> <%= t(:'storages.label_oauth_client_details') %></h3>
+      <h3 class="attributes-group--header-text"><%= t("storages.provider_types.#{@object.provider_type}.name") %> <%= t(:'storages.label_oauth_client_details') %></h3>
     </div>
   </div>
   <% if @object.oauth_client %>
     <div class="attributes-key-value">
-      <div class="attributes-key-value--key"><%= t("storages.provider_types.#{@object.provider_type}") %> <%= t(:'storages.label_oauth_client_id') %></div>
+      <div class="attributes-key-value--key"><%= t("storages.provider_types.#{@object.provider_type}.name") %> <%= t(:'storages.label_oauth_client_id') %></div>
       <div class="attributes-key-value--value-container">
         <div class="attributes-key-value--value -text">
           <span><%= @object.oauth_client.client_id %></span>
         </div>
       </div>
-      <div class="attributes-key-value--key"><%= t("storages.provider_types.#{@object.provider_type}") %> <%= t(:'storages.label_oauth_client_secret') %></div>
+      <div class="attributes-key-value--key"><%= t("storages.provider_types.#{@object.provider_type}.name") %> <%= t(:'storages.label_oauth_client_secret') %></div>
       <div class="attributes-key-value--value-container">
         <div class="attributes-key-value--value -text">
           <span><%= short_secret(@object.oauth_client.client_secret) %></span>

modules/storages/app/views/storages/admin/storages/show_oauth_application.html.erb

@@ -1,11 +1,21 @@
 <!-- Standard Ruby view, please see the controller for comments -->
-<% html_title t(:label_administration), t("project_module_storages"), @object.name, "#{t("storages.provider_types.#{@object.provider_type}")} #{t("storages.label_oauth_application_details")}" %>
+<% html_title t(:label_administration), t("project_module_storages"), @object.name, "#{t("storages.provider_types.#{@object.provider_type}.name")} #{t("storages.label_oauth_application_details")}" %>
-<% local_assigns[:additional_breadcrumb] = "#{t("storages.provider_types.#{@object.provider_type}")} #{t("storages.label_oauth_application_details")}" %>
+<% local_assigns[:additional_breadcrumb] = "#{t("storages.provider_types.#{@object.provider_type}.name")} #{t("storages.label_oauth_application_details")}" %>
-<%= toolbar title: "#{t("storages.provider_types.#{@object.provider_type}")} #{t("storages.label_oauth_application_details")}" %>
+<%= toolbar title: "#{t("storages.provider_types.#{@object.provider_type}.name")} #{t("storages.label_oauth_application_details")}" %>
 
 <div class="op-toast -warning -with-bottom-spacing">
   <div class="op-toast--content">
-    <p><%= t('repositories.storage.oauth_application_details') %></p>
+    <p>
+      <%= t('repositories.storage.oauth_application_details') %>
+      <a
+        href="<%= URI::join(@oauth_application.integration.host, 'settings/admin/openproject').to_s %>"
+        target="_blank"
+        class="spot-link"
+      >
+        <span class="spot-icon spot-icon_arrow-right3"></span>
+        <span><%= t('repositories.storage.oauth_application_details_link_text') %></span>
+      </a>
+    </p>
   </div>
 </div>
 

modules/storages/config/locales/en.yml

@@ -40,24 +40,27 @@ en:
       done_continue_setup: "Done. Continue setup"
       replace_openproject_oauth: "Replace OpenProject OAuth"
       replace_provider_type_oauth: "Replace %{provider_type} OAuth"
+      save_and_continue_setup: "Save and continue setup"
+      save_and_complete_setup: "Save and complete setup"
+      cancel: "Cancel"
     page_titles:
       project_settings:
         index: "File storages available in this project"
         new: "Enable a file storage for this project"
     instructions:
-      name: "Please use a short name. It will get used as a tab title in the work package view."
+      type: "Please make sure you have administration privileges in your Nextcloud instance and have the following application installed before doing the setup:"
-      host: "The URL of your storage instance. The URL should not be more than 255 characters long."
+      type_link_text: "“Integration OpenProject”"
+      name: "Give your storage a name so that users can differentiate between multiple storages."
+      host: "Please add the host address of your storage including the https://. It should not be longer than 255 characters."
       no_storage_set_up: "There are no file storages set up, yet."
       setting_up_additional_storages: "For setting up additional file storages, please visit"
       setting_up_additional_storages_non_admin: "Administrators can set up additional file storages in Administration / File Storages."
       setting_up_storages: "For setting up file storages, please visit"
       setting_up_storages_non_admin: "Administrators can set up file storages in Administration / File Storages."
       all_available_storages_already_added: "All available storages are already added to the project."
+      copy_from: "Copy this value from"
       nextcloud:
-        administration: Administration
+        integration: "Nextcloud Administration / OpenProject"
-        oauth2_clients: OAuth2 clients
-        oauth_client_id: Copy the value from
-        oauth_client_secret: Copy the value from
     delete_warning:
       storage: >
         Are you sure you want to delete this storage? This will also delete the storage from all projects where it is used.
@@ -81,7 +84,10 @@ en:
     label_oauth_client_secret: "OAuth Client Secret"
     provider_types:
       label: "Provider type"
-      nextcloud: "Nextcloud"
+      nextcloud:
+        name: "Nextcloud"
+        name_placeholder: "e.g. Nextcloud"
+        default_name: "My Nextcloud"
     confirm_replace_oauth_application: "Are you sure? All users will have to authorize again against OpenProject."
     confirm_replace_oauth_client: "Are you sure? All users will have to authorize again against the storage."
     oauth_client_details_missing: "To complete the setup, please add OAuth client credentials from your storage."

modules/storages/spec/features/admin_storages_spec.rb

@@ -48,17 +48,17 @@ describe 'Admin storages', :storage_server_helpers, with_flag: { storages_module
     expect(page).to have_title('New storage')
     expect(page.find('.title-container')).to have_text('New storage')
     expect(page).to have_select 'storages_storage[provider_type]', selected: 'Nextcloud', disabled: true
-    expect(page).to have_field('storages_storage[name]', with: 'Nextcloud')
+    expect(page).to have_field('storages_storage[name]', with: 'My Nextcloud')
 
     # Test the happy path for a valid storage server (host).
     # Mock a valid response (=200) for example.com, so the host validation should succeed
     mock_server_capabilities_response("https://example.com")
     page.find('#storages_storage_name').set("NC 1")
     page.find('#storages_storage_host').set("https://example.com")
-    page.find('button[type=submit]', text: "Continue").click
+    page.find('button[type=submit]', text: "Save and continue setup").click
 
     # Show created oauth application
-    storage_type = I18n.t('storages.provider_types.nextcloud')
+    storage_type = I18n.t('storages.provider_types.nextcloud.name')
     expect(page).to have_title("#{storage_type} #{I18n.t('storages.label_oauth_application_details')}")
     oauth_app_client_id = page.find('#client_id').value
     expect(oauth_app_client_id.length).to eq 43

modules/storages/spec/services/storages/oauth_applications/create_service_spec.rb

@@ -42,7 +42,7 @@ describe ::Storages::OAuthApplications::CreateService, type: :model do
       expect(subject).to be_success
       expect(subject.result).to be_a ::Doorkeeper::Application
       expect(subject.result.name).to include storage.name
-      expect(subject.result.name).to include I18n.t("storages.provider_types.#{storage.provider_type}")
+      expect(subject.result.name).to include I18n.t("storages.provider_types.#{storage.provider_type}.name")
       expect(subject.result.scopes.to_s).to eql "api_v3"
       expect(subject.result.redirect_uri).to include storage.host
       expect(subject.result.redirect_uri).to include 'apps/integration_openproject/oauth-redirect'

modules/storages/spec/services/storages/storages/set_attributes_service_spec.rb

@@ -90,8 +90,8 @@ describe ::Storages::Storages::SetAttributesService, type: :model do
       expect(subject.result.provider_type).to eq Storages::Storage::PROVIDER_TYPE_NEXTCLOUD
     end
 
-    it 'sets name to Nextcloud by default' do
+    it 'sets name to "My Nextcloud" by default' do
-      expect(subject.result.name).to eq I18n.t('storages.provider_types.nextcloud')
+      expect(subject.result.name).to eq I18n.t('storages.provider_types.nextcloud.default_name')
     end
 
     context 'when setting host' do