From d9010f5cfac560133e8bed969dd34d9c0e35dff5 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. (cherry picked from commit d97cf0e85466759917182e0e958839964829c091) --- 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 e34f90c73b..bf9fc5f993 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1258,7 +1258,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()) @@ -3133,7 +3133,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 be6511afaa..5c56a6f5ad 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1508,8 +1508,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 b103d0a388..fbbcf7217e 100644 --- a/tests/integration/block_test.go +++ b/tests/integration/block_test.go @@ -166,6 +166,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}) @@ -180,7 +181,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)() @@ -192,19 +198,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)() @@ -215,11 +240,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) { @@ -235,11 +261,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) }) })