diff --git a/internal/app/issue_filters_test.go b/internal/app/issue_filters_test.go index ab3d131b..01dfe694 100644 --- a/internal/app/issue_filters_test.go +++ b/internal/app/issue_filters_test.go @@ -7,6 +7,7 @@ import ( "context" "net/http" "strconv" + "strings" "testing" "time" @@ -155,6 +156,8 @@ func TestIssueListWebOpensTheFilteredPage(t *testing.T) { deps.Browser = browser require.NoError(t, Execute(context.Background(), deps, []string{"issue", "list", "-R", "alice/project", "-w", "-s", "closed", "-S", "login bug"})) require.Equal(t, server.URL+"/alice/project/issues?q=login+bug&state=closed", browser.resource) + require.NoError(t, Execute(context.Background(), deps, []string{"issue", "list", "-R", "alice/project", "-w", "--no-assignee"})) + require.Equal(t, server.URL+"/alice/project/issues?assignee=%28none%29&state=open", browser.resource) } func TestIssueListKindFilter(t *testing.T) { @@ -270,3 +273,31 @@ func TestIssueCommandsComplete(t *testing.T) { require.Equal(t, cobra.ShellCompDirectiveNoFileComp, directive&cobra.ShellCompDirectiveNoFileComp, test.args) } } + +func TestIssueListNoAssigneeFiltersEveryPageRead(t *testing.T) { + var pages []string + server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + query := r.URL.Query() + require.Empty(t, query.Get("assigned_by")) + require.Equal(t, "50", query.Get("limit")) + pages = append(pages, query.Get("page")) + start, _ := strconv.Atoi(query.Get("page")) + issues := []string{} + for n := (start-1)*50 + 1; n <= min(start*50, 60); n++ { + assignees := `[{"login":"bob"}]` + if n%20 == 0 { + assignees = `[]` + } + issues = append(issues, `{"number":`+strconv.Itoa(n)+`,"state":"open","assignees":`+assignees+`}`) + } + _, _ = w.Write([]byte("[" + strings.Join(issues, ",") + "]")) + })) + defer server.Close() + deps, out, _ := serverDeps(t, server.URL) + require.NoError(t, Execute(context.Background(), deps, []string{"issue", "list", "-R", "alice/project", "--no-assignee", "--json", "number"})) + require.JSONEq(t, `[{"number":20},{"number":40},{"number":60}]`, out.String()) + require.Equal(t, []string{"1", "2"}, pages) + err := Execute(context.Background(), deps, []string{"issue", "list", "-R", "alice/project", "--no-assignee", "--assignee", "bob"}) + require.Equal(t, clierr.Usage, clierr.ExitCode(err)) + require.Len(t, pages, 2) +} diff --git a/internal/app/issues.go b/internal/app/issues.go index cd34cf3e..0975f1cc 100644 --- a/internal/app/issues.go +++ b/internal/app/issues.go @@ -42,12 +42,16 @@ func (a *application) issueListCommand() *cobra.Command { cmd := &cobra.Command{Use: "list", Short: "List issues in a repository", Long: `List the issues of a repository, newest first. Filters combine: an issue must match all of them. Labels and milestones must exist in the repository.`, Example: `$ tea issue list --label bug --state all $ tea issue list --assignee @me --json number,title +$ tea issue list --no-assignee --label bug # Issues and pull requests in a milestone, as tab-separated values $ tea issue list --kind all --milestone v1.0 | cut -f1,2`, Args: argsExact(0), RunE: func(cmd *cobra.Command, _ []string) error { var err error if opts.Since, opts.Before, err = updated.parse(a.deps.Now()); err != nil { return err } + if opts.NoAssignee && opts.Assignee != "" { + return clierr.Wrap(clierr.Usage, errors.New("--assignee and --no-assignee are mutually exclusive")) + } if err := requireNames("label", "", labels); err != nil { return err } @@ -66,7 +70,7 @@ $ tea issue list --kind all --milestone v1.0 | cut -f1,2`, Args: argsExact(0), R if web { return a.openBrowser(cmd, issuesURL(target, opts)) } - filtered := opts.Search+opts.Assignee+opts.Author+opts.Mentions != "" || len(labels)+len(milestones) > 0 || opts.Since != nil || opts.Before != nil + filtered := opts.Search+opts.Assignee+opts.Author+opts.Mentions != "" || len(labels)+len(milestones) > 0 || opts.Since != nil || opts.Before != nil || opts.NoAssignee if opts.Labels, opts.Milestones, err = issueFilters(cmd, target, labels, milestones); err != nil { return err } @@ -82,6 +86,7 @@ $ tea issue list --kind all --milestone v1.0 | cut -f1,2`, Args: argsExact(0), R flags := cmd.Flags() flags.StringVarP(&opts.Search, "search", "S", "", "Search issue titles and bodies") flags.StringVarP(&opts.Assignee, "assignee", "a", "", "Filter by assignee; @me for yourself") + flags.BoolVar(&opts.NoAssignee, "no-assignee", false, "Only issues without an assignee") flags.StringVarP(&opts.Author, "author", "A", "", "Filter by author; @me for yourself") flags.StringVar(&opts.Mentions, "mentions", "", "Filter by mentioned user; @me for yourself") flags.StringSliceVarP(&labels, "label", "l", nil, "Filter by label `names`") @@ -97,7 +102,7 @@ $ tea issue list --kind all --milestone v1.0 | cut -f1,2`, Args: argsExact(0), R } // issuesURL is the web page of the repository's issues or pull requests in state, searched for -// the --search text. +// the --search text and limited to unassigned ones for --no-assignee. func issuesURL(t target, opts services.ListOptions) string { section := "issues" if opts.Kind == "pulls" { @@ -107,6 +112,9 @@ func issuesURL(t target, opts services.ListOptions) string { if opts.Search != "" { query.Set("q", opts.Search) } + if opts.NoAssignee { + query.Set("assignee", "(none)") + } return repositoryURL(t) + "/" + section + "?" + query.Encode() } diff --git a/internal/app/pulls.go b/internal/app/pulls.go index 8cf81061..fb06f358 100644 --- a/internal/app/pulls.go +++ b/internal/app/pulls.go @@ -45,6 +45,9 @@ each pull request, which tea only makes when --json names them.`, Args: argsExac if id, err := strconv.ParseInt(milestone, 10, 64); err == nil && id < 1 { return clierr.Wrap(clierr.Usage, errors.New("--milestone must be a milestone title or a positive ID")) } + if options.NoAssignee && options.Assignee != "" { + return clierr.Wrap(clierr.Usage, errors.New("--assignee and --no-assignee are mutually exclusive")) + } t, err := a.resolve(cmd.Context(), true) if err != nil { return err @@ -106,6 +109,7 @@ each pull request, which tea only makes when --json names them.`, Args: argsExac pageFlags(cmd, &limit, &page) cmd.Flags().StringVarP(&options.Author, "author", "A", "", "Filter by author; @me for yourself") cmd.Flags().StringVarP(&options.Assignee, "assignee", "a", "", "Filter by assignee; @me for yourself") + cmd.Flags().BoolVar(&options.NoAssignee, "no-assignee", false, "Only pull requests without an assignee") cmd.Flags().StringVarP(&options.Base, "base", "B", "", "Filter by base branch") cmd.Flags().StringVarP(&options.Head, "head", "H", "", "Filter by head branch, or OWNER:BRANCH for a fork") cmd.Flags().BoolVarP(&options.Draft, "draft", "d", false, "Show only drafts") diff --git a/internal/services/helpers.go b/internal/services/helpers.go index 1742fe73..65403ebe 100644 --- a/internal/services/helpers.go +++ b/internal/services/helpers.go @@ -5,6 +5,7 @@ package services import ( "context" + "errors" "fmt" "maps" "net/http" @@ -48,6 +49,45 @@ func listAll[T any](fetch func(page int) ([]T, error)) ([]T, error) { return nil, fmt.Errorf("listing exceeded %d pages", maxListPages) } +// filterPageSize is the page size filteredPage reads, Gitea's default maximum. +const filterPageSize = 50 + +// filteredPage applies a filter without a server equivalent: it reads pages of filterPageSize +// from fetch only until one item past the requested page is kept, and counts the matches when +// it reads them all. +func filteredPage[T any](limit, page int, fetch func(limit, page int) (Page[T], error), keep func(T) bool) (Page[T], error) { + if limit <= 0 || page <= 0 { + return Page[T]{}, errors.New("limit and page must be positive") + } + matching, wanted := []T{}, page*limit+1 + for serverPage := 1; ; serverPage++ { + if serverPage > maxListPages { + return Page[T]{}, fmt.Errorf("listing exceeded %d pages", maxListPages) + } + window, err := fetch(filterPageSize, serverPage) + if err != nil { + return Page[T]{}, err + } + for _, item := range window.Items { + if keep(item) { + matching = append(matching, item) + } + } + if len(matching) >= wanted { + items, err := pageOf(matching, limit, page) + return Page[T]{Items: items, More: true}, err + } + if len(window.Items) == 0 || (!window.More && len(window.Items) < filterPageSize) { + break + } + } + items, err := pageOf(matching, limit, page) + if err != nil { + return Page[T]{}, err + } + return Page[T]{Items: items, More: page*limit < len(matching), Total: len(matching), Counted: true}, nil +} + // pageHints reads whether the server advertises a next page and the collection size. func pageHints(headers http.Header) (next bool, total int, counted bool) { links, err := nextPageLinks(headers.Values("Link")) diff --git a/internal/services/issues.go b/internal/services/issues.go index 22b57f83..26569428 100644 --- a/internal/services/issues.go +++ b/internal/services/issues.go @@ -83,6 +83,9 @@ type ListOptions struct { Assignee, Labels, Milestones, Kind string Author, Mentions string Since, Before *time.Time + // NoAssignee keeps only issues without assignees. Gitea has no such filter, so ListPage + // applies it to every issue it reads. + NoAssignee bool } // Issues implements repository issue operations. @@ -121,14 +124,20 @@ func (s Issues) ListPage(ctx context.Context, opts ListOptions) (Page[Issue], er if opts.Before != nil { query.Before = *opts.Before } - issues, err := callPage(ctx, s.Client, opts.Limit, opts.Page, func(ctx context.Context, c *gitea.Client, list gitea.ListOptions) ([]*gitea.Issue, *gitea.Response, error) { - query.ListOptions = list - return c.Issues.ListRepoIssues(ctx, owner, repo, query) - }) - if err != nil { - return Page[Issue]{}, err + fetch := func(limit, page int) (Page[Issue], error) { + issues, err := callPage(ctx, s.Client, limit, page, func(ctx context.Context, c *gitea.Client, list gitea.ListOptions) ([]*gitea.Issue, *gitea.Response, error) { + query.ListOptions = list + return c.Issues.ListRepoIssues(ctx, owner, repo, query) + }) + if err != nil { + return Page[Issue]{}, err + } + return convertPage(issues, discussionFrom), nil } - return convertPage(issues, discussionFrom), nil + if opts.NoAssignee { + return filteredPage(opts.Limit, opts.Page, fetch, func(item Issue) bool { return len(item.Assignees) == 0 }) + } + return fetch(opts.Limit, opts.Page) } // filterValues splits a comma-separated filter, which the SDK joins again. diff --git a/internal/services/pulls.go b/internal/services/pulls.go index d8bb19e3..a999299a 100644 --- a/internal/services/pulls.go +++ b/internal/services/pulls.go @@ -143,15 +143,16 @@ type PullListOptions struct { Limit, Page int Milestone int64 Labels []int64 - // Draft, Head (BRANCH or OWNER:BRANCH), Assignee, Search (text in the title or body) and the - // merged state have no server filter; List applies them to every pull request it reads. - Draft bool + // Draft, Head (BRANCH or OWNER:BRANCH), Assignee, NoAssignee, Search (text in the title or + // body) and the merged state have no server filter; List applies them to every pull request + // it reads. + Draft, NoAssignee bool Head, Assignee, Search string } // filtered reports whether a filter needs every pull request read. func (o PullListOptions) filtered() bool { - return o.State == "merged" || o.Draft || o.Head != "" || o.Assignee != "" || o.Search != "" + return o.State == "merged" || o.Draft || o.NoAssignee || o.Head != "" || o.Assignee != "" || o.Search != "" } // matches applies the filters without a server equivalent to item. @@ -165,6 +166,7 @@ func (o PullListOptions) matches(item Pull) bool { return (o.State != "merged" || item.Merged) && (!o.Draft || item.Draft) && (branch == "" || (item.Head == branch && (owner == "" || strings.EqualFold(owner, headOwner)))) && (o.Assignee == "" || slices.ContainsFunc(item.Assignees, func(name string) bool { return strings.EqualFold(name, o.Assignee) })) && + (!o.NoAssignee || len(item.Assignees) == 0) && (search == "" || strings.Contains(strings.ToLower(item.Title), search) || strings.Contains(strings.ToLower(item.Body), search)) } @@ -182,41 +184,14 @@ func (s Pulls) ListPage(ctx context.Context, options PullListOptions) (Page[Pull if !options.filtered() { return s.listPage(ctx, options) } - if options.Limit <= 0 || options.Page <= 0 { - return Page[Pull]{}, errors.New("limit and page must be positive") - } query := options if query.State == "merged" { query.State = "closed" } - matching, wanted := []Pull{}, options.Page*options.Limit+1 - for page := 1; ; page++ { - if page > maxListPages { - return Page[Pull]{}, fmt.Errorf("listing exceeded %d pages", maxListPages) - } - query.Limit, query.Page = 50, page - window, err := s.listPage(ctx, query) - if err != nil { - return Page[Pull]{}, err - } - for _, item := range window.Items { - if options.matches(item) { - matching = append(matching, item) - } - } - if len(matching) >= wanted { - items, err := pageOf(matching, options.Limit, options.Page) - return Page[Pull]{Items: items, More: true}, err - } - if len(window.Items) == 0 || (!window.More && len(window.Items) < query.Limit) { - break - } - } - items, err := pageOf(matching, options.Limit, options.Page) - if err != nil { - return Page[Pull]{}, err - } - return Page[Pull]{Items: items, More: options.Page*options.Limit < len(matching), Total: len(matching), Counted: true}, nil + return filteredPage(options.Limit, options.Page, func(limit, page int) (Page[Pull], error) { + query.Limit, query.Page = limit, page + return s.listPage(ctx, query) + }, options.matches) } func (s Pulls) listPage(ctx context.Context, options PullListOptions) (Page[Pull], error) { diff --git a/internal/services/pulls_test.go b/internal/services/pulls_test.go index 22bec272..7edb7710 100644 --- a/internal/services/pulls_test.go +++ b/internal/services/pulls_test.go @@ -363,3 +363,20 @@ func TestPullClientFiltersCountWhenEveryPullWasRead(t *testing.T) { require.True(t, page.Counted) require.Equal(t, 2, page.Total) } + +func TestPullNoAssigneeKeepsUnassignedPulls(t *testing.T) { + client := newTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Empty(t, r.URL.Query().Get("assignee")) + if r.URL.Query().Get("page") != "1" { + _, _ = w.Write([]byte(`[]`)) + return + } + _, _ = w.Write([]byte(`[{"number":1,"assignees":[{"login":"bob"}]},{"number":2},{"number":3,"assignees":[]}]`)) + })) + page, err := (Pulls{Client: client, Repository: "alice/project"}).ListPage(context.Background(), PullListOptions{State: "open", NoAssignee: true, Limit: 30, Page: 1}) + require.NoError(t, err) + require.Len(t, page.Items, 2) + require.EqualValues(t, 2, page.Items[0].Number) + require.EqualValues(t, 3, page.Items[1].Number) + require.True(t, page.Counted) +}