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

Fix: Validation. Error key for field with asterisk #5609

Merged
merged 6 commits into from
Feb 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 18 additions & 6 deletions system/Validation/Validation.php
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,12 @@ public function run(?array $data = null, ?string $group = null, ?string $dbGroup
$rules = $this->splitRules($rules);
}

$values = dot_array_search($field, $data);
$values = strpos($field, '*') !== false
? array_filter(array_flatten_with_dots($data), static fn ($key) => preg_match(
'/^' . str_replace('\.\*', '\..+', preg_quote($field, '/')) . '$/',
$key
), ARRAY_FILTER_USE_KEY)
: dot_array_search($field, $data);

if ($values === []) {
// We'll process the values right away if an empty array
Expand All @@ -148,10 +153,10 @@ public function run(?array $data = null, ?string $group = null, ?string $dbGroup
continue;
}

if (strpos($field, '*') !== false && is_array($values)) {
if (strpos($field, '*') !== false) {
// Process multiple fields
foreach ($values as $value) {
$this->processRules($field, $setup['label'] ?? $field, $value, $rules, $data);
foreach ($values as $dotField => $value) {
$this->processRules($dotField, $setup['label'] ?? $field, $value, $rules, $data);
}
} else {
// Process single field
Expand Down Expand Up @@ -626,7 +631,9 @@ protected function fillPlaceholders(array $rules, array $data): array
*/
public function hasError(string $field): bool
{
return array_key_exists($field, $this->getErrors());
$pattern = '/^' . str_replace('\.\*', '\..+', preg_quote($field, '/')) . '$/';

return (bool) preg_grep($pattern, array_keys($this->getErrors()));
}

/**
Expand All @@ -639,7 +646,12 @@ public function getError(?string $field = null): string
$field = array_key_first($this->rules);
}

return array_key_exists($field, $this->getErrors()) ? $this->errors[$field] : '';
$errors = array_filter($this->getErrors(), static fn ($key) => preg_match(
'/^' . str_replace('\.\*', '\..+', preg_quote($field, '/')) . '$/',
$key
), ARRAY_FILTER_USE_KEY);

return $errors === [] ? '' : implode("\n", $errors);
}

/**
Expand Down
46 changes: 40 additions & 6 deletions tests/system/Validation/StrictRules/ValidationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -530,10 +530,24 @@ public function testNotRealCustomRule(): void

public function testHasError(): void
{
$data = ['foo' => 'notanumber'];
$this->validation->setRules(['foo' => 'is_numeric']);
$data = [
'foo' => 'notanumber',
'bar' => [
['baz' => 'string'],
['baz' => ''],
],
];

$this->validation->setRules([
'foo' => 'is_numeric',
'bar.*.baz' => 'required',
]);

$this->validation->run($data);
$this->assertTrue($this->validation->hasError('foo'));
$this->assertTrue($this->validation->hasError('bar.*.baz'));
$this->assertFalse($this->validation->hasError('bar.0.baz'));
$this->assertTrue($this->validation->hasError('bar.1.baz'));
}

public function testSplitRulesTrue(): void
Expand Down Expand Up @@ -722,22 +736,42 @@ public function testRulesForSingleRuleWithAsteriskWillReturnError(): void
],
'name_user' => [
123,
'alpha',
'xyz098',
],
'contacts' => [
'friends' => [
['name' => ''],
['name' => 'John'],
],
],
];

$request = new IncomingRequest($config, new URI(), 'php://input', new UserAgent());

$this->validation->setRules([
'id_user.*' => 'numeric',
'name_user.*' => 'alpha',
'id_user.*' => 'numeric',
'name_user.*' => 'alpha',
'contacts.*.name' => 'required',
]);

$this->validation->withRequest($request->withMethod('post'))->run();
$this->assertSame([
'id_user.*' => 'The id_user.* field must contain only numbers.',
'name_user.*' => 'The name_user.* field may only contain alphabetical characters.',
'id_user.0' => 'The id_user.* field must contain only numbers.',
'name_user.0' => 'The name_user.* field may only contain alphabetical characters.',
'name_user.2' => 'The name_user.* field may only contain alphabetical characters.',
'contacts.friends.0.name' => 'The contacts.*.name field is required.',
], $this->validation->getErrors());

$this->assertSame(
"The name_user.* field may only contain alphabetical characters.\n"
. 'The name_user.* field may only contain alphabetical characters.',
$this->validation->getError('name_user.*')
);
$this->assertSame(
'The contacts.*.name field is required.',
$this->validation->getError('contacts.*.name')
);
}

public function testRulesForSingleRuleWithSingleValue(): void
Expand Down
46 changes: 40 additions & 6 deletions tests/system/Validation/ValidationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -633,10 +633,24 @@ public function testNotRealCustomRule(): void

public function testHasError(): void
{
$data = ['foo' => 'notanumber'];
$this->validation->setRules(['foo' => 'is_numeric']);
$data = [
'foo' => 'notanumber',
'bar' => [
['baz' => 'string'],
['baz' => ''],
],
];

$this->validation->setRules([
'foo' => 'is_numeric',
'bar.*.baz' => 'required',
]);

$this->validation->run($data);
$this->assertTrue($this->validation->hasError('foo'));
$this->assertTrue($this->validation->hasError('bar.*.baz'));
$this->assertFalse($this->validation->hasError('bar.0.baz'));
$this->assertTrue($this->validation->hasError('bar.1.baz'));
}

public function testSplitRulesTrue(): void
Expand Down Expand Up @@ -825,22 +839,42 @@ public function testRulesForSingleRuleWithAsteriskWillReturnError(): void
],
'name_user' => [
123,
'alpha',
'xyz098',
],
'contacts' => [
'friends' => [
['name' => ''],
['name' => 'John'],
],
],
];

$request = new IncomingRequest($config, new URI(), 'php://input', new UserAgent());

$this->validation->setRules([
'id_user.*' => 'numeric',
'name_user.*' => 'alpha',
'id_user.*' => 'numeric',
'name_user.*' => 'alpha',
'contacts.*.name' => 'required',
]);

$this->validation->withRequest($request->withMethod('post'))->run();
$this->assertSame([
'id_user.*' => 'The id_user.* field must contain only numbers.',
'name_user.*' => 'The name_user.* field may only contain alphabetical characters.',
'id_user.0' => 'The id_user.* field must contain only numbers.',
'name_user.0' => 'The name_user.* field may only contain alphabetical characters.',
'name_user.2' => 'The name_user.* field may only contain alphabetical characters.',
'contacts.friends.0.name' => 'The contacts.*.name field is required.',
], $this->validation->getErrors());

$this->assertSame(
"The name_user.* field may only contain alphabetical characters.\n"
. 'The name_user.* field may only contain alphabetical characters.',
$this->validation->getError('name_user.*')
);
$this->assertSame(
'The contacts.*.name field is required.',
$this->validation->getError('contacts.*.name')
);
}

public function testRulesForSingleRuleWithSingleValue(): void
Expand Down
2 changes: 2 additions & 0 deletions user_guide_src/source/changelogs/v4.2.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ Changes
- Update minimal PHP requirement to 7.4.
- The current version of Content Security Policy (CSP) outputs one nonce for script and one for style tags. The previous version outputted one nonce for each tag.
- The process of sending cookies has been moved to the ``Response`` class. Now the ``Session`` class doesn't send cookies, set them to the Response.
- Validation. Changed generation of errors when using fields with a wildcard (*). Now the error key contains the full path. See :ref:`validation-getting-all-errors`.
- ``Validation::getError()`` when using a wildcard will return all found errors matching the mask as a string.

Deprecations
************
Expand Down
34 changes: 34 additions & 0 deletions user_guide_src/source/libraries/validation.rst
Original file line number Diff line number Diff line change
Expand Up @@ -603,6 +603,8 @@ We can simply use the language lines defined in this file, like this::
]
);

.. _validation-getting-all-errors:

Getting All Errors
==================

Expand All @@ -618,6 +620,26 @@ If you need to retrieve all error messages for failed fields, you can use the ``

If no errors exist, an empty array will be returned.

When using a wildcard, the error will point to a specific field, replacing the asterisk with the appropriate key/keys.::

// for data
'contacts' => [
'friends' => [
[
'name' => 'Fred Flinstone',
],
[
'name' => '',
],
]
]

// rule
contacts.*.name => 'required'

// error will be
'contacts.friends.1.name' => 'The contacts.*.name field is required.',

Getting a Single Error
======================

Expand All @@ -628,6 +650,8 @@ name::

If no error exists, an empty string will be returned.

.. note:: When using a wildcard, all found errors that match the mask will be combined into one line separated by the EOL character.

Check If Error Exists
=====================

Expand All @@ -637,6 +661,16 @@ You can check to see if an error exists with the ``hasError()`` method. The only
echo $validation->getError('username');
}

When specifying a field with a wildcard, all errors matching the mask will be checked.::

// for errors
[
'foo.0.bar' => 'Error',
'foo.baz.bar' => 'Error',
]

$validation->hasError('foo.*.bar'); // return true

Customizing Error Display
*************************

Expand Down