Opened 8 years ago
Closed 8 years ago
#27350 closed Bug (needsinfo)
Attaching signals to abstract models don't work as it used to be
Reported by: | Florent Messa | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 1.10 |
Severity: | Normal | Keywords: | |
Cc: | Loic Bistuer | Triage Stage: | Unreviewed |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Hi,
I'm currently porting an application from Django 1.9.x to 1.10.x, we are relying on managers to attach signals to allow developers overriding them in our generic applications, this does not work anymore.
A complete example:
from django.db import models from django.db.models import signals class Poll(models.Model): question = models.CharField(max_length=255) answers_count = models.PositiveIntegerField(default=0) class AnswerManager(models.Manager): def contribute_to_class(self, cls, name): signals.post_save.connect(self.post_save, sender=cls) return super(AnswerManager, self).contribute_to_class(cls, name) def post_save(self, instance, **kwargs): poll = instance.poll poll.answers_count = poll.answers.count() poll.save(update_fields=('answers_count', )) class AbstractAnswer(models.Model): text = models.CharField(max_length=255) poll = models.ForeignKey(Poll, related_name='answers') objects = AnswerManager() class Meta: abstract = True class Answer(AbstractAnswer): class Meta: abstract = False class ModelsTests(TestCase): def test_simple_compute(self): poll = Poll.objects.create(question='Weird?') Answer.objects.create(poll=poll, text='Yes') # refresh poll = Poll.objects.get(pk=poll.pk) # still 0 on Django 1.10 and 1 on Django 1.9 assert poll.answers_count == 1
The complete application is available here and travis is available here to test again both 1.10 and 1.9.
Is this some kind of undocumented new behavior or a bug?
Change History (6)
follow-up: 2 comment:1 by , 8 years ago
comment:2 by , 8 years ago
Replying to Tim Graham:
Hi, you should bisect to find the commit where the behavior changed. If I had to guess it might be ed0ff913c648b16c4471fc9a9441d1ee48cb5420 or 3a47d42fa33012b2156bf04058d933df6b3082d2.
It was introduced by 3a47d42fa33012b2156bf04058d933df6b3082d2
Build is available on travis, 110a is the parent commit of 3a47d42fa33012b2156bf04058d933df6b3082d2 and 110b is located at 3a47d42fa33012b2156bf04058d933df6b3082d2
follow-up: 4 comment:3 by , 8 years ago
Cc: | added |
---|
Looking at your code, I don't think we broke any public APIs, so I'd say it's up to you to investigate and say why it's a bug and to propose a fix. Maybe Loic can advise without too much effort.
comment:4 by , 8 years ago
Replying to Tim Graham:
Looking at your code, I don't think we broke any public APIs, so I'd say it's up to you to investigate and say why it's a bug and to propose a fix. Maybe Loic can advise without too much effort.
The contribute_to_class is called twice in 1.9.x on the abstract level and child.
That's not the case on 1.10.x your child model needs to declare explicitly the manager even if it's inherited.
I can update my code on my side on every child to redeclare the manager, as you have said it's not a public API as it's not documented. I can investigate more on my side and provide a fix for that but will you consider it?
Btw, thank you for you reactivity ;)
comment:5 by , 8 years ago
I didn't follow the manager changes closely enough to say what the expected behavior is.
comment:6 by , 8 years ago
Resolution: | → needsinfo |
---|---|
Status: | new → closed |
Hi, you should bisect to find the commit where the behavior changed. If I had to guess it might be ed0ff913c648b16c4471fc9a9441d1ee48cb5420 or 3a47d42fa33012b2156bf04058d933df6b3082d2.