Skip to content

Commit 078beb7

Browse files
KyleJujcscottiii
andauthored
Handle exclusions specified in the ExcludedFeatureKeys table (#1271)
* Handle exclusions specified in the ExcludedFeatureKeys table * lint * Update lib/gcpspanner/missing_one_implementation_feature_list_test.go Co-authored-by: James C Scott III <jcscottiii@users.noreply.github.com> --------- Co-authored-by: James C Scott III <jcscottiii@users.noreply.github.com>
1 parent 8d7d31e commit 078beb7

File tree

2 files changed

+87
-1
lines changed

2 files changed

+87
-1
lines changed

lib/gcpspanner/missing_one_implementation_feature_list.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ EXISTS (
8686
AND
8787
{{ end }}
8888
1=1
89+
{{ .ExcludedFeatureFilter }}
8990
ORDER BY KEY ASC
9091
LIMIT @limit
9192
{{ if .Offset }}
@@ -96,6 +97,7 @@ OFFSET {{ .Offset }}
9697
type missingOneImplFeatureListTemplateData struct {
9798
OtherBrowsersParamNames []string
9899
Offset int
100+
ExcludedFeatureFilter string
99101
}
100102

101103
func buildMissingOneImplFeatureListTemplate(
@@ -104,6 +106,7 @@ func buildMissingOneImplFeatureListTemplate(
104106
targetDate time.Time,
105107
cursor *missingOneImplFeatureListCursor,
106108
pageSize int,
109+
excludedFeatureIDs []string,
107110
) spanner.Statement {
108111
params := map[string]interface{}{}
109112
allBrowsers := make([]string, len(otherBrowsers)+1)
@@ -117,12 +120,19 @@ func buildMissingOneImplFeatureListTemplate(
117120
otherBrowsersParamNames = append(otherBrowsersParamNames, paramName)
118121
}
119122

123+
var excludedFeatureFilter string
124+
if len(excludedFeatureIDs) > 0 {
125+
params["excludedFeatureIDs"] = excludedFeatureIDs
126+
excludedFeatureFilter = "AND wf.ID NOT IN UNNEST(@excludedFeatureIDs)"
127+
}
128+
120129
params["targetDate"] = targetDate
121130
params["limit"] = pageSize
122131

123132
tmplData := missingOneImplFeatureListTemplateData{
124133
OtherBrowsersParamNames: otherBrowsersParamNames,
125134
Offset: cursor.Offset,
135+
ExcludedFeatureFilter: excludedFeatureFilter,
126136
}
127137

128138
sql := missingOneImplFeatureListTemplate.Execute(tmplData)
@@ -152,12 +162,19 @@ func (c *Client) MissingOneImplFeatureList(
152162
txn := c.ReadOnlyTransaction()
153163
defer txn.Close()
154164

165+
// Get ignored feature IDs
166+
ignoredFeatureIDs, err := c.getIgnoredFeatureIDsForStats(ctx, txn)
167+
if err != nil {
168+
return nil, err
169+
}
170+
155171
stmt := buildMissingOneImplFeatureListTemplate(
156172
targetBrowser,
157173
otherBrowsers,
158174
targetDate,
159175
cursor,
160176
pageSize,
177+
ignoredFeatureIDs,
161178
)
162179

163180
it := txn.Query(ctx, stmt)

lib/gcpspanner/missing_one_implementation_feature_list_test.go

Lines changed: 70 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ func testMissingOneImplFeatureListSuite(
154154
ctx context.Context,
155155
t *testing.T,
156156
) {
157-
t.Run("bazBrowser", func(t *testing.T) {
157+
t.Run("Query bazBrowser without exclusions", func(t *testing.T) {
158158
const targetBrowser = "bazBrowser"
159159
otherBrowsers := []string{
160160
"fooBrowser",
@@ -330,6 +330,75 @@ func testMissingOneImplFeatureListSuite(
330330
})
331331

332332
})
333+
334+
t.Run("with excluded/discouraged features", func(t *testing.T) {
335+
// Exclude Feature X
336+
excludedFeatures := []string{"FeatureX"}
337+
for _, featureKey := range excludedFeatures {
338+
err := spannerClient.InsertExcludedFeatureKey(ctx, featureKey)
339+
if err != nil {
340+
t.Fatalf("Failed to insert excluded feature key: %v", err)
341+
}
342+
}
343+
344+
// Discourage FeatureZ
345+
discouragedFeatures := []string{"FeatureZ"}
346+
for _, featureKey := range discouragedFeatures {
347+
err := spannerClient.UpsertFeatureDiscouragedDetails(ctx, featureKey, FeatureDiscouragedDetails{
348+
AccordingTo: nil,
349+
Alternatives: nil,
350+
})
351+
if err != nil {
352+
t.Fatalf("Failed to upsert feature discouraged details: %v", err)
353+
}
354+
}
355+
356+
// nolint:goconst // WONTFIX
357+
t.Run("simple query", func(t *testing.T) {
358+
targetBrowser := "bazBrowser"
359+
otherBrowsers := []string{"fooBrowser", "barBrowser"}
360+
targetDate := time.Date(2024, 4, 15, 0, 0, 0, 0, time.UTC)
361+
pageSize := 25
362+
token := encodeMissingOneImplFeatureListCursor(0)
363+
364+
expectedResult := &MissingOneImplFeatureListPage{
365+
NextPageToken: nil,
366+
FeatureList: []MissingOneImplFeature{
367+
// fooBrowser 113 release
368+
// Currently supported features:
369+
// fooBrowser: FeatureX, FeatureZ, FeatureY, FeatureW
370+
// barBrowser: FeatureX, FeatureZ, FeatureY, FeatureW
371+
// bazBrowser: FeatureX, FeatureY
372+
// Missing in on for bazBrowser: FeatureW (FeatureZ is excluded/discouraged)
373+
{
374+
WebFeatureID: "FeatureW",
375+
},
376+
},
377+
}
378+
// Assert with excluded/discouraged features
379+
assertMissingOneImplFeatureList(
380+
ctx,
381+
t,
382+
targetDate,
383+
targetBrowser,
384+
otherBrowsers,
385+
expectedResult,
386+
&token,
387+
pageSize,
388+
)
389+
})
390+
391+
// Clear the excluded and discouraged features after the test
392+
err := spannerClient.ClearExcludedFeatureKeys(ctx)
393+
if err != nil {
394+
t.Fatalf("Failed to clear excluded feature keys: %v", err)
395+
}
396+
397+
err = spannerClient.ClearFeatureDiscouragedDetails(ctx)
398+
if err != nil {
399+
t.Fatalf("Failed to clear feature discouraged details: %v", err)
400+
}
401+
})
333402
}
334403

335404
func TestListMissingOneImplFeatureList(t *testing.T) {

0 commit comments

Comments
 (0)