Dummy: Add support for JSON submissions

[3v0k4]

Fixes #1618

Before submitting your PR, check that you've:

• Bumped the version number
• Documented new APIs with Haddock markup
• Added @since declarations to the Haddocks for new, public APIs (see below)

After submitting your PR:

• Update the Changelog.md file with a link to your PR
• Check that CI passes (or if it fails, for reasons unrelated to your change, like CI timeouts)

Extra stuff:

• setCredsRedirect internally does provideJsonMessage "Login Successful"; the message doesn't make much sense whenever using the plugin without making use of the session (i.e. cookie). I guess it's not a biggie? In my case, I have authenticate setup to add the user to the database so logging in actually means registering.
• It seems like Haddock does not support @since for part of a paragraph. That's why I've added This plugin supports form submissions via JSON (since 1.6.8).. Please let me know if there's a better way.
• It's my first PR in Haskell: I'm super happy to get feedback and iterate on this.

[3v0k4]

@snoyberg this is ready to review. Unfortunately, as said in the PR description I can't find a solution to the @since.

Please feel free to share all the feedback you have!

[snoyberg]

Overall looks great, just one comment about error messages.

[snoyberg]

This changes the behavior regarding error messages slightly. I'd recommend one of the following:

• Try using parseCheckJsonBody first and if that fails, try runInputPost
• Check the mime type manually and then branch on which code path to take

[3v0k4]

You mean that we don't want to show the error that parseCheckJsonBody generates (i.e. Non-JSON content type: Just "application/x-www-form-urlencoded") when the mime type was application/x-www-form-urlencoded but runInputPostResult returned something else than FormSuccess, correct?

For checking the mime type I guess I could do what Yesod.Core.Json does with lookupHeader (https://github.com/yesodweb/yesod/blob/master/yesod-core/src/Yesod/Core/Json.hs#L136)

    mct <- lookupHeader "content-type"
    case fmap (B8.takeWhile (/= ';')) mct of
        Just "application/json" -> parseInsecureJsonBody
        _ -> return $ J.Error $ "Non-JSON content type: " ++ show mct

When it comes to the first suggestion (Try using parseCheckJsonBody first and if that fails, try runInputPost), wouldn't it create the same problem as described at the beginning of this comment. I mean, if we had

    dispatch "POST" [] = do
      (jsonResult :: Result Value) <- parseCheckJsonBody
      eIdent <- case jsonResult of
        Success val -> return $ A.parseEither identParser val
        Error   err -> return $ Left err
      case eIdent of
        Right ident -> setCredsRedirect $ Creds "dummy" ident []
        Left   _        -> do
          ident <- runInputPost $ ireq textField "ident"
          setCredsRedirect $ Creds "dummy" ident []

Then when A.parseEither identParser val returns Left because the JSON is of the wrong shape in a application/json request, we would try to run runInputPost which I feel is wrong. In fact, runInputPost would be the one calling invalidArgs. And prolly that error is misleading because a JSON request should never get the error that runInputPost generates.

[snoyberg]

When it comes to the first suggestion (Try using parseCheckJsonBody first and if that fails, try runInputPost), wouldn't it create the same problem as described at the beginning of this comment.

Yes, but:

1. That would be consistent with the current behavior, so at least it's not a change
2. The error message in the JSON case is less important, since it's less likely to be shown to a user

The header check that you mention would work, and is in line with what I was thinking. But it also makes me think that we should have a function in yesod-core to check if the content type is JSON-ish, so we don't need to duplicate that code here.

[3v0k4]

Thanks for the explanation! I changed the code as you suggested:

Try using parseCheckJsonBody first and if that fails, try runInputPost

Please review whenever convenient.

I'd like to take a look at

makes me think that we should have a function in yesod-core to check if the content type is JSON-ish

Are you willing to accept a PR for that? Any recommendations before I start working on it?

[snoyberg]

Yes, I'd definitely be up for a PR for that. I didn't have any specific recommendations in mind. What do you think about pausing this PR for the moment, waiting for that new function to be available, and then updating this PR to use it? Since it's all in the same repo, I'd also be OK with simply updating this PR to add the new function to yesod-core, whichever you prefer.

[3v0k4]

If this PR is fine in your opinion, would you be willing to merge? If I manage to take care of the other one I'm happy to come back and change the code here once again

[snoyberg]

Sure, makes sense.

[snoyberg]

Thanks!