[BUG] Return blocking errors as JSON errors

- These endspoints are since b71cb7acdc
JSON-based and should therefore return JSON errors.
- Integration tests adjusted.

(cherry picked from commit d97cf0e854)
This commit is contained in:
Gusted 2024-08-09 20:33:23 +02:00 committed by forgejo-backport-action
parent 4d0be867a2
commit d9010f5cfa
3 changed files with 46 additions and 20 deletions

View file

@ -1258,7 +1258,7 @@ func NewIssuePost(ctx *context.Context) {
if err := issue_service.NewIssue(ctx, repo, issue, labelIDs, attachments, assigneeIDs); err != nil { if err := issue_service.NewIssue(ctx, repo, issue, labelIDs, attachments, assigneeIDs); err != nil {
if errors.Is(err, user_model.ErrBlockedByUser) { 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 return
} else if repo_model.IsErrUserDoesNotHaveAccessToRepo(err) { } else if repo_model.IsErrUserDoesNotHaveAccessToRepo(err) {
ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err.Error()) 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) comment, err := issue_service.CreateIssueComment(ctx, ctx.Doer, ctx.Repo.Repository, issue, form.Content, attachments)
if err != nil { if err != nil {
if errors.Is(err, user_model.ErrBlockedByUser) { 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 { } else {
ctx.ServerError("CreateIssueComment", err) ctx.ServerError("CreateIssueComment", err)
} }

View file

@ -1508,8 +1508,7 @@ func CompareAndPullRequestPost(ctx *context.Context) {
if err := pull_service.NewPullRequest(ctx, repo, pullIssue, labelIDs, attachments, pullRequest, assigneeIDs); err != nil { if err := pull_service.NewPullRequest(ctx, repo, pullIssue, labelIDs, attachments, pullRequest, assigneeIDs); err != nil {
if errors.Is(err, user_model.ErrBlockedByUser) { 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 return
} else if repo_model.IsErrUserDoesNotHaveAccessToRepo(err) { } else if repo_model.IsErrUserDoesNotHaveAccessToRepo(err) {
ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err.Error()) ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err.Error())

View file

@ -166,6 +166,7 @@ func TestBlockActions(t *testing.T) {
doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
blockedUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) blockedUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
blockedUser2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 10}) 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}) repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2, OwnerID: doer.ID})
repo7 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 7, OwnerID: blockedUser2.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}) 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, blockedUser)
BlockUser(t, doer, blockedUser2) 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) { t.Run("Issue creation", func(t *testing.T) {
defer tests.PrintCurrentTest(t)() defer tests.PrintCurrentTest(t)()
@ -192,19 +198,38 @@ func TestBlockActions(t *testing.T) {
"title": "Title", "title": "Title",
"content": "Hello!", "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 // Ensures that comment creation on doer's owned repositories and doer's
// posted issues are blocked. // posted issues are blocked.
t.Run("Comment creation", func(t *testing.T) { 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) { t.Run("Blocked by repository owner", func(t *testing.T) {
defer tests.PrintCurrentTest(t)() defer tests.PrintCurrentTest(t)()
@ -215,11 +240,12 @@ func TestBlockActions(t *testing.T) {
"_csrf": GetCSRF(t, session, issue10URL), "_csrf": GetCSRF(t, session, issue10URL),
"content": "Not a kind comment", "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) { t.Run("Blocked by issue poster", func(t *testing.T) {
@ -235,11 +261,12 @@ func TestBlockActions(t *testing.T) {
"_csrf": GetCSRF(t, session, issueURL), "_csrf": GetCSRF(t, session, issueURL),
"content": "Not a kind comment", "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)
}) })
}) })