Skip to content

Commit 756ebfe

Browse files
kenjiskenjis
committed
fix: required_with does not work in Model updates with Entity
1. Validation in insert()/insertBatch() always uses entire rules (force to set $this->cleanValidationRules = false). 2. Validation in update()/updateBatch()/replace() uses $this->cleanValidationRules value. 3. You can change $this->cleanValidationRules value with cleanRules(). 4. If $this->cleanValidationRules = false, get all data ($onlyChanged = false) from the Entity.
1 parent dbb3426 commit 756ebfe

File tree

2 files changed

+149
-8
lines changed

2 files changed

+149
-8
lines changed

‎system/BaseModel.php

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ abstract class BaseModel
189189

190190
/**
191191
* Whether rules should be removed that do not exist
192-
* in the passed in data. Used between inserts/updates.
192+
* in the passed data. Used in updates.
193193
*
194194
* @var bool
195195
*/
@@ -701,13 +701,23 @@ public function insert($data = null, bool $returnID = true)
701701
{
702702
$this->insertID = 0;
703703

704+
// Set $cleanValidationRules to false temporary.
705+
$cleanValidationRules = $this->cleanValidationRules;
706+
$this->cleanValidationRules = false;
707+
704708
$data = $this->transformDataToArray($data, 'insert');
705709

706710
// Validate data before saving.
707-
if (! $this->skipValidation && ! $this->cleanRules()->validate($data)) {
711+
if (! $this->skipValidation && ! $this->validate($data)) {
712+
// Restore $cleanValidationRules
713+
$this->cleanValidationRules = $cleanValidationRules;
714+
708715
return false;
709716
}
710717

718+
// Restore $cleanValidationRules
719+
$this->cleanValidationRules = $cleanValidationRules;
720+
711721
// Must be called first so we don't
712722
// strip out created_at values.
713723
$data = $this->doProtectFields($data);
@@ -773,6 +783,10 @@ public function insert($data = null, bool $returnID = true)
773783
*/
774784
public function insertBatch(?array $set = null, ?bool $escape = null, int $batchSize = 100, bool $testing = false)
775785
{
786+
// Set $cleanValidationRules to false temporary.
787+
$cleanValidationRules = $this->cleanValidationRules;
788+
$this->cleanValidationRules = false;
789+
776790
if (is_array($set)) {
777791
foreach ($set as &$row) {
778792
// If $data is using a custom class with public or protected
@@ -789,8 +803,11 @@ public function insertBatch(?array $set = null, ?bool $escape = null, int $batch
789803
$row = (array) $row;
790804
}
791805

792-
// Validate every row..
793-
if (! $this->skipValidation && ! $this->cleanRules()->validate($row)) {
806+
// Validate every row.
807+
if (! $this->skipValidation && ! $this->validate($row)) {
808+
// Restore $cleanValidationRules
809+
$this->cleanValidationRules = $cleanValidationRules;
810+
794811
return false;
795812
}
796813

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

831+
// Restore $cleanValidationRules
832+
$this->cleanValidationRules = $cleanValidationRules;
833+
814834
return $this->doInsertBatch($set, $escape, $batchSize, $testing);
815835
}
816836

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

834854
// Validate data before saving.
835-
if (! $this->skipValidation && ! $this->cleanRules(true)->validate($data)) {
855+
if (! $this->skipValidation && ! $this->validate($data)) {
836856
return false;
837857
}
838858

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

908928
// Validate data before saving.
909-
if (! $this->skipValidation && ! $this->cleanRules(true)->validate($row)) {
929+
if (! $this->skipValidation && ! $this->validate($row)) {
910930
return false;
911931
}
912932

@@ -1027,7 +1047,7 @@ public function onlyDeleted()
10271047
public function replace(?array $data = null, bool $returnSQL = false)
10281048
{
10291049
// Validate data before saving.
1030-
if ($data && ! $this->skipValidation && ! $this->cleanRules(true)->validate($data)) {
1050+
if ($data && ! $this->skipValidation && ! $this->validate($data)) {
10311051
return false;
10321052
}
10331053

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

15871611
// If it's still a stdClass, go ahead and convert to

‎tests/system/Models/ValidationModelTest.php

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,11 @@
1212
namespace CodeIgniter\Models;
1313

1414
use CodeIgniter\Config\Factories;
15+
use CodeIgniter\Model;
1516
use Config\Validation;
1617
use stdClass;
1718
use Tests\Support\Models\JobModel;
19+
use Tests\Support\Models\SimpleEntity;
1820
use Tests\Support\Models\ValidErrorsModel;
1921
use Tests\Support\Models\ValidModel;
2022

@@ -367,4 +369,119 @@ public function testGetValidationMessages(): void
367369
$error = $this->model->getValidationMessages();
368370
$this->assertSame('Description field is required.', $error['description']);
369371
}
372+
373+
/**
374+
* @see https://github.com/codeigniter4/CodeIgniter4/issues/6577
375+
*/
376+
public function testUpdateEntityWithCleanRulesFalse()
377+
{
378+
$model = new class () extends Model {
379+
protected $table = 'test';
380+
protected $allowedFields = ['field1', 'field2', 'field3', 'field4'];
381+
protected $returnType = SimpleEntity::class;
382+
protected $validationRules = [
383+
'field1' => 'required_with[field2,field3,field4]',
384+
'field2' => 'permit_empty',
385+
'field3' => 'permit_empty',
386+
'field4' => 'permit_empty',
387+
];
388+
};
389+
390+
// Simulate to get the entity from the database.
391+
$entity = new SimpleEntity();
392+
$entity->setAttributes([
393+
'id' => '1',
394+
'field1' => 'value1',
395+
'field2' => 'value2',
396+
'field3' => '',
397+
'field4' => '',
398+
]);
399+
400+
// Change field1 value.
401+
$entity->field1 = '';
402+
403+
// Set $cleanValidationRules to false.
404+
$model->cleanRules(false)->save($entity);
405+
406+
$errors = $model->errors();
407+
$this->assertCount(1, $errors);
408+
$this->assertSame(
409+
$errors['field1'],
410+
'The field1 field is required when field2,field3,field4 is present.'
411+
);
412+
}
413+
414+
public function testUpdateEntityWithCleanValidationRulesFalse()
415+
{
416+
$model = new class () extends Model {
417+
protected $table = 'test';
418+
protected $allowedFields = ['field1', 'field2', 'field3', 'field4'];
419+
protected $returnType = SimpleEntity::class;
420+
protected $validationRules = [
421+
'field1' => 'required_with[field2,field3,field4]',
422+
'field2' => 'permit_empty',
423+
'field3' => 'permit_empty',
424+
'field4' => 'permit_empty',
425+
];
426+
427+
// Set to false.
428+
protected $cleanValidationRules = false;
429+
};
430+
431+
// Simulate to get the entity from the database.
432+
$entity = new SimpleEntity();
433+
$entity->setAttributes([
434+
'id' => '1',
435+
'field1' => 'value1',
436+
'field2' => 'value2',
437+
'field3' => '',
438+
'field4' => '',
439+
]);
440+
441+
// Change field1 value.
442+
$entity->field1 = '';
443+
444+
// Set $cleanValidationRules to false.
445+
$model->save($entity);
446+
447+
$errors = $model->errors();
448+
$this->assertCount(1, $errors);
449+
$this->assertSame(
450+
$errors['field1'],
451+
'The field1 field is required when field2,field3,field4 is present.'
452+
);
453+
}
454+
455+
public function testInsertEntityValidateEntireRules()
456+
{
457+
$model = new class () extends Model {
458+
protected $table = 'test';
459+
protected $allowedFields = ['field1', 'field2', 'field3', 'field4'];
460+
protected $returnType = SimpleEntity::class;
461+
protected $validationRules = [
462+
'field1' => 'required',
463+
'field2' => 'required',
464+
'field3' => 'permit_empty',
465+
'field4' => 'permit_empty',
466+
];
467+
};
468+
469+
$entity = new SimpleEntity();
470+
$entity->setAttributes([
471+
'field1' => 'value1',
472+
'field2' => '',
473+
'field3' => '',
474+
'field4' => '',
475+
]);
476+
477+
// Insert ignores $cleanValidationRules value.
478+
$model->insert($entity);
479+
480+
$errors = $model->errors();
481+
$this->assertCount(1, $errors);
482+
$this->assertSame(
483+
$errors['field2'],
484+
'The field2 field is required.'
485+
);
486+
}
370487
}

0 commit comments

Comments
 (0)