From 6182b36daf16bcd4f7b4e50696219b3ec1e83494 Mon Sep 17 00:00:00 2001 From: Tobias Kunze Date: Fri, 7 Nov 2025 11:36:28 +0100 Subject: [PATCH] Implicitly fix admin choices configuration --- src/servala/core/forms.py | 28 +++++- src/tests/test_form_config.py | 155 ++++++++++++++++++++++++++++++++++ 2 files changed, 181 insertions(+), 2 deletions(-) diff --git a/src/servala/core/forms.py b/src/servala/core/forms.py index 116e4d9..076a9d9 100644 --- a/src/servala/core/forms.py +++ b/src/servala/core/forms.py @@ -278,6 +278,32 @@ class ServiceDefinitionAdminForm(forms.ModelForm): if not mapping: return + field_name = field.get("label", mapping) + custom_choices = field.get("choices", []) + + # Single-element choices [value] are transformed to [value, value] + for i, choice in enumerate(custom_choices): + if not isinstance(choice, (list, tuple)): + errors.append( + _( + "Field '{}': Choice at index {} must be a list or tuple, " + "but got: {}" + ).format(field_name, i, repr(choice)) + ) + return + + choice_len = len(choice) + if choice_len == 1: + custom_choices[i] = [choice[0], choice[0]] + elif choice_len == 0 or choice_len > 2: + errors.append( + _( + "Field '{}': Choice at index {} must have 1 or 2 elements " + "(got {}): {}" + ).format(field_name, i, choice_len, repr(choice)) + ) + return + field_schema = self._get_field_schema(spec_schema, mapping, prefix) if not field_schema: return @@ -286,7 +312,6 @@ class ServiceDefinitionAdminForm(forms.ModelForm): if not control_plane_choices: return - custom_choices = field.get("choices", []) custom_choice_values = [choice[0] for choice in custom_choices] invalid_choices = [ @@ -296,7 +321,6 @@ class ServiceDefinitionAdminForm(forms.ModelForm): ] if invalid_choices: - field_name = field.get("label", mapping) errors.append( _( "Field '{}' has invalid choice values: {}. " diff --git a/src/tests/test_form_config.py b/src/tests/test_form_config.py index feede60..ba44d72 100644 --- a/src/tests/test_form_config.py +++ b/src/tests/test_form_config.py @@ -749,3 +749,158 @@ def test_form_config_handles_empty_string_as_none(): normalized_config = form._normalize_form_config_types(config_with_empty_strings) field = normalized_config["fieldsets"][0]["fields"][0] assert field["max_length"] is None + + +def test_single_element_choices_are_normalized(): + form = ServiceDefinitionAdminForm() + mock_crd = Mock() + mock_crd.resource_schema = { + "properties": { + "spec": { + "properties": { + "version": { + "type": "string", + "enum": ["6.2", "7.0", "7.2"], + } + } + } + } + } + + config_with_single_choices = { + "fieldsets": [ + { + "fields": [ + { + "type": "text", + "label": "Name", + "controlplane_field_mapping": "name", + }, + { + "type": "choice", + "label": "Version", + "controlplane_field_mapping": "spec.version", + "choices": [["6.2"]], # Single element - should be transformed + }, + ] + } + ] + } + + spec_schema = mock_crd.resource_schema["properties"]["spec"] + errors = [] + + for field in config_with_single_choices["fieldsets"][0]["fields"]: + if field.get("type") == "choice": + form._validate_choice_field( + field, field["controlplane_field_mapping"], spec_schema, "spec", errors + ) + + assert len(errors) == 0, f"Expected no errors but got: {errors}" + version_field = config_with_single_choices["fieldsets"][0]["fields"][1] + assert version_field["choices"] == [["6.2", "6.2"]] + + +def test_two_element_choices_work_correctly(): + form = ServiceDefinitionAdminForm() + mock_crd = Mock() + mock_crd.resource_schema = { + "properties": { + "spec": { + "properties": { + "version": { + "type": "string", + "enum": ["6.2", "7.0"], + } + } + } + } + } + + config_with_proper_choices = { + "fieldsets": [ + { + "fields": [ + { + "type": "choice", + "label": "Version", + "controlplane_field_mapping": "spec.version", + "choices": [["6.2", "Version 6.2"], ["7.0", "Version 7.0"]], + }, + ] + } + ] + } + + spec_schema = mock_crd.resource_schema["properties"]["spec"] + errors = [] + + for field in config_with_proper_choices["fieldsets"][0]["fields"]: + if field.get("type") == "choice": + form._validate_choice_field( + field, field["controlplane_field_mapping"], spec_schema, "spec", errors + ) + + assert len(errors) == 0, f"Expected no errors but got: {errors}" + version_field = config_with_proper_choices["fieldsets"][0]["fields"][0] + assert version_field["choices"] == [["6.2", "Version 6.2"], ["7.0", "Version 7.0"]] + + +def test_empty_choices_fail_validation(): + form = ServiceDefinitionAdminForm() + config_with_empty_choice = { + "fieldsets": [ + { + "fields": [ + { + "type": "choice", + "label": "Version", + "controlplane_field_mapping": "spec.version", + "choices": [[]], # Empty choice - invalid + }, + ] + } + ] + } + + errors = [] + + for field in config_with_empty_choice["fieldsets"][0]["fields"]: + if field.get("type") == "choice": + form._validate_choice_field( + field, field["controlplane_field_mapping"], {}, "spec", errors + ) + + assert len(errors) > 0 + assert "must have 1 or 2 elements" in str(errors[0]) + + +def test_three_plus_element_choices_fail_validation(): + form = ServiceDefinitionAdminForm() + config_with_long_choice = { + "fieldsets": [ + { + "fields": [ + { + "type": "choice", + "label": "Version", + "controlplane_field_mapping": "spec.version", + "choices": [ + ["6.2", "Version 6.2", "Extra"] + ], # 3 elements - invalid + }, + ] + } + ] + } + + errors = [] + + for field in config_with_long_choice["fieldsets"][0]["fields"]: + if field.get("type") == "choice": + form._validate_choice_field( + field, field["controlplane_field_mapping"], {}, "spec", errors + ) + + assert len(errors) > 0 + assert "must have 1 or 2 elements" in str(errors[0])