From e42b13bcff41161ecdb4322dce1ef98a401482ca Mon Sep 17 00:00:00 2001
From: Jeremy Benoist <jeremy.benoist@gmail.com>
Date: Mon, 30 May 2016 14:34:11 +0200
Subject: [PATCH] Change ManyToMany between entry & tag

Following https://gist.github.com/Ocramius/3121916

Be sure to remove the related entity when removing an entity.

Let say you have Entry -> EntryTag -> Tag.
If you remove the entry:

 - before that commit, the EntryTag will stay (at least using SQLite).
 - with that commit, the related entity is removed
---
 .../DataFixtures/ORM/LoadConfigData.php       |  1 -
 src/Wallabag/CoreBundle/Entity/Entry.php      | 17 ++++++++--
 src/Wallabag/CoreBundle/Entity/Tag.php        | 23 ++++++++++++-
 .../Tests/Controller/EntryControllerTest.php  | 34 +++++++++++++++++--
 4 files changed, 68 insertions(+), 7 deletions(-)

diff --git a/src/Wallabag/CoreBundle/DataFixtures/ORM/LoadConfigData.php b/src/Wallabag/CoreBundle/DataFixtures/ORM/LoadConfigData.php
index ebfebfea..03be9667 100644
--- a/src/Wallabag/CoreBundle/DataFixtures/ORM/LoadConfigData.php
+++ b/src/Wallabag/CoreBundle/DataFixtures/ORM/LoadConfigData.php
@@ -6,7 +6,6 @@ use Doctrine\Common\DataFixtures\AbstractFixture;
 use Doctrine\Common\DataFixtures\OrderedFixtureInterface;
 use Doctrine\Common\Persistence\ObjectManager;
 use Wallabag\CoreBundle\Entity\Config;
-use Wallabag\CoreBundle\Entity\TaggingRule;
 
 class LoadConfigData extends AbstractFixture implements OrderedFixtureInterface
 {
diff --git a/src/Wallabag/CoreBundle/Entity/Entry.php b/src/Wallabag/CoreBundle/Entity/Entry.php
index 84981414..ceae78b0 100644
--- a/src/Wallabag/CoreBundle/Entity/Entry.php
+++ b/src/Wallabag/CoreBundle/Entity/Entry.php
@@ -178,7 +178,15 @@ class Entry
 
     /**
      * @ORM\ManyToMany(targetEntity="Tag", inversedBy="entries", cascade={"persist"})
-     * @ORM\JoinTable
+     * @ORM\JoinTable(
+     *  name="entry_tag",
+     *  joinColumns={
+     *      @ORM\JoinColumn(name="entry_id", referencedColumnName="id")
+     *  },
+     *  inverseJoinColumns={
+     *      @ORM\JoinColumn(name="tag_id", referencedColumnName="id")
+     *  }
+     * )
      *
      * @Groups({"entries_for_user", "export_all"})
      */
@@ -526,13 +534,18 @@ class Entry
             }
         }
 
-        $this->tags[] = $tag;
+        $this->tags->add($tag);
         $tag->addEntry($this);
     }
 
     public function removeTag(Tag $tag)
     {
+        if (!$this->tags->contains($tag)) {
+            return;
+        }
+
         $this->tags->removeElement($tag);
+        $tag->removeEntry($this);
     }
 
     /**
diff --git a/src/Wallabag/CoreBundle/Entity/Tag.php b/src/Wallabag/CoreBundle/Entity/Tag.php
index b4adbbd3..4b480ff1 100644
--- a/src/Wallabag/CoreBundle/Entity/Tag.php
+++ b/src/Wallabag/CoreBundle/Entity/Tag.php
@@ -98,9 +98,30 @@ class Tag
         return $this->slug;
     }
 
+    /**
+     * @param Entry $entry
+     */
     public function addEntry(Entry $entry)
     {
-        $this->entries[] = $entry;
+        if ($this->entries->contains($entry)) {
+            return;
+        }
+
+        $this->entries->add($entry);
+        $entry->addTag($this);
+    }
+
+    /**
+     * @param Entry $entry
+     */
+    public function removeEntry(Entry $entry)
+    {
+        if (!$this->entries->contains($entry)) {
+            return;
+        }
+
+        $this->entries->removeElement($entry);
+        $entry->removeTag($this);
     }
 
     public function hasEntry($entry)
diff --git a/src/Wallabag/CoreBundle/Tests/Controller/EntryControllerTest.php b/src/Wallabag/CoreBundle/Tests/Controller/EntryControllerTest.php
index c7087a71..5ce893c1 100644
--- a/src/Wallabag/CoreBundle/Tests/Controller/EntryControllerTest.php
+++ b/src/Wallabag/CoreBundle/Tests/Controller/EntryControllerTest.php
@@ -163,7 +163,7 @@ class EntryControllerTest extends WallabagCoreTestCase
     /**
      * This test will require an internet connection.
      */
-    public function testPostNewThatWillBeTaggued()
+    public function testPostNewThatWillBeTagged()
     {
         $this->logInAs('admin');
         $client = $this->getClient();
@@ -181,8 +181,7 @@ class EntryControllerTest extends WallabagCoreTestCase
         $client->submit($form, $data);
 
         $this->assertEquals(302, $client->getResponse()->getStatusCode());
-
-        $client->followRedirect();
+        $this->assertContains('/', $client->getResponse()->getTargetUrl());
 
         $em = $client->getContainer()
             ->get('doctrine.orm.entity_manager');
@@ -196,6 +195,35 @@ class EntryControllerTest extends WallabagCoreTestCase
 
         $em->remove($entry);
         $em->flush();
+
+        // and now re-submit it to test the cascade persistence for tags after entry removal
+        // related https://github.com/wallabag/wallabag/issues/2121
+        $crawler = $client->request('GET', '/new');
+
+        $this->assertEquals(200, $client->getResponse()->getStatusCode());
+
+        $form = $crawler->filter('form[name=entry]')->form();
+
+        $data = [
+            'entry[url]' => $url = 'https://github.com/wallabag/wallabag/tree/master',
+        ];
+
+        $client->submit($form, $data);
+
+        $this->assertEquals(302, $client->getResponse()->getStatusCode());
+        $this->assertContains('/', $client->getResponse()->getTargetUrl());
+
+        $entry = $em
+            ->getRepository('WallabagCoreBundle:Entry')
+            ->findOneByUrl($url);
+
+        $tags = $entry->getTags();
+
+        $this->assertCount(1, $tags);
+        $this->assertEquals('wallabag', $tags[0]->getLabel());
+
+        $em->remove($entry);
+        $em->flush();
     }
 
     public function testArchive()
-- 
2.41.0