GeeWee / django-auto-prefetching

Automatic prefetching for Django
MIT License
231 stars 18 forks source link

fix: don't auto prefetch properties #33

Closed hemache closed 3 years ago

hemache commented 3 years ago

Avoid prefetching fields (using select_related) where Serializer.source param points to a property and not an actual field or field descriptor Error

Example

class User(Model, ...):
  @property
   def primary_phone_number(self):
     return self.phone_numbers.first()

class UserSerializer(...):
  primary_phone_number = PhoneNumberSerializer(read_only=True)

class UserViewSet(AutoPrefetchViewSetMixin, ...):
  pass
Results
FieldError at /api/users/
Invalid field name(s) given in select_related: 'primary_phone_number'. Choices are: profile
GeeWee commented 3 years ago

I'd like to merge this in - can you update the PR description to give a little more information so that the git changelog will reflect exactly what the change does?

hemache commented 3 years ago

@GeeWee done :pray:

GeeWee commented 3 years ago

Turns out that a lot of the previous tests fail (check the CI) - I have a feeling that your change probably breaks a bunch of stuff that you didn't mean for it to. I'll need you to either change the code, or demonstrate why the tests are broken/fix them to merge this in :)

hemache commented 3 years ago

fixed failing tests, could you please recheck now?

hemache commented 3 years ago

Hey @GeeWee, any updates?

GeeWee commented 3 years ago

Yeah sorry, I expect to merge this in and publish a new version during the weekend :)

GeeWee commented 3 years ago

0.1.10 released with this PR. Thanks for the patience! :champagne: