TagController: fix duplicated tags when renaming them

The fix relies on a workaround available on TagsAssigner, see the
AssignTagsToEntry() signature for detail.

I replaced the findOneByLabel in the corresponding test to assert that
there is no duplicate.

Fixes #4216

Signed-off-by: Kevin Decherf <kevin@kdecherf.com>
This commit is contained in:
Kevin Decherf 2020-04-04 21:03:22 +02:00
parent f3565ea2bf
commit 39133eb796
2 changed files with 37 additions and 8 deletions

View file

@ -152,6 +152,10 @@ class TagController extends Controller
$form->handleRequest($request); $form->handleRequest($request);
if ($form->isSubmitted() && $form->isValid()) { if ($form->isSubmitted() && $form->isValid()) {
$newTagLabel = $form->get('label')->getData();
$newTag = new Tag();
$newTag->setLabel($newTagLabel);
$entries = $this->get('wallabag_core.entry_repository')->findAllByTagId( $entries = $this->get('wallabag_core.entry_repository')->findAllByTagId(
$this->getUser()->getId(), $this->getUser()->getId(),
$tag->getId() $tag->getId()
@ -159,7 +163,8 @@ class TagController extends Controller
foreach ($entries as $entry) { foreach ($entries as $entry) {
$this->get('wallabag_core.tags_assigner')->assignTagsToEntry( $this->get('wallabag_core.tags_assigner')->assignTagsToEntry(
$entry, $entry,
$form->get('label')->getData() $newTagLabel,
[$newTag]
); );
$entry->removeTag($tag); $entry->removeTag($tag);
} }

View file

@ -179,15 +179,24 @@ class TagControllerTest extends WallabagCoreTestCase
public function testRenameTagUsingTheFormInsideTagList() public function testRenameTagUsingTheFormInsideTagList()
{ {
$newTagLabel = 'rename label';
$this->logInAs('admin'); $this->logInAs('admin');
$client = $this->getClient(); $client = $this->getClient();
$tag = new Tag(); $tag = new Tag();
$tag->setLabel($this->tagName); $tag->setLabel($this->tagName);
$entry = new Entry($this->getLoggedInUser()); $entry = new Entry($this->getLoggedInUser());
$entry->setUrl('http://0.0.0.0/foo'); $entry->setUrl('http://0.0.0.0/foo');
$entry->addTag($tag); $entry->addTag($tag);
$this->getEntityManager()->persist($entry); $this->getEntityManager()->persist($entry);
$entry2 = new Entry($this->getLoggedInUser());
$entry2->setUrl('http://0.0.0.0/bar');
$entry2->addTag($tag);
$this->getEntityManager()->persist($entry);
$this->getEntityManager()->flush(); $this->getEntityManager()->flush();
$this->getEntityManager()->clear(); $this->getEntityManager()->clear();
@ -196,7 +205,7 @@ class TagControllerTest extends WallabagCoreTestCase
$form = $crawler->filter('#tag-' . $tag->getId() . ' form')->form(); $form = $crawler->filter('#tag-' . $tag->getId() . ' form')->form();
$data = [ $data = [
'tag[label]' => 'specific label', 'tag[label]' => $newTagLabel,
]; ];
$client->submit($form, $data); $client->submit($form, $data);
@ -207,19 +216,34 @@ class TagControllerTest extends WallabagCoreTestCase
->getRepository('WallabagCoreBundle:Entry') ->getRepository('WallabagCoreBundle:Entry')
->find($entry->getId()); ->find($entry->getId());
$tags = $freshEntry->getTags()->toArray(); $freshEntry2 = $client->getContainer()
foreach ($tags as $key => $item) { ->get('doctrine.orm.entity_manager')
->getRepository('WallabagCoreBundle:Entry')
->find($entry2->getId());
$tags = [];
$tagsFromEntry = $freshEntry->getTags()->toArray();
foreach ($tagsFromEntry as $key => $item) {
$tags[$key] = $item->getLabel(); $tags[$key] = $item->getLabel();
} }
$this->assertFalse(array_search($tag->getLabel(), $tags, true), 'Previous tag is not attach to entry anymore.'); $tagsFromEntry2 = $freshEntry2->getTags()->toArray();
foreach ($tagsFromEntry2 as $key => $item) {
$tags[$key] = $item->getLabel();
}
$this->assertFalse(array_search($tag->getLabel(), $tags, true), 'Previous tag is not attach to entries anymore.');
$newTag = $client->getContainer() $newTag = $client->getContainer()
->get('doctrine.orm.entity_manager') ->get('doctrine.orm.entity_manager')
->getRepository('WallabagCoreBundle:Tag') ->getRepository('WallabagCoreBundle:Tag')
->findOneByLabel('specific label'); ->findByLabel($newTagLabel);
$this->assertInstanceOf(Tag::class, $newTag, 'Tag "specific label" exists.');
$this->assertTrue($newTag->hasEntry($freshEntry), 'Tag "specific label" is assigned to the entry.'); $this->assertCount(1, $newTag, 'New tag exists.');
$this->assertTrue($newTag[0]->hasEntry($freshEntry), 'New tag is assigned to the entry.');
$this->assertTrue($newTag[0]->hasEntry($freshEntry2), 'New tag is assigned to the entry2.');
} }
public function testAddUnicodeTagLabel() public function testAddUnicodeTagLabel()