Skip to content
Snippets Groups Projects

insights: Fix pagination for insights in GetAll

Merged 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

Merged by avatar (Jul 7, 2025 4:39am UTC)

Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
Please register or sign in to reply
Loading