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

[11.x] Fix nested rules custom attribute names #51805

Merged

Conversation

owenandrews
Copy link
Contributor

Updated PR

Reopening previously closed PR #51785 — now with tests and refactored to support the same functionality in both inline custom attributes and translated custom attributes. A new common method getAttributeFromLocalArray is used to ensure equivalent functionality when pulling from either source. This PR has been crafted to match the existing functionality of custom messages where applicable as I believe this is the expected developer experience.

Check these action results to see these tests failing against the current 11.x branch.

Original PR

Resolves an issue where wildcard custom attributes aren't matched when using NestedRules. See issue #51784.

The underlying cause of this issue is that NestedRules doesn't correctly generate the relevant implicitAttributes, meaning custom attributes cannot be connected with their explicit attributes. There doesn't seem to be a good way to fix this underlying problem without major refactoring and breaking changes.

This PR resolves this issue by iterating over the custom attributes and pattern matching their keys with the given attribute. This is consistent with the way custom messages are already resolved and therefore has some precedence.

@owenandrews owenandrews changed the title Fix nested rules custom attributes Fix nested rules custom attribute names Jun 14, 2024
@driesvints driesvints changed the title Fix nested rules custom attribute names [11.x] Fix nested rules custom attribute names Jun 17, 2024
@taylorotwell taylorotwell merged commit 07e12d3 into laravel:11.x Jun 17, 2024
28 checks passed
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

Successfully merging this pull request may close these issues.

2 participants