From 9e602a3d845b74182a7939270cb23c276e138c59 Mon Sep 17 00:00:00 2001 From: Antonio Salazar Cardozo Date: Mon, 7 Nov 2016 15:49:23 -0500 Subject: [PATCH 01/10] Store field name in RequestVar, abstract validation errors. Validation errors are now added using `addValidationErrors`, which uses S.error. We also store the field name in a RequestVar, and use that in addValidationErrors to avoid having to repeat the field name. --- .../hacklanta/formality/FieldHandling.scala | 89 ++++++++++++------- 1 file changed, 56 insertions(+), 33 deletions(-) diff --git a/src/main/scala/com/hacklanta/formality/FieldHandling.scala b/src/main/scala/com/hacklanta/formality/FieldHandling.scala index ac805ea..cef8de3 100644 --- a/src/main/scala/com/hacklanta/formality/FieldHandling.scala +++ b/src/main/scala/com/hacklanta/formality/FieldHandling.scala @@ -16,6 +16,11 @@ import net.liftweb.util._ private[formality] class FieldValueVar[FieldValueType](dflt: =>Box[FieldValueType] = Empty) extends TransientRequestVar[Box[FieldValueType]](dflt) { override def __nameSalt = randomString(10) } +// A container for a given field's name. This is a RequestVar, and it is salted +// so that each instance is mapped uniquely in the function map. +private[formality] class FieldNameVar(dflt: =>String) extends RequestVar[String](dflt) { + override def __nameSalt = randomString(10) +} /** * A FieldHolder, at its most basic, has a value that is a Box of its @@ -138,16 +143,38 @@ abstract class BaseFieldHolder[ def ->(eventHandler: EventHandler[EventHandlerType]) = handlingEvent(eventHandler) /** - * The handler function should set this TransientRequestVar to the + * The handler function should set this `TransientRequestVar` to the * appropriate value. */ val fieldValue = new FieldValueVar[FieldValueType]() + /** + * Stores the field name generated by `generateFunctionIdAndHandler`. The + * current field name should be accessed from here rather than directly + * through `generateFunctionIdAndHandler` except in case of clear preference. + */ + protected val fieldName = new FieldNameVar(generateFunctionIdAndHandler) + + /** + * Returns the current value of the field. If no value was seen in the last + * form submission, invokes `boxedValidations` and registers any resulting + * validation errors. + */ def value = fieldValue.is /** - * Creates a handler function, maps it (potentially using S.fmapFunc), + * Adds the passed error messages, using `S.error`. Can be overridden to use + * something other than `S.error`. + */ + protected def addValidationErrors(errorMessages: String*): Unit = { + errorMessages.foreach { errorMessage => + S.error(fieldName.is, errorMessage) + } + } + + /** + * Creates a handler function, maps it (potentially using `S.fmapFunc`), * and returns the resulting function id to bind to the HTML - * field. See the implementation in SimpleFieldHolder for a sample. + * field. See the implementation in `SimpleFieldHolder` for a sample. */ protected def generateFunctionIdAndHandler: String /** @@ -157,7 +184,7 @@ abstract class BaseFieldHolder[ * super.baseTransform and append your own transforms to that. */ protected def baseTransform: CssSel = { - val functionId = generateFunctionIdAndHandler + val functionId = fieldName.is val nameTransform = (selector + " [name]") #> functionId @@ -178,11 +205,11 @@ abstract class BaseFieldHolder[ * * If you override the binder, please make sure you properly * map your handler function, event handlers, and validations. This - * binder automatically calls generateFunctionIdAndHandler and binds + * binder automatically calls `[[generateFunctionIdAndHandler]]` and binds * its result to the name of the HTML field, and your code must do * something similar to ensure the server-side function you define is * called on form submission. - */ + */ def binder: CssSel = { val withValidations = validations.foldLeft(baseTransform)(_ & _.binder(selector)) @@ -230,23 +257,27 @@ case class SimpleFieldHolder[ def handler(incomingValue: String): Unit = { convertValue(incomingValue) match { case Full(convertedValue) => - val validationErrors = validations.reverse.flatMap(_(convertedValue)) + val validationErrors = + validations.reverse.flatMap(_(convertedValue)) ++ + boxedValidations.reverse.flatMap(_(Full(convertedValue))) - validationErrors.foreach { message => - S.error(functionId, message) - } + addValidationErrors(validationErrors: _*) - if (validationErrors.isEmpty) + if (validationErrors.isEmpty) { fieldValue(Full(convertedValue)) - else + } else { fieldValue(Failure(convertedValue + " failed validations.") ~> validationErrors) + } case failure @ Failure(failureError, _, _) => + val validationErrors = boxedValidations.reverse.flatMap(_(failure)) + fieldValue(failure ~> incomingValue) - S.error(functionId, failureError) + addValidationErrors(failureError) + case Empty => fieldValue(Failure("Unrecognized response.") ~> incomingValue) - S.error(functionId, "Unrecognized response.") + addValidationErrors("Unrecognized response.") } } @@ -394,7 +425,7 @@ case class SelectFieldHolder[ val withValidations = validations.foldLeft("nothing" #> PassThru)(_ & _.binder(selector)) val fullBinder = eventHandlers.foldLeft(withValidations)(_ & _.binder(selector, optionProcessor)) - + selector #> fullBinder(select) } @@ -414,9 +445,7 @@ case class SelectFieldHolder[ case Full(convertedValue) => val validationErrors = validations.reverse.flatMap(_(convertedValue)) - validationErrors.foreach { message => - S.error(functionId, message) - } + addValidationErrors(validationErrors: _*) if (validationErrors.isEmpty) fieldValue(Full(convertedValue)) @@ -425,10 +454,10 @@ case class SelectFieldHolder[ case failure @ Failure(failureError, _, _) => fieldValue(failure ~> incomingValue) - S.error(functionId, failureError) + addValidationErrors(failureError) case Empty => fieldValue(Failure("Unrecognized response.") ~> incomingValue) - S.error(functionId, "Unrecognized response.") + addValidationErrors("Unrecognized response.") } } @@ -590,7 +619,7 @@ case class MultiSelectFieldHolder[ val withValidations = validations.foldLeft("nothing" #> PassThru)(_ & _.binder(selector)) val fullBinder = eventHandlers.foldLeft(withValidations)(_ & _.binder(selector, (s: String)=>Full(optionProcessor(List(s))))) - + selector #> fullBinder(select) } @@ -611,9 +640,7 @@ case class MultiSelectFieldHolder[ case convertedValues => val validationErrors = validations.reverse.flatMap(_(convertedValues)) - validationErrors.foreach { message => - S.error(functionId, message) - } + addValidationErrors(validationErrors: _*) if (validationErrors.isEmpty) fieldValue(Full(convertedValues)) @@ -657,7 +684,7 @@ case class CheckboxFieldHolder( } override protected def baseTransform: CssSel = { - val functionId = generateFunctionIdAndHandler + val functionId = fieldName.is selector #> { ns: NodeSeq => ns match { case element: Elem => @@ -684,9 +711,7 @@ case class CheckboxFieldHolder( case Full(convertedValue) => val validationErrors = validations.reverse.flatMap(_(convertedValue)) - validationErrors.foreach { message => - S.error(functionId, message) - } + addValidationErrors(validationErrors: _*) if (validationErrors.isEmpty) fieldValue(Full(convertedValue)) @@ -750,9 +775,7 @@ case class FileFieldHolder[ case Full(convertedValue) => val validationErrors = validations.reverse.flatMap(_(convertedValue)) - validationErrors.foreach { message => - S.error(functionId, message) - } + addValidationErrors(validationErrors: _*) if (validationErrors.isEmpty) fieldValue(Full(convertedValue)) @@ -761,10 +784,10 @@ case class FileFieldHolder[ case failure @ Failure(failureError, _, _) => fieldValue(failure) - S.error(functionId, failureError) + addValidationErrors(failureError) case Empty => fieldValue(Failure("Unrecognized response.")) - S.error(functionId, "Unrecognized response.") + addValidationErrors("Unrecognized response.") } } From efbcab03c2fab5879bbc496bf0c57b7f4dce6cb2 Mon Sep 17 00:00:00 2001 From: Antonio Salazar Cardozo Date: Mon, 7 Nov 2016 15:59:45 -0500 Subject: [PATCH 02/10] Add support for boxed validations. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These will be run both on seen content and on fields that weren’t submitted with the form at all, so that fields that weren’t submitted can be marked as validation failures when necessary. --- .../hacklanta/formality/FieldHandling.scala | 62 ++++++++++++++++--- .../com/hacklanta/formality/Formality.scala | 14 ++--- .../hacklanta/formality/FormalityForm.scala | 8 +-- 3 files changed, 63 insertions(+), 21 deletions(-) diff --git a/src/main/scala/com/hacklanta/formality/FieldHandling.scala b/src/main/scala/com/hacklanta/formality/FieldHandling.scala index cef8de3..5b23945 100644 --- a/src/main/scala/com/hacklanta/formality/FieldHandling.scala +++ b/src/main/scala/com/hacklanta/formality/FieldHandling.scala @@ -37,7 +37,7 @@ trait FieldHolderBase[FieldValueType] { * String to the FieldValueType) failed. In case of a validation * failure, this should be a ParamFailure whose param is the list of * validation failures (a List[String]) - * + * * - If it is Full, it contains the value for the current form * submission. */ @@ -98,6 +98,7 @@ abstract class BaseFieldHolder[ selector: String, initialValue: Box[FieldValueType], validations: List[Validation[ValidationType]], + boxedValidations: List[Validation[Box[ValidationType]]], eventHandlers: List[EventHandler[EventHandlerType]] )( implicit eventHandlerValueConverter: (String)=>Box[FieldValueType] @@ -126,6 +127,25 @@ abstract class BaseFieldHolder[ /** * This should return a copy of this BaseFieldHolder with the + * specified validation attached. Left abstract because by far + * the best implementation is using a case class copy method. + */ + def validatingWith(boxedValidation: Validation[Box[ValidationType]])(implicit dummy: DummyImplicit): BaseFieldHolder[IncomingValueType, FieldValueType, ValidationType, EventHandlerType] + /** + * Adds the given boxed validation to the list of validations run on this + * field at form processing time. Note that validations may add + * attributes to the field to indicate on the client side what types of + * validations are expected; see the `[[Validation]]` trait for more. + * + * Note that `FieldHolder`s are immutable; this returns a copy of this + * `FieldHolder` with an updated validation list. + * + * Aliased as `[[validatingWith]]` for folks who don't like operators. + */ + def ?(boxedValidation: Validation[Box[ValidationType]])(implicit dummy: DummyImplicit) = validatingWith(boxedValidation) + + /** + * This should return a copy of this `BaseFieldHolder` with the * specified event handler attached. Left abstract because by far the * best implementation is using a case class copy method. */ @@ -133,12 +153,12 @@ abstract class BaseFieldHolder[ /** * Adds the given event handler to the list of event handlers this field * will have on the client. See EventHandler for more. Meant to be - * used with Formality.on for fluency (e.g., field -> on("change", checkStuff _)). + * used with Formality.on for fluency (e.g., `field -> on("change", checkStuff _)`). * - * Note that FieldHolders are immutable; this returns a copy of this - * FieldHolder with an updated event handler list. + * Note that `FieldHolder`s are immutable; this returns a copy of this + * `FieldHolder` with an updated event handler list. * - * Aliased as handlingEvent for folks who don't like operators. + * Aliased as `[[handlingEvent]]` for folks who don't like operators. */ def ->(eventHandler: EventHandler[EventHandlerType]) = handlingEvent(eventHandler) @@ -228,17 +248,23 @@ case class SimpleFieldHolder[ selector: String, initialValue: Box[FieldValueType], validations: List[Validation[ValidationType]], + boxedValidations: List[Validation[Box[ValidationType]]], eventHandlers: List[EventHandler[EventHandlerType]] )( implicit valueConverter: (String)=>Box[FieldValueType], valueSerializer: (FieldValueType)=>String -) extends BaseFieldHolder[String, FieldValueType, ValidationType, EventHandlerType](selector, initialValue, validations, eventHandlers) { +) extends BaseFieldHolder[String, FieldValueType, ValidationType, EventHandlerType]( + selector, initialValue, validations, boxedValidations, eventHandlers + ) { protected def convertValue(incomingValue: String) = valueConverter(incomingValue) protected def serializeValue(value: FieldValueType) = valueSerializer(value) def validatingWith(validation: Validation[ValidationType]) = { this.copy(validations = validation :: validations) } + def validatingWith(boxedValidation: Validation[Box[ValidationType]])(implicit dummy: DummyImplicit) = { + this.copy(boxedValidations = boxedValidation :: boxedValidations) + } def handlingEvent(eventHandler: EventHandler[EventHandlerType]) = { this.copy(eventHandlers = eventHandler :: eventHandlers) } @@ -313,10 +339,11 @@ case class SelectFieldHolder[ initialValue: Box[FieldValueType], options: List[SelectableOption[FieldValueType]], validations: List[Validation[ValidationType]], + boxedValidations: List[Validation[Box[ValidationType]]], eventHandlers: List[EventHandler[EventHandlerType]], asRadioButtons: Boolean ) extends BaseFieldHolder[String, FieldValueType, ValidationType, EventHandlerType]( - selector, initialValue, validations, eventHandlers + selector, initialValue, validations, boxedValidations, eventHandlers )( eventHandlerValueConverter = { eventHandlingValue: String => Empty /* we don't get file values for event handlers */ } ) { @@ -349,6 +376,9 @@ case class SelectFieldHolder[ def validatingWith(validation: Validation[ValidationType]) = { this.copy(validations = validation :: validations) } + def validatingWith(boxedValidation: Validation[Box[ValidationType]])(implicit dummy: DummyImplicit) = { + this.copy(boxedValidations = boxedValidation :: boxedValidations) + } def handlingEvent(eventHandler: EventHandler[EventHandlerType]) = { this.copy(eventHandlers = eventHandler :: eventHandlers) } @@ -494,10 +524,11 @@ case class MultiSelectFieldHolder[ initialValues: List[FieldValueType], options: List[SelectableOption[FieldValueType]], validations: List[Validation[List[ValidationType]]], + boxedValidations: List[Validation[Box[List[ValidationType]]]], eventHandlers: List[EventHandler[List[EventHandlerType]]], asCheckboxes: Boolean ) extends BaseFieldHolder[String, List[FieldValueType], List[ValidationType], List[EventHandlerType]]( - selector, Full(initialValues), validations, eventHandlers + selector, Full(initialValues), validations, boxedValidations, eventHandlers )( eventHandlerValueConverter = { eventHandlingValue: String => Empty /* we don't get file values for event handlers */ } ) { @@ -543,6 +574,9 @@ case class MultiSelectFieldHolder[ def validatingWith(validation: Validation[List[ValidationType]]) = { this.copy(validations = validation :: validations) } + def validatingWith(boxedValidation: Validation[Box[List[ValidationType]]])(implicit dummy: DummyImplicit) = { + this.copy(boxedValidations = boxedValidation :: boxedValidations) + } def handlingEvent(eventHandler: EventHandler[List[EventHandlerType]]) = { this.copy(eventHandlers = eventHandler :: eventHandlers) } @@ -660,9 +694,10 @@ case class CheckboxFieldHolder( selector: String, initialValue: Boolean, validations: List[Validation[Boolean]], + boxedValidations: List[Validation[Box[Boolean]]], eventHandlers: List[EventHandler[Boolean]] ) extends BaseFieldHolder[String, Boolean, Boolean, Boolean]( - selector, Full(initialValue), validations, eventHandlers + selector, Full(initialValue), validations, boxedValidations, eventHandlers )( eventHandlerValueConverter = (incomingValue: String) => Full(toBoolean(incomingValue)) ) { @@ -679,6 +714,9 @@ case class CheckboxFieldHolder( def validatingWith(validation: Validation[Boolean]) = { this.copy(validations = validation :: validations) } + def validatingWith(boxedValidation: Validation[Box[Boolean]])(implicit dummy: DummyImplicit) = { + this.copy(boxedValidations = boxedValidation :: boxedValidations) + } def handlingEvent(eventHandler: EventHandler[Boolean]) = { this.copy(eventHandlers = eventHandler :: eventHandlers) } @@ -741,11 +779,12 @@ case class FileFieldHolder[ ]( selector: String, validations: List[Validation[ValidationType]], + boxedValidations: List[Validation[Box[ValidationType]]], eventHandlers: List[EventHandler[EventHandlerType]] )( implicit valueConverter: (FileParamHolder)=>Box[FieldValueType] ) extends BaseFieldHolder[FileParamHolder, FieldValueType, ValidationType, EventHandlerType]( - selector, Empty, validations, eventHandlers + selector, Empty, validations, boxedValidations, eventHandlers )( eventHandlerValueConverter = { eventHandlingValue: String => Empty /* we don't get file values for event handlers */ } ) { @@ -755,6 +794,9 @@ case class FileFieldHolder[ def validatingWith(validation: Validation[ValidationType]) = { this.copy(validations = validation :: validations) } + def validatingWith(boxedValidation: Validation[Box[ValidationType]])(implicit dummy: DummyImplicit) = { + this.copy(boxedValidations = boxedValidation :: boxedValidations) + } def handlingEvent(eventHandler: EventHandler[EventHandlerType]) = { this.copy(eventHandlers = eventHandler :: eventHandlers) } diff --git a/src/main/scala/com/hacklanta/formality/Formality.scala b/src/main/scala/com/hacklanta/formality/Formality.scala index 59edd31..ad2666a 100644 --- a/src/main/scala/com/hacklanta/formality/Formality.scala +++ b/src/main/scala/com/hacklanta/formality/Formality.scala @@ -10,18 +10,18 @@ import net.liftweb.util._ object Formality extends FieldValueHelpers { def field[T](selector: String, initialValue: T)(implicit valueConverter: (String)=>Box[T], valueSerializer: (T)=>String): SimpleFieldHolder[T, T, T] = { - SimpleFieldHolder(selector, Full(initialValue), Nil, Nil)(valueConverter, valueSerializer) + SimpleFieldHolder(selector, Full(initialValue), Nil, Nil, Nil)(valueConverter, valueSerializer) } def field[T](selector: String)(implicit valueConverter: (String)=>Box[T], valueSerializer: (T)=>String): SimpleFieldHolder[T, T, T] = { - SimpleFieldHolder[T,T,T](selector, Empty, Nil, Nil)(valueConverter, valueSerializer) + SimpleFieldHolder[T,T,T](selector, Empty, Nil, Nil, Nil)(valueConverter, valueSerializer) } // Basic file upload field, spits out a FileParamHolder. def fileUploadField(selector: String): FileFieldHolder[FileParamHolder, FileParamHolder, FileParamHolder] = { - FileFieldHolder(selector, Nil, Nil)(fph => Full(fph)) + FileFieldHolder(selector, Nil, Nil, Nil)(fph => Full(fph)) } def typedFileUploadField[T](selector: String)(implicit valueConverter: (FileParamHolder)=>Box[T]): FileFieldHolder[T,T,T] = { - FileFieldHolder(selector, Nil, Nil)(valueConverter) + FileFieldHolder(selector, Nil, Nil, Nil)(valueConverter) } // Basic select fields. @@ -36,7 +36,7 @@ object Formality extends FieldValueHelpers { selectField[T](selector, values, default, false) } def selectField[T](selector: String, values: List[SelectableOption[T]], default: Box[T], asRadioButtons: Boolean): SelectFieldHolder[T,T,T] = { - SelectFieldHolder[T,T,T](selector, default, values, Nil, Nil, asRadioButtons) + SelectFieldHolder[T,T,T](selector, default, values, Nil, Nil, Nil, asRadioButtons) } def selectField[T](selector: String, values: List[(T, String)])(implicit dummy: DummyImplicit): SelectFieldHolder[T,T,T] = { selectField[T](selector, values.map(value => SelectableOption(value._1, value._2)), false) @@ -71,7 +71,7 @@ object Formality extends FieldValueHelpers { multiSelectField[T](selector, values, defaults, false) } def multiSelectField[T](selector: String, values: List[SelectableOption[T]], defaults: List[T], asCheckboxes: Boolean): MultiSelectFieldHolder[T,T,T] = { - MultiSelectFieldHolder[T,T,T](selector, defaults, values, Nil, Nil, asCheckboxes) + MultiSelectFieldHolder[T,T,T](selector, defaults, values, Nil, Nil, Nil, asCheckboxes) } def multiSelectField[T](selector: String, values: List[(T, String)])(implicit dummy: DummyImplicit): MultiSelectFieldHolder[T,T,T] = { multiSelectField[T](selector, values.map(value => SelectableOption(value._1, value._2))) @@ -99,7 +99,7 @@ object Formality extends FieldValueHelpers { } def checkboxField(selector: String, default: Boolean = false) = { - CheckboxFieldHolder(selector, default, Nil, Nil) + CheckboxFieldHolder(selector, default, Nil, Nil, Nil) } def on[T](eventName: String, handler: (T)=>JsCmd) = EventHandler[T](eventName, handler) diff --git a/src/main/scala/com/hacklanta/formality/FormalityForm.scala b/src/main/scala/com/hacklanta/formality/FormalityForm.scala index 62f1b5b..9d634c2 100644 --- a/src/main/scala/com/hacklanta/formality/FormalityForm.scala +++ b/src/main/scala/com/hacklanta/formality/FormalityForm.scala @@ -61,7 +61,7 @@ case class FormalityFormBase private[formality]() { * instead use the `withFields` or `withAjaxFields` helpers, which let you add * a group of fields all at once in proper order and leaves you with a form * that can accept submission handlers. - * + * * @return A new `FormailtyFormProto` with the given field prepended to its * list of fields. Note that you still need to call `formalize` or * `ajaxFormalize` on the form if you want to attach submission @@ -100,7 +100,7 @@ case class FormalityFormBase private[formality]() { * * Typing discussion * --------------- - * + * * shapeless lets us track our list of fields that can be String * fields, Int fields, DateMidnight fields, or any other arbitrary * type, while preserving those types and their order. As we build @@ -128,7 +128,7 @@ case class FormalityFormProto[ * instead use the `withFields` or `withAjaxFields` helpers, which let you add a group of fields * all at once in proper order and leaves you with a form * that can accept submission handlers. - * + * * @return A new `FormailtyFormProto` with the given field prepended to its * list of fields. Note that you still need to call `formalize` or * `ajaxFormalize` on that form if you want to attach submission @@ -537,7 +537,7 @@ case class AjaxFormalityForm[ def handleSubmit() = { val (valueBoxes, valueResult) = computeValues() - + submissionHandlers.foreach(_(valueBoxes)) valueResult match { From 49535ba3dda925cdd1a1fa2254a2b8ebe74561a3 Mon Sep 17 00:00:00 2001 From: Antonio Salazar Cardozo Date: Mon, 7 Nov 2016 16:04:40 -0500 Subject: [PATCH 03/10] Rework IntFieldScope a little. Allow omission of int field in certain cases. --- .../formality/FormSubmissionSpec.scala | 12 +++++++-- .../hacklanta/formality/IntFieldScope.scala | 25 +++++++++++-------- 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/src/test/scala/com/hacklanta/formality/FormSubmissionSpec.scala b/src/test/scala/com/hacklanta/formality/FormSubmissionSpec.scala index f902c4b..fa9d7fe 100644 --- a/src/test/scala/com/hacklanta/formality/FormSubmissionSpec.scala +++ b/src/test/scala/com/hacklanta/formality/FormSubmissionSpec.scala @@ -21,6 +21,7 @@ class FormSubmissionSpec extends Specification { handlerRan must_== true } + "properly convert to the target type" in new IntFieldScope { var processedValue = -1 val testForm = form.withField(formField).formalize onSuccess { value => @@ -31,6 +32,7 @@ class FormSubmissionSpec extends Specification { processedValue must_== 5 } + "properly fail on invalid input" in new IntFieldScope { var processedValue = -1 var failedValues: List[Failure] = Nil @@ -41,13 +43,14 @@ class FormSubmissionSpec extends Specification { failedValues = failures } - bindAndSubmitForm(testForm.binder, formFieldValue = "bad int") + bindAndSubmitForm(testForm.binder, formFieldValue = Some("bad int")) processedValue must_== -1 failedValues must beLike { case List(net.liftweb.common.Failure(_, _, _)) => ok } } + "provide input in ParamFailure if input was invalid" in new IntFieldScope { var failedValues: List[Failure] = Nil @@ -55,7 +58,7 @@ class FormSubmissionSpec extends Specification { failedValues = failures } - bindAndSubmitForm(testForm.binder, formFieldValue = "bad int") + bindAndSubmitForm(testForm.binder, formFieldValue = Some("bad int")) failedValues must beLike { case List(ParamFailure(_, _, _, fieldValue)) => @@ -77,6 +80,7 @@ class FormSubmissionSpec extends Specification { handlerRan must_== false } + "have no validation errors on successful submission" in new IntFieldScope { var handlerRan = false @@ -90,6 +94,7 @@ class FormSubmissionSpec extends Specification { handlerRan must_== true S.errors must haveLength(0) } + "spit out a validation error if there is one" in new IntFieldScope { val validatingField = formField ? { incoming: Int => Full("error") } val testForm = form.withField(validatingField).formalize @@ -98,6 +103,7 @@ class FormSubmissionSpec extends Specification { S.errors must haveLength(1) } + "associate the validation error with the field name" in new IntFieldScope { val validatingField = formField ? { incoming: Int => Full("error") } val testForm = form.withField(validatingField).formalize @@ -112,6 +118,7 @@ class FormSubmissionSpec extends Specification { } must haveLength(1) } } + "associate multiple validation errors with the field names" in new IntFieldScope { val validatingField = formField ? @@ -130,6 +137,7 @@ class FormSubmissionSpec extends Specification { } must haveLength(3) } } + "provide the validation errors in the failure handler as a ParamFailure param" in new IntFieldScope { var failedValues: List[Failure] = Nil diff --git a/src/test/scala/com/hacklanta/formality/IntFieldScope.scala b/src/test/scala/com/hacklanta/formality/IntFieldScope.scala index 87cc251..55c5fe7 100644 --- a/src/test/scala/com/hacklanta/formality/IntFieldScope.scala +++ b/src/test/scala/com/hacklanta/formality/IntFieldScope.scala @@ -37,7 +37,7 @@ trait IntFieldScope extends SScope { markup } - def submitForm(markup: NodeSeq, formFieldValue: String = "5", additionalValues: List[(String,String)] = Nil): Option[String] = { + def submitForm(markup: NodeSeq, formFieldValue: Option[String] = Some("5"), additionalValues: List[(String,String)] = Nil): Option[String] = { // Lift automatically does this when wrapping up the page // render. Without it, our fields won't register on submission. session.updateFunctionMap(S.functionMap, uniqueId = "render-version", when = 0l) @@ -45,26 +45,31 @@ trait IntFieldScope extends SScope { val intField = inputNameForId("int-field", markup) val submitField = inputNameForId("submit-field", markup) - for { - intFieldName <- intField - submitFieldName <- submitField - } yield { + submitField.flatMap { submitFieldName => val request = new MockHttpServletRequest(url = "/") + val baseParameters: List[(String, String)] = (submitFieldName -> "_") :: additionalValues request.parameters = - (intFieldName -> formFieldValue) :: - (submitFieldName -> "_") :: - additionalValues + { + for { + intFieldName <- intField + intFieldValue <- formFieldValue + } yield { + (intFieldName -> intFieldValue) :: baseParameters + } + } getOrElse { + baseParameters + } testReq(request) { req => session.runParams(req) } - intFieldName + intField } } // Returns the field id of the int field. - def bindAndSubmitForm(binder: (NodeSeq)=>NodeSeq, formFieldValue: String = "5", additionalValues: List[(String,String)] = Nil): Option[String] = { + def bindAndSubmitForm(binder: (NodeSeq)=>NodeSeq, formFieldValue: Option[String] = Some("5"), additionalValues: List[(String,String)] = Nil): Option[String] = { val markup = bindForm(binder) submitForm(markup, formFieldValue, additionalValues) } From 7f2710544b224f619980507f14569580dbc572f3 Mon Sep 17 00:00:00 2001 From: Antonio Salazar Cardozo Date: Mon, 7 Nov 2016 16:05:05 -0500 Subject: [PATCH 04/10] Correctly run boxed validations for empty values. --- .../hacklanta/formality/FieldHandling.scala | 26 ++++++++++++++++--- .../hacklanta/formality/ValidationSpec.scala | 26 +++++++++++++++++++ 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/src/main/scala/com/hacklanta/formality/FieldHandling.scala b/src/main/scala/com/hacklanta/formality/FieldHandling.scala index 5b23945..2b3d2d3 100644 --- a/src/main/scala/com/hacklanta/formality/FieldHandling.scala +++ b/src/main/scala/com/hacklanta/formality/FieldHandling.scala @@ -166,7 +166,12 @@ abstract class BaseFieldHolder[ * The handler function should set this `TransientRequestVar` to the * appropriate value. */ - val fieldValue = new FieldValueVar[FieldValueType]() + protected val fieldValue = new FieldValueVar[FieldValueType]() + /** + * This is the final computed field value after all validations have been + * handled. It memoizes the result of `computeFieldValue`. + */ + protected val computedFieldValue = new FieldValueVar[FieldValueType](computeFieldValue) /** * Stores the field name generated by `generateFunctionIdAndHandler`. The * current field name should be accessed from here rather than directly @@ -179,14 +184,29 @@ abstract class BaseFieldHolder[ * form submission, invokes `boxedValidations` and registers any resulting * validation errors. */ - def value = fieldValue.is + def value = computedFieldValue.is + + /** + * Computes the final field value and applies any validations. Ensures + * validations only run once no matter how many times the field value is + * accessed. + */ + protected def computeFieldValue = { + fieldValue.is pass { + case Empty => + val validationErrors = boxedValidations.reverse.flatMap(_(Empty)) + + addValidationErrors(validationErrors: _*) + case _ => + } + } /** * Adds the passed error messages, using `S.error`. Can be overridden to use * something other than `S.error`. */ protected def addValidationErrors(errorMessages: String*): Unit = { - errorMessages.foreach { errorMessage => + for (errorMessage <- errorMessages) { S.error(fieldName.is, errorMessage) } } diff --git a/src/test/scala/com/hacklanta/formality/ValidationSpec.scala b/src/test/scala/com/hacklanta/formality/ValidationSpec.scala index 30595cf..21907b0 100644 --- a/src/test/scala/com/hacklanta/formality/ValidationSpec.scala +++ b/src/test/scala/com/hacklanta/formality/ValidationSpec.scala @@ -29,6 +29,7 @@ class ValidationSpec extends Specification { handlerRan must_== false } + "have no validation errors on successful submission" in new IntFieldScope { var handlerRan = false @@ -42,6 +43,7 @@ class ValidationSpec extends Specification { handlerRan must_== true S.errors must haveLength(0) } + "spit out a validation error if there is one" in new IntFieldScope { val validatingField = formField ? { incoming: Int => Full("error") } val testForm = form.withField(validatingField).formalize @@ -50,6 +52,7 @@ class ValidationSpec extends Specification { S.errors must haveLength(1) } + "associate the validation error with the field name" in new IntFieldScope { val validatingField = formField ? { incoming: Int => Full("error") } val testForm = form.withField(validatingField).formalize @@ -64,6 +67,7 @@ class ValidationSpec extends Specification { } must haveLength(1) } } + "associate multiple validation errors with their field name" in new IntFieldScope { val validatingField = formField ? @@ -82,6 +86,7 @@ class ValidationSpec extends Specification { } must haveLength(3) } } + "associate validation errors with multiple field names if needed" in new IntFieldScope { val validatingField = formField ? @@ -117,6 +122,7 @@ class ValidationSpec extends Specification { } must haveLength(2) } } + "provide the validation errors in the failure handler as a ParamFailure param" in new IntFieldScope { var failedValues: List[Failure] = Nil @@ -136,5 +142,25 @@ class ValidationSpec extends Specification { validationErrors must_== List("error", "second error", "other error") } } + + "invoke boxed validations even if there is no value for the field" in new IntFieldScope { + override def bindForm(binder: (NodeSeq)=>NodeSeq): NodeSeq = { + val markup: NodeSeq = binder( +
{ + (inputWithId("submit-field") % ("type" -> "submit")) + }
+ ) + + markup + } + + var validatorRanTimes = 0 + val validatingField = formField ? { incoming: Box[Int] => validatorRanTimes += 1; Empty } + val testForm = form.withField(validatingField).formalize + + bindAndSubmitForm(testForm.binder) + + validatorRanTimes must_== 1 + } } } From d8f8204dbd2103eae8dd57cb5222d0f6431d6f19 Mon Sep 17 00:00:00 2001 From: Antonio Salazar Cardozo Date: Mon, 7 Nov 2016 16:05:26 -0500 Subject: [PATCH 05/10] Ignore ensime cache. --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 5657374..96071e1 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,7 @@ .DS_Store .DS_Store? .ensime +.ensime_cache/ .ensime_lucene/ .history .idea_modules/ From 6300cc4d63d60ba2ec409d95082c388ff069590d Mon Sep 17 00:00:00 2001 From: Antonio Salazar Cardozo Date: Mon, 7 Nov 2016 17:34:02 -0500 Subject: [PATCH 06/10] Set field value to failure when empty value fails validation. This changes the handling of an empty field value in terms of the reported value to the calling code to be more or less the same as the handling of regular failures. --- .../hacklanta/formality/FieldHandling.scala | 7 +- .../formality/FormSubmissionSpec.scala | 4 +- .../hacklanta/formality/IntFieldScope.scala | 22 +++--- .../hacklanta/formality/ValidationSpec.scala | 69 +++++++++++++++---- 4 files changed, 73 insertions(+), 29 deletions(-) diff --git a/src/main/scala/com/hacklanta/formality/FieldHandling.scala b/src/main/scala/com/hacklanta/formality/FieldHandling.scala index 2b3d2d3..cb5c8c2 100644 --- a/src/main/scala/com/hacklanta/formality/FieldHandling.scala +++ b/src/main/scala/com/hacklanta/formality/FieldHandling.scala @@ -192,12 +192,15 @@ abstract class BaseFieldHolder[ * accessed. */ protected def computeFieldValue = { - fieldValue.is pass { + fieldValue.is match { case Empty => val validationErrors = boxedValidations.reverse.flatMap(_(Empty)) addValidationErrors(validationErrors: _*) - case _ => + + Failure("Empty field failed validation.") ~> validationErrors + case other => + other } } diff --git a/src/test/scala/com/hacklanta/formality/FormSubmissionSpec.scala b/src/test/scala/com/hacklanta/formality/FormSubmissionSpec.scala index fa9d7fe..b8ede63 100644 --- a/src/test/scala/com/hacklanta/formality/FormSubmissionSpec.scala +++ b/src/test/scala/com/hacklanta/formality/FormSubmissionSpec.scala @@ -43,7 +43,7 @@ class FormSubmissionSpec extends Specification { failedValues = failures } - bindAndSubmitForm(testForm.binder, formFieldValue = Some("bad int")) + bindAndSubmitForm(testForm.binder, formFieldValue = "bad int") processedValue must_== -1 failedValues must beLike { @@ -58,7 +58,7 @@ class FormSubmissionSpec extends Specification { failedValues = failures } - bindAndSubmitForm(testForm.binder, formFieldValue = Some("bad int")) + bindAndSubmitForm(testForm.binder, formFieldValue = "bad int") failedValues must beLike { case List(ParamFailure(_, _, _, fieldValue)) => diff --git a/src/test/scala/com/hacklanta/formality/IntFieldScope.scala b/src/test/scala/com/hacklanta/formality/IntFieldScope.scala index 55c5fe7..bf5c7fd 100644 --- a/src/test/scala/com/hacklanta/formality/IntFieldScope.scala +++ b/src/test/scala/com/hacklanta/formality/IntFieldScope.scala @@ -37,7 +37,7 @@ trait IntFieldScope extends SScope { markup } - def submitForm(markup: NodeSeq, formFieldValue: Option[String] = Some("5"), additionalValues: List[(String,String)] = Nil): Option[String] = { + def submitForm(markup: NodeSeq, formFieldValue: String = "5", additionalValues: List[(String,String)] = Nil): Option[String] = { // Lift automatically does this when wrapping up the page // render. Without it, our fields won't register on submission. session.updateFunctionMap(S.functionMap, uniqueId = "render-version", when = 0l) @@ -48,17 +48,7 @@ trait IntFieldScope extends SScope { submitField.flatMap { submitFieldName => val request = new MockHttpServletRequest(url = "/") val baseParameters: List[(String, String)] = (submitFieldName -> "_") :: additionalValues - request.parameters = - { - for { - intFieldName <- intField - intFieldValue <- formFieldValue - } yield { - (intFieldName -> intFieldValue) :: baseParameters - } - } getOrElse { - baseParameters - } + request.parameters = parametersForForm(intField, formFieldValue) ++ baseParameters testReq(request) { req => session.runParams(req) @@ -68,8 +58,14 @@ trait IntFieldScope extends SScope { } } + def parametersForForm(intFieldName: Option[String], intFieldValue: String): List[(String,String)] = { + intFieldName.toList.map { fieldName => + (fieldName -> intFieldValue) + } + } + // Returns the field id of the int field. - def bindAndSubmitForm(binder: (NodeSeq)=>NodeSeq, formFieldValue: Option[String] = Some("5"), additionalValues: List[(String,String)] = Nil): Option[String] = { + def bindAndSubmitForm(binder: (NodeSeq)=>NodeSeq, formFieldValue: String = "5", additionalValues: List[(String,String)] = Nil): Option[String] = { val markup = bindForm(binder) submitForm(markup, formFieldValue, additionalValues) } diff --git a/src/test/scala/com/hacklanta/formality/ValidationSpec.scala b/src/test/scala/com/hacklanta/formality/ValidationSpec.scala index 21907b0..bbf2f1e 100644 --- a/src/test/scala/com/hacklanta/formality/ValidationSpec.scala +++ b/src/test/scala/com/hacklanta/formality/ValidationSpec.scala @@ -143,24 +143,69 @@ class ValidationSpec extends Specification { } } - "invoke boxed validations even if there is no value for the field" in new IntFieldScope { - override def bindForm(binder: (NodeSeq)=>NodeSeq): NodeSeq = { - val markup: NodeSeq = binder( -
{ - (inputWithId("submit-field") % ("type" -> "submit")) - }
- ) - - markup + "handle boxed validations even if there is no value for the field" in new IntFieldScope { + override def parametersForForm(intFieldName: Option[String], intFieldValue: String) = { + Nil } var validatorRanTimes = 0 - val validatingField = formField ? { incoming: Box[Int] => validatorRanTimes += 1; Empty } - val testForm = form.withField(validatingField).formalize + var seenFailures = List[Failure]() - bindAndSubmitForm(testForm.binder) + val validatingField = formField ? { incoming: Box[Int] => + validatorRanTimes += 1 + Full("had an issue") + } + val testForm = form.withField(validatingField).formalize onFailure { + case failures => + seenFailures = failures + } + + val fieldName = bindAndSubmitForm(testForm.binder) validatorRanTimes must_== 1 + seenFailures.length must_== 1 + seenFailures must beLike { + case List(ParamFailure(message, _, _, param)) => + message must_== "Empty field failed validation." + param must_== List("had an issue") + } + + fieldName must beLike { + case Some(fieldName) => + S.errors collect { + case (error, Full(name)) if fieldName == name => + error + } must haveLength(1) + } + } + + "handle boxed validations even if the field is submitted" in new IntFieldScope { + var seenFailures = List[Failure]() + + val validatingField = formField ? { incoming: Box[Int] => + Full("had an issue with " + incoming) + } + val testForm = form.withField(validatingField).formalize onFailure { + case failures => + seenFailures = failures + } + + val fieldName = bindAndSubmitForm(testForm.binder) + + seenFailures.length must_== 1 + seenFailures must beLike { + case List(ParamFailure(message, _, _, param)) => + message must_== "5 failed validations." + param must_== List("had an issue with Full(5)") + } + + fieldName must beLike { + case Some(fieldName) => + S.errors collect { + case (error, Full(name)) if fieldName == name => + error + } must haveLength(1) + } } } } From 21b9cadc1a192c343ac942cf38068374014f3425 Mon Sep 17 00:00:00 2001 From: Antonio Salazar Cardozo Date: Fri, 11 Nov 2016 18:51:19 -0500 Subject: [PATCH 07/10] Fix a few extraneous references to generateFunctionIdAndHandler. Select and multi-select fields were still calling generateFunctionIdAndHandler directly instead of indirecting through fieldName.is, which meant we had a different field name for the validations than the one being used to bind the actual fields. --- .../hacklanta/formality/FieldHandling.scala | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/main/scala/com/hacklanta/formality/FieldHandling.scala b/src/main/scala/com/hacklanta/formality/FieldHandling.scala index cb5c8c2..de118c5 100644 --- a/src/main/scala/com/hacklanta/formality/FieldHandling.scala +++ b/src/main/scala/com/hacklanta/formality/FieldHandling.scala @@ -406,14 +406,16 @@ case class SelectFieldHolder[ this.copy(eventHandlers = eventHandler :: eventHandlers) } - override val binder: CssSel = - if (asRadioButtons) + override val binder: CssSel = { + if (asRadioButtons) { radioButtonBinder - else + } else { selectBinder + } + } def radioButtonBinder = { - val functionId = generateFunctionIdAndHandler + val functionId = fieldName.is val withValidations = validations.foldLeft("nothing" #> PassThru)(_ & _.binder(selector)) val validationsAndEvents = eventHandlers.foldLeft(withValidations)(_ & _.binder(selector, optionProcessor)) @@ -467,7 +469,7 @@ case class SelectFieldHolder[ Null } - val functionId = generateFunctionIdAndHandler + val functionId = fieldName.is val select = { noncedOptions.map { option => From 1e1c93d9b60798e5825ab8f7b8b59e5a4f851b52 Mon Sep 17 00:00:00 2001 From: Antonio Salazar Cardozo Date: Fri, 11 Nov 2016 18:54:13 -0500 Subject: [PATCH 08/10] Fix some scaladoc validatingWith references. They were ambiguous after the boxed validation overload was added. --- src/main/scala/com/hacklanta/formality/FieldHandling.scala | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/main/scala/com/hacklanta/formality/FieldHandling.scala b/src/main/scala/com/hacklanta/formality/FieldHandling.scala index de118c5..0d37497 100644 --- a/src/main/scala/com/hacklanta/formality/FieldHandling.scala +++ b/src/main/scala/com/hacklanta/formality/FieldHandling.scala @@ -121,7 +121,8 @@ abstract class BaseFieldHolder[ * Note that FieldHolders are immutable; this returns a copy of this * FieldHolder with an updated validation list. * - * Aliased as validatingWith for folks who don't like operators. + * Aliased as `[[validatingWith(validation* validatingWith]]` for folks who + * don't like operators. */ def ?(validation: Validation[ValidationType]) = validatingWith(validation) @@ -140,7 +141,8 @@ abstract class BaseFieldHolder[ * Note that `FieldHolder`s are immutable; this returns a copy of this * `FieldHolder` with an updated validation list. * - * Aliased as `[[validatingWith]]` for folks who don't like operators. + * Aliased as `[[[validatingWith(boxed* validatingWith]]]` for folks who don't + * like operators. */ def ?(boxedValidation: Validation[Box[ValidationType]])(implicit dummy: DummyImplicit) = validatingWith(boxedValidation) From bc37363ac4d4cfe087528d15e3f956784cf80437 Mon Sep 17 00:00:00 2001 From: Antonio Salazar Cardozo Date: Mon, 14 Nov 2016 16:44:39 -0500 Subject: [PATCH 09/10] basicNotEmpty takes a Box. This allows basicNotEmpty to identify unsubmitted fields and blank submissions both. --- .../scala/com/hacklanta/formality/Validation.scala | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/main/scala/com/hacklanta/formality/Validation.scala b/src/main/scala/com/hacklanta/formality/Validation.scala index 5f0f40e..356a87e 100644 --- a/src/main/scala/com/hacklanta/formality/Validation.scala +++ b/src/main/scala/com/hacklanta/formality/Validation.scala @@ -13,7 +13,7 @@ import net.liftweb.util._ * A Validation that validates a value of type T. It should return a * Box[String] that is Full with a validation error if there are * validation problems, or an empty box if there are no such problems. - * + * * Validations can provide a css selector that attaches * validation-related attributes to the input field. * @@ -69,12 +69,13 @@ object basicInRange { * Example: * myField ? basicNotEmpty */ -class basicNotEmpty extends Validation[String] { - def apply(value: String) = { - if (value.isEmpty) +class basicNotEmpty extends Validation[Box[String]] { + def apply(value: Box[String]) = { + if (value.isEmpty || value.map(_.isEmpty) == Full(true)) { Full(S ? "validation.notEmpty") - else + } else { Empty + } } } object basicNotEmpty { From 5b28513a2deac3f58e1563895c5becbb3bfec0e4 Mon Sep 17 00:00:00 2001 From: Antonio Salazar Cardozo Date: Mon, 14 Nov 2016 16:44:47 -0500 Subject: [PATCH 10/10] Add note about boxed validators to README. --- README.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index d44c1eb..d26ff60 100644 --- a/README.md +++ b/README.md @@ -48,7 +48,7 @@ A basic form snippet would look like this: // Assuming a case class User(name: String, age: Int, phoneNumber: String, termsAndConditions: Boolean). User(name, age, phoneNumber, termsAndConditions).save } - + "form" #> registrationForm.binder ``` @@ -142,6 +142,15 @@ the set of validations that are supported by [Parsley.js](http://parsleyjs.org). These will have matching server-side validation implementations. +In certain very specific cases, it can be useful to run a validation even +if the value in question isn't actually submitted in the form. Typically +this is used for a check that a required field was submitted, particularly +when it comes from a checkbox or radio button. Either way, in these cases +a validation can take a `Box[T]` instead of `T`. It will receive an `Empty` +if the field is not submitted, and a `Full` with the deserialized value if +the field is submitted. Lift-formality's own `notBlank`/`notEmpty` validators +will correctly handle unsubmitted fields. + ### Event handling Formality can also add server-side event handlers to form inputs: