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

Refs #756: Handle no choices provided if unlisted_choice is present #778

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Refs #756: Handle no choices provided if unlisted_choice is present #778

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
49c3ef2
add failing test for no choices specified in config
batpad Sep 2, 2023
03219fd
render dropdown only if choices is present
batpad Sep 2, 2023
5ab3750
lintfix
batpad Sep 2, 2023
b829054
fix silly issues with spaces so hidden class gets applied correctly t…
batpad Jan 9, 2024
9e22248
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jan 9, 2024
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: 39 additions & 30 deletions kubespawner/templates/form.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,40 +19,49 @@ <h3>{{ profile.display_name }}</h3>
{%- for k, option in profile.profile_options.items() %}
<div class="js-options-container">
<div class="option">
<label for="profile-option-{{ profile.slug }}--{{ k }}"
class="js-profile-option-label">{{ option.display_name }}</label>
<select name="profile-option-{{ profile.slug }}--{{ k }}"
class="form-control js-profile-option-select">
{%- for k, choice in option['choices'].items() %}
<option value="{{ k }}" {% if choice.default %}selected{% endif %}>{{ choice.display_name }}</option>
{%- endfor %}
{%- if option['unlisted_choice']['enabled'] %}
<option value="unlisted-choice">{{ option['unlisted_choice']['display_name_in_choices'] }}</option>
{%- endif %}
</select>
{%- if option['choices'] %}
<label for="profile-option-{{ profile.slug }}--{{ k }}"
class="js-profile-option-label">{{ option.display_name }}</label>
<select name="profile-option-{{ profile.slug }}--{{ k }}"
class="form-control js-profile-option-select">
{%- for k, choice in option['choices'].items() %}
<option value="{{ k }}" {% if choice.default %}selected{% endif %}>{{ choice.display_name }}</option>
{%- endfor %}
{%- if option['unlisted_choice']['enabled'] %}
<option value="unlisted-choice">{{ option['unlisted_choice']['display_name_in_choices'] }}</option>
{%- endif %}
</select>
{%- endif %}
</div>
{%- if option['unlisted_choice']['enabled'] %}
<div class="option hidden js-other-input-container">
<div class="option js-other-input-container {%- if option['choices'] %}hidden{%- endif %}">
<label for="profile-option-{{ profile.slug }}--{{ k }}--unlisted-choice">
{{ option['unlisted_choice']['display_name'] }}
</label>
<input data-name="profile-option-{{ profile.slug }}--{{ k }}--unlisted-choice"
{%- if option['unlisted_choice']['validation_regex'] %}
pattern="{{ option['unlisted_choice']['validation_regex'] }}"
{%- endif %}
title="{{ option['unlisted_choice']['validation_message'] }}"
class="form-control js-other-input" />
</div>
{%- endif %}
</div>
{%- endfor %}
</div>
{%- endif %}
</div>
</label>
{%- endfor %}
</div>
<script>
<input
{#- FIXME: This has gotten a bit ugly, ideally we would handle a lot of this
logic all in Javascript and not need this conditional attribute rendering #}
{%- if option['choices'] %}
data-name="profile-option-{{ profile.slug }}--{{ k }}--unlisted-choice"
{%- else %}
name="profile-option-{{ profile.slug }}--{{ k }}--unlisted-choice"
{%- endif %}
{%- if option['unlisted_choice']['validation_regex'] %}
pattern="{{ option['unlisted_choice']['validation_regex'] }}"
{%- endif %}
title="{{ option['unlisted_choice']['validation_message'] }}"
class="form-control js-other-input" />
</div>
{%- endif %}
</div>
{%- endfor %}
</div>
{%- endif %}
</div>
</label>
{%- endfor %}
</div>
<script>
$('.js-profile-option-select, .js-profile-option-label').click(function() {
// we need this bit of JS to select the profile when a <select> inside is clicked.
$(this).parents('.js-profile-label')
Expand Down Expand Up @@ -95,4 +104,4 @@ <h3>{{ profile.display_name }}</h3>
$('.js-profile-option-select').trigger('change');
});

</script>
</script>
37 changes: 37 additions & 0 deletions tests/test_spawner.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -972,6 +972,20 @@ async def test_expansion_hyphens():
},
},
},
{
'display_name': 'Test with unlisted_choice and no choices',
'slug': 'no-choices',
'profile_options': {
'image': {
'display_name': 'Image',
'unlisted_choice': {
'enabled': True,
'display_name': 'Image Location',
'kubespawner_override': {'image': '{value}'},
},
},
},
},
]


Expand Down Expand Up @@ -1109,6 +1123,29 @@ async def test_user_options_set_from_form_no_regex():
assert getattr(spawner, 'image') == 'invalid/foo:latest'


async def test_user_options_unlisted_choice_without_choices():
"""
Test that if a profile does not have `choices` but has an unlisted_choice definition,
everything still works.
"""
spawner = KubeSpawner(_mock=True)
spawner.profile_list = _test_profiles
await spawner.get_options_form()
spawner.user_options = spawner.options_from_form(
{
'profile': [_test_profiles[5]['slug']],
'profile-option-no-choices--image--unlisted-choice': ['pangeo/test:1.2.3'],
}
)
assert spawner.user_options == {
'image--unlisted-choice': 'pangeo/test:1.2.3',
'profile': _test_profiles[5]['slug'],
}
assert spawner.cpu_limit is None
await spawner.load_user_options()
assert getattr(spawner, 'image') == 'pangeo/test:1.2.3'


async def test_kubespawner_override():
spawner = KubeSpawner(_mock=True)
spawner.profile_list = _test_profiles
Expand Down