1

We have a tagging system for users to filter their files by defined tags.

Here's how the models are set up:

class Tags(models.Model):
    name = models.CharField(max_length=100)
    user = models.ForeignKey(User)

class Files(models.Model):
    user = models.ForeignKey(User)
    name = models.CharField(max_length=100)
    tags = models.ManyToManyField(Tags, null=True, blank=True)

Now, because tags are not required, when we remove tags from a file they don't get deleted. This leaves a bunch of tags saved on our database and we want to clean them up.

I've tried redefining the save method on the Files model, and the clean method.

I've tried connecting an m2m_changed signal on the Files model: https://docs.djangoproject.com/en/dev/ref/signals/#m2m-changed

Last thing I tried was a pre_save signal: https://docs.djangoproject.com/en/dev/ref/signals/#pre-save

I was planning to iterate over the tags and delete the ones with empty files_set, but using these methods I can't reliably figure that out (i.e. I end up removing tags that aren't associated but are about to be associated (because m2m_changed fires several times with different actions)).

Here's what I thought would work:

def handle_tags (sender, instance, *args, **kwargs) :
    action = kwargs.get('action')
    if action == 'post_clear':
        # search through users tags... I guess?
        tags = Tags.objects.filter(user=instance.user)
        for tag in tags:
            if not tag.files_set.exists():
                tag.delete()
    return

m2m_changed.connect(handle_tags, sender=Files.tags.through)

But, as I said, it will delete a tag before it is added (and if it's added, we obviously don't want to delete it).

