Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
bossanova808 opened this issue May 23, 2023 · 0 comments

Comments

@bossanova808
Copy link
Contributor

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!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant