Paginate Code Scanning alerts instead of returning only the first page

pkg/github/codescanning.go

@@ -156,32 +156,42 @@ func (alerts CodeScanningWrapper) Frames() data.Frames {
 // https://docs.github.com/en/rest/reference/code-scanning#get-a-list-of-code-scanning-alerts-for-a-repository
 func GetCodeScanningAlerts(context context.Context, c models.Client, opt models.CodeScanningOptions, from time.Time, to time.Time) (CodeScanningWrapper, error) {
 	var alerts []*googlegithub.Alert
-	var err error
 
-	// if there is no repository provided show alerts in organization level
-	if opt.Repository == "" {
-		alerts, _, err = c.ListAlertsForOrg(
-			context,
-			opt.Owner,
-			&googlegithub.AlertListOptions{
-				State: opt.State,
-				Ref:   opt.Ref,
-			},
-		)
-	} else {
-		alerts, _, err = c.ListAlertsForRepo(
-			context,
-			opt.Owner,
-			opt.Repository,
-			&googlegithub.AlertListOptions{
-				State: opt.State,
-				Ref:   opt.Ref,
-			},
-		)
+	listOpts := &googlegithub.AlertListOptions{
+		State: opt.State,
+		Ref:   opt.Ref,
 	}
+	// Use offset pagination with a large page size. ListOptions is embedded
+	// explicitly (AlertListOptions also embeds ListCursorOptions) so Page/PerPage
+	// are unambiguous.
+	listOpts.ListOptions.PerPage = 100
+
+	page := 1
+	for page != 0 {
+		listOpts.ListOptions.Page = page
+
+		var (
+			pageAlerts []*googlegithub.Alert
+			resp       *googlegithub.Response
+			err        error
+		)
+
+		// if there is no repository provided show alerts in organization level
+		if opt.Repository == "" {
+			pageAlerts, resp, err = c.ListAlertsForOrg(context, opt.Owner, listOpts)
+		} else {
+			pageAlerts, resp, err = c.ListAlertsForRepo(context, opt.Owner, opt.Repository, listOpts)
+		}
+		if err != nil {
+			return nil, err
+		}
+
+		alerts = append(alerts, pageAlerts...)
 
-	if err != nil {
-		return nil, err
+		if resp == nil || resp.NextPage == 0 {
+			break
+		}
+		page = resp.NextPage
 	}
 
 	return CodeScanningWrapper(alerts), nil

pkg/github/codescanning_test.go

@@ -11,12 +11,35 @@ import (
 	"github.com/grafana/github-datasource/pkg/models"
 )
 
+type mockAlertPage struct {
+	alerts   []*googlegithub.Alert
+	nextPage int
+}
+
 type mockClient struct {
-	mockAlerts    []*googlegithub.Alert
-	mockResponse  *googlegithub.Response
-	expectedOwner string
-	expectedRepo  string
-	t             *testing.T
+	mockAlerts   []*googlegithub.Alert
+	mockResponse *googlegithub.Response
+	// pages, when set, makes successive ListAlertsFor* calls return successive
+	// pages so pagination can be exercised. requestedPages records the Page value
+	// requested on each call.
+	pages          []mockAlertPage
+	callCount      int
+	requestedPages []int
+	expectedOwner  string
+	expectedRepo   string
+	t              *testing.T
+}
+
+// nextAlertPage returns the alerts and response for the current call when
+// pagination is being simulated, or nil to fall back to mockAlerts/mockResponse.
+func (m *mockClient) nextAlertPage(opts *googlegithub.AlertListOptions) ([]*googlegithub.Alert, *googlegithub.Response, bool) {
+	if len(m.pages) == 0 {
+		return nil, nil, false
+	}
+	m.requestedPages = append(m.requestedPages, opts.ListOptions.Page)
+	page := m.pages[m.callCount]
+	m.callCount++
+	return page.alerts, &googlegithub.Response{NextPage: page.nextPage}, true
 }
 
 func (m *mockClient) Query(ctx context.Context, q interface{}, variables map[string]interface{}) error {
@@ -40,6 +63,9 @@ func (m *mockClient) ListAlertsForRepo(ctx context.Context, owner, repo string,
 		m.t.Errorf("Expected owner/repo to be %s/%s, got %s/%s", m.expectedOwner, m.expectedRepo, owner, repo)
 	}
 
+	if alerts, resp, ok := m.nextAlertPage(opts); ok {
+		return alerts, resp, nil
+	}
 	return m.mockAlerts, m.mockResponse, nil
 }
 
@@ -49,6 +75,9 @@ func (m *mockClient) ListAlertsForOrg(ctx context.Context, owner string, opts *g
 		m.t.Errorf("Expected owner to be %s, got %s", m.expectedOwner, owner)
 	}
 
+	if alerts, resp, ok := m.nextAlertPage(opts); ok {
+		return alerts, resp, nil
+	}
 	return m.mockAlerts, m.mockResponse, nil
 }
 
@@ -193,3 +222,78 @@ func TestCodeScanningWrapperFrames(t *testing.T) {
 		t.Errorf("Expected %d fields, got %d", expectedFields, len(frame.Fields))
 	}
 }
+
+// helper to build n alerts with distinct numbers
+func makeAlerts(n int) []*googlegithub.Alert {
+	alerts := make([]*googlegithub.Alert, n)
+	for i := range alerts {
+		num := i + 1
+		alerts[i] = &googlegithub.Alert{Number: &num}
+	}
+	return alerts
+}
+
+// Regression test for https://github.com/grafana/github-datasource/issues/773:
+// Code Scanning alerts must be paginated, not capped at the first page.
+func TestGetCodeScanningAlertsPagination(t *testing.T) {
+	ctx := context.Background()
+	from := time.Now().Add(-30 * 24 * time.Hour)
+	to := time.Now()
+
+	t.Run("repository walks all pages", func(t *testing.T) {
+		client := &mockClient{
+			expectedOwner: "grafana",
+			expectedRepo:  "grafana",
+			t:             t,
+			pages: []mockAlertPage{
+				{alerts: makeAlerts(3), nextPage: 2},
+				{alerts: makeAlerts(3), nextPage: 3},
+				{alerts: makeAlerts(2), nextPage: 0},
+			},
+		}
+		opts := models.CodeScanningOptions{Owner: "grafana", Repository: "grafana"}
+
+		alerts, err := GetCodeScanningAlerts(ctx, client, opts, from, to)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if len(alerts) != 8 {
+			t.Errorf("expected 8 alerts across 3 pages, got %d", len(alerts))
+		}
+		if want := []int{1, 2, 3}; !equalInts(client.requestedPages, want) {
+			t.Errorf("expected requested pages %v, got %v", want, client.requestedPages)
+		}
+	})
+
+	t.Run("organization walks all pages", func(t *testing.T) {
+		client := &mockClient{
+			expectedOwner: "grafana",
+			t:             t,
+			pages: []mockAlertPage{
+				{alerts: makeAlerts(3), nextPage: 2},
+				{alerts: makeAlerts(1), nextPage: 0},
+			},
+		}
+		opts := models.CodeScanningOptions{Owner: "grafana"} // no repository -> org
+
+		alerts, err := GetCodeScanningAlerts(ctx, client, opts, from, to)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if len(alerts) != 4 {
+			t.Errorf("expected 4 alerts across 2 pages, got %d", len(alerts))
+		}
+	})
+}
+
+func equalInts(a, b []int) bool {
+	if len(a) != len(b) {
+		return false
+	}
+	for i := range a {
+		if a[i] != b[i] {
+			return false
+		}
+	}
+	return true
+}