Skip to content

fix: $cleanValidationRules does not work in Model updates #6588

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Oct 3, 2022
Merged

fix: $cleanValidationRules does not work in Model updates #6588

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
756ebfe
fix: required_with does not work in Model updates with Entity
kenjis Sep 26, 2022
4010723
docs: update docs
kenjis Sep 29, 2022
75a6eb2
docs: fix typo
kenjis Sep 30, 2022
d0ca3e6
test: fix test data
kenjis Sep 30, 2022
68b50b7
docs: remove incorrect docmment
kenjis Sep 30, 2022
d83baa6
test: change method names
kenjis Sep 30, 2022
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
40 changes: 32 additions & 8 deletions system/BaseModel.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ abstract class BaseModel

/**
* Whether rules should be removed that do not exist
* in the passed in data. Used between inserts/updates.
* in the passed data. Used in updates.
*
* @var bool
*/
Expand Down Expand Up @@ -701,13 +701,23 @@ public function insert($data = null, bool $returnID = true)
{
$this->insertID = 0;

// Set $cleanValidationRules to false temporary.
$cleanValidationRules = $this->cleanValidationRules;
$this->cleanValidationRules = false;

$data = $this->transformDataToArray($data, 'insert');

// Validate data before saving.
if (! $this->skipValidation && ! $this->cleanRules()->validate($data)) {
if (! $this->skipValidation && ! $this->validate($data)) {
// Restore $cleanValidationRules
$this->cleanValidationRules = $cleanValidationRules;

return false;
}

// Restore $cleanValidationRules
$this->cleanValidationRules = $cleanValidationRules;

// Must be called first so we don't
// strip out created_at values.
$data = $this->doProtectFields($data);
Expand Down Expand Up @@ -773,6 +783,10 @@ public function insert($data = null, bool $returnID = true)
*/
public function insertBatch(?array $set = null, ?bool $escape = null, int $batchSize = 100, bool $testing = false)
{
// Set $cleanValidationRules to false temporary.
$cleanValidationRules = $this->cleanValidationRules;
$this->cleanValidationRules = false;

if (is_array($set)) {
foreach ($set as &$row) {
// If $data is using a custom class with public or protected
Expand All @@ -789,8 +803,11 @@ public function insertBatch(?array $set = null, ?bool $escape = null, int $batch
$row = (array) $row;
}

// Validate every row..
if (! $this->skipValidation && ! $this->cleanRules()->validate($row)) {
// Validate every row.
if (! $this->skipValidation && ! $this->validate($row)) {
// Restore $cleanValidationRules
$this->cleanValidationRules = $cleanValidationRules;

return false;
}

Expand All @@ -811,6 +828,9 @@ public function insertBatch(?array $set = null, ?bool $escape = null, int $batch
}
}

// Restore $cleanValidationRules
$this->cleanValidationRules = $cleanValidationRules;

return $this->doInsertBatch($set, $escape, $batchSize, $testing);
}

Expand All @@ -832,7 +852,7 @@ public function update($id = null, $data = null): bool
$data = $this->transformDataToArray($data, 'update');

// Validate data before saving.
if (! $this->skipValidation && ! $this->cleanRules(true)->validate($data)) {
if (! $this->skipValidation && ! $this->validate($data)) {
return false;
}

Expand Down Expand Up @@ -906,7 +926,7 @@ public function updateBatch(?array $set = null, ?string $index = null, int $batc
}

// Validate data before saving.
if (! $this->skipValidation && ! $this->cleanRules(true)->validate($row)) {
if (! $this->skipValidation && ! $this->validate($row)) {
return false;
}

Expand Down Expand Up @@ -1027,7 +1047,7 @@ public function onlyDeleted()
public function replace(?array $data = null, bool $returnSQL = false)
{
// Validate data before saving.
if ($data && ! $this->skipValidation && ! $this->cleanRules(true)->validate($data)) {
if ($data && ! $this->skipValidation && ! $this->validate($data)) {
return false;
}

Expand Down Expand Up @@ -1581,7 +1601,11 @@ protected function transformDataToArray($data, string $type): array
// properties representing the collection elements, we need to grab
// them as an array.
if (is_object($data) && ! $data instanceof stdClass) {
$data = $this->objectToArray($data, ($type === 'update'), true);
// If it validates with entire rules, all fields are needed.
$onlyChanged = ($this->skipValidation === false && $this->cleanValidationRules === false)
? false : ($type === 'update');

$data = $this->objectToArray($data, $onlyChanged, true);
}

// If it's still a stdClass, go ahead and convert to
Expand Down
116 changes: 116 additions & 0 deletions tests/system/Models/ValidationModelTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@
namespace CodeIgniter\Models;

use CodeIgniter\Config\Factories;
use CodeIgniter\Model;
use Config\Validation;
use stdClass;
use Tests\Support\Models\JobModel;
use Tests\Support\Models\SimpleEntity;
use Tests\Support\Models\ValidErrorsModel;
use Tests\Support\Models\ValidModel;

Expand Down Expand Up @@ -367,4 +369,118 @@ public function testGetValidationMessages(): void
$error = $this->model->getValidationMessages();
$this->assertSame('Description field is required.', $error['description']);
}

/**
* @see https://github.com/codeigniter4/CodeIgniter4/issues/6577
*/
public function testUpdateEntityWithPropertyCleanValidationRulesTrueAndCallingCleanRulesFalse()
{
$model = new class () extends Model {
protected $table = 'test';
protected $allowedFields = ['field1', 'field2', 'field3', 'field4'];
protected $returnType = SimpleEntity::class;
protected $validationRules = [
'field1' => 'required_with[field2,field3,field4]',
'field2' => 'permit_empty',
'field3' => 'permit_empty',
'field4' => 'permit_empty',
];
};

// Simulate to get the entity from the database.
$entity = new SimpleEntity();
$entity->setAttributes([
'id' => '1',
'field1' => 'value1',
'field2' => 'value2',
'field3' => '',
'field4' => '',
]);

// Change field1 value.
$entity->field1 = '';

// Set $cleanValidationRules to false by cleanRules()
$model->cleanRules(false)->save($entity);

$errors = $model->errors();
$this->assertCount(1, $errors);
$this->assertSame(
$errors['field1'],
'The field1 field is required when field2,field3,field4 is present.'
);
}

public function testUpdateEntityWithPropertyCleanValidationRulesFalse()
{
$model = new class () extends Model {
protected $table = 'test';
protected $allowedFields = ['field1', 'field2', 'field3', 'field4'];
protected $returnType = SimpleEntity::class;
protected $validationRules = [
'field1' => 'required_with[field2,field3,field4]',
'field2' => 'permit_empty',
'field3' => 'permit_empty',
'field4' => 'permit_empty',
];

// Set to false.
protected $cleanValidationRules = false;
};

// Simulate to get the entity from the database.
$entity = new SimpleEntity();
$entity->setAttributes([
'id' => '1',
'field1' => 'value1',
'field2' => 'value2',
'field3' => '',
'field4' => '',
]);

// Change field1 value.
$entity->field1 = '';

$model->save($entity);

$errors = $model->errors();
$this->assertCount(1, $errors);
$this->assertSame(
$errors['field1'],
'The field1 field is required when field2,field3,field4 is present.'
);
}

public function testInsertEntityValidateEntireRules()
{
$model = new class () extends Model {
protected $table = 'test';
protected $allowedFields = ['field1', 'field2', 'field3', 'field4'];
protected $returnType = SimpleEntity::class;
protected $validationRules = [
'field1' => 'required',
'field2' => 'required',
'field3' => 'permit_empty',
'field4' => 'permit_empty',
];
};

$entity = new SimpleEntity();
$entity->setAttributes([
'field1' => 'value1',
// field2 is missing
'field3' => '',
'field4' => '',
]);

// Insert ignores $cleanValidationRules value.
$model->insert($entity);

$errors = $model->errors();
$this->assertCount(1, $errors);
$this->assertSame(
$errors['field2'],
'The field2 field is required.'
);
}
}
26 changes: 22 additions & 4 deletions user_guide_src/source/models/model.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,10 +191,25 @@ $skipValidation
---------------

Whether validation should be skipped during all **inserts** and **updates**. The default
value is false, meaning that data will always attempt to be validated. This is
value is ``false``, meaning that data will always attempt to be validated. This is
primarily used by the ``skipValidation()`` method, but may be changed to ``true`` so
this model will never validate.

.. _clean-validation-rules:

$cleanValidationRules
---------------------

Whether validation rules should be removed that do not exist in the passed data.
This is used in **updates**.
The default value is ``true``, meaning that validation rules for the fields
that are not present in the passed data will be (temporarily) removed before the validation.
This is to avoid validation errors when updating only some fields.

You can also change the value by the ``cleanRules()`` method.

.. note:: Prior to v4.2.7, ``$cleanValidationRules`` did not work due to a bug.

$beforeInsert
-------------
$afterInsert
Expand Down Expand Up @@ -392,9 +407,12 @@ For many people, validating data in the model is the preferred way to ensure the
standard, without duplicating code. The Model class provides a way to automatically have all data validated
prior to saving to the database with the ``insert()``, ``update()``, or ``save()`` methods.

.. important:: When you update data, the validation in the model class only validate provided fields.
So when you set the rule ``required``, if you don't pass the required field data,
the validation won't fail. This is to avoid validation errors when updating only some fields.
.. important:: When you update data, by default, the validation in the model class only
validates provided fields. This is to avoid validation errors when updating only some fields.

But this means ``required*`` rules do not work as expected when updating.
If you want to check required fields, you can change the behavior by configuration.
See :ref:`clean-validation-rules` for details.

The first step is to fill out the ``$validationRules`` class property with the fields and rules that should
be applied. If you have custom error message that you want to use, place them in the ``$validationMessages`` array:
Expand Down