diff --git a/system/Model.php b/system/Model.php index 68c176f99751..1b395d3fcd63 100644 --- a/system/Model.php +++ b/system/Model.php @@ -727,6 +727,7 @@ public function update($id = null, $data = null) { if ($this->validate($data) === false) { + dd($this->errors()); return false; } } @@ -1254,17 +1255,27 @@ public function validate($data): bool $data = (array) $data; } + $rules = $this->validationRules; + $rules = $this->cleanValidationRules($rules, $data); + + // If no data existed that needs validation + // our job is done here. + if (empty($rules)) + { + return true; + } + // ValidationRules can be either a string, which is the group name, // or an array of rules. - if (is_string($this->validationRules)) + if (is_string($rules)) { - $valid = $this->validation->run($data, $this->validationRules, $this->DBGroup); + $valid = $this->validation->run($data, $rules, $this->DBGroup); } else { // Replace any placeholders (i.e. {id}) in the rules with // the value found in $data, if exists. - $rules = $this->fillPlaceholders($this->validationRules, $data); + $rules = $this->fillPlaceholders($rules, $data); $this->validation->setRules($rules, $this->validationMessages); $valid = $this->validation->run($data, null, $this->DBGroup); @@ -1275,6 +1286,33 @@ public function validate($data): bool //-------------------------------------------------------------------- + /** + * Removes any rules that apply to fields that have not been set + * currently so that rules don't block updating when only updating + * a partial row. + * + * @param array $rules + * + * @return array + */ + protected function cleanValidationRules(array $rules, array $data = null) + { + if (empty($data)) + { + return []; + } + + foreach ($rules as $field => $rule) + { + if (! array_key_exists($field, $data)) + { + unset($rules[$field]); + } + } + + return $rules; + } + /** * Replace any placeholders within the rules with the values that * match the 'key' of any properties being set. For example, if diff --git a/tests/system/Database/Live/ModelTest.php b/tests/system/Database/Live/ModelTest.php index ff45a2aef7f3..8b6e0ad37407 100644 --- a/tests/system/Database/Live/ModelTest.php +++ b/tests/system/Database/Live/ModelTest.php @@ -61,7 +61,8 @@ public function testFindActsAsGetWithNoParams() { $model = new JobModel($this->db); - $jobs = $model->asArray()->find(); + $jobs = $model->asArray() + ->find(); $this->assertCount(4, $jobs); @@ -78,7 +79,8 @@ public function testFindRespectsReturnArray() { $model = new JobModel($this->db); - $job = $model->asArray()->find(4); + $job = $model->asArray() + ->find(4); $this->assertInternalType('array', $job); } @@ -89,7 +91,8 @@ public function testFindRespectsReturnObject() { $model = new JobModel($this->db); - $job = $model->asObject()->find(4); + $job = $model->asObject() + ->find(4); $this->assertInternalType('object', $job); } @@ -98,15 +101,19 @@ public function testFindRespectsReturnObject() public function testFindRespectsSoftDeletes() { - $this->db->table('user')->where('id', 4)->update(['deleted' => 1]); + $this->db->table('user') + ->where('id', 4) + ->update(['deleted' => 1]); $model = new UserModel($this->db); - $user = $model->asObject()->find(4); + $user = $model->asObject() + ->find(4); $this->assertEmpty($user); - $user = $model->withDeleted()->find(4); + $user = $model->withDeleted() + ->find(4); // fix for PHP7.2 $count = is_array($user) ? count($user) : 1; @@ -123,7 +130,8 @@ public function testFindClearsBinds() $model->find(1); // Binds should be reset to 0 after each one - $binds = $model->builder()->getBinds(); + $binds = $model->builder() + ->getBinds(); $this->assertCount(0, $binds); $query = $model->getLastQuery(); @@ -167,7 +175,9 @@ public function testFindAllRespectsLimitsAndOffset() public function testFindAllRespectsSoftDeletes() { - $this->db->table('user')->where('id', 4)->update(['deleted' => 1]); + $this->db->table('user') + ->where('id', 4) + ->update(['deleted' => 1]); $model = new UserModel($this->db); @@ -175,7 +185,8 @@ public function testFindAllRespectsSoftDeletes() $this->assertCount(3, $user); - $user = $model->withDeleted()->findAll(); + $user = $model->withDeleted() + ->findAll(); $this->assertCount(4, $user); } @@ -186,7 +197,8 @@ public function testFirst() { $model = new UserModel(); - $user = $model->where('id >', 2)->first(); + $user = $model->where('id >', 2) + ->first(); // fix for PHP7.2 $count = is_array($user) ? count($user) : 1; @@ -198,7 +210,9 @@ public function testFirst() public function testFirstRespectsSoftDeletes() { - $this->db->table('user')->where('id', 1)->update(['deleted' => 1]); + $this->db->table('user') + ->where('id', 1) + ->update(['deleted' => 1]); $model = new UserModel(); @@ -209,7 +223,8 @@ public function testFirstRespectsSoftDeletes() $this->assertEquals(1, $count); $this->assertEquals(2, $user->id); - $user = $model->withDeleted()->first(); + $user = $model->withDeleted() + ->first(); $this->assertEquals(1, $user->id); } @@ -218,14 +233,16 @@ public function testFirstWithNoPrimaryKey() { $model = new SecondaryModel(); - $this->db->table('secondary')->insert([ - 'key' => 'foo', - 'value' => 'bar', - ]); - $this->db->table('secondary')->insert([ - 'key' => 'bar', - 'value' => 'baz', - ]); + $this->db->table('secondary') + ->insert([ + 'key' => 'foo', + 'value' => 'bar', + ]); + $this->db->table('secondary') + ->insert([ + 'key' => 'bar', + 'value' => 'baz', + ]); $record = $model->first(); @@ -243,7 +260,8 @@ public function testSaveNewRecordObject() $data->name = 'Magician'; $data->description = 'Makes peoples things dissappear.'; - $model->protect(false)->save($data); + $model->protect(false) + ->save($data); $this->seeInDatabase('job', ['name' => 'Magician']); } @@ -259,7 +277,8 @@ public function testSaveNewRecordArray() 'description' => 'That thing you do.', ]; - $result = $model->protect(false)->save($data); + $result = $model->protect(false) + ->save($data); $this->seeInDatabase('job', ['name' => 'Apprentice']); } @@ -276,7 +295,8 @@ public function testSaveUpdateRecordObject() 'description' => 'That thing you do.', ]; - $result = $model->protect(false)->save($data); + $result = $model->protect(false) + ->save($data); $this->seeInDatabase('job', ['name' => 'Apprentice']); $this->assertTrue($result); @@ -293,7 +313,8 @@ public function testSaveUpdateRecordArray() $data->name = 'Engineer'; $data->description = 'A fancier term for Developer.'; - $result = $model->protect(false)->save($data); + $result = $model->protect(false) + ->save($data); $this->seeInDatabase('job', ['name' => 'Engineer']); $this->assertTrue($result); @@ -311,7 +332,8 @@ public function testSaveProtected() $data->description = 'A fancier term for Developer.'; $data->random_thing = 'Something wicked'; // If not protected, this would kill the script. - $result = $model->protect(true)->save($data); + $result = $model->protect(true) + ->save($data); $this->assertTrue($result); } @@ -379,7 +401,8 @@ public function testDeleteNoParams() $this->seeInDatabase('job', ['name' => 'Developer']); - $model->where('id', 1)->delete(); + $model->where('id', 1) + ->delete(); $this->dontSeeInDatabase('job', ['name' => 'Developer']); } @@ -390,11 +413,14 @@ public function testPurgeDeleted() { $model = new UserModel(); - $this->db->table('user')->where('id', 1)->update(['deleted' => 1]); + $this->db->table('user') + ->where('id', 1) + ->update(['deleted' => 1]); $model->purgeDeleted(); - $users = $model->withDeleted()->findAll(); + $users = $model->withDeleted() + ->findAll(); $this->assertCount(3, $users); } @@ -405,9 +431,12 @@ public function testOnlyDeleted() { $model = new UserModel($this->db); - $this->db->table('user')->where('id', 1)->update(['deleted' => 1]); + $this->db->table('user') + ->where('id', 1) + ->update(['deleted' => 1]); - $users = $model->onlyDeleted()->findAll(); + $users = $model->onlyDeleted() + ->findAll(); $this->assertCount(1, $users); } @@ -434,6 +463,7 @@ public function testValidationBasics() $model = new ValidModel($this->db); $data = [ + 'name' => null, 'description' => 'some great marketing stuff', ]; @@ -481,7 +511,77 @@ public function testSkipValidation() 'description' => 'some great marketing stuff', ]; - $this->assertInternalType('numeric', $model->skipValidation(true)->insert($data)); + $this->assertInternalType('numeric', $model->skipValidation(true) + ->insert($data)); + } + + public function testCleanValidationRemovesAllWhenNoDataProvided() + { + $model = new Model($this->db); + $cleaner = $this->getPrivateMethodInvoker($model, 'cleanValidationRules'); + + $rules = [ + 'name' => 'required', + 'foo' => 'bar', + ]; + + $rules = $cleaner($rules, null); + + $this->assertEmpty($rules); + } + + public function testCleanValidationRemovesOnlyForFieldsNotProvided() + { + $model = new Model($this->db); + $cleaner = $this->getPrivateMethodInvoker($model, 'cleanValidationRules'); + + $rules = [ + 'name' => 'required', + 'foo' => 'required', + ]; + + $data = [ + 'foo' => 'bar', + ]; + + $rules = $cleaner($rules, $data); + + $this->assertTrue(array_key_exists('foo', $rules)); + $this->assertFalse(array_key_exists('name', $rules)); + } + + public function testCleanValidationReturnsAllWhenAllExist() + { + $model = new Model($this->db); + $cleaner = $this->getPrivateMethodInvoker($model, 'cleanValidationRules'); + + $rules = [ + 'name' => 'required', + 'foo' => 'required', + ]; + + $data = [ + 'foo' => 'bar', + 'name' => null, + ]; + + $rules = $cleaner($rules, $data); + + $this->assertTrue(array_key_exists('foo', $rules)); + $this->assertTrue(array_key_exists('name', $rules)); + } + + public function testValidationPassesWithMissingFields() + { + $model = new ValidModel(); + + $data = [ + 'foo' => 'bar', + ]; + + $result = $model->validate($data); + + $this->assertTrue($result); } //-------------------------------------------------------------------- @@ -490,7 +590,8 @@ public function testCanCreateAndSaveEntityClasses() { $model = new EntityModel($this->db); - $entity = $model->where('name', 'Developer')->first(); + $entity = $model->where('name', 'Developer') + ->first(); $this->assertInstanceOf(SimpleEntity::class, $entity); $this->assertEquals('Developer', $entity->name); @@ -593,7 +694,8 @@ public function testSetWorksWithInsert() 'email' => 'foo@example.com', 'name' => 'Foo Bar', 'country' => 'US', - ])->insert(); + ]) + ->insert(); $this->seeInDatabase('user', [ 'email' => 'foo@example.com', @@ -616,7 +718,8 @@ public function testSetWorksWithUpdate() $model->set([ 'name' => 'Fred Flintstone', - ])->update($userId); + ]) + ->update($userId); $this->seeInDatabase('user', [ 'id' => $userId, @@ -643,7 +746,8 @@ public function testSetWorksWithUpdateNoId() ->where('id', $userId) ->set([ 'name' => 'Fred Flintstone', - ])->update(); + ]) + ->update(); $this->seeInDatabase('user', [ 'id' => $userId, @@ -768,7 +872,9 @@ public function testSelectAndEntitiesSaveOnlyChangedValues() $model = new EntityModel(); - $job = $model->select('id, name')->where('name', 'Rocket Scientist')->first(); + $job = $model->select('id, name') + ->where('name', 'Rocket Scientist') + ->first(); $this->assertNull($job->description); $this->assertEquals('Rocket Scientist', $job->name); @@ -786,17 +892,19 @@ public function testUpdateNoPrimaryKey() { $model = new SecondaryModel(); - $this->db->table('secondary')->insert([ - 'key' => 'foo', - 'value' => 'bar', - ]); + $this->db->table('secondary') + ->insert([ + 'key' => 'foo', + 'value' => 'bar', + ]); $this->dontSeeInDatabase('secondary', [ 'key' => 'bar', 'value' => 'baz', ]); - $model->where('key', 'foo')->update(null, ['key' => 'bar', 'value' => 'baz']); + $model->where('key', 'foo') + ->update(null, ['key' => 'bar', 'value' => 'baz']); $this->seeInDatabase('secondary', [ 'key' => 'bar', @@ -814,8 +922,34 @@ public function testCountAllResultsRespectsSoftDeletes() // testSeeder has 4 users.... $this->assertEquals(4, $model->countAllResults()); - $model->where('name', 'Derek Jones')->delete(); + $model->where('name', 'Derek Jones') + ->delete(); $this->assertEquals(3, $model->countAllResults()); } + + /** + * @see https://github.com/codeigniter4/CodeIgniter4/issues/1584 + */ + public function testUpdateWithValidation() + { + $model = new ValidModel($this->db); + + $data = [ + 'description' => 'This is a first test!', + 'name' => 'valid', + 'id' => 42, + 'token' => 42, + ]; + + $id = $model->insert($data); + + $this->assertTrue((bool)$id); + + $data['description'] = 'This is a second test!'; + unset($data['name']); + + $result = $model->update($id, $data); + $this->assertTrue($result); + } } diff --git a/tests/system/Validation/RulesTest.php b/tests/system/Validation/RulesTest.php index 447e42dfb973..b2ddfafade9e 100644 --- a/tests/system/Validation/RulesTest.php +++ b/tests/system/Validation/RulesTest.php @@ -34,7 +34,7 @@ protected function setUp() { parent::setUp(); - $this->validation = new Validation((object) $this->config, \Config\Services::renderer()); + $this->validation = new Validation((object)$this->config, \Config\Services::renderer()); $this->validation->reset(); $_FILES = []; @@ -51,7 +51,7 @@ public function testRequiredNull() ]; $this->validation->setRules([ - 'foo' => 'required', + 'foo' => 'required|alpha', ]); $this->assertFalse($this->validation->run($data)); @@ -77,6 +77,7 @@ public function testRequiredTrueString() public function testRequiredFalseString() { $data = [ + 'foo' => null, 'bar' => 123, ]; @@ -160,7 +161,7 @@ public function ifExistProvider() ], [ ['foo' => 'required'], - [], + ['foo' => null], false, ], [ @@ -387,11 +388,12 @@ public function testDiffersFalse() public function testIsUniqueFalse() { $db = Database::connect(); - $db->table('user')->insert([ - 'name' => 'Derek Travis', - 'email' => 'derek@world.com', - 'country' => 'Elbonia', - ]); + $db->table('user') + ->insert([ + 'name' => 'Derek Travis', + 'email' => 'derek@world.com', + 'country' => 'Elbonia', + ]); $data = [ 'email' => 'derek@world.com', @@ -431,15 +433,15 @@ public function testIsUniqueIgnoresParams() { $db = Database::connect(); $user = $db->table('user') - ->insert([ - 'name' => 'Developer A', - 'email' => 'deva@example.com', - 'country' => 'Elbonia', - ]); + ->insert([ + 'name' => 'Developer A', + 'email' => 'deva@example.com', + 'country' => 'Elbonia', + ]); $row = $db->table('user') - ->limit(1) - ->get() - ->getRow(); + ->limit(1) + ->get() + ->getRow(); $data = [ 'email' => 'derek@world.co.uk',