mrkkrp/forma

Composability of the Two-Step-Validation Approach

sabine opened this issue · 7 comments

Hey,

today I stumbled upon your blog post about forma and found it very useful.

  1. I'm a newbie to Haskell/Purescript (who has written a bit of Elm) and
  2. I'm currently figuring out how to make a reasonable form building/validation library for Purescript.

Your idea of first performing applicative validation, and then, if successful, performing another validation in a monad looks interesting. I've been able to port the frontend-relevant parts of your code to purescript and get a rough sketch of an integration with purescript-halogen going.

I'm not 100% sold on doing applicative validation on the full form, then monadic validation on the resulting value. I may be missing a simple step to make the two-step approach to validation composable since I'm really quite clueless. :)

Imagine the classic signup form scenario:

  1. three form fields for Username, Password and PasswordConfirmation
  2. the password and password-confirmation fields need to be non-empty and match. This can be validated in an applicative fashion since these two fields can be composed into a formlet that returns a Tuple on which we can map another applicative validation.
  3. for the username field, we need to run the side-effect of checking whether the username is already taken (on the frontend, this can happen via AJAX, on the backend this happens by accessing the database).

With the current code, it looks to me like we can only validate whether the username is taken after the applicative validation of the full form succeeds, i.e. after the user at least entered something in the two password fields.

One way around that is to do the password-related checks in the monadic validation. Then, we can first validate that the username is still available and then check that the passwords match. Yet, if the monadic validation stops after the username is found to be unavailable, the user will only learn that their passwords didn't match after they found and entered a username that is available. It would be good to be able to show errors as soon as we know about them, both in the backend and in the frontend.

I do think that a solution could either lie in

  1. making the two-step approach composable, i.e. allow to define both validation steps, applicative and monadic, on any formlet in such a way that, for every sub-formlet, starting with the individual fields, we first run applicative, then monadic validation.
  2. somehow making a validation monad that doesn't stop at the first error but continues to run and collect errors/results when the particular validation doesn't depend on a previous validation result.

Does that make sense or am I rambling on about something that is a non-issue?

With the current code, it looks to me like we can only validate whether the username is taken after the applicative validation of the full form succeeds

I don't think this is the case, we can use a validator that lives in a database-enabled monad for the username field. Here, I've written a short example (in preudocode):

type SignupFields = '["username", "password", "password_confirmation"]

data SignupForm = SignupForm
  { loginUsername   :: Text
  , loginPassword   :: Text
  , loginPasswordConfirmation :: Text
  }

-- The DatabaseEnabled constraint is just a dummy thing here indicating that
-- the monad is capable of performing queries against a database.

signupForm :: (Monad m, DatabaseEnabled m) => FormParser SignupFields m SignupForm
signupForm = SignupForm
  <$> field @"username" (notEmpty >=> vacantName)
  <*> field @"password" notEmpty
  <*> field' @"password_confirmation" notEmpty

vacantName :: (Monad m, DatabaseEnabled m) => Text -> ExceptT Text m Text
vacantName username = do
  present <- lift (checkThatUsernameIsPresent username)
  if present
    then throwError "This username has been taken."
    else return username

notEmpty :: Monad m => Text -> ExceptT Text m Text
notEmpty txt =
  if T.null txt
    then throwError "This field cannot be empty."
    else return txt

With this code, validation of the "username" field will happen independently of password-related checks. You seem to miss the point that although individual checks are composed applicatively, but it doesn't mean they can't have monadic effects in the underlying monad (m in FormParser SignupFields m SignupForm). vacantName shows that it's perfectly possible to lookup something in database as part of those applicative checks.

About passwords: you can either check that they match in monadic part or merge them into one composite field and use applicative validation, the both approaches are OK.

Yes, I think I missed this because my intuition regarding monads / applicative isn't good at all yet. :)

So it is actually so that we can already use the monad in the validators that we put into the (applicatively-composable) form(let). That makes a lot more sense than what I thought. I have to try this, but I think you're right.

