Merge pull request #2122 from wallabag/fix-tags-deletion

Fix the deletion of Tags/Entries relation when delete an entry

17 files changed, 122 insertions, 41 deletions

diff --git a/src/Wallabag/CoreBundle/Controller/EntryController.php b/src/Wallabag/CoreBundle/Controller/EntryController.php
index 69dfd4b1..33b5e2ad 100644
--- a/src/Wallabag/CoreBundle/Controller/EntryController.php
+++ b/src/Wallabag/CoreBundle/Controller/EntryController.php
@@ -23,10 +23,16 @@ class EntryController extends Controller
     {
         try {
             $entry = $this->get('wallabag_core.content_proxy')->updateEntry($entry, $entry->getUrl());
+
             $em = $this->getDoctrine()->getManager();
             $em->persist($entry);
             $em->flush();
         } catch (\Exception $e) {
+            $this->get('logger')->error('Error while saving an entry', [
+                'exception' => $e,
+                'entry' => $entry,
+            ]);
+
             return false;
         }
 
@@ -60,11 +66,12 @@ class EntryController extends Controller
                 return $this->redirect($this->generateUrl('view', ['id' => $existingEntry->getId()]));
             }
 
-            $this->updateEntry($entry);
-            $this->get('session')->getFlashBag()->add(
-                'notice',
-                'flashes.entry.notice.entry_saved'
-            );
+            $message = 'flashes.entry.notice.entry_saved';
+            if (false === $this->updateEntry($entry)) {
+                $message = 'flashes.entry.notice.entry_saved_failed';
+            }
+
+            $this->get('session')->getFlashBag()->add('notice', $message);
 
             return $this->redirect($this->generateUrl('homepage'));
         }
diff --git a/src/Wallabag/CoreBundle/DataFixtures/ORM/LoadConfigData.php b/src/Wallabag/CoreBundle/DataFixtures/ORM/LoadConfigData.php
index 5a376453..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
 {
@@ -16,12 +15,6 @@ class LoadConfigData extends AbstractFixture implements OrderedFixtureInterface
     public function load(ObjectManager $manager)
     {
         $adminConfig = new Config($this->getReference('admin-user'));
-        $taggingRule = new TaggingRule();
-
-        $taggingRule->setConfig($adminConfig);
-        $taggingRule->setRule('title matches "wallabag"');
-        $taggingRule->setTags(['wallabag']);
-        $manager->persist($taggingRule);
 
         $adminConfig->setTheme('material');
         $adminConfig->setItemsPerPage(30);
diff --git a/src/Wallabag/CoreBundle/DataFixtures/ORM/LoadTaggingRuleData.php b/src/Wallabag/CoreBundle/DataFixtures/ORM/LoadTaggingRuleData.php
index 2e1cc270..7efe6356 100644
--- a/src/Wallabag/CoreBundle/DataFixtures/ORM/LoadTaggingRuleData.php
+++ b/src/Wallabag/CoreBundle/DataFixtures/ORM/LoadTaggingRuleData.php
@@ -28,6 +28,14 @@ class LoadTaggingRuleData extends AbstractFixture implements OrderedFixtureInter
 
         $manager->persist($tr2);
 
+        $tr3 = new TaggingRule();
+
+        $tr3->setRule('title matches "wallabag"');
+        $tr3->setTags(['wallabag']);
+        $tr3->setConfig($this->getReference('admin-config'));
+
+        $manager->persist($tr3);
+
         $manager->flush();
     }
 
diff --git a/src/Wallabag/CoreBundle/Entity/Entry.php b/src/Wallabag/CoreBundle/Entity/Entry.php
index 1271f1f5..ceae78b0 100644
--- a/src/Wallabag/CoreBundle/Entity/Entry.php
+++ b/src/Wallabag/CoreBundle/Entity/Entry.php
@@ -177,8 +177,16 @@ class Entry
     private $user;
 
     /**
-     * @ORM\ManyToMany(targetEntity="Tag", inversedBy="entries", cascade={"persist", "remove"})
-     * @ORM\JoinTable
+     * @ORM\ManyToMany(targetEntity="Tag", inversedBy="entries", cascade={"persist"})
+     * @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/Resources/translations/messages.da.yml b/src/Wallabag/CoreBundle/Resources/translations/messages.da.yml
index 1e58352d..213d5790 100644
--- a/src/Wallabag/CoreBundle/Resources/translations/messages.da.yml
+++ b/src/Wallabag/CoreBundle/Resources/translations/messages.da.yml
@@ -397,6 +397,7 @@ flashes:
         notice:
             # entry_already_saved: 'Entry already saved on %date%'
             # entry_saved: 'Entry saved'
+            # entry_saved_failed: 'Failed to save entry'
             # entry_updated: 'Entry updated'
             # entry_reloaded: 'Entry reloaded'
             # entry_reload_failed: 'Failed to reload entry'
diff --git a/src/Wallabag/CoreBundle/Resources/translations/messages.de.yml b/src/Wallabag/CoreBundle/Resources/translations/messages.de.yml
index 512b01bc..89cbbbf8 100644
--- a/src/Wallabag/CoreBundle/Resources/translations/messages.de.yml
+++ b/src/Wallabag/CoreBundle/Resources/translations/messages.de.yml
@@ -397,6 +397,7 @@ flashes:
         notice:
             entry_already_saved: 'Eintrag bereits am %date% gespeichert'
             entry_saved: 'Eintag gespeichert'
+            # entry_saved_failed: 'Failed to save entry'
             entry_updated: 'Eintrag aktualisiert'
             entry_reloaded: 'Eintrag neugeladen'
             entry_reload_failed: 'Neuladen des Eintrags fehlgeschlagen'
diff --git a/src/Wallabag/CoreBundle/Resources/translations/messages.en.yml b/src/Wallabag/CoreBundle/Resources/translations/messages.en.yml
index ae03f809..b65e4522 100644
--- a/src/Wallabag/CoreBundle/Resources/translations/messages.en.yml
+++ b/src/Wallabag/CoreBundle/Resources/translations/messages.en.yml
@@ -397,6 +397,7 @@ flashes:
         notice:
             entry_already_saved: 'Entry already saved on %date%'
             entry_saved: 'Entry saved'
+            entry_saved_failed: 'Failed to save entry'
             entry_updated: 'Entry updated'
             entry_reloaded: 'Entry reloaded'
             entry_reload_failed: 'Failed to reload entry'
diff --git a/src/Wallabag/CoreBundle/Resources/translations/messages.es.yml b/src/Wallabag/CoreBundle/Resources/translations/messages.es.yml
index 0ae2e243..8351bcf4 100644
--- a/src/Wallabag/CoreBundle/Resources/translations/messages.es.yml
+++ b/src/Wallabag/CoreBundle/Resources/translations/messages.es.yml
@@ -397,6 +397,7 @@ flashes:
         notice:
             entry_already_saved: 'Entrada ya guardada por %fecha%'
             entry_saved: 'Entrada guardada'
+            # entry_saved_failed: 'Failed to save entry'
             entry_updated: 'Entrada actualizada'
             entry_reloaded: 'Entrada recargada'
             entry_reload_failed: 'Entrada recargada reprobada'
diff --git a/src/Wallabag/CoreBundle/Resources/translations/messages.fa.yml b/src/Wallabag/CoreBundle/Resources/translations/messages.fa.yml
index b0601543..db6e5960 100644
--- a/src/Wallabag/CoreBundle/Resources/translations/messages.fa.yml
+++ b/src/Wallabag/CoreBundle/Resources/translations/messages.fa.yml
@@ -96,8 +96,8 @@ config:
         rule_label: 'قانون'
         tags_label: 'برچسب‌ها'
         faq:
-             title: 'پرسش‌های متداول'
-             tagging_rules_definition_title: 'برچسب‌گذاری خودکار یعنی چه؟'
+            title: 'پرسش‌های متداول'
+            tagging_rules_definition_title: 'برچسب‌گذاری خودکار یعنی چه؟'
         #     tagging_rules_definition_description: 'They are rules used by Wallabag to automatically tag new entries.<br />Each time a new entry is added, all the tagging rules will be used to add the tags you configured, thus saving you the trouble to manually classify your entries.'
         #     how_to_use_them_title: 'How do I use them?'
         #     how_to_use_them_description: 'Let assume you want to tag new entries as « <i>short reading</i> » when the reading time is inferior to 3 minutes.<br />In that case, you should put « readingTime &lt;= 3 » in the <i>Rule</i> field and « <i>short reading</i> » in the <i>Tags</i> field.<br />Several tags can added simultaneously by separating them by a comma: « <i>short reading, must read</i> »<br />Complex rules can be written by using predefined operators: if « <i>readingTime &gt;= 5 AND domainName = "github.com"</i> » then tag as « <i>long reading, github </i> »'
@@ -384,30 +384,31 @@ developer:
 flashes:
     config:
         notice:
-           config_saved: 'پیکربندی ذخیره شد. برخی از تنظیمات پس از این که قطع شدید اعمال می‌شود.'
-           password_updated: 'رمز به‌روز شد'
-           password_not_updated_demo: "در حالت نمایشی نمی‌توانید رمز کاربر را عوض کنید."
-           user_updated: 'اطلاعات به‌روز شد'
-           rss_updated: 'اطلاعات آر-اس-اس به‌روز شد'
-           tagging_rules_updated: 'برچسب‌گذاری خودکار به‌روز شد'
-           tagging_rules_deleted: 'قانون برچسب‌گذاری پاک شد'
-           user_added: 'کابر "%username%" افزوده شد'
-           rss_token_updated: 'کد آر-اس-اس به‌روز شد'
+            config_saved: 'پیکربندی ذخیره شد. برخی از تنظیمات پس از این که قطع شدید اعمال می‌شود.'
+            password_updated: 'رمز به‌روز شد'
+            password_not_updated_demo: "در حالت نمایشی نمی‌توانید رمز کاربر را عوض کنید."
+            user_updated: 'اطلاعات به‌روز شد'
+            rss_updated: 'اطلاعات آر-اس-اس به‌روز شد'
+            tagging_rules_updated: 'برچسب‌گذاری خودکار به‌روز شد'
+            tagging_rules_deleted: 'قانون برچسب‌گذاری پاک شد'
+            user_added: 'کابر "%username%" افزوده شد'
+            rss_token_updated: 'کد آر-اس-اس به‌روز شد'
     entry:
         notice:
-           entry_already_saved: 'این مقاله در تاریخ %date% ذخیره شده بود'
-           entry_saved: 'مقاله ذخیره شد'
-           entry_updated: 'مقاله به‌روز شد'
-           entry_reloaded: 'مقاله به‌روز شد'
-           entry_reload_failed: 'به‌روزرسانی مقاله شکست خورد'
-           entry_archived: 'مقاله بایگانی شد'
-           entry_unarchived: 'مقاله از بایگانی درآمد'
-           entry_starred: 'مقاله برگزیده شد'
-           entry_unstarred: 'مقاله نابرگزیده شد'
-           entry_deleted: 'مقاله پاک شد'
+            entry_already_saved: 'این مقاله در تاریخ %date% ذخیره شده بود'
+            entry_saved: 'مقاله ذخیره شد'
+            # entry_saved_failed: 'Failed to save entry'
+            entry_updated: 'مقاله به‌روز شد'
+            entry_reloaded: 'مقاله به‌روز شد'
+            entry_reload_failed: 'به‌روزرسانی مقاله شکست خورد'
+            entry_archived: 'مقاله بایگانی شد'
+            entry_unarchived: 'مقاله از بایگانی درآمد'
+            entry_starred: 'مقاله برگزیده شد'
+            entry_unstarred: 'مقاله نابرگزیده شد'
+            entry_deleted: 'مقاله پاک شد'
     tag:
         notice:
-           tag_added: 'برچسب افزوده شد'
+            tag_added: 'برچسب افزوده شد'
     import:
         notice:
             failed: 'درون‌ریزی شکست خورد. لطفاً دوباره تلاش کنید.'
diff --git a/src/Wallabag/CoreBundle/Resources/translations/messages.fr.yml b/src/Wallabag/CoreBundle/Resources/translations/messages.fr.yml
index 954ab0c3..9fc8ca2a 100644
--- a/src/Wallabag/CoreBundle/Resources/translations/messages.fr.yml
+++ b/src/Wallabag/CoreBundle/Resources/translations/messages.fr.yml
@@ -397,6 +397,7 @@ flashes:
         notice:
             entry_already_saved: 'Article déjà sauvergardé le %date%'
             entry_saved: 'Article enregistré'
+            entry_saved_failed: "L'enregistrement a échoué"
             entry_updated: 'Article mis à jour'
             entry_reloaded: 'Article rechargé'
             entry_reload_failed: "Le rechargement de l'article a échoué"
diff --git a/src/Wallabag/CoreBundle/Resources/translations/messages.it.yml b/src/Wallabag/CoreBundle/Resources/translations/messages.it.yml
index 4f3ac090..00ed9c98 100644
--- a/src/Wallabag/CoreBundle/Resources/translations/messages.it.yml
+++ b/src/Wallabag/CoreBundle/Resources/translations/messages.it.yml
@@ -396,6 +396,7 @@ flashes:
         notice:
             entry_already_saved: 'Contenuto già salvato in data %date%'
             entry_saved: 'Contenuto salvato'
+            # entry_saved_failed: 'Failed to save entry'
             entry_updated: 'Contenuto aggiornato'
             entry_reloaded: 'Contenuto ricaricato'
             entry_reload_failed: 'Errore nel ricaricamento del contenuto'
diff --git a/src/Wallabag/CoreBundle/Resources/translations/messages.oc.yml b/src/Wallabag/CoreBundle/Resources/translations/messages.oc.yml
index 24b8c6e2..c81c852b 100644
--- a/src/Wallabag/CoreBundle/Resources/translations/messages.oc.yml
+++ b/src/Wallabag/CoreBundle/Resources/translations/messages.oc.yml
@@ -397,6 +397,7 @@ flashes:
         notice:
             entry_already_saved: 'Article ja salvargardat lo %date%'
             entry_saved: 'Article enregistrat'
+            # entry_saved_failed: 'Failed to save entry'
             entry_updated: 'Article mes a jorn'
             entry_reloaded: 'Article recargat'
             entry_reload_failed: "Fracàs de l'actualizacion de l'article"
diff --git a/src/Wallabag/CoreBundle/Resources/translations/messages.pl.yml b/src/Wallabag/CoreBundle/Resources/translations/messages.pl.yml
index fa23f0b8..3707a857 100644
--- a/src/Wallabag/CoreBundle/Resources/translations/messages.pl.yml
+++ b/src/Wallabag/CoreBundle/Resources/translations/messages.pl.yml
@@ -397,6 +397,7 @@ flashes:
         notice:
             entry_already_saved: 'Wpis już został dodany %date%'
             entry_saved: 'Wpis zapisany'
+            # entry_saved_failed: 'Failed to save entry'
             entry_updated: 'Wpis zaktualizowany'
             entry_reloaded: 'Wpis ponownie załadowany'
             entry_reload_failed: 'Błąd ponownego załadowania'
diff --git a/src/Wallabag/CoreBundle/Resources/translations/messages.ro.yml b/src/Wallabag/CoreBundle/Resources/translations/messages.ro.yml
index 648ba370..27be1edc 100644
--- a/src/Wallabag/CoreBundle/Resources/translations/messages.ro.yml
+++ b/src/Wallabag/CoreBundle/Resources/translations/messages.ro.yml
@@ -397,6 +397,7 @@ flashes:
         notice:
             # entry_already_saved: 'Entry already saved on %date%'
             # entry_saved: 'Entry saved'
+            # entry_saved_failed: 'Failed to save entry'
             # entry_updated: 'Entry updated'
             # entry_reloaded: 'Entry reloaded'
             # entry_reload_failed: 'Failed to reload entry'
diff --git a/src/Wallabag/CoreBundle/Resources/translations/messages.tr.yml b/src/Wallabag/CoreBundle/Resources/translations/messages.tr.yml
index 0fc8d328..85ee7316 100644
--- a/src/Wallabag/CoreBundle/Resources/translations/messages.tr.yml
+++ b/src/Wallabag/CoreBundle/Resources/translations/messages.tr.yml
@@ -397,6 +397,7 @@ flashes:
         notice:
             entry_already_saved: 'Entry already saved on %date%'
             entry_saved: 'Makale kaydedildi'
+            # entry_saved_failed: 'Failed to save entry'
             # entry_updated: 'Entry updated'
             entry_reloaded: 'Makale içeriği yenilendi'
             # entry_reload_failed: 'Failed to reload 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()