OrderingFilter backend checks whether a view specifies an
ordering_fields orserializer_class attribute. Views can however
overrideget_serializer_class to determine the serializer class
but theOrderingFilter doesn't catch this. This patch fixes that.

Here's an example of a view that would benefit from this patch:

classMyView(generics.ListAPIView):queryset=MyModel.objects.all()filter_backends= (filters.OrderingFilter, )defget_serializer_class(self):returnMySerializer

This would formerly raise anImproperlyConfigured error.

xordoquy reviewed

Oct 9, 2015

View reviewed changes

tests/test_filters.py Outdated

Copy link

Contributor

xordoquyOct 9, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Do you know why it doesn't ?

Copy link

ContributorAuthor

kmwenjaOct 9, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

@xordoquy Sorry, I realized that theget_serializer_class method actually has its own assert to check forserializer_class:https://github.com/tomchristie/django-rest-framework/blob/master/rest_framework/generics.py#L123.

On that note, is it better to get the error fromget_serializer_class (AssertionError) or fromremove_invalid_fields (ImproperlyConfigured)? I can rewrite the patch to either catch theAssertionError inget_serializer_class or let it pass right up.

xordoquy added the Enhancement label

Oct 9, 2015

xordoquy added this to the3.3.0 Release milestone

Oct 9, 2015

Copy link

ContributorAuthor

kmwenja commentedOct 9, 2015

@xordoquy I preferred raisingImproperlyConfigured (since that was the initial intended purpose) but I can quickly change that.

Copy link

ContributorAuthor

kmwenja commentedOct 9, 2015

@xordoquy would it be presumptuous to update the docs at this point?

Copy link

Contributor

xordoquy commentedOct 9, 2015

We'll take some time to review it first.

lovelydinosaur modified the milestones:3.3.0 Release,3.3.1 Release

Oct 28, 2015

lovelydinosaur modified the milestones:3.3.1 Release,3.3.2 Release

Nov 4, 2015

xordoquy modified the milestones:3.3.2 Release,3.3.3 Release

Dec 14, 2015

xordoquy modified the milestones:3.3.3 Release,3.3.4 Release,3.4.0 Release

Feb 11, 2016

kmwenja added3 commits

May 26, 2016 07:59

Use get_serializer_class in ordering filter

4bcc8cd

OrderingFilter backend checks whether a view specifies anordering fields or serializer_class attribute. Views can howeveroverride get_serializer_class to determine the serializer_classbut the OrderingFilter doesn't catch this. This patch fixes that.

Test improper configuration of ordering

af22135

just checking that the ImproperlyConfigured error is raised ifordering_fields, serializer_class and get_serializer_class arenot provided in the view.

Raise ImproperlyConfigured when missing serializer_class

bf498b2

get_serializer_class raises an AssertionError if no serializer_classis found (either as a class attribute or from an overriding method).This commit catches it and raises ImproperlyConfigured instead.

kmwenja force-pushed theuse-get-serializer-class-in-ordering-filter branch fromc655de2 tobf498b2Compare

May 26, 2016 05:38

Copy link

codecov-io commentedMay 26, 2016•
edited
Loading

Current coverage is91.43%

Merging#3487 intomaster will increase coverage by<.01%

@@             master   #3487   diff @@=======================================  Files            51      49     -2     Lines          5476    5356   -120     Methods           0       0            Branches          0       0          =======================================- Hits           5005    4897   -108+ Misses          471     459    -12  Partials          0       0