Attaching signals to abstract models don't work as it used to be

[Florent Messa]

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?

[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.

[Florent Messa]

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

[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.

[Florent Messa]

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 your reactivity ;)

[Tim Graham]

I didn't follow the manager changes closely enough to say what the expected behavior is.