Skip to content

Commit

Permalink
Functional prototype for static values in Scripts (#3059)
Browse files Browse the repository at this point in the history
* Added support for static values to scripts forms

* Improved implementation for fixed values in Script forms

* Improved script edit JS for fixed fields

* Improved comments for fixed fields in edit script JS
  • Loading branch information
abujeda authored Sep 27, 2023
1 parent 6803b7b commit b24a37b
Show file tree
Hide file tree
Showing 14 changed files with 110 additions and 17 deletions.
10 changes: 6 additions & 4 deletions apps/dashboard/app/controllers/scripts_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@ class ScriptsController < ApplicationController
before_action :find_script, only: [:show, :edit, :destroy, :submit, :save]

SAVE_SCRIPT_KEYS = [
:cluster, :auto_accounts, :auto_accounts_exclude, :auto_scripts, :auto_scripts_exclude,
:auto_queues, :auto_queues_exclude, :auto_batch_clusters, :auto_batch_clusters_exclude,
:bc_num_slots, :bc_num_slots_min, :bc_num_slots_max,
:bc_num_hours, :bc_num_hours_min, :bc_num_hours_max
:cluster, :auto_accounts, :auto_accounts_exclude, :auto_accounts_fixed,
:auto_scripts, :auto_scripts_exclude, :auto_scripts_fixed,
:auto_queues, :auto_queues_exclude, :auto_queues_fixed,
:auto_batch_clusters, :auto_batch_clusters_exclude, :auto_batch_clusters_fixed,
:bc_num_slots, :bc_num_slots_fixed, :bc_num_slots_min, :bc_num_slots_max,
:bc_num_hours, :bc_num_hours_fixed, :bc_num_hours_min, :bc_num_hours_max
].freeze

def new
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
# Helper for creating new batch connect sessions.
module BatchConnect::SessionContextsHelper
def create_widget(form, attrib, format: nil, hide_excludable: true)
return '' if attrib.fixed?
def create_widget(form, attrib, format: nil, hide_excludable: true, hide_fixed: true)
return '' if hide_fixed && attrib.fixed?
return '' if attrib.hide_when_empty? && attrib.value.blank?

widget = attrib.widget
field_options = attrib.field_options(fmt: format)
all_options = attrib.all_options(fmt: format)

if attrib.fixed?
return render :partial => "batch_connect/session_contexts/fixed", :locals => { form: form, attrib: attrib, field_options: field_options, format: format }
end

case widget
when 'select'
form.select(attrib.id, attrib.select_choices(hide_excludable: hide_excludable), field_options, attrib.html_options)
Expand Down
6 changes: 5 additions & 1 deletion apps/dashboard/app/helpers/scripts_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@ module ScriptsHelper
def create_editable_widget(form, attrib, format: nil)
widget = attrib.widget
attrib.html_options = { class: 'real-field', autocomplete: 'off' }
locals = { form: form, attrib: attrib, format: format }
# For editable script elements, we want the standard render form even when they are fixed.
# We need to reset the fixed attribute to avoid being render as a read only text field.
fixed_attribute = attrib.fixed?
attrib.opts[:fixed] = false
locals = { form: form, attrib: attrib, format: format, fixed: fixed_attribute }

case widget
when 'number_field'
Expand Down
48 changes: 46 additions & 2 deletions apps/dashboard/app/javascript/packs/script_edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ function addNewField(_event) {
}

function updateNewFieldOptions(selectMenu) {
for(var newField in newFieldData) {
field = document.getElementById(`script_${newField}`);
for(let newField in newFieldData) {
const field = document.getElementById(`script_${newField}`);

// if the field doesn't already exist, it's an option for a new field.
if(field === null) {
Expand Down Expand Up @@ -121,10 +121,36 @@ function addInProgressField(event) {
justAdded.find('[data-select-toggler]')
.on('click', (event) => { enableOrDisableSelectOption(event) });

justAdded.find('[data-fixed-toggler]')
.on('click', (event) => { toggleFixedField(event) });

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

function fixedFieldEnabled(checkbox, dataElement) {
// Disable the element to avoid updates from the user
dataElement.disabled = true;
// As it is disabled, need to add a hidden field with the same name to send the fixed field value to the backend.
const input = $('<input>').attr('type','hidden').attr('name', dataElement.name).attr('value', dataElement.value);
$(checkbox).after(input);
}

function toggleFixedField(event) {
event.target.disabled = true;
const elementId = event.target.dataset.fixedToggler;
const dataElement = document.getElementById(elementId);
if (event.target.checked) {
fixedFieldEnabled(event.target, dataElement)
} else {
dataElement.disabled = false;
// Field enabled, remove the hidden field with the same name needed when disabled.
$(`input[type=hidden][name="${dataElement.name}"]`).remove();
}

event.target.disabled = false;
}

function enableOrDisableSelectOption(event) {
const toggleAction = event.target.dataset.selectToggler;
const li = event.target.parentElement;
Expand Down Expand Up @@ -199,6 +225,20 @@ function initSelectFields(){
});
}


function initFixedFields(){
const fixedCheckboxes = Array.from($('[data-fixed-toggler]'));

// find all the enabled 'fixed' checkboxes
fixedCheckboxes.filter((fixedFieldCheckbox) => {
return fixedFieldCheckbox.checked;
// now fix the value of the related field
}).map((fixedFieldCheckbox) => {
const dataElement = document.getElementById(fixedFieldCheckbox.dataset.fixedToggler);
fixedFieldEnabled(fixedFieldCheckbox, dataElement)
});
}

jQuery(() => {
newFieldTemplate = $('#new_field_template');
$('#add_new_field_button').on('click', (event) => { addNewField(event) });
Expand All @@ -215,5 +255,9 @@ jQuery(() => {
$('[data-select-toggler]')
.on('click', (event) => { enableOrDisableSelectOption(event) });

$('[data-fixed-toggler]')
.on('click', (event) => { toggleFixedField(event) });

initSelectFields();
initFixedFields();
});
5 changes: 3 additions & 2 deletions apps/dashboard/app/models/script.rb
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ def respond_to_missing?(method_name, include_private = false)
end

def original_parameter(string)
match = /([\w_]+)_(?:min|max|exclude)/.match(string)
match = /([\w_]+)_(?:min|max|exclude|fixed)/.match(string)
match[1]
end

Expand Down Expand Up @@ -201,7 +201,7 @@ def self.script_form_file(script_path)
# parameters you got from the controller that affect the attributes, not form.
# i.e., mins & maxes you set in the form but get serialized to the 'attributes' section.
def attribute_parameter?(name)
name.end_with?('_min') || name.end_with?('_max') || name.end_with?('_exclude')
['min', 'max', 'exclude', 'fixed'].any? { |postfix| name && name.end_with?("_#{postfix}") }
end

# update the 'form' portion of the yaml file given 'params' from the controller.
Expand All @@ -222,6 +222,7 @@ def update_attributes(params)
orig_param = original_parameter(key).to_sym
self[orig_param].min = value if key.end_with?('_min') && !value.to_s.empty?
self[orig_param].max = value if key.end_with?('_max') && !value.to_s.empty?
self[orig_param].opts[:fixed] = true if key.end_with?('_fixed')

if key.end_with?('_exclude')
exclude_list = value.split(',').to_a
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<%-
all_options = attrib.all_options(fmt: format)
all_options[:readonly] = true
%>

<%= form.text_field(attrib.id, all_options) %>
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<%-
field_id = "#{form.object_name}_#{attrib.id}"

fixed_id = "#{field_id}_fixed"
fixed_name = "#{form.object_name}[#{attrib.id}_fixed]"
-%>
<div class="list-group col-md-4">
<div class="list-group-item mb-3">
<input type="checkbox" id="<%= fixed_id %>" name="<%= fixed_name %>" value="true" <%= fixed ? 'checked' : '' %> data-fixed-toggler="<%= field_id %>">
<label class="form-check-label" for="<%= fixed_id %>"><%= t('dashboard.jobs_scripts_fixed_field') %></label>
</div>
</div>
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<%-
<%-
field_id = "#{form.object_name}_#{attrib.id}"
min_edit_id = "#{field_id}_min"
min_edit_name = "#{form.object_name}[#{attrib.id}_min]"
Expand All @@ -8,13 +8,18 @@

current_min = attrib.field_options[:min]
current_max = attrib.field_options[:max]

fixed_id = "#{field_id}_fixed"
fixed_name = "#{form.object_name}[#{attrib.id}_fixed]"
-%>

<div class="editable-form-field">
<%= create_widget(form, attrib, format: format) %>
<%= create_widget(form, attrib, format: format, hide_fixed: false) %>

<div class="d-none edit-group mb-3">

<%= render(partial: 'scripts/editable_form_fields/edit_fixed_field', locals: { form: form, attrib: attrib, fixed: fixed }) %>

<label for="<%= min_edit_id %>">Minimum</label>
<input min="1" step="1"
class="form-control edit-field" type="number" value="<%= current_min %>"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
-%>

<div class="editable-form-field">
<%= create_widget(form, attrib, format: format, hide_excludable: false) %>
<%= create_widget(form, attrib, format: format, hide_excludable: false, hide_fixed: false) %>

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


<%= render(partial: 'scripts/editable_form_fields/edit_fixed_field', locals: { form: form, attrib: attrib, fixed: fixed }) %>

<ol class="list-group text-center col-md-4 mb-3">
<%- attrib.select_choices(hide_excludable: false).each do |select_data| %>
<%-
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
-%>

<div class="editable-form-field">
<%= create_widget(form, attrib, format: format) %>
<%= create_widget(form, attrib, format: format, hide_fixed: false) %>

<div class="d-none edit-group">
</div>
Expand Down
2 changes: 1 addition & 1 deletion apps/dashboard/app/views/scripts/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<%= bootstrap_form_for(@script, url: submit_project_script_path) do |f| %>
<% @script.smart_attributes.each do |attrib| %>
<%# TODO generate render_format %>
<%= create_widget(f, attrib, format: nil) %>
<%= create_widget(f, attrib, format: nil, hide_fixed: false) %>
<% end %>

<%= f.submit t('dashboard.batch_connect_form_launch'), class: "btn btn-primary btn-block" %>
Expand Down
1 change: 1 addition & 0 deletions apps/dashboard/config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ en:
jobs_scripts_deleted: "Script successfully deleted!"
jobs_scripts_submitted: "Successfully submited job %{job_id}."
jobs_scripts_delete_script_confirmation: "Delete all contents of script?"
jobs_scripts_fixed_field: "Fixed Value"

settings_updated: "Settings updated"

Expand Down
10 changes: 10 additions & 0 deletions apps/dashboard/test/models/script_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,16 @@
require 'test_helper'

class ScriptTest < ActiveSupport::TestCase
test 'supported field postfix' do
target = Script.new({ project_dir: '/path/project', id: 1234, title: 'Test Script' })
refute target.send('attribute_parameter?', nil)
refute target.send('attribute_parameter?', '')
refute target.send('attribute_parameter?', 'account_notsupported')
assert target.send('attribute_parameter?', 'account_min')
assert target.send('attribute_parameter?', 'account_max')
assert target.send('attribute_parameter?', 'account_exclude')
assert target.send('attribute_parameter?', 'account_fixed')
end
test 'creates script' do
Dir.mktmpdir do |tmp|
projects_path = Pathname.new(tmp)
Expand Down
2 changes: 2 additions & 0 deletions apps/dashboard/test/system/jobs_app_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,7 @@ def add_bc_num_hours(project_id, script_id)
fill_in('script_bc_num_hours', with: 42)
fill_in('script_bc_num_hours_min', with: 20)
fill_in('script_bc_num_hours_max', with: 101)
find('#script_bc_num_hours_fixed').click
find('#save_script_bc_num_hours').click

# correctly saves
Expand Down Expand Up @@ -439,6 +440,7 @@ def add_bc_num_hours(project_id, script_id)
min: 20
step: 1
value: '42'
fixed: true
max: 101
label: Number of hours
help: ''
Expand Down

0 comments on commit b24a37b

Please sign in to comment.