Code

Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#17605 closed Bug (fixed)

It is not documented how to work with ManyToMany relations

Reported by: kmike Owned by: ramiro
Component: Documentation Version: 1.3
Severity: Release blocker Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

The docs for ManyToMany relations used to rely on doctests ("See the Many-to-many relationship model example for a full example.")
https://docs.djangoproject.com/en/dev/topics/db/models/#many-to-many-relationships

But since https://code.djangoproject.com/changeset/14285/ these doctests were removed. So ManyToMany relations are now almost undocumented (both in dev version of docs and in all old versions of docs).

Because this is a regression from Django 1.3 I've marked it as a release blocker.

Attachments (4)

17605-1.diff (3.1 KB) - added by claudep 2 years ago.
Adding ManyToManyField examples
17605-2.diff (33.7 KB) - added by ramiro 2 years ago.
Alternative patch adding a topics/db/examples/ hierarchy
patch_17605.diff (23.8 KB) - added by zsiciarz 2 years ago.
Trying to simplify the examples, probably needs some more tweaking.
17605-5.diff (24.7 KB) - added by ramiro 2 years ago.
zsiciarz's patch plus some ReST/Sphinx tweaks

Download all attachments as: .zip

Change History (14)

Changed 2 years ago by claudep

Adding ManyToManyField examples

comment:1 Changed 2 years ago by claudep

  • Has patch set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

I added some code examples in the docs. Don't know if it is enough to cover most usages, but it's a start at least. Double-check my English, as usual.

Changed 2 years ago by ramiro

Alternative patch adding a topics/db/examples/ hierarchy

comment:2 Changed 2 years ago by ramiro

In the '-2' patch I simply converted to ReST syntax the models.py files of the modeltests for m2m, FK and one2one relationships freezing them with the state they were in when their respective doctests were removed.

Also, removed all references to tests/modeltests/* from topics/db/models.txt.

Last edited 2 years ago by ramiro (previous) (diff)

comment:3 Changed 2 years ago by zsiciarz

  • Patch needs improvement set

comment:4 Changed 2 years ago by zsiciarz

The second patch is a good starting point. However, in my opinion the examples should be simplified. There's too much tests for various field lookups which are already documented elsewhere. If these are meant to be examples, they must be short, simple and useful. Also a few more lines of text would be welcome.

Changed 2 years ago by zsiciarz

Trying to simplify the examples, probably needs some more tweaking.

comment:5 Changed 2 years ago by ramiro

  • Owner changed from nobody to ramiro
  • Status changed from new to assigned

Changed 2 years ago by ramiro

zsiciarz's patch plus some ReST/Sphinx tweaks

comment:6 Changed 2 years ago by ramiro

  • Patch needs improvement unset

comment:7 Changed 2 years ago by aaugustin

  • Patch needs improvement set

The style shows that these documents used to be doctests, but it's a good basis IMO.

topics/db/models.txt

  • This page still refers to the "Many-to-one relationship model tests", but that link is pointing to a regular documentation page. Use the same wording as the other "See also" blocks?

topics/db/examples/many_to_one.txt

  • a = Article(id=None, ...: this isn't idiomatic; I'd skip the id kwarg entirely.
  • "Article objects have access to their related Reporter objects:": this was demonstrated just above, I'd reorganize this part a bit
  • >>> Article.objects.filter(reporter=1): I didn't know this was supported, and it feels unclean. Plus, there are two ways to do the same thing. Do we really want to document this possibility?
  • >>> Reporter.objects.filter(article=1): same comment

topics/db/examples/many_to_many.txt

  • "Add a Publication directly via publications.add...": shouldn't that be "publications.create"?
  • >>> Article.objects.filter(publications=1): same comment again
  • publications__in=[1,2]: missing space after the comma
  • publications__in=[p1,p2]: missing space after the comma
  • >>> Publication.objects.filter(article=1): same comment again
  • article__in=[1,2]: missing space after the comma
  • article__in=[a1,a2]: missing space after the comma
  • "After the delete, the QuerySet cache needs to be cleared, and the referenced objects should be gone": this sounds like an internal implementation detail, not something a developer using Django should worry about

topics/db/examples/one_to_one.txt

  • "Restaurant.objects.all()" should be in fixed width font.

I hope this helps!

comment:8 Changed 2 years ago by ubernostrum

  • Resolution set to fixed
  • Status changed from assigned to closed

In [17737]:

Fixed #17605: Restored deleted query documentation that used to live in doctests. Thanks zsiciarz for work on the patch.

comment:9 Changed 2 years ago by claudep

In [17832]:

Fixed #17999 -- Restored the links to examples from models documentation. Refs #17605.

comment:4 Changed 2 years ago by claudep

In [17833]:

[1.4.X] Fixed #17999 -- Restored the links to examples from models documentation. Refs #17605.

Backport of r17832 from trunk.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.