Skip to content

feat: allow FormRequest validation failures to reach controllers#10300

Closed
memleakd wants to merge 2 commits into
codeigniter4:4.8from
memleakd:feat/formrequest-continue-validation-failure
Closed

feat: allow FormRequest validation failures to reach controllers#10300
memleakd wants to merge 2 commits into
codeigniter4:4.8from
memleakd:feat/formrequest-continue-validation-failure

Conversation

@memleakd

Copy link
Copy Markdown
Contributor

Description

FormRequest currently assume that a validation failure owns the response: either the default redirect/JSON response, or whatever the request returns from failedValidation().

That works well for many requests, but it is too rigid for server-rendered forms where the invalid response belongs to the controller action. Sometimes the controller already has the route context, loaded resource, page surface, or view composition needed to render the failed form correctly.

This PR keeps the default behavior unchanged. If failedValidation() returns a ResponseInterface, the framework still short-circuits and sends it immediately.

The only new path is that failedValidation() may now return null. In that case, the resolved FormRequest is injected into the controller, and the controller can handle the failed validation itself.

This avoids adding a second flag method or framework-owned error state. The request class can store whatever state the application needs, and the framework only needs to understand one thing: response means stop, null means continue.

Tests cover the default short-circuit behavior, the new controller-handled path, prepared validation data, and authorization still stopping before validation.

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value (without duplication)
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

Let `FormRequest::failedValidation()` return `null` when an application wants the
controller action to render the invalid response.

- Keep existing ResponseInterface short-circuit behavior
- Preserve authorization failures as immediate responses
- Document controller-handled validation failures

Signed-off-by: memleakd <121398829+memleakd@users.noreply.github.com>
@github-actions github-actions Bot added the 4.8 PRs that target the `4.8` branch. label Jun 11, 2026
Signed-off-by: memleakd <121398829+memleakd@users.noreply.github.com>

@michalsn michalsn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A FormRequest parameter should mean "this request has been authorized and validated". Allowing failedValidation() to return null and continue into the controller weakens that contract, because the controller can now receive a FormRequest that did not pass validation. That feels surprising.

Also, failedValidation() can already return any ResponseInterface, including a rendered view or fragment response. So the common server-rendered form use case does not require continuing into the controller.

For cases where the controller really needs to handle invalid input, regular controller validation seems clearer and more explicit than using a FormRequest whose validation may have failed.

So, I don't think this is a good direction for FormRequest, but let's wait and see what others think.

@memleakd

Copy link
Copy Markdown
Contributor Author

Thank you for the review @michalsn

After thinking about it more, I agree this is not the right direction for FormRequest. #10259 already covers the simpler HTML response cases, and the more context-heavy cases are probably better kept explicit in the controller.

I'm going to close this PR so we don't spend more review time on a direction that does not feel quite right.

@memleakd memleakd closed this Jun 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4.8 PRs that target the `4.8` branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants