jrief / django-angular

Let AngularJS play well with Django
http://django-angular.awesto.com/
MIT License
1.23k stars 294 forks source link

When using Meta to create forms, djng.fields.ModelChoiceField isn't substituted for Django's ModelChoiceField #309

Closed asnoeyink closed 7 years ago

asnoeyink commented 7 years ago

TL;DR: Django-Angular's field substitution doesn't work on model forms for ModelChoiceField, and using getattr() causes an Attribute error for fields that don't have choices .

Say I have a model and a form like this:

class SomeModel(models.model):
    field_1 = models.CharField()
    field_2 = models.ForeignKey(SomeOtherModel)

class SomeForm(NgModelFormMixin, Bootstrap3Form):
    class Meta:
        model = SomeModel
        fields = ('field_1', 'field_2',)

Django creates a form based off of the fields in SomeForm's Meta class. It chooses form fields equivalent to the model fields (models.CharField -> forms.CharField and models.ForeignKey -> forms.ModelChoiceField). Django-Angular is supposed to substitute these Django form fields for its own form fields from djng.forms.fields. This works for all fields except ModelChoiceField, which causes an ImproperlyConfigured error.

Currently, in angular_base.py, formfield_callback is overridden to update formfield() with Django-Angular's new fields:

# recreate the formfield using our customized field class
    if getattr(formfield, 'choices', None):
        kwargs.update(choices_form_class=form_class)
    else:
        kwargs.update(form_class=form_class)
formfield = modelfield.formfield(**kwargs)

This code caused two errors for me: I first had to change getattr() to hasattr() in order to stop this code from hitting the database, which was causing any field that didn't have choices to error (AttributeError: 'CharField' object has no attribute 'choices').

Then I was able to fix the ImproperlyConfigured error for ModelChoiceFields by removing the else. I think this fixes the issue because Django's formfield() checks if the field has any choices. Since it was no longer hitting the database, the field did not have choices (just an empty ModelChoiceIterator, so Django skipped the block of code where it assigns choices_form_class to form_class. Removing the else allows Django-Angular's ModelChoiceField to replace Django's ModelChoiceField whether choices exist during the field's __init__ or not.

So the lines above end up like this:

# recreate the formfield using our customized field class
    if hasattr(formfield, 'choices', None):
        kwargs.update(choices_form_class=form_class)
    kwargs.update(form_class=form_class)
formfield = modelfield.formfield(**kwargs)

Is this the correct way to fix this, or am I misunderstanding the way either Django or Django-Angular handle this? I apologize in advance if any of this was unclear.

jrief commented 7 years ago

You are probably right on this. After the refactoring for 1.0 I only used the unit tests for Form, since ModelForm fields where adjusted automatically. Since this since in 1.0 has changed to the way Django intends doing it, that mapping might have to be adjusted. So feel free to send me a pull request. Thanks for reporting this problem.