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

Model class crashes when handling complex validation rules #1574

Closed
hwiesmann opened this issue Dec 2, 2018 · 4 comments
Closed

Model class crashes when handling complex validation rules #1574

hwiesmann opened this issue Dec 2, 2018 · 4 comments
Labels
database Issues or pull requests that affect the database layer

Comments

@hwiesmann
Copy link


name: Bug report
about: According to the documentation in section "Configuring Your Model" the validation rules can be specified as mentioned under "How to save your rules". This is not true.


Describe the bug
Validation rules cannot be specified as described in the documentation ("Configuring Your Model"):

$validationRules

Contains either an array of validation rules as described in How to save your rules or a string containing the name of a validation group, as described in the same section. Described in more detail below.

If errors are specified in the rules validation of the model results in a crash (see attached test file). The errors have to be specified in "$validationMessages".

PS: $validationRules must allow to set the label because otherwise generic error messages have no chance to show the right value for placeholder {field}.

CodeIgniter 4 version
CodeIgniter 4.0.0 Alpha 2 release

Affected module(s)
Model

Test case

<?php namespace CodeIgniter;

use Tests\Support\Database\MockConnection;

class ModelTest extends \CIUnitTestCase
{
	public function testModelValidation()
	{
		$testModel = $this->getModel();
		$this->assertEquals($testModel->validate(['foo' => '1234567']),true);
	}

	protected function getEntity()
	{
		return new class extends Entity
		{
			protected $foo;
		};
	}
	
	protected function getModel()
	{
		return new class extends Model
		{
			protected $db;

			public function __construct()
			{
				$db = new MockConnection([]);
				parent::__construct($db);
				
				$this->validationRules = ['foo' => ['errors' => ['min_length' => 'The length of foo should be at least 8 characters!'],
																						'label' => 'FOO',
																						'rules' => 'min_length[8]']];
			}
		};
	}
}

?>
@natanfelles
Copy link
Contributor

Looks like you forget to set the rules:

This:

$this->validationRules = ['foo' => ['errors' => ['min_length' => 'The length of foo should be at least 8 characters!'],

Must be:

$this->validationRules = [
	'foo' => [
		'errors' => [
			'min_length' => 'The length of {field} should be at least 8 characters!',
		],
		//'label'  => 'Foo',
		'rules'  => 'min_length[8]', // must be set
	],
]

or :

$this->validationRules = [
	'foo' => 'min_length[8]',
];
$this->validationMessages = [
	'foo' => [
		'min_length' => 'The length of foo should be at least 8 characters!',
	],
];

@hwiesmann
Copy link
Author

Hi,

the bug has nothing to do with setting the rules to check. My test code crashes because Model::fillPlaceholders crashes. It crashes exactly at

$row = strtr($row, $replacements); // within foreach loop

because $row is not a string but an array (element 'errors' is an array).

@jim-parry jim-parry added the database Issues or pull requests that affect the database layer label Dec 10, 2018
@lonnieezell
Copy link
Member

I'll investigate, but your issue says it's not possible to add validation like the documentation says, but then you don't create a test case that has the rules setup correctly. @natanfelles was correct to point that out, since there's an obvious disconnect between what your issue is reporting and what your test is showing.

The ModelTest already provides a number of tests with the model and validation that is showing that it does work.

Did we get something wrong in those tests?

@hwiesmann
Copy link
Author

Hi,

I created a test case that is identical to Natan's first example. Only because of some formatting issue here the rule stuff is far moved towards the right. But it exists! I have to admit that I did not see it either when commenting on Natan's remark. Otherwise my reply would have been different.

Anyway, the error has nothing to do with the rules. Even if there are no rules Model::fillPlaceholder will be called and crashes.

And no, ModelTest does not provide a test case like I have described because ValidModel does not contain an error array like in my example.

@lonnieezell lonnieezell added this to the 4.0.0-beta1 milestone Feb 19, 2019
lonnieezell added a commit that referenced this issue Feb 27, 2019
Ensure validation works in Model with errors as part of rules. Fixes #1574
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
database Issues or pull requests that affect the database layer
Projects
None yet
Development

No branches or pull requests

4 participants