Skip to content

Commit

Permalink
approve/unapprove: Provide better error messages for repeat actions
Browse files Browse the repository at this point in the history
Users are presented with the following error message if an already
approved MR is approved again:

2021/05/07 10:34:35 mr_approve.go:55: POST https://gitlab.com/api/v4/projects/24140364/merge_requests/119/approve: 401 {message: 401 Unauthorized}

Similarily, when an unapproved MR is unapproved again:

2021/05/07 10:35:01 mr_unapprove.go:55: POST https://gitlab.com/api/v4/projects/24140364/merge_requests/119/unapprove: 40

These messages are confusing and should state that the MR was already
approved or unapproved.

Add better error messages for repeat approve and unapprove actions.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
  • Loading branch information
prarit committed May 10, 2021
1 parent 2df755d commit bef39e0
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 3 deletions.
2 changes: 1 addition & 1 deletion cmd/ci_run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ func Test_getCIRunOptions(t *testing.T) {
[]string{},
nil, // https://gitlab.com/zaquestion/test project ID
"",
"gitlab project not found, verify you have access to the requested resource",
"GitLab project not found, verify you have access to the requested resource",
},
}

Expand Down
9 changes: 8 additions & 1 deletion cmd/mr_approve.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package cmd
import (
"fmt"
"log"
"os"

"github.com/rsteube/carapace"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -52,7 +53,13 @@ var mrApproveCmd = &cobra.Command{

err = lab.MRApprove(p.ID, int(id))
if err != nil {
log.Fatal(err)
if err == lab.ErrStatusForbidden {
log.Fatal(err)
}
if err == lab.ErrActionRepeated {
fmt.Printf("Merge Request !%d already approved\n", id)
os.Exit(1)
}
}
fmt.Printf("Merge Request !%d approved\n", id)
},
Expand Down
9 changes: 8 additions & 1 deletion cmd/mr_unapprove.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package cmd
import (
"fmt"
"log"
"os"

"github.com/rsteube/carapace"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -52,7 +53,13 @@ var mrUnapproveCmd = &cobra.Command{

err = lab.MRUnapprove(p.ID, int(id))
if err != nil {
log.Fatal(err)
if err == lab.ErrStatusForbidden {
log.Fatal(err)
}
if err == lab.ErrActionRepeated {
fmt.Printf("Merge Request !%d already unapproved\n", id)
os.Exit(1)
}
}
fmt.Printf("Merge Request !%d unapproved\n", id)
},
Expand Down
11 changes: 11 additions & 0 deletions internal/gitlab/gitlab.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ import (
)

var (
// ErrActionRepeated is returned when a GitLab action is executed again. For example
// this can be returned when an MR is approved twice.
ErrActionRepeated = errors.New("GitLab action repeated")
// ErrGroupNotFound is returned when a GitLab group cannot be found.
ErrGroupNotFound = errors.New("GitLab group not found")
// ErrNotModified is returned when adding an already existing item to a Todo list
Expand Down Expand Up @@ -526,6 +529,10 @@ func MRApprove(pid interface{}, id int) error {
if resp != nil && resp.StatusCode == http.StatusForbidden {
return ErrStatusForbidden
}
if resp != nil && resp.StatusCode == http.StatusUnauthorized {
// returns 401 if the MR has already been approved
return ErrActionRepeated
}
if err != nil {
return err
}
Expand All @@ -538,6 +545,10 @@ func MRUnapprove(pid interface{}, id int) error {
if resp != nil && resp.StatusCode == http.StatusForbidden {
return ErrStatusForbidden
}
if resp != nil && resp.StatusCode == http.StatusNotFound {
// returns 404 if the MR has already been unapproved
return ErrActionRepeated
}
if err != nil {
return err
}
Expand Down

0 comments on commit bef39e0

Please sign in to comment.