Merge pull request #6566 from mautic-inc/segment-delete-validation

Segment deletion validation.

app/bundles/LeadBundle/Controller/ListController.php

@@ -455,6 +455,8 @@ private function getPostActionVars($objectId = null)
      */
     public function deleteAction($objectId)
     {
+        /** @var ListModel $model */
+        $model     = $this->getModel('lead.list');
         $page      = $this->get('session')->get('mautic.segment.page', 1);
         $returnUrl = $this->generateUrl('mautic_segment_index', ['page' => $page]);
         $flashes   = [];
@@ -469,6 +471,22 @@ public function deleteAction($objectId)
             ],
         ];
 
+        $dependents = $model->getSegmentsWithDependenciesOnSegment($objectId);
+
+        if (!empty($dependents)) {
+            $flashes[] = [
+                    'type'    => 'error',
+                    'msg'     => 'mautic.lead.list.error.cannot.delete',
+                    'msgVars' => ['%segments%' => implode(', ', $dependents)],
+                ];
+
+            return $this->postActionRedirect(
+                array_merge($postActionVars, [
+                    'flashes' => $flashes,
+                ])
+            );
+        }
+
         if ($this->request->getMethod() == 'POST') {
             /** @var ListModel $model */
             $model = $this->getModel('lead.list');
@@ -531,12 +549,23 @@ public function batchDeleteAction()
 
         if ($this->request->getMethod() == 'POST') {
             /** @var ListModel $model */
-            $model     = $this->getModel('lead.list');
-            $ids       = json_decode($this->request->query->get('ids', '{}'));
-            $deleteIds = [];
+            $model           = $this->getModel('lead.list');
+            $ids             = json_decode($this->request->query->get('ids', '{}'));
+            $canNotBeDeleted = $model->canNotBeDeleted($ids);
+
+            if (!empty($canNotBeDeleted)) {
+                $flashes[] = [
+                    'type'    => 'error',
+                    'msg'     => 'mautic.lead.list.error.cannot.delete.batch',
+                    'msgVars' => ['%segments%' => implode(', ', $canNotBeDeleted)],
+                ];
+            }
+
+            $toBeDeleted = array_diff($ids, array_keys($canNotBeDeleted));
+            $deleteIds   = [];
 
             // Loop over the IDs to perform access checks pre-delete
-            foreach ($ids as $objectId) {
+            foreach ($toBeDeleted as $objectId) {
                 $entity = $model->getEntity($objectId);
 
                 if ($entity === null) {

app/bundles/LeadBundle/Model/ListModel.php

@@ -1735,4 +1735,103 @@ public function isFieldUsed(LeadField $field)
 
         return $this->getEntities(['filter' => $filter])->count() !== 0;
     }
+
+    /**
+     * Get segments which are dependent on given segment.
+     *
+     * @param int $segmentId
+     *
+     * @return array
+     */
+    public function getSegmentsWithDependenciesOnSegment($segmentId)
+    {
+        $page  = 1;
+        $limit = 1000;
+        $start = 0;
+
+        $filter = [
+            'force'  => [
+                ['column' => 'l.filters', 'expr' => 'LIKE', 'value'=>'%s:8:"leadlist"%'],
+                ['column' => 'l.id', 'expr' => 'neq', 'value'=>$segmentId],
+            ],
+        ];
+
+        $entities = $this->getEntities(
+            [
+                'start'      => $start,
+                'limit'      => $limit,
+                'filter'     => $filter,
+            ]
+        );
+        $dependents = [];
+
+        foreach ($entities as $entity) {
+            $retrFilters = $entity->getFilters();
+            foreach ($retrFilters as $eachFilter) {
+                if ($eachFilter['type'] === 'leadlist' && in_array($segmentId, $eachFilter['filter'])) {
+                    $dependents[] = $entity->getName();
+                }
+            }
+        }
+
+        return $dependents;
+    }
+
+    /**
+     * Get segments which are used as a dependent by other segments to prevent batch deletion of them.
+     *
+     * @param array $segmentIds
+     *
+     * @return array
+     */
+    public function canNotBeDeleted($segmentIds)
+    {
+        $filter = [
+            'force'  => [
+                ['column' => 'l.filters', 'expr' => 'LIKE', 'value'=>'%s:8:"leadlist"%'],
+            ],
+        ];
+
+        $entities = $this->getEntities(
+            [
+                'filter'     => $filter,
+            ]
+        );
+
+        $idsNotToBeDeleted   = [];
+        $namesNotToBeDeleted = [];
+        $dependency          = [];
+
+        foreach ($entities as $entity) {
+            $retrFilters = $entity->getFilters();
+            foreach ($retrFilters as $eachFilter) {
+                if ($eachFilter['type'] !== 'leadlist') {
+                    continue;
+                }
+
+                $idsNotToBeDeleted = array_unique(array_merge($idsNotToBeDeleted, $eachFilter['filter']));
+                foreach ($eachFilter['filter'] as $val) {
+                    if (!empty($dependency[$val])) {
+                        $dependency[$val] = array_merge($dependency[$val], [$entity->getId()]);
+                        $dependency[$val] = array_unique($dependency[$val]);
+                    } else {
+                        $dependency[$val] = [$entity->getId()];
+                    }
+                }
+            }
+        }
+        foreach ($dependency as $key => $value) {
+            if (array_intersect($value, $segmentIds) === $value) {
+                $idsNotToBeDeleted = array_unique(array_diff($idsNotToBeDeleted, [$key]));
+            }
+        }
+
+        $idsNotToBeDeleted = array_intersect($segmentIds, $idsNotToBeDeleted);
+
+        foreach ($idsNotToBeDeleted as $val) {
+            $namesNotToBeDeleted[$val] = $this->getEntity($val)->getName();
+        }
+
+        return $namesNotToBeDeleted;
+    }
 }

app/bundles/LeadBundle/Tests/Model/LeadListModelTest.php

@@ -0,0 +1,121 @@
+<?php
+
+namespace Mautic\LeadBundle\Tests\Model;
+
+use Mautic\LeadBundle\Entity\LeadList;
+use Mautic\LeadBundle\Model\ListModel;
+
+class LeadListModelTest extends \PHPUnit_Framework_TestCase
+{
+    protected $fixture;
+
+    public function setUp()
+    {
+        $mockListModel = $this->getMockBuilder(ListModel::class)
+            ->disableOriginalConstructor()
+            ->setMethods(['getEntities', 'getEntity'])
+            ->getMock();
+
+        $mockListModel->expects($this->any())
+            ->method('getEntity')
+            ->willReturnCallback(function ($id) {
+                $mockEntity = $this->getMockBuilder(LeadList::class)
+                    ->disableOriginalConstructor()
+                    ->setMethods(['getName'])
+                    ->getMock();
+
+                $mockEntity->expects($this->once())
+                    ->method('getName')
+                    ->willReturn((string) $id);
+
+                return $mockEntity;
+            });
+
+        $filters = 'a:1:{i:0;a:7:{s:4:"glue";s:3:"and";s:5:"field";s:8:"leadlist";s:6:"object";s:4:"lead";s:4:"type";s:8:"leadlist";s:6:"filter";a:2:{i:0;i:1;i:1;i:3;}s:7:"display";N;s:8:"operator";s:2:"in";}}';
+
+        $filters4 = 'a:1:{i:0;a:7:{s:4:"glue";s:3:"and";s:5:"field";s:8:"leadlist";s:6:"object";s:4:"lead";s:4:"type";s:8:"leadlist";s:6:"filter";a:1:{i:0;i:3;}s:7:"display";N;s:8:"operator";s:2:"in";}}';
+
+        $mockEntity = $this->getMockBuilder(LeadList::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $mockEntity1 = clone $mockEntity;
+        $mockEntity1->expects($this->once())
+            ->method('getFilters')
+            ->willReturn([]);
+        $mockEntity1->expects($this->any())
+            ->method('getId')
+            ->willReturn(1);
+
+        $mockEntity2 = clone $mockEntity;
+        $mockEntity2->expects($this->once())
+            ->method('getFilters')
+            ->willReturn(unserialize($filters));
+        $mockEntity2->expects($this->any())
+            ->method('getId')
+            ->willReturn(2);
+
+        $mockEntity3 = clone $mockEntity;
+        $mockEntity3->expects($this->once())
+            ->method('getFilters')
+            ->willReturn([]);
+        $mockEntity3->expects($this->any())
+            ->method('getId')
+            ->willReturn(3);
+
+        $mockEntity4 = clone $mockEntity;
+        $mockEntity4->expects($this->once())
+            ->method('getFilters')
+            ->willReturn(unserialize($filters4));
+        $mockEntity4->expects($this->any())
+            ->method('getId')
+            ->willReturn(4);
+
+        $mockListModel->expects($this->once())
+            ->method('getEntities')
+            ->willReturn([
+                1 => $mockEntity1,
+                2 => $mockEntity2,
+                3 => $mockEntity3,
+                4 => $mockEntity4,
+            ]);
+
+        $this->fixture = $mockListModel;
+    }
+
+    /**
+     * @dataProvider segmentTestDataProvider
+     */
+    public function testSegmentsCanBeDeletedCorrecty(array $arg, array $expected, $message)
+    {
+        $result = $this->fixture->canNotBeDeleted($arg);
+
+        $this->assertEquals($expected, $result, $message);
+    }
+
+    public function segmentTestDataProvider()
+    {
+        return [
+            [
+                [1],
+                [1 => '1'],
+                '2 is dependent on 1, so 1 cannot be deleted.',
+            ],
+            [
+                [1, 3],
+                [1 => '1', 3 => '3'],
+                '2 is dependent on 1 & 3, so 1 & 3 cannot be deleted.',
+            ],
+            [
+                [1, 2, 3, 4],
+                [],
+                'Since we are deleting all segments, it should not prevent any from being deleted.',
+            ],
+            [
+                [2],
+                [],
+                'Segments without any other segment dependent on them should always be able to be deleted.',
+            ],
+        ];
+    }
+}

app/bundles/LeadBundle/Translations/en_US/flashes.ini

@@ -10,6 +10,8 @@ mautic.lead.lead.notice.addedtolists="<a href='%url%' data-toggle='ajax' data-me
 mautic.lead.lead.notice.batch_deleted="%count% contacts have been deleted!"
 mautic.lead.lead.notice.removedfromlists="<a href='%url%' data-toggle='ajax' data-menu-link='mautic_contact_index'><strong>%name% (%id%)</strong></a> has been removed from %list%."
 mautic.lead.list.error.notfound="No list with an id of %id% was found!"
+mautic.lead.list.error.cannot.delete="Segment cannot be deleted, it is required by %segments%."
+mautic.lead.list.error.cannot.delete.batch="%segments% cannot be deleted, it is required by other segments."
 mautic.lead.list.notice.batch_deleted="%count% lists have been deleted!"
 mautic.lead.list.frequency.rules.msg="No "
 mautic.lead.batch.import.created="Import process was successfully created. You will be notified when finished."