Merge pull request #3093 from aaa2000/annotation-error-on-save

Displays an error with an annotation with a too long quote

app/DoctrineMigrations/Version20170511211659.php

@@ -0,0 +1,100 @@
+<?php
+
+namespace Application\Migrations;
+
+use Doctrine\DBAL\Migrations\AbstractMigration;
+use Doctrine\DBAL\Migrations\SkipMigrationException;
+use Doctrine\DBAL\Schema\Schema;
+use Symfony\Component\DependencyInjection\ContainerAwareInterface;
+use Symfony\Component\DependencyInjection\ContainerInterface;
+
+/**
+ * Increase the length of the "quote" column of "annotation" table
+ */
+class Version20170511211659 extends AbstractMigration implements ContainerAwareInterface
+{
+    /**
+     * @var ContainerInterface
+     */
+    private $container;
+
+    public function setContainer(ContainerInterface $container = null)
+    {
+        $this->container = $container;
+    }
+
+    private function getTable($tableName)
+    {
+        return $this->container->getParameter('database_table_prefix') . $tableName;
+    }
+
+    public function up(Schema $schema)
+    {
+        $tableName = $this->getTable('annotation');
+
+        switch ($this->connection->getDatabasePlatform()->getName()) {
+            case 'sqlite':
+                $this->addSql(<<<EOD
+CREATE TEMPORARY TABLE __temp__wallabag_annotation AS 
+    SELECT id, user_id, entry_id, text, created_at, updated_at, quote, ranges 
+    FROM ${tableName}
+EOD
+                );
+                $this->addSql('DROP TABLE ' . $tableName);
+                $this->addSql(<<<EOD
+CREATE TABLE ${tableName}
+(
+    id INTEGER PRIMARY KEY NOT NULL,
+    user_id INTEGER DEFAULT NULL,
+    entry_id INTEGER DEFAULT NULL,
+    text CLOB NOT NULL,
+    created_at DATETIME NOT NULL,
+    updated_at DATETIME NOT NULL,
+    quote CLOB NOT NULL,
+    ranges CLOB NOT NULL,
+    CONSTRAINT FK_A7AED006A76ED395 FOREIGN KEY (user_id) REFERENCES wallabag_user (id),
+    CONSTRAINT FK_A7AED006BA364942 FOREIGN KEY (entry_id) REFERENCES wallabag_entry (id) ON DELETE CASCADE
+);
+CREATE INDEX IDX_A7AED006A76ED395 ON wallabag_annotation (user_id);
+CREATE INDEX IDX_A7AED006BA364942 ON wallabag_annotation (entry_id);
+EOD
+                );
+
+                $this->addSql(<<<EOD
+INSERT INTO ${tableName} (id, user_id, entry_id, text, created_at, updated_at, quote, ranges) 
+SELECT id, user_id, entry_id, text, created_at, updated_at, quote, ranges 
+FROM __temp__wallabag_annotation;
+EOD
+                );
+                $this->addSql('DROP TABLE __temp__wallabag_annotation');
+                break;
+
+            case 'mysql':
+                $this->addSql('ALTER TABLE '.$tableName.' MODIFY quote TEXT NOT NULL');
+                break;
+
+            case 'postgresql':
+                $this->addSql('ALTER TABLE '.$tableName.' ALTER COLUMN quote TYPE TEXT');
+                break;
+        }
+    }
+
+    public function down(Schema $schema)
+    {
+        $tableName = $this->getTable('annotation');
+
+        switch ($this->connection->getDatabasePlatform()->getName()) {
+            case 'sqlite':
+                throw new SkipMigrationException('Too complex ...');
+                break;
+
+            case 'mysql':
+                $this->addSql('ALTER TABLE '.$tableName.' MODIFY quote VARCHAR(255) NOT NULL');
+                break;
+
+            case 'postgresql':
+                $this->addSql('ALTER TABLE '.$tableName.' ALTER COLUMN quote TYPE VARCHAR(255)');
+                break;
+        }
+    }
+}

app/Resources/static/themes/_global/index.js

@@ -34,7 +34,21 @@ $(document).ready(() => {
     app.registry.registerUtility(authorization, 'authorizationPolicy');
 
     const x = JSON.parse($('#annotationroutes').html());
-    app.include(annotator.storage.http, x);
+    app.include(annotator.storage.http, $.extend({}, x, {
+      onError(msg, xhr) {
+        if (!Object.prototype.hasOwnProperty.call(xhr, 'responseJSON')) {
+          annotator.notification.banner('An error occurred', 'error');
+          return;
+        }
+        $.each(xhr.responseJSON.children, (k, v) => {
+          if (v.errors) {
+            $.each(v.errors, (n, errorText) => {
+              annotator.notification.banner(errorText, 'error');
+            });
+          }
+        });
+      },
+    }));
 
     app.start().then(() => {
       app.annotations.load({ entry: x.entryId });

src/Wallabag/AnnotationBundle/Controller/WallabagAnnotationController.php

@@ -7,6 +7,8 @@ use Symfony\Component\HttpFoundation\JsonResponse;
 use Symfony\Component\HttpFoundation\Request;
 use Sensio\Bundle\FrameworkExtraBundle\Configuration\ParamConverter;
 use Wallabag\AnnotationBundle\Entity\Annotation;
+use Wallabag\AnnotationBundle\Form\EditAnnotationType;
+use Wallabag\AnnotationBundle\Form\NewAnnotationType;
 use Wallabag\CoreBundle\Entity\Entry;
 
 class WallabagAnnotationController extends FOSRestController
@@ -49,25 +51,25 @@ class WallabagAnnotationController extends FOSRestController
         $data = json_decode($request->getContent(), true);
 
         $em = $this->getDoctrine()->getManager();
-
         $annotation = new Annotation($this->getUser());
-
-        $annotation->setText($data['text']);
-        if (array_key_exists('quote', $data)) {
-            $annotation->setQuote($data['quote']);
-        }
-        if (array_key_exists('ranges', $data)) {
-            $annotation->setRanges($data['ranges']);
-        }
-
         $annotation->setEntry($entry);
 
-        $em->persist($annotation);
+        $form = $this->get('form.factory')->createNamed('', NewAnnotationType::class, $annotation, [
-        $em->flush();
+            'csrf_protection' => false,
+            'allow_extra_fields' => true,
+        ]);
+        $form->submit($data);
 
-        $json = $this->get('serializer')->serialize($annotation, 'json');
+        if ($form->isValid()) {
+            $em->persist($annotation);
+            $em->flush();
 
-        return (new JsonResponse())->setJson($json);
+            $json = $this->get('serializer')->serialize($annotation, 'json');
+
+            return JsonResponse::fromJsonString($json);
+        }
+
+        return $form;
     }
 
     /**
@@ -86,16 +88,23 @@ class WallabagAnnotationController extends FOSRestController
     {
         $data = json_decode($request->getContent(), true);
 
-        if (!is_null($data['text'])) {
+        $form = $this->get('form.factory')->createNamed('', EditAnnotationType::class, $annotation, [
-            $annotation->setText($data['text']);
+            'csrf_protection' => false,
+            'allow_extra_fields' => true,
+        ]);
+        $form->submit($data);
+
+        if ($form->isValid()) {
+            $em = $this->getDoctrine()->getManager();
+            $em->persist($annotation);
+            $em->flush();
+
+            $json = $this->get('serializer')->serialize($annotation, 'json');
+
+            return JsonResponse::fromJsonString($json);
         }
 
-        $em = $this->getDoctrine()->getManager();
+        return $form;
-        $em->flush();
-
-        $json = $this->get('serializer')->serialize($annotation, 'json');
-
-        return (new JsonResponse())->setJson($json);
     }
 
     /**

src/Wallabag/AnnotationBundle/Entity/Annotation.php

@@ -8,6 +8,7 @@ use JMS\Serializer\Annotation\Exclude;
 use JMS\Serializer\Annotation\VirtualProperty;
 use JMS\Serializer\Annotation\SerializedName;
 use JMS\Serializer\Annotation\Groups;
+use Symfony\Component\Validator\Constraints as Assert;
 use Wallabag\UserBundle\Entity\User;
 use Wallabag\CoreBundle\Entity\Entry;
 
@@ -56,7 +57,11 @@ class Annotation
     /**
      * @var string
      *
-     * @ORM\Column(name="quote", type="string")
+     * @Assert\Length(
+     *     max = 10000,
+     *     maxMessage = "validator.quote_length_too_high"
+     * )
+     * @ORM\Column(name="quote", type="text")
      *
      * @Groups({"entries_for_user", "export_all"})
      */

src/Wallabag/AnnotationBundle/Form/EditAnnotationType.php

@@ -0,0 +1,18 @@
+<?php
+
+namespace Wallabag\AnnotationBundle\Form;
+
+use Symfony\Component\Form\AbstractType;
+use Symfony\Component\Form\FormBuilderInterface;
+
+class EditAnnotationType extends AbstractType
+{
+    public function buildForm(FormBuilderInterface $builder, array $options)
+    {
+        $builder
+            ->add('text', null, [
+                'empty_data' => '',
+            ])
+        ;
+    }
+}

src/Wallabag/AnnotationBundle/Form/NewAnnotationType.php

@@ -0,0 +1,35 @@
+<?php
+
+namespace Wallabag\AnnotationBundle\Form;
+
+use Symfony\Component\Form\AbstractType;
+use Symfony\Component\Form\Extension\Core\Type\CollectionType;
+use Symfony\Component\Form\FormBuilderInterface;
+use Symfony\Component\OptionsResolver\OptionsResolver;
+use Wallabag\AnnotationBundle\Entity\Annotation;
+
+class NewAnnotationType extends AbstractType
+{
+    public function buildForm(FormBuilderInterface $builder, array $options)
+    {
+        $builder
+            ->add('text', null, [
+                'empty_data' => '',
+            ])
+            ->add('quote', null, [
+                'empty_data' => null,
+            ])
+            ->add('ranges', CollectionType::class, [
+                'entry_type' => RangeType::class,
+                'allow_add' => true,
+            ])
+        ;
+    }
+
+    public function configureOptions(OptionsResolver $resolver)
+    {
+        $resolver->setDefaults([
+            'data_class' => Annotation::class,
+        ]);
+    }
+}

src/Wallabag/AnnotationBundle/Form/RangeType.php

@@ -0,0 +1,19 @@
+<?php
+
+namespace Wallabag\AnnotationBundle\Form;
+
+use Symfony\Component\Form\AbstractType;
+use Symfony\Component\Form\FormBuilderInterface;
+
+class RangeType extends AbstractType
+{
+    public function buildForm(FormBuilderInterface $builder, array $options)
+    {
+        $builder
+            ->add('start')
+            ->add('startOffset')
+            ->add('end')
+            ->add('endOffset')
+        ;
+    }
+}

src/Wallabag/CoreBundle/Resources/translations/validators.da.yml

@@ -4,3 +4,4 @@ validator:
     # password_wrong_value: 'Wrong value for your current password'
     # item_per_page_too_high: 'This will certainly kill the app'
     # rss_limit_too_high: 'This will certainly kill the app'
+    # quote_length_too_high: 'The quote is too long. It should have {{ limit }} characters or less.'

src/Wallabag/CoreBundle/Resources/translations/validators.de.yml

@@ -4,3 +4,4 @@ validator:
     password_wrong_value: 'Falscher Wert für dein aktuelles Kennwort'
     item_per_page_too_high: 'Dies wird die Anwendung möglicherweise beenden'
     rss_limit_too_high: 'Dies wird die Anwendung möglicherweise beenden'
+    # quote_length_too_high: 'The quote is too long. It should have {{ limit }} characters or less.'

src/Wallabag/CoreBundle/Resources/translations/validators.en.yml

@@ -4,3 +4,4 @@ validator:
     password_wrong_value: 'Wrong value for your current password'
     item_per_page_too_high: 'This will certainly kill the app'
     rss_limit_too_high: 'This will certainly kill the app'
+    quote_length_too_high: 'The quote is too long. It should have {{ limit }} characters or less.'

src/Wallabag/CoreBundle/Resources/translations/validators.es.yml

@@ -4,3 +4,4 @@ validator:
     password_wrong_value: 'Entrada equivocada para su contraseña actual'
     item_per_page_too_high: 'Esto matará la aplicación'
     rss_limit_too_high: 'Esto matará la aplicación'
+    # quote_length_too_high: 'The quote is too long. It should have {{ limit }} characters or less.'

src/Wallabag/CoreBundle/Resources/translations/validators.fa.yml

@@ -4,3 +4,4 @@ validator:
     password_wrong_value: 'رمز فعلی را اشتباه وارد کرده‌اید'
     item_per_page_too_high: 'با این تعداد برنامه به فنا می‌رود'
     rss_limit_too_high: 'با این تعداد برنامه به فنا می‌رود'
+    # quote_length_too_high: 'The quote is too long. It should have {{ limit }} characters or less.'

src/Wallabag/CoreBundle/Resources/translations/validators.fr.yml

@@ -4,3 +4,4 @@ validator:
     password_wrong_value: "Votre mot de passe actuel est faux"
     item_per_page_too_high: "Ça ne va pas plaire à l’application"
     rss_limit_too_high: "Ça ne va pas plaire à l’application"
+    quote_length_too_high: "La citation est trop longue. Elle doit avoir au maximum {{ limit }} caractères."

src/Wallabag/CoreBundle/Resources/translations/validators.it.yml

@@ -4,3 +4,4 @@ validator:
     password_wrong_value: 'Valore inserito per la password corrente errato'
     item_per_page_too_high: 'Questo valore è troppo alto'
     rss_limit_too_high: 'Questo valore è troppo alto'
+    # quote_length_too_high: 'The quote is too long. It should have {{ limit }} characters or less.'

src/Wallabag/CoreBundle/Resources/translations/validators.oc.yml

@@ -4,3 +4,4 @@ validator:
     password_wrong_value: 'Vòstre senhal actual es pas bon'
     item_per_page_too_high: "Aquò li agradarà pas a l'aplicacion"
     rss_limit_too_high: "Aquò li agradarà pas a l'aplicacion"
+    # quote_length_too_high: 'The quote is too long. It should have {{ limit }} characters or less.'

src/Wallabag/CoreBundle/Resources/translations/validators.pl.yml

@@ -4,3 +4,4 @@ validator:
     password_wrong_value: 'Twoje obecne hasło jest błędne'
     item_per_page_too_high: 'To może spowodować problemy z aplikacją'
     rss_limit_too_high: 'To może spowodować problemy z aplikacją'
+    # quote_length_too_high: 'The quote is too long. It should have {{ limit }} characters or less.'

src/Wallabag/CoreBundle/Resources/translations/validators.pt.yml

@@ -4,3 +4,4 @@ validator:
     password_wrong_value: 'A senha atual informada está errada'
     item_per_page_too_high: 'Certamente isso pode matar a aplicação'
     rss_limit_too_high: 'Certamente isso pode matar a aplicação'
+    # quote_length_too_high: 'The quote is too long. It should have {{ limit }} characters or less.'

src/Wallabag/CoreBundle/Resources/translations/validators.ro.yml

@@ -4,3 +4,4 @@ validator:
     # password_wrong_value: 'Wrong value for your current password'
     # item_per_page_too_high: 'This will certainly kill the app'
     # rss_limit_too_high: 'This will certainly kill the app'
+    # quote_length_too_high: 'The quote is too long. It should have {{ limit }} characters or less.'

src/Wallabag/CoreBundle/Resources/translations/validators.tr.yml

@@ -4,3 +4,4 @@ validator:
     # password_wrong_value: 'Wrong value for your current password'
     # item_per_page_too_high: 'This will certainly kill the app'
     # rss_limit_too_high: 'This will certainly kill the app'
+    # quote_length_too_high: 'The quote is too long. It should have {{ limit }} characters or less.'

tests/Wallabag/AnnotationBundle/Controller/AnnotationControllerTest.php

@@ -84,7 +84,9 @@ class AnnotationControllerTest extends WallabagAnnotationTestCase
         $content = json_encode([
             'text' => 'my annotation',
             'quote' => 'my quote',
-            'ranges' => ['start' => '', 'startOffset' => 24, 'end' => '', 'endOffset' => 31],
+            'ranges' => [
+                ['start' => '', 'startOffset' => 24, 'end' => '', 'endOffset' => 31]
+            ],
         ]);
         $this->client->request('POST', $prefixUrl.'/'.$entry->getId().'.json', [], [], $headers, $content);
 
@@ -106,6 +108,36 @@ class AnnotationControllerTest extends WallabagAnnotationTestCase
         $this->assertEquals('my annotation', $annotation->getText());
     }
 
+    /**
+     * @dataProvider dataForEachAnnotations
+     */
+    public function testSetAnnotationWithQuoteTooLong($prefixUrl)
+    {
+        $em = $this->client->getContainer()->get('doctrine.orm.entity_manager');
+
+        if ('annotations' === $prefixUrl) {
+            $this->logInAs('admin');
+        }
+
+        /** @var Entry $entry */
+        $entry = $em
+            ->getRepository('WallabagCoreBundle:Entry')
+            ->findOneByUsernameAndNotArchived('admin');
+
+        $longQuote = str_repeat('a', 10001);
+        $headers = ['CONTENT_TYPE' => 'application/json'];
+        $content = json_encode([
+            'text' => 'my annotation',
+            'quote' => $longQuote,
+            'ranges' => [
+                ['start' => '', 'startOffset' => 24, 'end' => '', 'endOffset' => 31]
+            ],
+        ]);
+        $this->client->request('POST', $prefixUrl.'/'.$entry->getId().'.json', [], [], $headers, $content);
+
+        $this->assertEquals(400, $this->client->getResponse()->getStatusCode());
+    }
+
     /**
      * Test editing an existing annotation.
      *