Remove unused tag #474

Grafikart · 2017-02-14T13:14:55Z

When a tag is detached from a post it stays in the Tag table filling the table over time. When we update (or delete a post) we clean up unused (unlinked tags)

Improvments ideas 💡

I wanted to add this behaviour in an EventSubscriber but I don't know how to detect when a tag relation is persisted
The query to get unused tag could be improved avoiding the double LEFT JOIN, but I don't know how to ask doctrine to limit the LEFT JOIN to the relation table.

yceruto · 2017-02-14T13:40:56Z

src/AppBundle/Entity/Tag.php

+     * @var array
+     *
+     * @ORM\ManyToMany(targetEntity="AppBundle\Entity\Post", mappedBy="tags")
+     */


@stof: we don't need it. You never use it (and it does not really make sense to have it anyway as tags don't need to be aware of posts). #192 (diff)

I agree with @yceruto. There is a Doctrine best practice:

Avoid bidirectional relationships.

http://docs.doctrine-project.org/projects/doctrine-orm/en/latest/reference/best-practices.html#constrain-relationships-as-much-as-possible

I put this relation to make the query in the TagRepository

@Grafikart, you can delete unused tags using just one SQL query.

and even in DQL, you don't need to have a relation to be able to do a join

yceruto · 2017-02-14T13:44:38Z

src/AppBundle/Controller/Admin/BlogController.php

        if ($form->isSubmitted() && $form->isValid()) {
            $post->setSlug($this->get('slugger')->slugify($post->getTitle()));
            $entityManager->flush();
+            $this->getDoctrine()->getRepository(Tag::class)->cleanUnusedTags();


Why is it necessary to do this? the unused tags could be used for new posts.

Personal choice mainly (I understand if you close the issue).

Having tags attached to nothing could create some problem if you choose to create a tagcloud later for instance.

as you do it using a separate query, you should wrap the flush and this query in a transaction, to be atomic.

stof · 2017-02-14T14:39:38Z

src/AppBundle/Entity/Tag.php

+     *
+     * @return Tag
+     */
+    public function addPost(\AppBundle\Entity\Post $post)


this adder is worse than anything else you did, because it will not add the post (as you don't update the owning side of the relation)

Removing everything from this entity sorry

stof · 2017-02-14T14:39:51Z

src/AppBundle/Repository/TagRepository.php

+use AppBundle\Entity\Tag;
+use Doctrine\ORM\EntityRepository;
+use Doctrine\ORM\Query;
+use Pagerfanta\Adapter\DoctrineORMAdapter;


why pagerfanta ?

stof · 2017-02-14T14:40:03Z

src/AppBundle/Repository/TagRepository.php

+ * See http://symfony.com/doc/current/book/doctrine.html#custom-repository-classes
+ *
+ * @author Ryan Weaver <[email protected]>
+ * @author Javier Eguiluz <[email protected]>


authors look wrong here

stof · 2017-02-14T14:41:48Z

src/AppBundle/Controller/Admin/BlogController.php

        if ($form->isSubmitted() && $form->isValid()) {
            $post->setSlug($this->get('slugger')->slugify($post->getTitle()));
            $entityManager->flush();
+            $this->getDoctrine()->getRepository(Tag::class)->cleanUnusedTags();


you could make this code much more efficient, by getting the old list of tags of the Post in an array before handleRequest, then comparing it to the list of new tags here. The difference will give you the list of tags removed from the post, which are the only ones you need to check for being unused

Agreed, but it would add more code to the controller, is it acceptable ?

Grafikart · 2017-02-14T16:51:31Z

I can't figure how to do this query using DQL https://github.com/symfony/symfony-demo/pull/474/files#diff-2629962eca7be81a3484a3f64ad18074R36 (can't find a way to do a join on the join table without having a ManyToMany between Tag and Post owned by Tag)

voronkovich · 2017-02-14T16:55:34Z

@Grafikart, see http://stackoverflow.com/questions/21513086/how-to-join-without-relational-table-in-symfony-doctrine-with-querybuilder-betwe

javiereguiluz · 2017-02-15T09:00:29Z

src/AppBundle/Controller/Admin/BlogController.php

        if ($form->isSubmitted() && $form->isValid()) {
            $post->setSlug($this->get('slugger')->slugify($post->getTitle()));
-            $entityManager->flush();
+            $entityManager->transactional(function ($entityManager) {


I appreciate your contribution ... but using $entityManager->transactional is "light years" ahead of what we want to teach in the Symfony Demo app. I'm sorry but we can't use this obscure feature. We'll need to find a much simpler alternative solution. Thanks for understanding it.

voronkovich · 2017-02-15T13:01:47Z

src/AppBundle/Repository/TagRepository.php

+        $joinTable = $this->getEntityManager()->getClassMetadata(Post::class)->getAssociationMapping('tags')['joinTable']['name'];
+        $tagTable = $this->getClassMetadata()->getTableName();
+        $unused_tags = $this->getEntityManager()->createNativeQuery("
+            DELETE FROM `$tagTable` WHERE id IN 


@Grafikart , try to use:

DELETE FROM tags t LEFT JOIN tags_posts j ON j.tag_id = t.id WHERE j.tag_id IS NULL

Grafikart added 2 commits February 14, 2017 13:27

Added TagRepository

6ce25d8

Clean up unused Tags

34e9558

yceruto reviewed Feb 14, 2017

View reviewed changes

stof reviewed Feb 14, 2017

View reviewed changes

Cleaned TagRepositoy

04c32a1

Fixed typo

33b3a70

javiereguiluz reviewed Feb 15, 2017

View reviewed changes

voronkovich reviewed Feb 15, 2017

View reviewed changes

Grafikart closed this Mar 11, 2017

Grafikart deleted the unused-tag branch March 11, 2017 11:53

Uh oh!

Remove unused tag #474

Remove unused tag #474

Uh oh!

Conversation

Grafikart commented Feb 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Improvments ideas 💡

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Grafikart commented Feb 14, 2017

Uh oh!

voronkovich commented Feb 14, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Grafikart commented Feb 14, 2017 •

edited

Loading