BoolField.validate is broken

[MrMage]

BoolField.validate currently contains the following code:

// If POSTing from a `checkbox` style input, the key's absence means `false`.
let bool = value.bool ?? false

However, if a checkbox named e.g. "someBoolValue" has been checked, its form value will be a string with content "someBoolValue". Thus, you should be checking for the field's string value instead:

// If POSTing from a `checkbox` style input, the key's absence means `false`.
let bool = value.string != nil

or check both, as a failsafe:

// If POSTing from a `checkbox` style input, the key's absence means `false`.
let bool = value.string != nil || (value.bool ?? false)

See e.g. https://github.com/nodes-vapor/admin-panel-provider/blob/master/Sources/AdminPanelProvider/Models/AdminPanelUserForm.swift (lines 62/63 at the moment) for examples of where this is done correctly:

    let shouldResetPassword = data["shouldResetPassword"]?.string != nil
    let sendEmail = data["sendEmail"]?.string != nil

[MrMage]

This also leads to a follow-up problem with Fieldset.validate: Currently, that code reads

fields.forEach { fieldName, fieldDefinition in
  // For each field, see if there's a matching value in the Content
  // Fail if no matching value for a required field
  let value: Node
  if let nodeValue = content[fieldName] {
    value = nodeValue
  } else {
    // check if required, then bail
  }

The problem is that unchecked checkboxes do not transmit a value at all, so this would cause an unchecked checkbox to never show up in validatedData. That in turn is a problem with e.g. AdminPanelProvider.CheckboxGroup (https://github.com/nodes-vapor/admin-panel-provider/blob/master/Sources/AdminPanelProvider/Tags/Leaf/CheckboxGroup.swift):

let inputValue = fieldset?["value"]?.bool ?? arguments[1]?.bool ?? false

If the checkbox is unchecked, this will always fall back to arguments[1]?.bool.

So I would suggest to add a special case in Fieldset.validate:

fields.forEach { fieldName, fieldDefinition in
  // For each field, see if there's a matching value in the Content
  // Fail if no matching value for a required field
  let value: Node
  if let nodeValue = content[fieldName] {
    value = nodeValue
  } else if fieldDefinition is BoolField {
    value = false  // or something similar
  } else {
    // check if required, then bail
  }

/cc @bygri

[bygri]

I'm not the maintainer of this package; I'm not sure anyone is. This was built for an early version of Vapor where I explicitly added support for this. In the browsers I tested, the value of a checked input was always on unless explicitly overridden in the HTML. A PR would probably be accepted, though.

[MrMage]

Thanks for the info! I might prepare a PR then. FYI, checkboxes by default do not transmit "on" or "off", but nothing if they are unchecked and their value attribute if checked, see e.g. https://stackoverflow.com/questions/4260918/why-do-html-checkboxes-only-transmit-an-on-value.

[bygri]

Incorrect, on Safari and Firefox at least, a checkbox type input with no explicitly supplied value is transmitted as on by default.

[MrMage]

My bad, sorry, I missed that. Thanks for the clarification!