From d97cf0e85466759917182e0e958839964829c091 Mon Sep 17 00:00:00 2001 From: Gusted Date: Fri, 9 Aug 2024 20:33:23 +0200 Subject: [PATCH] [BUG] Return blocking errors as JSON errors - These endspoints are since b71cb7acdc8840c9fc16b496c90a048051d15823 JSON-based and should therefore return JSON errors. - Integration tests adjusted. --- routers/web/repo/issue.go | 4 +-- routers/web/repo/pull.go | 3 +- tests/integration/block_test.go | 59 ++++++++++++++++++++++++--------- 3 files changed, 46 insertions(+), 20 deletions(-) diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index afac2c5266..c21c7d52c6 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1263,7 +1263,7 @@ func NewIssuePost(ctx *context.Context) { if err := issue_service.NewIssue(ctx, repo, issue, labelIDs, attachments, assigneeIDs); err != nil { if errors.Is(err, user_model.ErrBlockedByUser) { - ctx.RenderWithErr(ctx.Tr("repo.issues.blocked_by_user"), tplIssueNew, form) + ctx.JSONError(ctx.Tr("repo.issues.blocked_by_user")) return } else if repo_model.IsErrUserDoesNotHaveAccessToRepo(err) { ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err.Error()) @@ -3197,7 +3197,7 @@ func NewComment(ctx *context.Context) { comment, err := issue_service.CreateIssueComment(ctx, ctx.Doer, ctx.Repo.Repository, issue, form.Content, attachments) if err != nil { if errors.Is(err, user_model.ErrBlockedByUser) { - ctx.Flash.Error(ctx.Tr("repo.issues.comment.blocked_by_user")) + ctx.JSONError(ctx.Tr("repo.issues.comment.blocked_by_user")) } else { ctx.ServerError("CreateIssueComment", err) } diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index a9213790cb..5ab139776c 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1525,8 +1525,7 @@ func CompareAndPullRequestPost(ctx *context.Context) { if err := pull_service.NewPullRequest(ctx, repo, pullIssue, labelIDs, attachments, pullRequest, assigneeIDs); err != nil { if errors.Is(err, user_model.ErrBlockedByUser) { - ctx.Flash.Error(ctx.Tr("repo.pulls.blocked_by_user")) - ctx.Redirect(ctx.Link) + ctx.JSONError(ctx.Tr("repo.pulls.blocked_by_user")) return } else if repo_model.IsErrUserDoesNotHaveAccessToRepo(err) { ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err.Error()) diff --git a/tests/integration/block_test.go b/tests/integration/block_test.go index 7105472a09..b17a445bf8 100644 --- a/tests/integration/block_test.go +++ b/tests/integration/block_test.go @@ -159,6 +159,7 @@ func TestBlockActions(t *testing.T) { doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) blockedUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) blockedUser2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 10}) + repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1, OwnerID: doer.ID}) repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2, OwnerID: doer.ID}) repo7 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 7, OwnerID: blockedUser2.ID}) issue4 := unittest.AssertExistsAndLoadBean(t, &issue_model.Issue{ID: 4, RepoID: repo2.ID}) @@ -173,7 +174,12 @@ func TestBlockActions(t *testing.T) { BlockUser(t, doer, blockedUser) BlockUser(t, doer, blockedUser2) - // Ensures that issue creation on doer's ownen repositories are blocked. + type errorJSON struct { + Error string `json:"errorMessage"` + } + locale := translation.NewLocale("en-US") + + // Ensures that issue creation on doer's owned repositories are blocked. t.Run("Issue creation", func(t *testing.T) { defer tests.PrintCurrentTest(t)() @@ -185,19 +191,38 @@ func TestBlockActions(t *testing.T) { "title": "Title", "content": "Hello!", }) - resp := session.MakeRequest(t, req, http.StatusOK) + resp := session.MakeRequest(t, req, http.StatusBadRequest) - htmlDoc := NewHTMLParser(t, resp.Body) - assert.Contains(t, - htmlDoc.doc.Find(".ui.negative.message").Text(), - translation.NewLocale("en-US").Tr("repo.issues.blocked_by_user"), - ) + var errorResp errorJSON + DecodeJSON(t, resp, &errorResp) + + assert.EqualValues(t, locale.Tr("repo.issues.blocked_by_user"), errorResp.Error) + }) + + // Ensures that pull creation on doer's owned repositories are blocked. + t.Run("Pull creation", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + session := loginUser(t, blockedUser.Name) + link := fmt.Sprintf("%s/compare/v1.1...master", repo1.FullName()) + + req := NewRequestWithValues(t, "POST", link, map[string]string{ + "_csrf": GetCSRF(t, session, link), + "title": "Title", + "content": "Hello!", + }) + resp := session.MakeRequest(t, req, http.StatusBadRequest) + + var errorResp errorJSON + DecodeJSON(t, resp, &errorResp) + + assert.EqualValues(t, locale.Tr("repo.pulls.blocked_by_user"), errorResp.Error) }) // Ensures that comment creation on doer's owned repositories and doer's // posted issues are blocked. t.Run("Comment creation", func(t *testing.T) { - expectedFlash := "error%3DYou%2Bcannot%2Bcreate%2Ba%2Bcomment%2Bon%2Bthis%2Bissue%2Bbecause%2Byou%2Bare%2Bblocked%2Bby%2Bthe%2Brepository%2Bowner%2Bor%2Bthe%2Bposter%2Bof%2Bthe%2Bissue." + expectedMessage := locale.Tr("repo.issues.comment.blocked_by_user") t.Run("Blocked by repository owner", func(t *testing.T) { defer tests.PrintCurrentTest(t)() @@ -208,11 +233,12 @@ func TestBlockActions(t *testing.T) { "_csrf": GetCSRF(t, session, issue10URL), "content": "Not a kind comment", }) - session.MakeRequest(t, req, http.StatusOK) + resp := session.MakeRequest(t, req, http.StatusBadRequest) - flashCookie := session.GetCookie(forgejo_context.CookieNameFlash) - assert.NotNil(t, flashCookie) - assert.EqualValues(t, expectedFlash, flashCookie.Value) + var errorResp errorJSON + DecodeJSON(t, resp, &errorResp) + + assert.EqualValues(t, expectedMessage, errorResp.Error) }) t.Run("Blocked by issue poster", func(t *testing.T) { @@ -228,11 +254,12 @@ func TestBlockActions(t *testing.T) { "_csrf": GetCSRF(t, session, issueURL), "content": "Not a kind comment", }) - session.MakeRequest(t, req, http.StatusOK) + resp := session.MakeRequest(t, req, http.StatusBadRequest) - flashCookie := session.GetCookie(forgejo_context.CookieNameFlash) - assert.NotNil(t, flashCookie) - assert.EqualValues(t, expectedFlash, flashCookie.Value) + var errorResp errorJSON + DecodeJSON(t, resp, &errorResp) + + assert.EqualValues(t, expectedMessage, errorResp.Error) }) })