Improvements in Signals.m2m_changed documentation

[David Larlet]

I think it should be ​explicitly mentioned in the doc that the sender must be the through table when the signal is connected:

m2m_changed.connect(yourfunction, sender=Model.m2mfield.through)

Otherwise, it's a bit difficult, I had to read tests to understand this. Thoughts?

[Russell Keith-Magee]

Quoting the docs: "sender: The intermediate model class describing the ManyToManyField. This class is automatically created when a many-to-many field is defined; you can access it using the through attribute on the many-to-many field.".

So, to my reading, it is explicitly mentioned. If you want to suggest an alternate wording, please reopen with a patch that contains draft text.

[David Larlet]

OK, now that I re-read it, it seems obvious. My bad.
Thanks for the time you spent on this and sorry for the noise.

[jedie]

I have just stumbled across the same problem. I saw my failure, after I have gone through django/tests/modeltests/m2m_signals/models.py

IMHO it's a good idea to insert a code example with m2m_changed.connect() to the docs.

[Adam Nelson]

I agree with jedie. Good example:

    def toppings_changed(sender, **kwargs):
        # Do something
    
    m2m_changed.connect(profile_city_changed, sender=Pizza.toppings.through)

I think the documentation could also be explicit that checking for what has changed is not possible at this time (until #6707 is addressed) and that people are advised to make their own through model and override the save method there if they want to know what is changing.

[Adam Nelson]

Oops, meant:

m2m_changed.connect(toppings_changed, sender=Pizza.toppings.through)

[Jonathan Morgan]

Adding that code sample would help, as well as more specifics, such as:

• "sender" expects to be assigned the class Type of the class whose signals you want to receive.

• When you reference it in code, "<code><class_reference>.<ManyToManyField_name>.through</code>" (example: "Pizza.toppings.through" in the example above) actually returns a class Type instance.

• You can access the <code><ManyToManyField name></code> reference (example: "Pizza.toppings" in the code above) through a reference to its containing class, not just through an instance of that class (this might be obvious in python, but I don't think you can assume everyone would grasp this).

• the class that contains the "through" must either have been imported into the current file or defined there.

• it would be polite to document somewhere the exact characteristics of the default m2m "through" class. I looked today and could not find that anywhere in the documentation.

• It would be good also to document how the M2M save in the admin works (clearing all, then re-adding, and so never firing the remove signals). I would never have thought it worked that way, and it took me a few hours of debugging to figure it out since I couldn't find the details on that documented anywhere. Most will be using these hooks to get around problems they can't address in post-save overriding in the admin. The documentation should ground this example in the admin, so that one knows where the three pre-post pairs of signals occur there, and it should note that in the current implementation of the admin, where most of these signals will occur for the majority of users, one of those signals never gets fired. So, you might need to implement a remove handler, but it won't get called if you manage M2M in the admin, so if there are actions you need to do on remove, well, tough luck, or process them all at clear, then undo some of them on re-add.

[Jonathan Morgan]

And apologies for the bad formatting. In the above, the <code> tags should be removed.

[Gabriel Hurley]

An improved example certainly couldn't hurt. Perhaps adding some more obvious language about through is possible (I'd have to see a patch).

However, most of what jonathanmorgan suggests is covered elsewhere in the signals documentation (sender being the class you wish to receive signals from), or is inherent in Python (re: importing or defining the model you wish to reference). The last two items may be valid (I haven't looked for existing docs for either), but they're separate issues and don't belong in this ticket. If you want to open separate tickets for them you may.

I'll mark the ticket as accepted but strike the milestone until someone provides a patch that offers concrete improvement.

[Jonathan Morgan]

Hello,

I would be happy to work up a patch. What format does a documentation patch take (I looked on the page about how to contribute and couldn't find instructions - if there is a page, please just point me to it, and I apologize for not finding it myself)? I will also make separate tickets for the last two items.

On your comments:

• I did not see anywhere in the signals doc for the m2m_changed signal an explanation of the "<class>.<field>.through" class and how it relates to a model class and creating a ManyToManyField relation. The documentation on this signal says you have to give the class you wish to receive signals from. It does not specify that this class it not the Model class that contains the ManyToManyField, but is the "through" class for a given ManyToManyField, and that a given Model class can have more than one, and that you can configure an m2m_changed signal handler to listen to any or all of them, but that you must configure it for each field, and can't just have it listen to all many-to-many fields on a given model class.

• on things being inherent to python - That might be the case, but I know I personally regularly program in ruby and rails, PHP, Java, perl, and ColdFusion in addition to Python and django, and I think you should aim for thoroughness in examples in discrete areas of documentation, so a given piece of your documentation is as easy to use as possible for the widest range of people with as little jumping around needed for troubleshooting. In looking at python doc, there are two different ways classes can be implemented in python, an old and new way, and I couldn't find anywhere an explicit statement of which django uses. If you had that somewhere - "all django classes are python Type instances, not old-style python classes." That would probably be sufficient, but I couldn't find a straightforward statement of that anywhere in the doc.

This is great software, and I'd like to help make it more accessible to new adopters.

Thanks,

Jonathan

[Gabriel Hurley]

@jonathanmorgan -- There are two pieces of documentation on writing documentation patches:

• ​http://docs.djangoproject.com/en/dev/internals/contributing/#documentation-style
• ​http://docs.djangoproject.com/en/dev/internals/documentation/

To answer the question more directly, the docs are all contained within the Django repository so it's easy to start editing and submitting patches, the docs are constructed using ReST and Sphinx (so there's some amount of markup to understand), and patches should be supplied as diffs against trunk. Take a look at any of the docs tickets that have a patch if you want examples.

I saw your other ticket, and agree that documenting both subjects are separate but related tasks. Thanks for opening that.

Regarding things inherent to Python, I agree about making the docs as easy as possible for newcomers, but there's a line at which we're no longer explaining Django, we're explaining Python--and that's not the goal. But to be specific to this case, if you want to sneak in a line in these docs that says something like "Don't forget to import the class you wish to receive signals from unless it's defined in the same file." then that's the beauty of being the person who writes the patch ;-)

[Aymeric Augustin]

Change UI/UX from NULL to False.

[Aymeric Augustin]

Change Easy pickings from NULL to False.

[Tim Graham <timograham@…>]

In 1360bd4186239d7e4c4481b7d6a1a650fe69d12f:

Fixed #13586 - Added an example of how to connect a m2m_changed signal handler.

[Tim Graham <timograham@…>]

In 3a64adef611ba152eb96d77645480e1953825803:

[1.4.X] Fixed #13586 - Added an example of how to connect a m2m_changed signal handler.

Backport of 1360bd4186 from master