From 8a5aa298e055fad2796c104a48f6547fb697bf37 Mon Sep 17 00:00:00 2001 From: Aday BA Date: Mon, 25 Mar 2024 15:53:16 +0000 Subject: [PATCH] Added saved settings menu and details view (#3434) * Added saved settings menu and details view * Moved saved settings operations to UserSettingsStore * Created full_page_spinner.js file + added permit for expected saved settings request parameters * Added unit tests for Settings and UserSettingsStore * Added check to show page for missing saved settings --- .../app/assets/stylesheets/application.scss | 1 + .../batch_connect/saved_settings.scss | 5 + .../session_contexts_controller.rb | 7 ++ .../batch_connect/sessions_controller.rb | 7 ++ .../batch_connect/settings_controller.rb | 55 +++++++++++ .../app/javascript/full_page_spinner.js | 10 ++ .../app/models/batch_connect/settings.rb | 33 +++++++ .../app/models/concerns/user_setting_store.rb | 19 +++- .../session_contexts/new.html.erb | 1 + .../batch_connect/sessions/index.html.erb | 1 + .../batch_connect/settings/show.html.erb | 97 +++++++++++++++++++ .../shared/_full_page_spinner.html.erb | 6 ++ .../shared/_saved_settings_menu.html.erb | 34 +++++++ apps/dashboard/config/locales/en.yml | 13 +++ apps/dashboard/config/routes.rb | 1 + .../models/batch_connect/settings_test.rb | 54 +++++++++++ .../concerns/user_setting_store_test.rb | 85 ++++++++++++++-- 17 files changed, 420 insertions(+), 9 deletions(-) create mode 100644 apps/dashboard/app/assets/stylesheets/batch_connect/saved_settings.scss create mode 100644 apps/dashboard/app/controllers/batch_connect/settings_controller.rb create mode 100644 apps/dashboard/app/javascript/full_page_spinner.js create mode 100644 apps/dashboard/app/models/batch_connect/settings.rb create mode 100644 apps/dashboard/app/views/batch_connect/settings/show.html.erb create mode 100644 apps/dashboard/app/views/batch_connect/shared/_full_page_spinner.html.erb create mode 100644 apps/dashboard/app/views/batch_connect/shared/_saved_settings_menu.html.erb create mode 100644 apps/dashboard/test/models/batch_connect/settings_test.rb diff --git a/apps/dashboard/app/assets/stylesheets/application.scss b/apps/dashboard/app/assets/stylesheets/application.scss index c26c7ac1b1..4d37e6d0b7 100644 --- a/apps/dashboard/app/assets/stylesheets/application.scss +++ b/apps/dashboard/app/assets/stylesheets/application.scss @@ -158,6 +158,7 @@ small.form-text { @import "products"; @import "batch_connect/sessions"; @import "batch_connect/session_contexts"; +@import "batch_connect/saved_settings"; @import "insufficient_quota"; @import "insufficient_balance"; @import "fa_shims"; diff --git a/apps/dashboard/app/assets/stylesheets/batch_connect/saved_settings.scss b/apps/dashboard/app/assets/stylesheets/batch_connect/saved_settings.scss new file mode 100644 index 0000000000..69cc8d955b --- /dev/null +++ b/apps/dashboard/app/assets/stylesheets/batch_connect/saved_settings.scss @@ -0,0 +1,5 @@ +#saved-settings-menu { + .saved-settings-list .app-icon { + color: rgba(0, 0, 0, 0.25); + } +} \ No newline at end of file diff --git a/apps/dashboard/app/controllers/batch_connect/session_contexts_controller.rb b/apps/dashboard/app/controllers/batch_connect/session_contexts_controller.rb index 2bb79b38f7..a5119811dc 100644 --- a/apps/dashboard/app/controllers/batch_connect/session_contexts_controller.rb +++ b/apps/dashboard/app/controllers/batch_connect/session_contexts_controller.rb @@ -26,6 +26,7 @@ def new end set_app_groups + set_saved_settings set_my_quotas end @@ -54,6 +55,7 @@ def create set_prefill_templates format.html do set_app_groups + set_saved_settings render :new end format.json { render json: @session_context.errors, status: :unprocessable_entity } @@ -76,6 +78,11 @@ def set_app_groups @apps_menu_group = bc_custom_apps_group end + # Set the all the saved settings to render the navigation + def set_saved_settings + @bc_saved_settings = all_bc_templates + end + # Set the session context from the app def set_session_context @session_context = @app.build_session_context diff --git a/apps/dashboard/app/controllers/batch_connect/sessions_controller.rb b/apps/dashboard/app/controllers/batch_connect/sessions_controller.rb index 9346b076b2..2c020e922d 100644 --- a/apps/dashboard/app/controllers/batch_connect/sessions_controller.rb +++ b/apps/dashboard/app/controllers/batch_connect/sessions_controller.rb @@ -1,6 +1,7 @@ # The controller for active batch connect sessions. class BatchConnect::SessionsController < ApplicationController include BatchConnectConcern + include UserSettingStore # GET /batch_connect/sessions # GET /batch_connect/sessions.json @@ -9,6 +10,7 @@ def index @sessions.each(&:update_cache_completed!) set_app_groups + set_saved_settings set_my_quotas end @@ -62,6 +64,11 @@ def set_app_groups @apps_menu_group = bc_custom_apps_group end + # Set the all the saved settings to render the navigation + def set_saved_settings + @bc_saved_settings = all_bc_templates + end + def delete_session_panel? params[:delete] ? params[:delete] == 'true' : true end diff --git a/apps/dashboard/app/controllers/batch_connect/settings_controller.rb b/apps/dashboard/app/controllers/batch_connect/settings_controller.rb new file mode 100644 index 0000000000..8be2f811ce --- /dev/null +++ b/apps/dashboard/app/controllers/batch_connect/settings_controller.rb @@ -0,0 +1,55 @@ +# The controller to manage BatchConnect saved settings +class BatchConnect::SettingsController < ApplicationController + include BatchConnectConcern + include UserSettingStore + + # GET /batch_connect//settings/ + def show + set_app_groups + set_saved_settings + + request_params = settings_request_params + app_token = request_params[:token] + settings_name = request_params[:id] + settings_values = bc_templates(app_token)[settings_name.to_sym] + if settings_values.nil? + redirect_to new_batch_connect_session_context_path(token: app_token), + alert: t('dashboard.bc_saved_settings.missing_settings') + return + end + + @settings = BatchConnect::Settings.new(app_token, settings_name, settings_values) + if @settings.outdated? + flash.now[:alert] = t('dashboard.bc_saved_settings.outdated_message', app_title: @settings.app.title) + end + end + + # DELETE /batch_connect//settings/ + def destroy + request_params = settings_request_params + app_token = request_params[:token] + settings_name = request_params[:id] + delete_bc_template(app_token, settings_name) + redirect_to new_batch_connect_session_context_path(token: app_token), + notice: t('dashboard.bc_saved_settings.deleted_message', settings_name: settings_name) + end + + private + + def settings_request_params + params.permit(:token, :id) + end + + # Set the all the saved settings to render the navigation + def set_saved_settings + @bc_saved_settings = all_bc_templates + end + + # Set list of app lists for navigation + def set_app_groups + @sys_app_groups = bc_sys_app_groups + @usr_app_groups = bc_usr_app_groups + @dev_app_groups = bc_dev_app_groups + @apps_menu_group = bc_custom_apps_group + end +end diff --git a/apps/dashboard/app/javascript/full_page_spinner.js b/apps/dashboard/app/javascript/full_page_spinner.js new file mode 100644 index 0000000000..a2715e4de4 --- /dev/null +++ b/apps/dashboard/app/javascript/full_page_spinner.js @@ -0,0 +1,10 @@ +jQuery(function (){ + function showSpinner() { + $('body').addClass('modal-open'); + $('#full-page-spinner').removeClass('d-none'); + } + + $('.full-page-spinner').each((index, element) => { + $(element).closest('form').on('submit', showSpinner); + }); +}); \ No newline at end of file diff --git a/apps/dashboard/app/models/batch_connect/settings.rb b/apps/dashboard/app/models/batch_connect/settings.rb new file mode 100644 index 0000000000..28def4fe06 --- /dev/null +++ b/apps/dashboard/app/models/batch_connect/settings.rb @@ -0,0 +1,33 @@ +module BatchConnect + # An Interactive Application saved parameters from the form. + class Settings + include ActiveModel::Model + + attr_reader :token, :name, :values + + def initialize(token, name, values) + @token = token.to_s + @name = name.to_s + @values = values.to_h + end + + def app + @app ||= BatchConnect::App.from_token token + end + + def outdated? + outdated = false + # CHECK IF THERE ARE NEW ATTRIBUTES NOT IN THE VALUES HASH + app.attributes.each do |attribute| + outdated = true unless values.key?(attribute.id.to_sym) + end + # CHECK IF THERE ARE OLD VALUES NO LONGER IN THE APP ATTRIBUTES STILL IN THE VALUES HASH + values.each do |attribute_id, _| + outdated = true if app.attributes.select { |attribute| attribute.id.to_sym == attribute_id }.empty? + end + + outdated + end + + end +end diff --git a/apps/dashboard/app/models/concerns/user_setting_store.rb b/apps/dashboard/app/models/concerns/user_setting_store.rb index 47cdd3194e..52fee1c0a1 100644 --- a/apps/dashboard/app/models/concerns/user_setting_store.rb +++ b/apps/dashboard/app/models/concerns/user_setting_store.rb @@ -40,9 +40,13 @@ def user_settings_path Pathname.new(::Configuration.user_settings_file) end + def all_bc_templates + user_settings[BC_TEMPLATES].to_h + end + def bc_templates(app_token) - templates = user_settings[BC_TEMPLATES] - return {} if templates.nil? || templates.empty? + templates = all_bc_templates + return {} if templates.empty? user_settings[BC_TEMPLATES][app_token.to_sym].to_h end @@ -60,4 +64,15 @@ def save_bc_template(app_token, name, key_values) update_user_settings(new_settings) end + + def delete_bc_template(app_token, name) + current_templates = all_bc_templates + current_templates.fetch(app_token.to_sym, {}).delete(name.to_sym) + # Delete app_token group when empty + current_templates.delete(app_token.to_sym) if current_templates.fetch(app_token.to_sym, {}).empty? + + new_settings = {} + new_settings[BC_TEMPLATES] = current_templates + update_user_settings(new_settings) + end end diff --git a/apps/dashboard/app/views/batch_connect/session_contexts/new.html.erb b/apps/dashboard/app/views/batch_connect/session_contexts/new.html.erb index a847120128..284c303be5 100644 --- a/apps/dashboard/app/views/batch_connect/session_contexts/new.html.erb +++ b/apps/dashboard/app/views/batch_connect/session_contexts/new.html.erb @@ -58,6 +58,7 @@ locals: {
+ <%= render partial: 'batch_connect/shared/saved_settings_menu' %> <%= render( partial: "batch_connect/shared/app_menu", diff --git a/apps/dashboard/app/views/batch_connect/sessions/index.html.erb b/apps/dashboard/app/views/batch_connect/sessions/index.html.erb index a637ce4971..eb0e69a5d5 100644 --- a/apps/dashboard/app/views/batch_connect/sessions/index.html.erb +++ b/apps/dashboard/app/views/batch_connect/sessions/index.html.erb @@ -21,6 +21,7 @@ locals: {
<%- if any_apps -%>
+ <%= render partial: 'batch_connect/shared/saved_settings_menu' %> <%= render( partial: "batch_connect/shared/app_menu", diff --git a/apps/dashboard/app/views/batch_connect/settings/show.html.erb b/apps/dashboard/app/views/batch_connect/settings/show.html.erb new file mode 100644 index 0000000000..7219036419 --- /dev/null +++ b/apps/dashboard/app/views/batch_connect/settings/show.html.erb @@ -0,0 +1,97 @@ +<% content_for :title, t('dashboard.bc_saved_settings.title') %> + +<%= render partial: 'batch_connect/shared/breadcrumb', +locals: { + links: [ + { + text: t('dashboard.breadcrumbs_home'), + href: root_path + }, + { + text: "#{@settings.app.title}", + href: new_batch_connect_session_context_path(token: @settings.token) + }, + { + text: @settings.name + }] +} +%> + +
+
+ <%= render partial: 'batch_connect/shared/saved_settings_menu' %> + <%= + render( + partial: "batch_connect/shared/app_menu", + locals: { + sys_app_groups: @sys_app_groups, + usr_app_groups: @usr_app_groups, + dev_app_groups: @dev_app_groups, + apps_menu_group: @apps_menu_group + } + ) + %> +
+
+
+
+
+
+ <% + if @settings.app.valid? + params = @settings.values.map{|name, value| ["batch_connect_session_context[#{name}]", value]}.to_h + title = t('dashboard.bc_saved_settings.launch_title', app_title: @settings.app.title, settings_name: @settings.name) + %> + <%= + button_to( + batch_connect_session_contexts_path(token: @settings.token), + method: :post, + class: %w[btn btn-warning full-page-spinner], + form_class: %w[d-inline], + title: title, + 'aria-label': title, + data: { toggle: "tooltip", placement: "left" }, + params: params + ) do + "#{fa_icon('play', classes: nil)} ".html_safe + end + %> + | + <% end %> + <% + title = t('dashboard.bc_saved_settings.delete_title', settings_name: @settings.name) + %> + <%= + button_to( + batch_connect_setting_path(token: @settings.token, id: @settings.name), + method: :delete, + class: %w[btn btn-danger full-page-spinner], + form_class: %w[d-inline], + title: title, + 'aria-label': title, + data: { confirm: t('dashboard.bc_saved_settings.delete_confirm'), toggle: "tooltip", placement: "left"} + ) do + "#{fa_icon('times-circle', classes: nil)} ".html_safe + end + %> +
+ + <%= @settings.name %> + <%= @settings.app.title %> +
+
+ +

<%= t('dashboard.bc_saved_settings.settings_values_label') %>

+
+ <% @settings.app.attributes.each do |attribute| %> +

+ <%= attribute.label %>: + <%= @settings.values[attribute.id.to_sym] %> +

+ <% end %> +
+
+ +
+
+<%= render partial: 'batch_connect/shared/full_page_spinner' %> \ No newline at end of file diff --git a/apps/dashboard/app/views/batch_connect/shared/_full_page_spinner.html.erb b/apps/dashboard/app/views/batch_connect/shared/_full_page_spinner.html.erb new file mode 100644 index 0000000000..aa8fadc6de --- /dev/null +++ b/apps/dashboard/app/views/batch_connect/shared/_full_page_spinner.html.erb @@ -0,0 +1,6 @@ +<%- content_for :head do -%> + <%= javascript_include_tag('full_page_spinner', nonce: true) %> +<%- end -%> +
+ +
\ No newline at end of file diff --git a/apps/dashboard/app/views/batch_connect/shared/_saved_settings_menu.html.erb b/apps/dashboard/app/views/batch_connect/shared/_saved_settings_menu.html.erb new file mode 100644 index 0000000000..fbd09f49b3 --- /dev/null +++ b/apps/dashboard/app/views/batch_connect/shared/_saved_settings_menu.html.erb @@ -0,0 +1,34 @@ +<%- if Configuration.bc_saved_settings? -%> + <% all_settings = @bc_saved_settings %> +
+
<%= t('dashboard.bc_saved_settings.title') %>
+
+ <% if all_settings.empty? %> + + <%= t('dashboard.bc_saved_settings.no_settings_title') %> + + <% end %> + <% all_settings.sort.each_with_index do | (app_token, app_saved_settings), index| + app = BatchConnect::App.from_token(app_token.to_s) + link = app.link + %> +

+ " aria-expanded="true" aria-controls="<%= "saved-settings-#{index}" %>"> + <%= icon_tag(link.icon_uri) unless link.icon_uri.to_s.blank? %> + <%= app.title %> + +

+
" class="show saved-settings-list"> + <% app_saved_settings.sort.each do |setting_name, _| %> + + + <%= setting_name %> + + <% end %> +
+ <% end %> +
+
+<% end %> \ No newline at end of file diff --git a/apps/dashboard/config/locales/en.yml b/apps/dashboard/config/locales/en.yml index 8aab718ebd..67023d345c 100644 --- a/apps/dashboard/config/locales/en.yml +++ b/apps/dashboard/config/locales/en.yml @@ -255,6 +255,19 @@ en: settings_updated: "Settings updated" + bc_saved_settings: + title: "Saved Settings" + no_settings_title: "You have no saved settings." + missing_settings: "Selected saved settings not found." + launch_label: "Launch" + launch_title: "Launch %{app_title} with %{settings_name} parameters" + delete_label: "Delete" + delete_title: "Delete %{settings_name} saved settings" + delete_confirm: "Are you sure?" + deleted_message: "Saved settings %{settings_name} deleted." + settings_values_label: "Values" + outdated_message: "%{app_title} parameters have changed since these settings were created." + user_configuration: support_ticket_error: "support_ticket is misconfigured. \"email\" or \"rt_api\" sections are required in the configuration YAML." diff --git a/apps/dashboard/config/routes.rb b/apps/dashboard/config/routes.rb index fe523a014c..3128273e2b 100644 --- a/apps/dashboard/config/routes.rb +++ b/apps/dashboard/config/routes.rb @@ -48,6 +48,7 @@ resources :sessions, only: [:index, :destroy] post 'sessions/:id/cancel', to: 'sessions#cancel', as: 'cancel_session' scope '*token', constraints: { token: %r{((usr/[^/]+)|dev|sys)/[^/]+(/[^/]+)?} } do + resources :settings, only: [:show, :destroy] resources :session_contexts, only: [:new, :create] root 'session_contexts#new' end diff --git a/apps/dashboard/test/models/batch_connect/settings_test.rb b/apps/dashboard/test/models/batch_connect/settings_test.rb new file mode 100644 index 0000000000..1265e64c0e --- /dev/null +++ b/apps/dashboard/test/models/batch_connect/settings_test.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require 'test_helper' + +module BatchConnect + class SettingsTest < ActionView::TestCase + + test 'can set values' do + target = BatchConnect::Settings.new('app/token', 'settings name', { name: 'value' }) + + assert_equal('app/token', target.token) + assert_equal('settings name', target.name) + assert_equal({ name: 'value' }, target.values) + end + + test 'app returns BatchConnect::App based on token' do + target = BatchConnect::Settings.new('sys/token', 'settings name', { name: 'value' }) + + assert_instance_of(BatchConnect::App, target.app) + assert_equal('sys/token', target.app.token) + end + + test 'outdated? returns true when settings keys do not match app attribute ids' do + app_token = 'sys/token' + setting_values = { bc_account: 'engineering', bc_cluster: 'test', bc_hours: 4 } + target = BatchConnect::Settings.new(app_token, 'settings name', setting_values) + create_app(app_token, ['bc_account', 'bc_cluster']) + assert_equal(true, target.outdated?) + + setting_values = { bc_account: 'engineering' } + target = BatchConnect::Settings.new(app_token, 'settings name', setting_values) + create_app(app_token, ['bc_account', 'bc_cluster']) + assert_equal(true, target.outdated?) + end + + test 'outdated? returns false when settings keys match app attribute ids' do + app_token = 'sys/token' + setting_values = { bc_account: 'engineering', bc_cluster: 'test' } + target = BatchConnect::Settings.new(app_token, 'settings name', setting_values) + create_app(app_token, ['bc_account', 'bc_cluster']) + + assert_equal(false, target.outdated?) + end + + private + + def create_app(token, attributes) + router = Router.router_from_token(token) + app = BatchConnect::App.new(router: router) + app.stubs(:attributes).returns(attributes.map { |id| SmartAttributes::AttributeFactory.build(id, {}) }) + BatchConnect::App.stubs(:from_token).with(app.token).returns(app) + end + end +end diff --git a/apps/dashboard/test/models/concerns/user_setting_store_test.rb b/apps/dashboard/test/models/concerns/user_setting_store_test.rb index 5ec68000df..7d95353c96 100644 --- a/apps/dashboard/test/models/concerns/user_setting_store_test.rb +++ b/apps/dashboard/test/models/concerns/user_setting_store_test.rb @@ -33,12 +33,10 @@ def setup end test 'update_user_settings should create data directory when is not available' do - Dir.mktmpdir do |temp_data_dir| - data_root = Pathname.new(temp_data_dir).join('update_test') + with_user_settings_file('update_test') do + data_root = Pathname.new(File.dirname(Configuration.user_settings_file)) assert_equal false, data_root.exist? - Configuration.stubs(:user_settings_file).returns("#{data_root.to_s}/settings.yml") - update_user_settings({}) assert_equal true, data_root.exist? @@ -46,9 +44,7 @@ def setup end test 'update_user_settings should update internal data and user settings file' do - Dir.mktmpdir do |temp_data_dir| - Configuration.stubs(:user_settings_file).returns("#{temp_data_dir}/settings.yml") - + with_user_settings_file do settings = user_settings settings[:profile] = 'profile_value' update_user_settings(settings) @@ -58,4 +54,79 @@ def setup end end + test 'all_bc_templates returns empty hash when there is no data' do + with_user_settings_file do + assert_equal({}, all_bc_templates) + end + end + + test 'bc_templates returns empty hash when there is no data' do + with_user_settings_file do + assert_equal({}, bc_templates('app/token')) + end + end + + test 'bc_templates returns application saved settings' do + with_user_settings_file do + app_token = 'app/token' + values = { name1: 'value1', name2: 'value2' } + + save_bc_template(app_token, 'settings1', values) + save_bc_template(app_token, 'settings2', values) + assert_equal({ settings1: values, settings2: values }, bc_templates(app_token)) + end + end + + test 'save_bc_template save settings with expected structure' do + with_user_settings_file do + app_token = 'app/token' + values = { name1: 'value1', name2: 'value2' } + + save_bc_template(app_token, 'settings1', values) + save_bc_template(app_token, 'settings2', values) + assert_equal({ app_token.to_sym => { settings1: values, settings2: values } }, all_bc_templates) + end + end + + test 'delete_bc_template deletes settings data' do + with_user_settings_file do + assert_equal({}, all_bc_templates) + app_token = 'app/token' + values = { name1: 'value1', name2: 'value2' } + + save_bc_template(app_token, 'settings1', values) + save_bc_template(app_token, 'settings2', values) + assert_equal({ app_token.to_sym => { settings1: values, settings2: values } }, all_bc_templates) + + delete_bc_template(app_token, 'settings1') + assert_equal({ app_token.to_sym => { settings2: values } }, all_bc_templates) + end + end + + test 'delete_bc_template deletes app entry when empty' do + with_user_settings_file do + assert_equal({}, all_bc_templates) + app_token = 'app/token' + values = { name1: 'value1', name2: 'value2' } + + save_bc_template('app/token', 'settings name', values) + assert_equal({ app_token.to_sym => { :'settings name' => values } }, all_bc_templates) + + delete_bc_template('app/token', 'settings name') + assert_equal({}, all_bc_templates) + end + end + + private + + def with_user_settings_file(sub_folder = nil) + Dir.mktmpdir do |temp_data_dir| + path_parts = [temp_data_dir, sub_folder, 'settings.yml'].compact + Configuration.stubs(:user_settings_file).returns(File.join(path_parts)) + + yield if block_given? + + end + end + end