diff --git a/internal/app/errors.go b/internal/app/errors.go index 698872f6..197b1083 100644 --- a/internal/app/errors.go +++ b/internal/app/errors.go @@ -186,6 +186,10 @@ func conflictHint(ctx context.Context, apiErr *transport.APIError, conflict *ser if conflict.Key != "" { return "enable it with: tea workflow enable " + conflict.Key } + case services.ReasonNotDone: + if conflict.Kind == "run" && conflict.Key != "" { + return "cancel it first: tea run cancel " + conflict.Key + } case services.ReasonOwnsRepos: return "transfer or delete them first" } diff --git a/internal/app/errors_test.go b/internal/app/errors_test.go index aba5670c..0f0728f4 100644 --- a/internal/app/errors_test.go +++ b/internal/app/errors_test.go @@ -13,6 +13,7 @@ import ( "time" "gitea.dev/tea/internal/clierr" + "gitea.dev/tea/internal/transport" "github.com/stretchr/testify/require" ) @@ -76,6 +77,28 @@ func TestExplainNamesResourcesAndTellsWhyRepositoriesFail(t *testing.T) { } } +func TestExplainAddsTheNextStepToRefusals(t *testing.T) { + for _, tc := range []struct { + method, endpoint, message string + status int + want string + }{ + {"PATCH", "/api/v1/repos/alice/x/pulls/3", "cannot change state of this pull request, it was already merged", 412, "pull request #3 is already merged; its state can no longer change"}, + {"POST", "/api/v1/repos/alice/x/pulls/3/merge", "The PR is already merged", 405, "pull request #3 is already merged"}, + {"POST", "/api/v1/repos/alice/x/pulls/5/merge", "Work in progress PRs cannot be merged", 405, "pull request #5 is a draft; mark it ready with: tea pr edit 5 --ready"}, + {"POST", "/api/v1/repos/alice/x/pulls/1/reviews", "approve your own pull is not allowed", 422, "you can't approve or request changes on your own pull request; leave a comment instead"}, + {"POST", "/api/v1/repos/alice/x/pulls", "Invalid PullRequest: There are no changes between the head and the base", 422, "no commits between the base and head branches; check out your feature branch or pass --head"}, + {"POST", "/api/v1/repos/alice/x/actions/workflows/nightly.yaml/dispatches", "workflow is disabled", 403, "workflow nightly.yaml is disabled; enable it with: tea workflow enable nightly.yaml"}, + {"DELETE", "/api/v1/repos/alice/x/actions/runs/37", "this workflow run is not done", 400, "run 37 has not finished; cancel it first: tea run cancel 37"}, + {"POST", "/api/v1/repos/alice/x/wiki/new", "wiki page already exists [title: Home.md]", 400, `wiki page "Home" already exists in alice/x; edit it with: tea wiki edit "Home"`}, + {"DELETE", "/api/v1/admin/users/uxadm1", "user still has ownership of repositories [uid: 7]", 422, `user "uxadm1" still owns repositories; transfer or delete them first`}, + {"POST", "/api/v1/user/keys", "Key content has been used as non-deploy key", 422, "this SSH key is already registered"}, + } { + apiErr := &transport.APIError{Status: tc.status, Method: tc.method, Endpoint: tc.endpoint, Message: tc.message, Instance: "http://localhost:3001"} + require.EqualError(t, Explain(context.Background(), Dependencies{}, apiErr), tc.want, tc.endpoint) + } +} + func TestExplainWordsCredentialAndConnectionFailures(t *testing.T) { rejecting := fakeGitea(t, map[string][2]string{"GET /api/v1/user": {"401", `{"message":"token is invalid"}`}}) deps, _, _ := serverDeps(t, rejecting.URL) diff --git a/internal/services/errors.go b/internal/services/errors.go index b5623961..73047ac6 100644 --- a/internal/services/errors.go +++ b/internal/services/errors.go @@ -58,6 +58,7 @@ const ( ReasonNoChanges = "no changes" // the head branch has no commits the base lacks ReasonDisabled = "disabled" // the workflow is disabled ReasonCompleted = "completed" // the run already finished + ReasonNotDone = "not done" // the run has not finished yet ReasonKeyInUse = "key in use" // the SSH key is registered already ReasonOwnsRepos = "owns repositories" // the user still owns repositories ) @@ -98,6 +99,8 @@ func (e *ConflictError) Error() string { text = cmp.Or(subject, "the workflow") + " is disabled" case ReasonCompleted: text = cmp.Or(subject, "the run") + " is already completed" + case ReasonNotDone: + text = cmp.Or(subject, "the run") + " has not finished" case ReasonKeyInUse: text = "this SSH key is already registered" case ReasonOwnsRepos: @@ -145,6 +148,7 @@ var refusals = []struct { {ReasonNoChanges, nil, []string{"no changes between the head and the base"}}, {ReasonDisabled, nil, []string{"workflow is disabled"}}, {ReasonCompleted, nil, []string{"already completed"}}, + {ReasonNotDone, []int{http.StatusBadRequest}, []string{"not done"}}, {ReasonKeyInUse, nil, []string{"key content has been used"}}, {ReasonOwnsRepos, nil, []string{"ownership of repositories"}}, {ReasonExists, []int{http.StatusBadRequest, http.StatusConflict, http.StatusUnprocessableEntity}, []string{"already exist", "release is has no tag"}}, diff --git a/internal/services/errors_test.go b/internal/services/errors_test.go index 98aa49ac..b63c59e0 100644 --- a/internal/services/errors_test.go +++ b/internal/services/errors_test.go @@ -52,6 +52,7 @@ func TestClassifyRequestNamesTheResource(t *testing.T) { {"repository exists", "POST", "/api/v1/user/repos", "The repository with the same name already exists.", 409, "repository already exists"}, {"org repository exists", "POST", "/api/v1/orgs/acme/repos", "The repository with the same name already exists.", 409, "repository already exists"}, {"run completed", "POST", "/api/v1/repos/alice/x/actions/runs/35/cancel", "run is already completed", 409, "run 35 is already completed"}, + {"run not done", "DELETE", "/api/v1/repos/alice/x/actions/runs/37", "this workflow run is not done", 400, "run 37 has not finished"}, {"workflow disabled", "POST", "/api/v1/repos/alice/x/actions/workflows/nightly.yaml/dispatches", "workflow is disabled", 403, "workflow nightly.yaml is disabled"}, {"key in use", "POST", "/api/v1/user/keys", "Key content has been used as non-deploy key", 422, "this SSH key is already registered"}, {"unknown assignee", "POST", "/api/v1/repos/alice/x/issues", "Assignee does not exist: [name: user does not exist [uid: 0, name: nobody123]]", 422, `user "nobody123" not found`},