fix(pr): review decision ignores comment reviews, paged review list, pending review comments hidden, team request names (follow gh)

Signed-off-by: bircni <bircni@icloud.com>
This commit is contained in:
bircni committed 2026-10-18 20:07:47 +02:00
1 parent aa181a2b5c
commit efd3383ff9
19 files changed
+258 -50

No files matched your search

+16
View File
@@ -56,6 +56,22 @@ func complete(t *testing.T, deps Dependencies, out *bytes.Buffer, args ...string
return lines[:len(lines)-1], cobra.ShellCompDirective(directive)
}
func TestPullCompletionStopsAfterTheLastPositionalArgument(t *testing.T) {
requests := 0
server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
requests++
_, _ = w.Write([]byte(`[{"id":9,"number":9,"title":"More"}]`))
}))
defer server.Close()
deps, out, _ := serverDeps(t, server.URL)
for _, args := range []string{"pr view 5 ", "pr review list 5 ", "pr review view 5 9 ", "pr review submit 5 9 ", "pr review reply 5 9 "} {
items, directive := complete(t, deps, out, append(strings.Fields(args), "-R", "owner/repo", "")...)
require.Empty(t, items, args)
require.Equal(t, cobra.ShellCompDirectiveNoFileComp, directive, args)
}
require.Zero(t, requests, "no argument is left to complete")
}
func TestEnumFlagsShareValidationAndCompletion(t *testing.T) {
deps, out, _ := testDeps(t)
var state string
+9
View File
@@ -312,6 +312,15 @@ func TestHelpConfirmAndArgumentsAnnotations(t *testing.T) {
require.Contains(t, out.String(), "\nEXAMPLES\n # Drop issue 7\n $ tea issue drop 7\n")
}
func TestHelpDefaultsToCurrentBranchOnlyWhereThePullRequestIsOptional(t *testing.T) {
deps, out, _ := testDeps(t)
for path, defaults := range map[string]bool{"pr review list": true, "pr view": true, "pr review view": false, "pr review dismiss": false, "pr checkout": false} {
out.Reset()
require.NoError(t, Execute(context.Background(), deps, append(strings.Fields(path), "--help")))
require.Equal(t, defaults, strings.Contains(out.String(), "Without an argument, the pull request of the current branch"), path)
}
}
func TestHelpTopicsAndUnknownTopics(t *testing.T) {
deps, out, _ := testDeps(t)
for args, want := range map[string]string{
+2 -2
View File
@@ -76,7 +76,7 @@ func (a *application) pullCheckoutCommand() *cobra.Command {
var branch string
var detached, update bool
cmd := &cobra.Command{Use: "checkout {<number> | <url> | <branch>}", Aliases: []string{"co"}, Short: "Check out a pull request", Long: `Check out the head of a pull request in a local branch that tracks the head branch, so
that git push updates it and pr commands without an argument select the pull request.`, Annotations: pullArgumentsAnnotation(nil), Args: argsExact(1), RunE: func(cmd *cobra.Command, args []string) error {
that git push updates it and pr commands without an argument select the pull request.`, Annotations: pullArgumentRequiredAnnotation(nil), Args: argsExact(1), RunE: func(cmd *cobra.Command, args []string) error {
if detached && (cmd.Flags().Changed("branch") || update) {
return clierr.Wrap(clierr.Usage, errors.New("--detach and --branch/--update are mutually exclusive"))
}
@@ -149,7 +149,7 @@ that git push updates it and pr commands without an argument select the pull req
cmd.Flags().BoolVar(&update, "update", false, "Fast-forward an existing branch to the PR head; reject divergence")
cmd.Flags().BoolVar(&detached, "detach", false, "Check out the PR commit without creating or changing a branch")
cmd.Flags().StringVarP(&branch, "branch", "b", "", "Local branch name (default: the head branch name when free, else OWNER/BRANCH or pr/NUMBER)")
cmd.ValidArgsFunction = a.completePulls("open")
cmd.ValidArgsFunction = firstArg(a.completePulls("open"))
cmd.Example = `$ tea pr checkout 13
$ tea pr co feature/login
$ tea pr checkout 13 --update`
+1 -1
View File
@@ -86,7 +86,7 @@ tea exits with 8 while checks are pending and with 1 when a check failed, like g
cmd.Flags().DurationVar(&interval, "interval", 5*time.Second, "Time between refreshes with --watch")
cmd.Flags().DurationVar(&timeout, "timeout", time.Hour, "Longest time to wait with --watch")
cmd.Flags().BoolVarP(&web, "web", "w", false, "Open the pull request in the browser")
cmd.ValidArgsFunction = a.completePulls("open")
cmd.ValidArgsFunction = firstArg(a.completePulls("open"))
cmd.Example = `# Show the checks of the current branch's pull request
$ tea pr checks
+1 -4
View File
@@ -238,10 +238,7 @@ func pullReviewComments(cmd *cobra.Command, t target, n int64, reviews []service
return nil, err
}
for _, comment := range inline {
location := comment.Path
if comment.Line > 0 {
location = fmt.Sprintf("%s:%d", comment.Path, comment.Line)
}
location := comment.Location()
result = append(result, discussionComment{
author: comment.Author, createdAt: comment.CreatedAt, body: comment.Body, id: comment.ID,
heading: "commented on " + location, meta: [2]string{"path", location},
+1 -1
View File
@@ -120,7 +120,7 @@ func (a *application) pullCleanCommand() *cobra.Command {
return a.jsonResult(cmd, result)
}}
cmd.Flags().BoolVar(&ignoreSHA, "ignore-sha", false, "Find the local branch by name instead of commit hash")
cmd.ValidArgsFunction = a.completePulls("closed")
cmd.ValidArgsFunction = firstArg(a.completePulls("closed"))
cmd.Example = `# After #13 was merged, delete its branch locally and on Gitea
$ tea pr clean 13`
return cmd
+2 -2
View File
@@ -149,7 +149,7 @@ pull request, or mark it as a draft or ready for review.`, Annotations: pullArgu
}
_ = cmd.RegisterFlagCompletionFunc("base", a.completeBranches())
_ = cmd.RegisterFlagCompletionFunc("milestone", a.completeMilestones())
cmd.ValidArgsFunction = a.completePulls("open")
cmd.ValidArgsFunction = firstArg(a.completePulls("open"))
cmd.Example = `$ tea pr edit 13 --title "Add login support" --add-reviewer carol
$ tea pr edit --base develop
$ tea pr edit 13 --editor`
@@ -206,7 +206,7 @@ removes or adds that marker.`, Annotations: pullArgumentsAnnotation(nil), Args:
return a.jsonResult(cmd, item)
}}
cmd.Flags().BoolVar(&undo, "undo", false, "Turn the pull request back into a draft")
cmd.ValidArgsFunction = a.completePulls("open")
cmd.ValidArgsFunction = firstArg(a.completePulls("open"))
cmd.Example = `$ tea pr ready
$ tea pr ready 15
$ tea pr ready 15 --undo`
+3 -3
View File
@@ -111,7 +111,7 @@ func (a *application) pullStateCommand(closing bool) *cobra.Command {
return a.jsonResult(cmd, item)
}}
cmd.Flags().StringVarP(&comment, "comment", "c", "", "Leave a comment before changing the state")
cmd.ValidArgsFunction = a.completePulls(map[bool]string{true: "open", false: "closed"}[closing])
cmd.ValidArgsFunction = firstArg(a.completePulls(map[bool]string{true: "open", false: "closed"}[closing]))
if closing {
cmd.Flags().BoolVarP(&deleteBranch, "delete-branch", "d", false, "Delete the head branch on Gitea and, when it is checked out, locally")
cmd.Example = `$ tea pr close 13
@@ -219,7 +219,7 @@ tea pr diff 13 | git apply works.`, Annotations: pullArgumentsAnnotation(textOut
cmd.Flags().BoolVar(&nameOnly, "name-only", false, "List only the names of the changed files")
enumFlag(cmd, &color, "color", "", "auto", "Color the diff", "auto", "always", "never")
cmd.Flags().BoolVarP(&web, "web", "w", false, "Open the changed files in the browser")
cmd.ValidArgsFunction = a.completePulls("all")
cmd.ValidArgsFunction = firstArg(a.completePulls("all"))
cmd.Example = `$ tea pr diff
$ tea pr diff 13 --name-only
$ tea pr diff 13 | git apply`
@@ -384,7 +384,7 @@ the base branch.`, Annotations: pullArgumentsAnnotation(map[string]string{"tea.c
}
return c.Root().FlagErrorFunc()(c, err)
})
cmd.ValidArgsFunction = a.completePulls("open")
cmd.ValidArgsFunction = firstArg(a.completePulls("open"))
cmd.Example = `# Merge the pull request of the current branch with the repository's default style
$ tea pr merge
+32 -21
View File
@@ -19,18 +19,30 @@ import (
"github.com/spf13/cobra"
)
const pullArguments = `A pull request can be supplied as argument in any of the following formats:
const pullArgumentFormats = `A pull request can be supplied as argument in any of the following formats:
- by number, such as "13" or "#13"
- by URL, such as "https://gitea.com/OWNER/REPO/pulls/13"
- by the name of its head branch, such as "feature/login" or "bob:feature/login"
- by the name of its head branch, such as "feature/login" or "bob:feature/login"`
const pullArguments = pullArgumentFormats + `
Without an argument, the pull request of the current branch is selected.`
// pullArgumentsAnnotation documents a pull request argument that may be left out.
func pullArgumentsAnnotation(annotations map[string]string) map[string]string {
return argumentsAnnotation(annotations, pullArguments)
}
// pullArgumentRequiredAnnotation documents a pull request argument that has no default.
func pullArgumentRequiredAnnotation(annotations map[string]string) map[string]string {
return argumentsAnnotation(annotations, pullArgumentFormats)
}
func argumentsAnnotation(annotations map[string]string, text string) map[string]string {
if annotations == nil {
annotations = map[string]string{}
}
annotations["tea.arguments"] = pullArguments
annotations["tea.arguments"] = text
return annotations
}
@@ -242,7 +254,7 @@ func (a *application) reviewCreateCommand(use, short string) *cobra.Command {
cmd.Flags().BoolVarP(&useEditor, "editor", "e", false, "Write the body in your editor")
cmd.Flags().StringVar(&commentsFile, "comments-file", "", "Read inline comments from a JSON `file`: an array of path, body, and new_position or old_position")
cmd.Flags().StringVar(&commit, "commit", "", "Commit `sha` to review (default: the head of the pull request)")
cmd.ValidArgsFunction = a.completePulls("open")
cmd.ValidArgsFunction = firstArg(a.completePulls("open"))
return cmd
}
@@ -442,7 +454,7 @@ func reviewPossessive(item services.Review) string {
case "REQUEST_CHANGES":
noun = "change request"
}
return item.Author + "'s " + noun
return item.Reviewer() + "'s " + noun
}
// pullAndID resolves the pull request and the ID that review subcommands take as their two arguments.
@@ -462,17 +474,17 @@ func (a *application) pullReviewListCommand() *cobra.Command {
if err != nil {
return err
}
items, err := (services.Reviews{Client: t.client, Repository: t.Repo}).List(cmd.Context(), n, limit, page)
window, err := (services.Reviews{Client: t.client, Repository: t.Repo}).ListPage(cmd.Context(), n, limit, page)
if err != nil {
return err
}
ref := fmt.Sprintf("%s#%d", t.Repo, n)
return writeTable(a, cmd, items, listSpec[services.Review]{
return writeTable(a, cmd, window.Items, listSpec[services.Review]{
Columns: []terminal.Column{{Header: "ID"}, {Header: "Reviewer"}, {Header: "State"}, {Header: "Submitted"}, {Header: "Comments"}, {Header: "Body", Flex: true}},
Row: func(item services.Review) []terminal.Cell {
return []terminal.Cell{
{Text: strconv.FormatInt(item.ID, 10), Link: item.URL},
{Text: item.Author},
{Text: item.Reviewer()},
reviewStateCell(item),
{Time: terminal.ParseTime(item.SubmittedAt)},
{Text: strconv.Itoa(item.CommentsCount)},
@@ -480,17 +492,17 @@ func (a *application) pullReviewListCommand() *cobra.Command {
}
},
Empty: "no reviews on " + ref, Context: "reviews on " + ref,
}, listPage{Number: page, More: len(items) == limit})
}, servicePage(window, page))
}}
pageFlags(cmd, &limit, &page)
cmd.Example = `$ tea pr review list 13
$ tea pr review list --json id,author,state`
cmd.ValidArgsFunction = a.completePulls("all")
cmd.ValidArgsFunction = firstArg(a.completePulls("all"))
return cmd
}
func (a *application) pullReviewViewCommand() *cobra.Command {
cmd := &cobra.Command{Use: "view <number> <review-id>", Short: "View a review", Annotations: pullArgumentsAnnotation(nil), Args: argsExact(2), RunE: func(cmd *cobra.Command, args []string) error {
cmd := &cobra.Command{Use: "view <number> <review-id>", Short: "View a review", Annotations: pullArgumentRequiredAnnotation(nil), Args: argsExact(2), RunE: func(cmd *cobra.Command, args []string) error {
t, n, id, err := a.pullAndID(cmd, args)
if err != nil {
return err
@@ -501,14 +513,14 @@ func (a *application) pullReviewViewCommand() *cobra.Command {
}
now := a.deps.Now()
return writeView(a, cmd, item, func(item services.Review) terminal.View {
meta := item.Author
meta := item.Reviewer()
if submitted := terminal.ParseTime(item.SubmittedAt); !submitted.IsZero() {
meta += " • " + terminal.RelativeTime(now, submitted)
}
return terminal.View{
Title: fmt.Sprintf("Review on %s#%d", t.Repo, n), ID: strconv.FormatInt(item.ID, 10), Badges: []terminal.Cell{reviewStateCell(item)}, Meta: meta,
Fields: []terminal.Field{
{Label: "Reviewer", Value: item.Author, PipedOnly: true},
{Label: "Reviewer", Value: item.Reviewer(), PipedOnly: true},
{Label: "State", Value: item.State, PipedOnly: true},
{Label: "Inline comments", Value: strconv.Itoa(item.CommentsCount)},
{Label: "Commit", Value: item.Commit},
@@ -541,11 +553,7 @@ func (a *application) pullReviewCommentsCommand() *cobra.Command {
return writeTable(a, cmd, items, listSpec[services.ReviewComment]{
Columns: []terminal.Column{{Header: "ID"}, {Header: "File"}, {Header: "Author"}, {Header: "Resolved"}, {Header: "Body", Flex: true}},
Row: func(item services.ReviewComment) []terminal.Cell {
location := item.Path
if item.Line > 0 {
location = fmt.Sprintf("%s:%d", item.Path, item.Line)
}
return []terminal.Cell{{Text: strconv.FormatInt(item.ID, 10), Link: item.URL}, {Text: location}, {Text: item.Author}, {Text: item.Resolver}, {Text: item.Body}}
return []terminal.Cell{{Text: strconv.FormatInt(item.ID, 10), Link: item.URL}, {Text: item.Location()}, {Text: item.Author}, {Text: item.Resolver}, {Text: item.Body}}
},
Empty: "no inline review comments on " + ref, Context: "inline review comments on " + ref,
}, listPage{Number: page, More: len(items) == limit})
@@ -554,7 +562,7 @@ func (a *application) pullReviewCommentsCommand() *cobra.Command {
cmd.Flags().Int64Var(&reviewID, "review", 0, "Limit comments to one review `ID`")
cmd.Example = `$ tea pr review comments 13
$ tea pr review comments 13 --review 6`
cmd.ValidArgsFunction = a.completePulls("all")
cmd.ValidArgsFunction = firstArg(a.completePulls("all"))
return cmd
}
@@ -585,7 +593,7 @@ func (a *application) pullReviewResolveCommand(resolve bool) *cobra.Command {
func (a *application) pullReviewReplyCommand() *cobra.Command {
var body, file string
var useEditor bool
cmd := &cobra.Command{Use: "reply <number> <comment-id>", Short: "Reply to an inline review comment", Annotations: pullArgumentsAnnotation(nil), Args: argsExact(2), RunE: func(cmd *cobra.Command, args []string) (runErr error) {
cmd := &cobra.Command{Use: "reply <number> <comment-id>", Short: "Reply to an inline review comment", Annotations: pullArgumentRequiredAnnotation(nil), Args: argsExact(2), RunE: func(cmd *cobra.Command, args []string) (runErr error) {
var draft terminal.Draft
defer finishDraft(&draft, &runErr)
if err := validateBodySources(cmd); err != nil {
@@ -639,6 +647,9 @@ func (a *application) completeReviewArgs(inline bool) cobra.CompletionFunc {
if len(args) == 0 {
return pulls(cmd, args, toComplete)
}
if len(args) > 1 {
return nil, cobra.ShellCompDirectiveNoFileComp
}
n, err := number(args[0])
if err != nil {
return nil, cobra.ShellCompDirectiveNoFileComp
@@ -653,7 +664,7 @@ func (a *application) completeReviewArgs(inline bool) cobra.CompletionFunc {
}
items, err := service.List(ctx, n, completionLimit, 1)
return describe(items, func(item services.Review) (string, string) {
return strconv.FormatInt(item.ID, 10), reviewStateCell(item).Text + " by " + item.Author
return strconv.FormatInt(item.ID, 10), reviewStateCell(item).Text + " by " + item.Reviewer()
}), err
})(cmd, args, toComplete)
}
+11 -3
View File
@@ -123,7 +123,7 @@ func (a *application) pendingReview(cmd *cobra.Command, service services.Reviews
}
func (a *application) pullReviewDeleteCommand() *cobra.Command {
cmd := &cobra.Command{Use: "delete <number> <review-id>", Short: "Delete a review", Annotations: pullArgumentsAnnotation(textOutput("review deletion does not return a resource")), Args: argsExact(2), RunE: func(cmd *cobra.Command, args []string) error {
cmd := &cobra.Command{Use: "delete <number> <review-id>", Short: "Delete a review", Annotations: pullArgumentRequiredAnnotation(textOutput("review deletion does not return a resource")), Args: argsExact(2), RunE: func(cmd *cobra.Command, args []string) error {
t, n, id, err := a.pullAndID(cmd, args)
if err != nil {
return err
@@ -133,7 +133,7 @@ func (a *application) pullReviewDeleteCommand() *cobra.Command {
if err != nil {
return err
}
subject := fmt.Sprintf("review %d by %s on %s#%d", id, item.Author, t.Repo, n)
subject := fmt.Sprintf("review %d by %s on %s#%d", id, item.Reviewer(), t.Repo, n)
return a.deleteConfirmed(cmd, subject, func() error { return service.Delete(cmd.Context(), n, id) })
}}
cmd.Annotations["tea.confirm"] = "true"
@@ -149,7 +149,7 @@ func (a *application) pullReviewDismissCommand(dismiss bool) *cobra.Command {
if dismiss {
use, short = "dismiss <number> <review-id>", "Dismiss an approval or change request"
}
cmd := &cobra.Command{Use: use, Short: short, Annotations: pullArgumentsAnnotation(nil), Args: argsExact(2), RunE: func(cmd *cobra.Command, args []string) (runErr error) {
cmd := &cobra.Command{Use: use, Short: short, Annotations: pullArgumentRequiredAnnotation(nil), Args: argsExact(2), RunE: func(cmd *cobra.Command, args []string) (runErr error) {
var draft terminal.Draft
defer finishDraft(&draft, &runErr)
var err error
@@ -183,6 +183,14 @@ func (a *application) pullReviewDismissCommand(dismiss bool) *cobra.Command {
return err
}
subject := fmt.Sprintf("%s on %s#%d", reviewPossessive(current), t.Repo, n)
switch {
case dismiss && current.State != "APPROVED" && current.State != "REQUEST_CHANGES":
// Gitea refuses other review types with a misleading "permission denied".
return clierr.Wrap(clierr.Usage, fmt.Errorf("only approvals and change requests can be dismissed; %s is not one", subject))
case !dismiss && !current.Dismissed:
a.warn(cmd, "%s is not dismissed", subject)
return a.jsonResult(cmd, current)
}
if dismiss {
if err := a.confirmAction(cmd, terminal.Confirmation{Prompt: "Dismiss " + subject + "?"}); err != nil {
return err
+1 -1
View File
@@ -47,7 +47,7 @@ func (a *application) pullReviewRequestCommand(add bool) *cobra.Command {
cmd.Flags().StringSliceVarP(&input.Users, "reviewer", "r", nil, "Reviewer usernames or email addresses; @me for yourself")
cmd.Flags().StringSliceVar(&input.Teams, "team", nil, "Teams of the repository's organization")
_ = cmd.RegisterFlagCompletionFunc("reviewer", a.completeUsers())
cmd.ValidArgsFunction = a.completePulls("open")
cmd.ValidArgsFunction = firstArg(a.completePulls("open"))
if add {
cmd.Example = `$ tea pr review request 13 --reviewer carol,dave
$ tea pr review request --team developers`
+18
View File
@@ -270,6 +270,24 @@ func TestReviewCommentsFileValidatedBeforeRequests(t *testing.T) {
}
}
func TestReviewDismissChecksTheReviewStateFirst(t *testing.T) {
mutations := 0
server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodGet {
mutations++
}
_, _ = w.Write([]byte(`{"id":2,"state":"COMMENT","user":{"login":"bob"}}`))
}))
defer server.Close()
deps, _, diagnostics := serverDeps(t, server.URL)
err := Execute(context.Background(), deps, []string{"pr", "review", "dismiss", "1", "2", "-R", "owner/repo", "--body", "Reason", "--yes"})
require.Equal(t, clierr.Usage, clierr.ExitCode(err))
require.ErrorContains(t, err, "only approvals and change requests can be dismissed")
require.NoError(t, Execute(context.Background(), deps, []string{"pr", "review", "undismiss", "1", "2", "-R", "owner/repo"}))
require.Contains(t, diagnostics.String(), "is not dismissed")
require.Zero(t, mutations)
}
func TestReviewDeleteAndDismissRequireYesWithoutInput(t *testing.T) {
mutations := 0
server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+2 -2
View File
@@ -166,7 +166,7 @@ func (a *application) pullViewCommand() *cobra.Command {
if details.reviews, err = (services.Reviews{Client: t.client, Repository: t.Repo}).All(cmd.Context(), n); err != nil {
return err
}
item.ReviewDecision = services.ReviewDecision(services.LatestReviews(details.reviews), len(item.Reviewers)+len(item.ReviewerTeams) > 0)
item.ReviewDecision = services.ReviewDecision(details.reviews, len(item.Reviewers)+len(item.ReviewerTeams) > 0)
if a.options.json != "" {
return a.writeJSON(cmd, item)
}
@@ -179,7 +179,7 @@ func (a *application) pullViewCommand() *cobra.Command {
}}
cmd.Flags().BoolVarP(&web, "web", "w", false, "Open the pull request in the browser")
cmd.Flags().BoolVarP(&comments, "comments", "c", false, "Show comments and reviews")
cmd.ValidArgsFunction = a.completePulls("all")
cmd.ValidArgsFunction = firstArg(a.completePulls("all"))
cmd.Example = `# View the pull request of the current branch
$ tea pr view
+1 -1
View File
@@ -106,7 +106,7 @@ func (a *application) currentPullStatus(cmd *cobra.Command) (*services.Pull, pul
return nil, details, branch
}
if details.reviews, err = (services.Reviews{Client: t.client, Repository: t.Repo}).All(cmd.Context(), n); err == nil {
pull.ReviewDecision = services.ReviewDecision(services.LatestReviews(details.reviews), len(pull.Reviewers)+len(pull.ReviewerTeams) > 0)
pull.ReviewDecision = services.ReviewDecision(details.reviews, len(pull.Reviewers)+len(pull.ReviewerTeams) > 0)
}
if pull.HeadSHA != "" {
details.checks, _ = service.CommitChecks(cmd.Context(), pull.HeadSHA)