Merge pull request #2051 from MGatner/deletedat

Soft deletes use deleted_at

system/Model.php

@@ -120,7 +120,7 @@ class Model
 
 	/**
 	 * If this model should use "softDeletes" and
-	 * simply set a flag when rows are deleted, or
+	 * simply set a date when rows are deleted, or
 	 * do hard deletes.
 	 *
 	 * @var boolean
@@ -182,7 +182,7 @@ class Model
 	 *
 	 * @var string
 	 */
-	protected $deletedField = 'deleted';
+	protected $deletedField = 'deleted_at';
 
 	/**
 	 * Used by asArray and asObject to provide
@@ -356,7 +356,7 @@ public function find($id = null)
 
 		if ($this->tempUseSoftDeletes === true)
 		{
-			$builder->where($this->table . '.' . $this->deletedField, 0);
+			$builder->where($this->table . '.' . $this->deletedField, null);
 		}
 
 		if (is_array($id))
@@ -427,7 +427,7 @@ public function findAll(int $limit = 0, int $offset = 0)
 
 		if ($this->tempUseSoftDeletes === true)
 		{
-			$builder->where($this->table . '.' . $this->deletedField, 0);
+			$builder->where($this->table . '.' . $this->deletedField, null);
 		}
 
 		$row = $builder->limit($limit, $offset)
@@ -457,7 +457,7 @@ public function first()
 
 		if ($this->tempUseSoftDeletes === true)
 		{
-			$builder->where($this->table . '.' . $this->deletedField, 0);
+			$builder->where($this->table . '.' . $this->deletedField, null);
 		}
 
 		// Some databases, like PostgreSQL, need order
@@ -713,17 +713,21 @@ public function insert($data = null, bool $returnID = true)
 		$result = $this->builder()
 				->set($data['data'], '', $escape)
 				->insert();
-
+
+		// If insertion succeeded then save the insert ID
+		if ($result)
+		{
+			$this->insertID = $this->db->insertID();
+		}
+
 		$this->trigger('afterInsert', ['data' => $originalData, 'result' => $result]);
 
-		// If insertion failed, get our of here
+		// If insertion failed, get out of here
 		if (! $result)
 		{
 			return $result;
 		}
 
-		$this->insertID = $this->db->insertID();
-
 		// otherwise return the insertID, if requested.
 		return $returnID ? $this->insertID : $result;
 	}
@@ -908,7 +912,7 @@ public function delete($id = null, bool $purge = false)
 
 		if ($this->useSoftDeletes && ! $purge)
 		{
-			$set[$this->deletedField] = 1;
+			$set[$this->deletedField] = $this->setDate();
 
 			if ($this->useTimestamps && ! empty($this->updatedField))
 			{
@@ -943,8 +947,8 @@ public function purgeDeleted()
 		}
 
 		return $this->builder()
-						->where($this->deletedField, 1)
-						->delete();
+			    ->where($this->table . '.' . $this->deletedField . ' IS NOT NULL')
+			    ->delete();
 	}
 
 	//--------------------------------------------------------------------
@@ -977,7 +981,7 @@ public function onlyDeleted()
 		$this->tempUseSoftDeletes = false;
 
 		$this->builder()
-				->where($this->deletedField, 1);
+		     ->where($this->table . '.' . $this->deletedField . ' IS NOT NULL');
 
 		return $this;
 	}
@@ -1539,7 +1543,7 @@ public function countAllResults(bool $reset = true, bool $test = false)
 	{
 		if ($this->tempUseSoftDeletes === true)
 		{
-			$this->builder()->where($this->deletedField, 0);
+			$this->builder()->where($this->table . '.' . $this->deletedField, null);
 		}
 
 		return $this->builder()->countAllResults($reset, $test);

tests/_support/Database/Migrations/20160428212500_Create_test_tables.php

@@ -26,11 +26,6 @@ public function up()
 				'type'       => 'VARCHAR',
 				'constraint' => 40,
 			],
-			'deleted'    => [
-				'type'       => 'TINYINT',
-				'constraint' => 1,
-				'default'    => '0',
-			],
 			'created_at' => [
 				'type' => 'DATETIME',
 				'null' => true,
@@ -39,6 +34,10 @@ public function up()
 				'type' => 'DATETIME',
 				'null' => true,
 			],
+			'deleted_at'    => [
+				'type' => 'DATETIME',
+				'null' => true,
+			],
 		]);
 		$this->forge->addKey('id', true);
 		$this->forge->createTable('user', true);
@@ -58,11 +57,6 @@ public function up()
 				'type' => 'TEXT',
 				'null' => true,
 			],
-			'deleted'     => [
-				'type'       => 'TINYINT',
-				'constraint' => 1,
-				'default'    => '0',
-			],
 			'created_at'  => [
 				'type'       => 'INTEGER',
 				'constraint' => 11,
@@ -73,6 +67,11 @@ public function up()
 				'constraint' => 11,
 				'null'       => true,
 			],
+			'deleted_at'     => [
+				'type'       => 'INTEGER',
+				'constraint' => 11,
+				'null'       => true,
+			],
 		]);
 		$this->forge->addKey('id', true);
 		$this->forge->createTable('job', true);

tests/_support/Models/EntityModel.php

@@ -12,7 +12,7 @@ class EntityModel extends Model
 
 	protected $dateFormat = 'int';
 
-	protected $deletedField = 'deleted';
+	protected $deletedField = 'deleted_at';
 
 	protected $allowedFields = [
 		'name',

tests/_support/Models/EventModel.php

@@ -16,7 +16,7 @@ class EventModel extends Model
 		'name',
 		'email',
 		'country',
-		'deleted',
+		'deleted_at',
 	];
 
 	protected $beforeInsert = ['beforeInsertMethod'];

tests/_support/Models/UserModel.php

@@ -10,7 +10,7 @@ class UserModel extends Model
 		'name',
 		'email',
 		'country',
-		'deleted',
+		'deleted_at',
 	];
 
 	protected $returnType = 'object';

tests/system/Database/Live/DbUtilsTest.php

@@ -188,7 +188,7 @@ public function testUtilsCSVFromResult()
 
 		$data = array_filter(preg_split('/(\r\n|\n|\r)/', $data));
 
-		$this->assertEquals('"1","Developer","Awesome job, but sometimes makes you bored","0","",""', $data[1]);
+		$this->assertEquals('"1","Developer","Awesome job, but sometimes makes you bored","","",""', $data[1]);
 	}
 
 	//--------------------------------------------------------------------
@@ -203,7 +203,7 @@ public function testUtilsXMLFromResult()
 
 		$data = $util->getXMLFromResult($data);
 
-		$expected = '<root><element><id>4</id><name>Musician</name><description>Only Coldplay can actually called Musician</description><deleted></deleted><created_at></created_at><updated_at></updated_at></element></root>';
+		$expected = '<root><element><id>4</id><name>Musician</name><description>Only Coldplay can actually called Musician</description><created_at></created_at><updated_at></updated_at><deleted_at></deleted_at></element></root>';
 
 		$actual = preg_replace('#\R+#', '', $data);
 		$actual = preg_replace('/[ ]{2,}|[\t]/', '', $actual);

tests/system/Database/Live/ModelTest.php

@@ -143,7 +143,7 @@ public function testFindRespectsSoftDeletes()
 	{
 		$this->db->table('user')
 				 ->where('id', 4)
-				 ->update(['deleted' => 1]);
+				 ->update(['deleted_at' => date('Y-m-d H:i:s')]);
 
 		$model = new UserModel($this->db);
 
@@ -219,7 +219,7 @@ public function testFindAllRespectsSoftDeletes()
 	{
 		$this->db->table('user')
 				 ->where('id', 4)
-				 ->update(['deleted' => 1]);
+				 ->update(['deleted_at' => date('Y-m-d H:i:s')]);
 
 		$model = new UserModel($this->db);
 
@@ -254,7 +254,7 @@ public function testFirstRespectsSoftDeletes()
 	{
 		$this->db->table('user')
 				 ->where('id', 1)
-				 ->update(['deleted' => 1]);
+				 ->update(['deleted_at' => date('Y-m-d H:i:s')]);
 
 		$model = new UserModel();
 
@@ -403,11 +403,11 @@ public function testDeleteWithSoftDeletes()
 	{
 		$model = new UserModel();
 
-		$this->seeInDatabase('user', ['name' => 'Derek Jones', 'deleted' => 0]);
+		$this->seeInDatabase('user', ['name' => 'Derek Jones', 'deleted_at IS NULL' => null]);
 
 		$model->delete(1);
 
-		$this->seeInDatabase('user', ['name' => 'Derek Jones', 'deleted' => 1]);
+		$this->seeInDatabase('user', ['name' => 'Derek Jones', 'deleted_at IS NOT NULL' => null]);
 	}
 
 	//--------------------------------------------------------------------
@@ -416,7 +416,7 @@ public function testDeleteWithSoftDeletesPurge()
 	{
 		$model = new UserModel();
 
-		$this->seeInDatabase('user', ['name' => 'Derek Jones', 'deleted' => 0]);
+		$this->seeInDatabase('user', ['name' => 'Derek Jones', 'deleted_at IS NULL' => null]);
 
 		$model->delete(1, true);
 
@@ -461,7 +461,7 @@ public function testPurgeDeleted()
 
 		$this->db->table('user')
 				 ->where('id', 1)
-				 ->update(['deleted' => 1]);
+				 ->update(['deleted_at' => date('Y-m-d H:i:s')]);
 
 		$model->purgeDeleted();
 
@@ -479,7 +479,7 @@ public function testOnlyDeleted()
 
 		$this->db->table('user')
 				 ->where('id', 1)
-				 ->update(['deleted' => 1]);
+				 ->update(['deleted_at' => date('Y-m-d H:i:s')]);
 
 		$users = $model->onlyDeleted()
 					   ->findAll();
@@ -757,7 +757,6 @@ public function testPasswordsStoreCorrectly()
 			'name'    => $pass,
 			'email'   => '[email protected]',
 			'country' => 'US',
-			'deleted' => 0,
 		];
 
 		$model->insert($data);
@@ -1403,7 +1402,7 @@ public function testDeleteWithSoftDelete()
 
 		$model->delete(1);
 
-		$this->seeInDatabase('job', ['id' => 1, 'deleted' => 1]);
+		$this->seeInDatabase('job', ['id' => 1, 'deleted_at IS NOT NULL' => null]);
 	}
 
 	//--------------------------------------------------------------------
@@ -1414,7 +1413,7 @@ public function testPurgeDeletedWithSoftDeleteFalse()
 
 		$this->db->table('job')
 				 ->where('id', 1)
-				 ->update(['deleted' => 1]);
+				 ->update(['deleted_at' => time()]);
 
 		$model->purgeDeleted();
 
@@ -1629,7 +1628,7 @@ public function testSoftDeleteWithTableJoinsFindAll()
 	{
 		$model = new UserModel();
 
-		$this->seeInDatabase('user', ['name' => 'Derek Jones', 'deleted' => 0]);
+		$this->seeInDatabase('user', ['name' => 'Derek Jones', 'deleted_at' => null]);
 
 		$results = $model->join('job', 'job.id = user.id')
 			->findAll();
@@ -1645,7 +1644,7 @@ public function testSoftDeleteWithTableJoinsFind()
 	{
 		$model = new UserModel();
 
-		$this->seeInDatabase('user', ['name' => 'Derek Jones', 'deleted' => 0]);
+		$this->seeInDatabase('user', ['name' => 'Derek Jones', 'deleted_at' => null]);
 
 		$results = $model->join('job', 'job.id = user.id')
 						 ->find(1);
@@ -1661,7 +1660,7 @@ public function testSoftDeleteWithTableJoinsFirst()
 	{
 		$model = new UserModel();
 
-		$this->seeInDatabase('user', ['name' => 'Derek Jones', 'deleted' => 0]);
+		$this->seeInDatabase('user', ['name' => 'Derek Jones', 'deleted_at' => null]);
 
 		$results = $model->join('job', 'job.id = user.id')
 						 ->first(1);

user_guide_src/source/models/model.rst

@@ -106,6 +106,7 @@ what table to use and how we can find the required records::
 		protected $useTimestamps = false;
 		protected $createdField  = 'created_at';
 		protected $updatedField  = 'updated_at';
+		protected $deletedField  = 'deleted_at';
 
 		protected $validationRules    = [];
 		protected $validationMessages = [];
@@ -137,13 +138,15 @@ method.
 
 **$useSoftDeletes**
 
-If true, then any delete* method calls will simply set a flag in the database, instead of
+If true, then any delete* method calls will set ``deleted_at`` in the database, instead of
 actually deleting the row. This can preserve data when it might be referenced elsewhere, or
 can maintain a "recycle bin" of objects that can be restored, or even simply preserve it as
 part of a security trail. If true, the find* methods will only return non-deleted rows, unless
 the withDeleted() method is called prior to calling the find* method.
 
-This requires an INT or TINYINT field to be present in the table for storing state. The default field name is  ``deleted`` however this name can be configured to any name of your choice by using $deletedField property.
+This requires either a DATETIME or INTEGER field in the database as per the model's
+$dateFormat setting. The default field name is ``deleted_at`` however this name can be
+configured to any name of your choice by using $deletedField property.
 
 **$allowedFields**
 
@@ -262,8 +265,8 @@ Returns the first row in the result set. This is best used in combination with t
 
 **withDeleted()**
 
-If $useSoftDeletes is true, then the find* methods will not return any rows where 'deleted = 1'. To
-temporarily override this, you can use the withDeleted() method prior to calling the find* method.
+If $useSoftDeletes is true, then the find* methods will not return any rows where 'deleted_at IS NOT NULL'.
+To temporarily override this, you can use the withDeleted() method prior to calling the find* method.
 ::
 
 	// Only gets non-deleted rows (deleted = 0)
@@ -421,8 +424,8 @@ Takes a primary key value as the first parameter and deletes the matching record
 
 	$userModel->delete(12);
 
-If the model's $useSoftDeletes value is true, this will update the row to set 'deleted = 1'. You can force
-a permanent delete by setting the second parameter as true.
+If the model's $useSoftDeletes value is true, this will update the row to set ``deleted_at`` to the current
+date and time. You can force a permanent delete by setting the second parameter as true.
 
 An array of primary keys can be passed in as the first parameter to delete multiple records at once::
 
@@ -435,7 +438,7 @@ previously::
 
 **purgeDeleted()**
 
-Cleans out the database table by permanently removing all rows that have 'deleted = 1'. ::
+Cleans out the database table by permanently removing all rows that have 'deleted_at IS NOT NULL'. ::
 
 	$userModel->purgeDeleted();
 

user_guide_src/source/testing/database.rst

@@ -157,7 +157,7 @@ Asserts that a number of matching rows are found in the database that match ``$c
 ::
 
     $criteria = [
-        'deleted' => 1
+        'active' => 1
     ];
     $this->seeNumRecords(2, 'users', $criteria);