Fix custom choices not being used
This commit is contained in:
parent
b3bb41b322
commit
089dbb663a
3 changed files with 326 additions and 0 deletions
|
|
@ -297,6 +297,11 @@ class CustomFormMixin(FormGeneratorMixin):
|
||||||
)
|
)
|
||||||
elif field_type == "array":
|
elif field_type == "array":
|
||||||
field.widget = DynamicArrayWidget()
|
field.widget = DynamicArrayWidget()
|
||||||
|
elif field_type == "choice":
|
||||||
|
if hasattr(field, "choices") and field.choices:
|
||||||
|
field._controlplane_choices = list(field.choices)
|
||||||
|
if custom_choices := field_config.get("choices"):
|
||||||
|
field.choices = [tuple(choice) for choice in custom_choices]
|
||||||
|
|
||||||
if field_type == "number":
|
if field_type == "number":
|
||||||
min_val = field_config.get("min_value")
|
min_val = field_config.get("min_value")
|
||||||
|
|
@ -330,6 +335,25 @@ class CustomFormMixin(FormGeneratorMixin):
|
||||||
|
|
||||||
return fieldsets
|
return fieldsets
|
||||||
|
|
||||||
|
def clean(self):
|
||||||
|
cleaned_data = super().clean()
|
||||||
|
|
||||||
|
for field_name, field in self.fields.items():
|
||||||
|
if hasattr(field, "_controlplane_choices"):
|
||||||
|
value = cleaned_data.get(field_name)
|
||||||
|
if value:
|
||||||
|
valid_values = [choice[0] for choice in field._controlplane_choices]
|
||||||
|
if value not in valid_values:
|
||||||
|
self.add_error(
|
||||||
|
field_name,
|
||||||
|
forms.ValidationError(
|
||||||
|
f"'{value}' is not a valid choice. "
|
||||||
|
f"Must be one of: {valid_values.join(', ')}"
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
return cleaned_data
|
||||||
|
|
||||||
def get_nested_data(self):
|
def get_nested_data(self):
|
||||||
nested = {}
|
nested = {}
|
||||||
for field_name in self.fields.keys():
|
for field_name in self.fields.keys():
|
||||||
|
|
|
||||||
|
|
@ -218,6 +218,11 @@ class ServiceDefinitionAdminForm(forms.ModelForm):
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
|
|
||||||
|
if field.get("type") == "choice" and field.get("choices"):
|
||||||
|
self._validate_choice_field(
|
||||||
|
field, mapping, spec_schema, "spec", errors
|
||||||
|
)
|
||||||
|
|
||||||
if "name" not in included_mappings:
|
if "name" not in included_mappings:
|
||||||
raise forms.ValidationError(
|
raise forms.ValidationError(
|
||||||
{
|
{
|
||||||
|
|
@ -230,6 +235,62 @@ class ServiceDefinitionAdminForm(forms.ModelForm):
|
||||||
if errors:
|
if errors:
|
||||||
raise forms.ValidationError({"form_config": errors})
|
raise forms.ValidationError({"form_config": errors})
|
||||||
|
|
||||||
|
def _validate_choice_field(self, field, mapping, spec_schema, prefix, errors):
|
||||||
|
if not mapping:
|
||||||
|
return
|
||||||
|
|
||||||
|
field_schema = self._get_field_schema(spec_schema, mapping, prefix)
|
||||||
|
if not field_schema:
|
||||||
|
return
|
||||||
|
|
||||||
|
control_plane_choices = field_schema.get("enum", [])
|
||||||
|
if not control_plane_choices:
|
||||||
|
return
|
||||||
|
|
||||||
|
custom_choices = field.get("choices", [])
|
||||||
|
custom_choice_values = [choice[0] for choice in custom_choices]
|
||||||
|
|
||||||
|
invalid_choices = [
|
||||||
|
value
|
||||||
|
for value in custom_choice_values
|
||||||
|
if value not in control_plane_choices
|
||||||
|
]
|
||||||
|
|
||||||
|
if invalid_choices:
|
||||||
|
field_name = field.get("label", mapping)
|
||||||
|
errors.append(
|
||||||
|
_(
|
||||||
|
"Field '{}' has invalid choice values: {}. "
|
||||||
|
"Valid choices from control plane are: {}"
|
||||||
|
).format(
|
||||||
|
field_name,
|
||||||
|
", ".join(f"'{c}'" for c in invalid_choices),
|
||||||
|
", ".join(f"'{c}'" for c in control_plane_choices),
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
def _get_field_schema(self, schema, field_path, prefix):
|
||||||
|
if not field_path or not schema:
|
||||||
|
return None
|
||||||
|
|
||||||
|
if field_path.startswith(prefix + "."):
|
||||||
|
field_path = field_path[len(prefix) + 1 :]
|
||||||
|
|
||||||
|
parts = field_path.split(".")
|
||||||
|
current_schema = schema
|
||||||
|
|
||||||
|
for part in parts:
|
||||||
|
if not isinstance(current_schema, dict):
|
||||||
|
return None
|
||||||
|
|
||||||
|
properties = current_schema.get("properties", {})
|
||||||
|
if part not in properties:
|
||||||
|
return None
|
||||||
|
|
||||||
|
current_schema = properties[part]
|
||||||
|
|
||||||
|
return current_schema
|
||||||
|
|
||||||
def _extract_field_paths(self, schema, prefix=""):
|
def _extract_field_paths(self, schema, prefix=""):
|
||||||
paths = set()
|
paths = set()
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -216,3 +216,244 @@ def test_form_config_schema_validates_full_config():
|
||||||
]
|
]
|
||||||
}
|
}
|
||||||
jsonschema.validate(instance=full_config, schema=schema)
|
jsonschema.validate(instance=full_config, schema=schema)
|
||||||
|
|
||||||
|
|
||||||
|
def test_choice_field_uses_custom_choices_from_form_config():
|
||||||
|
"""Test that choice fields use custom choices when provided in form_config"""
|
||||||
|
|
||||||
|
class TestModel(models.Model):
|
||||||
|
name = models.CharField(max_length=100)
|
||||||
|
environment = models.CharField(
|
||||||
|
max_length=20,
|
||||||
|
choices=[
|
||||||
|
("dev", "Development"),
|
||||||
|
("staging", "Staging"),
|
||||||
|
("prod", "Production"),
|
||||||
|
("test", "Testing"),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
|
||||||
|
class Meta:
|
||||||
|
app_label = "test"
|
||||||
|
|
||||||
|
form_config = {
|
||||||
|
"fieldsets": [
|
||||||
|
{
|
||||||
|
"title": "General",
|
||||||
|
"fields": [
|
||||||
|
{
|
||||||
|
"type": "text",
|
||||||
|
"label": "Name",
|
||||||
|
"controlplane_field_mapping": "name",
|
||||||
|
"required": True,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"type": "choice",
|
||||||
|
"label": "Environment",
|
||||||
|
"controlplane_field_mapping": "environment",
|
||||||
|
"required": True,
|
||||||
|
"choices": [["dev", "Development"], ["prod", "Production"]],
|
||||||
|
},
|
||||||
|
],
|
||||||
|
}
|
||||||
|
]
|
||||||
|
}
|
||||||
|
|
||||||
|
form_class = generate_custom_form_class(form_config, TestModel)
|
||||||
|
form = form_class()
|
||||||
|
|
||||||
|
environment_field = form.fields["environment"]
|
||||||
|
assert list(environment_field.choices) == [
|
||||||
|
("dev", "Development"),
|
||||||
|
("prod", "Production"),
|
||||||
|
]
|
||||||
|
|
||||||
|
assert hasattr(environment_field, "_controlplane_choices")
|
||||||
|
assert len(environment_field._controlplane_choices) == 5 # 4 choices + empty choice
|
||||||
|
|
||||||
|
|
||||||
|
def test_choice_field_uses_control_plane_choices_when_no_custom_choices():
|
||||||
|
|
||||||
|
class TestModel(models.Model):
|
||||||
|
name = models.CharField(max_length=100)
|
||||||
|
environment = models.CharField(
|
||||||
|
max_length=20,
|
||||||
|
choices=[
|
||||||
|
("dev", "Development"),
|
||||||
|
("staging", "Staging"),
|
||||||
|
("prod", "Production"),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
|
||||||
|
class Meta:
|
||||||
|
app_label = "test"
|
||||||
|
|
||||||
|
form_config = {
|
||||||
|
"fieldsets": [
|
||||||
|
{
|
||||||
|
"title": "General",
|
||||||
|
"fields": [
|
||||||
|
{
|
||||||
|
"type": "text",
|
||||||
|
"label": "Name",
|
||||||
|
"controlplane_field_mapping": "name",
|
||||||
|
"required": True,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"type": "choice",
|
||||||
|
"label": "Environment",
|
||||||
|
"controlplane_field_mapping": "environment",
|
||||||
|
"required": True,
|
||||||
|
},
|
||||||
|
],
|
||||||
|
}
|
||||||
|
]
|
||||||
|
}
|
||||||
|
|
||||||
|
form_class = generate_custom_form_class(form_config, TestModel)
|
||||||
|
form = form_class()
|
||||||
|
|
||||||
|
environment_field = form.fields["environment"]
|
||||||
|
choices_list = list(environment_field.choices)
|
||||||
|
assert len(choices_list) == 4 # 3 choices + empty choice
|
||||||
|
assert ("dev", "Development") in choices_list
|
||||||
|
|
||||||
|
|
||||||
|
def test_choice_field_validates_against_control_plane_choices():
|
||||||
|
class TestModel(models.Model):
|
||||||
|
name = models.CharField(max_length=100)
|
||||||
|
environment = models.CharField(
|
||||||
|
max_length=20,
|
||||||
|
choices=[
|
||||||
|
("dev", "Development"),
|
||||||
|
("staging", "Staging"),
|
||||||
|
("prod", "Production"),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
|
||||||
|
class Meta:
|
||||||
|
app_label = "test"
|
||||||
|
|
||||||
|
form_config = {
|
||||||
|
"fieldsets": [
|
||||||
|
{
|
||||||
|
"title": "General",
|
||||||
|
"fields": [
|
||||||
|
{
|
||||||
|
"type": "text",
|
||||||
|
"label": "Name",
|
||||||
|
"controlplane_field_mapping": "name",
|
||||||
|
"required": True,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"type": "choice",
|
||||||
|
"label": "Environment",
|
||||||
|
"controlplane_field_mapping": "environment",
|
||||||
|
"required": True,
|
||||||
|
"choices": [["dev", "Development"], ["prod", "Production"]],
|
||||||
|
},
|
||||||
|
],
|
||||||
|
}
|
||||||
|
]
|
||||||
|
}
|
||||||
|
|
||||||
|
form_class = generate_custom_form_class(form_config, TestModel)
|
||||||
|
|
||||||
|
form = form_class(data={"name": "test-service", "environment": "dev"})
|
||||||
|
form.fields["context"].required = False # Skip context validation
|
||||||
|
assert form.is_valid(), f"Form should be valid but has errors: {form.errors}"
|
||||||
|
|
||||||
|
form = form_class(data={"name": "test-service", "environment": "prod"})
|
||||||
|
form.fields["context"].required = False # Skip context validation
|
||||||
|
assert form.is_valid(), f"Form should be valid but has errors: {form.errors}"
|
||||||
|
|
||||||
|
form = form_class(data={"name": "test-service", "environment": "invalid"})
|
||||||
|
form.fields["context"].required = False # Skip context validation
|
||||||
|
assert not form.is_valid()
|
||||||
|
assert "environment" in form.errors
|
||||||
|
|
||||||
|
|
||||||
|
def test_admin_form_validates_choice_values_against_schema():
|
||||||
|
from servala.core.forms import ServiceDefinitionAdminForm
|
||||||
|
|
||||||
|
form = ServiceDefinitionAdminForm()
|
||||||
|
mock_crd = Mock()
|
||||||
|
mock_crd.resource_schema = {
|
||||||
|
"properties": {
|
||||||
|
"spec": {
|
||||||
|
"properties": {
|
||||||
|
"environment": {
|
||||||
|
"type": "string",
|
||||||
|
"enum": ["dev", "staging", "prod"],
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
valid_form_config = {
|
||||||
|
"fieldsets": [
|
||||||
|
{
|
||||||
|
"fields": [
|
||||||
|
{
|
||||||
|
"type": "text",
|
||||||
|
"label": "Name",
|
||||||
|
"controlplane_field_mapping": "name",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"type": "choice",
|
||||||
|
"label": "Environment",
|
||||||
|
"controlplane_field_mapping": "spec.environment",
|
||||||
|
"choices": [["dev", "Development"], ["prod", "Production"]],
|
||||||
|
},
|
||||||
|
]
|
||||||
|
}
|
||||||
|
]
|
||||||
|
}
|
||||||
|
|
||||||
|
spec_schema = mock_crd.resource_schema["properties"]["spec"]
|
||||||
|
errors = []
|
||||||
|
|
||||||
|
for field in valid_form_config["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}"
|
||||||
|
|
||||||
|
invalid_form_config = {
|
||||||
|
"fieldsets": [
|
||||||
|
{
|
||||||
|
"fields": [
|
||||||
|
{
|
||||||
|
"type": "text",
|
||||||
|
"label": "Name",
|
||||||
|
"controlplane_field_mapping": "name",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"type": "choice",
|
||||||
|
"label": "Environment",
|
||||||
|
"controlplane_field_mapping": "spec.environment",
|
||||||
|
"choices": [
|
||||||
|
["dev", "Development"],
|
||||||
|
["invalid", "Invalid Environment"],
|
||||||
|
],
|
||||||
|
},
|
||||||
|
]
|
||||||
|
}
|
||||||
|
]
|
||||||
|
}
|
||||||
|
|
||||||
|
errors = []
|
||||||
|
|
||||||
|
for field in invalid_form_config["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, "Expected validation errors but got none"
|
||||||
|
error_message = str(errors[0])
|
||||||
|
assert "invalid" in error_message.lower()
|
||||||
|
assert "Environment" in error_message
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue