Skip to content

Commit 5154d97

Browse files
committed
internal/frontend: don't paginate for grouped search
When the search-grouping experiment is on, don't paginate the results. Instead present all of them, and let the user choose how many they want from a fixed list. For golang/go#47315 Change-Id: Ia7639f619bf1cce62067c69983f946d964161b17 Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/336770 Trust: Jonathan Amsterdam <[email protected]> Run-TryBot: Jonathan Amsterdam <[email protected]> Reviewed-by: Jamal Carvalho <[email protected]> Reviewed-by: Julie Qiu <[email protected]>
1 parent ef61804 commit 5154d97

File tree

6 files changed

+90
-23
lines changed

6 files changed

+90
-23
lines changed

internal/frontend/paginate.go

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import (
2020
// 1, 2, 3, .... Each page except possibly the last has the same number of results.
2121
type pagination struct {
2222
baseURL *url.URL // URL common to all pages
23-
limit int // the maximum number of results on a page
23+
Limit int // the maximum number of results on a page
2424
ResultCount int // number of results on this page
2525
TotalCount int // total number of results
2626
Approximate bool // whether or not the total count is approximate
@@ -29,6 +29,7 @@ type pagination struct {
2929
NextPage int // " " " next page, usually Page+1, but zero on the last page
3030
Offset int // offset of the first item on the current page
3131
Pages []int // consecutive page numbers to be displayed for navigation
32+
Limits []int // limits to be displayed
3233
}
3334

3435
// PageURL constructs a URL that displays the given page.
@@ -40,6 +41,15 @@ func (p pagination) PageURL(page int) string {
4041
return p.baseURL.String()
4142
}
4243

44+
// LimitURL constructs a URL that adds a "limit" query parameter to the base
45+
// URL.
46+
func (p pagination) LimitURL(limit int) string {
47+
newQuery := p.baseURL.Query()
48+
newQuery.Set("limit", strconv.Itoa(limit))
49+
p.baseURL.RawQuery = newQuery.Encode()
50+
return p.baseURL.String()
51+
}
52+
4353
// newPagination constructs a pagination. Call it after some results have been
4454
// obtained.
4555
// resultCount is the number of results in the current page.
@@ -50,11 +60,12 @@ func newPagination(params paginationParams, resultCount, totalCount int) paginat
5060
TotalCount: totalCount,
5161
ResultCount: resultCount,
5262
Offset: params.offset(),
53-
limit: params.limit,
63+
Limit: params.limit,
5464
Page: params.page,
5565
PrevPage: prev(params.page),
5666
NextPage: next(params.page, params.limit, totalCount),
5767
Pages: pagesToLink(params.page, numPages(params.limit, totalCount), defaultNumPagesToLink),
68+
Limits: []int{10, 30, maxSearchPageSize},
5869
}
5970
}
6071

internal/frontend/search.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,14 @@ type subResult struct {
6767
func fetchSearchPage(ctx context.Context, db *postgres.DB, query string, pageParams paginationParams, searchSymbols bool) (*SearchPage, error) {
6868
maxResultCount := maxSearchOffset + pageParams.limit
6969

70+
offset := pageParams.offset()
71+
if experiment.IsActive(ctx, internal.ExperimentSearchGrouping) {
72+
// When using search grouping, do pageless search: always start from the beginning.
73+
offset = 0
74+
}
7075
dbresults, err := db.Search(ctx, query, postgres.SearchOptions{
7176
MaxResults: pageParams.limit,
72-
Offset: pageParams.offset(),
77+
Offset: offset,
7378
MaxResultCount: maxResultCount,
7479
SearchSymbols: searchSymbols,
7580
})

internal/frontend/search_test.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,9 +111,10 @@ func TestFetchSearchPage(t *testing.T) {
111111
ResultCount: 1,
112112
PrevPage: 0,
113113
NextPage: 0,
114-
limit: 20,
114+
Limit: 20,
115115
Page: 1,
116116
Pages: []int{1},
117+
Limits: []int{10, 30, 100},
117118
},
118119
Results: []*SearchResult{
119120
{
@@ -137,9 +138,10 @@ func TestFetchSearchPage(t *testing.T) {
137138
ResultCount: 1,
138139
PrevPage: 0,
139140
NextPage: 0,
140-
limit: 20,
141+
Limit: 20,
141142
Page: 1,
142143
Pages: []int{1},
144+
Limits: []int{10, 30, 100},
143145
},
144146
Results: []*SearchResult{
145147
{

internal/frontend/server_test.go

Lines changed: 51 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -702,17 +702,6 @@ func serverTestCases() []serverTestCase {
702702
wantStatusCode: http.StatusOK,
703703
want: in("", hasText("User-agent: *"), hasText(regexp.QuoteMeta("Disallow: /search?*"))),
704704
},
705-
{
706-
name: "search",
707-
urlPath: fmt.Sprintf("/search?q=%s", sample.PackageName),
708-
wantStatusCode: http.StatusOK,
709-
want: in("",
710-
in(".SearchResults-resultCount", hasText("2 results")),
711-
in(".LegacySearchSnippet-header",
712-
in("a",
713-
href("/"+sample.ModulePath+"/foo"),
714-
hasText(sample.ModulePath+"/foo")))),
715-
},
716705
{
717706
name: "search large offset",
718707
urlPath: "/search?q=github.com&page=1002",
@@ -1129,6 +1118,48 @@ var linksTestCases = []serverTestCase{
11291118
},
11301119
}
11311120

1121+
var searchTestCase = serverTestCase{
1122+
name: "search",
1123+
urlPath: fmt.Sprintf("/search?q=%s", sample.PackageName),
1124+
wantStatusCode: http.StatusOK,
1125+
want: in("",
1126+
in(".SearchResults-resultCount", hasText("2 results")),
1127+
in(".LegacySearchSnippet-header",
1128+
in("a",
1129+
href("/"+sample.ModulePath+"/foo"),
1130+
hasText(sample.ModulePath+"/foo")))),
1131+
}
1132+
1133+
var searchGroupingTestCases = []serverTestCase{
1134+
{
1135+
name: "search",
1136+
urlPath: fmt.Sprintf("/search?q=%s", sample.PackageName),
1137+
wantStatusCode: http.StatusOK,
1138+
want: in("",
1139+
in(".SearchResults-resultCount", hasText("2 results")),
1140+
in(".SearchSnippet-header",
1141+
in("a",
1142+
href("/"+sample.ModulePath+"/foo"),
1143+
hasText("foo")))),
1144+
},
1145+
{
1146+
name: "pageless search",
1147+
urlPath: fmt.Sprintf("/search?q=%s", sample.PackageName),
1148+
wantStatusCode: http.StatusOK,
1149+
want: in("",
1150+
in(".SearchResults-resultCount",
1151+
hasText("2 results"),
1152+
hasText("Fetch"),
1153+
hasText("results.")),
1154+
notIn(".Pagination-navInner")),
1155+
},
1156+
{
1157+
name: "search large limit",
1158+
urlPath: fmt.Sprintf("/search?q=%s&limit=101", sample.PackageName),
1159+
wantStatusCode: http.StatusBadRequest,
1160+
},
1161+
}
1162+
11321163
// TestServer checks the contents of served pages by looking for
11331164
// strings and elements in the parsed HTML response body.
11341165
//
@@ -1150,7 +1181,7 @@ func TestServer(t *testing.T) {
11501181
{
11511182
name: "no experiments",
11521183
testCasesFunc: func() []serverTestCase {
1153-
return append(serverTestCases(), linksTestCases...)
1184+
return append(append(serverTestCases(), linksTestCases...), searchTestCase)
11541185
},
11551186
},
11561187
{
@@ -1173,6 +1204,13 @@ func TestServer(t *testing.T) {
11731204
internal.ExperimentSymbolHistoryVersionsPage,
11741205
},
11751206
},
1207+
{
1208+
name: "search grouping",
1209+
testCasesFunc: func() []serverTestCase {
1210+
return append(serverTestCases(), searchGroupingTestCases...)
1211+
},
1212+
experiments: []string{internal.ExperimentSearchGrouping},
1213+
},
11761214
} {
11771215
t.Run(test.name, func(t *testing.T) {
11781216
testServer(t, test.testCasesFunc(), test.experiments...)
@@ -1565,6 +1603,7 @@ func TestCheckTemplates(t *testing.T) {
15651603
{"homepage", nil, basePage{}},
15661604
{"license-policy", nil, licensePolicyPage{}},
15671605
{"search", nil, SearchPage{}},
1606+
{"legacy_search", nil, SearchPage{}},
15681607
{"search-help", nil, basePage{}},
15691608
{"unit_details.tmpl", nil, UnitPage{}},
15701609
{

static/frontend/legacy_search/legacy_search.tmpl

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,6 @@
7575
{{range $i, $v := .Links}}
7676
<a href="/{{$v.Href}}" data-gtmc="search result" data-gtmv="{{$i}}">{{$v.Body}}</a>
7777
{{end}}
78-
<span>{{.Suffix}}</span>
7978
</div>
8079
{{end}}
8180
{{with .OtherMajor}}

static/frontend/search/search.tmpl

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,21 @@
2323
<div class="go-Content">
2424
<h1 class="SearchResults-header">Results for “{{.Query}}”</h1>
2525
<div class="SearchResults-resultCount go-textSubtle">
26-
{{template "pagination_summary" .Pagination}} {{pluralize .Pagination.TotalCount "result"}}
26+
{{with .Pagination}}
27+
{{ $p := . }}
28+
<div class="Pagination-nav">
29+
Displaying {{.ResultCount}} of {{if .Approximate}}about {{end}}{{.TotalCount}} {{pluralize .TotalCount "result"}}.
30+
Fetch
31+
{{range .Limits}}
32+
{{if eq . $p.Limit}}
33+
<b class="Pagination-number">{{.}}</b>
34+
{{else}}
35+
<a class="Pagination-number" href="{{$p.LimitURL .}}">{{.}}</a>
36+
{{end}}
37+
{{end}}
38+
results.
39+
</div>
40+
{{end}}
2741
<div class="SearchResults-help"><a href="/search-help">Search help</a></div>
2842
</div>
2943
{{if eq (len .Results) 0}}
@@ -46,9 +60,6 @@
4660
{{template "grouped_search" .}}
4761
</div>
4862
{{end}}
49-
<div class="SearchResults-footer">
50-
{{template "pagination_nav" .Pagination}}
51-
</div>
5263
</div>
5364
</main>
5465
{{end}}
@@ -65,7 +76,7 @@
6576
{{$v.Name}}
6677
</a>
6778
</h2>
68-
<span class="SearchSnippet-header-path">{{$v.PackagePath}}</span>
79+
<span class="SearchSnippet-header-path">{{$v.PackagePath}}</span>
6980
</div>
7081
<p class="SearchSnippet-synopsis" data-test-id="snippet-synopsis">
7182
{{$v.Synopsis}}

0 commit comments

Comments
 (0)