]> git.immae.eu Git - github/wallabag/wallabag.git/commitdiff
Avoid losing entry when fetching fail 1941/head
authorJeremy Benoist <jeremy.benoist@gmail.com>
Sat, 17 Sep 2016 05:40:56 +0000 (07:40 +0200)
committerJeremy Benoist <jeremy.benoist@gmail.com>
Sat, 17 Sep 2016 05:40:56 +0000 (07:40 +0200)
Instead of just say “Failed to save entry” we’ll save the entry at all cost and try to fetch content. If fetching content failed, the entry will still be saved at least, but without content.

18 files changed:
src/Wallabag/CoreBundle/Controller/EntryController.php
src/Wallabag/CoreBundle/Resources/translations/messages.da.yml
src/Wallabag/CoreBundle/Resources/translations/messages.de.yml
src/Wallabag/CoreBundle/Resources/translations/messages.en.yml
src/Wallabag/CoreBundle/Resources/translations/messages.es.yml
src/Wallabag/CoreBundle/Resources/translations/messages.fa.yml
src/Wallabag/CoreBundle/Resources/translations/messages.fr.yml
src/Wallabag/CoreBundle/Resources/translations/messages.it.yml
src/Wallabag/CoreBundle/Resources/translations/messages.oc.yml
src/Wallabag/CoreBundle/Resources/translations/messages.pl.yml
src/Wallabag/CoreBundle/Resources/translations/messages.ro.yml
src/Wallabag/CoreBundle/Resources/translations/messages.tr.yml
src/Wallabag/ImportBundle/Import/AbstractImport.php
src/Wallabag/ImportBundle/Import/PocketImport.php
src/Wallabag/ImportBundle/Import/ReadabilityImport.php
src/Wallabag/ImportBundle/Import/WallabagImport.php
tests/Wallabag/ImportBundle/Import/PocketImportTest.php
tests/Wallabag/ImportBundle/Import/WallabagV2ImportTest.php

