implement basic error handling and validation for forms

This commit is contained in:
Jakob Ketterl 2021-03-24 22:46:51 +01:00
parent 4cbce9c840
commit 6ddced4689
9 changed files with 206 additions and 99 deletions

View File

@ -1,6 +1,7 @@
from owrx.config import Config
from owrx.controllers.admin import AuthorizationMixin
from owrx.controllers.template import WebpageController
from owrx.form.error import FormError
from abc import ABCMeta, abstractmethod
from urllib.parse import parse_qs
@ -10,16 +11,16 @@ class Section(object):
self.title = title
self.inputs = inputs
def render_input(self, input, data):
return input.render(data)
def render_input(self, input, data, errors):
return input.render(data, errors)
def render_inputs(self, data):
return "".join([self.render_input(i, data) for i in self.inputs])
def render_inputs(self, data, errors):
return "".join([self.render_input(i, data, errors) for i in self.inputs])
def classes(self):
return ["col-12", "settings-section"]
def render(self, data):
def render(self, data, errors):
return """
<div class="{classes}">
<h3 class="settings-header">
@ -28,11 +29,20 @@ class Section(object):
{inputs}
</div>
""".format(
classes=" ".join(self.classes()), title=self.title, inputs=self.render_inputs(data)
classes=" ".join(self.classes()), title=self.title, inputs=self.render_inputs(data, errors)
)
def parse(self, data):
return {k: v for i in self.inputs for k, v in i.parse(data).items()}
parsed_data = {}
errors = []
for i in self.inputs:
try:
parsed_data.update(i.parse(data))
except FormError as e:
errors.append(e)
except Exception as e:
errors.append(FormError(i.id, "{}: {}".format(type(e).__name__, e)))
return parsed_data, errors
class SettingsController(AuthorizationMixin, WebpageController):
@ -41,6 +51,10 @@ class SettingsController(AuthorizationMixin, WebpageController):
class SettingsFormController(AuthorizationMixin, WebpageController, metaclass=ABCMeta):
def __init__(self, handler, request, options):
super().__init__(handler, request, options)
self.errors = {}
@abstractmethod
def getSections(self):
pass
@ -52,8 +66,11 @@ class SettingsFormController(AuthorizationMixin, WebpageController, metaclass=AB
def getData(self):
return Config.get()
def getErrors(self):
return self.errors
def render_sections(self):
sections = "".join(section.render(self.getData()) for section in self.getSections())
sections = "".join(section.render(self.getData(), self.getErrors()) for section in self.getSections())
buttons = self.render_buttons()
return """
<form class="settings-body" method="POST">
@ -84,15 +101,34 @@ class SettingsFormController(AuthorizationMixin, WebpageController, metaclass=AB
def parseFormData(self):
data = parse_qs(self.get_body().decode("utf-8"), keep_blank_values=True)
return {k: v for i in self.getSections() for k, v in i.parse(data).items()}
result = {}
errors = []
for section in self.getSections():
section_data, section_errors = section.parse(data)
result.update(section_data)
errors += section_errors
return result, errors
def getSuccessfulRedirect(self):
return self.request.path
def _mergeErrors(self, errors):
result = {}
for e in errors:
if e.getKey() not in result:
result[e.getKey()] = []
result[e.getKey()].append(e.getMessage())
return result
def processFormData(self):
self.processData(self.parseFormData())
self.store()
self.send_redirect(self.getSuccessfulRedirect())
data, errors = self.parseFormData()
if errors:
self.errors = self._mergeErrors(errors)
self.indexAction()
else:
self.processData(data)
self.store()
self.send_redirect(self.getSuccessfulRedirect())
def processData(self, data):
config = self.getData()

View File

@ -8,6 +8,7 @@ from owrx.controllers.settings import Section
from urllib.parse import quote, unquote
from owrx.sdr import SdrService
from owrx.form import TextInput, DropdownInput, Option
from owrx.form.validator import RequiredValidator
from owrx.property import PropertyLayer, PropertyStack
from abc import ABCMeta, abstractmethod
@ -235,7 +236,7 @@ class SdrDeviceController(SdrFormControllerWithModal):
if self.device is None:
self.send_response("device not found", code=404)
return
self.serve_template("settings/general.html", **self.template_variables())
super().indexAction()
def processFormData(self):
if self.device is None:
@ -276,7 +277,7 @@ class NewSdrDeviceController(SettingsFormController):
"New device settings",
TextInput("name", "Device name"),
DropdownInput("type", "Device type", [Option(name, name) for name in SdrDeviceDescription.getTypes()]),
TextInput("id", "Device ID"),
TextInput("id", "Device ID", validator=RequiredValidator()),
)
]
@ -331,7 +332,7 @@ class SdrProfileController(SdrFormControllerWithModal):
if self.profile is None:
self.send_response("profile not found", code=404)
return
self.serve_template("settings/general.html", **self.template_variables())
super().indexAction()
def processFormData(self):
if self.profile is None:
@ -377,7 +378,7 @@ class NewProfileController(SdrProfileController):
return [
Section(
"New profile settings",
TextInput("id", "Profile ID"),
TextInput("id", "Profile ID", validator=RequiredValidator()),
)
] + super().getSections()

View File

@ -1,16 +1,18 @@
from abc import ABC, abstractmethod
from abc import ABC
from owrx.modes import Modes
from owrx.config import Config
from owrx.form.validator import Validator
from owrx.form.converter import Converter, NullConverter, IntConverter, FloatConverter, EnumConverter
from enum import Enum
class Input(ABC):
def __init__(self, id, label, infotext=None, converter: Converter = None, disabled=False, removable=False):
def __init__(self, id, label, infotext=None, converter: Converter = None, validator: Validator = None, disabled=False, removable=False):
self.id = id
self.label = label
self.infotext = infotext
self.converter = self.defaultConverter() if converter is None else converter
self.validator = validator
self.disabled = disabled
self.removable = removable
@ -24,7 +26,6 @@ class Input(ABC):
return NullConverter()
def bootstrap_decorate(self, input):
infotext = "<small>{text}</small>".format(text=self.infotext) if self.infotext else ""
return """
<div class="form-group row" data-field="{id}">
<label class="col-form-label col-form-label-sm col-3" for="{id}">{label}</label>
@ -40,19 +41,22 @@ class Input(ABC):
id=self.id,
label=self.label,
input=input,
infotext=infotext,
infotext="<small>{text}</small>".format(text=self.infotext) if self.infotext else "",
removable="removable" if self.removable else "",
removebutton='<button type="button" class="btn btn-sm btn-danger option-remove-button">Remove</button>'
if self.removable
else "",
)
def input_classes(self):
return " ".join(["form-control", "form-control-sm"])
def input_classes(self, error):
classes = ["form-control", "form-control-sm"]
if error:
classes.append("is-invalid")
return " ".join(classes)
def input_properties(self, value):
def input_properties(self, value, error):
props = {
"class": self.input_classes(),
"class": self.input_classes(error),
"id": self.id,
"name": self.id,
"placeholder": self.label,
@ -62,26 +66,35 @@ class Input(ABC):
props["disabled"] = "disabled"
return props
def render_input_properties(self, value):
return " ".join('{}="{}"'.format(prop, value) for prop, value in self.input_properties(value).items())
def render_input_properties(self, value, error):
return " ".join('{}="{}"'.format(prop, value) for prop, value in self.input_properties(value, error).items())
def render_input(self, value):
return "<input {properties} />".format(properties=self.render_input_properties(value))
def render_errors(self, errors):
return "".join("""<div class="invalid-feedback">{msg}</div>""".format(msg=e) for e in errors)
def render(self, config):
def render_input(self, value, errors):
return "<input {properties} />{errors}".format(
properties=self.render_input_properties(value, errors), errors=self.render_errors(errors)
)
def render(self, config, errors):
value = config[self.id] if self.id in config else None
return self.bootstrap_decorate(self.render_input(self.converter.convert_to_form(value)))
error = errors[self.id] if self.id in errors else []
return self.bootstrap_decorate(self.render_input(self.converter.convert_to_form(value), error))
def parse(self, data):
return {self.id: self.converter.convert_from_form(data[self.id][0])} if self.id in data else {}
value = self.converter.convert_from_form(data[self.id][0])
if self.validator is not None:
self.validator.validate(self.id, value)
return {self.id: value} if self.id in data else {}
def getLabel(self):
return self.label
class TextInput(Input):
def input_properties(self, value):
props = super().input_properties(value)
def input_properties(self, value, errors):
props = super().input_properties(value, errors)
props["type"] = "text"
return props
@ -95,14 +108,14 @@ class NumberInput(Input):
def defaultConverter(self):
return IntConverter()
def input_properties(self, value):
props = super().input_properties(value)
def input_properties(self, value, errors):
props = super().input_properties(value, errors)
props["type"] = "number"
if self.step:
props["step"] = self.step
return props
def render_input(self, value):
def render_input(self, value, errors):
if self.append:
append = """
<div class="input-group-append">
@ -120,7 +133,7 @@ class NumberInput(Input):
{append}
</div>
""".format(
input=super().render_input(value),
input=super().render_input(value, errors),
append=append,
)
@ -135,7 +148,8 @@ class FloatInput(NumberInput):
class LocationInput(Input):
def render_input(self, value):
def render_input(self, value, errors):
# TODO display errors
return """
<div class="row">
{inputs}
@ -145,11 +159,11 @@ class LocationInput(Input):
</div>
""".format(
id=self.id,
inputs="".join(self.render_sub_input(value, id) for id in ["lat", "lon"]),
inputs="".join(self.render_sub_input(value, id, errors) for id in ["lat", "lon"]),
key=Config.get()["google_maps_api_key"],
)
def render_sub_input(self, value, id):
def render_sub_input(self, value, id, errors):
return """
<div class="col">
<input type="number" class="{classes}" id="{id}" name="{id}" placeholder="{label}" value="{value}"
@ -158,7 +172,7 @@ class LocationInput(Input):
""".format(
id="{0}-{1}".format(self.id, id),
label=self.label,
classes=self.input_classes(),
classes=self.input_classes(errors),
value=value[id],
disabled="disabled" if self.disabled else "",
)
@ -168,11 +182,16 @@ class LocationInput(Input):
class TextAreaInput(Input):
def render_input(self, value):
def render_input(self, value, errors):
return """
<textarea class="{classes}" id="{id}" name="{id}" style="height:200px;" {disabled}>{value}</textarea>
{errors}
""".format(
id=self.id, classes=self.input_classes(), value=value, disabled="disabled" if self.disabled else ""
id=self.id,
classes=self.input_classes(errors),
value=value,
disabled="disabled" if self.disabled else "",
errors=self.render_errors(errors),
)
@ -181,7 +200,7 @@ class CheckboxInput(Input):
super().__init__(id, "", infotext=infotext, converter=converter)
self.checkboxText = checkboxText
def render_input(self, value):
def render_input(self, value, errors):
return """
<div class="{classes}">
<input type="hidden" name="{id}" value="0" {disabled}>
@ -189,17 +208,22 @@ class CheckboxInput(Input):
<label class="form-check-label" for="{id}">
{checkboxText}
</label>
{errors}
</div>
""".format(
id=self.id,
classes=self.input_classes(),
classes=self.input_classes(errors),
checked="checked" if value else "",
disabled="disabled" if self.disabled else "",
checkboxText=self.checkboxText,
errors=self.render_errors(errors)
)
def input_classes(self):
return " ".join(["form-check", "form-control-sm"])
def input_classes(self, error):
classes = ["form-check", "form-control-sm"]
if error:
classes.append("is-invalid")
return " ".join(classes)
def parse(self, data):
if self.id in data:
@ -222,13 +246,14 @@ class MultiCheckboxInput(Input):
super().__init__(id, label, infotext=infotext)
self.options = options
def render_input(self, value):
return "".join(self.render_checkbox(o, value) for o in self.options)
def render_input(self, value, errors):
# TODO display errors
return "".join(self.render_checkbox(o, value, errors) for o in self.options)
def checkbox_id(self, option):
return "{0}-{1}".format(self.id, option.value)
def render_checkbox(self, option, value):
def render_checkbox(self, option, value, errors):
return """
<div class="{classes}">
<input class="form-check-input" type="checkbox" id="{id}" name="{id}" {checked} {disabled}>
@ -238,7 +263,7 @@ class MultiCheckboxInput(Input):
</div>
""".format(
id=self.checkbox_id(option),
classes=self.input_classes(),
classes=self.input_classes(errors),
checked="checked" if option.value in value else "",
checkboxText=option.text,
disabled="disabled" if self.disabled else "",
@ -251,8 +276,11 @@ class MultiCheckboxInput(Input):
return {self.id: [o.value for o in self.options if in_response(o)]}
def input_classes(self):
return " ".join(["form-check", "form-control-sm"])
def input_classes(self, error):
classes = ["form-check", "form-control-sm"]
if error:
classes.append("is-invalid")
return " ".join(classes)
class ServicesCheckboxInput(MultiCheckboxInput):
@ -286,14 +314,16 @@ class DropdownInput(Input):
self.options = options
super().__init__(id, label, infotext=infotext, converter=converter)
def render_input(self, value):
def render_input(self, value, errors):
return """
<select class="{classes}" id="{id}" name="{id}" {disabled}>{options}</select>
{errors}
""".format(
classes=self.input_classes(),
classes=self.input_classes(errors),
id=self.id,
options=self.render_options(value),
disabled="disabled" if self.disabled else "",
errors=self.render_errors(errors),
)
def render_options(self, value):
@ -329,13 +359,13 @@ class ExponentialInput(Input):
def defaultConverter(self):
return IntConverter()
def input_properties(self, value):
props = super().input_properties(value)
def input_properties(self, value, errors):
props = super().input_properties(value, errors)
props["type"] = "number"
props["step"] = "any"
return props
def render_input(self, value):
def render_input(self, value, errors):
append = """
<div class="input-group-append">
<select class="input-group-text exponent" name="{id}-exponent" {disabled}>
@ -358,7 +388,7 @@ class ExponentialInput(Input):
{append}
</div>
""".format(
input=super().render_input(value),
input=super().render_input(value, errors),
append=append,
)

View File

@ -10,7 +10,8 @@ class GainInput(Input):
self.has_agc = has_agc
self.gain_stages = gain_stages
def render_input(self, value):
def render_input(self, value, errors):
# TODO display errors
try:
display_value = float(value)
except (ValueError, TypeError):
@ -29,11 +30,11 @@ class GainInput(Input):
</div>
""".format(
id=self.id,
classes=self.input_classes(),
classes=self.input_classes(errors),
value=display_value,
label=self.label,
options=self.render_options(value),
stageoption="" if self.gain_stages is None else self.render_stage_option(value),
stageoption="" if self.gain_stages is None else self.render_stage_option(value, errors),
disabled="disabled" if self.disabled else "",
)
@ -71,7 +72,7 @@ class GainInput(Input):
return "stages"
def render_stage_option(self, value):
def render_stage_option(self, value, errors):
try:
value_dict = {k: v for item in SoapySettings.parse(value) for k, v in item.items()}
except (AttributeError, ValueError):
@ -93,7 +94,7 @@ class GainInput(Input):
id=self.id,
stage=stage,
value=value_dict[stage] if stage in value_dict else "",
classes=self.input_classes(),
classes=self.input_classes(errors),
disabled="disabled" if self.disabled else "",
)
for stage in self.gain_stages
@ -172,12 +173,12 @@ class SchedulerInput(Input):
super().__init__(id, label)
self.profiles = {}
def render(self, config):
def render(self, config, errors):
if "profiles" in config:
self.profiles = config["profiles"]
return super().render(config)
return super().render(config, errors)
def render_profiles_select(self, value, config_key, stage, extra_classes=""):
def render_profiles_select(self, value, errors, config_key, stage, extra_classes=""):
stage_value = ""
if value and "schedule" in value and config_key in value["schedule"]:
stage_value = value["schedule"][config_key]
@ -188,7 +189,7 @@ class SchedulerInput(Input):
</select>
""".format(
id="{}-{}".format(self.id, stage),
classes=self.input_classes(),
classes=self.input_classes(errors),
extra_classes=extra_classes,
disabled="disabled" if self.disabled else "",
options="".join(
@ -203,7 +204,7 @@ class SchedulerInput(Input):
),
)
def render_static_entires(self, value):
def render_static_entires(self, value, errors):
def render_time_inputs(v):
values = ["{}:{}".format(x[0:2], x[2:4]) for x in [v[0:4], v[5:9]]]
return '<div class="p-1">-</div>'.join(
@ -211,7 +212,7 @@ class SchedulerInput(Input):
<input type="time" class="{classes}" id="{id}" name="{id}" {disabled} value="{value}">
""".format(
id="{}-{}-{}".format(self.id, "time", "start" if i == 0 else "end"),
classes=self.input_classes(),
classes=self.input_classes(errors),
disabled="disabled" if self.disabled else "",
value=v,
)
@ -231,7 +232,7 @@ class SchedulerInput(Input):
</div>
""".format(
time_inputs=render_time_inputs(slot),
select=self.render_profiles_select(value, slot, "profile"),
select=self.render_profiles_select(value, errors, slot, "profile"),
)
for slot, entry in schedule.items()
)
@ -249,10 +250,10 @@ class SchedulerInput(Input):
""".format(
rows=rows,
time_inputs=render_time_inputs("0000-0000"),
select=self.render_profiles_select("", "0000-0000", "profile"),
select=self.render_profiles_select("", errors, "0000-0000", "profile"),
)
def render_daylight_entries(self, value):
def render_daylight_entries(self, value, errors):
return "".join(
"""
<div class="row">
@ -261,12 +262,12 @@ class SchedulerInput(Input):
</div>
""".format(
name=name,
select=self.render_profiles_select(value, stage, stage, extra_classes="col-9"),
select=self.render_profiles_select(value, errors, stage, stage, extra_classes="col-9"),
)
for stage, name in [("day", "Day"), ("night", "Night"), ("greyline", "Greyline")]
)
def render_input(self, value):
def render_input(self, value, errors):
return """
<div id="{id}">
<select class="{classes} mode" id="{id}-select" name="{id}-select" {disabled}>
@ -281,11 +282,11 @@ class SchedulerInput(Input):
</div>
""".format(
id=self.id,
classes=self.input_classes(),
classes=self.input_classes(errors),
disabled="disabled" if self.disabled else "",
options=self.render_options(value),
entries=self.render_static_entires(value),
stages=self.render_daylight_entries(value),
entries=self.render_static_entires(value, errors),
stages=self.render_daylight_entries(value, errors),
)
def _get_mode(self, value):
@ -341,7 +342,8 @@ class SchedulerInput(Input):
class WaterfallLevelsInput(Input):
def render_input(self, value):
def render_input(self, value, errors):
# TODO display errors
return """
<div class="row" id="{id}">
{inputs}
@ -364,7 +366,7 @@ class WaterfallLevelsInput(Input):
name=name,
label=label,
value=value[name] if value and name in value else "0",
classes=self.input_classes(),
classes=self.input_classes(errors),
disabled="disabled" if self.disabled else "",
)
for name, label in [("min", "Minimum"), ("max", "Maximum")]

15
owrx/form/error.py Normal file
View File

@ -0,0 +1,15 @@
class FormError(Exception):
def __init__(self, key, message):
super().__init__("Error processing form data for {}: {}".format(key, message))
self.key = key
self.message = message
def getKey(self):
return self.key
def getMessage(self):
return self.message
class ValidationError(FormError):
pass

View File

@ -4,7 +4,8 @@ from datetime import datetime
class ImageInput(Input, metaclass=ABCMeta):
def render_input(self, value):
def render_input(self, value, errors):
# TODO display errors
return """
<div class="imageupload">
<input type="hidden" id="{id}" name="{id}">

14
owrx/form/validator.py Normal file
View File

@ -0,0 +1,14 @@
from abc import ABC, abstractmethod
from owrx.form.error import ValidationError
class Validator(ABC):
@abstractmethod
def validate(self, key, value):
pass
class RequiredValidator(Validator):
def validate(self, key, value):
if value is None or value == "":
raise ValidationError(key, "Field is required")

View File

@ -9,7 +9,7 @@ class Q65ModeMatrix(Input):
def checkbox_id(self, mode, interval):
return "{0}-{1}-{2}".format(self.id, mode.value, interval.value)
def render_checkbox(self, mode: Q65Mode, interval: Q65Interval, value):
def render_checkbox(self, mode: Q65Mode, interval: Q65Interval, value, errors):
return """
<div class="{classes}">
<input class="form-check-input" type="checkbox" id="{id}" name="{id}" {checked} {disabled}>
@ -18,16 +18,16 @@ class Q65ModeMatrix(Input):
</label>
</div>
""".format(
classes=self.input_classes(),
classes=self.input_classes(errors),
id=self.checkbox_id(mode, interval),
checked="checked" if "{}{}".format(mode.name, interval.value) in value else "",
checkboxText="Mode {} interval {}s".format(mode.name, interval.value),
disabled="" if interval.is_available(mode) and not self.disabled else "disabled",
)
def render_input(self, value):
def render_input(self, value, errors):
checkboxes = "".join(
self.render_checkbox(mode, interval, value) for interval in Q65Interval for mode in Q65Mode
self.render_checkbox(mode, interval, value, errors) for interval in Q65Interval for mode in Q65Mode
)
return """
<div class="matrix q65-matrix">
@ -37,8 +37,11 @@ class Q65ModeMatrix(Input):
checkboxes=checkboxes
)
def input_classes(self):
return " ".join(["form-check", "form-control-sm"])
def input_classes(self, error):
classes = ["form-check", "form-control-sm"]
if error:
classes.append("is-invalid")
return " ".join(classes)
def parse(self, data):
def in_response(mode, interval):
@ -59,7 +62,7 @@ class WsjtDecodingDepthsInput(Input):
def defaultConverter(self):
return JsonConverter()
def render_input(self, value):
def render_input(self, value, errors):
def render_mode(m):
return """
<option value={mode}>{name}</option>
@ -76,11 +79,11 @@ class WsjtDecodingDepthsInput(Input):
</div>
""".format(
id=self.id,
classes=self.input_classes(),
classes=self.input_classes(errors),
value=html.escape(value),
options="".join(render_mode(m) for m in Modes.getAvailableModes() if isinstance(m, WsjtMode)),
disabled="disabled" if self.disabled else ""
)
def input_classes(self):
return super().input_classes() + " wsjt-decoding-depths"
def input_classes(self, error):
return super().input_classes(error) + " wsjt-decoding-depths"

