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

sfValidatorDate throws sfValidatorError with array as $value and therefore not usable in the message #317

Open
flohoss opened this issue Jan 19, 2024 · 6 comments
Assignees

Comments

@flohoss
Copy link

flohoss commented Jan 19, 2024

When using the Date Validator and providing a Date 2024-11-31 (only 30 Days in November), the Validator correctly throws a sfValidatorError but with the value as array. This happens multiple times in that file...

The exact position it happend is inside the convertDateArrayToString function where the value is an array but not yet converted to a string.

// convert array to date string
if (is_array($value))
{
  $value = $this->convertDateArrayToString($value);
}

image

When trying to use %value% in the error message it won't be replaced, because getArguments of the class correctly checks for an array and returns nothing in the end. Therefore nothing of the error message will be replaced.

image

@connorhu
Copy link
Collaborator

I think PR does not solve the problem, but bypasses the API of sfValidatorError. If you look at how the tests use sfValidatorError you will see that it uses the keys of the array passed in argument 3 when creating the message, in the form %key%. The problem here is that the value is an array that you cannot correctly use in the message. We need to find another solution to solve the problem.

@flohoss
Copy link
Author

flohoss commented Jan 19, 2024

The PR would give the option to use %year% %month% and %day% in the message but not %value%.
I agree, it will not solve the problem but should give an idea what the problem here is...
Let's think about it and come up with a solution. Difficult because how can you use

$clean = sprintf(
    '%04d-%02d-%02d %02d:%02d:%02d',
    (int) $value['year'],
    (int) $value['month'],
    (int) $value['day'],
    0,
    0,
    0
);

without validating the values first 😕

@connorhu
Copy link
Collaborator

connorhu commented Jan 19, 2024

My idea is create a new option for the date validator: array_value_format. If it is exists and filled with value like this: %year%-%month%-%day%. Validator convert the array back to string and you can use it in message.
This solution rather follows the format of other validator messages. On the other hand, your solution doesn't break anything, it just requires the key of the date array to be specified in the message instead of %value% placeholder. In any case, we need to write a test case for the example you provided and test the expected behavior.

@connorhu
Copy link
Collaborator

@connorhu
Copy link
Collaborator

connorhu commented Jan 20, 2024

One possible solution:
master...connorhu:symfony1:fix/issue317
Another direction is that the format can be changed via a option.
WDYT guys? // @thePanz @thirsch @alquerci

@alquerci
Copy link
Contributor

alquerci commented Jan 20, 2024

Yes, let's go this way.

I suggest being more strict.

As, the value will be displayed to end-users.
And input value is going from end-users too.

  1. To avoid the array to string conversion.
    a. Replace non-scalar item values with 0 or an empty string.
  2. To avoid undefined index.
    a. Add missing keys with 0 value
  3. Ignore extra keys like the validator do.

Hum, what is the end-user interface that use this validator ?

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 a pull request may close this issue.

3 participants