From a163c512386321d3311ee3cd603f25b04854f12e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20Faruk=20Abac=C4=B1?= Date: Thu, 22 Oct 2020 19:47:34 +0300 Subject: [PATCH 1/3] Add failing tests for ordering filter with model property --- tests/test_filters.py | 51 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/tests/test_filters.py b/tests/test_filters.py index 567e5f83fc..37ae4c7cf3 100644 --- a/tests/test_filters.py +++ b/tests/test_filters.py @@ -424,6 +424,10 @@ class OrderingFilterModel(models.Model): title = models.CharField(max_length=20, verbose_name='verbose title') text = models.CharField(max_length=100) + @property + def description(self): + return self.title + ": " + self.text + class OrderingFilterRelatedModel(models.Model): related_object = models.ForeignKey(OrderingFilterModel, related_name="relateds", on_delete=models.CASCADE) @@ -436,6 +440,17 @@ class Meta: fields = '__all__' +class OrderingFilterSerializerWithModelProperty(serializers.ModelSerializer): + class Meta: + model = OrderingFilterModel + fields = ( + "id", + "title", + "text", + "description" + ) + + class OrderingDottedRelatedSerializer(serializers.ModelSerializer): related_text = serializers.CharField(source='related_object.text') related_title = serializers.CharField(source='related_object.title') @@ -551,6 +566,42 @@ class OrderingListView(generics.ListAPIView): {'id': 1, 'title': 'zyx', 'text': 'abc'}, ] + def test_ordering_without_ordering_fields(self): + class OrderingListView(generics.ListAPIView): + queryset = OrderingFilterModel.objects.all() + serializer_class = OrderingFilterSerializerWithModelProperty + filter_backends = (filters.OrderingFilter,) + ordering = ('title',) + + view = OrderingListView.as_view() + + # Model field ordering works fine. + request = factory.get('/', {'ordering': 'text'}) + response = view(request) + assert response.data == [ + {'id': 1, 'title': 'zyx', 'text': 'abc', 'description': 'zyx: abc'}, + {'id': 2, 'title': 'yxw', 'text': 'bcd', 'description': 'yxw: bcd'}, + {'id': 3, 'title': 'xwv', 'text': 'cde', 'description': 'xwv: cde'}, + ] + + # `incorrectfield` ordering works fine. + request = factory.get('/', {'ordering': 'foobar'}) + response = view(request) + assert response.data == [ + {'id': 3, 'title': 'xwv', 'text': 'cde', 'description': 'xwv: cde'}, + {'id': 2, 'title': 'yxw', 'text': 'bcd', 'description': 'yxw: bcd'}, + {'id': 1, 'title': 'zyx', 'text': 'abc', 'description': 'zyx: abc'}, + ] + + # `description` is a Model property, which should be ignored. + request = factory.get('/', {'ordering': 'description'}) + response = view(request) + assert response.data == [ + {'id': 3, 'title': 'xwv', 'text': 'cde', 'description': 'xwv: cde'}, + {'id': 2, 'title': 'yxw', 'text': 'bcd', 'description': 'yxw: bcd'}, + {'id': 1, 'title': 'zyx', 'text': 'abc', 'description': 'zyx: abc'}, + ] + def test_default_ordering(self): class OrderingListView(generics.ListAPIView): queryset = OrderingFilterModel.objects.all() From 62113a794a638c652e06ce74e51a50bc84d99e5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20Faruk=20Abac=C4=B1?= Date: Thu, 22 Oct 2020 19:48:14 +0300 Subject: [PATCH 2/3] Fix get_default_valid_fields of OrderingFilter --- rest_framework/filters.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/rest_framework/filters.py b/rest_framework/filters.py index 3665775195..e2aad42722 100644 --- a/rest_framework/filters.py +++ b/rest_framework/filters.py @@ -226,10 +226,17 @@ def get_default_valid_fields(self, queryset, view, context={}): ) raise ImproperlyConfigured(msg % self.__class__.__name__) + model_field_names = [field.name for field in queryset.model._meta.fields] + return [ (field.source.replace('.', '__') or field_name, field.label) for field_name, field in serializer_class(context=context).fields.items() - if not getattr(field, 'write_only', False) and not field.source == '*' + if ( + not getattr(field, 'write_only', False) and + not field.source == '*' and ( + field_name in model_field_names or field.source in model_field_names + ) + ) ] def get_valid_fields(self, queryset, view, context={}): From c7f8b1456adb9cda56096b4a13bec94efadf0fbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20Faruk=20Abac=C4=B1?= Date: Thu, 11 Mar 2021 13:40:05 +0300 Subject: [PATCH 3/3] Filter model properties in get_default_valid_fields of OrderingFilter --- rest_framework/filters.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/rest_framework/filters.py b/rest_framework/filters.py index e2aad42722..1ffd9edc02 100644 --- a/rest_framework/filters.py +++ b/rest_framework/filters.py @@ -226,16 +226,19 @@ def get_default_valid_fields(self, queryset, view, context={}): ) raise ImproperlyConfigured(msg % self.__class__.__name__) - model_field_names = [field.name for field in queryset.model._meta.fields] + model_class = queryset.model + model_property_names = [ + # 'pk' is a property added in Django's Model class, however it is valid for ordering. + attr for attr in dir(model_class) if isinstance(getattr(model_class, attr), property) and attr != 'pk' + ] return [ (field.source.replace('.', '__') or field_name, field.label) for field_name, field in serializer_class(context=context).fields.items() if ( not getattr(field, 'write_only', False) and - not field.source == '*' and ( - field_name in model_field_names or field.source in model_field_names - ) + not field.source == '*' and + field.source not in model_property_names ) ]