search

somnambulist-tech / validation

A re-write of rakit/validation, a standalone validation library inspired by Laravel Validation
MIT License
44 stars 13 forks source link

Patch 2 #7

Closed xJuvi closed 1 year ago

xJuvi commented 2 years ago

With this PR i make it possible to change the default separator which used when rule has multiple attributes. For some rules, it is required to use the : in die rule and not as separator.

After merge this PR, the Factory get a new variable, which define the current separator.

Kind regards

dave-redfern commented 2 years ago

@xJuvi what is the use case for this change? Do you have a sample to show the issue you are trying to address?

xJuvi commented 2 years ago

Hey for example I have a rule that verify and modify timestamps. The rule can have an attribute in which format the timestamp will be modified. As devider for hour and minutes, the colon (:) it is required to change it.

Currently I use another sign and replace it into the rule. But for a clean code I thought it was better when I can change the rule attribute devider. This is also only one of few situations. Another situation is an order of. This contains also an colon and can't be verified.

Is this clear now?

Kind regards

dave-redfern commented 2 years ago

@xJuvi I added a quick test locally for the date rule and set the format to Y-m-d H:i:s making the rule: date:Y-m-d H:i:s. This worked without issue and was correctly split. Similarly, I mocked a rule that had id:ASC,createdAt:DESC as attributes and added a basic debug dump of the parsed parameters and again, these came through without issue (array containing: id:ASC and createdAt:DESC as elements).

How are you defining these rules? Can you share an example of the rules? (not any rule implementation, just how the rules have been defined).

If this is an actual issue / bug, then I do need to have an example of the breaking case and a unit test that makes appropriate assertions.

dave-redfern commented 1 year ago

I am closing this PR as I was unable to replicate and without an example showing the issue, I am reluctant to accept this change. I added additional unit tests to check specifically for handling of : in rules that all pass.

If a failing example can be provided, I will be more than happy to look at this again.