On Tue, 2019-10-15 at 17:30 -0400, Jeremy Cline wrote:
> By default, the events API orders events by date in descending order
> (newest first). However, it's useful to be able to order the events by
> oldest events first. For example, when a client is polling the events
> API for new events since a given date and wishes to process them in
> chronological order.
>
> Signed-off-by: Jeremy Cline <jcl...@redhat.com>
I'd purposefully avoided doing this initially because I wanted
'/events' to be thought of as a firehose that should be just consumed
as things were generated. We could have started deleting old events
after e.g. 4 weeks and kill pagination entirely. In hindsight though,
mistakes I made during implementation, such as the use of date-based
rather than cursor-based pagination, and the lack of webhooks or
another non-polling mechanism meant things couldn't _really_ work like
this. In addition, there's the series that aims to add an "actor" for
auditing purposes, meaning we probably should kill the idea of ever
deleting old events. So, overall, perhaps my original goal no longer
makes sense and we should just do this? Daniel - what are your
thoughts?
In any case, this unfortunately needs to be a little more complicated
than it is at the moment. Notes below.
> ---
> patchwork/api/event.py | 2 +-
> patchwork/tests/api/test_event.py | 18 ++++++++++++++++++
> ...-order-events-by-date-7484164761c5231b.yaml | 5 +++++
> 3 files changed, 24 insertions(+), 1 deletion(-)
> create mode 100644
> releasenotes/notes/api-order-events-by-date-7484164761c5231b.yaml
>
> diff --git a/patchwork/api/event.py b/patchwork/api/event.py
> index c0d973d..e6d467d 100644
> --- a/patchwork/api/event.py
> +++ b/patchwork/api/event.py
> @@ -77,7 +77,7 @@ class EventList(ListAPIView):
> serializer_class = EventSerializer
> filter_class = filterset_class = EventFilterSet
> page_size_query_param = None # fixed page size
> - ordering_fields = ()
> + ordering_fields = ('date',)
This is going to apply to all API versions, from v1.0 to v1.2. However,
we actually want it to only apply to v1.2, just so API v1.0 behaves the
exact same on a Patchwork v2.0 instance as it does on a v2.2 instance.
I don't know if we've done versioning on fields before, but it should
be easy to override whatever method in 'ListAPIView' is responsible for
consuming 'ordering_field' from the querystring to ignore 'date' if API
version < 1.2. Let me know if you need help here.
> ordering = '-date'
>
> def get_queryset(self):
> diff --git a/patchwork/tests/api/test_event.py
> b/patchwork/tests/api/test_event.py
> index 8816538..bff8f40 100644
> --- a/patchwork/tests/api/test_event.py
> +++ b/patchwork/tests/api/test_event.py
> @@ -149,6 +149,24 @@ class TestEventAPI(utils.APITestCase):
> resp = self.client.get(self.api_url(), {'series': 999999})
> self.assertEqual(0, len(resp.data))
>
> + def test_order_by_date_default(self):
> + """Assert the default ordering is by date descending."""
> + self._create_events()
> +
> + resp = self.client.get(self.api_url())
> + events = Event.objects.order_by("-date").all()
> + for api_event, event in zip(resp.data, events):
> + self.assertEqual(api_event["id"], event.id)
> +
> + def test_order_by_date_ascending(self):
> + """Assert the default ordering is by date descending."""
> + self._create_events()
> +
> + resp = self.client.get(self.api_url(), {'order': 'date'})
> + events = Event.objects.order_by("date").all()
> + for api_event, event in zip(resp.data, events):
> + self.assertEqual(api_event["id"], event.id)
> +
> def test_create(self):
> """Ensure creates aren't allowed"""
> user = create_maintainer()
> diff --git
> a/releasenotes/notes/api-order-events-by-date-7484164761c5231b.yaml
> b/releasenotes/notes/api-order-events-by-date-7484164761c5231b.yaml
> new file mode 100644
> index 0000000..5d5328d
> --- /dev/null
> +++ b/releasenotes/notes/api-order-events-by-date-7484164761c5231b.yaml
> @@ -0,0 +1,5 @@
> +---
> +features:
We have an 'api' section for this stuff which should be used here.
> + - |
> + Allow ordering events from the events API by date. This can be done by
> + adding ``order=date`` or ``order=-date`` (the default) parameters.
_______________________________________________
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
