Merge pull request #3330 from franek/set-starred-date

Add starred_at field which is set when an entry is starred

app/DoctrineMigrations/Version20170824113337.php

@@ -0,0 +1,63 @@
+<?php
+
+namespace Application\Migrations;
+
+use Doctrine\DBAL\Migrations\AbstractMigration;
+use Doctrine\DBAL\Schema\Schema;
+use Symfony\Component\DependencyInjection\ContainerAwareInterface;
+use Symfony\Component\DependencyInjection\ContainerInterface;
+
+/**
+ * Add starred_at column and set its value to updated_at for is_starred entries.
+ */
+class Version20170824113337 extends AbstractMigration implements ContainerAwareInterface
+{
+    /**
+     * @var ContainerInterface
+     */
+    private $container;
+
+    public function setContainer(ContainerInterface $container = null)
+    {
+        $this->container = $container;
+    }
+
+    /**
+     * @param Schema $schema
+     */
+    public function up(Schema $schema)
+    {
+        $entryTable = $schema->getTable($this->getTable('entry'));
+
+        $this->skipIf($entryTable->hasColumn('starred_at'), 'It seems that you already played this migration.');
+
+        $entryTable->addColumn('starred_at', 'datetime', [
+            'notnull' => false,
+        ]);
+    }
+
+    public function postUp(Schema $schema)
+    {
+        $entryTable = $schema->getTable($this->getTable('entry'));
+        $this->skipIf(!$entryTable->hasColumn('starred_at'), 'Unable to add starred_at colum');
+
+        $this->connection->executeQuery('UPDATE ' . $this->getTable('entry') . ' SET starred_at = updated_at WHERE is_starred = true');
+    }
+
+    /**
+     * @param Schema $schema
+     */
+    public function down(Schema $schema)
+    {
+        $entryTable = $schema->getTable($this->getTable('entry'));
+
+        $this->skipIf(!$entryTable->hasColumn('starred_at'), 'It seems that you already played this migration.');
+
+        $entryTable->dropColumn('starred_at');
+    }
+
+    private function getTable($tableName)
+    {
+        return $this->container->getParameter('database_table_prefix') . $tableName;
+    }
+}

src/Wallabag/ApiBundle/Controller/EntryRestController.php

@@ -361,7 +361,7 @@ class EntryRestController extends WallabagRestController
         }
 
         if (null !== $data['isStarred']) {
-            $entry->setStarred((bool) $data['isStarred']);
+            $entry->updateStar((bool) $data['isStarred']);
         }
 
         if (!empty($data['tags'])) {
@@ -464,7 +464,7 @@ class EntryRestController extends WallabagRestController
         }
 
         if (null !== $data['isStarred']) {
-            $entry->setStarred((bool) $data['isStarred']);
+            $entry->updateStar((bool) $data['isStarred']);
         }
 
         if (!empty($data['tags'])) {

src/Wallabag/CoreBundle/Controller/EntryController.php

@@ -333,6 +333,7 @@ class EntryController extends Controller
         $this->checkUserAction($entry);
 
         $entry->toggleStar();
+        $entry->updateStar($entry->isStarred());
         $this->getDoctrine()->getManager()->flush();
 
         $message = 'flashes.entry.notice.entry_unstarred';

src/Wallabag/CoreBundle/Entity/Entry.php

@@ -142,6 +142,15 @@ class Entry
      */
     private $publishedBy;
 
+    /**
+     * @var \DateTime
+     *
+     * @ORM\Column(name="starred_at", type="datetime", nullable=true)
+     *
+     * @Groups({"entries_for_user", "export_all"})
+     */
+    private $starredAt = null;
+
     /**
      * @ORM\OneToMany(targetEntity="Wallabag\AnnotationBundle\Entity\Annotation", mappedBy="entry", cascade={"persist", "remove"})
      * @ORM\JoinTable
@@ -475,6 +484,44 @@ class Entry
         return $this->updatedAt;
     }
 
+    /**
+     * @return \DateTime|null
+     */
+    public function getStarredAt()
+    {
+        return $this->starredAt;
+    }
+
+    /**
+     * @param \DateTime|null $starredAt
+     *
+     * @return Entry
+     */
+    public function setStarredAt($starredAt = null)
+    {
+        $this->starredAt = $starredAt;
+
+        return $this;
+    }
+
+    /**
+     * update isStarred and starred_at fields.
+     *
+     * @param bool $isStarred
+     *
+     * @return Entry
+     */
+    public function updateStar($isStarred = false)
+    {
+        $this->setStarred($isStarred);
+        $this->setStarredAt(null);
+        if ($this->isStarred()) {
+            $this->setStarredAt(new \DateTime());
+        }
+
+        return $this;
+    }
+
     /**
      * @return ArrayCollection<Annotation>
      */

src/Wallabag/CoreBundle/Repository/EntryRepository.php

@@ -65,7 +65,7 @@ class EntryRepository extends EntityRepository
     public function getBuilderForStarredByUser($userId)
     {
         return $this
-            ->getBuilderByUser($userId)
+            ->getBuilderByUser($userId, 'starredAt', 'desc')
             ->andWhere('e.isStarred = true')
         ;
     }
@@ -401,15 +401,16 @@ class EntryRepository extends EntityRepository
     /**
      * Return a query builder to used by other getBuilderFor* method.
      *
-     * @param int $userId
+     * @param int    $userId
+     * @param string $sortBy
+     * @param string $direction
      *
      * @return QueryBuilder
      */
-    private function getBuilderByUser($userId)
+    private function getBuilderByUser($userId, $sortBy = 'createdAt', $direction = 'desc')
     {
         return $this->createQueryBuilder('e')
             ->andWhere('e.user = :userId')->setParameter('userId', $userId)
-            ->orderBy('e.createdAt', 'desc')
-        ;
+            ->orderBy(sprintf('e.%s', $sortBy), $direction);
     }
 }

tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php

@@ -407,6 +407,7 @@ class EntryRestControllerTest extends WallabagApiTestCase
         $this->assertSame('http://www.lemonde.fr/pixels/article/2015/03/28/plongee-dans-l-univers-d-ingress-le-jeu-de-google-aux-frontieres-du-reel_4601155_4408996.html', $content['url']);
         $this->assertSame(0, $content['is_archived']);
         $this->assertSame(0, $content['is_starred']);
+        $this->assertNull($content['starred_at']);
         $this->assertSame('New title for my article', $content['title']);
         $this->assertSame(1, $content['user_id']);
         $this->assertCount(2, $content['tags']);
@@ -483,6 +484,7 @@ class EntryRestControllerTest extends WallabagApiTestCase
 
     public function testPostArchivedAndStarredEntry()
     {
+        $now = new \DateTime();
         $this->client->request('POST', '/api/entries.json', [
             'url' => 'http://www.lemonde.fr/idees/article/2016/02/08/preserver-la-liberte-d-expression-sur-les-reseaux-sociaux_4861503_3232.html',
             'archive' => '1',
@@ -497,6 +499,7 @@ class EntryRestControllerTest extends WallabagApiTestCase
         $this->assertSame('http://www.lemonde.fr/idees/article/2016/02/08/preserver-la-liberte-d-expression-sur-les-reseaux-sociaux_4861503_3232.html', $content['url']);
         $this->assertSame(1, $content['is_archived']);
         $this->assertSame(1, $content['is_starred']);
+        $this->assertGreaterThanOrEqual($now->getTimestamp(), (new \DateTime($content['starred_at']))->getTimestamp());
         $this->assertSame(1, $content['user_id']);
     }
 
@@ -753,6 +756,7 @@ class EntryRestControllerTest extends WallabagApiTestCase
 
     public function testSaveIsStarredAfterPatch()
     {
+        $now = new \DateTime();
         $entry = $this->client->getContainer()
             ->get('doctrine.orm.entity_manager')
             ->getRepository('WallabagCoreBundle:Entry')
@@ -770,6 +774,7 @@ class EntryRestControllerTest extends WallabagApiTestCase
         $content = json_decode($this->client->getResponse()->getContent(), true);
 
         $this->assertSame(1, $content['is_starred']);
+        $this->assertGreaterThanOrEqual($now->getTimestamp(), (new \DateTime($content['starred_at']))->getTimestamp());
     }
 
     public function dataForEntriesExistWithUrl()