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.

Sign up for GitHub

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

Data-set-* and Data-suggest-* #4044

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Data-set-* and Data-suggest-* #4044

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fb147f8
Data-set-* and Data-suggest-*
HazelGrant Jan 2, 2025
6000ed9
Uses data-suggest in fixture where that's the intended behavior for t…
HazelGrant Jan 2, 2025
635de27
Refactoring
HazelGrant Jan 6, 2025
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
69 changes: 53 additions & 16 deletions apps/dashboard/app/javascript/dynamic_forms.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ const exclusiveOptionForHandlerCache = {};
// simples array of string ids for elements that have a handler
const minMaxHandlerCache = [];
const setHandlerCache = [];
const suggestHandlerCache = [];

// hide handler cache is a map in the form '{ from: [hideThing1, hideThing2] }'
const hideHandlerCache = {};
const labelHandlerCache = {};
Expand All @@ -25,6 +27,7 @@ const labelHandlerCache = {};
// for different directives.
const minMaxLookup = {};
const setValueLookup = {};
const suggestValueLookup = {};
const hideLookup = {};
const labelLookup = {};

Expand Down Expand Up @@ -142,8 +145,8 @@ function makeChangeHandlers(prefix){
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')) {
addSetHandler(element['id'], opt.value, key, data[key]);
} else if(key.startsWith('set') || key.startsWith('suggest')) {
addSuggestAndSetHandlers(element['id'], opt.value, key, data[key]);
} else if(key.startsWith('hide')) {
addHideHandler(element['id'], opt.value, key, data[key]);
} else if(key.startsWith('label')) {
Expand Down Expand Up @@ -296,45 +299,79 @@ function addMinMaxForHandler(subjectId, option, key, configValue) {
* data-set-account: 'phy3005'
* ]
*/
function addSetHandler(optionId, option, key, configValue) {
const k = key.replace(/^set/,'');
function addSuggestAndSetHandlers(optionId, option, key, configValue) {
let k = '';

if (key.startsWith('set')) {
k = key.replace(/^set/,'');
} else if (key.startsWith('suggest')) {
k = key.replace(/^suggest/,'');
}

const id = String(idFromToken(k));
if(id === 'undefined') return;

// id is account. optionId is classroom
let cacheKey = `${id}_${optionId}`
if(setValueLookup[cacheKey] === undefined) setValueLookup[cacheKey] = new Table(optionId, undefined);
const table = setValueLookup[cacheKey];

let lookup = '';
let cache = '';

if (key.startsWith('set')) {
lookup = setValueLookup;
cache = setHandlerCache;
} else if (key.startsWith('suggest')) {
lookup = suggestValueLookup;
cache = suggestHandlerCache;
}

if(lookup[cacheKey] === undefined) lookup[cacheKey] = new Table(optionId, undefined);
const table = lookup[cacheKey];
table.put(option, undefined, configValue);

if(!setHandlerCache.includes(cacheKey)) {
if(!cache.includes(cacheKey)) {
const changeElement = $(`#${optionId}`);

changeElement.on('change', (event) => {
setValue(event, id);
setOrSuggestValue(event, id, key);
});

setHandlerCache.push(cacheKey);
cache.push(cacheKey);
}

setValue({ target: document.querySelector(`#${optionId}`) }, id);
setOrSuggestValue({ target: document.querySelector(`#${optionId}`) }, id, key)
}

function setValue(event, changeId) {
function setOrSuggestValue(event, changeId, key) {
const chosenVal = event.target.value;
const cacheKey = `${changeId}_${event.target['id']}`
const table = setValueLookup[cacheKey];
if (table === undefined) return;

const changeVal = table.get(chosenVal, undefined);
let table = undefined;

if (key.startsWith('set')) {
table = setValueLookup[cacheKey];
} else if (key.startsWith('suggest')) {
table = suggestValueLookup[cacheKey];
}
if (table === undefined) return;

const changeVal = table.get(chosenVal, undefined);

const element = document.getElementById(changeId);

if(changeVal !== undefined) {
const element = document.getElementById(changeId);
if(element['type'] == 'checkbox') {
setCheckboxValue(element, changeVal);
} else {
element.value = changeVal;
}

if (key.startsWith('set')) {
element.disabled = true;
}
} else {
if (key.startsWith('set')) {
element.disabled = false;
}
}
}

Expand Down
22 changes: 11 additions & 11 deletions apps/dashboard/test/fixtures/sys_with_gateway_apps/bc_jupyter/form.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,34 +127,34 @@ attributes:
"2.7",
data-option-for-node-type-advanced: false,
data-option-for-node-type-other-40ish-option: false,
data-set-bc-account-other: 'other_account_python27',
data-set-bc-account: 'python27'
data-suggest-bc-account-other: 'other_account_python27',
data-suggest-bc-account: 'python27'
Comment on lines -130 to +131
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You changed all these data-set to data-suggest. This implies to me that there's some compatibility issue with data-set, is there?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

data-set- was behaving how data-suggest- behaves now in some tests. So data-set- was not disabling the set select option before. This does make it a breaking change.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I'll have to look over the implementation, but I don't think we should break data-set directives. We definitely can't in 4.0, but I wouldn't want to in future versions either really.

]
- [
"3.1",
data-option-for-node-type-advanced: false,
data-option-for-node-type-other-40ish-option: false,
data-set-bc-account-other: 'other_account_python31',
data-set-bc-account: 'python31'
data-suggest-bc-account-other: 'other_account_python31',
data-suggest-bc-account: 'python31'
]
- [
"3.2",
data-option-for-node-type-advanced: false,
data-option-for-node-type-other-40ish-option: false,
data-set-bc-account-other: 'other_account_python32',
data-set-bc-account: 'python32'
data-suggest-bc-account-other: 'other_account_python32',
data-suggest-bc-account: 'python32'
]
- [
"3.6",
data-set-hidden-change-thing: 'python36'
data-suggest-hidden-change-thing: 'python36'
]
- [
"3.7",
data-set-hidden-change-thing: 'python37',
data-suggest-hidden-change-thing: 'python37',
]
- [
"4.0.nightly",
data-set-hidden-change-thing: 'python4nightly',
data-suggest-hidden-change-thing: 'python4nightly',
]
cuda_version:
widget: select
Expand Down Expand Up @@ -184,13 +184,13 @@ attributes:
'medium', 'medium',
data-option-for-classroom-astronomy-5678: false,
data-option-for-classroom-astronomy-with/other-characters/8846.31.4: false,
data-set-checkbox-test: 0,
data-suggest-checkbox-test: 0,
]
- [
'large', 'large',
data-option-for-classroom-astronomy-5678: false,
data-option-for-classroom-astronomy-with/other-characters/8846.31.4: false,
data-set-checkbox-test: 1,
data-suggest-checkbox-test: 1,
data-option-for-classroom-123ABC: false,
data-option-for-classroom-456def: false,
]
Expand Down
102 changes: 102 additions & 0 deletions apps/dashboard/test/system/batch_connect_widgets_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -537,4 +537,106 @@ def make_bc_app(dir, form)
assert_equal 'standard', find('#batch_connect_session_context_node_type').value
end
end

test 'data-set-' 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:
- project
- node_type
attributes:
project:
widget: 'select'
options:
- ['project1', data-set-node-type: 'standard']
- ['project2', data-set-node-type: 'gpu']
- 'project3'
node_type:
widget: 'select'
options:
- 'standard'
- 'gpu'

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')

select('project1', from: 'batch_connect_session_context_project')

node_type = find('#batch_connect_session_context_node_type')

assert_equal 'standard', node_type.value
assert node_type.disabled?

select('project2', from: 'batch_connect_session_context_project')

assert_equal 'gpu', node_type.value
assert node_type.disabled?

select('project3', from: 'batch_connect_session_context_project')

refute node_type.disabled?
end
end

test 'data-suggest-' 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:
- project
- node_type
attributes:
project:
widget: 'select'
options:
- ['project1', data-suggest-node-type: 'standard']
- ['project2', data-suggest-node-type: 'gpu']
- 'project3'
node_type:
widget: 'select'
options:
- 'standard'
- 'gpu'

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')

select('project1', from: 'batch_connect_session_context_project')

node_type = find('#batch_connect_session_context_node_type')

assert_equal 'standard', node_type.value
refute node_type.disabled?

select('project2', from: 'batch_connect_session_context_project')

assert_equal 'gpu', node_type.value
refute node_type.disabled?

select('project3', from: 'batch_connect_session_context_project')

refute node_type.disabled?
end
end
end
Loading