Skip to content

Conversation

@CamKem
Copy link
Contributor

@CamKem CamKem commented Feb 16, 2025

This Pull Request is a resubmit of the work from #52707 - I believe given the PR contained no breaking changes and that is a major constraint that been set for the scope of 12.x that is it worth considering inclusion of the improvements in this PR.

The PR will give users the ability to use granular error messages translation strings for more precise validation feedback for users on how the user can rectify the specific issues with the uploaded asset provided.

Currently, no matter how many constraints you set for the Image Dimensions rule, the framework only provides 1 validation message advising that the image upload is not within the constraints set.

Rule::dimension()->minWidth(100)->minHeight(100)->maxWidth(1000)->maxHeight(1000)->ratioBetween(5/2, 2/5)

I believe it's important to bring all of the Validation messages inline with the other fluent rules, and this is one of the last ones to be Macroable, DataAware & to use the framework initially setup in #43271 by @lukeraymonddowning and implemented in many other PR's on the other Fluent validation rules like #54067

Logic has been added ensure compatibility with the existing string representations of the image rule & prevent any breaking changes to existing codebases that upgrade to 12.x, as per this section here:

 // This is to correctly parse string representations for dimensions // like so "dimensions:min_width=100,min_height=200" if (! Arr::isAssoc($parameters)) { $this->requireParameterCount(1, $parameters, 'dimensions'); $parameters = $this->parseStringParameters($parameters); }

Link to the code in diff

NOTE: The only behavioural change that is of note & the reason it's more suited to a major release, is any existing codebase that has a custom translation string for the string representation of the dimension rule, eg:

'images.*.dimension' => 'The image must be between :min_width x :min_height pixels and :max_width x :max_height pixels, and have an aspect ratio between :min_ratio and :max_ratio'

Will now return the granular default messages that have been added into validation.php, so while this is not a breaking change, you may describe it as unexpected behaviour.

Having said this, I think that the additional granularity in feedback far outweighs the possibility of defaulting to the grandular message string in instances where the users have added custom messages for the string representation.

@CamKem CamKem marked this pull request as draft February 16, 2025 11:27
@CamKem CamKem closed this Feb 17, 2025
@CamKem CamKem deleted the feat/granular-dimension-msgs branch February 17, 2025 06:55
@CamKem
Copy link
Contributor Author

CamKem commented Feb 17, 2025

Closed & moved to a new branch due to issues with branch state artifacts remaining from cherry picking causing failures in GH actions tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant