Skip to content

Commit a4939f2

Browse files
committed
Don't run validation rules for data that's not getting set. Fixes #1584
1 parent 2ae38d4 commit a4939f2

2 files changed

Lines changed: 106 additions & 11 deletions

File tree

system/Model.php

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -727,6 +727,7 @@ public function update($id = null, $data = null)
727727
{
728728
if ($this->validate($data) === false)
729729
{
730+
dd($this->errors());
730731
return false;
731732
}
732733
}
@@ -1253,18 +1254,28 @@ public function validate($data): bool
12531254
{
12541255
$data = (array) $data;
12551256
}
1256-
dd($this->validationRules);
1257+
1258+
$rules = $this->validationRules;
1259+
$rules = $this->cleanValidationRules($rules, $data);
1260+
1261+
// If no data existed that needs validation
1262+
// our job is done here.
1263+
if (empty($rules))
1264+
{
1265+
return true;
1266+
}
1267+
12571268
// ValidationRules can be either a string, which is the group name,
12581269
// or an array of rules.
1259-
if (is_string($this->validationRules))
1270+
if (is_string($rules))
12601271
{
1261-
$valid = $this->validation->run($data, $this->validationRules, $this->DBGroup);
1272+
$valid = $this->validation->run($data, $rules, $this->DBGroup);
12621273
}
12631274
else
12641275
{
12651276
// Replace any placeholders (i.e. {id}) in the rules with
12661277
// the value found in $data, if exists.
1267-
$rules = $this->fillPlaceholders($this->validationRules, $data);
1278+
$rules = $this->fillPlaceholders($rules, $data);
12681279

12691280
$this->validation->setRules($rules, $this->validationMessages);
12701281
$valid = $this->validation->run($data, null, $this->DBGroup);
@@ -1275,6 +1286,33 @@ public function validate($data): bool
12751286

12761287
//--------------------------------------------------------------------
12771288

1289+
/**
1290+
* Removes any rules that apply to fields that have not been set
1291+
* currently so that rules don't block updating when only updating
1292+
* a partial row.
1293+
*
1294+
* @param array $rules
1295+
*
1296+
* @return array
1297+
*/
1298+
protected function cleanValidationRules(array $rules, array $data = null)
1299+
{
1300+
if (empty($data))
1301+
{
1302+
return [];
1303+
}
1304+
1305+
foreach ($rules as $field => $rule)
1306+
{
1307+
if (! array_key_exists($field, $data))
1308+
{
1309+
unset($rules[$field]);
1310+
}
1311+
}
1312+
1313+
return $rules;
1314+
}
1315+
12781316
/**
12791317
* Replace any placeholders within the rules with the values that
12801318
* match the 'key' of any properties being set. For example, if

tests/system/Database/Live/ModelTest.php

Lines changed: 64 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -515,17 +515,73 @@ public function testSkipValidation()
515515
->insert($data));
516516
}
517517

518-
public function testValidationSkipsNonExistingFields()
518+
public function testCleanValidationRemovesAllWhenNoDataProvided()
519519
{
520-
$model = new ValidModel($this->db);
520+
$model = new Model($this->db);
521+
$cleaner = $this->getPrivateMethodInvoker($model, 'cleanValidationRules');
522+
523+
$rules = [
524+
'name' => 'required',
525+
'foo' => 'bar',
526+
];
527+
528+
$rules = $cleaner($rules, null);
529+
530+
$this->assertEmpty($rules);
531+
}
532+
533+
public function testCleanValidationRemovesOnlyForFieldsNotProvided()
534+
{
535+
$model = new Model($this->db);
536+
$cleaner = $this->getPrivateMethodInvoker($model, 'cleanValidationRules');
537+
538+
$rules = [
539+
'name' => 'required',
540+
'foo' => 'required',
541+
];
521542

522543
$data = [
523-
'description' => 'some great marketing stuff',
524-
'id' => 42,
525-
'token' => 42,
544+
'foo' => 'bar',
545+
];
546+
547+
$rules = $cleaner($rules, $data);
548+
549+
$this->assertTrue(array_key_exists('foo', $rules));
550+
$this->assertFalse(array_key_exists('name', $rules));
551+
}
552+
553+
public function testCleanValidationReturnsAllWhenAllExist()
554+
{
555+
$model = new Model($this->db);
556+
$cleaner = $this->getPrivateMethodInvoker($model, 'cleanValidationRules');
557+
558+
$rules = [
559+
'name' => 'required',
560+
'foo' => 'required',
526561
];
527562

528-
$this->assertEquals(5, $model->insert($data));
563+
$data = [
564+
'foo' => 'bar',
565+
'name' => null,
566+
];
567+
568+
$rules = $cleaner($rules, $data);
569+
570+
$this->assertTrue(array_key_exists('foo', $rules));
571+
$this->assertTrue(array_key_exists('name', $rules));
572+
}
573+
574+
public function testValidationPassesWithMissingFields()
575+
{
576+
$model = new ValidModel();
577+
578+
$data = [
579+
'foo' => 'bar',
580+
];
581+
582+
$result = $model->validate($data);
583+
584+
$this->assertTrue($result);
529585
}
530586

531587
//--------------------------------------------------------------------
@@ -893,6 +949,7 @@ public function testUpdateWithValidation()
893949
$data['description'] = 'This is a second test!';
894950
unset($data['name']);
895951

896-
$this->assertTrue($model->update($id, $data));
952+
$result = $model->update($id, $data);
953+
$this->assertTrue($result);
897954
}
898955
}

0 commit comments

Comments
 (0)