Remove user reference in tag

Fix #1543
This commit is contained in:
Jeremy Benoist 2015-12-29 14:50:52 +01:00
parent c997cfcc9c
commit fc73222723
13 changed files with 123 additions and 99 deletions

View file

@ -27,7 +27,7 @@ class WallabagRestController extends FOSRestController
->findOneByLabel($label);
if (is_null($tagEntity)) {
$tagEntity = new Tag($this->getUser());
$tagEntity = new Tag();
$tagEntity->setLabel($label);
}
@ -74,8 +74,7 @@ class WallabagRestController extends FOSRestController
$perPage = (int) $request->query->get('perPage', 30);
$tags = $request->query->get('tags', []);
$pager = $this
->getDoctrine()
$pager = $this->getDoctrine()
->getRepository('WallabagCoreBundle:Entry')
->findEntries($this->getUser()->getId(), $isArchived, $isStarred, $sort, $order);
@ -311,7 +310,12 @@ class WallabagRestController extends FOSRestController
public function getTagsAction()
{
$this->validateAuthentication();
$json = $this->get('serializer')->serialize($this->getUser()->getTags(), 'json');
$tags = $this->getDoctrine()
->getRepository('WallabagCoreBundle:Tag')
->findAllTags($this->getUser()->getId());
$json = $this->get('serializer')->serialize($tags, 'json');
return $this->renderJsonResponse($json);
}
@ -328,7 +332,10 @@ class WallabagRestController extends FOSRestController
public function deleteTagAction(Tag $tag)
{
$this->validateAuthentication();
$this->validateUserAccess($tag->getUser()->getId());
$this->getDoctrine()
->getRepository('WallabagCoreBundle:Entry')
->removeTag($this->getUser()->getId(), $tag);
$em = $this->getDoctrine()->getManager();
$em->remove($tag);

View file

@ -208,7 +208,7 @@ class WallabagRestControllerTest extends WallabagApiTestCase
$tags = array();
foreach ($entry->getTags() as $tag) {
$tags[] = array('id' => $tag->getId(), 'label' => $tag->getLabel());
$tags[] = array('id' => $tag->getId(), 'label' => $tag->getLabel(), 'slug' => $tag->getSlug());
}
$this->client->request('GET', '/api/entries/'.$entry->getId().'/tags');
@ -309,5 +309,6 @@ class WallabagRestControllerTest extends WallabagApiTestCase
$this->assertArrayHasKey('label', $content);
$this->assertEquals($tag['label'], $content['label']);
$this->assertEquals($tag['slug'], $content['slug']);
}
}

View file

@ -20,26 +20,24 @@ class TagController extends Controller
*/
public function addTagFormAction(Request $request, Entry $entry)
{
$tag = new Tag($this->getUser());
$tag = new Tag();
$form = $this->createForm(new NewTagType(), $tag);
$form->handleRequest($request);
if ($form->isValid()) {
$existingTag = $this->getDoctrine()
->getRepository('WallabagCoreBundle:Tag')
->findOneByLabelAndUserId($tag->getLabel(), $this->getUser()->getId());
->findOneByLabel($tag->getLabel());
$em = $this->getDoctrine()->getManager();
if (is_null($existingTag)) {
$entry->addTag($tag);
$em->persist($tag);
} else {
if (!$existingTag->hasEntry($entry)) {
} elseif (!$existingTag->hasEntry($entry)) {
$entry->addTag($existingTag);
$em->persist($existingTag);
}
}
$em->flush();

View file

@ -6,7 +6,6 @@ use Doctrine\Common\DataFixtures\AbstractFixture;
use Doctrine\Common\DataFixtures\OrderedFixtureInterface;
use Doctrine\Common\Persistence\ObjectManager;
use Wallabag\CoreBundle\Entity\Entry;
use Wallabag\CoreBundle\Entity\Tag;
class LoadEntryData extends AbstractFixture implements OrderedFixtureInterface
{
@ -50,13 +49,8 @@ class LoadEntryData extends AbstractFixture implements OrderedFixtureInterface
$entry3->setContent('This is my content /o/');
$entry3->setLanguage('en');
$tag1 = new Tag($this->getReference('bob-user'));
$tag1->setLabel('foo');
$tag2 = new Tag($this->getReference('bob-user'));
$tag2->setLabel('bar');
$entry3->addTag($tag1);
$entry3->addTag($tag2);
$entry3->addTag($this->getReference('foo-tag'));
$entry3->addTag($this->getReference('bar-tag'));
$manager->persist($entry3);
@ -71,13 +65,8 @@ class LoadEntryData extends AbstractFixture implements OrderedFixtureInterface
$entry4->setContent('This is my content /o/');
$entry4->setLanguage('en');
$tag1 = new Tag($this->getReference('admin-user'));
$tag1->setLabel('foo');
$tag2 = new Tag($this->getReference('admin-user'));
$tag2->setLabel('bar');
$entry4->addTag($tag1);
$entry4->addTag($tag2);
$entry4->addTag($this->getReference('foo-tag'));
$entry4->addTag($this->getReference('bar-tag'));
$manager->persist($entry4);

View file

@ -0,0 +1,41 @@
<?php
namespace Wallabag\CoreBundle\DataFixtures\ORM;
use Doctrine\Common\DataFixtures\AbstractFixture;
use Doctrine\Common\DataFixtures\OrderedFixtureInterface;
use Doctrine\Common\Persistence\ObjectManager;
use Wallabag\CoreBundle\Entity\Tag;
class LoadTagData extends AbstractFixture implements OrderedFixtureInterface
{
/**
* {@inheritdoc}
*/
public function load(ObjectManager $manager)
{
$tag1 = new Tag();
$tag1->setLabel('foo');
$manager->persist($tag1);
$this->addReference('foo-tag', $tag1);
$tag2 = new Tag();
$tag2->setLabel('bar');
$manager->persist($tag2);
$this->addReference('bar-tag', $tag2);
$manager->flush();
}
/**
* {@inheritdoc}
*/
public function getOrder()
{
return 25;
}
}

View file

@ -465,7 +465,7 @@ class Entry
// check if tag already exist but has not yet be persisted
// it seems that the previous condition with `contains()` doesn't check that case
foreach ($this->tags as $existingTag) {
if ($existingTag->getUser() !== $tag->getUser() || $existingTag->getLabel() === $tag->getLabel()) {
if ($existingTag->getLabel() === $tag->getLabel()) {
return;
}
}

View file

@ -38,6 +38,7 @@ class Tag
private $label;
/**
* @Expose
* @Gedmo\Slug(fields={"label"})
* @ORM\Column(length=128, unique=true)
*/
@ -48,14 +49,8 @@ class Tag
*/
private $entries;
/**
* @ORM\ManyToOne(targetEntity="Wallabag\UserBundle\Entity\User", inversedBy="tags")
*/
private $user;
public function __construct(\Wallabag\UserBundle\Entity\User $user)
public function __construct()
{
$this->user = $user;
$this->entries = new ArrayCollection();
}
@ -112,12 +107,4 @@ class Tag
{
return $this->entries->contains($entry);
}
/**
* @return User
*/
public function getUser()
{
return $this->user;
}
}

View file

@ -37,7 +37,7 @@ class RuleBasedTagger
}
foreach ($rule->getTags() as $label) {
$tag = $this->getTag($entry->getUser(), $label);
$tag = $this->getTag($label);
$entry->addTag($tag);
}
@ -62,7 +62,7 @@ class RuleBasedTagger
foreach ($entries as $entry) {
foreach ($rule->getTags() as $label) {
$tag = $this->getTag($user, $label);
$tag = $this->getTag($label);
$entry->addTag($tag);
}
@ -73,19 +73,18 @@ class RuleBasedTagger
}
/**
* Fetch a tag for a user.
* Fetch a tag.
*
* @param User $user
* @param string $label The tag's label.
*
* @return Tag
*/
private function getTag(User $user, $label)
private function getTag($label)
{
$tag = $this->tagRepository->findOneByLabelAndUserId($label, $user->getId());
$tag = $this->tagRepository->findOneByLabel($label);
if (!$tag) {
$tag = new Tag($user);
$tag = new Tag();
$tag->setLabel($label);
}

View file

@ -5,6 +5,7 @@ namespace Wallabag\CoreBundle\Repository;
use Doctrine\ORM\EntityRepository;
use Pagerfanta\Adapter\DoctrineORMAdapter;
use Pagerfanta\Pagerfanta;
use Wallabag\CoreBundle\Entity\Tag;
class EntryRepository extends EntityRepository
{
@ -179,4 +180,22 @@ class EntryRepository extends EntityRepository
->getQuery()
->getSingleResult();
}
/**
* Remove a tag from all user entries.
* We are using a native SQL query because Doctrine doesn't know EntryTag entity because it's a ManyToMany relation.
* Instead of that SQL query we should loop on every entry and remove the tag, could be really long ...
*
* @param int $userId
* @param Tag $tag
*/
public function removeTag($userId, Tag $tag)
{
$sql = 'DELETE et FROM entry_tag et WHERE et.entry_id IN ( SELECT e.id FROM entry e WHERE e.user_id = :userId ) AND et.tag_id = :tagId';
$stmt = $this->getEntityManager()->getConnection()->prepare($sql);
$stmt->execute([
'userId' => $userId,
'tagId' => $tag->getId(),
]);
}
}

View file

@ -9,16 +9,29 @@ use Pagerfanta\Pagerfanta;
class TagRepository extends EntityRepository
{
/**
* Find Tags.
* Return only the QueryBuilder to retrieve all tags for a given user.
*
* @param int $userId
*
* @return array
* @return QueryBuilder
*/
private function getQbForAllTags($userId)
{
return $this->createQueryBuilder('t')
->leftJoin('t.entries', 'e')
->where('e.user = :userId')->setParameter('userId', $userId);
}
/**
* Find Tags and return a Pager.
*
* @param int $userId
*
* @return Pagerfanta
*/
public function findTags($userId)
{
$qb = $this->createQueryBuilder('t')
->where('t.user =:userId')->setParameter('userId', $userId);
$qb = $this->getQbForAllTags($userId);
$pagerAdapter = new DoctrineORMAdapter($qb);
@ -26,19 +39,16 @@ class TagRepository extends EntityRepository
}
/**
* Find a tag by its label and its owner.
* Find Tags.
*
* @param string $label
* @param int $userId
*
* @return Tag|null
* @return array
*/
public function findOneByLabelAndUserId($label, $userId)
public function findAllTags($userId)
{
return $this->createQueryBuilder('t')
->where('t.label = :label')->setParameter('label', $label)
->andWhere('t.user = :user_id')->setParameter('user_id', $userId)
return $this->getQbForAllTags($userId)
->getQuery()
->getOneOrNullResult();
->getResult();
}
}

View file

@ -44,7 +44,7 @@ class ConfigControllerTest extends WallabagCoreTestCase
$form = $crawler->filter('button[id=config_save]')->form();
$data = array(
'config[theme]' => 0,
'config[theme]' => 'baggy',
'config[items_per_page]' => '30',
'config[language]' => 'en',
);
@ -63,7 +63,7 @@ class ConfigControllerTest extends WallabagCoreTestCase
{
return array(
array(array(
'config[theme]' => 0,
'config[theme]' => 'baggy',
'config[items_per_page]' => '',
'config[language]' => 'en',
)),

View file

@ -69,9 +69,7 @@ class RuleBasedTaggerTest extends \PHPUnit_Framework_TestCase
$tags = $entry->getTags();
$this->assertSame('foo', $tags[0]->getLabel());
$this->assertSame($user, $tags[0]->getUser());
$this->assertSame('bar', $tags[1]->getLabel());
$this->assertSame($user, $tags[1]->getUser());
}
public function testTagWithAMixOfMatchingRules()
@ -92,7 +90,6 @@ class RuleBasedTaggerTest extends \PHPUnit_Framework_TestCase
$tags = $entry->getTags();
$this->assertSame('foo', $tags[0]->getLabel());
$this->assertSame($user, $tags[0]->getUser());
}
public function testWhenTheTagExists()
@ -100,7 +97,8 @@ class RuleBasedTaggerTest extends \PHPUnit_Framework_TestCase
$taggingRule = $this->getTaggingRule('rule as string', array('foo'));
$user = $this->getUser([$taggingRule]);
$entry = new Entry($user);
$tag = new Tag($user);
$tag = new Tag();
$tag->setLabel('foo');
$this->rulerz
->expects($this->once())
@ -110,7 +108,9 @@ class RuleBasedTaggerTest extends \PHPUnit_Framework_TestCase
$this->tagRepository
->expects($this->once())
->method('findOneByLabelAndUserId')
// the method `findOneByLabel` doesn't exist, EntityRepository will then call `_call` method
// to magically call the `findOneBy` with ['label' => 'foo']
->method('__call')
->willReturn($tag);
$this->tagger->tag($entry);
@ -118,7 +118,7 @@ class RuleBasedTaggerTest extends \PHPUnit_Framework_TestCase
$this->assertFalse($entry->getTags()->isEmpty());
$tags = $entry->getTags();
$this->assertSame($tag, $tags[0]);
$this->assertSame($tag->getLabel(), $tags[0]->getLabel());
}
public function testSameTagWithDifferentfMatchingRules()

View file

@ -13,7 +13,6 @@ use JMS\Serializer\Annotation\Expose;
use FOS\UserBundle\Model\User as BaseUser;
use Wallabag\CoreBundle\Entity\Config;
use Wallabag\CoreBundle\Entity\Entry;
use Wallabag\CoreBundle\Entity\Tag;
/**
* User.
@ -69,11 +68,6 @@ class User extends BaseUser implements TwoFactorInterface, TrustedComputerInterf
*/
protected $config;
/**
* @ORM\OneToMany(targetEntity="Wallabag\CoreBundle\Entity\Tag", mappedBy="user", cascade={"remove"})
*/
protected $tags;
/**
* @ORM\Column(type="integer", nullable=true)
*/
@ -94,7 +88,6 @@ class User extends BaseUser implements TwoFactorInterface, TrustedComputerInterf
{
parent::__construct();
$this->entries = new ArrayCollection();
$this->tags = new ArrayCollection();
$this->roles = array('ROLE_USER');
}
@ -171,26 +164,6 @@ class User extends BaseUser implements TwoFactorInterface, TrustedComputerInterf
return $this->entries;
}
/**
* @param Entry $entry
*
* @return User
*/
public function addTag(Tag $tag)
{
$this->tags[] = $tag;
return $this;
}
/**
* @return ArrayCollection<Tag>
*/
public function getTags()
{
return $this->tags;
}
public function isEqualTo(UserInterface $user)
{
return $this->username === $user->getUsername();