Skip to content

Commit 0937c9d

Browse files
committed
internal/gaby: allow (project,issue) query in overview/rules page
Allow formats like golang/go#12345 github.com/golang/go/issues/12345 go.dev/issues/12345 in addition to issue number 12345 (default to golang/go) For #51 Change-Id: Id9e26edd2cd876a708dca4ec3cdecb500bcccf07 Reviewed-on: https://go-review.googlesource.com/c/oscar/+/628015 Reviewed-by: Jonathan Amsterdam <jba@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Zvonimir Pavlinovic <zpavlinovic@google.com>
1 parent 498d98c commit 0937c9d

6 files changed

Lines changed: 148 additions & 31 deletions

File tree

internal/gaby/overview.go

Lines changed: 58 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ type overviewResult struct {
3535

3636
// overviewForm holds the raw inputs to the overview form.
3737
type overviewForm struct {
38-
Query string // the issue ID to lookup
38+
Query string // the issue ID to lookup, or golang/go#12345 or github.com/golang/go/issues/12345 form
3939
OverviewType string // the type of overview to generate
4040
}
4141

@@ -90,6 +90,45 @@ func fmtTimeString(s string) string {
9090
return t.Format(time.DateOnly)
9191
}
9292

93+
// parseIssueNumber parses the issue number from the given issue ID string.
94+
// The issue ID string can be in one of the following formats:
95+
// - "12345" (by default, assume it is a golang/go project's issue)
96+
// - "golang/go#12345"
97+
// - "github.com/golang/go/issues/12345" or "https://github.com/golang/go/issues/12345"
98+
// - "go.dev/issues/12345" or "https://go.dev/issues/12345"
99+
func parseIssueNumber(issueID string) (project string, issue int64, _ error) {
100+
issueID = strings.TrimSpace(issueID)
101+
if issueID == "" {
102+
return "", 0, nil
103+
}
104+
split := func(q string) (string, string) {
105+
q = strings.TrimPrefix(q, "https://")
106+
// recognize github.com/golang/go/issues/12345
107+
if proj, ok := strings.CutPrefix(q, "github.com/"); ok {
108+
i := strings.LastIndex(proj, "/issues/")
109+
if i < 0 {
110+
return "", q
111+
}
112+
return proj[:i], proj[i+len("/issues/"):]
113+
}
114+
// recognize "go.dev/issues/12345"
115+
if num, ok := strings.CutPrefix(q, "go.dev/issues/"); ok {
116+
return "golang/go", num
117+
}
118+
// recognize golang/go#12345
119+
if proj, num, ok := strings.Cut(q, "#"); ok {
120+
return proj, num
121+
}
122+
return "", q
123+
}
124+
proj, num := split(issueID)
125+
issue, err := strconv.ParseInt(num, 10, 64)
126+
if err != nil || issue <= 0 {
127+
return "", 0, fmt.Errorf("invalid issue number %q", issueID)
128+
}
129+
return proj, issue, nil
130+
}
131+
93132
// populateOverviewPage returns the contents of the overview page.
94133
func (g *Gaby) populateOverviewPage(r *http.Request) overviewPage {
95134
p := overviewPage{
@@ -98,20 +137,22 @@ func (g *Gaby) populateOverviewPage(r *http.Request) overviewPage {
98137
OverviewType: r.FormValue("t"),
99138
},
100139
}
101-
q := strings.TrimSpace(p.Form.Query)
102-
if q == "" {
140+
proj, issue, err := parseIssueNumber(p.Form.Query)
141+
if err != nil {
142+
p.Error = fmt.Errorf("invalid form value: %v", err)
103143
return p
104144
}
105-
issue, err := strconv.ParseInt(q, 10, 64)
106-
if err != nil {
107-
p.Error = fmt.Errorf("invalid form value %q: %w", q, err)
145+
if proj == "" {
146+
proj = g.githubProject // default to golang/go
147+
}
148+
if g.githubProject != proj {
149+
p.Error = fmt.Errorf("invalid form value (unrecognized project): %q", p.Form.Query)
108150
return p
109151
}
110-
if issue < 0 {
111-
p.Error = fmt.Errorf("invalid form value %q", q)
152+
if issue <= 0 {
112153
return p
113154
}
114-
overview, err := g.overview(r.Context(), issue, p.Form.OverviewType)
155+
overview, err := g.overview(r.Context(), proj, issue, p.Form.OverviewType)
115156
if err != nil {
116157
p.Error = err
117158
return p
@@ -121,20 +162,20 @@ func (g *Gaby) populateOverviewPage(r *http.Request) overviewPage {
121162
}
122163

123164
// overview generates an overview of the issue of the given type.
124-
func (g *Gaby) overview(ctx context.Context, issue int64, overviewType string) (*overviewResult, error) {
165+
func (g *Gaby) overview(ctx context.Context, proj string, issue int64, overviewType string) (*overviewResult, error) {
125166
switch overviewType {
126167
case "", issueOverviewType:
127-
return g.issueOverview(ctx, issue)
168+
return g.issueOverview(ctx, proj, issue)
128169
case relatedOverviewType:
129-
return g.relatedOverview(ctx, issue)
170+
return g.relatedOverview(ctx, proj, issue)
130171
default:
131172
return nil, fmt.Errorf("unknown overview type %q", overviewType)
132173
}
133174
}
134175

135176
// issueOverview generates an overview of the issue and its comments.
136-
func (g *Gaby) issueOverview(ctx context.Context, issue int64) (*overviewResult, error) {
137-
overview, err := github.IssueOverview(ctx, g.llm, g.db, g.githubProject, issue)
177+
func (g *Gaby) issueOverview(ctx context.Context, proj string, issue int64) (*overviewResult, error) {
178+
overview, err := github.IssueOverview(ctx, g.llm, g.db, proj, issue)
138179
if err != nil {
139180
return nil, err
140181
}
@@ -145,8 +186,8 @@ func (g *Gaby) issueOverview(ctx context.Context, issue int64) (*overviewResult,
145186
}
146187

147188
// relatedOverview generates an overview of the issue and its related documents.
148-
func (g *Gaby) relatedOverview(ctx context.Context, issue int64) (*overviewResult, error) {
149-
iss, err := github.LookupIssue(g.db, g.githubProject, issue)
189+
func (g *Gaby) relatedOverview(ctx context.Context, proj string, issue int64) (*overviewResult, error) {
190+
iss, err := github.LookupIssue(g.db, proj, issue)
150191
if err != nil {
151192
return nil, err
152193
}
@@ -165,7 +206,7 @@ func (g *Gaby) relatedOverview(ctx context.Context, issue int64) (*overviewResul
165206
}
166207

167208
// Related returns the relative URL of the related-entity search
168-
// for the issue.
209+
// for the issue. This is used in the overview page template.
169210
func (r *overviewResult) Related() string {
170211
return fmt.Sprintf("/search?q=%s", r.Issue.HTMLURL)
171212
}

internal/gaby/overview_test.go

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,3 +188,72 @@ func TestPopulateOverviewPage(t *testing.T) {
188188
}
189189

190190
}
191+
192+
func TestParseOverviewPageQuery(t *testing.T) {
193+
tests := []struct {
194+
in string
195+
wantProject string
196+
wantIssue int64
197+
wantErr bool
198+
}{
199+
{
200+
in: "",
201+
},
202+
{
203+
in: "12345",
204+
wantIssue: 12345,
205+
},
206+
{
207+
in: "golang/go#12345",
208+
wantProject: "golang/go",
209+
wantIssue: 12345,
210+
},
211+
{
212+
in: " 123",
213+
wantIssue: 123,
214+
},
215+
{
216+
in: "x012x",
217+
wantErr: true,
218+
},
219+
{
220+
in: "golang/go",
221+
wantErr: true,
222+
},
223+
{
224+
in: "https://github.com/foo/bar/issues/12345",
225+
wantProject: "foo/bar",
226+
wantIssue: 12345,
227+
},
228+
{
229+
in: "https://go.dev/issues/234",
230+
wantProject: "golang/go",
231+
wantIssue: 234,
232+
},
233+
{
234+
in: "github.com/foo/bar/issues/12345",
235+
wantProject: "foo/bar",
236+
wantIssue: 12345,
237+
},
238+
{
239+
in: "go.dev/issues/234",
240+
wantProject: "golang/go",
241+
wantIssue: 234,
242+
},
243+
{
244+
in: "https://example.com/foo/bar/issues/12345",
245+
wantErr: true,
246+
},
247+
}
248+
for _, tt := range tests {
249+
t.Run(tt.in, func(t *testing.T) {
250+
proj, issue, err := parseIssueNumber(tt.in)
251+
if (err != nil) != tt.wantErr {
252+
t.Fatalf("parseOverviewPageQuery(%q) error = %v, wantErr %v", tt.in, err, tt.wantErr)
253+
}
254+
if proj != tt.wantProject || issue != tt.wantIssue {
255+
t.Errorf("parseOverviewPageQuery(%q) = (%q, %q, %v), want (%q, %q, _)", tt.in, proj, issue, err, tt.wantProject, tt.wantIssue)
256+
}
257+
})
258+
}
259+
}

internal/gaby/rules.go

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@ package main
77
import (
88
"fmt"
99
"net/http"
10-
"strconv"
11-
"strings"
1210

1311
"github.com/google/safehtml"
1412
"github.com/google/safehtml/template"
@@ -48,20 +46,29 @@ func (g *Gaby) populateRulesPage(r *http.Request) rulesPage {
4846
form := rulesForm{
4947
Query: r.FormValue("q"),
5048
}
49+
p := rulesPage{
50+
rulesForm: form,
51+
}
5152
if form.Query == "" {
52-
return rulesPage{
53-
rulesForm: form,
54-
}
53+
return p
5554
}
56-
issue, err := strconv.Atoi(strings.TrimSpace(form.Query))
55+
proj, issue, err := parseIssueNumber(form.Query)
5756
if err != nil {
58-
return rulesPage{
59-
rulesForm: form,
60-
Error: fmt.Errorf("invalid form value %q: %w", form.Query, err).Error(),
61-
}
57+
p.Error = fmt.Errorf("invalid form value %q: %w", form.Query, err).Error()
58+
return p
59+
}
60+
if proj == "" {
61+
proj = g.githubProject // default to golang/go
62+
}
63+
if g.githubProject != proj {
64+
p.Error = fmt.Errorf("invalid form value (unrecognized project): %q", form.Query).Error()
65+
return p
66+
}
67+
if issue <= 0 {
68+
return p
6269
}
6370
// Find issue in database.
64-
i, err := github.LookupIssue(g.db, g.githubProject, int64(issue))
71+
i, err := github.LookupIssue(g.db, proj, issue)
6572
if err != nil {
6673
return rulesPage{
6774
rulesForm: form,

internal/gaby/tmpl/overviewpage.tmpl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ license that can be found in the LICENSE file.
1414
<div class="filter-tips-box">
1515
<div class="toggle" onclick="toggleTips()">[show/hide input tips]</div>
1616
<ul id="filter-tips">
17-
<li><b>issue</b> (<code>int</code>): the issue ID (in the github.com/golang/go repo) of the issue to summarize</li>
17+
<li><b>issue</b> (<code>int</code>): the issue ID of the issue to summarize (e.g. 1234, golang/go#1234, https://github.com/golang/go/issues/1234)</li>
1818
<li><b>overview type</b> (choice): "issue and comments" generates an overview of the issue and its comments; "related documents" searches for related documents and summarizes them</li>
1919
</ul>
2020
</div>

internal/gaby/tmpl/rulespage.tmpl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ license that can be found in the LICENSE file.
1717
<div class="filter-tips-box">
1818
<div class="toggle" onclick="toggleTips()">[show/hide input tips]</div>
1919
<ul id="filter-tips">
20-
<li><b>issue</b> (<code>int</code>): the issue ID (in the github.com/golang/go repo) of the issue to check</li>
20+
<li><b>issue</b> (<code>int</code>): the issue ID of the issue to check< (e.g. 1234, golang/go#1234, https://github.com/golang/go/issues/1234)/li>
2121
</ul>
2222
</div>
2323
<form id="form" action="/rules" method="GET">

internal/github/overview.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func IssueOverview(ctx context.Context, lc *llmapp.Client, db storage.DB, projec
4242
comments = append(comments, doc)
4343
}
4444
if post == nil {
45-
return nil, fmt.Errorf("github.IssueOverview: issue %d not in db", issue)
45+
return nil, fmt.Errorf("github.IssueOverview: issue %s#%d not in db", project, issue)
4646
}
4747
overview, err := lc.PostOverview(ctx, post, comments)
4848
if err != nil {

0 commit comments

Comments
 (0)