Merge pull request '[v8.0/forgejo] [BUG] Return blocking errors as JSON errors' (#4918) from bp-v8.0/forgejo-d97cf0e into v8.0/forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/4918
Reviewed-by: Gusted <gusted@noreply.codeberg.org>

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)
 		}

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.JSONError(ctx.Tr("repo.pulls.blocked_by_user"))
-			ctx.Redirect(ctx.Link)
 			return
 		} else if repo_model.IsErrUserDoesNotHaveAccessToRepo(err) {
 			ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err.Error())

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)
+		var errorResp errorJSON
-		assert.Contains(t,
+		DecodeJSON(t, resp, &errorResp)
-			htmlDoc.doc.Find(".ui.negative.message").Text(),
+
-			translation.NewLocale("en-US").Tr("repo.issues.blocked_by_user"),
+		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)
+			var errorResp errorJSON
-			assert.NotNil(t, flashCookie)
+			DecodeJSON(t, resp, &errorResp)
-			assert.EqualValues(t, expectedFlash, flashCookie.Value)
+
+			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)
+			var errorResp errorJSON
-			assert.NotNil(t, flashCookie)
+			DecodeJSON(t, resp, &errorResp)
-			assert.EqualValues(t, expectedFlash, flashCookie.Value)
+
+			assert.EqualValues(t, expectedMessage, errorResp.Error)
 		})
 	})