Multiple tag search was broken from API

First, the setParameter() were done on the same parameter which in fact
just duplicated the condition in the SQL query (like `where t.label =
'test' and t.label = 'test'`.

Changed the parameter doesn't help because the query was then wrong.

Changing the way to match associated tags for an entry and it worked.
This commit is contained in:
Jeremy Benoist 2017-08-03 12:46:20 +02:00
parent 78b36d4dbe
commit 7c04b7396a
No known key found for this signature in database
GPG key ID: BCA73962457ACC3C
5 changed files with 55 additions and 10 deletions

View file

@ -19,7 +19,7 @@ class LoadTagData extends AbstractFixture implements OrderedFixtureInterface
$manager->persist($tag1); $manager->persist($tag1);
$this->addReference('foo-tag', $tag1); $this->addReference('foo-bar-tag', $tag1);
$tag2 = new Tag(); $tag2 = new Tag();
$tag2->setLabel('bar'); $tag2->setLabel('bar');
@ -35,6 +35,13 @@ class LoadTagData extends AbstractFixture implements OrderedFixtureInterface
$this->addReference('baz-tag', $tag3); $this->addReference('baz-tag', $tag3);
$tag4 = new Tag();
$tag4->setLabel('foo');
$manager->persist($tag4);
$this->addReference('foo-tag', $tag4);
$manager->flush(); $manager->flush();
} }

View file

@ -133,7 +133,7 @@ class EntryRepository extends EntityRepository
{ {
$qb = $this->createQueryBuilder('e') $qb = $this->createQueryBuilder('e')
->leftJoin('e.tags', 't') ->leftJoin('e.tags', 't')
->where('e.user =:userId')->setParameter('userId', $userId); ->where('e.user = :userId')->setParameter('userId', $userId);
if (null !== $isArchived) { if (null !== $isArchived) {
$qb->andWhere('e.isArchived = :isArchived')->setParameter('isArchived', (bool) $isArchived); $qb->andWhere('e.isArchived = :isArchived')->setParameter('isArchived', (bool) $isArchived);
@ -152,8 +152,23 @@ class EntryRepository extends EntityRepository
} }
if ('' !== $tags) { if ('' !== $tags) {
foreach (explode(',', $tags) as $tag) { foreach (explode(',', $tags) as $i => $tag) {
$qb->andWhere('t.label = :label')->setParameter('label', $tag); $entryAlias = 'e' . $i;
$tagAlias = 't' . $i;
// Complexe queries to ensure multiple tags is associated to an entry
// https://stackoverflow.com/a/6638146/569101
$qb->andWhere($qb->expr()->in(
'e.id',
$this->createQueryBuilder($entryAlias)
->select($entryAlias . '.id')
->leftJoin($entryAlias . '.tags', $tagAlias)
->where($tagAlias . '.label = :label' . $i)
->getDQL()
));
// bound parameter to the main query builder
$qb->setParameter('label' . $i, $tag);
} }
} }
@ -181,7 +196,7 @@ class EntryRepository extends EntityRepository
->innerJoin('e.tags', 't') ->innerJoin('e.tags', 't')
->innerJoin('e.user', 'u') ->innerJoin('e.user', 'u')
->addSelect('t', 'u') ->addSelect('t', 'u')
->where('e.user=:userId')->setParameter('userId', $userId) ->where('e.user = :userId')->setParameter('userId', $userId)
; ;
return $qb->getQuery()->getResult(); return $qb->getQuery()->getResult();
@ -323,7 +338,27 @@ class EntryRepository extends EntityRepository
{ {
$qb = $this->createQueryBuilder('e') $qb = $this->createQueryBuilder('e')
->select('count(e)') ->select('count(e)')
->where('e.user=:userId')->setParameter('userId', $userId) ->where('e.user = :userId')->setParameter('userId', $userId)
;
return (int) $qb->getQuery()->getSingleScalarResult();
}
/**
* Count all entries for a tag and a user.
*
* @param int $userId
* @param int $tagId
*
* @return int
*/
public function countAllEntriesByUserIdAndTagId($userId, $tagId)
{
$qb = $this->createQueryBuilder('e')
->select('count(e.id)')
->leftJoin('e.tags', 't')
->where('e.user = :userId')->setParameter('userId', $userId)
->andWhere('t.id = :tagId')->setParameter('tagId', $tagId)
; ;
return (int) $qb->getQuery()->getSingleScalarResult(); return (int) $qb->getQuery()->getSingleScalarResult();

View file

@ -292,6 +292,9 @@ class EntryRestControllerTest extends WallabagApiTestCase
$this->assertSame(1, $content['page']); $this->assertSame(1, $content['page']);
$this->assertGreaterThanOrEqual(1, $content['pages']); $this->assertGreaterThanOrEqual(1, $content['pages']);
$this->assertContains('foo', array_column($content['_embedded']['items'][0]['tags'], 'label'), 'Entries tags should have "foo" tag');
$this->assertContains('bar', array_column($content['_embedded']['items'][0]['tags'], 'label'), 'Entries tags should have "bar" tag');
$this->assertArrayHasKey('_links', $content); $this->assertArrayHasKey('_links', $content);
$this->assertArrayHasKey('self', $content['_links']); $this->assertArrayHasKey('self', $content['_links']);
$this->assertArrayHasKey('first', $content['_links']); $this->assertArrayHasKey('first', $content['_links']);

View file

@ -239,7 +239,7 @@ class ExportControllerTest extends WallabagCoreTestCase
$this->assertSame($contentInDB->getLanguage(), $content[0]['language']); $this->assertSame($contentInDB->getLanguage(), $content[0]['language']);
$this->assertSame($contentInDB->getReadingtime(), $content[0]['reading_time']); $this->assertSame($contentInDB->getReadingtime(), $content[0]['reading_time']);
$this->assertSame($contentInDB->getDomainname(), $content[0]['domain_name']); $this->assertSame($contentInDB->getDomainname(), $content[0]['domain_name']);
$this->assertSame(['foo bar', 'baz'], $content[0]['tags']); $this->assertSame(['baz', 'foo'], $content[0]['tags']);
} }
public function testXmlExport() public function testXmlExport()

View file

@ -184,13 +184,13 @@ class RssControllerTest extends WallabagCoreTestCase
$em->flush(); $em->flush();
$client = $this->getClient(); $client = $this->getClient();
$client->request('GET', '/admin/SUPERTOKEN/tags/foo-bar.xml'); $client->request('GET', '/admin/SUPERTOKEN/tags/foo.xml');
$this->assertSame(200, $client->getResponse()->getStatusCode()); $this->assertSame(200, $client->getResponse()->getStatusCode());
$this->validateDom($client->getResponse()->getContent(), 'tag (foo bar)', 'tags/foo-bar'); $this->validateDom($client->getResponse()->getContent(), 'tag (foo)', 'tags/foo');
$client->request('GET', '/admin/SUPERTOKEN/tags/foo-bar.xml?page=3000'); $client->request('GET', '/admin/SUPERTOKEN/tags/foo.xml?page=3000');
$this->assertSame(302, $client->getResponse()->getStatusCode()); $this->assertSame(302, $client->getResponse()->getStatusCode());
} }
} }