insights: allow to save the default number of series samples

dev/gqltest/code_insights_test.go

@@ -559,6 +559,75 @@ func TestUpdateInsight(t *testing.T) {
 		}
 	})
 
+	t.Run("default filters are saved on update", func(t *testing.T) {
+		repos := []string{"repo1"}
+		intervalUnit := "MONTH"
+		intervalValue := 4
+		dataSeries := map[string]any{
+			"query": "lang:css",
+			"options": map[string]string{
+				"label":     "insights",
+				"lineColor": "#6495ED",
+			},
+		}
+		repoScope := map[string]any{
+			"repositories": repos,
+		}
+		timeScope := map[string]any{
+			"stepInterval": map[string]any{
+				"unit":  intervalUnit,
+				"value": intervalValue,
+			},
+		}
+		insight, err := client.CreateSearchInsight("my gqltest insight", dataSeries, repoScope, timeScope)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if insight.InsightViewId == "" {
+			t.Fatal("Did not get an insight view ID")
+		}
+		defer func() {
+			if err := client.DeleteInsightView(insight.InsightViewId); err != nil {
+				t.Fatalf("couldn't disable insight series: %v", err)
+			}
+		}()
+
+		if insight.Label != "insights" {
+			t.Errorf("wrong label: %v", insight.Label)
+		}
+		if insight.Color != "#6495ED" {
+			t.Errorf("wrong color: %v", insight.Color)
+		}
+
+		dataSeries["seriesId"] = insight.SeriesId
+		dataSeries["options"] = map[string]any{
+			"label":     "insights 2",
+			"lineColor": "green",
+		}
+
+		var numSamples int32 = 32
+		updatedInsight, err := client.UpdateSearchInsight(insight.InsightViewId, map[string]any{
+			"dataSeries": []any{
+				dataSeries,
+			},
+			"presentationOptions": map[string]string{},
+			"viewControls": map[string]any{
+				"filters": struct{}{},
+				"seriesDisplayOptions": map[string]int32{
+					"numSamples": numSamples,
+				},
+			},
+			"repositoryScope": repoScope,
+			"timeScope":       timeScope,
+		})
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		if updatedInsight.NumSamples != numSamples {
+			t.Errorf("wrong number of samples: %d", updatedInsight.NumSamples)
+		}
+	})
 }
 
 func TestSaveInsightAsNewView(t *testing.T) {

enterprise/cmd/frontend/internal/insights/resolvers/insight_series_resolver.go

@@ -324,7 +324,13 @@ func getRecordedSeriesPointOpts(ctx context.Context, db database.DB, timeseriesS
 	opts.ID = &definition.InsightSeriesID
 	opts.SupportsAugmentation = definition.SupportsAugmentation
 
-	oldest, err := timeseriesStore.GetOffsetNRecordingTime(ctx, definition.InsightSeriesID, options.NumSamples, false)
+	// by this point the numSamples option should be set correctly but we're reusing the same struct across functions
+	// so set max again.
+	numSamples := 90
+	if options.NumSamples != nil && *options.NumSamples < 90 && *options.NumSamples > 0 {
+		numSamples = int(*options.NumSamples)
+	}
+	oldest, err := timeseriesStore.GetOffsetNRecordingTime(ctx, definition.InsightSeriesID, numSamples, false)
 	if err != nil {
 		return nil, errors.Wrap(err, "GetOffsetNRecordingTime")
 	}

enterprise/cmd/frontend/internal/insights/resolvers/insight_series_resolver_test.go

@@ -11,6 +11,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/sourcegraph/log/logtest"
+
 	"github.com/sourcegraph/sourcegraph/internal/api"
 
 	"github.com/sourcegraph/sourcegraph/cmd/frontend/graphqlbackend"
@@ -359,7 +360,7 @@ func Test_NumSamplesFiltering(t *testing.T) {
 
 	tests := []struct {
 		name       string
-		numSamples int
+		numSamples int32
 	}{
 		{
 			name:       "one",
@@ -380,10 +381,10 @@ func Test_NumSamplesFiltering(t *testing.T) {
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			points, err := fetchSeries(ctx, types.InsightViewSeries{SeriesID: series.SeriesID, InsightSeriesID: series.ID}, types.InsightViewFilters{}, types.SeriesDisplayOptions{NumSamples: test.numSamples}, &base)
+			points, err := fetchSeries(ctx, types.InsightViewSeries{SeriesID: series.SeriesID, InsightSeriesID: series.ID}, types.InsightViewFilters{}, types.SeriesDisplayOptions{NumSamples: &test.numSamples}, &base)
 			require.NoError(t, err)
 
-			assert.Equal(t, test.numSamples, len(points))
+			assert.Equal(t, int(test.numSamples), len(points))
 			t.Log(points)
 			for i := range points {
 				assert.Equal(t, times[len(points)-i-1].Timestamp, points[i].Time)

enterprise/cmd/frontend/internal/insights/resolvers/insight_view_resolvers.go

@@ -103,9 +103,7 @@ func (i *insightViewSeriesDisplayOptionsResolver) SortOptions(ctx context.Contex
 }
 
 func (i *insightViewSeriesDisplayOptionsResolver) NumSamples() *int32 {
-	v := int32(i.seriesDisplayOptions.NumSamples)
-	return &v
-
+	return i.seriesDisplayOptions.NumSamples
 }
 
 type insightViewSeriesSortOptionsResolver struct {
@@ -589,6 +587,7 @@ func (r *Resolver) UpdateLineChartSearchInsight(ctx context.Context, args *graph
 		SeriesSortMode:      seriesSortMode,
 		SeriesSortDirection: seriesSortDirection,
 		SeriesLimit:         args.Input.ViewControls.SeriesDisplayOptions.Limit,
+		SeriesNumSamples:    args.Input.ViewControls.SeriesDisplayOptions.NumSamples,
 	})
 	if err != nil {
 		return nil, errors.Wrap(err, "UpdateView")
@@ -1094,12 +1093,10 @@ func (d *InsightViewQueryConnectionResolver) Nodes(ctx context.Context) ([]graph
 					Direction: types.SeriesSortDirection(d.args.SeriesDisplayOptions.SortOptions.Direction),
 				}
 			}
-			numSamples := 90
-			if d.args.SeriesDisplayOptions.NumSamples != nil {
-				numSamples = int(*d.args.SeriesDisplayOptions.NumSamples)
-				if numSamples > 90 {
-					numSamples = 90
-				}
+			numSamples := d.args.SeriesDisplayOptions.NumSamples
+			if numSamples != nil && *numSamples > 90 {
+				var maxNumSamples int32 = 90
+				numSamples = &maxNumSamples
 			}
 			resolver.overrideSeriesOptions = &types.SeriesDisplayOptions{
 				SortOptions: sortOptions,
@@ -1303,8 +1300,8 @@ func createAndAttachSeries(ctx context.Context, tx *store.InsightStore, startSer
 	}
 
 	// Don't try to match on non-global series, since they are always replaced
-	// Also don't try to match on series that use repo critieria
-	// TODO: Reconsider matching on on criteria based series. If so the edit case would need work to ensure other insights remain the same.
+	// Also don't try to match on series that use repo criteria
+	// TODO: Reconsider matching on criteria based series. If so the edit case would need work to ensure other insights remain the same.
 	if len(series.RepositoryScope.Repositories) == 0 && series.RepositoryScope.RepositoryCriteria == nil {
 		matchingSeries, foundSeries, err = tx.FindMatchingSeries(ctx, store.MatchSeriesArgs{
 			Query:                     series.Query,

enterprise/internal/insights/store/insight_store.go

@@ -262,6 +262,7 @@ func (s *InsightStore) GroupByView(ctx context.Context, viewSeries []types.Insig
 			SeriesOptions: types.SeriesDisplayOptions{
 				SortOptions: sortOptions,
 				Limit:       seriesSet[0].SeriesLimit,
+				NumSamples:  seriesSet[0].SeriesNumSamples,
 			},
 		})
 	}
@@ -482,6 +483,7 @@ func scanInsightViewSeries(rows *sql.Rows, queryErr error) (_ []types.InsightVie
 			&temp.SeriesSortMode,
 			&temp.SeriesSortDirection,
 			&temp.SeriesLimit,
+			&temp.SeriesNumSamples,
 			&temp.GroupBy,
 			&temp.BackfillAttempts,
 			&temp.SupportsAugmentation,
@@ -615,6 +617,7 @@ func (s *InsightStore) UpdateView(ctx context.Context, view types.InsightView) (
 		view.SeriesSortMode,
 		view.SeriesSortDirection,
 		view.SeriesLimit,
+		view.SeriesNumSamples,
 		view.UniqueID,
 	))
 	var id int
@@ -958,7 +961,7 @@ returning id;`
 const updateInsightViewSql = `
 UPDATE insight_view SET title = %s, description = %s, default_filter_include_repo_regex = %s, default_filter_exclude_repo_regex = %s,
 default_filter_search_contexts = %s, other_threshold = %s, presentation_type = %s, series_sort_mode = %s, series_sort_direction = %s,
-series_limit = %s
+series_limit = %s, series_num_samples = %s
 WHERE unique_id = %s
 RETURNING id;`
 
@@ -976,8 +979,8 @@ i.id, i.series_id, i.query, i.created_at, i.oldest_historical_at, i.last_recorde
 i.next_recording_after, i.backfill_queued_at, i.last_snapshot_at, i.next_snapshot_after, i.repositories,
 i.sample_interval_unit, i.sample_interval_value, iv.default_filter_include_repo_regex, iv.default_filter_exclude_repo_regex,
 iv.other_threshold, iv.presentation_type, i.generated_from_capture_groups, i.just_in_time, i.generation_method, iv.is_frozen,
-default_filter_search_contexts, iv.series_sort_mode, iv.series_sort_direction, iv.series_limit, i.group_by, i.backfill_attempts,
-i.supports_augmentation, i.repository_criteria
+default_filter_search_contexts, iv.series_sort_mode, iv.series_sort_direction, iv.series_limit, iv.series_num_samples,
+i.group_by, i.backfill_attempts, i.supports_augmentation, i.repository_criteria
 FROM (%s) iv
          JOIN insight_view_series ivs ON iv.id = ivs.insight_view_id
          JOIN insight_series i ON ivs.insight_series_id = i.id
@@ -991,8 +994,8 @@ i.id, i.series_id, i.query, i.created_at, i.oldest_historical_at, i.last_recorde
 i.next_recording_after, i.backfill_queued_at, i.last_snapshot_at, i.next_snapshot_after, i.repositories,
 i.sample_interval_unit, i.sample_interval_value, iv.default_filter_include_repo_regex, iv.default_filter_exclude_repo_regex,
 iv.other_threshold, iv.presentation_type, i.generated_from_capture_groups, i.just_in_time, i.generation_method, iv.is_frozen,
-default_filter_search_contexts, iv.series_sort_mode, iv.series_sort_direction, iv.series_limit, i.group_by, i.backfill_attempts,
-i.supports_augmentation, i.repository_criteria
+default_filter_search_contexts, iv.series_sort_mode, iv.series_sort_direction, iv.series_limit, iv.series_num_samples,
+i.group_by, i.backfill_attempts, i.supports_augmentation, i.repository_criteria
 FROM dashboard_insight_view as dbiv
 		 JOIN insight_view iv ON iv.id = dbiv.insight_view_id
          JOIN insight_view_series ivs ON iv.id = ivs.insight_view_id
@@ -1032,8 +1035,8 @@ SELECT iv.id, 0 as dashboard_insight_id, iv.unique_id, iv.title, iv.description,
        i.next_recording_after, i.backfill_queued_at, i.last_snapshot_at, i.next_snapshot_after, i.repositories,
        i.sample_interval_unit, i.sample_interval_value, iv.default_filter_include_repo_regex, iv.default_filter_exclude_repo_regex,
 	   iv.other_threshold, iv.presentation_type, i.generated_from_capture_groups, i.just_in_time, i.generation_method, iv.is_frozen,
-	   default_filter_search_contexts, iv.series_sort_mode, iv.series_sort_direction, iv.series_limit, i.group_by, i.backfill_attempts,
-	   i.supports_augmentation, i.repository_criteria
+	   default_filter_search_contexts, iv.series_sort_mode, iv.series_sort_direction, iv.series_limit, iv.series_num_samples,
+	   i.group_by, i.backfill_attempts, i.supports_augmentation, i.repository_criteria
 
 FROM insight_view iv
 JOIN insight_view_series ivs ON iv.id = ivs.insight_view_id

enterprise/internal/insights/store/insight_store_test.go

@@ -287,9 +287,10 @@ func TestGetAll(t *testing.T) {
 	groupByRepo := "repo"
 	ctx := context.Background()
 
+	store := NewInsightStore(insightsDB)
+
 	// First test the method on an empty database.
 	t.Run("test empty database", func(t *testing.T) {
-		store := NewInsightStore(insightsDB)
 		got, err := store.GetAll(ctx, InsightQueryArgs{})
 		if err != nil {
 			t.Fatal(err)
@@ -348,8 +349,7 @@ func TestGetAll(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	t.Run("test all results", func(t *testing.T) {
-		store := NewInsightStore(insightsDB)
+	t.Run("all results", func(t *testing.T) {
 		got, err := store.GetAll(ctx, InsightQueryArgs{})
 		if err != nil {
 			t.Fatal(err)
@@ -476,7 +476,7 @@ func TestGetAll(t *testing.T) {
 			t.Errorf("unexpected insight view series want/got: %s", diff)
 		}
 	})
-	t.Run("test first result", func(t *testing.T) {
+	t.Run("first result", func(t *testing.T) {
 		store := NewInsightStore(insightsDB)
 		got, err := store.GetAll(ctx, InsightQueryArgs{Limit: 1})
 		if err != nil {
@@ -535,8 +535,7 @@ func TestGetAll(t *testing.T) {
 			t.Errorf("unexpected insight view series want/got: %s", diff)
 		}
 	})
-	t.Run("test second result", func(t *testing.T) {
-		store := NewInsightStore(insightsDB)
+	t.Run("second result", func(t *testing.T) {
 		got, err := store.GetAll(ctx, InsightQueryArgs{Limit: 1, After: "b"})
 		if err != nil {
 			t.Fatal(err)
@@ -594,8 +593,7 @@ func TestGetAll(t *testing.T) {
 			t.Errorf("unexpected insight view series want/got: %s", diff)
 		}
 	})
-	t.Run("test last 2 results", func(t *testing.T) {
-		store := NewInsightStore(insightsDB)
+	t.Run("last 2 results", func(t *testing.T) {
 		got, err := store.GetAll(ctx, InsightQueryArgs{After: "b"})
 		if err != nil {
 			t.Fatal(err)
@@ -676,8 +674,7 @@ func TestGetAll(t *testing.T) {
 			t.Errorf("unexpected insight view series want/got: %s", diff)
 		}
 	})
-	t.Run("test find by title results ", func(*testing.T) {
-		store := NewInsightStore(insightsDB)
+	t.Run("find by title results", func(*testing.T) {
 		got, err := store.GetAll(ctx, InsightQueryArgs{Find: "view 3"})
 		if err != nil {
 			t.Fatal(err)
@@ -735,9 +732,7 @@ func TestGetAll(t *testing.T) {
 			t.Errorf("unexpected insight view series want/got: %s", diff)
 		}
 	})
-
-	t.Run("test find by series label results ", func(*testing.T) {
-		store := NewInsightStore(insightsDB)
+	t.Run("find by series label results", func(*testing.T) {
 		got, err := store.GetAll(ctx, InsightQueryArgs{Find: "label5-1"})
 		if err != nil {
 			t.Fatal(err)
@@ -796,7 +791,6 @@ func TestGetAll(t *testing.T) {
 		}
 	})
 	t.Run("exclude insight ids from results", func(t *testing.T) {
-		store := NewInsightStore(insightsDB)
 		got, err := store.GetAll(ctx, InsightQueryArgs{ExcludeIDs: []string{"b", "e"}})
 		if err != nil {
 			t.Fatal(err)
@@ -854,6 +848,32 @@ func TestGetAll(t *testing.T) {
 			t.Errorf("unexpected insight view series want/got: %s", diff)
 		}
 	})
+	t.Run("returns expected number of samples", func(t *testing.T) {
+		// Set the series_num_samples value
+		numSamples := int32(50)
+		view, err := store.UpdateView(ctx, types.InsightView{
+			UniqueID:         "d",
+			PresentationType: types.Line, // setting for null constraint
+			SeriesNumSamples: &numSamples,
+		})
+		if err != nil {
+			t.Fatal(err)
+		}
+		if diff := cmp.Diff(&numSamples, view.SeriesNumSamples); diff != "" {
+			t.Errorf("unexpected insight view series num samples want/got: %s", diff)
+		}
+
+		series, err := store.GetAll(ctx, InsightQueryArgs{UniqueIDs: []string{"d"}})
+		if err != nil {
+			t.Fatal(err)
+		}
+		// we're only testing the number of samples in this test cases
+		for _, s := range series {
+			if diff := cmp.Diff(&numSamples, s.SeriesNumSamples); diff != "" {
+				t.Errorf("unexpected insight view series num samples want/got: %s", diff)
+			}
+		}
+	})
 }
 
 func TestGetAllOnDashboard(t *testing.T) {
@@ -1196,7 +1216,6 @@ func TestCreateSeries(t *testing.T) {
 		}
 	})
 	t.Run("test create and get capture groups series", func(t *testing.T) {
-		store := NewInsightStore(insightsDB)
 		sampleIntervalUnit := "MONTH"
 		repoCriteria := "repo:a"
 		_, err := store.CreateSeries(ctx, types.InsightSeries{

enterprise/internal/insights/types/types.go

@@ -42,6 +42,7 @@ type InsightViewSeries struct {
 	BackfillAttempts              int32
 	SupportsAugmentation          bool
 	RepositoryCriteria            *string
+	SeriesNumSamples              *int32
 }
 
 type Insight struct {
@@ -83,6 +84,7 @@ type InsightView struct {
 	SeriesSortMode      *SeriesSortMode
 	SeriesSortDirection *SeriesSortDirection
 	SeriesLimit         *int32
+	SeriesNumSamples    *int32
 }
 
 // InsightSeries is a single data series for a Code Insight. This contains some metadata about the data series, as well
@@ -186,7 +188,7 @@ const (
 type SeriesDisplayOptions struct {
 	SortOptions *SeriesSortOptions
 	Limit       *int32
-	NumSamples  int
+	NumSamples  *int32
 }
 
 type SeriesSortOptions struct {

internal/database/schema.codeinsights.json

@@ -1606,6 +1606,19 @@
           "GenerationExpression": "",
           "Comment": ""
         },
+        {
+          "Name": "series_num_samples",
+          "Index": 14,
+          "TypeName": "integer",
+          "IsNullable": true,
+          "Default": "",
+          "CharacterMaximumLength": 0,
+          "IsIdentity": false,
+          "IdentityGeneration": "",
+          "IsGenerated": "NEVER",
+          "GenerationExpression": "",
+          "Comment": ""
+        },
         {
           "Name": "series_sort_direction",
           "Index": 12,