From a6eb108693db07808cbe9b3ace27fcac2e8208e3 Mon Sep 17 00:00:00 2001 From: HazelGrant Date: Tue, 19 Mar 2024 09:45:05 -0400 Subject: [PATCH 01/11] Launchers form w/ basic auto_environment_variable widget - WIP --- apps/dashboard/Gemfile | 1 + apps/dashboard/Gemfile.lock | 5 +++ .../dashboard/app/helpers/launchers_helper.rb | 5 +++ .../dashboard/app/javascript/launcher_edit.js | 4 +++ apps/dashboard/app/lib/smart_attributes.rb | 1 + .../lib/smart_attributes/attribute_factory.rb | 5 +++ .../attributes/auto_environment_variable.rb | 32 +++++++++++++++++++ .../app/views/launchers/edit.html.erb | 4 +++ 8 files changed, 57 insertions(+) create mode 100644 apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb diff --git a/apps/dashboard/Gemfile b/apps/dashboard/Gemfile index 912388956d..49bcbcf6f1 100644 --- a/apps/dashboard/Gemfile +++ b/apps/dashboard/Gemfile @@ -21,6 +21,7 @@ gem 'sdoc', group: :doc, require: false group :development, :test do # Call 'byebug' anywhere in the code to stop execution and get a debugger console gem 'byebug' + gem 'pry' gem 'climate_control', '~> 0.2' gem 'timecop', '~> 0.9' end diff --git a/apps/dashboard/Gemfile.lock b/apps/dashboard/Gemfile.lock index 005edddc8f..3fa9d23787 100644 --- a/apps/dashboard/Gemfile.lock +++ b/apps/dashboard/Gemfile.lock @@ -88,6 +88,7 @@ GEM xpath (~> 3.2) childprocess (4.1.0) climate_control (0.2.0) + coderay (1.1.3) coffee-rails (5.0.0) coffee-script (>= 2.2.0) railties (>= 5.2.0) @@ -181,6 +182,9 @@ GEM ood_support (0.0.5) pbs (2.2.1) ffi (~> 1.9, >= 1.9.6) + pry (0.14.2) + coderay (~> 1.1) + method_source (~> 1.0) psych (5.1.2) stringio public_suffix (5.0.4) @@ -304,6 +308,7 @@ DEPENDENCIES ood_core (~> 0.24.1) ood_support (~> 0.0.2) pbs (~> 2.2.1) + pry rails (= 7.0.8) redcarpet (~> 3.3) rest-client (~> 2.0) diff --git a/apps/dashboard/app/helpers/launchers_helper.rb b/apps/dashboard/app/helpers/launchers_helper.rb index 1dd1b313ab..0318099258 100644 --- a/apps/dashboard/app/helpers/launchers_helper.rb +++ b/apps/dashboard/app/helpers/launchers_helper.rb @@ -60,6 +60,11 @@ def auto_job_name_template create_editable_widget(script_form_double, attrib) end + def auto_environment_variable_template + attrib = SmartAttributes::AttributeFactory.build_auto_environment_variable + create_editable_widget(script_form_double, attrib) + end + # We need a form builder to build the template divs. These are # templates so that they are not a part of the _actual_ form (yet). # Otherwise you'd have required fields that you cannot actually edit diff --git a/apps/dashboard/app/javascript/launcher_edit.js b/apps/dashboard/app/javascript/launcher_edit.js index 96c68485bc..bf35df66fa 100644 --- a/apps/dashboard/app/javascript/launcher_edit.js +++ b/apps/dashboard/app/javascript/launcher_edit.js @@ -22,6 +22,10 @@ const newFieldData = { bc_num_slots: { label: "Nodes", help: "How many nodes the job will run on." + }, + auto_environment_variable: { + label: 'Environment Variable', + help: 'Add an environment variable.' } } diff --git a/apps/dashboard/app/lib/smart_attributes.rb b/apps/dashboard/app/lib/smart_attributes.rb index 0fd17482d7..86cdc5b435 100644 --- a/apps/dashboard/app/lib/smart_attributes.rb +++ b/apps/dashboard/app/lib/smart_attributes.rb @@ -21,4 +21,5 @@ module SmartAttributes require 'smart_attributes/attributes/bc_queue' require 'smart_attributes/attributes/bc_vnc_idle' require 'smart_attributes/attributes/bc_vnc_resolution' + require 'smart_attributes/attributes/auto_environment_variable' end diff --git a/apps/dashboard/app/lib/smart_attributes/attribute_factory.rb b/apps/dashboard/app/lib/smart_attributes/attribute_factory.rb index 433f9d496d..d598d460bc 100644 --- a/apps/dashboard/app/lib/smart_attributes/attribute_factory.rb +++ b/apps/dashboard/app/lib/smart_attributes/attribute_factory.rb @@ -2,6 +2,7 @@ module SmartAttributes class AttributeFactory AUTO_MODULES_REX = /\Aauto_modules_([\w-]+)\z/.freeze + AUTO_ENVIRONMENT_VARIABLE_REX = /\Aauto_environment_variable_([\w-]+)\z/.freeze class << self # Build an attribute object from an id and its options @@ -14,6 +15,10 @@ def build(id, opts = {}) hpc_mod = id.match(AUTO_MODULES_REX)[1] id = 'auto_modules' opts = opts.merge({'module' => hpc_mod}) + elsif id.match?(AUTO_ENVIRONMENT_VARIABLE_REX) + env_variable = id.match(AUTO_ENVIRONMENT_VARIABLE_REX)[1] + id = 'auto_environment_variable' + opts = opts.merge({'key' => env_variable}) end build_method = "build_#{id}" diff --git a/apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb b/apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb new file mode 100644 index 0000000000..01c1c40e6b --- /dev/null +++ b/apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb @@ -0,0 +1,32 @@ +module SmartAttributes + class AttributeFactory + extend AccountCache + + def self.build_auto_environment_variable(opts = {}) + Attributes::AutoEnvironmentVariable.new('auto_environment_variable', opts) + end + end + + module Attributes + class AutoEnvironmentVariable < Attribute + def initialize(id, opts = {}) + super + + @key = @opts[:key] + @id = "#{id}_#{normalize_module(@key)}" # reset the id to be unique from other auto_environment_variable_* + end + + def widget + 'text_field' + end + + def label(*) + (opts[:label] || 'Environment Variable').to_s + end + + def normalize_module(module_name) + module_name.to_s.gsub('-', '_') + end + end + end + end \ No newline at end of file diff --git a/apps/dashboard/app/views/launchers/edit.html.erb b/apps/dashboard/app/views/launchers/edit.html.erb index 383448e8d7..d1196fceb2 100644 --- a/apps/dashboard/app/views/launchers/edit.html.erb +++ b/apps/dashboard/app/views/launchers/edit.html.erb @@ -34,3 +34,7 @@ + + \ No newline at end of file From a1ed50a1012670884f0e21f4907847d903a51e43 Mon Sep 17 00:00:00 2001 From: HazelGrant Date: Tue, 19 Mar 2024 13:27:51 -0400 Subject: [PATCH 02/11] Adds new auto_environment_variable functionality to launchers form --- .../app/controllers/launchers_controller.rb | 10 +++++- .../dashboard/app/javascript/launcher_edit.js | 34 ++++++++++++++++++- .../attributes/auto_environment_variable.rb | 13 ++++--- .../_editable_text_field.erb | 11 +++++- .../test/system/project_manager_test.rb | 26 +++++++++++++- 5 files changed, 86 insertions(+), 8 deletions(-) diff --git a/apps/dashboard/app/controllers/launchers_controller.rb b/apps/dashboard/app/controllers/launchers_controller.rb index f968e59083..e6d7edeb9f 100644 --- a/apps/dashboard/app/controllers/launchers_controller.rb +++ b/apps/dashboard/app/controllers/launchers_controller.rb @@ -94,7 +94,15 @@ def submit_script_params end def save_script_params - params.permit({ launcher: SAVE_SCRIPT_KEYS }, :project_id, :id) + auto_env_params = params[:launcher].keys.select { |k| k.match?('auto_environment_variable') } + + auto_env_params.each do |p| + params[:launcher][p] = params[:launcher][p].match(/.*=(.*)/)[1] + end + + whitelisted_params = SAVE_SCRIPT_KEYS + auto_env_params + + params.permit({ launcher: whitelisted_params }, :project_id, :id) end def find_project diff --git a/apps/dashboard/app/javascript/launcher_edit.js b/apps/dashboard/app/javascript/launcher_edit.js index bf35df66fa..1d7a92f4e0 100644 --- a/apps/dashboard/app/javascript/launcher_edit.js +++ b/apps/dashboard/app/javascript/launcher_edit.js @@ -10,7 +10,7 @@ const newFieldData = { auto_queues: { label: "Queues", help: "Which queue the job will submit too.", - }, + }, auto_accounts: { label: "Account", help: "The account the job will be submitted with." @@ -151,11 +151,43 @@ function addInProgressField(event) { justAdded.find('[data-fixed-toggler]') .on('click', (event) => { toggleFixedField(event) }); + justAdded.find('#aev_name') + .on('keyup', (event) => { updateAutoEnvironmentVariableName(event) }); + + justAdded.find('#aev_value') + .on('keyup', (event) => { updateAutoEnvironmentVariableValue(event) }); + const entireDiv = event.target.parentElement.parentElement.parentElement; entireDiv.remove(); enableNewFieldButton(); } +function updateAutoEnvironmentVariableName(event) { + var aev_name = event.target.value; + if ($('#launcher_auto_environment_variable_' + aev_name)[0] == undefined) { + var input_field = event.target.parentElement.parentElement.children[0].children[1]; + var value_field = event.target.parentElement.children[4]; + var aev_value = value_field.value; + value_field.disabled = false; + + input_field.id = `launcher_auto_environment_variable_${aev_name}`; + input_field.name = `launcher[auto_environment_variable_${aev_name}]`; + input_field.value = `${aev_name}=${aev_value}` + } +} + +function updateAutoEnvironmentVariableValue(event) { + var aev_value = event.target.value; + var aev_name = event.target.parentElement.children[2].value; + + if ($('#launcher_auto_environment_variable_' + aev_name)[0] != undefined) { + var input_field = event.target.parentElement.parentElement.children[0].children[1]; + input_field.value = `${aev_name}=${aev_value}`; + } else { + // this should never happen bc value is disabled unless name is defined + } +} + function fixExcludeBasedOnSelect(selectElement) { const excludeElementId = selectElement.dataset.excludeId; const selectOptions = Array.from(selectElement.options); diff --git a/apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb b/apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb index 01c1c40e6b..0c4fb3b1ec 100644 --- a/apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb +++ b/apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb @@ -12,8 +12,9 @@ class AutoEnvironmentVariable < Attribute def initialize(id, opts = {}) super - @key = @opts[:key] - @id = "#{id}_#{normalize_module(@key)}" # reset the id to be unique from other auto_environment_variable_* + @key = @opts.delete(:key) + # reset the id to be unique from other auto_environment_variable_* + @id = @key ? "#{id}_#{normalize_key(@key)}" : id end def widget @@ -23,9 +24,13 @@ def widget def label(*) (opts[:label] || 'Environment Variable').to_s end + + def field_options(fmt: nil) + super.merge({readonly: true}) + end - def normalize_module(module_name) - module_name.to_s.gsub('-', '_') + def normalize_key(key_name) + key_name.to_s.gsub('-', '_') end end end diff --git a/apps/dashboard/app/views/launchers/editable_form_fields/_editable_text_field.erb b/apps/dashboard/app/views/launchers/editable_form_fields/_editable_text_field.erb index ffc6ae708e..50c2a807e9 100644 --- a/apps/dashboard/app/views/launchers/editable_form_fields/_editable_text_field.erb +++ b/apps/dashboard/app/views/launchers/editable_form_fields/_editable_text_field.erb @@ -7,7 +7,16 @@
<%= render(partial: 'launchers/editable_form_fields/edit_fixed_field', locals: { form: form, attrib: attrib, fixed: fixed }) %> -
+ + <% if field_id.match?('auto_environment_variable') %> + + + + + + <% end %> + + <%= render(partial: 'launchers/editable_form_fields/edit_field_buttons', locals: { field_id: field_id }) %> diff --git a/apps/dashboard/test/system/project_manager_test.rb b/apps/dashboard/test/system/project_manager_test.rb index ed9794c461..7131760c9e 100644 --- a/apps/dashboard/test/system/project_manager_test.rb +++ b/apps/dashboard/test/system/project_manager_test.rb @@ -75,6 +75,13 @@ def add_bc_num_hours(project_id, script_id) click_on(I18n.t('dashboard.save')) end + def add_auto_environment_variable(project_id, script_id, save: true) + # now add 'auto_environment_variable' + click_on('Add new option') + select('Environment Variable', from: 'add_new_field_select') + click_on(I18n.t('dashboard.add')) + end + test 'create a new project on fs and display the table entry' do Dir.mktmpdir do |dir| project_id = setup_project(dir) @@ -442,7 +449,7 @@ def add_bc_num_hours(project_id, script_id) actual_new_options = page.all("##{new_field_id} option").map(&:value).to_set expected_new_options = [ 'bc_num_hours', 'auto_queues', 'bc_num_slots', - 'auto_accounts', 'auto_job_name' + 'auto_accounts', 'auto_job_name', 'auto_environment_variable' ].to_set assert_equal expected_new_options, actual_new_options end @@ -486,6 +493,17 @@ def add_bc_num_hours(project_id, script_id) find('#launcher_bc_num_hours_fixed').click find('#save_launcher_bc_num_hours').click + # add auto_environment_variable + add_auto_environment_variable(project_id, script_id) + find('#edit_launcher_auto_environment_variable').click + fill_in('aev_name', with: 'SOME_VARIABLE') + fill_in('aev_value', with: 'some_value') + + field = find('#launcher_auto_environment_variable_SOME_VARIABLE') + assert_equal field.value, "SOME_VARIABLE=some_value" + + find('#save_launcher_auto_environment_variable').click + # correctly saves click_on(I18n.t('dashboard.save')) success_message = I18n.t('dashboard.jobs_scripts_updated') @@ -501,6 +519,7 @@ def add_bc_num_hours(project_id, script_id) - auto_scripts - auto_batch_clusters - bc_num_hours + - auto_environment_variable_SOME_VARIABLE attributes: auto_scripts: options: @@ -530,6 +549,11 @@ def add_bc_num_hours(project_id, script_id) label: Number of hours help: '' required: true + auto_environment_variable_SOME_VARIABLE: + value: some_value + label: Environment Variable + help: '' + required: false HEREDOC assert_equal(expected_yml, File.read("#{dir}/projects/#{project_id}/.ondemand/scripts/#{script_id}/form.yml")) From 94be465178e4d24298058890f12ed954e8f10d4d Mon Sep 17 00:00:00 2001 From: HazelGrant Date: Tue, 19 Mar 2024 15:02:25 -0400 Subject: [PATCH 03/11] Comments out unrelated failing test --- apps/dashboard/test/system/project_manager_test.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/dashboard/test/system/project_manager_test.rb b/apps/dashboard/test/system/project_manager_test.rb index 9adc64071e..7cd150a97c 100644 --- a/apps/dashboard/test/system/project_manager_test.rb +++ b/apps/dashboard/test/system/project_manager_test.rb @@ -875,7 +875,7 @@ def add_auto_environment_variable(project_id, script_id, save: true) end end end - +=begin test 'submitting launchers from a template project works' do Dir.mktmpdir do |dir| # use different accounts than what the template was generated with @@ -913,4 +913,5 @@ def add_auto_environment_variable(project_id, script_id, save: true) assert_selector('.alert-success', text: 'job-id-123') end end +=end end From ab179f7018ffa6e674b8a68f4b3ecff34ed6ef64 Mon Sep 17 00:00:00 2001 From: HazelGrant Date: Thu, 21 Mar 2024 09:52:06 -0400 Subject: [PATCH 04/11] Adds ability to update environment variables --- .../app/controllers/launchers_controller.rb | 8 ++-- .../dashboard/app/javascript/launcher_edit.js | 38 +++++++++---------- .../attributes/auto_environment_variable.rb | 4 +- .../_editable_text_field.erb | 17 +++++++-- .../test/system/project_manager_test.rb | 9 +++-- 5 files changed, 42 insertions(+), 34 deletions(-) diff --git a/apps/dashboard/app/controllers/launchers_controller.rb b/apps/dashboard/app/controllers/launchers_controller.rb index e6d7edeb9f..61cedea2f8 100644 --- a/apps/dashboard/app/controllers/launchers_controller.rb +++ b/apps/dashboard/app/controllers/launchers_controller.rb @@ -94,12 +94,10 @@ def submit_script_params end def save_script_params - auto_env_params = params[:launcher].keys.select { |k| k.match?('auto_environment_variable') } - - auto_env_params.each do |p| - params[:launcher][p] = params[:launcher][p].match(/.*=(.*)/)[1] + auto_env_params = params[:launcher].keys.select do |k| + k.match?('auto_environment_variable') end - + whitelisted_params = SAVE_SCRIPT_KEYS + auto_env_params params.permit({ launcher: whitelisted_params }, :project_id, :id) diff --git a/apps/dashboard/app/javascript/launcher_edit.js b/apps/dashboard/app/javascript/launcher_edit.js index 1d7a92f4e0..46155c55cb 100644 --- a/apps/dashboard/app/javascript/launcher_edit.js +++ b/apps/dashboard/app/javascript/launcher_edit.js @@ -151,10 +151,10 @@ function addInProgressField(event) { justAdded.find('[data-fixed-toggler]') .on('click', (event) => { toggleFixedField(event) }); - justAdded.find('#aev_name') + justAdded.find('[data-auto-environment-variable="name"]') .on('keyup', (event) => { updateAutoEnvironmentVariableName(event) }); - justAdded.find('#aev_value') + justAdded.find('[data-auto-environment-variable="value"]') .on('keyup', (event) => { updateAutoEnvironmentVariableValue(event) }); const entireDiv = event.target.parentElement.parentElement.parentElement; @@ -164,28 +164,22 @@ function addInProgressField(event) { function updateAutoEnvironmentVariableName(event) { var aev_name = event.target.value; - if ($('#launcher_auto_environment_variable_' + aev_name)[0] == undefined) { - var input_field = event.target.parentElement.parentElement.children[0].children[1]; - var value_field = event.target.parentElement.children[4]; - var aev_value = value_field.value; - value_field.disabled = false; - - input_field.id = `launcher_auto_environment_variable_${aev_name}`; - input_field.name = `launcher[auto_environment_variable_${aev_name}]`; - input_field.value = `${aev_name}=${aev_value}` - } + var input_field = event.target.parentElement.parentElement.children[0].children[1]; + var label_field = event.target.parentElement.parentElement.children[0].children[0]; + var value_field = event.target.parentElement.children[4]; + var aev_value = value_field.value; + value_field.removeAttribute('readonly'); + + input_field.id = `launcher_auto_environment_variable_${aev_name}`; + input_field.name = `launcher[auto_environment_variable_${aev_name}]`; + label_field.innerHTML = aev_name; } function updateAutoEnvironmentVariableValue(event) { var aev_value = event.target.value; var aev_name = event.target.parentElement.children[2].value; - - if ($('#launcher_auto_environment_variable_' + aev_name)[0] != undefined) { - var input_field = event.target.parentElement.parentElement.children[0].children[1]; - input_field.value = `${aev_name}=${aev_value}`; - } else { - // this should never happen bc value is disabled unless name is defined - } + var input_field = event.target.parentElement.parentElement.children[0].children[1]; + input_field.value = aev_value } function fixExcludeBasedOnSelect(selectElement) { @@ -374,6 +368,12 @@ jQuery(() => { $('[data-fixed-toggler]') .on('click', (event) => { toggleFixedField(event) }); + $('[data-auto-environment-variable="name"]') + .on('keyup', (event) => { updateAutoEnvironmentVariableName(event) }); + + $('[data-auto-environment-variable="value"]') + .on('keyup', (event) => { updateAutoEnvironmentVariableValue(event) }); + initSelectFields(); initFixedFields(); }); diff --git a/apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb b/apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb index 0c4fb3b1ec..79dad860ec 100644 --- a/apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb +++ b/apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb @@ -22,13 +22,13 @@ def widget end def label(*) - (opts[:label] || 'Environment Variable').to_s + (@key|| 'Environment Variable').to_s end def field_options(fmt: nil) super.merge({readonly: true}) end - + def normalize_key(key_name) key_name.to_s.gsub('-', '_') end diff --git a/apps/dashboard/app/views/launchers/editable_form_fields/_editable_text_field.erb b/apps/dashboard/app/views/launchers/editable_form_fields/_editable_text_field.erb index 50c2a807e9..1b8f3e5035 100644 --- a/apps/dashboard/app/views/launchers/editable_form_fields/_editable_text_field.erb +++ b/apps/dashboard/app/views/launchers/editable_form_fields/_editable_text_field.erb @@ -9,11 +9,20 @@ <%= render(partial: 'launchers/editable_form_fields/edit_fixed_field', locals: { form: form, attrib: attrib, fixed: fixed }) %> <% if field_id.match?('auto_environment_variable') %> - - + + + > - - + + + type="text" + data-auto-environment-variable="value" + value=<%= attrib.value %> + > <% end %> diff --git a/apps/dashboard/test/system/project_manager_test.rb b/apps/dashboard/test/system/project_manager_test.rb index 7cd150a97c..a39e7dd74b 100644 --- a/apps/dashboard/test/system/project_manager_test.rb +++ b/apps/dashboard/test/system/project_manager_test.rb @@ -496,11 +496,12 @@ def add_auto_environment_variable(project_id, script_id, save: true) # add auto_environment_variable add_auto_environment_variable(project_id, script_id) find('#edit_launcher_auto_environment_variable').click - fill_in('aev_name', with: 'SOME_VARIABLE') - fill_in('aev_value', with: 'some_value') + + find("[data-auto-environment-variable='name']").fill_in(with: 'SOME_VARIABLE') + find("[data-auto-environment-variable='value']").fill_in(with: 'some_value') field = find('#launcher_auto_environment_variable_SOME_VARIABLE') - assert_equal field.value, "SOME_VARIABLE=some_value" + assert_equal field.value, "some_value" find('#save_launcher_auto_environment_variable').click @@ -551,7 +552,7 @@ def add_auto_environment_variable(project_id, script_id, save: true) required: true auto_environment_variable_SOME_VARIABLE: value: some_value - label: Environment Variable + label: SOME_VARIABLE help: '' required: false HEREDOC From 3569fdc240635a30ea30cb6c2142b19c7c11ad8c Mon Sep 17 00:00:00 2001 From: HazelGrant Date: Thu, 21 Mar 2024 12:36:46 -0400 Subject: [PATCH 05/11] Additional work on auto_environment_variable --- .../dashboard/app/javascript/launcher_edit.js | 20 +++---------------- .../attributes/auto_environment_variable.rb | 14 ++++++++++--- .../_editable_text_field.erb | 9 +-------- .../test/system/project_manager_test.rb | 7 ++----- 4 files changed, 17 insertions(+), 33 deletions(-) diff --git a/apps/dashboard/app/javascript/launcher_edit.js b/apps/dashboard/app/javascript/launcher_edit.js index 46155c55cb..783952452e 100644 --- a/apps/dashboard/app/javascript/launcher_edit.js +++ b/apps/dashboard/app/javascript/launcher_edit.js @@ -154,9 +154,6 @@ function addInProgressField(event) { justAdded.find('[data-auto-environment-variable="name"]') .on('keyup', (event) => { updateAutoEnvironmentVariableName(event) }); - justAdded.find('[data-auto-environment-variable="value"]') - .on('keyup', (event) => { updateAutoEnvironmentVariableValue(event) }); - const entireDiv = event.target.parentElement.parentElement.parentElement; entireDiv.remove(); enableNewFieldButton(); @@ -164,22 +161,14 @@ function addInProgressField(event) { function updateAutoEnvironmentVariableName(event) { var aev_name = event.target.value; + const labelString = event.target.dataset.labelString; var input_field = event.target.parentElement.parentElement.children[0].children[1]; var label_field = event.target.parentElement.parentElement.children[0].children[0]; - var value_field = event.target.parentElement.children[4]; - var aev_value = value_field.value; - value_field.removeAttribute('readonly'); + input_field.removeAttribute('readonly'); input_field.id = `launcher_auto_environment_variable_${aev_name}`; input_field.name = `launcher[auto_environment_variable_${aev_name}]`; - label_field.innerHTML = aev_name; -} - -function updateAutoEnvironmentVariableValue(event) { - var aev_value = event.target.value; - var aev_name = event.target.parentElement.children[2].value; - var input_field = event.target.parentElement.parentElement.children[0].children[1]; - input_field.value = aev_value + label_field.innerHTML = `${labelString}: ${aev_name}`; } function fixExcludeBasedOnSelect(selectElement) { @@ -371,9 +360,6 @@ jQuery(() => { $('[data-auto-environment-variable="name"]') .on('keyup', (event) => { updateAutoEnvironmentVariableName(event) }); - $('[data-auto-environment-variable="value"]') - .on('keyup', (event) => { updateAutoEnvironmentVariableValue(event) }); - initSelectFields(); initFixedFields(); }); diff --git a/apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb b/apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb index 79dad860ec..801ad22f47 100644 --- a/apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb +++ b/apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb @@ -20,13 +20,21 @@ def initialize(id, opts = {}) def widget 'text_field' end - + + def original_label + if @opts[:label] + @opts[:label].to_s.gsub(": #{@key}", "") + else + 'Environment Variable' + end + end + def label(*) - (@key|| 'Environment Variable').to_s + "#{original_label}: #{@key}" end def field_options(fmt: nil) - super.merge({readonly: true}) + @key.blank? ? super.merge({readonly: true}) : super end def normalize_key(key_name) diff --git a/apps/dashboard/app/views/launchers/editable_form_fields/_editable_text_field.erb b/apps/dashboard/app/views/launchers/editable_form_fields/_editable_text_field.erb index 1b8f3e5035..7c0f9a3c2d 100644 --- a/apps/dashboard/app/views/launchers/editable_form_fields/_editable_text_field.erb +++ b/apps/dashboard/app/views/launchers/editable_form_fields/_editable_text_field.erb @@ -13,16 +13,9 @@ value=<%= attrib.id.gsub(/auto_environment_variable_?/, '') %> > - - - - type="text" - data-auto-environment-variable="value" - value=<%= attrib.value %> - > <% end %> diff --git a/apps/dashboard/test/system/project_manager_test.rb b/apps/dashboard/test/system/project_manager_test.rb index a39e7dd74b..4c8165cd99 100644 --- a/apps/dashboard/test/system/project_manager_test.rb +++ b/apps/dashboard/test/system/project_manager_test.rb @@ -498,10 +498,7 @@ def add_auto_environment_variable(project_id, script_id, save: true) find('#edit_launcher_auto_environment_variable').click find("[data-auto-environment-variable='name']").fill_in(with: 'SOME_VARIABLE') - find("[data-auto-environment-variable='value']").fill_in(with: 'some_value') - - field = find('#launcher_auto_environment_variable_SOME_VARIABLE') - assert_equal field.value, "some_value" + find("#launcher_auto_environment_variable_SOME_VARIABLE").fill_in(with: 'some_value') find('#save_launcher_auto_environment_variable').click @@ -552,7 +549,7 @@ def add_auto_environment_variable(project_id, script_id, save: true) required: true auto_environment_variable_SOME_VARIABLE: value: some_value - label: SOME_VARIABLE + label: 'Environment Variable: SOME_VARIABLE' help: '' required: false HEREDOC From e5ff0c09dc3f7c3cfcf5c424846e9bdca56ee442 Mon Sep 17 00:00:00 2001 From: HazelGrant Date: Thu, 21 Mar 2024 12:43:52 -0400 Subject: [PATCH 06/11] Small method refactor --- .../attributes/auto_environment_variable.rb | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb b/apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb index 801ad22f47..e7a36903e5 100644 --- a/apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb +++ b/apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb @@ -22,11 +22,8 @@ def widget end def original_label - if @opts[:label] - @opts[:label].to_s.gsub(": #{@key}", "") - else - 'Environment Variable' - end + return 'Environment Variable' unless @opts[:label] + @opts[:label].to_s.gsub(": #{@key}", "") end def label(*) From 233ac40fd052dd609ef5aa7af114697922ee9b60 Mon Sep 17 00:00:00 2001 From: HazelGrant Date: Thu, 21 Mar 2024 13:04:03 -0400 Subject: [PATCH 07/11] Uncomments out test that has been fixed on master --- apps/dashboard/test/system/project_manager_test.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/apps/dashboard/test/system/project_manager_test.rb b/apps/dashboard/test/system/project_manager_test.rb index 4c8165cd99..74d5bfbef6 100644 --- a/apps/dashboard/test/system/project_manager_test.rb +++ b/apps/dashboard/test/system/project_manager_test.rb @@ -873,7 +873,7 @@ def add_auto_environment_variable(project_id, script_id, save: true) end end end -=begin + test 'submitting launchers from a template project works' do Dir.mktmpdir do |dir| # use different accounts than what the template was generated with @@ -911,5 +911,4 @@ def add_auto_environment_variable(project_id, script_id, save: true) assert_selector('.alert-success', text: 'job-id-123') end end -=end end From 59c4c5ccdf1f5fe7968e6d3c053c916ab8c740fa Mon Sep 17 00:00:00 2001 From: HazelGrant Date: Wed, 27 Mar 2024 09:34:23 -0400 Subject: [PATCH 08/11] Changes variable name --- apps/dashboard/app/controllers/launchers_controller.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/dashboard/app/controllers/launchers_controller.rb b/apps/dashboard/app/controllers/launchers_controller.rb index 61cedea2f8..519b7c7829 100644 --- a/apps/dashboard/app/controllers/launchers_controller.rb +++ b/apps/dashboard/app/controllers/launchers_controller.rb @@ -98,9 +98,9 @@ def save_script_params k.match?('auto_environment_variable') end - whitelisted_params = SAVE_SCRIPT_KEYS + auto_env_params + allowlist = SAVE_SCRIPT_KEYS + auto_env_params - params.permit({ launcher: whitelisted_params }, :project_id, :id) + params.permit({ launcher: allowlist }, :project_id, :id) end def find_project From 3abb3a50b8238231aaf71866b7e677dda42d647c Mon Sep 17 00:00:00 2001 From: HazelGrant Date: Wed, 27 Mar 2024 09:36:54 -0400 Subject: [PATCH 09/11] Minor fixes --- .../smart_attributes/attributes/auto_environment_variable.rb | 4 ++++ .../launchers/editable_form_fields/_editable_text_field.erb | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb b/apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb index e7a36903e5..8c18e06a00 100644 --- a/apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb +++ b/apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb @@ -37,6 +37,10 @@ def field_options(fmt: nil) def normalize_key(key_name) key_name.to_s.gsub('-', '_') end + + def submit(*) + { script: { job_environment: { @key.to_s.upcase: value }}} + end end end end \ No newline at end of file diff --git a/apps/dashboard/app/views/launchers/editable_form_fields/_editable_text_field.erb b/apps/dashboard/app/views/launchers/editable_form_fields/_editable_text_field.erb index 7c0f9a3c2d..0a35442219 100644 --- a/apps/dashboard/app/views/launchers/editable_form_fields/_editable_text_field.erb +++ b/apps/dashboard/app/views/launchers/editable_form_fields/_editable_text_field.erb @@ -15,7 +15,7 @@ data-auto-environment-variable="name" data-label-string=<%= attrib.original_label.gsub(' ', ' ').force_encoding('utf-8') %> value=<%= attrib.id.gsub(/auto_environment_variable_?/, '') %> - > + > <% end %> From 761a08fb6b1c9da13352b1aca73cea86fdcafdda Mon Sep 17 00:00:00 2001 From: HazelGrant Date: Wed, 27 Mar 2024 09:39:56 -0400 Subject: [PATCH 10/11] Fixes syntax --- .../smart_attributes/attributes/auto_environment_variable.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb b/apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb index 8c18e06a00..f8d3afc8be 100644 --- a/apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb +++ b/apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb @@ -39,7 +39,7 @@ def normalize_key(key_name) end def submit(*) - { script: { job_environment: { @key.to_s.upcase: value }}} + { script: { job_environment: { @key.to_s.upcase => value }}} end end end From 112c9ad7ad98cddfd89ae977dcaedeb4b10e60af Mon Sep 17 00:00:00 2001 From: HazelGrant Date: Thu, 28 Mar 2024 10:00:08 -0400 Subject: [PATCH 11/11] Rsolves the problem where if the user chooses their own label, they can't see the name of the variable in the form. --- .../dashboard/app/javascript/launcher_edit.js | 17 ++++++++------ .../attributes/auto_environment_variable.rb | 6 ++--- .../_editable_text_field.erb | 22 +++++++++---------- 3 files changed, 23 insertions(+), 22 deletions(-) diff --git a/apps/dashboard/app/javascript/launcher_edit.js b/apps/dashboard/app/javascript/launcher_edit.js index 783952452e..fdd1c065dd 100644 --- a/apps/dashboard/app/javascript/launcher_edit.js +++ b/apps/dashboard/app/javascript/launcher_edit.js @@ -3,7 +3,7 @@ let newFieldTemplate = undefined; const newFieldData = { - "bc_num_hours": { + bc_num_hours: { label: "Hours", help: "How long the job can run for.", }, @@ -152,23 +152,26 @@ function addInProgressField(event) { .on('click', (event) => { toggleFixedField(event) }); justAdded.find('[data-auto-environment-variable="name"]') - .on('keyup', (event) => { updateAutoEnvironmentVariableName(event) }); + .on('keyup', (event) => { updateAutoEnvironmentVariable(event) }); const entireDiv = event.target.parentElement.parentElement.parentElement; entireDiv.remove(); enableNewFieldButton(); } -function updateAutoEnvironmentVariableName(event) { +function updateAutoEnvironmentVariable(event) { var aev_name = event.target.value; const labelString = event.target.dataset.labelString; - var input_field = event.target.parentElement.parentElement.children[0].children[1]; - var label_field = event.target.parentElement.parentElement.children[0].children[0]; + var input_field = event.target.parentElement.children[2].children[1]; input_field.removeAttribute('readonly'); input_field.id = `launcher_auto_environment_variable_${aev_name}`; input_field.name = `launcher[auto_environment_variable_${aev_name}]`; - label_field.innerHTML = `${labelString}: ${aev_name}`; + + if (labelString.match(/Environment( |\s)Variable/)) { + var label_field = event.target.parentElement.children[2].children[0]; + label_field.innerHTML = `Environment Variable: ${aev_name}`; + } } function fixExcludeBasedOnSelect(selectElement) { @@ -358,7 +361,7 @@ jQuery(() => { .on('click', (event) => { toggleFixedField(event) }); $('[data-auto-environment-variable="name"]') - .on('keyup', (event) => { updateAutoEnvironmentVariableName(event) }); + .on('keyup', (event) => { updateAutoEnvironmentVariable(event) }); initSelectFields(); initFixedFields(); diff --git a/apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb b/apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb index f8d3afc8be..8d59af0907 100644 --- a/apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb +++ b/apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb @@ -22,12 +22,12 @@ def widget end def original_label - return 'Environment Variable' unless @opts[:label] - @opts[:label].to_s.gsub(": #{@key}", "") + return 'Environment Variable' if !@opts[:label] || opts[:label].match('Environment Variable') + @opts[:label].to_s end def label(*) - "#{original_label}: #{@key}" + (opts[:label] || "Environment Variable: #{@key}").to_s end def field_options(fmt: nil) diff --git a/apps/dashboard/app/views/launchers/editable_form_fields/_editable_text_field.erb b/apps/dashboard/app/views/launchers/editable_form_fields/_editable_text_field.erb index 0a35442219..b63aacfe12 100644 --- a/apps/dashboard/app/views/launchers/editable_form_fields/_editable_text_field.erb +++ b/apps/dashboard/app/views/launchers/editable_form_fields/_editable_text_field.erb @@ -3,22 +3,20 @@ -%>
+<% if field_id.match?('auto_environment_variable') %> + + + value=<%= attrib.id.gsub(/auto_environment_variable_?/, '') %> + > +<% end %> <%= create_widget(form, attrib, format: format, hide_fixed: false) %>
<%= render(partial: 'launchers/editable_form_fields/edit_fixed_field', locals: { form: form, attrib: attrib, fixed: fixed }) %> - - <% if field_id.match?('auto_environment_variable') %> - - - value=<%= attrib.id.gsub(/auto_environment_variable_?/, '') %> - > - <% end %> - -
+
<%= render(partial: 'launchers/editable_form_fields/edit_field_buttons', locals: { field_id: field_id }) %>