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 <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Zvonimir Pavlinovic <[email protected]>
golang · Nov 14, 2024 · 0937c9d · 0937c9d
1 parent 498d98c
commit 0937c9d
Show file tree

Hide file tree

Showing 6 changed files with 148 additions and 31 deletions.
diff --git a/internal/gaby/overview.go b/internal/gaby/overview.go
@@ -35,7 +35,7 @@ type overviewResult struct {
 
 // overviewForm holds the raw inputs to the overview form.
 type overviewForm struct {
-	Query        string // the issue ID to lookup
+	Query        string // the issue ID to lookup, or golang/go#12345 or github.com/golang/go/issues/12345 form
 	OverviewType string // the type of overview to generate
 }
 
@@ -90,6 +90,45 @@ func fmtTimeString(s string) string {
 	return t.Format(time.DateOnly)
 }
 
+// parseIssueNumber parses the issue number from the given issue ID string.
+// The issue ID string can be in one of the following formats:
+//   - "12345" (by default, assume it is a golang/go project's issue)
+//   - "golang/go#12345"
+//   - "github.com/golang/go/issues/12345" or "https://github.com/golang/go/issues/12345"
+//   - "go.dev/issues/12345" or "https://go.dev/issues/12345"
+func parseIssueNumber(issueID string) (project string, issue int64, _ error) {
+	issueID = strings.TrimSpace(issueID)
+	if issueID == "" {
+		return "", 0, nil
+	}
+	split := func(q string) (string, string) {
+		q = strings.TrimPrefix(q, "https://")
+		// recognize github.com/golang/go/issues/12345
+		if proj, ok := strings.CutPrefix(q, "github.com/"); ok {
+			i := strings.LastIndex(proj, "/issues/")
+			if i < 0 {
+				return "", q
+			}
+			return proj[:i], proj[i+len("/issues/"):]
+		}
+		// recognize "go.dev/issues/12345"
+		if num, ok := strings.CutPrefix(q, "go.dev/issues/"); ok {
+			return "golang/go", num
+		}
+		// recognize golang/go#12345
+		if proj, num, ok := strings.Cut(q, "#"); ok {
+			return proj, num
+		}
+		return "", q
+	}
+	proj, num := split(issueID)
+	issue, err := strconv.ParseInt(num, 10, 64)
+	if err != nil || issue <= 0 {
+		return "", 0, fmt.Errorf("invalid issue number %q", issueID)
+	}
+	return proj, issue, nil
+}
+
 // populateOverviewPage returns the contents of the overview page.
 func (g *Gaby) populateOverviewPage(r *http.Request) overviewPage {
 	p := overviewPage{
@@ -98,20 +137,22 @@ func (g *Gaby) populateOverviewPage(r *http.Request) overviewPage {
 			OverviewType: r.FormValue("t"),
 		},
 	}
-	q := strings.TrimSpace(p.Form.Query)
-	if q == "" {
+	proj, issue, err := parseIssueNumber(p.Form.Query)
+	if err != nil {
+		p.Error = fmt.Errorf("invalid form value: %v", err)
 		return p
 	}
-	issue, err := strconv.ParseInt(q, 10, 64)
-	if err != nil {
-		p.Error = fmt.Errorf("invalid form value %q: %w", q, err)
+	if proj == "" {
+		proj = g.githubProject // default to golang/go
+	}
+	if g.githubProject != proj {
+		p.Error = fmt.Errorf("invalid form value (unrecognized project): %q", p.Form.Query)
 		return p
 	}
-	if issue < 0 {
-		p.Error = fmt.Errorf("invalid form value %q", q)
+	if issue <= 0 {
 		return p
 	}
-	overview, err := g.overview(r.Context(), issue, p.Form.OverviewType)
+	overview, err := g.overview(r.Context(), proj, issue, p.Form.OverviewType)
 	if err != nil {
 		p.Error = err
 		return p
@@ -121,20 +162,20 @@ func (g *Gaby) populateOverviewPage(r *http.Request) overviewPage {
 }
 
 // overview generates an overview of the issue of the given type.
-func (g *Gaby) overview(ctx context.Context, issue int64, overviewType string) (*overviewResult, error) {
+func (g *Gaby) overview(ctx context.Context, proj string, issue int64, overviewType string) (*overviewResult, error) {
 	switch overviewType {
 	case "", issueOverviewType:
-		return g.issueOverview(ctx, issue)
+		return g.issueOverview(ctx, proj, issue)
 	case relatedOverviewType:
-		return g.relatedOverview(ctx, issue)
+		return g.relatedOverview(ctx, proj, issue)
 	default:
 		return nil, fmt.Errorf("unknown overview type %q", overviewType)
 	}
 }
 
 // issueOverview generates an overview of the issue and its comments.
-func (g *Gaby) issueOverview(ctx context.Context, issue int64) (*overviewResult, error) {
-	overview, err := github.IssueOverview(ctx, g.llm, g.db, g.githubProject, issue)
+func (g *Gaby) issueOverview(ctx context.Context, proj string, issue int64) (*overviewResult, error) {
+	overview, err := github.IssueOverview(ctx, g.llm, g.db, proj, issue)
 	if err != nil {
 		return nil, err
 	}
@@ -145,8 +186,8 @@ func (g *Gaby) issueOverview(ctx context.Context, issue int64) (*overviewResult,
 }
 
 // relatedOverview generates an overview of the issue and its related documents.
-func (g *Gaby) relatedOverview(ctx context.Context, issue int64) (*overviewResult, error) {
-	iss, err := github.LookupIssue(g.db, g.githubProject, issue)
+func (g *Gaby) relatedOverview(ctx context.Context, proj string, issue int64) (*overviewResult, error) {
+	iss, err := github.LookupIssue(g.db, proj, issue)
 	if err != nil {
 		return nil, err
 	}
@@ -165,7 +206,7 @@ func (g *Gaby) relatedOverview(ctx context.Context, issue int64) (*overviewResul
 }
 
 // Related returns the relative URL of the related-entity search
-// for the issue.
+// for the issue. This is used in the overview page template.
 func (r *overviewResult) Related() string {
 	return fmt.Sprintf("/search?q=%s", r.Issue.HTMLURL)
 }
diff --git a/internal/gaby/overview_test.go b/internal/gaby/overview_test.go
@@ -188,3 +188,72 @@ func TestPopulateOverviewPage(t *testing.T) {
 	}
 
 }
+
+func TestParseOverviewPageQuery(t *testing.T) {
+	tests := []struct {
+		in          string
+		wantProject string
+		wantIssue   int64
+		wantErr     bool
+	}{
+		{
+			in: "",
+		},
+		{
+			in:        "12345",
+			wantIssue: 12345,
+		},
+		{
+			in:          "golang/go#12345",
+			wantProject: "golang/go",
+			wantIssue:   12345,
+		},
+		{
+			in:        " 123",
+			wantIssue: 123,
+		},
+		{
+			in:      "x012x",
+			wantErr: true,
+		},
+		{
+			in:      "golang/go",
+			wantErr: true,
+		},
+		{
+			in:          "https://github.com/foo/bar/issues/12345",
+			wantProject: "foo/bar",
+			wantIssue:   12345,
+		},
+		{
+			in:          "https://go.dev/issues/234",
+			wantProject: "golang/go",
+			wantIssue:   234,
+		},
+		{
+			in:          "github.com/foo/bar/issues/12345",
+			wantProject: "foo/bar",
+			wantIssue:   12345,
+		},
+		{
+			in:          "go.dev/issues/234",
+			wantProject: "golang/go",
+			wantIssue:   234,
+		},
+		{
+			in:      "https://example.com/foo/bar/issues/12345",
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.in, func(t *testing.T) {
+			proj, issue, err := parseIssueNumber(tt.in)
+			if (err != nil) != tt.wantErr {
+				t.Fatalf("parseOverviewPageQuery(%q) error = %v, wantErr %v", tt.in, err, tt.wantErr)
+			}
+			if proj != tt.wantProject || issue != tt.wantIssue {
+				t.Errorf("parseOverviewPageQuery(%q) = (%q, %q, %v), want (%q, %q, _)", tt.in, proj, issue, err, tt.wantProject, tt.wantIssue)
+			}
+		})
+	}
+}
diff --git a/internal/gaby/rules.go b/internal/gaby/rules.go
@@ -7,8 +7,6 @@ package main
 import (
 	"fmt"
 	"net/http"
-	"strconv"
-	"strings"
 
 	"github.com/google/safehtml"
 	"github.com/google/safehtml/template"
@@ -48,20 +46,29 @@ func (g *Gaby) populateRulesPage(r *http.Request) rulesPage {
 	form := rulesForm{
 		Query: r.FormValue("q"),
 	}
+	p := rulesPage{
+		rulesForm: form,
+	}
 	if form.Query == "" {
-		return rulesPage{
-			rulesForm: form,
-		}
+		return p
 	}
-	issue, err := strconv.Atoi(strings.TrimSpace(form.Query))
+	proj, issue, err := parseIssueNumber(form.Query)
 	if err != nil {
-		return rulesPage{
-			rulesForm: form,
-			Error:     fmt.Errorf("invalid form value %q: %w", form.Query, err).Error(),
-		}
+		p.Error = fmt.Errorf("invalid form value %q: %w", form.Query, err).Error()
+		return p
+	}
+	if proj == "" {
+		proj = g.githubProject // default to golang/go
+	}
+	if g.githubProject != proj {
+		p.Error = fmt.Errorf("invalid form value (unrecognized project): %q", form.Query).Error()
+		return p
+	}
+	if issue <= 0 {
+		return p
 	}
 	// Find issue in database.
-	i, err := github.LookupIssue(g.db, g.githubProject, int64(issue))
+	i, err := github.LookupIssue(g.db, proj, issue)
 	if err != nil {
 		return rulesPage{
 			rulesForm: form,

diff --git a/internal/gaby/tmpl/overviewpage.tmpl b/internal/gaby/tmpl/overviewpage.tmpl
@@ -14,7 +14,7 @@ license that can be found in the LICENSE file.
 		<div class="filter-tips-box">
 			<div class="toggle" onclick="toggleTips()">[show/hide input tips]</div>
 			<ul id="filter-tips">
-			    <li><b>issue</b> (<code>int</code>): the issue ID (in the github.com/golang/go repo) of the issue to summarize</li>
+			    <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>
 				<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>
 			</ul>
 		</div>

diff --git a/internal/gaby/tmpl/rulespage.tmpl b/internal/gaby/tmpl/rulespage.tmpl
@@ -17,7 +17,7 @@ license that can be found in the LICENSE file.
 		<div class="filter-tips-box">
 			<div class="toggle" onclick="toggleTips()">[show/hide input tips]</div>
 			<ul id="filter-tips">
-			    <li><b>issue</b> (<code>int</code>): the issue ID (in the github.com/golang/go repo) of the issue to check</li>
+			    <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>
 			</ul>
 		</div>
 	 <form id="form" action="/rules" method="GET">

diff --git a/internal/github/overview.go b/internal/github/overview.go
@@ -42,7 +42,7 @@ func IssueOverview(ctx context.Context, lc *llmapp.Client, db storage.DB, projec
 		comments = append(comments, doc)
 	}
 	if post == nil {
-		return nil, fmt.Errorf("github.IssueOverview: issue %d not in db", issue)
+		return nil, fmt.Errorf("github.IssueOverview: issue %s#%d not in db", project, issue)
 	}
 	overview, err := lc.PostOverview(ctx, post, comments)
 	if err != nil {