feat: [Validation] add required_if rule
#9028
Conversation
kenjis
added
enhancement
| */ | ||
| public function required_if($str = null, ?string $fieldWithValue = null, array $data = []): bool | ||
| { | ||
| if ($fieldWithValue === null || $data === []) { |
There was a problem hiding this comment.
When will data be filled up? Using your original example: ['identity_number' => 'required_if[is_internal,1]', $fieldWithValue is is_internal,1.
| * @param string|null $fieldWithValue that we should check if present | ||
| * @param array<string, mixed> $data Complete list of field from the form | ||
| */ | ||
| public function required_if($str = null, ?string $fieldWithValue = null, array $data = []): bool |
There was a problem hiding this comment.
Do we need the nullability here for both $str and $fieldWithValue?
| public function required_if($str = null, ?string $fieldWithValue = null, array $data = []): bool | ||
| { | ||
| if ($fieldWithValue === null || $data === []) { | ||
| throw new InvalidArgumentException('You must supply the parameters: field,expected_values, data.'); |
There was a problem hiding this comment.
The message is not reflecting the correct arguments.
There was a problem hiding this comment.
How about this?
throw new InvalidArgumentException('You must supply the parameters: field,value1,value2,...');| throw new InvalidArgumentException('You must supply the parameters: field,expected_values, data.'); | ||
| } | ||
|
|
||
| // Separate fields and expected values |
There was a problem hiding this comment.
Please refrain from adding comments that just describes what the code is doing. They're superfluous.
There was a problem hiding this comment.
OK, noted. i will update later.
| $expectedValues = $parts; // The remainder is the expected value | ||
|
|
||
| if (trim($field) === '' || $expectedValues === []) { | ||
| throw new InvalidArgumentException('You must supply the expected values of field: E.g. field,value1,value2,...'); |
There was a problem hiding this comment.
It may be better to have a more descriptive exception message by saying which field needs to be supplied with the expected values.
There was a problem hiding this comment.
how about this one?
throw new InvalidArgumentException("You must supply the field value: {$field},value? or you can add multiple value like this {$field},value1,value2 and so on.");| * This field is required when any of the other required fields have their expected values present | ||
| * in the data. | ||
| * | ||
| * Example (The special_option field is required when the normal_option,1,2 field has the given value.): |
There was a problem hiding this comment.
This error message is confusing:
The special_option field is required when the normal_option,1,2 field has the given value.
The other field is normal_option, not normal_option,1,2.
There was a problem hiding this comment.
I thought about that too, but the fields and values are combined in the {param} placeholder. How to extract fields and values from {param}? At least we can separate them and differentiate between fields and values then provide proper error messages. do you have any ideas?
Co-authored-by: kenjis <[email protected]>
|
👋 Hi, @warcooft! |
Description
Supersedes #9018
Checklist: