Django

Code

Ticket #10349 (closed: fixed)

Opened 1 year ago

Last modified 11 months ago

ManyToMany field does not take dynamic initial values into account

Reported by: fas Assigned to: fas
Milestone: 1.1 Component: Database layer (models, ORM)
Version: 1.0 Keywords: ManyToMany, initial, dynamic, lambda
Cc: mbencun@gmail.com Triage Stage: Accepted
Has patch: 1 Needs documentation: 0
Needs tests: 1 Patch needs improvement: 0

Description

For dbfields it is possible to provide the initial value in form of a anonymous function, e.g. models.CharField?(..., initial=lambda: "I am dynamic!"). The formfield method of the ManyToMany? field does not take this into account:

defaults['initial'] = [i._get_pk_val() for i in defaults['initial']]

The fix is simple and provided in a patch, diffed against release 1.0.2.

Attachments

manytomany_dynamic_initial_against_1.0.2.diff (0.8 kB) - added by fas on 02/24/09 12:46:20.
simple fix

Change History

02/24/09 12:46:20 changed by fas

  • attachment manytomany_dynamic_initial_against_1.0.2.diff added.

simple fix

02/25/09 13:51:44 changed by

  • milestone deleted.

Milestone post-1.0 deleted

02/28/09 13:10:31 changed by jacob

  • needs_better_patch changed.
  • needs_docs changed.
  • stage changed from Unreviewed to Accepted.
  • needs_tests changed.
  • milestone set to 1.1.

In the future, please provide patches against trunk if possible. We backport patches from trunk to the release branches.

04/13/09 01:08:20 changed by russellm

  • needs_tests set to 1.

It would also be helpful if you provided a testcase as part of your patch - or, at the very least, an example of the usage that is corrected by applying the patch. It isn't immediately obvious to me what problem this patch is trying to solve.

04/19/09 09:09:59 changed by fas

  • cc set to mbencun@gmail.com.

When using Field.formfield you can pass a lambda function as the initial value which will be passed over to the form field. When rendering the form field, the initial value will be evaluated in case it is a function. This is done in forms/forms.py, method as_widget.

The ManyToManyField? however overwrites the formfield method in order to set its initial value to be a list of IDs. It assumes that the given initial value is a list of related objects. It also assumes that the form class to be used is forms.ModelMultipleChoiceField? or that the form field to be used takes a list of IDs. Both assumptions are incorrect.

    def formfield(self, **kwargs):
        defaults = {'form_class': forms.ModelMultipleChoiceField, 'queryset': self.rel.to._default_manager.complex_filter(self.rel.limit_choices_to)}
        defaults.update(kwargs)
        # If initial is passed in, it's a list of related objects, but the                                                                                                          
        # MultipleChoiceField takes a list of IDs.                                                                                                                                  
        if defaults.get('initial') is not None:
            defaults['initial'] = [i._get_pk_val() for i in defaults['initial']]
        return super(ManyToManyField, self).formfield(**defaults)

This makes it impossible to pass a function for the initial value and also makes it impossible to create a custom form field that needs the related object and not only its IDs. I revoke my initial patch and propose that we should find a solution that works without the two assumptions made above. I think ModelChoiceField? (forms/models.py) might the the right place. Applying the same transformation (object->ID) there before passing the initial to the super (ChoiceField?) should solve the problem.

Any suggestions?

04/25/09 07:04:58 changed by russellm

My suggestion is that you provide a test case, like I asked you to.

I completely understand what the code is trying to do. I can see the analog with what happens with normal form fields. On the face of it, the patch looks fine. However, I can't come up with a single code example that would actually exercise the patch you have provided.

If you provide a callable to the initial argument when instantiating a form, the callable is evaluated. If you define a form field with a callable initial value, it is evaluated.

Don't just explain your patch with words - you need to show me an example of code where you can provide a callable for an m2m field that doesn't work with the current Django trunk, but does work when you apply your patch. Ideally, that example would be integrated into the existing Django test suite, but at this point, I'd take any example, standalone or otherwise.

04/25/09 11:25:10 changed by fas

Ok, here is an example. We have a many-to-many relation between articles and publications.

from django.db import models

class Publication(models.Model):
    title = models.CharField(max_length=30)
    date = models.DateField()

class Article(models.Model):
    headline = models.CharField(max_length=100)
    publications = models.ManyToManyField(Publication)

Then we create a model form that has a callback for the form field. Such a callback is used in the admin. In the following example, the initial value of 'publications' of the article form should be the ten latest publications.

from django.forms.models import modelform_factory

def formfield_for_dbfield(db_field, **kwargs):
    if db_field.name == 'publications':
        kwargs['initial'] = lambda: Publication.objects.all().order_by('-date')[:10]
    return db_field.formfield(**kwargs)

modelform_factory(Article, formfield_callback=formfield_for_dbfield)

A lambda expression is used so that the expression will not be evaluated once but every time, preventing the initial selection from getting outdated (same reason you would use date.today instead of date.today() as an initial value for a date field).

05/02/09 02:03:34 changed by russellm

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

(In [10652]) Fixed #10349 -- Modified ManyToManyFields? to allow initial form values to be callables. Thanks to fas for the report and patch.

05/02/09 02:16:31 changed by russellm

(In [10653]) [1.0.X] Fixed #10349 -- Modified ManyToManyFields? to allow initial form values to be callables. Thanks to fas for the report and patch.

Merge of r10652 from trunk.


Add/Change #10349 (ManyToMany field does not take dynamic initial values into account)




Change Properties
Action