Merge pull request #1981 from codeigniter4/softdelete

Using soft deletes should not return an ambiguous field message when joining tables. Closes #1881
codeigniter4 · May 2, 2019 · e5be1f1 · e5be1f1
2 parents b56dac9 + f07188b
commit e5be1f1
Show file tree

Hide file tree

Showing 2 changed files with 85 additions and 39 deletions.
diff --git a/system/Model.php b/system/Model.php
@@ -322,7 +322,7 @@ public function find($id = null)
 
 		if ($this->tempUseSoftDeletes === true)
 		{
-			$builder->where($this->deletedField, 0);
+			$builder->where($this->table . '.' . $this->deletedField, 0);
 		}
 
 		if (is_array($id))
@@ -361,7 +361,6 @@ public function find($id = null)
 	 * @param string $columnName
 	 *
 	 * @return array|null   The resulting row of data, or null if no data found.
-	 *
 	 */
 	public function findColumn(string $columnName)
 	{
@@ -371,10 +370,10 @@ public function findColumn(string $columnName)
 		}
 
 		$resultSet = $this->select($columnName)
-		                  ->asArray()
-		                  ->find();
+						  ->asArray()
+						  ->find();
 
-		return (!empty($resultSet)) ? array_column($resultSet, $columnName) : null;
+		return (! empty($resultSet)) ? array_column($resultSet, $columnName) : null;
 	}
 
 	//--------------------------------------------------------------------
@@ -394,7 +393,7 @@ public function findAll(int $limit = 0, int $offset = 0)
 
 		if ($this->tempUseSoftDeletes === true)
 		{
-			$builder->where($this->deletedField, 0);
+			$builder->where($this->table . '.' . $this->deletedField, 0);
 		}
 
 		$row = $builder->limit($limit, $offset)
@@ -424,7 +423,7 @@ public function first()
 
 		if ($this->tempUseSoftDeletes === true)
 		{
-			$builder->where($this->deletedField, 0);
+			$builder->where($this->table . '.' . $this->deletedField, 0);
 		}
 
 		// Some databases, like PostgreSQL, need order

diff --git a/tests/system/Database/Live/ModelTest.php b/tests/system/Database/Live/ModelTest.php
@@ -324,7 +324,7 @@ public function testSaveNewRecordArray()
 		];
 
 		$model->protect(false)
-		      ->save($data);
+			  ->save($data);
 
 		$this->seeInDatabase('job', ['name' => 'Apprentice']);
 	}
@@ -736,8 +736,8 @@ public function testCanCreateAndSaveEntityClasses()
 		$this->assertTrue($model->save($entity));
 
 		$result = $model->where('name', 'Senior Developer')
-		                ->get()
-		                ->getFirstRow();
+						->get()
+						->getFirstRow();
 
 		$this->assertEquals(date('Y-m-d', $time), date('Y-m-d', $result->created_at));
 	}
@@ -1033,8 +1033,8 @@ public function testSelectAndEntitiesSaveOnlyChangedValues()
 
 		// get only id, name column
 		$job = $model->select('id, name')
-		             ->where('name', 'Rocket Scientist')
-		             ->first();
+					 ->where('name', 'Rocket Scientist')
+					 ->first();
 
 		// Hence getting Null as description column not in select clause
 		$this->assertNull($job->description);
@@ -1055,8 +1055,8 @@ public function testSelectAndEntitiesSaveOnlyChangedValues()
 
 		// select all columns from job table
 		$job = $model->select('id, name, description')
-		             ->where('name', 'Rocket Scientist')
-		             ->first();
+					 ->where('name', 'Rocket Scientist')
+					 ->first();
 
 		// check whether the Null value successfully updated or not
 		$this->assertEquals('Some guitar description', $job->description);
@@ -1234,7 +1234,7 @@ public function testInsertID()
 		];
 
 		$model->protect(false)
-		      ->save($data);
+			  ->save($data);
 
 		$lastInsertId = $model->getInsertID();
 
@@ -1255,7 +1255,7 @@ public function testSetTable()
 		];
 
 		$model->protect(false)
-		      ->save($data);
+			  ->save($data);
 
 		$lastInsertId = $model->getInsertID();
 
@@ -1286,8 +1286,8 @@ public function testValidationByObject()
 			public $token = '';
 		};
 
-		$data->name = 'abc';
-		$data->id = '13';
+		$data->name  = 'abc';
+		$data->id    = '13';
 		$data->token = '13';
 
 		$this->assertTrue($model->validate($data));
@@ -1348,7 +1348,7 @@ public function testSaveObject()
 
 		$testModel = new JobModel();
 
-		$testModel->name = 'my name';
+		$testModel->name        = 'my name';
 		$testModel->description = 'some description';
 
 		$this->setPrivateProperty($model, 'useTimestamps', true);
@@ -1361,15 +1361,15 @@ public function testSaveObject()
 	}
 
 	//--------------------------------------------------------------------
-	
+
 	public function testEmptySaveData()
 	{
 		$model = new JobModel();
 
 		$data = [];
 
 		$data = $model->protect(false)
-		              ->save($data);
+					  ->save($data);
 
 		$this->assertTrue($data);
 	}
@@ -1382,7 +1382,7 @@ public function testUpdateObject()
 
 		$testModel = new JobModel();
 
-		$testModel->name = 'my name';
+		$testModel->name        = 'my name';
 		$testModel->description = 'some description';
 
 		$this->setPrivateProperty($model, 'useTimestamps', true);
@@ -1413,8 +1413,8 @@ public function testPurgeDeletedWithSoftDeleteFalse()
 		$model = new JobModel();
 
 		$this->db->table('job')
-		         ->where('id', 1)
-		         ->update(['deleted' => 1]);
+				 ->where('id', 1)
+				 ->update(['deleted' => 1]);
 
 		$model->purgeDeleted();
 
@@ -1463,7 +1463,7 @@ public function testGetValidationMessagesForReplace()
 
 	public function testSaveNewEntityWithDateTime()
 	{
-		$entity = new class extends Entity{
+		$entity    = new class extends Entity{
 			protected $id;
 			protected $name;
 			protected $email;
@@ -1484,10 +1484,10 @@ public function testSaveNewEntityWithDateTime()
 		};
 		$testModel = new UserModel();
 
-		$entity->name = 'Mark';
-		$entity->email = 'mark@example.com';
-		$entity->country = 'India';
-		$entity->deleted = 0;
+		$entity->name       = 'Mark';
+		$entity->email      = 'mark@example.com';
+		$entity->country    = 'India';
+		$entity->deleted    = 0;
 		$entity->created_at = new Time('now');
 
 		$this->setPrivateProperty($testModel, 'useTimestamps', true);
@@ -1499,7 +1499,7 @@ public function testSaveNewEntityWithDateTime()
 
 	public function testSaveNewEntityWithDate()
 	{
-		$entity = new class extends Entity
+		$entity    = new class extends Entity
 		{
 			protected $id;
 			protected $name;
@@ -1517,17 +1517,17 @@ public function testSaveNewEntityWithDate()
 		};
 		$testModel = new class extends Model
 		{
-			protected $table = 'empty';
-			protected $allowedFields = [
+			protected $table          = 'empty';
+			protected $allowedFields  = [
 				'name',
 			];
-			protected $returnType = 'object';
+			protected $returnType     = 'object';
 			protected $useSoftDeletes = true;
-			protected $dateFormat = 'date';
-			public $name = '';
+			protected $dateFormat     = 'date';
+			public $name              = '';
 		};
 
-		$entity->name = 'Mark';
+		$entity->name       = 'Mark';
 		$entity->created_at = new Time('now');
 
 		$this->setPrivateProperty($testModel, 'useTimestamps', true);
@@ -1556,7 +1556,7 @@ public function testUndefinedEntityPropertyException()
 	public function testInsertWithNoDataException()
 	{
 		$model = new UserModel();
-		$data = [];
+		$data  = [];
 		$this->expectException(DataException::class);
 		$this->expectExceptionMessage('There is no data to insert.');
 		$model->insert($data);
@@ -1596,7 +1596,7 @@ public function testInvalidAllowedFieldException()
 			'description' => 'That thing you do.',
 		];
 
-		$this->setPrivateProperty($model,'allowedFields', []);
+		$this->setPrivateProperty($model, 'allowedFields', []);
 
 		$this->expectException(DataException::class);
 		$this->expectExceptionMessage('Allowed fields must be specified for model: Tests\Support\Models\JobModel');
@@ -1617,7 +1617,7 @@ public function testInvalidEventException()
 			'deleted' => 0,
 		];
 
-		$this->setPrivateProperty($model,'beforeInsert',['anotherBeforeInsertMethod']);
+		$this->setPrivateProperty($model, 'beforeInsert', ['anotherBeforeInsertMethod']);
 
 		$this->expectException(DataException::class);
 		$this->expectExceptionMessage('anotherBeforeInsertMethod is not a valid Model Event callback.');
@@ -1627,4 +1627,51 @@ public function testInvalidEventException()
 
 	//--------------------------------------------------------------------
 
+	/**
+	 * @see https://github.com/codeigniter4/CodeIgniter4/issues/1881
+	 */
+	public function testSoftDeleteWithTableJoinsFindAll()
+	{
+		$model = new UserModel();
+
+		$this->seeInDatabase('user', ['name' => 'Derek Jones', 'deleted' => 0]);
+
+		$results = $model->join('job', 'job.id = user.id')
+			->findAll();
+
+		// Just making sure it didn't throw ambiguous delete error
+		$this->assertCount(4, $results);
+	}
+
+	/**
+	 * @see https://github.com/codeigniter4/CodeIgniter4/issues/1881
+	 */
+	public function testSoftDeleteWithTableJoinsFind()
+	{
+		$model = new UserModel();
+
+		$this->seeInDatabase('user', ['name' => 'Derek Jones', 'deleted' => 0]);
+
+		$results = $model->join('job', 'job.id = user.id')
+						 ->find(1);
+
+		// Just making sure it didn't throw ambiguous deleted error
+		$this->assertEquals(1, $results->id);
+	}
+
+	/**
+	 * @see https://github.com/codeigniter4/CodeIgniter4/issues/1881
+	 */
+	public function testSoftDeleteWithTableJoinsFirst()
+	{
+		$model = new UserModel();
+
+		$this->seeInDatabase('user', ['name' => 'Derek Jones', 'deleted' => 0]);
+
+		$results = $model->join('job', 'job.id = user.id')
+						 ->first(1);
+
+		// Just making sure it didn't throw ambiguous deleted error
+		$this->assertEquals(1, $results->id);
+	}
 }