diff --git a/lib/gcpspanner/missing_one_implementation_feature_list.go b/lib/gcpspanner/missing_one_implementation_feature_list.go index 45f012a5b..35d69c2c1 100644 --- a/lib/gcpspanner/missing_one_implementation_feature_list.go +++ b/lib/gcpspanner/missing_one_implementation_feature_list.go @@ -86,6 +86,7 @@ EXISTS ( AND {{ end }} 1=1 +{{ .ExcludedFeatureFilter }} ORDER BY KEY ASC LIMIT @limit {{ if .Offset }} @@ -96,6 +97,7 @@ OFFSET {{ .Offset }} type missingOneImplFeatureListTemplateData struct { OtherBrowsersParamNames []string Offset int + ExcludedFeatureFilter string } func buildMissingOneImplFeatureListTemplate( @@ -104,6 +106,7 @@ func buildMissingOneImplFeatureListTemplate( targetDate time.Time, cursor *missingOneImplFeatureListCursor, pageSize int, + excludedFeatureIDs []string, ) spanner.Statement { params := map[string]interface{}{} allBrowsers := make([]string, len(otherBrowsers)+1) @@ -117,12 +120,19 @@ func buildMissingOneImplFeatureListTemplate( otherBrowsersParamNames = append(otherBrowsersParamNames, paramName) } + var excludedFeatureFilter string + if len(excludedFeatureIDs) > 0 { + params["excludedFeatureIDs"] = excludedFeatureIDs + excludedFeatureFilter = "AND wf.ID NOT IN UNNEST(@excludedFeatureIDs)" + } + params["targetDate"] = targetDate params["limit"] = pageSize tmplData := missingOneImplFeatureListTemplateData{ OtherBrowsersParamNames: otherBrowsersParamNames, Offset: cursor.Offset, + ExcludedFeatureFilter: excludedFeatureFilter, } sql := missingOneImplFeatureListTemplate.Execute(tmplData) @@ -152,12 +162,19 @@ func (c *Client) MissingOneImplFeatureList( txn := c.ReadOnlyTransaction() defer txn.Close() + // Get ignored feature IDs + ignoredFeatureIDs, err := c.getIgnoredFeatureIDsForStats(ctx, txn) + if err != nil { + return nil, err + } + stmt := buildMissingOneImplFeatureListTemplate( targetBrowser, otherBrowsers, targetDate, cursor, pageSize, + ignoredFeatureIDs, ) it := txn.Query(ctx, stmt) diff --git a/lib/gcpspanner/missing_one_implementation_feature_list_test.go b/lib/gcpspanner/missing_one_implementation_feature_list_test.go index 5a881767c..1fbf16fa7 100644 --- a/lib/gcpspanner/missing_one_implementation_feature_list_test.go +++ b/lib/gcpspanner/missing_one_implementation_feature_list_test.go @@ -154,7 +154,7 @@ func testMissingOneImplFeatureListSuite( ctx context.Context, t *testing.T, ) { - t.Run("bazBrowser", func(t *testing.T) { + t.Run("Query bazBrowser without exclusions", func(t *testing.T) { const targetBrowser = "bazBrowser" otherBrowsers := []string{ "fooBrowser", @@ -330,6 +330,75 @@ func testMissingOneImplFeatureListSuite( }) }) + + t.Run("with excluded/discouraged features", func(t *testing.T) { + // Exclude Feature X + excludedFeatures := []string{"FeatureX"} + for _, featureKey := range excludedFeatures { + err := spannerClient.InsertExcludedFeatureKey(ctx, featureKey) + if err != nil { + t.Fatalf("Failed to insert excluded feature key: %v", err) + } + } + + // Discourage FeatureZ + discouragedFeatures := []string{"FeatureZ"} + for _, featureKey := range discouragedFeatures { + err := spannerClient.UpsertFeatureDiscouragedDetails(ctx, featureKey, FeatureDiscouragedDetails{ + AccordingTo: nil, + Alternatives: nil, + }) + if err != nil { + t.Fatalf("Failed to upsert feature discouraged details: %v", err) + } + } + + // nolint:goconst // WONTFIX + t.Run("simple query", func(t *testing.T) { + targetBrowser := "bazBrowser" + otherBrowsers := []string{"fooBrowser", "barBrowser"} + targetDate := time.Date(2024, 4, 15, 0, 0, 0, 0, time.UTC) + pageSize := 25 + token := encodeMissingOneImplFeatureListCursor(0) + + expectedResult := &MissingOneImplFeatureListPage{ + NextPageToken: nil, + FeatureList: []MissingOneImplFeature{ + // fooBrowser 113 release + // Currently supported features: + // fooBrowser: FeatureX, FeatureZ, FeatureY, FeatureW + // barBrowser: FeatureX, FeatureZ, FeatureY, FeatureW + // bazBrowser: FeatureX, FeatureY + // Missing in on for bazBrowser: FeatureW (FeatureZ is excluded/discouraged) + { + WebFeatureID: "FeatureW", + }, + }, + } + // Assert with excluded/discouraged features + assertMissingOneImplFeatureList( + ctx, + t, + targetDate, + targetBrowser, + otherBrowsers, + expectedResult, + &token, + pageSize, + ) + }) + + // Clear the excluded and discouraged features after the test + err := spannerClient.ClearExcludedFeatureKeys(ctx) + if err != nil { + t.Fatalf("Failed to clear excluded feature keys: %v", err) + } + + err = spannerClient.ClearFeatureDiscouragedDetails(ctx) + if err != nil { + t.Fatalf("Failed to clear feature discouraged details: %v", err) + } + }) } func TestListMissingOneImplFeatureList(t *testing.T) {