craft-recaptcha icon indicating copy to clipboard operation
craft-recaptcha copied to clipboard

Why return null? In Craft 4, controllers generally return a status code of 400 and a message if there is an error

Open bossanova808 opened this issue 2 years ago • 0 comments

https://github.com/matt-west/craft-recaptcha/blob/d95e16a559899dca16afd50ecb88452569ecf216/src/controllers/RecaptchaController.php#LL40C4-L40C16

I don't understand the behaviour or intention of returning null here (albeit this was the Craft 3 way I suppose).

If I e.g. use this to try and verify a user registration form, then if the recaptcha fails, I get a 404 not found response, which is...odd.

In an ajax scenario this is awkward (at best) to handle, and the flash message of Unable to verify your submission is not accessible via javascript (I believe?).

Controllers in Craft 4 should be using the new asSuccess and asFailure returns, with an optional message. See: https://craftcms.com/docs/4.x/extend/updating-plugins.html#controller-responses

So really this should be something like:

$error = "Unable to verify your submission";
return $this->asFailure($error, [], ['message' => $error]);`

That will return the correct 400 status code, set a message in the routeParams and a message in the repsonseJSON so that in your error handler you can then e.g.

                    // Handle errors coming back from Craft
                    if (request.status === 400){
                         console.log(request.responseJSON);
                    }

Would you accept a PR for this change? It will obviously change the behaviour, but would be changing it to the proper Craft 4 way (as I understand it!)

bossanova808 avatar May 23 '23 02:05 bossanova808