View File

@ -488,19 +488,23 @@ class OptionalSection(Section):
)
)
def render_optional_inputs(self, data):
def render_optional_inputs(self, data, errors):
return """
<div class="optional-inputs" style="display: none;">
{inputs}
</div>
""".format(
inputs="".join(self.render_input(input, data) for input in self.optional_inputs)
inputs="".join(self.render_input(input, data, errors) for input in self.optional_inputs)
)
def render_inputs(self, data):
return super().render_inputs(data) + self.render_optional_select() + self.render_optional_inputs(data)
def render_inputs(self, data, errors):
return (
super().render_inputs(data, errors)
+ self.render_optional_select()
+ self.render_optional_inputs(data, errors)
)
def render(self, data):
def render(self, data, errors):
indexed_inputs = {input.id: input for input in self.inputs}
visible_keys = set(self.mandatory + [k for k in self.optional if k in data])
optional_keys = set(k for k in self.optional if k not in data)
@ -512,15 +516,15 @@ class OptionalSection(Section):
for input in self.optional_inputs:
input.setRemovable()
input.setDisabled()
return super().render(data)
return super().render(data, errors)
def parse(self, data):
data = super().parse(data)
data, errors = super().parse(data)
# remove optional keys if they have been removed from the form
for k in self.optional:
if k not in data:
data[k] = None
return data
return data, errors
class SdrDeviceDescription(object):
@ -542,6 +546,7 @@ class SdrDeviceDescription(object):
return True
except SdrDeviceDescriptionMissing:
return False
return [module_name for _, module_name, _ in pkgutil.walk_packages(__path__) if has_description(module_name)]
def getDeviceInputs(self) -> List[Input]: