deschler / django-modeltranslation

Translates Django models using a registration approach.
BSD 3-Clause "New" or "Revised" License
1.37k stars 259 forks source link

Handling objects.populate(True) in a ModelForm #172

Open benjaoming opened 11 years ago

benjaoming commented 11 years ago

Please consider including this tip in the docs as it's quite crucial to make auto-population work in ModelForms - a model form does not create new instances through objects.create, rather it calls the constructor. Thus, the save method would have to explicitly call objects.populate(True).create() to get auto population. I achieve this in the save method. It may not be a drop-in replacement since someone may modify form.instance before calling the save() method, which definitely breaks this design.

Thanks :)

def save(self, *args, **kwargs):
    if not self.instance.pk:
        extra_fields = kwargs.pop('extra_fields', {})
        fields = extra_fields
        fields.update({k: self.cleaned_data[k] for k in self._meta.fields})
        self.instance = self._meta.model.objects.populate(True).create(
            **fields
        )
    return super(CreateEventForm, self).save(*args, **kwargs)
zlorf commented 11 years ago

Well, instance constructor behaves exactly the same as objects.create in terms of auto-population and field name rewriting. So to do this in easy way:

from modelstranslation.utils import auto_populate

def save(self, *args, **kwargs):
    with auto_populate():
        return super(CreateEventForm, self).save(*args, **kwargs)

or one can even set MODELTRANSLATION_AUTO_POPULATE.

benjaoming commented 11 years ago

I tried both the context decorator and the global setting, none of them worked.

benjaoming commented 11 years ago

The model instance constructor does not invoke the model manager AFAIK. Or well, my empirical results confirm this.

zlorf commented 11 years ago

Yes, I does not invoke model manager. But auto-population actually takes place in model constructor: https://github.com/deschler/django-modeltranslation/blob/master/modeltranslation/translator.py#L152 and manager's create only wraps inner call (which, in turns, call object constructor) inside auto_populate context manager: https://github.com/deschler/django-modeltranslation/blob/master/modeltranslation/manager.py#L202

benjaoming commented 11 years ago

I've tested it on a ModelForm instance, the auto_populate decorator does not work. Global setting does not work.

BUT! I get the problem now!! :)

Yes, patch_constructor is called correctly. Problem is, that after invoking the constructor (in ModelForm.__init__), the ModelForm will then set the field values from cleaned_data (ModelForm.save). This part is not auto_populated. Hence, we need a ModelForm that creates the new instance with a method that is caught by the populator.

benjaoming commented 11 years ago

Btw thanks for the pointers, it's nice to see modeltranslation under the hood. It's solid work, I really enjoy using it!!

ghost commented 9 years ago

@benjaoming I have the same problem. Did you solve it? Can you explain your solution? Thanks!

benjaoming commented 9 years ago

@sabriinfo the work-around for a ModelForm instance is given in the original first post :)

You can use a similar method if you find that other things are not caught by the auto population.