bozdoz
  • 12,550
  • 7
  • 67
  • 96
  • Are you sure [this](http://stackoverflow.com/questions/3937194/django-cascade-deletion-in-manytomanyrelation) doesn't solve your problem. – yorodm Apr 15 '14 at 21:39
  • You think it would be better to make the tags into ForeignKeys? @yorodm What about untagged Files? A standard `untagged` tag? Also, that would limit us to only one tag per file. – bozdoz Apr 15 '14 at 22:06
  • If you are willing to change code maybe you can use [django-taggit](http://django-taggit.readthedocs.org/en/latest/), it is an off-the-shelf tagging solution for django-apps. – yorodm Apr 15 '14 at 22:12
  • Check [this](https://github.com/alex/django-taggit/blob/develop/taggit/managers.py) out. I think it will solve your problem. @bozdoz – yorodm Apr 15 '14 at 22:12

3 Answers3

3

You we're on the right track when using the m2m_changed signal.

Your problem is that when responding to the post_clear signal the tags have already been deleted so you won't be able to access them like that.

You actually need to dispatch your method before the tags are deleted, which means handling the pre_clear signal.

Something like this:

@receiver(m2m_changed, sender=Files.tags.through)
def handle_tags(sender, **kwargs):

    action = kwargs['action']

    if action == "pre_clear":
        tags_pk_set = kwargs['instance'].tags.values_list('pk')
    elif action == "pre_remove":
        tags_pk_set = kwargs.get('pk_set')
    else:
        return

    # I'm using Count() just so I don't have to iterate over the tag objects
    annotated_tags = Tags.objects.annotate(n_files=Count('files'))
    unreferenced = annotated_tags.filter(pk__in=tags_pk_set).filter(n_files=1)
    unreferenced.delete()

I've also added the handling of the pre_remove signal in which you can use the pk_set argument to get the actual tags that will be removed.

UPDATE

Of course the previous listener won't delete the unreferenced tags when deleting the files, since it's only handling the pre_clear and pre_remove signals from the Tags model. In order to do what you want, you should also handle the pre_delete signal of the Files model.

In the code below I've added an utility function remove_tags_if_orphan, a slightly modified version of handle_tags and a new handler called handle_file_deletion to remove the tags which will become unreferenced once the File is deleted.

def remove_tags_if_orphan(tags_pk_set):
    """Removes tags in tags_pk_set if they're associated with only 1 File."""

    annotated_tags = Tags.objects.annotate(n_files=Count('files'))
    unreferenced = annotated_tags.filter(pk__in=tags_pk_set).filter(n_files=1)
    unreferenced.delete()


# This will clean unassociated Tags when clearing or removing Tags from a File
@receiver(m2m_changed, sender=Files.tags.through)
def handle_tags(sender, **kwargs):
    action = kwargs['action']
    if action == "pre_clear":
        tags_pk_set = kwargs['instance'].tags.values_list('pk')
    elif action == "pre_remove":
        tags_pk_set = kwargs.get('pk_set')
    else:
        return
    remove_tags_if_orphan(tags_pk_set)


# This will clean unassociated Tags when deleting/bulk-deleting File objects
@receiver(pre_delete, sender=Files)
def handle_file_deletion(sender, **kwargs):
    associated_tags = kwargs['instance'].tags.values_list('pk')
    remove_tags_if_orphan(associated_tags)

Hope this clears things up.

kirbuchi
  • 2,274
  • 2
  • 23
  • 35
  • I think I tried something like this already. I had issues with adding and removing. Sometimes it removed tags before they were added, and sometimes it checked for unreferenced tags before they were unreferenced. Anyway, I'll give it a try and let you know! – bozdoz Apr 16 '14 at 23:24
  • Curious, why are you filtering by n_files=1 and not n_files=0? – bozdoz Apr 18 '14 at 16:25
  • Anyway, it seems to work for most scenarios, which is great. However, if you delete the file, it won't delete the tags associated to the file. I suppose we could do another check for file deletion. – bozdoz Apr 18 '14 at 16:29
  • 1
    @bozdoz I'm checking n_files=1 because I'm handling the pre_clear and pre_remove events, meaning the tags haven't been unassociated from the File at this point. Also, I've updated my answer to handle the case when File objects are deleted. – kirbuchi Apr 19 '14 at 07:27
  • Very good answer, and great solution. Can't believe it's not built into django. – bozdoz Apr 20 '14 at 22:03
0

Just to sum up with hopefully a cleaner answer:

from django.db.models.signals import m2m_changed
from django.dispatch import receiver

class Tags(models.Model):
    name = models.CharField(max_length=100)
    user = models.ForeignKey(User)

class Files(models.Model):
    user = models.ForeignKey(User)
    name = models.CharField(max_length=100)
    tags = models.ManyToManyField(Tags, null=True, blank=True)


@receiver(m2m_changed, sender=Files.tags.through)
def delete_orphean_dateranges(sender, **kwargs):
    if kwargs['action'] == 'post_remove':
        Tags.objects.filter(pk__in=kwargs['pk_set'], files_set=None).delete()

post_remove ensure that the callback is fired when a Tag was disassociated from a File

Guillaume Lebreton
  • 2,586
  • 16
  • 25
-1

I think you go deeper than it required. Just define related_name for Tag, and process post_save signal from File.

class Files(models.Model):
    user = models.ForeignKey(User)
    name = models.CharField(max_length=100)
    tags = models.ManyToManyField(Tags, null=True, blank=True, related_name='files')


def clean_empty_tags(sender, instance, *args, **kwargs):
     Tags.objects.filter(user=instance.user, files=None).delete()

post_save.connect(clean_empty_tags, sender=Files)
Eugene Soldatov
  • 9,755
  • 2
  • 35
  • 43
  • When you add a tag, it is at first unreferenced, so this would delete tags before it adds tags; therefore, it prevents adding tags. – bozdoz Apr 18 '14 at 16:40
  • Hm, if tags tied to User, how can it be deleted? As you can see I propose delete only tags of particular user who add/edit file. User creates file and adds tags. Tags will be saved before File. After that you will save file. New tags will not be deleted. Or you want firstly add many tags? And not tie them to files at once? Maybe you shouldn't allow user to create unrelated to file tags if you wan't such tags to exist in database? – Eugene Soldatov Apr 18 '14 at 18:08
  • The method will be fired too often, when not needed, leading to unnecessary calls to the database – Guillaume Lebreton Nov 12 '18 at 18:24