diff --git a/src/servala/core/crd/forms.py b/src/servala/core/crd/forms.py index 659684e..99e2737 100644 --- a/src/servala/core/crd/forms.py +++ b/src/servala/core/crd/forms.py @@ -6,27 +6,6 @@ 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. @@ -299,20 +278,12 @@ class CustomFormMixin(FormGeneratorMixin): def _apply_field_config(self): for fieldset in self.form_config.get("fieldsets", []): - for fc in fieldset.get("fields", []): - field_name = fc.get("controlplane_field_mapping") + for field_config in fieldset.get("fields", []): + field_name = field_config.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") @@ -350,13 +321,6 @@ class CustomFormMixin(FormGeneratorMixin): if "default_value" in field_config and field.initial is None: field.initial = field_config["default_value"] - if field_type in ("text", "textarea") and field_config.get( - "max_length" - ): - field.max_length = field_config.get("max_length") - if hasattr(field.widget, "attrs"): - field.widget.attrs["maxlength"] = field_config.get("max_length") - field.controlplane_field_mapping = field_name def get_fieldsets(self): diff --git a/src/servala/core/forms.py b/src/servala/core/forms.py index 69132fc..076a9d9 100644 --- a/src/servala/core/forms.py +++ b/src/servala/core/forms.py @@ -6,7 +6,6 @@ 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 = { @@ -245,26 +244,8 @@ 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("label", field.get("name", mapping)) + field_name = field.get("name", mapping) errors.append( _( "Field '{}' has invalid mapping '{}'. Valid paths are: {}" @@ -281,13 +262,14 @@ class ServiceDefinitionAdminForm(forms.ModelForm): field, mapping, spec_schema, "spec", errors ) - 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 "name" not in included_mappings: + raise forms.ValidationError( + { + "form_config": _( + "You must include a `name` field in the custom form config." + ) + } + ) if errors: raise forms.ValidationError({"form_config": errors}) diff --git a/src/servala/static/js/bootstrap-tabs.js b/src/servala/static/js/bootstrap-tabs.js index 1d9eb89..d382475 100644 --- a/src/servala/static/js/bootstrap-tabs.js +++ b/src/servala/static/js/bootstrap-tabs.js @@ -3,22 +3,15 @@ (function() { 'use strict'; - console.log("Loading bootstrap tab setup") - const initBootstrapTabs = () => { - console.log("setting up bootstrap tabs") const customTabList = document.querySelectorAll('#myTab button[data-bs-toggle="tab"]'); - console.log("found custom tabs", customTabList) customTabList.forEach(function(tabButton) { new bootstrap.Tab(tabButton); - console.log("setting up custom tab", tabButton) }); const expertTabList = document.querySelectorAll('#expertTab button[data-bs-toggle="tab"]'); - console.log("found expert tabs", expertTabList) expertTabList.forEach(function(tabButton) { new bootstrap.Tab(tabButton); - console.log("setting up expert tab", tabButton) }); } diff --git a/src/servala/static/js/fqdn.js b/src/servala/static/js/fqdn.js index ec70dad..178c586 100644 --- a/src/servala/static/js/fqdn.js +++ b/src/servala/static/js/fqdn.js @@ -1,9 +1,7 @@ const initializeFqdnGeneration = (prefix) => { const nameField = document.querySelector(`input#id_${prefix}-name`); - const fqdnFieldContainer = document.getElementById(`${prefix}-spec.parameters.service.fqdn_container`) - if (!nameField || !fqdnFieldContainer) return - const fqdnField = fqdnFieldContainer.querySelector('input.array-item-input'); + const fqdnField = document.getElementById(`${prefix}-spec.parameters.service.fqdn_container`).querySelector('input.array-item-input'); if (nameField && fqdnField) { const generateFqdn = (instanceName) => { diff --git a/src/tests/test_form_config.py b/src/tests/test_form_config.py index 7188d61..ba44d72 100644 --- a/src/tests/test_form_config.py +++ b/src/tests/test_form_config.py @@ -5,7 +5,6 @@ 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 @@ -905,182 +904,3 @@ 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