-
Notifications
You must be signed in to change notification settings - Fork 490
Description
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:
- Create model with history.
- Create djangorestframework serializer for HistoricalModel.
- Create a ListAPIView with OrderingFilter.
- 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.