]> git.immae.eu Git - github/wallabag/wallabag.git/commitdiff
Merge pull request #3161 from wallabag/scrutinizer-2.3
authorNicolas Lœuillet <nicolas@loeuillet.org>
Wed, 31 May 2017 09:46:41 +0000 (11:46 +0200)
committerGitHub <noreply@github.com>
Wed, 31 May 2017 09:46:41 +0000 (11:46 +0200)
Fix some Scrutinizer issues

src/Wallabag/ApiBundle/Controller/UserRestController.php [new file with mode: 0644]
src/Wallabag/ApiBundle/Resources/config/routing_rest.yml
src/Wallabag/ImportBundle/Import/WallabagV2Import.php
src/Wallabag/UserBundle/Controller/ManageController.php
src/Wallabag/UserBundle/Entity/User.php
tests/Wallabag/ApiBundle/Controller/UserRestControllerTest.php [new file with mode: 0644]
tests/Wallabag/ApiBundle/WallabagApiTestCase.php
tests/Wallabag/ImportBundle/Controller/WallabagV2ControllerTest.php
tests/Wallabag/ImportBundle/fixtures/wallabag-v2.json
tests/Wallabag/UserBundle/Controller/ManageControllerTest.php

diff --git a/src/Wallabag/ApiBundle/Controller/UserRestController.php b/src/Wallabag/ApiBundle/Controller/UserRestController.php
new file mode 100644 (file)
index 0000000..a1b78e3
--- /dev/null
@@ -0,0 +1,139 @@
+<?php
+
+namespace Wallabag\ApiBundle\Controller;
+
+use FOS\UserBundle\Event\UserEvent;
+use FOS\UserBundle\FOSUserEvents;
+use JMS\Serializer\SerializationContext;
+use Nelmio\ApiDocBundle\Annotation\ApiDoc;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\JsonResponse;
+use Wallabag\UserBundle\Entity\User;
+
+class UserRestController extends WallabagRestController
+{
+    /**
+     * Retrieve current logged in user informations.
+     *
+     * @ApiDoc()
+     *
+     * @return JsonResponse
+     */
+    public function getUserAction()
+    {
+        $this->validateAuthentication();
+
+        return $this->sendUser($this->getUser());
+    }
+
+    /**
+     * Register an user.
+     *
+     * @ApiDoc(
+     *      requirements={
+     *          {"name"="username", "dataType"="string", "required"=true, "description"="The user's username"},
+     *          {"name"="password", "dataType"="string", "required"=true, "description"="The user's password"},
+     *          {"name"="email", "dataType"="string", "required"=true, "description"="The user's email"}
+     *      }
+     * )
+     *
+     * @todo Make this method (or the whole API) accessible only through https
+     *
+     * @return JsonResponse
+     */
+    public function putUserAction(Request $request)
+    {
+        if (!$this->container->getParameter('fosuser_registration')) {
+            $json = $this->get('serializer')->serialize(['error' => "Server doesn't allow registrations"], 'json');
+
+            return (new JsonResponse())->setJson($json)->setStatusCode(403);
+        }
+
+        $userManager = $this->get('fos_user.user_manager');
+        $user = $userManager->createUser();
+        // enable created user by default
+        $user->setEnabled(true);
+
+        $form = $this->createForm('Wallabag\UserBundle\Form\NewUserType', $user, [
+            'csrf_protection' => false,
+        ]);
+
+        // simulate form submission
+        $form->submit([
+            'username' => $request->request->get('username'),
+            'plainPassword' => [
+                'first' => $request->request->get('password'),
+                'second' => $request->request->get('password'),
+            ],
+            'email' => $request->request->get('email'),
+        ]);
+
+        if ($form->isSubmitted() && false === $form->isValid()) {
+            $view = $this->view($form, 400);
+            $view->setFormat('json');
+
+            // handle errors in a more beautiful way than the default view
+            $data = json_decode($this->handleView($view)->getContent(), true)['children'];
+            $errors = [];
+
+            if (isset($data['username']['errors'])) {
+                $errors['username'] = $this->translateErrors($data['username']['errors']);
+            }
+
+            if (isset($data['email']['errors'])) {
+                $errors['email'] = $this->translateErrors($data['email']['errors']);
+            }
+
+            if (isset($data['plainPassword']['children']['first']['errors'])) {
+                $errors['password'] = $this->translateErrors($data['plainPassword']['children']['first']['errors']);
+            }
+
+            $json = $this->get('serializer')->serialize(['error' => $errors], 'json');
+
+            return (new JsonResponse())->setJson($json)->setStatusCode(400);
+        }
+
+        $userManager->updateUser($user);
+
+        // dispatch a created event so the associated config will be created
+        $event = new UserEvent($user, $request);
+        $this->get('event_dispatcher')->dispatch(FOSUserEvents::USER_CREATED, $event);
+
+        return $this->sendUser($user);
+    }
+
+    /**
+     * Send user response.
+     *
+     * @param User $user
+     *
+     * @return JsonResponse
+     */
+    private function sendUser(User $user)
+    {
+        $json = $this->get('serializer')->serialize(
+            $user,
+            'json',
+            SerializationContext::create()->setGroups(['user_api'])
+        );
+
+        return (new JsonResponse())->setJson($json);
+    }
+
+    /**
+     * Translate errors message.
+     *
+     * @param array $errors
+     *
+     * @return array
+     */
+    private function translateErrors($errors)
+    {
+        $translatedErrors = [];
+        foreach ($errors as $error) {
+            $translatedErrors[] = $this->get('translator')->trans($error);
+        }
+
+        return $translatedErrors;
+    }
+}
index 57d37f4b454fa3aa15d3f4d6a36f194fd428d2ee..c0283e71f159603317b1179bbf194c21514c12fa 100644 (file)
@@ -17,3 +17,8 @@ misc:
   type: rest
   resource: "WallabagApiBundle:WallabagRest"
   name_prefix:  api_
