Skip to content

insights: Fix pagination for insights in GetAll

Administrator requested to merge insights/api/pagination-fix into main

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 returning insight_series, not insight_views, so it would have limited the wrong thing there as well. So what I did was to first select insight_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.

Merge request reports

Loading