konform-kt / konform

Portable validations for Kotlin
https://www.konform.io
MIT License
651 stars 39 forks source link

How to put a hint on an empty required block? #36

Open cies opened 2 years ago

cies commented 2 years ago

From the README it is not clear how I put a hint on an empty required block.

    val validator = Validation<DealFormDto> {
      DealFormDto::closedBy required {}
    }

I get the "is required" default message when closedBy is not present. But how to set my own message?

This does not work:

    val validator = Validation<DealFormDto> {
      DealFormDto::closedBy required { hint "Closed by is missing" }
    }
cies commented 2 years ago

We now use this custom validation this in order to specify the hint on a required property:

fun <T : Any?> ValidationBuilder<T>.required() = addConstraint("is required") {
  if (it == null) {
    false
  } else {
    when (it) {
      is String -> it.isNotBlank()
      is Collection<*> -> it.isNotEmpty()
      else -> true
    }
  }
}

Which we use like this:

      FormDto::estimatedAnnualSales { required() hint "Estimated sales is required" } // Note we control the error message
      FormDto::estimatedAnnualSales ifPresent { minimum(0) hint "Estimated sales cannot be negative" }

Where before we'd do something like this:

      FormDto::estimatedAnnualSales required { minimum(0) hint "Estimated sales cannot be negative" }
      // Note we have no control over the error given when `estimatedAnnualSales` is omitted, it will always be "is required"
cies commented 2 years ago

And we now use this one as well:

/** Convenience method, same as `required()` but takes a name. */
fun <T : Any?> ValidationBuilder<T>.requiredAs(name: String) = addConstraint("{0} is required", name) {
  if (it == null) {
    false
  } else {
    when (it) {
      is String -> it.isNotBlank()
      is Collection<*> -> it.isNotEmpty()
      else -> true
    }
  }
}
nlochschmidt commented 2 years ago

Awesome that you found a solution that works using the existing extension points 👏

Which we use like this:

    FormDto::estimatedAnnualSales { required() hint "Estimated sales is required" } // Note we control the error message
    FormDto::estimatedAnnualSales ifPresent { minimum(0) hint "Estimated sales cannot be negative" }

I agree, that doesn't look very good to me

So additionally to the one design option you proposed there are at least two other design options that I think are interesting:

Option 1: Your proposed option

DealFormDto::closedBy required { hint "Closed by is missing" }

Option 2: Using optional hint parameter on required

DealFormDto::closedBy required(hint = "Closed by is missing") { }

Option 3: Using infix after the required block

DealFormDto::closedBy required { } hint "Closed by is missing"

I think Option 3 is most in line with the existing idioms and, might possibly be easier to implement as well.

dhoepelman commented 3 months ago

Option 3 makes sense. Currently many things return Unit, and they could be returning something along the lines of Constraint to which hints can be added. Alternatively, they return ValidationResultBuilder and there's a way to add hints to those. This might also be a way to solve #76