+
+user:
+  type: rest
+  resource: "WallabagApiBundle:UserRest"
+  name_prefix:  api_
index d2a89d79aa77dd2e54abe2ddd479b5880fa36b75..3e085ecff6461b8a36fdc6ebd34225e5835c0623 100644 (file)
@@ -36,8 +36,8 @@ class WallabagV2Import extends WallabagImport
         return [
             'html' => $entry['content'],
             'content_type' => $entry['mimetype'],
-            'is_archived' => (int) ($entry['is_archived'] || $this->markAsRead),
-            'is_starred' => false,
+            'is_archived' => (bool) ($entry['is_archived'] || $this->markAsRead),
+            'is_starred' => (bool) $entry['is_starred'],
         ] + $entry;
     }
 
index 1c5c86d45501f41c987edba0efa48bf320f3fa93..084f2c67e11ec176a3144cb1fff8ceec622e6139 100644 (file)
@@ -33,9 +33,7 @@ class ManageController extends Controller
         // enable created user by default
         $user->setEnabled(true);
 
-        $form = $this->createForm('Wallabag\UserBundle\Form\NewUserType', $user, [
-            'validation_groups' => ['Profile'],
-        ]);
+        $form = $this->createForm('Wallabag\UserBundle\Form\NewUserType', $user);
         $form->handleRequest($request);
 
         if ($form->isSubmitted() && $form->isValid()) {
index 3a167de740608567ae03b6e42f88ab8d7d512bf6..1ff3046a8c798a226e5964d8ebdde28b170a3040 100644 (file)
@@ -4,11 +4,11 @@ namespace Wallabag\UserBundle\Entity;
 
 use Doctrine\Common\Collections\ArrayCollection;
 use Doctrine\ORM\Mapping as ORM;
+use JMS\Serializer\Annotation\Groups;
+use JMS\Serializer\Annotation\XmlRoot;
 use Scheb\TwoFactorBundle\Model\Email\TwoFactorInterface;
 use Scheb\TwoFactorBundle\Model\TrustedComputerInterface;
 use FOS\UserBundle\Model\User as BaseUser;
-use JMS\Serializer\Annotation\ExclusionPolicy;
-use JMS\Serializer\Annotation\Expose;
 use Symfony\Bridge\Doctrine\Validator\Constraints\UniqueEntity;
 use Symfony\Component\Security\Core\User\UserInterface;
 use Wallabag\ApiBundle\Entity\Client;
@@ -18,23 +18,25 @@ use Wallabag\CoreBundle\Entity\Entry;
 /**
  * User.
  *
+ * @XmlRoot("user")
  * @ORM\Entity(repositoryClass="Wallabag\UserBundle\Repository\UserRepository")
  * @ORM\Table(name="`user`")
  * @ORM\HasLifecycleCallbacks()
- * @ExclusionPolicy("all")
  *
  * @UniqueEntity("email")
  * @UniqueEntity("username")
  */
 class User extends BaseUser implements TwoFactorInterface, TrustedComputerInterface
 {
+    /** @Serializer\XmlAttribute */
     /**
      * @var int
      *
-     * @Expose
      * @ORM\Column(name="id", type="integer")
      * @ORM\Id
      * @ORM\GeneratedValue(strategy="AUTO")
+     *
+     * @Groups({"user_api"})
      */
     protected $id;
 
@@ -42,13 +44,31 @@ class User extends BaseUser implements TwoFactorInterface, TrustedComputerInterf
      * @var string
      *
      * @ORM\Column(name="name", type="text", nullable=true)
+     *
+     * @Groups({"user_api"})
      */
     protected $name;
 
+    /**
+     * @var string
+     *
+     * @Groups({"user_api"})
+     */
+    protected $username;
+
+    /**
+     * @var string
+     *
+     * @Groups({"user_api"})
+     */
+    protected $email;
+
     /**
      * @var date
      *
      * @ORM\Column(name="created_at", type="datetime")
+     *
+     * @Groups({"user_api"})
      */
     protected $createdAt;
 
@@ -56,6 +76,8 @@ class User extends BaseUser implements TwoFactorInterface, TrustedComputerInterf
      * @var date
      *
      * @ORM\Column(name="updated_at", type="datetime")
+     *
+     * @Groups({"user_api"})
      */
     protected $updatedAt;
 
diff --git a/tests/Wallabag/ApiBundle/Controller/UserRestControllerTest.php b/tests/Wallabag/ApiBundle/Controller/UserRestControllerTest.php
new file mode 100644 (file)
index 0000000..3f4969a
--- /dev/null
@@ -0,0 +1,110 @@
+<?php
+
+namespace Tests\Wallabag\ApiBundle\Controller;
+
+use Tests\Wallabag\ApiBundle\WallabagApiTestCase;
+
+class UserRestControllerTest extends WallabagApiTestCase
+{
+    public function testGetUser()
+    {
+        $this->client->request('GET', '/api/user.json');
+        $this->assertEquals(200, $this->client->getResponse()->getStatusCode());
+
+        $content = json_decode($this->client->getResponse()->getContent(), true);
+
+        $this->assertArrayHasKey('id', $content);
+        $this->assertArrayHasKey('email', $content);
+        $this->assertArrayHasKey('name', $content);
+        $this->assertArrayHasKey('username', $content);
+        $this->assertArrayHasKey('created_at', $content);
+        $this->assertArrayHasKey('updated_at', $content);
+
+        $this->assertEquals('bigboss@wallabag.org', $content['email']);
+        $this->assertEquals('Big boss', $content['name']);
+        $this->assertEquals('admin', $content['username']);
+
+        $this->assertEquals('application/json', $this->client->getResponse()->headers->get('Content-Type'));
+    }
+
+    public function testCreateNewUser()
+    {
+        $this->client->request('PUT', '/api/user.json', [
+            'username' => 'google',
+            'password' => 'googlegoogle',
+            'email' => 'wallabag@google.com',
+        ]);
+
+        $this->assertEquals(200, $this->client->getResponse()->getStatusCode());
+
+        $content = json_decode($this->client->getResponse()->getContent(), true);
+
+        $this->assertArrayHasKey('id', $content);
+        $this->assertArrayHasKey('email', $content);
+        $this->assertArrayHasKey('username', $content);
+        $this->assertArrayHasKey('created_at', $content);
+        $this->assertArrayHasKey('updated_at', $content);
+
+        $this->assertEquals('wallabag@google.com', $content['email']);
+        $this->assertEquals('google', $content['username']);
+
+        $this->assertEquals('application/json', $this->client->getResponse()->headers->get('Content-Type'));
+
+        // remove the created user to avoid side effect on other tests
+        // @todo remove these lines when test will be isolated
+        $em = $this->client->getContainer()->get('doctrine.orm.entity_manager');
+
+        $query = $em->createQuery('DELETE FROM Wallabag\CoreBundle\Entity\Config c WHERE c.user = :user_id');
+        $query->setParameter('user_id', $content['id']);
+        $query->execute();
+
+        $query = $em->createQuery('DELETE FROM Wallabag\UserBundle\Entity\User u WHERE u.id = :id');
+        $query->setParameter('id', $content['id']);
+        $query->execute();
+    }
+
+    public function testCreateNewUserWithExistingEmail()
+    {
+        $this->client->request('PUT', '/api/user.json', [
+            'username' => 'admin',
+            'password' => 'googlegoogle',
+            'email' => 'bigboss@wallabag.org',
+        ]);
+
+        $this->assertEquals(400, $this->client->getResponse()->getStatusCode());
+
+        $content = json_decode($this->client->getResponse()->getContent(), true);
+
+        $this->assertArrayHasKey('error', $content);
+        $this->assertArrayHasKey('username', $content['error']);
+        $this->assertArrayHasKey('email', $content['error']);
+
+        // $this->assertEquals('fos_user.username.already_used', $content['error']['username'][0]);
+        // $this->assertEquals('fos_user.email.already_used', $content['error']['email'][0]);
+        // This shouldn't be translated ...
+        $this->assertEquals('This value is already used.', $content['error']['username'][0]);
+        $this->assertEquals('This value is already used.', $content['error']['email'][0]);
+
+        $this->assertEquals('application/json', $this->client->getResponse()->headers->get('Content-Type'));
+    }
+
+    public function testCreateNewUserWithTooShortPassword()
+    {
+        $this->client->request('PUT', '/api/user.json', [
+            'username' => 'facebook',
+            'password' => 'face',
+            'email' => 'facebook@wallabag.org',
+        ]);
+
+        $this->assertEquals(400, $this->client->getResponse()->getStatusCode());
+
+        $content = json_decode($this->client->getResponse()->getContent(), true);
+
+        $this->assertArrayHasKey('error', $content);
+        $this->assertArrayHasKey('password', $content['error']);
+
+        $this->assertEquals('validator.password_too_short', $content['error']['password'][0]);
+
+        $this->assertEquals('application/json', $this->client->getResponse()->headers->get('Content-Type'));
+    }
+}
index cf9b33471cb2d13fa36251378e4d9ce109ea7ca9..a67655c8620abcb5266eaabe4a8119c0e2b4607c 100644 (file)
@@ -37,7 +37,7 @@ abstract class WallabagApiTestCase extends WebTestCase
         $firewallName = $container->getParameter('fos_user.firewall_name');
 
         $this->user = $userManager->findUserBy(['username' => 'admin']);
-        $loginManager->loginUser($firewallName, $this->user);
+        $loginManager->logInUser($firewallName, $this->user);
 
         // save the login token into the session and put it in a cookie
         $container->get('session')->set('_security_'.$firewallName, serialize($container->get('security.token_storage')->getToken()));
index 556ab1bdcfa3490802f10b900ad65139e10cdfc6..335115fecfc0ead6595e87d5c132d330bd280185 100644 (file)
@@ -138,6 +138,7 @@ class WallabagV2ControllerTest extends WallabagCoreTestCase
         $this->assertEquals(3, count($content->getTags()));
         $this->assertInstanceOf(\DateTime::class, $content->getCreatedAt());
         $this->assertEquals('2016-09-08', $content->getCreatedAt()->format('Y-m-d'));
+        $this->assertTrue($content->isStarred(), 'Entry is starred');
     }
 
     public function testImportWallabagWithEmptyFile()
index efa8faf26cb4ffbb9b06ee09a771a7f903e2037d..1bdff7f5c191d7272186c615eb2085b75efd9626 100644 (file)
@@ -6,7 +6,7 @@
         "is_archived": false,
         "created_at": "2016-09-08T11:55:58+0200",
         "updated_at": "2016-09-08T11:57:16+0200",
-        "is_starred": false,
+        "is_starred": true,
         "content": "<div alt=\"li\">Édition <a href=\"https://blogs.mediapart.fr/edition/camedia-0\">CAMédia</a>\n<h3 class=\"title\"><a href=\"https://blogs.mediapart.fr/edition/camedia/article/180116/deux-nouvelles-editions-pour-debattre-dans-le-club-sur-la-laicite-et-sur-la-democratie\">Deux nouvelles éditions pour débattre dans le club sur la laïcité et sur la démocratie</a></h3>\n<p>18 janv. 2016 | Par </p>\n<p>CAMédia après un échange sur « l'éthique du débat » a lancé deux discussions , l'une sur le thème de la laïcité, l'autre ( encore en cours) sur celui de la démocratie. Nous sommes heureux de pouvoir signaler la création de deux nouvelles éditions participatives sur ces thèmes. Nous vous invitons à les lire et à participer à leurs débats.</p>\n</div><div alt=\"li\">\n<h3 class=\"title\"><a href=\"https://blogs.mediapart.fr/lucile-longre/blog/170116/de-limportance-de-rever-eloge-du-merveilleux\">De l'importance de rêver, éloge du merveilleux</a></h3>\n<p>17 janv. 2016 | Par </p>\n<p>Je parlerai ici des rêves comme moteur de vie, de ces rêves qui vous rattachent et vous font espérer à ce qu’il y a de plus humain dans l’homme, même au milieu de la plus noire des détresses.</p>\n</div><div alt=\"li\">\n<h3 class=\"title\"><a href=\"https://blogs.mediapart.fr/barbara-romagnan/blog/180116/fins-dune-toute-puissance\">Fin(s) d'une toute-puissance</a></h3>\n<p>18 janv. 2016 | Par </p>\n<p>En ce début d’année, je recommande la lecture du dernier ouvrage de Guillaume Duval, La France ne sera jamais plus une grande puissance ? Tant mieux !</p>\n</div><div alt=\"li\">\n<h3 class=\"title\"><a href=\"https://blogs.mediapart.fr/jean-pierre-thibaudat/blog/170116/l-allier-departement-de-destruction-massive-du-tissu-culturel\">L’Allier, département de destruction massive du tissu culturel</a></h3>\n<p>18 janv. 2016 | Par </p>\n<p>Les temps sont durs pour les petites structures, les associations culturelles qui, de bourgades en villages, travaillent au cœur des régions. Leurs subventions sont souvent revues à la baisse. Le département de l’Allier les a carrément supprimées. Pour favoriser « l’événementiel ».</p>\n</div><div alt=\"li\">Édition <a href=\"https://blogs.mediapart.fr/edition/les-invites-de-mediapart\">Les invités de Mediapart</a>\n<h3 class=\"title\"><a href=\"https://blogs.mediapart.fr/edition/les-invites-de-mediapart/article/180116/la-democratie-deja-attaquee-par-la-cooperation-reglementaire-transatlantiqu\">La démocratie déjà attaquée par la coopération réglementaire transatlantique</a></h3>\n<p>18 janv. 2016 | Par </p>\n<p>Lora Verheecke et David Lundy travaillent pour Corporate Europe Observatory, une ONG basée à Bruxelles qui enquête sur le pouvoir des lobbies des grandes entreprises sur la politique de l’Union européenne. Ils révèlent que depuis 25 ans le projet de « coopération réglementaire » mené par l’Union européenne et les États-Unis a été dominé par les grandes entreprises. ET que le TTIP cherche à entériner ce projet.</p>\n</div><div alt=\"li\">\n<h3 class=\"title\"><a href=\"https://blogs.mediapart.fr/jacqueline-derens/blog/180116/2016-une-annee-test-pour-jacob-zuma-et-son-gouvernement\">2016, une année test pour Jacob Zuma et son gouvernement</a></h3>\n<p>18 janv. 2016 | Par </p>\n<p>Les turbulences de l’an passé ont toutes les chances de continuer à troubler le climat politique et social de l’Afrique du Sud en 2016. La situation exige des changements profonds dans la conduite des affaires du pays. Jacob Zuma tout en admettant la nécessité de ces changements, est-il l’homme de la situation ? Son gouvernement répondra-t-il aux attentes des citoyens sud-africains ?</p>\n</div><div alt=\"li\">\n<h3 class=\"title\"><a href=\"https://blogs.mediapart.fr/marie-cosnay/blog/140116/un-mal-fou-janvier-2016\">Un mal fou (janvier 2016)</a></h3>\n<p>14 janv. 2016 | Par </p>\n<p>J’ai une fringale d’aventure, d’aventures à venir. J’ai la fringale de la fringale des aventures et soudain, rupture. Je n’y arrive plus, tout est bloqué, tout empêché. Faut dire que depuis un an environ, tout est devenu plus compliqué. Ecrire va de moins en moins de soi.</p>\n</div><div alt=\"li\">\n<h3 class=\"title\"><a href=\"https://blogs.mediapart.fr/jean-pierre-veran/blog/170116/redoublement-le-changement-bas-bruit\">Redoublement : le changement à bas bruit ?</a></h3>\n<p>17 janv. 2016 | Par </p>\n<p>S’il est une caractéristique de la forme scolaire française bien établie dans la culture des personnels, des élèves et des parents, c’est bien le redoublement, censé sanctionner des résultats insuffisants pour envisager le passage dans la classe supérieure. Or, en ce domaine, l’évolution est nette.</p>\n</div><div alt=\"li\">\n<h3 class=\"title\"><a href=\"https://blogs.mediapart.fr/michel-de-pracontal/blog/160116/samedi-sciences-196-des-chasseurs-de-mammouths-en-arctique-il-y-45-000-ans\">Samedi-sciences (196): des chasseurs de mammouths en Arctique il y a 45 000 ans</a></h3>\n<p>16 janv. 2016 | Par <a href=\"https://blogs.mediapart.fr/michel-de-pracontal\" class=\"journalist\">Michel de Pracontal</a></p>\n<p>Les restes d’un mammouth retrouvés en Arctique sibérien, datés de 45 000 ans, portent les traces de blessures infligées par des chasseurs humains. Les scientifiques pensaient jusqu’ici que notre espèce ne s’était pas aventurée dans cette région glaciale il y a plus de 30 000 ou 35 0000 ans. En réalité, des hommes ont réussi à survivre en Arctique au moins 10 000 ans plus tôt que l’on croyait.</p>\n</div><div alt=\"li\">\n<h3 class=\"title\"><a href=\"https://blogs.mediapart.fr/alain-zolty/blog/140116/de-la-democratie-du-citoyen-et-de-lethique\">De la démocratie, du citoyen et de l'éthique</a></h3>\n<p>14 janv. 2016 | Par </p>\n<p>Trois ouvrages sont parus au Seuil, qui font état de la nécessité d’intégrer le citoyen dans la gouvernance de la nation. Non pas à titre consultatif mais doté d’un pouvoir délibératif pour constituer une contre-force face aux clans politico-financiers qui dominent la vie publique.</p>\n</div>",
         "mimetype": "text/html",
         "language": "fr",
index 44b9a0301c51e11b4ba255c6b0c13a266f6f0af8..b46256a6eea17b76be1f984e60906ab6effe0352 100644 (file)
@@ -30,8 +30,8 @@ class ManageControllerTest extends WallabagCoreTestCase
         $form = $crawler->selectButton('user.form.save')->form(array(
             'new_user[username]' => 'test_user',
             'new_user[email]' => 'test@test.io',
-            'new_user[plainPassword][first]' => 'test',
-            'new_user[plainPassword][second]' => 'test',
+            'new_user[plainPassword][first]' => 'testtest',
+            'new_user[plainPassword][second]' => 'testtest',
         ));
 
         $client->submit($form);