Stephen Finucane <step...@that.guru> writes:
> 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?
I think date ordering is probably fine. We can try again for
cursor-based pagination and so on in a future version of the API.
FWIW, I think having an undocumented field in old versions of the API is
probably not the end of the world.
Regards,
Daniel
> 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
