diff options
author | Kate Butler <kate@innocraft.com> | 2019-09-03 05:03:04 +0300 |
---|---|---|
committer | Thomas Steur <tsteur@users.noreply.github.com> | 2019-09-03 05:03:04 +0300 |
commit | ea18198684c34d8bbb45e15874707e302273dcd9 (patch) | |
tree | 15c4d96d5f6b8644c3360faa4c15769d711ecb29 | |
parent | dc3bf2cff5984bfdc44d6d9a0efedf29f54b0dde (diff) |
Soft delete segments (#14846)
* Soft delete segments
* Bug fixes and tests
-rw-r--r-- | plugins/CoreAdminHome/tests/Integration/TasksTest.php | 30 | ||||
-rw-r--r-- | plugins/SegmentEditor/Model.php | 10 | ||||
-rw-r--r-- | plugins/SegmentEditor/tests/Integration/ModelTest.php | 158 |
3 files changed, 196 insertions, 2 deletions
diff --git a/plugins/CoreAdminHome/tests/Integration/TasksTest.php b/plugins/CoreAdminHome/tests/Integration/TasksTest.php index 98765ab8cc..830da7ef97 100644 --- a/plugins/CoreAdminHome/tests/Integration/TasksTest.php +++ b/plugins/CoreAdminHome/tests/Integration/TasksTest.php @@ -255,6 +255,36 @@ class TasksTest extends IntegrationTestCase $this->assertEquals($expected, $segmentsByIdSite); } + public function test_getSegmentHashesByIdSite_deletedSegment() + { + $model = new Model(); + $model->createSegment(array( + 'name' => 'Test Segment 1', + 'definition' => 'continentCode==eur', + 'enable_only_idsite' => 0, + 'deleted' => 0 + )); + $model->createSegment(array( + 'name' => 'Test Segment 2', + 'definition' => 'countryCode==nz', + 'enable_only_idsite' => 0, + 'deleted' => 1 + )); + $model->createSegment(array( + 'name' => 'Test Segment 3', + 'definition' => 'countryCode==au', + 'enable_only_idsite' => 2, + 'deleted' => 0 + )); + + $segmentsByIdSite = $this->tasks->getSegmentHashesByIdSite(); + $expected = array( + 0 => array('be90051048558489e1d62f4245a6dc65'), + 2 => array('cffd4336c22c6782211f853495076b1a') + ); + $this->assertEquals($expected, $segmentsByIdSite); + } + public function test_getSegmentHashesByIdSite_invalidSegment() { $model = new Model(); diff --git a/plugins/SegmentEditor/Model.php b/plugins/SegmentEditor/Model.php index da30fe5359..0025c7c15c 100644 --- a/plugins/SegmentEditor/Model.php +++ b/plugins/SegmentEditor/Model.php @@ -9,6 +9,7 @@ namespace Piwik\Plugins\SegmentEditor; use Piwik\Common; +use Piwik\Date; use Piwik\Db; use Piwik\DbHelper; @@ -121,7 +122,7 @@ class Model public function getSegmentByDefinition($definition) { - $sql = $this->buildQuerySortedByName("definition = ?"); + $sql = $this->buildQuerySortedByName("definition = ? AND deleted = 0"); $bind = [$definition]; $segment = $this->getDb()->fetchRow($sql, $bind); @@ -130,8 +131,13 @@ class Model public function deleteSegment($idSegment) { + $fieldsToSet = array( + 'deleted' => 1, + 'ts_last_edit' => Date::factory('now')->toString('Y-m-d H:i:s') + ); + $db = $this->getDb(); - $db->delete($this->getTable(), 'idsegment = ' . (int) $idSegment); + $db->update($this->getTable(), $fieldsToSet, 'idsegment = ' . (int) $idSegment); } public function updateSegment($idSegment, $segment) diff --git a/plugins/SegmentEditor/tests/Integration/ModelTest.php b/plugins/SegmentEditor/tests/Integration/ModelTest.php new file mode 100644 index 0000000000..92034f3efc --- /dev/null +++ b/plugins/SegmentEditor/tests/Integration/ModelTest.php @@ -0,0 +1,158 @@ +<?php +/** + * Matomo - free/libre analytics platform + * + * @link https://matomo.org + * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later + * + */ + +namespace Piwik\Plugins\SegmentEditor\tests\Integration; + +use Piwik\Common; +use Piwik\Date; +use Piwik\Db; +use Piwik\Plugins\SegmentEditor\Model; +use Piwik\Tests\Framework\TestCase\IntegrationTestCase; + +class ModelTest extends IntegrationTestCase +{ + private $model; + + private $idSegment1; + + private $idSegment2; + + private $idSegment3; + + public function setUp() + { + parent::setUp(); + $this->model = new Model(); + $this->idSegment1 = $this->model->createSegment(array( + 'name' => 'Narnia', + 'definition' => 'country==Narnia', + 'login' => 'user1', + 'enable_only_idsite' => 0, + )); + $this->idSegment2 = $this->model->createSegment(array( + 'name' => 'Genovia', + 'definition' => 'country==Genovia', + 'auto_archive' => 1, + 'enable_only_idsite' => 0, + 'enable_all_users' => 1 + )); + $this->idSegment3 = $this->model->createSegment(array( + 'name' => 'Hobbiton', + 'definition' => 'country==Hobbiton', + 'auto_archive' => 0, + 'login' => 'user2', + 'enable_only_idsite' => 1, + )); + } + + public function tearDown() + { + parent::tearDown(); + // Force a hard delete of segment + $idsToDelete = $this->idSegment1 . ', ' . $this->idSegment2 . ', ' . $this->idSegment3; + Db::query( + "DELETE FROM " . Common::prefixTable('segment') . " WHERE idsegment IN ($idsToDelete)" + ); + } + + public function test_deleteSegment_doesSoftDelete() + { + $preDeleteTimestamp = Date::getNowTimestamp(); + $this->model->deleteSegment($this->idSegment1); + + // None of the model methods should return it as it's deleted - so we need to get it manually from DB + $result = Db::query( + 'SELECT * FROM ' . Common::prefixTable('segment') . ' WHERE idsegment = ' . $this->idSegment1 + ); + $row = $result->fetch(); + + $this->assertNotEmpty($row); + $this->assertEquals(1, $row['deleted']); + $deletedTimestamp = Date::factory($row['ts_last_edit'])->getTimestamp(); + $this->assertGreaterThanOrEqual($deletedTimestamp, $preDeleteTimestamp); + } + + public function test_getAllSegmentsAndIgnoreVisibility_withDeletedSegment() + { + $segments = $this->model->getAllSegmentsAndIgnoreVisibility(); + $this->assertEquals(3, count($segments)); + + $this->model->deleteSegment($this->idSegment2); + $segments = $this->model->getAllSegmentsAndIgnoreVisibility(); + + $this->assertReturnedIdsMatch(array($this->idSegment1, $this->idSegment3), $segments); + } + + public function test_getSegmentsToAutoArchive_withDeletedSegment() + { + $segments = $this->model->getSegmentsToAutoArchive(); + $this->assertEquals(1, count($segments)); + $this->assertReturnedIdsMatch(array($this->idSegment2), $segments); + + $this->model->deleteSegment($this->idSegment2); + $segments = $this->model->getSegmentsToAutoArchive(); + + $this->assertEmpty($segments); + } + + public function test_getAllSegments_withDeletedSegment() + { + $segments = $this->model->getAllSegments('user1'); + $this->assertEquals(2, count($segments)); + + $this->model->deleteSegment($this->idSegment1); + $segments = $this->model->getAllSegments('user1'); + + $this->assertReturnedIdsMatch(array($this->idSegment2), $segments); + } + + public function test_getAllSegmentsForSite_withDeletedSegment() + { + $segments = $this->model->getAllSegmentsForSite(1, 'user1'); + $this->assertEquals(2, count($segments)); + $this->assertReturnedIdsMatch(array($this->idSegment1, $this->idSegment2), $segments); + + $this->model->deleteSegment($this->idSegment2); + $segments = $this->model->getAllSegmentsForSite(1, 'user1'); + + $this->assertReturnedIdsMatch(array($this->idSegment1), $segments); + } + + public function test_getAllSegmentsForAllUsers_withDeletedSegment() + { + $segments = $this->model->getAllSegmentsForAllUsers(); + $this->assertEquals(3, count($segments)); + + $this->model->deleteSegment($this->idSegment3); + $segments = $this->model->getAllSegmentsForAllUsers(); + + $this->assertReturnedIdsMatch(array($this->idSegment1, $this->idSegment2), $segments); + } + + public function test_getSegmentByDefinition_withDeletedSegment() + { + $segment = $this->model->getSegmentByDefinition('Country==Genovia'); + $this->assertNotEmpty($segment); + + $this->model->deleteSegment($this->idSegment2); + $segment = $this->model->getSegmentByDefinition('Country==Genovia'); + + $this->assertEmpty($segment); + } + + private function assertReturnedIdsMatch(array $expectedIds, array $resultSet) + { + $this->assertEquals(count($expectedIds), count($resultSet)); + + $returnedIds = array_column($resultSet, 'idsegment'); + sort($returnedIds); + + $this->assertEquals(array_values($expectedIds), array_values($returnedIds)); + } +}
\ No newline at end of file |