From c7c22aa265b7e234e0799816d98c78f79e86911b Mon Sep 17 00:00:00 2001 From: Tobias Kunze Date: Mon, 10 Nov 2025 14:02:34 +0100 Subject: [PATCH] Add default config --- src/servala/core/crd/forms.py | 33 ++++++- src/servala/core/forms.py | 36 +++++-- src/tests/test_form_config.py | 180 ++++++++++++++++++++++++++++++++++ 3 files changed, 238 insertions(+), 11 deletions(-) diff --git a/src/servala/core/crd/forms.py b/src/servala/core/crd/forms.py index 99e2737..bdb8ed6 100644 --- a/src/servala/core/crd/forms.py +++ b/src/servala/core/crd/forms.py @@ -6,6 +6,27 @@ from servala.core.crd.utils import deslugify from servala.core.models import ControlPlaneCRD from servala.frontend.forms.widgets import DynamicArrayWidget +# Fields that must be present in every form +MANDATORY_FIELDS = ["name"] + +# Default field configurations - fields that can be included with just a mapping +# to avoid administrators having to duplicate common information +DEFAULT_FIELD_CONFIGS = { + "name": { + "type": "text", + "label": "Instance Name", + "help_text": "Unique name for the new instance", + "required": True, + "max_length": 63, + }, + "spec.parameters.service.fqdn": { + "type": "array", + "label": "FQDNs", + "help_text": "Domain names for accessing this service", + "required": False, + }, +} + class FormGeneratorMixin: """Shared base class for ModelForm classes based on our generated CRD models. @@ -278,12 +299,20 @@ class CustomFormMixin(FormGeneratorMixin): def _apply_field_config(self): for fieldset in self.form_config.get("fieldsets", []): - for field_config in fieldset.get("fields", []): - field_name = field_config.get("controlplane_field_mapping") + for fc in fieldset.get("fields", []): + field_name = fc.get("controlplane_field_mapping") if field_name not in self.fields: continue + field_config = fc.copy() + # Merge with defaults if field has default config + if field_name in DEFAULT_FIELD_CONFIGS: + field_config = DEFAULT_FIELD_CONFIGS[field_name].copy() + for key, value in fc.items(): + if value or (value is False): + field_config[key] = value + field = self.fields[field_name] field_type = field_config.get("type") diff --git a/src/servala/core/forms.py b/src/servala/core/forms.py index 076a9d9..69132fc 100644 --- a/src/servala/core/forms.py +++ b/src/servala/core/forms.py @@ -6,6 +6,7 @@ from django import forms from django.utils.translation import gettext_lazy as _ from django_jsonform.widgets import JSONFormWidget +from servala.core.crd.forms import DEFAULT_FIELD_CONFIGS, MANDATORY_FIELDS from servala.core.models import ControlPlane, ServiceDefinition CONTROL_PLANE_USER_INFO_SCHEMA = { @@ -244,8 +245,26 @@ class ServiceDefinitionAdminForm(forms.ModelForm): for field in fieldset.get("fields", []): mapping = field.get("controlplane_field_mapping") included_mappings.add(mapping) + + # Validate that fields without defaults have required properties + if mapping not in DEFAULT_FIELD_CONFIGS: + if not field.get("label"): + errors.append( + _( + "Field with mapping '{}' must have a 'label' property " + "(or use a mapping with default config)" + ).format(mapping) + ) + if not field.get("type"): + errors.append( + _( + "Field with mapping '{}' must have a 'type' property " + "(or use a mapping with default config)" + ).format(mapping) + ) + if mapping and mapping not in valid_paths: - field_name = field.get("name", mapping) + field_name = field.get("label", field.get("name", mapping)) errors.append( _( "Field '{}' has invalid mapping '{}'. Valid paths are: {}" @@ -262,14 +281,13 @@ class ServiceDefinitionAdminForm(forms.ModelForm): field, mapping, spec_schema, "spec", errors ) - if "name" not in included_mappings: - raise forms.ValidationError( - { - "form_config": _( - "You must include a `name` field in the custom form config." - ) - } - ) + for mandatory_field in MANDATORY_FIELDS: + if mandatory_field not in included_mappings: + errors.append( + _( + "Required field '{}' must be included in the form configuration" + ).format(mandatory_field) + ) if errors: raise forms.ValidationError({"form_config": errors}) diff --git a/src/tests/test_form_config.py b/src/tests/test_form_config.py index ba44d72..7188d61 100644 --- a/src/tests/test_form_config.py +++ b/src/tests/test_form_config.py @@ -5,6 +5,7 @@ from django.core.validators import MaxValueValidator, MinValueValidator from django.db import models from servala.core.crd import generate_custom_form_class +from servala.core.crd.forms import DEFAULT_FIELD_CONFIGS, MANDATORY_FIELDS from servala.core.forms import ServiceDefinitionAdminForm from servala.core.models import ControlPlaneCRD @@ -904,3 +905,182 @@ def test_three_plus_element_choices_fail_validation(): assert len(errors) > 0 assert "must have 1 or 2 elements" in str(errors[0]) + + +def test_field_with_default_config_only_needs_mapping(): + + class TestModel(models.Model): + name = models.CharField(max_length=100) + + class Meta: + app_label = "test" + + minimal_config = { + "fieldsets": [ + { + "fields": [ + { + "controlplane_field_mapping": "name", + }, + ] + } + ] + } + + form_class = generate_custom_form_class(minimal_config, TestModel) + form = form_class() + + name_field = form.fields["name"] + assert name_field.label == DEFAULT_FIELD_CONFIGS["name"]["label"] + assert name_field.help_text == DEFAULT_FIELD_CONFIGS["name"]["help_text"] + assert name_field.required == DEFAULT_FIELD_CONFIGS["name"]["required"] + + +def test_field_with_default_config_can_override_defaults(): + + class TestModel(models.Model): + name = models.CharField(max_length=100) + + class Meta: + app_label = "test" + + override_config = { + "fieldsets": [ + { + "fields": [ + { + "controlplane_field_mapping": "name", + "label": "Custom Name Label", + "required": False, + }, + ] + } + ] + } + + form_class = generate_custom_form_class(override_config, TestModel) + form = form_class() + + name_field = form.fields["name"] + assert name_field.label == "Custom Name Label" + assert name_field.required is False + assert name_field.help_text == DEFAULT_FIELD_CONFIGS["name"]["help_text"] + + +def test_admin_form_validates_mandatory_fields_present(): + + mock_crd = Mock() + mock_crd.resource_schema = { + "properties": { + "spec": { + "properties": { + "environment": { + "type": "string", + "enum": ["dev", "prod"], + } + } + } + } + } + + config_without_name = { + "fieldsets": [ + { + "fields": [ + { + "type": "choice", + "label": "Environment", + "controlplane_field_mapping": "spec.environment", + "choices": [["dev", "Development"]], + }, + ] + } + ] + } + + errors = [] + included_mappings = set() + for fieldset in config_without_name.get("fieldsets", []): + for field in fieldset.get("fields", []): + mapping = field.get("controlplane_field_mapping") + included_mappings.add(mapping) + + for mandatory_field in MANDATORY_FIELDS: + if mandatory_field not in included_mappings: + errors.append(f"Required field '{mandatory_field}' must be included") + + assert len(errors) > 0 + assert "name" in str(errors[0]).lower() + + +def test_admin_form_validates_fields_without_defaults_need_label_and_type(): + config_with_incomplete_field = { + "fieldsets": [ + { + "fields": [ + {"controlplane_field_mapping": "name"}, # Has defaults - OK + { + "controlplane_field_mapping": "spec.unknown", # No defaults + # Missing label and type + }, + ] + } + ] + } + + errors = [] + + for fieldset in config_with_incomplete_field.get("fieldsets", []): + for field in fieldset.get("fields", []): + mapping = field.get("controlplane_field_mapping") + + if mapping not in DEFAULT_FIELD_CONFIGS: + if not field.get("label"): + errors.append( + f"Field with mapping '{mapping}' must have a 'label' property" + ) + if not field.get("type"): + errors.append( + f"Field with mapping '{mapping}' must have a 'type' property" + ) + + assert len(errors) == 2 + assert any("label" in str(e) for e in errors) + assert any("type" in str(e) for e in errors) + + +def test_empty_values_dont_override_default_configs(): + + class TestModel(models.Model): + name = models.CharField(max_length=100) + + class Meta: + app_label = "test" + + admin_form_config = { + "fieldsets": [ + { + "fields": [ + { + "controlplane_field_mapping": "name", + "type": "", + "label": "", + "help_text": None, + "max_length": None, + "required": False, + }, + ] + } + ] + } + + form_class = generate_custom_form_class(admin_form_config, TestModel) + form = form_class() + + name_field = form.fields["name"] + + assert name_field.label == DEFAULT_FIELD_CONFIGS["name"]["label"] + assert name_field.help_text == DEFAULT_FIELD_CONFIGS["name"]["help_text"] + assert name_field.max_length == DEFAULT_FIELD_CONFIGS["name"]["max_length"] + + assert name_field.required is False # Was overridden by explicit False