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 <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Jamal Carvalho <jamal@golang.org>
Reviewed-by: Julie Qiu <julie@golang.org>
diff --git a/internal/frontend/paginate.go b/internal/frontend/paginate.go
index c651380..7af08ad 100644
--- a/internal/frontend/paginate.go
+++ b/internal/frontend/paginate.go
@@ -20,7 +20,7 @@
// 1, 2, 3, .... Each page except possibly the last has the same number of results.
type pagination struct {
baseURL *url.URL // URL common to all pages
- limit int // the maximum number of results on a page
+ Limit int // the maximum number of results on a page
ResultCount int // number of results on this page
TotalCount int // total number of results
Approximate bool // whether or not the total count is approximate
@@ -29,6 +29,7 @@
NextPage int // " " " next page, usually Page+1, but zero on the last page
Offset int // offset of the first item on the current page
Pages []int // consecutive page numbers to be displayed for navigation
+ Limits []int // limits to be displayed
}
// PageURL constructs a URL that displays the given page.
@@ -40,6 +41,15 @@
return p.baseURL.String()
}
+// LimitURL constructs a URL that adds a "limit" query parameter to the base
+// URL.
+func (p pagination) LimitURL(limit int) string {
+ newQuery := p.baseURL.Query()
+ newQuery.Set("limit", strconv.Itoa(limit))
+ p.baseURL.RawQuery = newQuery.Encode()
+ return p.baseURL.String()
+}
+
// newPagination constructs a pagination. Call it after some results have been
// obtained.
// resultCount is the number of results in the current page.
@@ -50,11 +60,12 @@
TotalCount: totalCount,
ResultCount: resultCount,
Offset: params.offset(),
- limit: params.limit,
+ Limit: params.limit,
Page: params.page,
PrevPage: prev(params.page),
NextPage: next(params.page, params.limit, totalCount),
Pages: pagesToLink(params.page, numPages(params.limit, totalCount), defaultNumPagesToLink),
+ Limits: []int{10, 30, maxSearchPageSize},
}
}
diff --git a/internal/frontend/search.go b/internal/frontend/search.go
index 6b7d633..3ef24bb 100644
--- a/internal/frontend/search.go
+++ b/internal/frontend/search.go
@@ -67,9 +67,14 @@
func fetchSearchPage(ctx context.Context, db *postgres.DB, query string, pageParams paginationParams, searchSymbols bool) (*SearchPage, error) {
maxResultCount := maxSearchOffset + pageParams.limit
+ offset := pageParams.offset()
+ if experiment.IsActive(ctx, internal.ExperimentSearchGrouping) {
+ // When using search grouping, do pageless search: always start from the beginning.
+ offset = 0
+ }
dbresults, err := db.Search(ctx, query, postgres.SearchOptions{
MaxResults: pageParams.limit,
- Offset: pageParams.offset(),
+ Offset: offset,
MaxResultCount: maxResultCount,
SearchSymbols: searchSymbols,
})
diff --git a/internal/frontend/search_test.go b/internal/frontend/search_test.go
index d6ef346..afdaaa3 100644
--- a/internal/frontend/search_test.go
+++ b/internal/frontend/search_test.go
@@ -111,9 +111,10 @@
ResultCount: 1,
PrevPage: 0,
NextPage: 0,
- limit: 20,
+ Limit: 20,
Page: 1,
Pages: []int{1},
+ Limits: []int{10, 30, 100},
},
Results: []*SearchResult{
{
@@ -137,9 +138,10 @@
ResultCount: 1,
PrevPage: 0,
NextPage: 0,
- limit: 20,
+ Limit: 20,
Page: 1,
Pages: []int{1},
+ Limits: []int{10, 30, 100},
},
Results: []*SearchResult{
{
diff --git a/internal/frontend/server_test.go b/internal/frontend/server_test.go
index cf68c81..1d94929 100644
--- a/internal/frontend/server_test.go
+++ b/internal/frontend/server_test.go
@@ -703,17 +703,6 @@
want: in("", hasText("User-agent: *"), hasText(regexp.QuoteMeta("Disallow: /search?*"))),
},
{
- name: "search",
- urlPath: fmt.Sprintf("/search?q=%s", sample.PackageName),
- wantStatusCode: http.StatusOK,
- want: in("",
- in(".SearchResults-resultCount", hasText("2 results")),
- in(".LegacySearchSnippet-header",
- in("a",
- href("/"+sample.ModulePath+"/foo"),
- hasText(sample.ModulePath+"/foo")))),
- },
- {
name: "search large offset",
urlPath: "/search?q=github.com&page=1002",
wantStatusCode: http.StatusBadRequest,
@@ -1129,6 +1118,48 @@
},
}
+var searchTestCase = serverTestCase{
+ name: "search",
+ urlPath: fmt.Sprintf("/search?q=%s", sample.PackageName),
+ wantStatusCode: http.StatusOK,
+ want: in("",
+ in(".SearchResults-resultCount", hasText("2 results")),
+ in(".LegacySearchSnippet-header",
+ in("a",
+ href("/"+sample.ModulePath+"/foo"),
+ hasText(sample.ModulePath+"/foo")))),
+}
+
+var searchGroupingTestCases = []serverTestCase{
+ {
+ name: "search",
+ urlPath: fmt.Sprintf("/search?q=%s", sample.PackageName),
+ wantStatusCode: http.StatusOK,
+ want: in("",
+ in(".SearchResults-resultCount", hasText("2 results")),
+ in(".SearchSnippet-header",
+ in("a",
+ href("/"+sample.ModulePath+"/foo"),
+ hasText("foo")))),
+ },
+ {
+ name: "pageless search",
+ urlPath: fmt.Sprintf("/search?q=%s", sample.PackageName),
+ wantStatusCode: http.StatusOK,
+ want: in("",
+ in(".SearchResults-resultCount",
+ hasText("2 results"),
+ hasText("Fetch"),
+ hasText("results.")),
+ notIn(".Pagination-navInner")),
+ },
+ {
+ name: "search large limit",
+ urlPath: fmt.Sprintf("/search?q=%s&limit=101", sample.PackageName),
+ wantStatusCode: http.StatusBadRequest,
+ },
+}
+
// TestServer checks the contents of served pages by looking for
// strings and elements in the parsed HTML response body.
//
@@ -1150,7 +1181,7 @@
{
name: "no experiments",
testCasesFunc: func() []serverTestCase {
- return append(serverTestCases(), linksTestCases...)
+ return append(append(serverTestCases(), linksTestCases...), searchTestCase)
},
},
{
@@ -1173,6 +1204,13 @@
internal.ExperimentSymbolHistoryVersionsPage,
},
},
+ {
+ name: "search grouping",
+ testCasesFunc: func() []serverTestCase {
+ return append(serverTestCases(), searchGroupingTestCases...)
+ },
+ experiments: []string{internal.ExperimentSearchGrouping},
+ },
} {
t.Run(test.name, func(t *testing.T) {
testServer(t, test.testCasesFunc(), test.experiments...)
@@ -1565,6 +1603,7 @@
{"homepage", nil, basePage{}},
{"license-policy", nil, licensePolicyPage{}},
{"search", nil, SearchPage{}},
+ {"legacy_search", nil, SearchPage{}},
{"search-help", nil, basePage{}},
{"unit_details.tmpl", nil, UnitPage{}},
{
diff --git a/static/frontend/legacy_search/legacy_search.tmpl b/static/frontend/legacy_search/legacy_search.tmpl
index cd3a0e9..7347763 100644
--- a/static/frontend/legacy_search/legacy_search.tmpl
+++ b/static/frontend/legacy_search/legacy_search.tmpl
@@ -75,7 +75,6 @@
{{range $i, $v := .Links}}
<a href="/{{$v.Href}}" data-gtmc="search result" data-gtmv="{{$i}}">{{$v.Body}}</a>
{{end}}
- <span>{{.Suffix}}</span>
</div>
{{end}}
{{with .OtherMajor}}
diff --git a/static/frontend/search/search.tmpl b/static/frontend/search/search.tmpl
index ed26898..672b675 100644
--- a/static/frontend/search/search.tmpl
+++ b/static/frontend/search/search.tmpl
@@ -23,7 +23,21 @@
<div class="go-Content">
<h1 class="SearchResults-header">Results for “{{.Query}}”</h1>
<div class="SearchResults-resultCount go-textSubtle">
- {{template "pagination_summary" .Pagination}} {{pluralize .Pagination.TotalCount "result"}}
+ {{with .Pagination}}
+ {{ $p := . }}
+ <div class="Pagination-nav">
+ Displaying {{.ResultCount}} of {{if .Approximate}}about {{end}}{{.TotalCount}} {{pluralize .TotalCount "result"}}.
+ Fetch
+ {{range .Limits}}
+ {{if eq . $p.Limit}}
+ <b class="Pagination-number">{{.}}</b>
+ {{else}}
+ <a class="Pagination-number" href="{{$p.LimitURL .}}">{{.}}</a>
+ {{end}}
+ {{end}}
+ results.
+ </div>
+ {{end}}
<div class="SearchResults-help"><a href="/search-help">Search help</a></div>
</div>
{{if eq (len .Results) 0}}
@@ -46,9 +60,6 @@
{{template "grouped_search" .}}
</div>
{{end}}
- <div class="SearchResults-footer">
- {{template "pagination_nav" .Pagination}}
- </div>
</div>
</main>
{{end}}
@@ -65,7 +76,7 @@
{{$v.Name}}
</a>
</h2>
- <span class="SearchSnippet-header-path">{{$v.PackagePath}}</span>
+ <span class="SearchSnippet-header-path">{{$v.PackagePath}}</span>
</div>
<p class="SearchSnippet-synopsis" data-test-id="snippet-synopsis">
{{$v.Synopsis}}