Followup question: Doesn't that mean we don't really need the library to explicitly support the second validation step? After all, wouldn't mapping another monadic validation over the complete form give an equivalent result?

No, it wouldn't (well with some hacking you could manipulate the resulting JSON answer and adjust it anyway, but it's a fragile approach). The point is that when you validate with this second step, you can seamlessly make your validation errors appear just like the ones generated in the first step. Without this, you would need to show a message at the top of your page or something (that's what I had to do with Yesod). The second step allows you re-check things easily and the library will handle formatting of the JSON answer for you.

I'm totally with you that hacking the resulting JSON after the fact is bad and that a form library should offer a flexible/convenient way to say on what names global validation errors should display.

There's probably a reason somewhere that I don't see that makes having a function like this impossible:

addValidation :: Monad m => FormParser names m a -> (a -> Either (FieldError names) b) -> FormParser names m b
addValidation (FormParser f) check = FormParser $ \v -> do
    r <- f v
    case r of
        Succeeded x ->
            case check x of
                Left err ->
                    ValidationFailed err
                Right val ->
                    Succeeded val 
        e ->
            return e

validatePasswordsMatch :: FieldError names -> Either (FieldError names) String 
validatePasswordsMatch error (Tuple p1 p2) =
    if p1 == p2 then Right p1 else Left error

newPasswordForm :: Monad m => FormParser SignupFields m String
newPasswordForm =
    addValidation
        (Tuple <$> field @"password" notEmpty <*> field @"password_confirmation" notEmpty)
        (validatePasswordsMatch (mkFieldError @"password_confirmation" "Passwords don't match!"))

My vague impression is that, if there's a way to add FieldErrors to the validation result when wrapping a validation around a FormParser, you do have full control over where the errors will appear in the form. (You could even use names that don't correspond to a particular input field, if you wanted, I assume.)

I tried this in my simplified purescript code and it seems to work there, but it also means that any of my forms that use addValidation require m to be a monad instead of just being an applicative, since I need to run r <- f v. Still, my code (which is supposed to run as part of a SPA in the browser) is quite a bit shorter/simpler because I don't have to do JSON parsing because I assume the form will get its input values provided as a Map, and I haven't implemented anything for JSON serializing of the form yet.

Feel free to laugh if this is bullshit. I'm in way over my head here with monads and all that, so I'm very grateful if you can point out where I'm wrong.

Edit: syntax highlighting added and some types.

I like the idea with the addValidation function, I actually think this is a missing thing in the package. Not that it would remove the necessity of the callback in runForm (it's also used to perform actual actions like database queries creating a user, etc., not only for aborting), but it's certainly a valid and powerful addition.

Do you want to try to make a PR adding it?

Note though that the checker should have the type a -> ExceptT e m a, like in field.

the callback in runForm [is] also used to perform actual actions like database queries creating a user, etc., not only for aborting

ah, I see. That makes sense. There's other reasons than validation why executing a POST can fail on the server and those should be reported back as form errors on the appropriate fields as well. ✔️ Yes, I do like that. With addValidation, one could keep all the validation-related things inside the FormParser and the callback's single responsibility could be to perform whatever effect happens when POSTing a valid form (e.g. attempt to mutate the database, etc.). That sounds really nice.

I will refactor the Purescript code I have to not do a second validation step since it's not needed on the frontend. What a frontend form lib instead needs is

  • to merge server-side errors into the form view (which I think will be simple when the server and frontend both use the same names), and
  • to generate JSON output to send to the server, when a valid form is submitted.

Note though that the checker should have the type a -> ExceptT e m a, like in field

Agreed, that does make sense. I've been going overboard with simplifying things in some places just to quickly have something I can run without solving too many type puzzles.

Using ExceptT e m a does help with making the checkers composable, too, right?

Do you want to try to make a PR adding it?

Sure, it would be educational for me to at least make an attempt and find out where or if I get stuck. The task does look to be achievable.