Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#24198 closed Bug (invalid)

Options.get_field behavior change

Reported by: Xavier Ordoquy Owned by: nobody
Component: Database layer (models, ORM) Version: 1.8alpha1
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Up to Django 1.7 included Options.get_field would return a FieldDoesNotExist when requested a GenericForeignKey field. However Django 1.8a1 has changed that and get_field now does return it.

If you are interested in fixing this I can provide a test case against Django test suite.

Context:
I started working on the Django REST Framework port to Django 1.8a1. We do have a couple of failing test:

tests/test_relations_generic.py:104: in test_generic_fk
    self.assertEqual(serializer.data, expected)
rest_framework/serializers.py:615: in data
    ret = super(ListSerializer, self).data
rest_framework/serializers.py:212: in data
    self._data = self.to_representation(self.instance)
rest_framework/serializers.py:565: in to_representation
    self.child.to_representation(item) for item in iterable
rest_framework/serializers.py:565: in <listcomp>
    self.child.to_representation(item) for item in iterable
rest_framework/serializers.py:419: in to_representation
    fields = [field for field in self.fields.values() if not field.write_only]
rest_framework/serializers.py:312: in fields
    for key, value in self.get_fields().items():
rest_framework/serializers.py:950: in get_fields
    model_field.unique_for_date,
E   AttributeError: 'GenericForeignKey' object has no attribute 'unique_for_date'

The associated code in the serializers:

        for model_field_name, field_name in model_field_mapping.items():
            try:
                model_field = model._meta.get_field(model_field_name)
            except FieldDoesNotExist:
                continue

            unique_constraint_names |= set([
                model_field.unique_for_date,
                model_field.unique_for_month,
                model_field.unique_for_year
            ])

As for Django < 1.8, the unique_for_date will never be reached because the get_field would raise a FieldDoesNotExist.

Change History (5)

comment:1 Changed 3 years ago by Xavier Ordoquy

Summary: Options.get_field slight behavior changeOptions.get_field behavior change

comment:2 Changed 3 years ago by Tim Graham

Resolution: invalid
Status: newclosed

This is a backwards incompatible change as described in the migration guide.

comment:3 Changed 3 years ago by Xavier Ordoquy

Thanks Tim. I'm sorry I totally missed the migration guide.

comment:4 in reply to:  3 Changed 3 years ago by pirosb3

Replying to xordoquy:

Thanks Tim. I'm sorry I totally missed the migration guide.

Hi,

As Tim confirmed, the new Meta API allows GenericForeignKeys too. If you are concerned this can be an issue on your side, you can easily filter it our using the field flags. It should be the only field that has a one-to-many cardinality without a specific related_model set.
As stated in the guide, related_model can be None if the relation is generic.
Please feel free to contact me on IRC for any issues.

Dan

comment:5 Changed 3 years ago by Xavier Ordoquy

Hi Daniel,

Unfortunately I don't think it is as easy as it sounds. This happened with a GFK, but could be with any field that doesn't inherit from Field.
I didn't read the migration part because I'm not just migrating: I'm supporting Django 1.4 to 1.8. My workaround is there: https://github.com/linovia/django-rest-framework/commit/857185cf07bb539083a90bc75a6dd951da8e2206
I am not sure the field flag you are mentioning was available for 1.4

Anyway, I feel it's kind of a documentation issue but I'm not sure what's the best way to deal with this.

get_field isn't even mentioned in the release notes. I don't think it was part of the public API so I'm not sure it should even be mentioned. However, as the behavior change, I'd assume it is worth adding a notice about this in the backward incompatible change section since its behavior has changed.

Note: See TracTickets for help on using tickets.
Back to Top