fix(health): use effective list filters
This commit is contained in:
parent
b45241dcda
commit
a1d3a85ab8
|
|
@ -9,36 +9,57 @@ import (
|
||||||
)
|
)
|
||||||
|
|
||||||
// v1RepoPath constructs the v1 API path for a repository.
|
// v1RepoPath constructs the v1 API path for a repository.
|
||||||
// Issue operations use the v1 API (/v1/{owner}/{repo}) to match shortcuts/issue conventions,
|
|
||||||
// while PR operations use the v2 API via ctx.RepoPath() (/{owner}/{repo}/pulls) to match shortcuts/pr conventions.
|
|
||||||
|
|
||||||
func v1RepoPath(owner, repo string) string {
|
func v1RepoPath(owner, repo string) string {
|
||||||
return fmt.Sprintf("/v1/%s/%s", owner, repo)
|
return fmt.Sprintf("/v1/%s/%s", owner, repo)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func normalizePRListStatus(state string) string {
|
||||||
|
switch state {
|
||||||
|
case "open", "opened":
|
||||||
|
return "0"
|
||||||
|
case "merged":
|
||||||
|
return "1"
|
||||||
|
case "closed":
|
||||||
|
return "2"
|
||||||
|
default:
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func normalizeIssueListCategory(state string) string {
|
||||||
|
switch state {
|
||||||
|
case "open", "opened":
|
||||||
|
return "opened"
|
||||||
|
case "closed":
|
||||||
|
return "closed"
|
||||||
|
default:
|
||||||
|
return "all"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func fetchPRListPage(ctx *common.RuntimeContext, state string, page, limit int) ([]interface{}, error) {
|
func fetchPRListPage(ctx *common.RuntimeContext, state string, page, limit int) ([]interface{}, error) {
|
||||||
q := url.Values{}
|
q := url.Values{}
|
||||||
q.Set("page", fmt.Sprintf("%d", page))
|
q.Set("page", fmt.Sprintf("%d", page))
|
||||||
q.Set("limit", fmt.Sprintf("%d", limit))
|
q.Set("limit", fmt.Sprintf("%d", limit))
|
||||||
if state != "" {
|
if status := normalizePRListStatus(state); status != "" {
|
||||||
q.Set("state", state)
|
q.Set("status", status)
|
||||||
}
|
}
|
||||||
env, err := ctx.CallAPIWithQuery("GET", ctx.RepoPath()+"/pulls", q)
|
env, err := ctx.CallAPIWithQuery("GET", v1RepoPath(ctx.Owner, ctx.Repo)+"/pulls", q)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
fmt.Fprintf(os.Stderr, " CLI error: pr +list state=%s page=%d: %v\n", state, page, err)
|
fmt.Fprintf(os.Stderr, " CLI error: pr list state=%s page=%d: %v\n", state, page, err)
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
if !env.OK {
|
if !env.OK {
|
||||||
err := fmt.Errorf("API error: pr +list state=%s page=%d", state, page)
|
err := fmt.Errorf("API error: pr list state=%s page=%d", state, page)
|
||||||
fmt.Fprintf(os.Stderr, " %v\n", err)
|
fmt.Fprintf(os.Stderr, " %v\n", err)
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
data, ok := env.Data.(map[string]interface{})
|
data, ok := env.Data.(map[string]interface{})
|
||||||
if !ok {
|
if !ok {
|
||||||
return nil, fmt.Errorf("unexpected response type for pr +list")
|
return nil, fmt.Errorf("unexpected response type for pr list")
|
||||||
}
|
}
|
||||||
issues, _ := data["issues"].([]interface{})
|
pulls, _ := data["pulls"].([]interface{})
|
||||||
return issues, nil
|
return pulls, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// fetchPRDetail retrieves full PR detail to extract merged_at timestamp.
|
// fetchPRDetail retrieves full PR detail to extract merged_at timestamp.
|
||||||
|
|
@ -86,22 +107,20 @@ func fetchIssueListPage(ctx *common.RuntimeContext, owner, repo, state string, p
|
||||||
q := url.Values{}
|
q := url.Values{}
|
||||||
q.Set("page", fmt.Sprintf("%d", page))
|
q.Set("page", fmt.Sprintf("%d", page))
|
||||||
q.Set("limit", fmt.Sprintf("%d", limit))
|
q.Set("limit", fmt.Sprintf("%d", limit))
|
||||||
if state != "" {
|
q.Set("category", normalizeIssueListCategory(state))
|
||||||
q.Set("state", state)
|
|
||||||
}
|
|
||||||
env, err := ctx.CallAPIWithQuery("GET", v1RepoPath(owner, repo)+"/issues", q)
|
env, err := ctx.CallAPIWithQuery("GET", v1RepoPath(owner, repo)+"/issues", q)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
fmt.Fprintf(os.Stderr, " CLI error: issue +list state=%s page=%d: %v\n", state, page, err)
|
fmt.Fprintf(os.Stderr, " CLI error: issue list state=%s page=%d: %v\n", state, page, err)
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
if !env.OK {
|
if !env.OK {
|
||||||
err := fmt.Errorf("API error: issue +list state=%s page=%d", state, page)
|
err := fmt.Errorf("API error: issue list state=%s page=%d", state, page)
|
||||||
fmt.Fprintf(os.Stderr, " %v\n", err)
|
fmt.Fprintf(os.Stderr, " %v\n", err)
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
data, ok := env.Data.(map[string]interface{})
|
data, ok := env.Data.(map[string]interface{})
|
||||||
if !ok {
|
if !ok {
|
||||||
return nil, fmt.Errorf("unexpected response type for issue +list")
|
return nil, fmt.Errorf("unexpected response type for issue list")
|
||||||
}
|
}
|
||||||
issues, _ := data["issues"].([]interface{})
|
issues, _ := data["issues"].([]interface{})
|
||||||
return issues, nil
|
return issues, nil
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,108 @@
|
||||||
|
package health
|
||||||
|
|
||||||
|
import (
|
||||||
|
"encoding/json"
|
||||||
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/gitlink-org/gitlink-cli/internal/client"
|
||||||
|
"github.com/gitlink-org/gitlink-cli/shortcuts/common"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestFetchPRListPageUsesV1StatusFilter(t *testing.T) {
|
||||||
|
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
if r.Method != "GET" {
|
||||||
|
t.Fatalf("method=%s, want GET", r.Method)
|
||||||
|
}
|
||||||
|
if r.URL.Path != "/v1/owner/repo/pulls.json" {
|
||||||
|
t.Fatalf("path=%s, want /v1/owner/repo/pulls.json", r.URL.Path)
|
||||||
|
}
|
||||||
|
if got := r.URL.Query().Get("status"); got != "1" {
|
||||||
|
t.Fatalf("status=%q, want 1", got)
|
||||||
|
}
|
||||||
|
if got := r.URL.Query().Get("state"); got != "" {
|
||||||
|
t.Fatalf("state should not be sent, got %q", got)
|
||||||
|
}
|
||||||
|
writeHealthJSON(t, w, map[string]interface{}{
|
||||||
|
"total_count": 1,
|
||||||
|
"pulls": []map[string]interface{}{
|
||||||
|
{"id": 15414, "index": 109, "status": "merged"},
|
||||||
|
},
|
||||||
|
})
|
||||||
|
}))
|
||||||
|
defer server.Close()
|
||||||
|
|
||||||
|
pulls, err := fetchPRListPage(healthTestContext(server), "merged", 2, 5)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("fetchPRListPage failed: %v", err)
|
||||||
|
}
|
||||||
|
if len(pulls) != 1 {
|
||||||
|
t.Fatalf("len(pulls)=%d, want 1", len(pulls))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestFetchIssueListPageUsesCategoryFilter(t *testing.T) {
|
||||||
|
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
if r.Method != "GET" {
|
||||||
|
t.Fatalf("method=%s, want GET", r.Method)
|
||||||
|
}
|
||||||
|
if r.URL.Path != "/v1/owner/repo/issues.json" {
|
||||||
|
t.Fatalf("path=%s, want /v1/owner/repo/issues.json", r.URL.Path)
|
||||||
|
}
|
||||||
|
if got := r.URL.Query().Get("category"); got != "closed" {
|
||||||
|
t.Fatalf("category=%q, want closed", got)
|
||||||
|
}
|
||||||
|
if got := r.URL.Query().Get("state"); got != "" {
|
||||||
|
t.Fatalf("state should not be sent, got %q", got)
|
||||||
|
}
|
||||||
|
writeHealthJSON(t, w, map[string]interface{}{
|
||||||
|
"total_count": 1,
|
||||||
|
"issues": []map[string]interface{}{
|
||||||
|
{"id": 140801, "project_issues_index": 1},
|
||||||
|
},
|
||||||
|
})
|
||||||
|
}))
|
||||||
|
defer server.Close()
|
||||||
|
|
||||||
|
issues, err := fetchIssueListPage(healthTestContext(server), "owner", "repo", "closed", 1, 20)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("fetchIssueListPage failed: %v", err)
|
||||||
|
}
|
||||||
|
if len(issues) != 1 {
|
||||||
|
t.Fatalf("len(issues)=%d, want 1", len(issues))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestNormalizeListFilters(t *testing.T) {
|
||||||
|
prCases := map[string]string{"open": "0", "opened": "0", "merged": "1", "closed": "2", "all": ""}
|
||||||
|
for input, want := range prCases {
|
||||||
|
if got := normalizePRListStatus(input); got != want {
|
||||||
|
t.Fatalf("normalizePRListStatus(%q)=%q, want %q", input, got, want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
issueCases := map[string]string{"open": "opened", "opened": "opened", "closed": "closed", "all": "all"}
|
||||||
|
for input, want := range issueCases {
|
||||||
|
if got := normalizeIssueListCategory(input); got != want {
|
||||||
|
t.Fatalf("normalizeIssueListCategory(%q)=%q, want %q", input, got, want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func healthTestContext(server *httptest.Server) *common.RuntimeContext {
|
||||||
|
return &common.RuntimeContext{
|
||||||
|
Client: &client.Client{HTTP: server.Client(), BaseURL: server.URL},
|
||||||
|
Owner: "owner",
|
||||||
|
Repo: "repo",
|
||||||
|
Format: "json",
|
||||||
|
Args: map[string]string{},
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func writeHealthJSON(t *testing.T, w http.ResponseWriter, payload interface{}) {
|
||||||
|
t.Helper()
|
||||||
|
w.Header().Set("Content-Type", "application/json")
|
||||||
|
if err := json.NewEncoder(w).Encode(payload); err != nil {
|
||||||
|
t.Fatalf("write json: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -139,6 +139,97 @@ func extractTagNames(data map[string]interface{}, key string) []string {
|
||||||
return names
|
return names
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func nestedMap(data map[string]interface{}, key string) map[string]interface{} {
|
||||||
|
obj, _ := data[key].(map[string]interface{})
|
||||||
|
return obj
|
||||||
|
}
|
||||||
|
|
||||||
|
func extractPullNumber(pr map[string]interface{}) int {
|
||||||
|
for _, key := range []string{"pull_request_number", "index"} {
|
||||||
|
if v, ok := pr[key].(float64); ok && v > 0 {
|
||||||
|
return int(v)
|
||||||
|
}
|
||||||
|
if v, ok := pr[key].(int); ok && v > 0 {
|
||||||
|
return v
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return 0
|
||||||
|
}
|
||||||
|
|
||||||
|
func extractPullAuthorLogin(pr map[string]interface{}) string {
|
||||||
|
if login, _ := pr["author_login"].(string); login != "" {
|
||||||
|
return login
|
||||||
|
}
|
||||||
|
if login := extractLogin(pr, "author", ""); login != "" {
|
||||||
|
return login
|
||||||
|
}
|
||||||
|
if issue := nestedMap(pr, "issue"); issue != nil {
|
||||||
|
return extractLogin(issue, "author", "")
|
||||||
|
}
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
|
||||||
|
func extractPullAssigneeLogin(pr map[string]interface{}) string {
|
||||||
|
if login, _ := pr["assign_user_login"].(string); login != "" {
|
||||||
|
return login
|
||||||
|
}
|
||||||
|
if issue := nestedMap(pr, "issue"); issue != nil {
|
||||||
|
if login := extractLogin(issue, "assign_user", "assign_user_login"); login != "" {
|
||||||
|
return login
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
|
||||||
|
func extractPullStatus(pr map[string]interface{}) string {
|
||||||
|
statusMap := map[int]string{0: "open", 1: "merged", 2: "closed"}
|
||||||
|
if v, ok := pr["pull_request_status"].(float64); ok {
|
||||||
|
return statusMap[int(v)]
|
||||||
|
}
|
||||||
|
if v, ok := pr["pull_request_status"].(int); ok {
|
||||||
|
return statusMap[v]
|
||||||
|
}
|
||||||
|
for _, key := range []string{"pull_request_staus", "status", "state"} {
|
||||||
|
if status, _ := pr[key].(string); status != "" {
|
||||||
|
switch status {
|
||||||
|
case "open", "opened":
|
||||||
|
return "open"
|
||||||
|
case "merged":
|
||||||
|
return "merged"
|
||||||
|
case "closed", "close":
|
||||||
|
return "closed"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return "open"
|
||||||
|
}
|
||||||
|
|
||||||
|
func extractPullCreateTime(pr map[string]interface{}) string {
|
||||||
|
for _, key := range []string{"pr_full_time", "created_at", "create_time"} {
|
||||||
|
if v, _ := pr[key].(string); v != "" {
|
||||||
|
return v
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if issue := nestedMap(pr, "issue"); issue != nil {
|
||||||
|
for _, key := range []string{"created_at", "create_time"} {
|
||||||
|
if v, _ := issue[key].(string); v != "" {
|
||||||
|
return v
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
|
||||||
|
func extractPullTagNames(pr map[string]interface{}) []string {
|
||||||
|
if names := extractTagNames(pr, "issue_tags"); len(names) > 0 {
|
||||||
|
return names
|
||||||
|
}
|
||||||
|
if issue := nestedMap(pr, "issue"); issue != nil {
|
||||||
|
return extractTagNames(issue, "issue_tags")
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
// mergeTimeFromList tries to extract merged_at directly from the list API response
|
// mergeTimeFromList tries to extract merged_at directly from the list API response
|
||||||
// (preferred, avoids an extra API call per PR).
|
// (preferred, avoids an extra API call per PR).
|
||||||
func mergeTimeFromList(pr map[string]interface{}) string {
|
func mergeTimeFromList(pr map[string]interface{}) string {
|
||||||
|
|
@ -165,28 +256,22 @@ func savePull(db *sql.DB, repoID int, pr map[string]interface{}, mergedAt string
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
prNumber, _ := pr["pull_request_number"].(float64)
|
prNumber := extractPullNumber(pr)
|
||||||
if prNumber == 0 {
|
if prNumber == 0 {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
author, _ := pr["author_login"].(string)
|
createrID, _ := getOrCreateUser(db, extractPullAuthorLogin(pr))
|
||||||
createrID, _ := getOrCreateUser(db, author)
|
|
||||||
|
|
||||||
statusCode := 0
|
status := extractPullStatus(pr)
|
||||||
if v, ok := pr["pull_request_status"].(float64); ok {
|
|
||||||
statusCode = int(v)
|
|
||||||
}
|
|
||||||
statusMap := map[int]string{0: "open", 1: "merged", 2: "closed"}
|
|
||||||
status := statusMap[statusCode]
|
|
||||||
if status == "" {
|
if status == "" {
|
||||||
status = "open"
|
status = "open"
|
||||||
}
|
}
|
||||||
|
|
||||||
createTime, _ := pr["pr_full_time"].(string)
|
createTime := extractPullCreateTime(pr)
|
||||||
|
|
||||||
var processorID *int
|
var processorID *int
|
||||||
if assignee, _ := pr["assign_user_login"].(string); assignee != "" {
|
if assignee := extractPullAssigneeLogin(pr); assignee != "" {
|
||||||
pid, _ := getOrCreateUser(db, assignee)
|
pid, _ := getOrCreateUser(db, assignee)
|
||||||
processorID = &pid
|
processorID = &pid
|
||||||
}
|
}
|
||||||
|
|
@ -201,12 +286,12 @@ func savePull(db *sql.DB, repoID int, pr map[string]interface{}, mergedAt string
|
||||||
|
|
||||||
if _, err := db.Exec(`INSERT OR REPLACE INTO pulls (id, repo_id, number, creater_id, status, processor_id, create_time, merged_at)
|
if _, err := db.Exec(`INSERT OR REPLACE INTO pulls (id, repo_id, number, creater_id, status, processor_id, create_time, merged_at)
|
||||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?)`,
|
VALUES (?, ?, ?, ?, ?, ?, ?, ?)`,
|
||||||
int(prID), repoID, int(prNumber), createrID, status, processorID, createTime, mergedAtVal); err != nil {
|
int(prID), repoID, prNumber, createrID, status, processorID, createTime, mergedAtVal); err != nil {
|
||||||
fmt.Fprintf(os.Stderr, " DB error: save pull %d: %v\n", int(prID), err)
|
fmt.Fprintf(os.Stderr, " DB error: save pull %d: %v\n", int(prID), err)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Save tags
|
// Save tags
|
||||||
tagNames := extractTagNames(pr, "issue_tags")
|
tagNames := extractPullTagNames(pr)
|
||||||
var tagIDs []int
|
var tagIDs []int
|
||||||
for _, name := range tagNames {
|
for _, name := range tagNames {
|
||||||
if tid, err := getOrCreateTag(db, repoID, name); err == nil {
|
if tid, err := getOrCreateTag(db, repoID, name); err == nil {
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,62 @@
|
||||||
|
package health
|
||||||
|
|
||||||
|
import (
|
||||||
|
"path/filepath"
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestSavePullAcceptsV1ListShape(t *testing.T) {
|
||||||
|
db, err := openDB(filepath.Join(t.TempDir(), "health.db"))
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("openDB: %v", err)
|
||||||
|
}
|
||||||
|
defer db.Close()
|
||||||
|
|
||||||
|
repoID, err := getOrCreateRepo(db, "repo", "owner")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("getOrCreateRepo: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
savePull(db, repoID, map[string]interface{}{
|
||||||
|
"id": float64(15414),
|
||||||
|
"index": float64(109),
|
||||||
|
"status": "merged",
|
||||||
|
"issue": map[string]interface{}{
|
||||||
|
"author": map[string]interface{}{"login": "alice"},
|
||||||
|
"issue_tags": []interface{}{
|
||||||
|
map[string]interface{}{"name": "docs"},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}, "2026-06-05T12:00:00+08:00")
|
||||||
|
|
||||||
|
var number int
|
||||||
|
var status string
|
||||||
|
var author string
|
||||||
|
var mergedAt string
|
||||||
|
if err := db.QueryRow(`
|
||||||
|
SELECT pulls.number, pulls.status, users.user_name, pulls.merged_at
|
||||||
|
FROM pulls JOIN users ON users.id = pulls.creater_id
|
||||||
|
WHERE pulls.id = ?`, 15414).Scan(&number, &status, &author, &mergedAt); err != nil {
|
||||||
|
t.Fatalf("query saved pull: %v", err)
|
||||||
|
}
|
||||||
|
if number != 109 {
|
||||||
|
t.Fatalf("number=%d, want 109", number)
|
||||||
|
}
|
||||||
|
if status != "merged" {
|
||||||
|
t.Fatalf("status=%q, want merged", status)
|
||||||
|
}
|
||||||
|
if author != "alice" {
|
||||||
|
t.Fatalf("author=%q, want alice", author)
|
||||||
|
}
|
||||||
|
if mergedAt == "" {
|
||||||
|
t.Fatal("merged_at was not saved")
|
||||||
|
}
|
||||||
|
|
||||||
|
var tagCount int
|
||||||
|
if err := db.QueryRow(`SELECT COUNT(*) FROM pull_tags WHERE pull_id = ?`, 15414).Scan(&tagCount); err != nil {
|
||||||
|
t.Fatalf("query tags: %v", err)
|
||||||
|
}
|
||||||
|
if tagCount != 1 {
|
||||||
|
t.Fatalf("tagCount=%d, want 1", tagCount)
|
||||||
|
}
|
||||||
|
}
|
||||||
Loading…
Reference in New Issue