Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

djangorestframework 3.12.3 introduced bug where HistoricalModel cannot be used with OrderingFilter #821

Closed
hwalinga opened this issue Apr 24, 2021 · 0 comments · Fixed by #822

Comments

@hwalinga
Copy link
Contributor

hwalinga commented Apr 24, 2021

Describe the bug
When using a historical model in djangorestframework versions 3.12.3 and newer you cannot use OrderingFilter for the ListAPIView. If so an exception get raised in the serializer.

To Reproduce
Steps to reproduce the behavior:

  1. Create model with history.
  2. Create djangorestframework serializer for HistoricalModel.
  3. Create a ListAPIView with OrderingFilter.
  4. Head over to a link (eg /api/systems/configs-history/36) and see the error message.

example:

# urls.py

...
    path('configs-history/<int:config_id>', HistoricalConfigListAPIView.as_view(), name='configs-view'),
...

# views.py

from .serializers import HistoricalConfigSerializer
from rest_framework import filters, generics


class HistoricalConfigListAPIView(generics.ListAPIView):
    serializer_class = HistoricalConfigSerializer
    filter_backends  = [filters.OrderingFilter]

    def get_queryset(self):
        config_obj_id = self.kwargs['config_id']
        config = Config.objects.get(config_id=config_obj_id)
        return config.history.all()

Expected behavior
No exception when you do this.

Error logs:
https://i.imgur.com/J1ga93D.png
https://termbin.com/9fzh

Reason of the bug (did some debugging for you):
The internals of OrderingFilter call getattr for history_object on the historical model class. This resolves to calling __get__ on HistoricalObjectDescriptor (there is some metaprogramming checking all attributes of the class) but because it was called on the class not the instance, this method fails because it doesn't account for the case where instance is None (ie a call made on the class instead of the instance).

https://docs.python.org/3/reference/datamodel.html#object.__get__

while instance is the instance that the attribute was accessed through, or None when the attribute is accessed through the owner.

This bug got introduced in:

encode/django-rest-framework#7609

Environment (please complete the following information):

  • OS: Debian 10
  • Browser: Firefox
  • Django Simple History Version: 3.0.0
  • Django Version: 2.2
  • Database Version postgresql: 11.11

Solution:
Solution should be simple if I understood it correctly, add if instance is None: return self to the __get__ function and djangorestframework continues without errors when it calls for the history_object on the model class. I will make a PR for that.

Workaround:
Internals are only called if ordering_fields are not specified. Specify them to get rid of the bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant