Code

Opened 8 years ago

Closed 8 years ago

#2609 closed defect (fixed)

CurrentSiteManager needs models.Manager() for admin to function.

Reported by: dean@… Owned by: adrian
Component: contrib.admin Version: master
Severity: normal Keywords: CurrentSiteManager
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

When using the CurrentSiteManager in your objects you must include the models.Manager() for the admin interface to work. In the docs on the "sites" framework under The CurrentSiteManager the Photo object example is given. In this example the CurrentSiteManager and the models.Manager() are both explicitly defined so this works. If you do not define the models.Manager() when you use the CurrentSiteManager the admin interface will fail with a ChangeManipulator error (contrib/admin/views/main.py line 317). Nowhere in the docs does it state that you must define both. I think that this is an error since the models.Manager() should still be automatically created even if you use a CurrentSiteManager. A workaround is to define both as shown in the example.

Attachments (0)

Change History (5)

comment:1 Changed 8 years ago by Dean Nevins

Not only do you need to define the models.Manager but you must use the same order as in the example or even that won't work.

For example, this works:

class Rabbit(models.Model):
    title = models.CharField(maxlength=50,core=True,blank=False)
    site = models.ForeignKey(Site,core=True)
    objects = models.Manager()
    on_site = CurrentSiteManager()

This doesn't:

class Rabbit(models.Model):
    title = models.CharField(maxlength=50,core=True,blank=False)
    site = models.ForeignKey(Site,core=True)
    on_site = CurrentSiteManager()
    objects = models.Manager()

comment:2 Changed 8 years ago by ubernostrum

This isn't an error, but the docs could do a better job of pointing at the documentation for custom managers, which does explain that the first manager defined for a model will be the default one, and that defining one or more custom managers will cause the "automatic" objects manager not to be created for you, so that you'll have to create it manually if you want to keep it.

comment:3 Changed 8 years ago by adrian

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

(In [3686]) Fixed #2609 -- Improved docs/sites.txt CurrentSiteManager to explain gotcha with models.Manager

comment:4 Changed 8 years ago by Lakin Wecker <lakin@…>

  • Resolution fixed deleted
  • Status changed from closed to reopened

Actually, the documentation is somewhat wrong ...

The administration doesn't use the appropriate object all of the time ... check out line 216 of db/models/options.py

self.manager = manager or Manager()

This manager gets used in a number of places, more importantly it gets used by the ChangeList class for the admin interface, so the change list shows instances of the model for all sites, regardless of whether your CurrentSiteManager is first or not ...

I hope you don't mind that I'm re-opening the ticket. At the very least, the documentation should be updated.

comment:5 Changed 8 years ago by Lakin Wecker <lakin@…>

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

Sorry, my comments are a duplicate of #2892 #2644. Changing back to fixed ...

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.