From d11eb0b92b89753cf51bacf3d5939866f2197d63 Mon Sep 17 00:00:00 2001 From: Jeff Ohrstrom Date: Fri, 12 Jan 2024 13:23:16 -0500 Subject: [PATCH] Bc templates (#3280) Refactor batch connect templates to store this data in the UserStore. --- .../session_contexts_controller.rb | 26 +++------------ .../models/batch_connect/session_context.rb | 6 +++- .../app/models/concerns/user_setting_store.rb | 23 +++++++++++++ .../_prefill_templates.html.erb | 8 ++--- .../user_settings/simple_bc_test.yml | 8 +++++ .../test/system/batch_connect_test.rb | 32 +++++++++++++++++++ apps/dashboard/test/test_helper.rb | 4 +++ 7 files changed, 80 insertions(+), 27 deletions(-) create mode 100644 apps/dashboard/test/fixtures/file_output/user_settings/simple_bc_test.yml 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 1b13fc0cda..def2ee7220 100644 --- a/apps/dashboard/app/controllers/batch_connect/session_contexts_controller.rb +++ b/apps/dashboard/app/controllers/batch_connect/session_contexts_controller.rb @@ -1,6 +1,7 @@ # The controller for creating batch connect sessions. class BatchConnect::SessionContextsController < ApplicationController include BatchConnectConcern + include UserSettingStore # GET /batch_connect//session_contexts/new def new @@ -57,6 +58,7 @@ def create end private + # Set the app from the token def set_app @app = BatchConnect::App.from_token params[:token] @@ -82,22 +84,13 @@ def set_render_format # Set the rendering format for displaying attributes def set_prefill_templates - @prefill_templates ||= begin - return {} unless @app.valid? - - json_path = prefill_templates_root.join("*.json") - Dir.glob(json_path).map do |path| - [File.basename(path, '.json'), File.read(path)] - end.to_h - end + @prefill_templates ||= bc_templates(@app.token) end def save_template return unless params[:save_template].present? && params[:save_template] == "on" && params[:template_name].present? - safe_name = params[:template_name].gsub(/[\x00\/\\:\*\?\"<>\| ]/, '_') - path = prefill_templates_root.join(safe_name.to_s + '.json') - path.write(@session_context.to_json) + save_bc_template(@app.token, params[:template_name], @session_context.to_h) end # Only permit certian parameters @@ -111,15 +104,4 @@ def cache_file p.mkpath unless p.exist? end.join(@app.cache_file) end - - # Root path to the prefill templates - # @return [Pathname] root directory of prefill templates - def prefill_templates_root - cluster = ::Configuration.per_cluster_dataroot? ? cluster_id : nil - - base = BatchConnect::Session.dataroot(@app.token, cluster: cluster) - base.join('prefill_templates').tap do |p| - p.mkpath unless p.exist? - end - end end diff --git a/apps/dashboard/app/models/batch_connect/session_context.rb b/apps/dashboard/app/models/batch_connect/session_context.rb index d8e853b65d..f65e797248 100644 --- a/apps/dashboard/app/models/batch_connect/session_context.rb +++ b/apps/dashboard/app/models/batch_connect/session_context.rb @@ -66,8 +66,12 @@ def update_with_cache(cache) self.attributes = cache.select { |k,v| self[k.to_sym] && self[k.to_sym].cacheable?(app_specific_cache_enabled?) } end + def to_h + Hash[*map { |a| [a.id.to_sym, a.value] }.flatten] + end + def to_openstruct(addons: {}) - context_attrs = Hash[*map { |a| [a.id.to_sym, a.value] }.flatten] + context_attrs = to_h illegal_attrs = OpenStruct.new.methods & context_attrs.keys raise ArgumentError, "#{illegal_attrs.inspect} are keywords that cannot be used as names for form items" unless illegal_attrs.empty? diff --git a/apps/dashboard/app/models/concerns/user_setting_store.rb b/apps/dashboard/app/models/concerns/user_setting_store.rb index e369b5d209..2927986882 100644 --- a/apps/dashboard/app/models/concerns/user_setting_store.rb +++ b/apps/dashboard/app/models/concerns/user_setting_store.rb @@ -1,5 +1,7 @@ module UserSettingStore + BC_TEMPLATES = :batch_connect_templates + def user_settings @user_settings = read_user_settings if @user_settings.nil? @user_settings.clone @@ -37,4 +39,25 @@ def save_user_settings def user_settings_path Pathname.new(::Configuration.dataroot).join(::Configuration.user_settings_file) end + + def bc_templates(app_token) + templates = user_settings[BC_TEMPLATES] + return {} if templates.nil? || templates.empty? + + user_settings[BC_TEMPLATES][app_token.to_sym].to_h + end + + def save_bc_template(app_token, name, key_values) + current_templates = user_settings[BC_TEMPLATES] || {} + current_app_templates = current_templates[app_token.to_sym] || {} + + new_template = {} + new_template[name.to_sym] = key_values + + new_settings = {} + new_settings[BC_TEMPLATES] = {} + new_settings[BC_TEMPLATES][app_token.to_sym] = current_app_templates.merge(new_template) + + update_user_settings(new_settings) + end end diff --git a/apps/dashboard/app/views/batch_connect/session_contexts/_prefill_templates.html.erb b/apps/dashboard/app/views/batch_connect/session_contexts/_prefill_templates.html.erb index aa303c0ddb..0bfe6079a1 100644 --- a/apps/dashboard/app/views/batch_connect/session_contexts/_prefill_templates.html.erb +++ b/apps/dashboard/app/views/batch_connect/session_contexts/_prefill_templates.html.erb @@ -3,8 +3,8 @@ @@ -33,8 +33,8 @@
diff --git a/apps/dashboard/test/fixtures/file_output/user_settings/simple_bc_test.yml b/apps/dashboard/test/fixtures/file_output/user_settings/simple_bc_test.yml new file mode 100644 index 0000000000..887b59b5f4 --- /dev/null +++ b/apps/dashboard/test/fixtures/file_output/user_settings/simple_bc_test.yml @@ -0,0 +1,8 @@ +--- +batch_connect_templates: + sys/bc_paraview: + test template: + cluster: quick + bc_num_hours: '5' + bc_account: abc123 + bc_vnc_resolution: 500x600 diff --git a/apps/dashboard/test/system/batch_connect_test.rb b/apps/dashboard/test/system/batch_connect_test.rb index dc998ba74b..39402d529a 100644 --- a/apps/dashboard/test/system/batch_connect_test.rb +++ b/apps/dashboard/test/system/batch_connect_test.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'application_system_test_case' +require 'ood_core/job/adapters/slurm' class BatchConnectTest < ApplicationSystemTestCase def setup @@ -1293,4 +1294,35 @@ def make_bc_app(dir, form) assert_equal("#{Rails.root}/Gemfile", text_field.value) end end + + test 'saves settings as a template' do + with_modified_env({ ENABLE_NATIVE_VNC: 'true' }) do + Dir.mktmpdir do |dir| + Configuration.stubs(:dataroot).returns(Pathname.new(dir)) + BatchConnect::Session.any_instance.stubs(:save).returns(true) + BatchConnect::Session.any_instance.stubs(:job_id).returns('job-id-123') + OodCore::Job::Adapters::Slurm.any_instance + .stubs(:info) + .returns(OodCore::Job::Info.new(id: 'job-id-123', status: :running)) + + visit new_batch_connect_session_context_url('sys/bc_paraview') + + fill_in(bc_ele_id('bc_num_hours'), with: 5) + fill_in(bc_ele_id('bc_account'), with: 'abc123') + fill_in('bc_vnc_resolution_x_field', with: '500') + fill_in('bc_vnc_resolution_y_field', with: '600') + + check('batch_connect_session_save_template') + fill_in('modal_input_template_new_name', with: 'test template') + sleep 5 # modal needs to sleep? + click_on('Save') + + click_on('Launch', wait: 30) + expected = output_fixture('user_settings/simple_bc_test.yml') + actual = File.read("#{dir}/.ood") + + assert_equal(expected, actual) + end + end + end end diff --git a/apps/dashboard/test/test_helper.rb b/apps/dashboard/test/test_helper.rb index 08f4d765ee..8b0221efe1 100644 --- a/apps/dashboard/test/test_helper.rb +++ b/apps/dashboard/test/test_helper.rb @@ -99,6 +99,10 @@ def stub_sacctmgr .with({}, 'sacctmgr', '-nP', 'show', 'users', 'withassoc', 'format=account,cluster,partition,qos', 'where', 'user=me', stdin_data: '') .returns([File.read('test/fixtures/cmd_output/sacctmgr_show_accts.txt'), '', exit_success]) end + + def output_fixture(file) + File.read("#{Rails.root}/test/fixtures/file_output/#{file}") + end end end