From 7f1bdb4906be0b92e61c694661cbf111e7c91074 Mon Sep 17 00:00:00 2001 From: HazelGrant <hrandquist@osc.edu> Date: Wed, 18 Dec 2024 15:12:48 -0500 Subject: [PATCH 1/7] Adds basic test for data-option-for-* --- .../test/system/batch_connect_widgets_test.rb | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/apps/dashboard/test/system/batch_connect_widgets_test.rb b/apps/dashboard/test/system/batch_connect_widgets_test.rb index ce7bad488..a5851f741 100644 --- a/apps/dashboard/test/system/batch_connect_widgets_test.rb +++ b/apps/dashboard/test/system/batch_connect_widgets_test.rb @@ -431,4 +431,53 @@ def make_bc_app(dir, form) end end end + + test 'data-options-for-*' do + Dir.mktmpdir do |dir| + "#{dir}/app".tap { |d| Dir.mkdir(d) } + SysRouter.stubs(:base_path).returns(Pathname.new(dir)) + stub_scontrol + stub_sacctmgr + stub_git("#{dir}/app") + + form = <<~HEREDOC + --- + form: + - cluster + - node_type + attributes: + cluster: + widget: "select" + options: + - owens + - pitzer + node_type: + widget: "select" + options: + - standard + - ['gpu', 'gpu', data-option-for-cluster-pitzer: false] + HEREDOC + + Pathname.new("#{dir}/app/").join('form.yml').write(form) + base_id = 'batch_connect_session_context_path' + + visit new_batch_connect_session_context_url('sys/app') + + # owens is selected, standard and gpu are both visible + select('owens', from: 'batch_connect_session_context_cluster') + options = find_all("#batch_connect_session_context_node_type option") + + assert_equal "standard", options[0]["innerHTML"] + assert_equal "gpu", options[1]["innerHTML"] + + # pitzer is selected, gpu is not visible + select('pitzer', from: 'batch_connect_session_context_cluster') + options = find_all("#batch_connect_session_context_node_type option") + + display_property = { "display" => "none" } + + assert_equal "standard", options[0]["innerHTML"] + assert_equal display_property, options[1].style('display') + end + end end From 4205ae7cb75654c8a24fa1bf48353a55b09ac7a9 Mon Sep 17 00:00:00 2001 From: HazelGrant <hrandquist@osc.edu> Date: Wed, 18 Dec 2024 15:16:28 -0500 Subject: [PATCH 2/7] Adds failing test for data-option-exclusive-for-* --- .../test/system/batch_connect_widgets_test.rb | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/apps/dashboard/test/system/batch_connect_widgets_test.rb b/apps/dashboard/test/system/batch_connect_widgets_test.rb index a5851f741..ac20b7837 100644 --- a/apps/dashboard/test/system/batch_connect_widgets_test.rb +++ b/apps/dashboard/test/system/batch_connect_widgets_test.rb @@ -480,4 +480,53 @@ def make_bc_app(dir, form) assert_equal display_property, options[1].style('display') end end + + test 'data-option-exlusive-for-*' do + Dir.mktmpdir do |dir| + "#{dir}/app".tap { |d| Dir.mkdir(d) } + SysRouter.stubs(:base_path).returns(Pathname.new(dir)) + stub_scontrol + stub_sacctmgr + stub_git("#{dir}/app") + + form = <<~HEREDOC + --- + form: + - cluster + - node_type + attributes: + cluster: + widget: "select" + options: + - owens + - pitzer + node_type: + widget: "select" + options: + - standard + - ['gpu', 'gpu', data-option-exclusive-for-cluster-owens: true] + HEREDOC + + Pathname.new("#{dir}/app/").join('form.yml').write(form) + base_id = 'batch_connect_session_context_path' + + visit new_batch_connect_session_context_url('sys/app') + + # owens is selected, standard and gpu are both visible + select('owens', from: 'batch_connect_session_context_cluster') + options = find_all("#batch_connect_session_context_node_type option") + + assert_equal "standard", options[0]["innerHTML"] + assert_equal "gpu", options[1]["innerHTML"] + + # pitzer is selected, gpu is not visible + select('pitzer', from: 'batch_connect_session_context_cluster') + options = find_all("#batch_connect_session_context_node_type option") + + display_property = { "display" => "none" } + + assert_equal "standard", options[0]["innerHTML"] + assert_equal display_property, options[1].style('display') + end + end end From 58755849c329454e68ec5a906a5ca5aaf4224f49 Mon Sep 17 00:00:00 2001 From: HazelGrant <hrandquist@osc.edu> Date: Thu, 19 Dec 2024 10:58:46 -0500 Subject: [PATCH 3/7] Adds code to make the broken test pass --- .../dashboard/app/javascript/dynamic_forms.js | 142 +++++++++++++++++- .../test/system/batch_connect_widgets_test.rb | 10 +- 2 files changed, 147 insertions(+), 5 deletions(-) diff --git a/apps/dashboard/app/javascript/dynamic_forms.js b/apps/dashboard/app/javascript/dynamic_forms.js index a088382ad..2f7e126bf 100644 --- a/apps/dashboard/app/javascript/dynamic_forms.js +++ b/apps/dashboard/app/javascript/dynamic_forms.js @@ -11,6 +11,7 @@ const formTokens = []; // elements. I.e., {'cluster': [ 'node_type' ] } means that changes to cluster // trigger changes to node_type const optionForHandlerCache = {}; +const exclusiveOptionForHandlerCache = {}; // simples array of string ids for elements that have a handler @@ -110,6 +111,7 @@ function memorizeElements(elements) { elements.each((_i, ele) => { formTokens.push(mountainCaseWords(shortId(ele['id']))); optionForHandlerCache[ele['id']] = []; + exclusiveOptionForHandlerCache[ele['id']] = []; }); }; @@ -137,6 +139,9 @@ function makeChangeHandlers(prefix){ if(key.startsWith('optionFor')) { let token = key.replace(/^optionFor/,''); addOptionForHandler(idFromToken(token), element['id']); + } else if (key.startsWith('exclusiveOptionFor')) { + let token = key.replace(/^exclusiveOptionFor/, ''); + addExclusiveOptionForHandler(idFromToken(token), element['id']); } else if(key.startsWith('max') || key.startsWith('min')) { addMinMaxForHandler(element['id'], opt.value, key, data[key]); } else if(key.startsWith('set')) { @@ -537,7 +542,7 @@ function clamp(currentValue, previous, next) { function addOptionForHandler(causeId, targetId) { const changeId = String(causeId || ''); - + if(changeId.length == 0 || optionForHandlerCache[causeId].includes(targetId)) { // nothing to do. invalid causeId or we already have a handler between the 2 return; @@ -558,6 +563,29 @@ function addOptionForHandler(causeId, targetId) { } }; +function addExclusiveOptionForHandler(causeId, targetId) { + const changeId = String(causeId || ''); + + if(changeId.length == 0 || exclusiveOptionForHandlerCache[causeId].includes(targetId)) { + // nothing to do. invalid causeId or we already have a handler between the 2 + return; + } + + let causeElement = $(`#${causeId}`); + + if(targetId && causeElement) { + // cache the fact that there's a new handler here + exclusiveOptionForHandlerCache[causeId].push(targetId); + + causeElement.on('change', (event) => { + toggleExclusiveOptionsFor(event, targetId); + }); + + // fake an event to initialize + toggleExclusiveOptionsFor({ target: document.querySelector(`#${causeId}`) }, targetId); + } +}; + function parseCheckedWhen(key) { const tokens = key.match(/^hide(\w+)When(\w+)$/); @@ -709,6 +737,27 @@ function optionForFromToken(str) { })[0]; } +/** + * Extract the option for out of an exclusive option for directive. + * + * @example + * exclusiveOptionForClusterOakley -> Cluster + * + * @param {*} str + * @returns - the option for string + */ + function exclusiveOptionForFromToken(str) { + return formTokens.map((token) => { + let match = str.match(`^exclusiveOptionFor${token}`); + + if (match && match.length >= 1) { + return token; + } + }).filter((id) => { + return id !== undefined; + })[0]; +} + /** * Hide or show options of an element based on which cluster is * currently selected and the data-option-for-CLUSTER attributes @@ -798,6 +847,97 @@ function optionForFromToken(str) { document.getElementById(elementId).dispatchEvent((new Event('change', { bubbles: true }))); }; +/** + * Hide or show options of an element based on which cluster is + * currently selected and the data-exclusive-option-for-CLUSTER attributes + * for each option + * + * @param {string} element_name The name of the element with options to toggle + */ + function toggleExclusiveOptionsFor(_event, elementId) { + const options = [...document.querySelectorAll(`#${elementId} option`)]; + let hideSelectedValue = undefined; + + options.forEach(option => { + let hide = false; + + // even though an event occured - an option may be hidden based on the value of + // something else entirely. We're going to hide this option if _any_ of the + // exlusive-option-for- directives apply. + for (let key of Object.keys(option.dataset)) { + let exclusiveOptionFor = exclusiveOptionForFromToken(key); + let exclusiveOptionForId = idFromToken(key.replace(/^exclusiveOptionFor/,'')); + + // it's some other directive type, so just keep going and/or not real + if(!key.startsWith('exclusiveOptionFor') || exclusiveOptionForId === undefined) { + continue; + } + + let exclusiveOptionForValue = mountainCaseWords(document.getElementById(exclusiveOptionForId).value); + + // handle special case where the very first token here is a number. + // browsers expect a prefix of hyphens as if it's the next token. + // if (exclusiveOptionForValue.match(/^\d/)) { + // exclusiveOptionForValue = `-${exclusiveOptionForValue}`; + // } + let exclusiveForCluster = option.dataset[`exclusiveOptionFor${exclusiveOptionFor}${exclusiveOptionForValue}`]; + + hide = !(exclusiveForCluster === 'true') + + if (hide) { + break; + } + }; + + if(hide) { + if(option.selected) { + option.selected = false; + hideSelectedValue = option.textContent; + } + + option.style.display = 'none'; + option.disabled = true; + } else { + option.style.display = ''; + option.disabled = false; + } + }); + + // now that we've hidden/shown everything, let's choose what should now + // be the current selected value. + // if you've hidden what _was_ selected. + if(hideSelectedValue !== undefined) { + let others = [...document.querySelectorAll(`#${elementId} option[value='${hideSelectedValue}']`)]; + let newSelectedOption = undefined; + + // You have hidden what _was_ selected, so try to find a duplicate option that is visible + if(others.length > 1) { + others.forEach(ele => { + if(ele.style.display === '') { + newSelectedOption = ele; + return; + } + }); + } + + // no duplciates are visible, so just pick the first visible option + if (newSelectedOption === undefined) { + others = document.querySelectorAll(`#${elementId} option`); + others.forEach(ele => { + if(newSelectedOption === undefined && ele.style.display === '') { + newSelectedOption = ele; + } + }); + } + + if (newSelectedOption !== undefined) { + newSelectedOption.selected = true; + } + } + + // now that we're done, propogate this change to data-set or data-hide handlers + document.getElementById(elementId).dispatchEvent((new Event('change', { bubbles: true }))); +}; export { makeChangeHandlers diff --git a/apps/dashboard/test/system/batch_connect_widgets_test.rb b/apps/dashboard/test/system/batch_connect_widgets_test.rb index ac20b7837..191ce8ce8 100644 --- a/apps/dashboard/test/system/batch_connect_widgets_test.rb +++ b/apps/dashboard/test/system/batch_connect_widgets_test.rb @@ -481,7 +481,7 @@ def make_bc_app(dir, form) end end - test 'data-option-exlusive-for-*' do + test 'data-option-exlusive-for-' do Dir.mktmpdir do |dir| "#{dir}/app".tap { |d| Dir.mkdir(d) } SysRouter.stubs(:base_path).returns(Pathname.new(dir)) @@ -504,7 +504,7 @@ def make_bc_app(dir, form) widget: "select" options: - standard - - ['gpu', 'gpu', data-option-exclusive-for-cluster-owens: true] + - ['gpu', 'gpu', data-exclusive-option-for-cluster-owens: true] HEREDOC Pathname.new("#{dir}/app/").join('form.yml').write(form) @@ -516,13 +516,15 @@ def make_bc_app(dir, form) select('owens', from: 'batch_connect_session_context_cluster') options = find_all("#batch_connect_session_context_node_type option") + display_property = { "display" => "block" } + assert_equal "standard", options[0]["innerHTML"] - assert_equal "gpu", options[1]["innerHTML"] + assert_equal display_property, options[1].style('display') # pitzer is selected, gpu is not visible select('pitzer', from: 'batch_connect_session_context_cluster') options = find_all("#batch_connect_session_context_node_type option") - + display_property = { "display" => "none" } assert_equal "standard", options[0]["innerHTML"] From 3079c8503c192a52becf211b268451e0b0485a5e Mon Sep 17 00:00:00 2001 From: HazelGrant <hrandquist@osc.edu> Date: Thu, 19 Dec 2024 12:59:14 -0500 Subject: [PATCH 4/7] Refactoring --- .../dashboard/app/javascript/dynamic_forms.js | 204 +++++------------- 1 file changed, 56 insertions(+), 148 deletions(-) diff --git a/apps/dashboard/app/javascript/dynamic_forms.js b/apps/dashboard/app/javascript/dynamic_forms.js index 2f7e126bf..17ba700e5 100644 --- a/apps/dashboard/app/javascript/dynamic_forms.js +++ b/apps/dashboard/app/javascript/dynamic_forms.js @@ -540,10 +540,17 @@ function clamp(currentValue, previous, next) { } } -function addOptionForHandler(causeId, targetId) { +function sharedOptionForHandler(causeId, targetId, optionForType) { const changeId = String(causeId || ''); + let handlerCache = null; + + if (optionForType == 'optionFor') { + handlerCache = optionForHandlerCache; + } else if (optionForType == 'exclusiveOptionFor') { + handlerCache = exclusiveOptionForHandlerCache; + } - if(changeId.length == 0 || optionForHandlerCache[causeId].includes(targetId)) { + if(changeId.length == 0 || handlerCache[causeId].includes(targetId)) { // nothing to do. invalid causeId or we already have a handler between the 2 return; } @@ -552,38 +559,31 @@ function addOptionForHandler(causeId, targetId) { if(targetId && causeElement) { // cache the fact that there's a new handler here - optionForHandlerCache[causeId].push(targetId); + handlerCache[causeId].push(targetId); causeElement.on('change', (event) => { - toggleOptionsFor(event, targetId); + if (optionForType == 'exclusiveOptionFor') { + toggleExclusiveOptionsFor(event, targetId); + } else if (optionForType == 'optionFor') { + toggleOptionsFor(event, targetId); + } }); // fake an event to initialize - toggleOptionsFor({ target: document.querySelector(`#${causeId}`) }, targetId); + if (optionForType == 'exclusiveOptionFor') { + toggleExclusiveOptionsFor({ target: document.querySelector(`#${causeId}`) }, targetId); + } else if (optionForType == 'optionFor') { + toggleOptionsFor({ target: document.querySelector(`#${causeId}`) }, targetId); + } } +} + +function addOptionForHandler(causeId, targetId) { + sharedOptionForHandler(causeId, targetId, 'optionFor'); }; function addExclusiveOptionForHandler(causeId, targetId) { - const changeId = String(causeId || ''); - - if(changeId.length == 0 || exclusiveOptionForHandlerCache[causeId].includes(targetId)) { - // nothing to do. invalid causeId or we already have a handler between the 2 - return; - } - - let causeElement = $(`#${causeId}`); - - if(targetId && causeElement) { - // cache the fact that there's a new handler here - exclusiveOptionForHandlerCache[causeId].push(targetId); - - causeElement.on('change', (event) => { - toggleExclusiveOptionsFor(event, targetId); - }); - - // fake an event to initialize - toggleExclusiveOptionsFor({ target: document.querySelector(`#${causeId}`) }, targetId); - } + sharedOptionForHandler(causeId, targetId, 'exclusiveOptionFor'); }; function parseCheckedWhen(key) { @@ -715,19 +715,19 @@ function idFromToken(str) { } } - /** * Extract the option for out of an option for directive. * * @example * optionForClusterOakley -> Cluster + * exclusiveOptionForClusterOakley -> Cluster * * @param {*} str * @returns - the option for string */ -function optionForFromToken(str) { +function sharedOptionForFromToken(str, optionForType) { return formTokens.map((token) => { - let match = str.match(`^optionFor${token}`); + let match = str.match(`^${optionForType}${token}`); if (match && match.length >= 1) { return token; @@ -737,35 +737,15 @@ function optionForFromToken(str) { })[0]; } -/** - * Extract the option for out of an exclusive option for directive. - * - * @example - * exclusiveOptionForClusterOakley -> Cluster - * - * @param {*} str - * @returns - the option for string - */ - function exclusiveOptionForFromToken(str) { - return formTokens.map((token) => { - let match = str.match(`^exclusiveOptionFor${token}`); +function optionForFromToken(str) { + return sharedOptionForFromToken(str, 'optionFor'); +} - if (match && match.length >= 1) { - return token; - } - }).filter((id) => { - return id !== undefined; - })[0]; +function exclusiveOptionForFromToken(str) { + return sharedOptionForFromToken(str, 'exclusiveOptionFor'); } -/** - * Hide or show options of an element based on which cluster is - * currently selected and the data-option-for-CLUSTER attributes - * for each option - * - * @param {string} element_name The name of the element with options to toggle - */ - function toggleOptionsFor(_event, elementId) { +function sharedToggleOptionsFor(_event, elementId, contextStr) { const options = [...document.querySelectorAll(`#${elementId} option`)]; let hideSelectedValue = undefined; @@ -776,11 +756,17 @@ function optionForFromToken(str) { // something else entirely. We're going to hide this option if _any_ of the // option-for- directives apply. for (let key of Object.keys(option.dataset)) { - let optionFor = optionForFromToken(key); - let optionForId = idFromToken(key.replace(/^optionFor/,'')); + let optionFor = ''; + + if (contextStr == 'optionFor') { + optionFor = optionForFromToken(key); + } else if (contextStr == 'exclusiveOptionFor') { + optionFor = exclusiveOptionForFromToken(key); + } + let optionForId = idFromToken(key.replace(new RegExp(`^${contextStr}`),'')); // it's some other directive type, so just keep going and/or not real - if(!key.startsWith('optionFor') || optionForId === undefined) { + if(!key.startsWith(contextStr) || optionForId === undefined) { continue; } @@ -791,99 +777,13 @@ function optionForFromToken(str) { optionForValue = `-${optionForValue}`; } - hide = option.dataset[`optionFor${optionFor}${optionForValue}`] === 'false'; - if (hide) { - break; - } - }; - - if(hide) { - if(option.selected) { - option.selected = false; - hideSelectedValue = option.textContent; - } - - option.style.display = 'none'; - option.disabled = true; - } else { - option.style.display = ''; - option.disabled = false; - } - }); - - // now that we've hidden/shown everything, let's choose what should now - // be the current selected value. - // if you've hidden what _was_ selected. - if(hideSelectedValue !== undefined) { - let others = [...document.querySelectorAll(`#${elementId} option[value='${hideSelectedValue}']`)]; - let newSelectedOption = undefined; - - // You have hidden what _was_ selected, so try to find a duplicate option that is visible - if(others.length > 1) { - others.forEach(ele => { - if(ele.style.display === '') { - newSelectedOption = ele; - return; - } - }); - } - - // no duplciates are visible, so just pick the first visible option - if (newSelectedOption === undefined) { - others = document.querySelectorAll(`#${elementId} option`); - others.forEach(ele => { - if(newSelectedOption === undefined && ele.style.display === '') { - newSelectedOption = ele; - } - }); - } - - if (newSelectedOption !== undefined) { - newSelectedOption.selected = true; - } - } - - // now that we're done, propogate this change to data-set or data-hide handlers - document.getElementById(elementId).dispatchEvent((new Event('change', { bubbles: true }))); -}; - -/** - * Hide or show options of an element based on which cluster is - * currently selected and the data-exclusive-option-for-CLUSTER attributes - * for each option - * - * @param {string} element_name The name of the element with options to toggle - */ - function toggleExclusiveOptionsFor(_event, elementId) { - const options = [...document.querySelectorAll(`#${elementId} option`)]; - let hideSelectedValue = undefined; - - options.forEach(option => { - let hide = false; - - // even though an event occured - an option may be hidden based on the value of - // something else entirely. We're going to hide this option if _any_ of the - // exlusive-option-for- directives apply. - for (let key of Object.keys(option.dataset)) { - let exclusiveOptionFor = exclusiveOptionForFromToken(key); - let exclusiveOptionForId = idFromToken(key.replace(/^exclusiveOptionFor/,'')); - - // it's some other directive type, so just keep going and/or not real - if(!key.startsWith('exclusiveOptionFor') || exclusiveOptionForId === undefined) { - continue; + if (contextStr == 'optionFor') { + hide = option.dataset[`optionFor${optionFor}${optionForValue}`] === 'false'; + } else if (contextStr == 'exclusiveOptionFor') { + const exclusiveForCluster = option.dataset[`exclusiveOptionFor${optionFor}${optionForValue}`]; + hide = !(exclusiveForCluster === 'true') } - let exclusiveOptionForValue = mountainCaseWords(document.getElementById(exclusiveOptionForId).value); - - // handle special case where the very first token here is a number. - // browsers expect a prefix of hyphens as if it's the next token. - // if (exclusiveOptionForValue.match(/^\d/)) { - // exclusiveOptionForValue = `-${exclusiveOptionForValue}`; - // } - let exclusiveForCluster = option.dataset[`exclusiveOptionFor${exclusiveOptionFor}${exclusiveOptionForValue}`]; - - hide = !(exclusiveForCluster === 'true') - if (hide) { break; } @@ -937,6 +837,14 @@ function optionForFromToken(str) { // now that we're done, propogate this change to data-set or data-hide handlers document.getElementById(elementId).dispatchEvent((new Event('change', { bubbles: true }))); +} + +function toggleOptionsFor(_event, elementId) { + sharedToggleOptionsFor(_event, elementId, 'optionFor'); +} + +function toggleExclusiveOptionsFor(_event, elementId) { + sharedToggleOptionsFor(_event, elementId, 'exclusiveOptionFor'); }; export { From a9569326c1e51967f47b670f3163b307a5fab437 Mon Sep 17 00:00:00 2001 From: HazelGrant <hrandquist@osc.edu> Date: Thu, 19 Dec 2024 13:20:47 -0500 Subject: [PATCH 5/7] Small refactor to match code style & additional assertions to test minor details of option-for-* directives --- apps/dashboard/app/javascript/dynamic_forms.js | 3 +-- .../test/system/batch_connect_widgets_test.rb | 14 +++++++++++++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/apps/dashboard/app/javascript/dynamic_forms.js b/apps/dashboard/app/javascript/dynamic_forms.js index 17ba700e5..f989a9c85 100644 --- a/apps/dashboard/app/javascript/dynamic_forms.js +++ b/apps/dashboard/app/javascript/dynamic_forms.js @@ -780,8 +780,7 @@ function sharedToggleOptionsFor(_event, elementId, contextStr) { if (contextStr == 'optionFor') { hide = option.dataset[`optionFor${optionFor}${optionForValue}`] === 'false'; } else if (contextStr == 'exclusiveOptionFor') { - const exclusiveForCluster = option.dataset[`exclusiveOptionFor${optionFor}${optionForValue}`]; - hide = !(exclusiveForCluster === 'true') + hide = !(option.dataset[`exclusiveOptionFor${optionFor}${optionForValue}`] === 'true') } if (hide) { diff --git a/apps/dashboard/test/system/batch_connect_widgets_test.rb b/apps/dashboard/test/system/batch_connect_widgets_test.rb index 191ce8ce8..e4ccd27e8 100644 --- a/apps/dashboard/test/system/batch_connect_widgets_test.rb +++ b/apps/dashboard/test/system/batch_connect_widgets_test.rb @@ -432,7 +432,7 @@ def make_bc_app(dir, form) end end - test 'data-options-for-*' do + test 'data-options-for-' do Dir.mktmpdir do |dir| "#{dir}/app".tap { |d| Dir.mkdir(d) } SysRouter.stubs(:base_path).returns(Pathname.new(dir)) @@ -470,6 +470,9 @@ def make_bc_app(dir, form) assert_equal "standard", options[0]["innerHTML"] assert_equal "gpu", options[1]["innerHTML"] + # select gpu, to test that it's deselected properly when pitzer is selected + select('gpu', from: 'batch_connect_session_context_node_type') + # pitzer is selected, gpu is not visible select('pitzer', from: 'batch_connect_session_context_cluster') options = find_all("#batch_connect_session_context_node_type option") @@ -478,6 +481,9 @@ def make_bc_app(dir, form) assert_equal "standard", options[0]["innerHTML"] assert_equal display_property, options[1].style('display') + + # value of node_type has gone back to standard + assert_equal 'standard', find('#batch_connect_session_context_node_type').value end end @@ -521,6 +527,9 @@ def make_bc_app(dir, form) assert_equal "standard", options[0]["innerHTML"] assert_equal display_property, options[1].style('display') + # select gpu, to test that it's deselected properly when pitzer is selected + select('gpu', from: 'batch_connect_session_context_node_type') + # pitzer is selected, gpu is not visible select('pitzer', from: 'batch_connect_session_context_cluster') options = find_all("#batch_connect_session_context_node_type option") @@ -529,6 +538,9 @@ def make_bc_app(dir, form) assert_equal "standard", options[0]["innerHTML"] assert_equal display_property, options[1].style('display') + + # value of node_type has gone back to standard + assert_equal 'standard', find('#batch_connect_session_context_node_type').value end end end From eeba467649157e0a78186d51bc70e6c473eb48cb Mon Sep 17 00:00:00 2001 From: HazelGrant <hrandquist@osc.edu> Date: Fri, 20 Dec 2024 08:34:26 -0500 Subject: [PATCH 6/7] Moves instiation lines for optionFor handlers to optionForHandler function --- apps/dashboard/app/javascript/dynamic_forms.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/dashboard/app/javascript/dynamic_forms.js b/apps/dashboard/app/javascript/dynamic_forms.js index f989a9c85..301826dea 100644 --- a/apps/dashboard/app/javascript/dynamic_forms.js +++ b/apps/dashboard/app/javascript/dynamic_forms.js @@ -110,8 +110,6 @@ function snakeCaseWords(str) { function memorizeElements(elements) { elements.each((_i, ele) => { formTokens.push(mountainCaseWords(shortId(ele['id']))); - optionForHandlerCache[ele['id']] = []; - exclusiveOptionForHandlerCache[ele['id']] = []; }); }; @@ -545,8 +543,10 @@ function sharedOptionForHandler(causeId, targetId, optionForType) { let handlerCache = null; if (optionForType == 'optionFor') { + if (optionForHandlerCache[causeId] == undefined) optionForHandlerCache[causeId] = []; handlerCache = optionForHandlerCache; } else if (optionForType == 'exclusiveOptionFor') { + if (exclusiveOptionForHandlerCache[causeId] == undefined) exclusiveOptionForHandlerCache[causeId] = []; handlerCache = exclusiveOptionForHandlerCache; } From bc0f1ad48c0e28b97fd4164620eb6c863a202baf Mon Sep 17 00:00:00 2001 From: HazelGrant <hrandquist@osc.edu> Date: Fri, 20 Dec 2024 08:42:34 -0500 Subject: [PATCH 7/7] Matches code style to existing tests --- .../test/system/batch_connect_widgets_test.rb | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/apps/dashboard/test/system/batch_connect_widgets_test.rb b/apps/dashboard/test/system/batch_connect_widgets_test.rb index e4ccd27e8..64bb4ffc6 100644 --- a/apps/dashboard/test/system/batch_connect_widgets_test.rb +++ b/apps/dashboard/test/system/batch_connect_widgets_test.rb @@ -468,7 +468,7 @@ def make_bc_app(dir, form) options = find_all("#batch_connect_session_context_node_type option") assert_equal "standard", options[0]["innerHTML"] - assert_equal "gpu", options[1]["innerHTML"] + assert_equal '', find_option_style('node_type', 'gpu') # select gpu, to test that it's deselected properly when pitzer is selected select('gpu', from: 'batch_connect_session_context_node_type') @@ -477,10 +477,8 @@ def make_bc_app(dir, form) select('pitzer', from: 'batch_connect_session_context_cluster') options = find_all("#batch_connect_session_context_node_type option") - display_property = { "display" => "none" } - assert_equal "standard", options[0]["innerHTML"] - assert_equal display_property, options[1].style('display') + assert_equal 'display: none;', find_option_style('node_type', 'gpu') # value of node_type has gone back to standard assert_equal 'standard', find('#batch_connect_session_context_node_type').value @@ -522,10 +520,8 @@ def make_bc_app(dir, form) select('owens', from: 'batch_connect_session_context_cluster') options = find_all("#batch_connect_session_context_node_type option") - display_property = { "display" => "block" } - assert_equal "standard", options[0]["innerHTML"] - assert_equal display_property, options[1].style('display') + assert_equal '', find_option_style('node_type', 'gpu') # select gpu, to test that it's deselected properly when pitzer is selected select('gpu', from: 'batch_connect_session_context_node_type') @@ -534,10 +530,8 @@ def make_bc_app(dir, form) select('pitzer', from: 'batch_connect_session_context_cluster') options = find_all("#batch_connect_session_context_node_type option") - display_property = { "display" => "none" } - assert_equal "standard", options[0]["innerHTML"] - assert_equal display_property, options[1].style('display') + assert_equal 'display: none;', find_option_style('node_type', 'gpu') # value of node_type has gone back to standard assert_equal 'standard', find('#batch_connect_session_context_node_type').value