From 2db946d2a4ee1e4da848b6514216c8f153de521b Mon Sep 17 00:00:00 2001
From: Benjamin Bertrand <benjamin.bertrand@esss.se>
Date: Fri, 15 Dec 2017 16:19:18 +0100
Subject: [PATCH] Fix validation error when field is disabled
If a selectfield is disabled, None is returned (not a string).
To pass wtforms validation, that value shall be part of the choices.
We use None as the value for SelectField that are optional (instead of
'').
We were previously converting '' to None when creating a model.
We now use the coerce function to let the form convert 'None' to None.
---
app/inventory/forms.py | 20 ++++++++++----------
app/inventory/views.py | 12 ++++++------
app/networks/forms.py | 10 +++++-----
app/networks/views.py | 4 ++--
app/utils.py | 18 +++++++++++++++---
5 files changed, 38 insertions(+), 26 deletions(-)
diff --git a/app/inventory/forms.py b/app/inventory/forms.py
index 6ce5ff4..e284403 100644
--- a/app/inventory/forms.py
+++ b/app/inventory/forms.py
@@ -33,11 +33,11 @@ class ItemForm(FlaskForm):
validators.Regexp(models.ICS_ID_RE)])
serial_number = StringField('Serial number',
validators=[validators.InputRequired()])
- manufacturer_id = SelectField('Manufacturer')
- model_id = SelectField('Model')
- location_id = SelectField('Location')
- status_id = SelectField('Status')
- parent_id = SelectField('Parent')
+ manufacturer_id = SelectField('Manufacturer', coerce=utils.coerce_to_str_or_none)
+ model_id = SelectField('Model', coerce=utils.coerce_to_str_or_none)
+ location_id = SelectField('Location', coerce=utils.coerce_to_str_or_none)
+ status_id = SelectField('Status', coerce=utils.coerce_to_str_or_none)
+ parent_id = SelectField('Parent', coerce=utils.coerce_to_str_or_none)
mac_addresses = StringField(
'MAC addresses',
description='space separated list of MAC addresses',
@@ -45,11 +45,11 @@ class ItemForm(FlaskForm):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
- self.manufacturer_id.choices = utils.get_model_choices(models.Manufacturer, allow_blank=True)
- self.model_id.choices = utils.get_model_choices(models.Model, allow_blank=True)
- self.location_id.choices = utils.get_model_choices(models.Location, allow_blank=True)
- self.status_id.choices = utils.get_model_choices(models.Status, allow_blank=True)
- self.parent_id.choices = utils.get_model_choices(models.Item, allow_blank=True, attr='ics_id')
+ self.manufacturer_id.choices = utils.get_model_choices(models.Manufacturer, allow_none=True)
+ self.model_id.choices = utils.get_model_choices(models.Model, allow_none=True)
+ self.location_id.choices = utils.get_model_choices(models.Location, allow_none=True)
+ self.status_id.choices = utils.get_model_choices(models.Status, allow_none=True)
+ self.parent_id.choices = utils.get_model_choices(models.Item, allow_none=True, attr='ics_id')
class CommentForm(FlaskForm):
diff --git a/app/inventory/views.py b/app/inventory/views.py
index 6457d57..24d527b 100644
--- a/app/inventory/views.py
+++ b/app/inventory/views.py
@@ -58,11 +58,11 @@ def create_item():
session[key] = getattr(form, key).data
item = models.Item(ics_id=form.ics_id.data,
serial_number=form.serial_number.data,
- manufacturer_id=form.manufacturer_id.data or None,
- model_id=form.model_id.data or None,
- location_id=form.location_id.data or None,
- status_id=form.status_id.data or None,
- parent_id=form.parent_id.data or None)
+ manufacturer_id=form.manufacturer_id.data,
+ model_id=form.model_id.data,
+ location_id=form.location_id.data,
+ status_id=form.status_id.data,
+ parent_id=form.parent_id.data)
item.macs = [models.Mac(address=address) for address in form.mac_addresses.data.split()]
current_app.logger.debug(f'Trying to create: {item!r}')
db.session.add(item)
@@ -109,7 +109,7 @@ def edit_item(ics_id):
if form.validate_on_submit():
item.serial_number = form.serial_number.data
for key in ('manufacturer_id', 'model_id', 'location_id', 'status_id', 'parent_id'):
- setattr(item, key, getattr(form, key).data or None)
+ setattr(item, key, getattr(form, key).data)
new_addresses = form.mac_addresses.data.split()
# Delete the MAC addresses that have been removed
for (index, mac) in enumerate(item.macs):
diff --git a/app/networks/forms.py b/app/networks/forms.py
index 987e128..345e56b 100644
--- a/app/networks/forms.py
+++ b/app/networks/forms.py
@@ -34,7 +34,7 @@ class HostForm(FlaskForm):
filters=[utils.lowercase_field])
type = SelectField('Type', choices=utils.get_choices(('Virtual', 'Physical')))
description = TextAreaField('Description')
- item_id = SelectField('Item')
+ item_id = SelectField('Item', coerce=utils.coerce_to_str_or_none)
network_id = SelectField('Network')
# The list of IPs is dynamically created on the browser side
# depending on the selected network
@@ -44,10 +44,10 @@ class HostForm(FlaskForm):
description='name must be 2-20 characters long and contain only letters, numbers and dash',
validators=[validators.InputRequired(), validators.Regexp(models.HOST_NAME_RE)],
filters=[utils.lowercase_field])
- mac_id = SelectField('MAC')
+ mac_id = SelectField('MAC', coerce=utils.coerce_to_str_or_none)
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
- self.item_id.choices = utils.get_model_choices(models.Item, allow_blank=True, attr='ics_id')
- self.network_id.choices = utils.get_model_choices(models.Network, allow_blank=False, attr='vlan_name')
- self.mac_id.choices = utils.get_model_choices(models.Mac, allow_blank=True, attr='address')
+ self.item_id.choices = utils.get_model_choices(models.Item, allow_none=True, attr='ics_id')
+ self.network_id.choices = utils.get_model_choices(models.Network, allow_none=False, attr='vlan_name')
+ self.mac_id.choices = utils.get_model_choices(models.Mac, allow_none=True, attr='address')
diff --git a/app/networks/views.py b/app/networks/views.py
index 87dffe9..b8c0da7 100644
--- a/app/networks/views.py
+++ b/app/networks/views.py
@@ -44,12 +44,12 @@ def create_host():
host = models.Host(name=form.name.data,
type=form.type.data,
description=form.description.data or None,
- item_id=form.item_id.data or None)
+ item_id=form.item_id.data)
host.interfaces = [
models.Interface(name=form.interface_name.data,
ip=form.ip.data,
network_id=network_id,
- mac_id=form.mac_id.data or None)
+ mac_id=form.mac_id.data)
]
current_app.logger.debug(f'Trying to create: {host!r}')
db.session.add(host)
diff --git a/app/utils.py b/app/utils.py
index 9dc30ce..232cc52 100644
--- a/app/utils.py
+++ b/app/utils.py
@@ -107,11 +107,11 @@ def get_choices(iterable, allow_blank=False, allow_null=False):
return choices
-def get_model_choices(model, allow_blank=False, attr='name'):
+def get_model_choices(model, allow_none=False, attr='name'):
"""Return a list of (value, label)"""
choices = []
- if allow_blank:
- choices = [('', '')]
+ if allow_none:
+ choices = [(None, '')]
choices.extend([(str(instance.id), getattr(instance, attr)) for instance in model.query.all()])
return choices
@@ -138,3 +138,15 @@ def lowercase_field(value):
return value.lower()
except AttributeError:
return value
+
+
+# coerce function to use with SelectField that can accept a None value
+# wtforms always coerce to string by default
+# Values returned from the form are usually strings but if a field is disabled
+# None is returned
+# To pass wtforms validation, the value returned must be part of choices
+def coerce_to_str_or_none(value):
+ """Convert '', None and 'None' to None"""
+ if not value or value == 'None':
+ return None
+ return str(value)
--
GitLab