Conversation
|
Cool, I tried to add the OrderingFilter backend to our entire app last night (like what we do for the Filter backend) but I couldn't get it to work with the paginated endpoints. I think we may want that eventually but this works for now. |
| @@ -177,6 +178,8 @@ class RepositoryVersionViewSet(NamedModelViewSet, | |||
| serializer_class = RepositoryVersionSerializer | |||
| queryset = RepositoryVersion.objects.exclude(complete=False) | |||
| filter_class = RepositoryVersionFilter | |||
| filter_backends = (OrderingFilter,) | |||
| ordering = ('-number',) | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like this should count up and not down. But then again I guess if this is eventually paginated, you'd want to see the latest ones first?
| @@ -42,6 +43,8 @@ class TaskViewSet(NamedModelViewSet, | |||
| 'list': MinimalTaskSerializer, | |||
| 'default': TaskSerializer | |||
| } | |||
| filter_backends = (OrderingFilter,) | |||
| ordering = ('-created') | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit hesitant to sort by a field that's not visible to the end user. I'm wondering if we should expose created or maybe sort by started_at?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm also cool with just leaving this as is so I'll go ahead and approve.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking that since started_at can be None, it's better to avoid any potential edge cases by using creation instead.
If you've got a cancelled task, I would rather it showed up in the list in the correct order rather than at one end or another.
|
I tested this out and it works great 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
closes: 3576
https://pulp.plan.io/issues/3576