Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds auto_environment_variable smart attribute #3301

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions apps/dashboard/Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions apps/dashboard/Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,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)
Expand Down Expand Up @@ -174,6 +175,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)
Expand Down Expand Up @@ -295,6 +299,7 @@ DEPENDENCIES
ood_core (~> 0.24.1)
ood_support (~> 0.0.2)
pbs (~> 2.2.1)
pry
rails (= 6.1.7.6)
redcarpet (~> 3.3)
rest-client (~> 2.0)
Expand Down
8 changes: 6 additions & 2 deletions apps/dashboard/app/controllers/scripts_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def destroy
end
end

# POST /projects/:project_id/scripts/:id/save
# POST /projects/:project_id/:id/save
# save the script after editing
def save
@script.update(save_script_params[:script])
Expand Down Expand Up @@ -90,7 +90,11 @@ def submit_script_params
end

def save_script_params
params.permit({ script: SAVE_SCRIPT_KEYS }, :project_id, :id)
params.permit({ script: SAVE_SCRIPT_KEYS + auto_environment_variable_params }, :project_id, :id)
end

def auto_environment_variable_params
params[:script].keys.select { |key| key.match("auto_environment_variable") }
end

def find_project
Expand Down
9 changes: 8 additions & 1 deletion apps/dashboard/app/helpers/scripts_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@ def create_editable_widget(form, attrib, format: nil)
locals = { form: form, attrib: attrib, format: format, fixed: fixed_attribute }

case widget
when 'number_field'
when 'number_field'
render(partial: editable_partial('editable_number'), locals: locals)
when 'select'
render(partial: editable_partial('editable_select'), locals: locals)
when 'key_value_pair'
render(partial: editable_partial('editable_key_value_pair'), locals: locals)
else
render(partial: editable_partial('generic'), locals: locals)
end
Expand Down Expand Up @@ -53,6 +55,11 @@ def auto_accounts_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
Expand Down
20 changes: 19 additions & 1 deletion apps/dashboard/app/javascript/script_edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,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."
}
}

Expand Down Expand Up @@ -65,7 +69,8 @@ function updateNewFieldOptions(selectMenu) {
const field = document.getElementById(`script_${newField}`);

// if the field doesn't already exist, it's an option for a new field.
if(field === null) {
// TODO: maybe JS equiv of ALLOW_MULTIPLE_FIELDS.include?(newField)
if(field === null || newField == "auto_environment_variable") {
const option = document.createElement("option");
option.value = newField;
option.text = newFieldData[newField].label;
Expand Down Expand Up @@ -143,11 +148,24 @@ function addInProgressField(event) {
justAdded.find('[data-fixed-toggler]')
.on('click', (event) => { toggleFixedField(event) });

justAdded.find('[data-multiple-input="name"]')
.on('change', (event) => { updateMultiple(event) });

const entireDiv = event.target.parentElement.parentElement.parentElement;
entireDiv.remove();
enableNewFieldButton();
}

function updateMultiple(event) {
const group = event.target.parentElement.parentElement;
event.target.id += `_${event.target.value}`
event.target.name = `script[${event.target.id.replace("script_", "")}]`;

let valueInput = group.children[1].children[1];
valueInput.id = event.target.id.replace("_name", "") + "_value";
valueInput.name = `script[${valueInput.id.replace("script_", "")}]`;
}

function fixExcludeBasedOnSelect(selectElement) {
const excludeElementId = selectElement.dataset.excludeId;
const selectOptions = Array.from(selectElement.options);
Expand Down
1 change: 1 addition & 0 deletions apps/dashboard/app/lib/smart_attributes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,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
5 changes: 5 additions & 0 deletions apps/dashboard/app/lib/smart_attributes/attribute_factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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}"
Expand Down
Original file line number Diff line number Diff line change
@@ -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
'key_value_pair'
end

def label(*)
(opts[:label] || 'Environment Variable').to_s
end

def normalize_module(module_name)
module_name.to_s.gsub('-', '_')
end
end
end
end
10 changes: 7 additions & 3 deletions apps/dashboard/app/models/script.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,16 +65,19 @@ def initialize(opts = {})
}

add_required_fields(**sm_opts)

@smart_attributes = build_smart_attributes(**sm_opts)
end

def build_smart_attributes(form: [], attributes: {})
form.map do |form_item_id|
attrs = form.map do |form_item_id|
attrs = attributes[form_item_id.to_sym].to_h.symbolize_keys
cache_value = cached_values[form_item_id]
attrs[:value] = cache_value if cache_value.present?
SmartAttributes::AttributeFactory.build(form_item_id, attrs)
end

attrs
end

def to_yaml
Expand All @@ -83,8 +86,9 @@ def to_yaml
end.deep_stringify_keys

hsh = { 'title' => title, 'created_at' => created_at }
hsh.merge!({ 'form' => smart_attributes.map { |sm| sm.id.to_s } })
hsh.merge!({ 'form' => smart_attributes.map { |sm| sm.id.to_s }.compact })
hsh.merge!({ 'attributes' => attributes })

hsh.to_yaml
end

Expand Down Expand Up @@ -157,7 +161,7 @@ def destroy
end

def update(params)
# reset smart attributes becuase the user could have removed some fields
# reset smart attributes because the user could have removed some fields
@smart_attributes = []

# deal with things that would be in the 'form' section first to initialize
Expand Down
4 changes: 4 additions & 0 deletions apps/dashboard/app/views/scripts/edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,7 @@
<template id="auto_accounts_template">
<%= auto_accounts_template %>
</template>

<template id="auto_environment_variable_template">
<%= auto_environment_variable_template %>
</template>
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<%-
field_id = "#{form.object_name}_#{attrib.id}"
-%>

<div class="editable-form-field">
<div data-multiple-input="group">
<%= form.text_field :auto_environment_variable_name, data: { multiple_input: 'name' } %>
<%= form.text_field :auto_environment_variable_value, data: { multiple_input: 'value' } %>
</div>

<div class="d-none edit-group">
</div>

<%= render(partial: 'scripts/editable_form_fields/edit_field_buttons', locals: { field_id: field_id }) %>
</div>
41 changes: 39 additions & 2 deletions apps/dashboard/test/system/jobs_app_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ def add_bc_num_hours(project_id, script_id)
new_field_id = 'add_new_field_select'

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'].to_set
expected_new_options = ['bc_num_hours', 'auto_queues', 'bc_num_slots', 'auto_accounts', 'auto_environment_variable'].to_set
assert_equal expected_new_options, actual_new_options
end
end
Expand Down Expand Up @@ -415,6 +415,30 @@ def add_bc_num_hours(project_id, script_id)
find('#script_bc_num_hours_fixed').click
find('#save_script_bc_num_hours').click

# add auto_environment_variable
click_on('Add new option')
select('Environment Variable', from: 'add_new_field_select')
click_on(I18n.t('dashboard.add'))
assert find('#script_auto_environment_variable_name')

fill_in('script_auto_environment_variable_name', with: 'SOME_VARIABLE')
fill_in('script_auto_environment_variable_value', with: 'some_value')

assert find('#script_auto_environment_variable_name_SOME_VARIABLE')
assert find('#script_auto_environment_variable_SOME_VARIABLE_value')

# add multiple auto_environment_variables
click_on('Add new option')
select('Environment Variable', from: 'add_new_field_select')
click_on(I18n.t('dashboard.add'))
assert find('#script_auto_environment_variable_name')

fill_in('script_auto_environment_variable_name', with: 'ANOTHER_VARIABLE')
fill_in('script_auto_environment_variable_value', with: 'some_other_value')

assert find('#script_auto_environment_variable_name_ANOTHER_VARIABLE')
assert find('#script_auto_environment_variable_ANOTHER_VARIABLE_value')

# correctly saves
click_on(I18n.t('dashboard.save'))
success_message = I18n.t('dashboard.jobs_scripts_updated')
Expand Down Expand Up @@ -459,9 +483,22 @@ def add_bc_num_hours(project_id, script_id)
label: Number of hours
help: ''
required: true
auto_environment_variable_SOME_VARIABLE:
value: some_value
auto_environment_variable_ANOTHER_VARIABLE:
value: some_other_value
HEREDOC

file = File.read("#{dir}/projects/#{project_id}/.ondemand/scripts/#{script_id}/form.yml")

assert_equal(expected_yml, File.read("#{dir}/projects/#{project_id}/.ondemand/scripts/#{script_id}/form.yml"))
assert_equal(expected_yml, file)

# correctly rebuilds form
# find("[href='#{edit_script_path}']").click

# shows all previously input fields
# assert find('#script_auto_environment_variable_name_SOME_VARIABLE')
# assert find('#script_auto_environment_variable_SOME_VARIABLE_value')
end
end

Expand Down
Loading