insights: Fix pagination for insights in GetAll
Created by: CristinaBirkel
Closes #29576 (closed)
Description
I believe there were a few issues here:
- The
LIMIT
clause in the query was limiting results before filtering by permissions. This resulted in fewer results than expected, depending on ordering and permissions. - The issue with simply moving the
LIMIT
clause to the end was that this query is returninginsight_series
, notinsight_views
, so it would have limited the wrong thing there as well. So what I did was to first selectinsight_views
, and then join with the other tables necessary to return all the columns we wanted. - The
pageInfo
wasn't working well for the last page because it returned a cursor whenever there were results. -
GroupByView
was ordering by id instead of unique_id as well, which needs to be consistent for this to work.
Ideally I would like to refactor this more, but given the scope of the bug as I did what felt like the minimum viable change.
Testing Steps
- I tested this out via the API. I made sure to include a lot of "permissions gaps" in the data, (lots of insights my user doesn't have permissions to view locally,) and then tested the paginated results against what I expected.
- Also added some unit tests for
GetAll
which focus on pagination
Let me know if you see anything unexpected! I am a bit concerned modifying this complex query, but it all seems to be working from what I tested.