Opened 4 years ago

Closed 3 years ago

#16073 closed Bug (duplicate)

Django admin save not sending post remove action with m2m changed signal

Reported by: sam@… Owned by: nobody
Component: contrib.admin Version: 1.3
Severity: Normal Keywords: m2m_changed
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Hi all

I'm trying to get a many to many model to update when I save a related model. This should be possible using the m2m_changed signal (and it works! but not in the admin?) e.g.

# i want the references field to update when related model is saved.
# so just call count_references

class Tag(models.Model):
    """Group everything into categories"""
    # stuff stuff stuff
    references = models.IntegerField(default=0, editable=False)

    def count_references(self):
        # just add up references each time to save headaches
        self.references = 0
        # search for reverse managers
        sets = re.compile('^\w+_set$')
        for rel_set in [method for method in dir(self) if sets.match(method)]:
            self.references += getattr(self, rel_set).count()
        self.save()

class Entry(models.Model):
    """Blog entry"""
    # stuff stuff stuff
    tags = models.ManyToManyField('Tag', blank=True)

# this will call count_references when entry adds or removes tags

@receiver(m2m_changed, sender=Entry.tags.through)
def update_tag_ref_count(sender, instance, action, reverse, model, pk_set, **kwargs):
    print action
    if not reverse and action == 'post_add' or action == 'post_remove':
        for tag_pk in pk_set:
            print tag_pk
            Tag.objects.get(pk=tag_pk).count_references()
            print Tag.objects.get(pk=tag_pk).references

Everything works perfectly when run in the shell. e.g. with a tests.py like so:

t = Tag.objects.all()[0]
s = Snippet.objects.all()[0]

s.tags.remove(t)
s.save()

s.tags.add(t)
s.save()

I get the following (where 'test' is the tag name being printed):

pre_remove
post_remove
test
0
pre_add
post_add
test
1

perfect! And when I add a tag to an entry in the admin I get the following (between HTTP stuff):

pre_clear
post_clear
pre_add
post_add
test
1

still good! not sure what pre/post_clear was called for... and when I remove:

pre_clear
post_clear

argh! pre/post_remove is not called! pre/post_clear is useless as well as it doesn't provide any primary keys. this feels like a bug in the admin implementation. any suggestions?

Change History (9)

comment:1 Changed 4 years ago by anonymous

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Also, I'm displaying the tags field with the horizontal filter (filter_horizontal).

I will look at the admin code when I get a chance to see how they're handling m2m dispatching.

Thanks,

Sam

comment:2 Changed 4 years ago by aaugustin

  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 4 years ago by aaugustin

  • UI/UX unset

#16634 was a duplicate.

comment:4 Changed 4 years ago by chrisdpratt@…

Any update on this? Just ran into this issue myself. Seems absolutely incredible to me that this is still an outstanding bug even after the 1.3.1 update.

comment:5 Changed 4 years ago by cole.mickens@…

I can confirm that this occurs in the admin interface for user and groups.

Go to a user page, remove a group from their list of groups. pre/post_remove are not fired for m2m_changed with sender=User.groups.through.

As others note, this makes it impossible to do somethings as PKs are not available for pre/post_clear.

comment:6 Changed 4 years ago by jblaine

I can confirm, sadly, that this bug still exists in the 1.4-alpha-1 release.

Version 0, edited 4 years ago by jblaine (next)

comment:7 Changed 4 years ago by jblaine

Comment 11 on bug 6707 (related) indicates this is not a problem just with the 'Admin' interface, but ModelForm itself : https://code.djangoproject.com/ticket/6707#comment:11

comment:8 follow-up: Changed 3 years ago by anonymous

Can I suggest that a note be added to the django documentation about this limitation - it would've saved me many hours of hair-tearing and self-recrimination.

comment:9 in reply to: ↑ 8 Changed 3 years ago by carljm

  • Resolution set to duplicate
  • Status changed from new to closed

Replying to anonymous:

Can I suggest that a note be added to the django documentation about this limitation - it would've saved me many hours of hair-tearing and self-recrimination.

This ticket and #6707 are both in Accepted state, meaning that the intention is to fix them, not to declare them a known limitation.

Closing this as a duplicate of #6707 - this bug is just a symptom of that one.

Note: See TracTickets for help on using tickets.
Back to Top