Django

Code

Ticket #3987 (closed: fixed)

Opened 3 years ago

Last modified 1 year ago

ModelAdmin should allow for overriding of ForeignKey/ManyToMany options

Reported by: Baptiste <baptiste.goupil@gmail.com> Assigned to: nobody
Milestone: Component: django.contrib.admin
Version: newforms-admin Keywords: nfa-someday
Cc: brosner@gmail.com, apollo13, mwdiers Triage Stage: Design decision needed
Has patch: 1 Needs documentation: 0
Needs tests: 1 Patch needs improvement: 1

Description (Last modified by adrian)

See discussion in comments.

Attachments

customfilters.diff (7.8 kB) - added by Baptiste <baptiste.goupil@gmail.com> on 04/10/07 05:51:42.
the patch
diff-dynamic.diff (7.7 kB) - added by Baptiste <baptiste.goupil@gmail.com> on 07/10/07 16:10:01.
new patch (cleaner ?)
diff-dynamic.2.diff (7.6 kB) - added by Baptiste <baptiste.goupil@gmail.com> on 07/10/07 18:14:21.
oops, fixed a little error
diff-dynamic.3.diff (8.6 kB) - added by Baptiste <baptiste.goupil@gmail.com> on 07/11/07 02:28:35.
security fix : the choice of an item not in the list of choices must fail
dynamic_related_3987.diff (6.4 kB) - added by ext on 09/27/07 15:17:26.
Patch updated to work with version 6426
dynamic_related_3987.2.diff (4.6 kB) - added by mikeblake on 07/30/08 06:25:25.
Updated to revision 8151
doc_diff.txt (1.2 kB) - added by ikse hefem on 08/01/08 09:06:40.
tentative feature documentation
doc.diff (1.3 kB) - added by ikse hefem on 08/01/08 09:09:37.
New patch (sorry, fisrt try) , forget/delete the preceding one if possible
diff.txt (4.3 kB) - added by Xniver on 08/23/08 15:23:56.
updated, works with revision 8502
dynamic.diff (4.3 kB) - added by Xniver on 08/23/08 15:25:29.
sorry, forgot to change extension
dynamic_choice_9014.diff (4.4 kB) - added by mwdiers on 09/13/08 14:10:37.
Updated patch to rev 9014. This will work with 1.0 also.

Change History

04/10/07 05:51:42 changed by Baptiste <baptiste.goupil@gmail.com>

  • attachment customfilters.diff added.

the patch

04/10/07 05:52:48 changed by Baptiste <baptiste.goupil@gmail.com>

  • needs_better_patch set to 1.
  • has_patch set to 1.
  • needs_tests changed.
  • needs_docs changed.

04/11/07 05:33:56 changed by Simon G. <dev@simon.net.nz>

  • stage changed from Unreviewed to Design decision needed.

06/07/07 16:55:34 changed by adrian

  • version changed from new-admin to SVN.

06/07/07 17:35:11 changed by adrian

  • version changed from SVN to newforms-admin.

06/07/07 17:39:44 changed by adrian

  • summary changed from Customs filters for related keys in the admin (newforms-admin) to ModelAdmin should allow for overriding of ForeignKey/ManyToMany options.
  • description changed.
  • stage changed from Design decision needed to Accepted.

I like the idea, but the implementation is a bit brittle. What about adding method hooks on ModelAdmin that would allow you to specify the options for a given ForeignKey field? These methods could get passed the request object, so you could do per-user options. Something like this:

# Model

class Book(models.Model):
    title = models.CharField(maxlength=100)
    author = models.ForeignKey(Author)

# Admin

class BookAdmin(ModelAdmin):
    def dynamic_author_choices(self, request, book):
        # Default implementation:
        # return book.author_set.all()
        return book.author_set.filter(use_in_admin=True)

06/08/07 01:33:47 changed by Baptiste <baptiste.goupil@gmail.com>

If dynamic_field_choices() is automatically called for every field when admin is displayed, that would be great, but should also work for ManyToManyField?.

06/22/07 14:19:01 changed by Brian Rosner <brosner@gmail.com>

  • cc set to brosner@gmail.com.

07/10/07 16:10:01 changed by Baptiste <baptiste.goupil@gmail.com>

  • attachment diff-dynamic.diff added.

new patch (cleaner ?)

07/10/07 16:15:42 changed by Baptiste <baptiste.goupil@gmail.com>

  • needs_better_patch deleted.

Okay, here is a new patch that I hope cleaner.

It works almost like Adrian suggested it :

# Model

class Book(models.Model):
    title = models.CharField(maxlength=100)
    author = models.ForeignKey(Author)

# Admin

class BookAdmin(ModelAdmin):
    def dynamic_author_choices(self, request, model):
        # Default implementation:
        # return book.author_set.all()
        return model.objects.filter(use_in_admin=True)

And also allows:

class BookAdmin(ModelAdmin):
    def dynamic_author_choices(self, request, model):
        if request.user.is_superuser:
            return model.objects.all()
        return model.objects.filter(use_in_admin=True)            

07/10/07 18:14:21 changed by Baptiste <baptiste.goupil@gmail.com>

  • attachment diff-dynamic.2.diff added.

oops, fixed a little error

07/11/07 02:28:35 changed by Baptiste <baptiste.goupil@gmail.com>

  • attachment diff-dynamic.3.diff added.

security fix : the choice of an item not in the list of choices must fail

09/07/07 00:46:22 changed by anonymous

  • cc changed from brosner@gmail.com to brosner@gmail.com, django@apolloner.eu.

09/10/07 12:41:11 changed by anonymous

I am +1 for this patch. This is really a useful addition.

09/26/07 01:46:25 changed by ext

I attached an updated version of patch. It now works with revision: 6426.

A little change I did to the patch is removal of all occurences of:

defaults = {'form_class': forms.ModelChoiceField} 
#To prevent an useless query
if not 'queryset' in kwargs: 
   defaults['queryset'] = self.rel.to._default_manager.all()

in favour of original code:

defaults = {'form_class': forms.ModelChoiceField, 'queryset': self.rel.to._default_manager.all()} 

The reason is that QuerySets are lazy so there is no "useless query".

09/27/07 15:17:26 changed by ext

  • attachment dynamic_related_3987.diff added.

Patch updated to work with version 6426

09/27/07 15:21:51 changed by ext

I replaced the previous version of the patch, because it had a small bug. I did simple replacement in line 203 of models.py (s/value/val/)

12/09/07 09:20:38 changed by Karen Tracey <kmtracey@gmail.com>

  • keywords set to nfa-someday.
  • needs_docs set to 1.
  • needs_tests set to 1.

New function so it shouldn't block newforms-admin merge. Also it needs docs and tests, I think.

(follow-ups: ↓ 15 ↓ 16 ) 12/10/07 01:43:30 changed by Baptiste

It doesn't block the merge since it is a newforms-admin patch.

But I am not going to write doc and tests since I haven't had any feedback about this and I have no idea if it has a chance to be accepted! If someone said it could be accepted, I would do it, sure.

(in reply to: ↑ 14 ) 12/10/07 07:25:02 changed by Karen Tracey <kmtracey@gmail.com>

Replying to Baptiste:

But I am not going to write doc and tests since I haven't had any feedback about this and I have no idea if it has a chance to be accepted! If someone said it could be accepted, I would do it, sure.

Triage stage is Accepted, and you've updated the patch to be in line with Adrian's suggestion, so I was thinking it had been accepted. I believe a complete package is more likely than just a code patch to get the review needed to move on to ready for checkin.

(in reply to: ↑ 14 ) 12/10/07 07:51:26 changed by ext

Replying to Baptiste:

But I am not going to write doc and tests since I haven't had any feedback about this and I have no idea if it has a chance to be accepted! If someone said it could be accepted, I would do it, sure.

It should be accepted. Enough? ;) Hey! This patch is very important to me. It would be great to have it merged with the trunk!

07/30/08 06:25:25 changed by mikeblake

  • attachment dynamic_related_3987.2.diff added.

Updated to revision 8151

(in reply to: ↑ description ; follow-up: ↓ 18 ) 07/30/08 06:28:59 changed by mikeblake

Replying to Baptiste <baptiste.goupil@gmail.com>:

See discussion in comments.

I think I have upgraded the patch to work with latest SVN copy. First time looking into the code!

Would greatly appreciate it if this patch was included in 1.0 as it's an incredibly handy feature.

(in reply to: ↑ 17 ) 07/30/08 06:44:22 changed by Karen Tracey <kmtracey@gmail.com>

Replying to mikeblake:

Would greatly appreciate it if this patch was included in 1.0 as it's an incredibly handy feature.

It was accepted by Adrian and seems to have multiple people interested in it, however it's got no docs nor tests. That's probably preventing it from moving along in the process.

08/01/08 09:06:40 changed by ikse hefem

  • attachment doc_diff.txt added.

tentative feature documentation

08/01/08 09:09:37 changed by ikse hefem

  • attachment doc.diff added.

New patch (sorry, fisrt try) , forget/delete the preceding one if possible

08/01/08 09:12:08 changed by ikse hefem

First proposal for a documentation. Forget the first patch, totally wrong.

08/06/08 06:27:19 changed by apollo13

  • cc changed from brosner@gmail.com, django@apolloner.eu to brosner@gmail.com, apollo13.

08/23/08 14:23:07 changed by anonymous

  • owner changed from nobody to anonymous.
  • status changed from new to assigned.

08/23/08 14:23:33 changed by anonymous

  • status changed from assigned to closed.
  • resolution set to fixed.

08/23/08 14:35:33 changed by anonymous

  • status changed from closed to reopened.
  • needs_docs deleted.
  • resolution deleted.
  • needs_tests deleted.
  • stage changed from Accepted to Ready for checkin.

08/23/08 14:56:35 changed by apollo13

  • owner changed from anonymous to nobody.
  • status changed from reopened to new.
  • stage changed from Ready for checkin to Someday/Maybe.

08/23/08 15:23:56 changed by Xniver

  • attachment diff.txt added.

updated, works with revision 8502

08/23/08 15:25:29 changed by Xniver

  • attachment dynamic.diff added.

sorry, forgot to change extension

08/24/08 05:16:41 changed by anonymous

  • status changed from new to closed.
  • resolution set to fixed.

08/24/08 05:16:55 changed by anonymous

  • status changed from closed to reopened.
  • resolution deleted.

08/24/08 05:59:58 changed by anonymous

  • status changed from reopened to closed.
  • resolution set to fixed.

08/24/08 06:12:03 changed by julien

  • status changed from closed to reopened.
  • resolution deleted.

Looks like Mr Anonymous is having a bit of fun here...

08/25/08 04:32:49 changed by anonymous

I'ld like to show in model A list_filter model B fields that have a ForeignKey to model A. I've applied dynamic_related_3987.2.diff to django svn trunk. I've add a method get_b() to the A admin class ("class AAdmin(admin.ModelAdmin?):") that returns a.b_set.all(), but I don't know how I've to include it in list_filter. I've put "list_filter = (...,'get_b')" but get the error:

Error while importing URLconf 'myapp.urls': `AAdmin.list_filter[9]` refers to field `get_b` that is missing from model `A`.

What should be solve the error?

Thanks in advance.

09/13/08 14:10:37 changed by mwdiers

  • attachment dynamic_choice_9014.diff added.

Updated patch to rev 9014. This will work with 1.0 also.

09/13/08 14:31:21 changed by mwdiers

  • cc changed from brosner@gmail.com, apollo13 to brosner@gmail.com, apollo13, mwdiers.

09/13/08 16:08:26 changed by mwdiers

There is a serious problem with this patch. It changes the formfield_for_dbfield method of ModelAdmin?, requiring a request object to passed in as a positional argument. This method is not in the docs, but it is commonly overridden, and represents a significant change to the API for the Admin.

There is a way to do what this patch does on 1.0, without any patching:

class SiteAdmin(ModelAdmin):
    def __call__(self, request, url):
        #Add in the request object, so that it may be referenced
        #later in the formfield_for_dbfield function.
        self.request = request
        return super(SiteAdmin, self).__call__(request, url)
    
    def get_form(self, request, obj=None):
        form = super(SiteAdmin, self).get_form(request, obj)
        #print form
        return form
            
    def formfield_for_dbfield(self, db_field, **kwargs):
        field = super(SiteAdmin, self).formfield_for_dbfield(db_field, **kwargs) # Get the default field
        if db_field.name == 'home_page': 
            #Add the null object
            my_choices = [('', '---------')]
            #Grab the current site id from the URL.
            my_choices.extend(Page.objects.filter(site=self.request.META['PATH_INFO'].split('/')[-2]).values_list('id','name'))
            print my_choices
            field.choices = my_choices
        return field

The trick is the override of _ _call_ _, adding the request object as an attribute of the instance. Once you have this, you can do pretty much anything you need in the formfield_for_dbfield override. Granted, this is not very elegant. But it works in 1.0, and our users are already accustomed to overriding formfield_for_dbfield to make custom form tweaks.

09/13/08 16:23:23 changed by mwdiers

  • needs_better_patch set to 1.
  • needs_tests set to 1.
  • stage changed from Someday/Maybe to Design decision needed.

Sorry. Had some superfluous code in that example. Updated version for reference:

class SiteAdmin(ModelAdmin):
    def __call__(self, request, url):
        #Add in the request object, so that it may be referenced
        #later in the formfield_for_dbfield function.
        self.request = request
        return super(SiteAdmin, self).__call__(request, url)
    
    def formfield_for_dbfield(self, db_field, **kwargs):
        field = super(SiteAdmin, self).formfield_for_dbfield(db_field, **kwargs) # Get the default field
        if db_field.name == 'home_page': 
            #Add the null object
            my_choices = [('', '---------')]
            #Grab the current site id from the URL.
            my_choices.extend(Page.objects.filter(site=self.request.META['PATH_INFO'].split('/')[-2]).values_list('id','name'))
            print my_choices
            field.choices = my_choices
        return field

01/16/09 09:32:31 changed by jacob

  • status changed from reopened to closed.
  • resolution set to fixed.

(In [9760]) Cleaned up and refactored ModelAdmin.formfield_for_dbfield:

  • The new method uses an admin configuration option (formfield_overrides); this makes custom admin widgets especially easy.
  • Refactored what was left of formfield_for_dbfield into a handful of smaller methods so that it's easier to hook in and return custom fields where needed.
  • These formfield_for_* methods now pass around request so that you can easily modify fields based on request (as in #3987).

Fixes #8306, #3987, #9148.

Thanks to James Bennet for the original patch; Alex Gaynor and Brian Rosner also contributed.


Add/Change #3987 (ModelAdmin should allow for overriding of ForeignKey/ManyToMany options)




Change Properties
Action