Skip to content

ordering on fields which use field.source #3398

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

ordering on fields which use field.source #3398

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 18 additions & 8 deletions rest_framework/filters.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,26 +156,36 @@ def get_default_ordering(self, view):
return ordering

def remove_invalid_fields(self, queryset, fields, view):
valid_fields = getattr(view, 'ordering_fields', self.ordering_fields)
ordering_fields = getattr(view, 'ordering_fields', self.ordering_fields)

if valid_fields is None:
# Default to allowing filtering on serializer fields
serializer_class = getattr(view, 'serializer_class')
if not ordering_fields == '__all__':
serializer_class = getattr(view, 'serializer_class') or view.get_serializer_class()
Copy link
Member

Choose a reason for hiding this comment

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

getattr will raise an exception (an AttributeError, usually) when the requested attribute is not found on the object. in this case, you are probably looking for something like

serializer_class = view.get_serializer_class() if hasattr(view, 'get_serializer_class) else view.serializer_class

or the rough equivalent of

serializer_class = getattr(view, 'serializer_class', None)

if serializer_class is None:
    serializer_class = view.get_serializer_class()

Then again, it may just be better to prefer view.get_serializer_class() instead of relying on the serializer_class attribute.

if serializer_class is None:
msg = ("Cannot use %s on a view which does not have either a "
"'serializer_class' or 'ordering_fields' attribute.")
raise ImproperlyConfigured(msg % self.__class__.__name__)

if ordering_fields is None:
# Default to allowing filtering on serializer field names (return field sources)
valid_fields = [
field.source or field_name
(field.source, field_name)
for field_name, field in serializer_class().fields.items()
if not getattr(field, 'write_only', False)
]
elif valid_fields == '__all__':
return [term[0] for term in valid_fields if term[0] != "*"]
elif ordering_fields == '__all__':
# View explicitly allows filtering on any model field
valid_fields = [field.name for field in queryset.model._meta.fields]
valid_fields += queryset.query.aggregates.keys()

return [term for term in fields if term.lstrip('-') in valid_fields]
return [term for term in fields if term.lstrip('-') in valid_fields]
else:
# Allow filtering on defined field name (return field sources)
valid_fields = [
(field.source, field_name)
for field_name, field in serializer_class().fields.items()
if not getattr(field, 'write_only', False)
]
return [term[0] for term in valid_fields if term[0] != "*" and term[1].lstrip('-') in fields]

def filter_queryset(self, request, queryset, view):
ordering = self.get_ordering(request, queryset, view)
Expand Down