index 624576b5bee67a43be98418a72e5c4ce93831233..40111af0b5e37a8209d28745d4e9281bee0225f7 100644 (file)
@@ -17,26 +17,35 @@ use Sensio\Bundle\FrameworkExtraBundle\Configuration\Cache;
 class EntryController extends Controller
 {
     /**
-     * @param Entry $entry
+     * Fetch content and update entry.
+     * In case it fails, entry will return to avod loosing the data.
+     *
+     * @param Entry  $entry
+     * @param string $prefixMessage Should be the translation key: entry_saved or entry_reloaded
+     *
+     * @return Entry
      */
-    private function updateEntry(Entry $entry)
+    private function updateEntry(Entry $entry, $prefixMessage = 'entry_saved')
     {
+        // put default title in case of fetching content failed
+        $entry->setTitle('No title found');
+
+        $message = 'flashes.entry.notice.'.$prefixMessage;
+
         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;
+            $message = 'flashes.entry.notice.'.$prefixMessage.'_failed';
         }
 
-        return true;
+        $this->get('session')->getFlashBag()->add('notice', $message);
+
+        return $entry;
     }
 
     /**
@@ -66,12 +75,11 @@ class EntryController extends Controller
                 return $this->redirect($this->generateUrl('view', ['id' => $existingEntry->getId()]));
             }
 
-            $message = 'flashes.entry.notice.entry_saved';
-            if (false === $this->updateEntry($entry)) {
-                $message = 'flashes.entry.notice.entry_saved_failed';
-            }
+            $this->updateEntry($entry);
 
-            $this->get('session')->getFlashBag()->add('notice', $message);
+            $em = $this->getDoctrine()->getManager();
+            $em->persist($entry);
+            $em->flush();
 
             return $this->redirect($this->generateUrl('homepage'));
         }
@@ -95,6 +103,10 @@ class EntryController extends Controller
 
         if (false === $this->checkIfEntryAlreadyExists($entry)) {
             $this->updateEntry($entry);
+
+            $em = $this->getDoctrine()->getManager();
+            $em->persist($entry);
+            $em->flush();
         }
 
         return $this->redirect($this->generateUrl('homepage'));
@@ -316,15 +328,11 @@ class EntryController extends Controller
     {
         $this->checkUserAction($entry);
 
-        $message = 'flashes.entry.notice.entry_reloaded';
-        if (false === $this->updateEntry($entry)) {
-            $message = 'flashes.entry.notice.entry_reload_failed';
-        }
+        $this->updateEntry($entry, 'entry_reloaded');
 
-        $this->get('session')->getFlashBag()->add(
-            'notice',
-            $message
-        );
+        $em = $this->getDoctrine()->getManager();
+        $em->persist($entry);
+        $em->flush();
 
         return $this->redirect($this->generateUrl('view', ['id' => $entry->getId()]));
     }
index 0a7c6e8cf6812b636efe8857fcf1dbc6a4b78cb3..9f051edbd66bb52b514ef6045b81cb4ff0a6bfdb 100644 (file)
@@ -414,10 +414,10 @@ flashes:
         notice:
             # entry_already_saved: 'Entry already saved on %date%'
             # entry_saved: 'Entry saved'
-            # entry_saved_failed: 'Failed to save entry'
+            # entry_saved_failed: 'Entry saved but fetching content failed'
             # entry_updated: 'Entry updated'
             # entry_reloaded: 'Entry reloaded'
-            # entry_reload_failed: 'Failed to reload entry'
+            # entry_reload_failed: 'Entry reloaded but fetching content failed'
             entry_archived: 'Artikel arkiveret'
             entry_unarchived: 'Artikel ikke længere arkiveret'
             entry_starred: 'Artikel markeret som favorit'
index a400686eafdcdb34e4355de453e77f03f5d0188f..cbfacd55386510289a4a9cea255ebaf1d4c5eac3 100644 (file)
@@ -414,10 +414,10 @@ flashes:
         notice:
             entry_already_saved: 'Eintrag bereits am %date% gespeichert'
             entry_saved: 'Eintrag gespeichert'
-            # entry_saved_failed: 'Failed to save entry'
+            # entry_saved_failed: 'Entry saved but fetching content failed'
             entry_updated: 'Eintrag aktualisiert'
             entry_reloaded: 'Eintrag neugeladen'
-            entry_reload_failed: 'Neuladen des Eintrags fehlgeschlagen'
+            # entry_reload_failed: 'Entry reloaded but fetching content failed'
             entry_archived: 'Artikel archiviert'
             entry_unarchived: 'Artikel dearchiviert'
             entry_starred: 'Artikel favorisiert'
index 2d097cafd054eb91838b0a980dd019d97eb1dfab..21e2405ca9395e92eccba928508967043d657306 100644 (file)
@@ -416,10 +416,10 @@ flashes:
         notice:
             entry_already_saved: 'Entry already saved on %date%'
             entry_saved: 'Entry saved'
-            entry_saved_failed: 'Failed to save entry'
+            entry_saved_failed: 'Entry saved but fetching content failed'
             entry_updated: 'Entry updated'
             entry_reloaded: 'Entry reloaded'
-            entry_reload_failed: 'Failed to reload entry'
+            entry_reload_failed: 'Entry reloaded but fetching content failed'
             entry_archived: 'Entry archived'
             entry_unarchived: 'Entry unarchived'
             entry_starred: 'Entry starred'
index 1dbff0225c391cf400fd5c2790b96ec5178a380d..43f376d4b2e2a9db5d4ec9ef4052abfd545380f2 100644 (file)
@@ -414,10 +414,10 @@ flashes:
         notice:
             entry_already_saved: 'Entrada ya guardada por %fecha%'
             entry_saved: 'Entrada guardada'
-            # entry_saved_failed: 'Failed to save entry'
+            # entry_saved_failed: 'Entry saved but fetching content failed'
             entry_updated: 'Entrada actualizada'
             entry_reloaded: 'Entrada recargada'
-            entry_reload_failed: 'Entrada recargada reprobada'
+            # entry_reload_failed: 'Entry reloaded but fetching content failed'
             entry_archived: 'Artículo archivado'
             entry_unarchived: 'Artículo desarchivado'
             entry_starred: 'Artículo guardado en los favoritos'
index ad13c9404b8c5cbf667a56234ad144427c628795..56418ef9e8a6cf9044cdaa82c1810ff99ea69a4e 100644 (file)
@@ -414,10 +414,10 @@ flashes:
         notice:
             entry_already_saved: 'این مقاله در تاریخ %date% ذخیره شده بود'
             entry_saved: 'مقاله ذخیره شد'
-            # entry_saved_failed: 'Failed to save entry'
+            # entry_saved_failed: 'Entry saved but fetching content failed'
             entry_updated: 'مقاله به‌روز شد'
             entry_reloaded: 'مقاله به‌روز شد'
-            entry_reload_failed: 'به‌روزرسانی مقاله شکست خورد'
+            # entry_reload_failed: 'Entry reloaded but fetching content failed'
             entry_archived: 'مقاله بایگانی شد'
             entry_unarchived: 'مقاله از بایگانی درآمد'
             entry_starred: 'مقاله برگزیده شد'
index 35e5c9d075964ddf59386ffb8b7c518aec406fd6..bde21866e9a2914f361be0b7a6879da86be85e60 100644 (file)
@@ -416,10 +416,10 @@ flashes:
         notice:
             entry_already_saved: 'Article déjà sauvergardé le %date%'
             entry_saved: 'Article enregistré'
-            entry_saved_failed: "L'enregistrement a échoué"
+            entry_saved_failed: 'Article enregistré mais impossible de récupérer le contenu'
             entry_updated: 'Article mis à jour'
             entry_reloaded: 'Article rechargé'
-            entry_reload_failed: "Le rechargement de l'article a échoué"
+            entry_reload_failed: "Article mis à jour mais impossible de récupérer le contenu"
             entry_archived: 'Article marqué comme lu'
             entry_unarchived: 'Article marqué comme non lu'
             entry_starred: 'Article ajouté dans les favoris'
index 5bc896c3d1e6b886885cffe4e2625a7ffc58c48a..26bb31ba76da455627a36dd61b50f9b802e6671b 100644 (file)
@@ -413,10 +413,10 @@ flashes:
         notice:
             entry_already_saved: 'Contenuto già salvato in data %date%'
             entry_saved: 'Contenuto salvato'
-            # entry_saved_failed: 'Failed to save entry'
+            # entry_saved_failed: 'Entry saved but fetching content failed'
             entry_updated: 'Contenuto aggiornato'
             entry_reloaded: 'Contenuto ricaricato'
-            entry_reload_failed: 'Errore nel ricaricamento del contenuto'
+            # entry_reload_failed: 'Entry reloaded but fetching content failed'
             entry_archived: 'Contenuto archiviato'
             entry_unarchived: 'Contenuto dis-archiviato'
             entry_starred: 'Contenuto segnato come preferito'
index b0194c5948b9a2af9f086e3d7839fc02632b4e12..2c4df867070dd582e55ca484b6dc1796b539de6a 100644 (file)
@@ -414,10 +414,10 @@ flashes:
         notice:
             entry_already_saved: 'Article ja salvargardat lo %date%'
             entry_saved: 'Article enregistrat'
-            entry_saved_failed: "Fracàs de l'enregistrament de l'entrada"
+            # entry_saved_failed: 'Entry saved but fetching content failed'
             entry_updated: 'Article mes a jorn'
             entry_reloaded: 'Article recargat'
-            entry_reload_failed: "Fracàs de l'actualizacion de l'article"
+            # entry_reload_failed: 'Entry reloaded but fetching content failed'
             entry_archived: 'Article marcat coma legit'
             entry_unarchived: 'Article marcat coma pas legit'
             entry_starred: 'Article apondut dins los favorits'
index 6412ad16945073998fd404464f84d540991f858a..fb8219666dde6d537397d01547d20facff77f586 100644 (file)
@@ -414,10 +414,10 @@ flashes:
         notice:
             entry_already_saved: 'Wpis już został dodany %date%'
             entry_saved: 'Wpis zapisany'
-            entry_saved_failed: 'Zapis artykułu się nie powiódł'
+            # entry_saved_failed: 'Entry saved but fetching content failed'
             entry_updated: 'Wpis zaktualizowany'
             entry_reloaded: 'Wpis ponownie załadowany'
-            entry_reload_failed: 'Błąd ponownego załadowania'
+            # entry_reload_failed: 'Entry reloaded but fetching content failed'
             entry_archived: 'Wpis dodany do archiwum'
             entry_unarchived: 'Wpis usunięty z archiwum'
             entry_starred: 'Wpis oznaczony gwiazdką'
index 11b744c762eba4a9e63bf8f61f26cad5030fb4ce..3d22e29d34b3ae42095024557c473d8bc84686dd 100644 (file)
@@ -414,10 +414,10 @@ flashes:
         notice:
             # entry_already_saved: 'Entry already saved on %date%'
             # entry_saved: 'Entry saved'
-            # entry_saved_failed: 'Failed to save entry'
+            # entry_saved_failed: 'Entry saved but fetching content failed'
             # entry_updated: 'Entry updated'
             # entry_reloaded: 'Entry reloaded'
-            # entry_reload_failed: 'Failed to reload entry'
+            # entry_reload_failed: 'Entry reloaded but fetching content failed'
             entry_archived: 'Articol arhivat'
             entry_unarchived: 'Articol dezarhivat'
             entry_starred: 'Articol adăugat la favorite'
index d6aaacfec4255b92b42242334f27661a728eafef..5099b002c29a1a3e635d124d7faf424abd41ab93 100644 (file)
@@ -414,10 +414,10 @@ flashes:
         notice:
             entry_already_saved: 'Entry already saved on %date%'
             entry_saved: 'Makale kaydedildi'
-            # entry_saved_failed: 'Failed to save entry'
+            # entry_saved_failed: 'Entry saved but fetching content failed'
             # entry_updated: 'Entry updated'
             entry_reloaded: 'Makale içeriği yenilendi'
-            # entry_reload_failed: 'Failed to reload entry'
+            # entry_reload_failed: 'Entry reloaded but fetching content failed'
             entry_archived: 'Makale arşivlendi'
             entry_unarchived: 'Makale arşivden çıkartıldı'
             entry_starred: 'Makale favorilere eklendi'
index 2af0e69b473a0e117f02c2efd66de5512004331c..a1a14576f13213e0869f940ab05ee5ec1bc3f224 100644 (file)
@@ -79,20 +79,20 @@ abstract class AbstractImport implements ImportInterface
 
     /**
      * Fetch content from the ContentProxy (using graby).
-     * If it fails return false instead of the updated entry.
+     * If it fails return the given entry to be saved in all case (to avoid user to loose the content).
      *
      * @param Entry  $entry   Entry to update
      * @param string $url     Url to grab content for
      * @param array  $content An array with AT LEAST keys title, html, url, language & content_type to skip the fetchContent from the url
      *
-     * @return Entry|false
+     * @return Entry
      */
     protected function fetchContent(Entry $entry, $url, array $content = [])
     {
         try {
             return $this->contentProxy->updateEntry($entry, $url, $content);
         } catch (\Exception $e) {
-            return false;
+            return $entry;
         }
     }
 
index 1bf22d6874f8f2ec24f1b8d03d163fb559562186..e00eb44b369f30306c7163fdb663c89e8053280d 100644 (file)
@@ -199,24 +199,16 @@ class PocketImport extends AbstractImport
         }
 
         $entry = new Entry($this->user);
-        $entry = $this->fetchContent($entry, $url);
-
-        // jump to next entry in case of problem while getting content
-        if (false === $entry) {
-            ++$this->skippedEntries;
+        $entry->setUrl($url);
 
-            return;
-        }
+        // update entry with content (in case fetching failed, the given entry will be return)
+        $entry = $this->fetchContent($entry, $url);
 
         // 0, 1, 2 - 1 if the item is archived - 2 if the item should be deleted
-        if ($importedEntry['status'] == 1 || $this->markAsRead) {
-            $entry->setArchived(true);
-        }
+        $entry->setArchived($importedEntry['status'] == 1 || $this->markAsRead);
 
         // 0 or 1 - 1 If the item is starred
-        if ($importedEntry['favorite'] == 1) {
-            $entry->setStarred(true);
-        }
+        $entry->setStarred($importedEntry['favorite'] == 1);
 
         $title = 'Untitled';
         if (isset($importedEntry['resolved_title']) && $importedEntry['resolved_title'] != '') {
@@ -226,7 +218,6 @@ class PocketImport extends AbstractImport
         }
 
         $entry->setTitle($title);
-        $entry->setUrl($url);
 
         // 0, 1, or 2 - 1 if the item has images in it - 2 if the item is an image
         if (isset($importedEntry['has_image']) && $importedEntry['has_image'] > 0 && isset($importedEntry['images'][1])) {
index b852f8f01c4066a20a849f89da9acbe1fee3b19c..fa2b705391707848389189a5810c2937303b5498 100644 (file)
@@ -103,18 +103,12 @@ class ReadabilityImport extends AbstractImport
             'created_at' => $importedEntry['date_added'],
         ];
 
-        $entry = $this->fetchContent(
-            new Entry($this->user),
-            $data['url'],
-            $data
-        );
-
-        // jump to next entry in case of problem while getting content
-        if (false === $entry) {
-            ++$this->skippedEntries;
+        $entry = new Entry($this->user);
+        $entry->setUrl($data['url']);
+        $entry->setTitle($data['title']);
 
-            return;
-        }
+        // update entry with content (in case fetching failed, the given entry will be return)
+        $entry = $this->fetchContent($entry, $data['url'], $data);
 
         $entry->setArchived($data['is_archived']);
         $entry->setStarred($data['is_starred']);
index 969a6a04981a8eaf4a4339059f43e3c52a33b5a5..043bb0a2363e3e6982a7906a0a80791acba0da13 100644 (file)
@@ -101,18 +101,12 @@ abstract class WallabagImport extends AbstractImport
 
         $data = $this->prepareEntry($importedEntry);
 
-        $entry = $this->fetchContent(
-            new Entry($this->user),
-            $importedEntry['url'],
-            $data
-        );
-
-        // jump to next entry in case of problem while getting content
-        if (false === $entry) {
-            ++$this->skippedEntries;
+        $entry = new Entry($this->user);
+        $entry->setUrl($data['url']);
+        $entry->setTitle($data['title']);
 
-            return;
-        }
+        // update entry with content (in case fetching failed, the given entry will be return)
+        $entry = $this->fetchContent($entry, $data['url'], $data);
 
         if (array_key_exists('tags', $data)) {
             $this->contentProxy->assignTagsToEntry(
index 48fbbfb615ea7fd97abf184bee3b601635a017f1..952521a2a90759b19e461b19adc01ba758cdda7c 100644 (file)
@@ -566,6 +566,8 @@ JSON;
                     "status": 1,
                     "list": {
                         "229279689": {
+                            "status": "1",
+                            "favorite": "1",
                             "resolved_url": "http://www.grantland.com/blog/the-triangle/post/_/id/38347/ryder-cup-preview"
                         }
                     }
@@ -603,6 +605,6 @@ JSON;
         $res = $pocketImport->import();
 
         $this->assertTrue($res);
-        $this->assertEquals(['skipped' => 1, 'imported' => 0, 'queued' => 0], $pocketImport->getSummary());
+        $this->assertEquals(['skipped' => 0, 'imported' => 1, 'queued' => 0], $pocketImport->getSummary());
     }
 }
index b4017f720ccb9ded0a8a184330aaa8a5264785de..12bd6bdd0e8a3ab56278b40ab56e3849575d4d9f 100644 (file)
@@ -256,6 +256,6 @@ class WallabagV2ImportTest extends \PHPUnit_Framework_TestCase
         $res = $wallabagV2Import->import();
 
         $this->assertTrue($res);
-        $this->assertEquals(['skipped' => 24, 'imported' => 0, 'queued' => 0], $wallabagV2Import->getSummary());
+        $this->assertEquals(['skipped' => 22, 'imported' => 2, 'queued' => 0], $wallabagV2Import->getSummary());
     }
 }