Merge branch 'master' into graceful-queues

routers/api/v1/api.go

@@ -664,10 +664,10 @@ func RegisterRoutes(m *macaron.Macaron) {
 							m.Combo("", reqToken()).
 								Patch(mustNotBeArchived, bind(api.EditIssueCommentOption{}), repo.EditIssueComment).
 								Delete(repo.DeleteIssueComment)
-							m.Combo("/reactions", reqToken()).
+							m.Combo("/reactions").
 								Get(repo.GetIssueCommentReactions).
-								Post(bind(api.EditReactionOption{}), repo.PostIssueCommentReaction).
-								Delete(bind(api.EditReactionOption{}), repo.DeleteIssueCommentReaction)
+								Post(bind(api.EditReactionOption{}), reqToken(), repo.PostIssueCommentReaction).
+								Delete(bind(api.EditReactionOption{}), reqToken(), repo.DeleteIssueCommentReaction)
 						})
 					})
 					m.Group("/:index", func() {
@@ -704,10 +704,10 @@ func RegisterRoutes(m *macaron.Macaron) {
 							m.Put("/:user", reqToken(), repo.AddIssueSubscription)
 							m.Delete("/:user", reqToken(), repo.DelIssueSubscription)
 						})
-						m.Combo("/reactions", reqToken()).
+						m.Combo("/reactions").
 							Get(repo.GetIssueReactions).
-							Post(bind(api.EditReactionOption{}), repo.PostIssueReaction).
-							Delete(bind(api.EditReactionOption{}), repo.DeleteIssueReaction)
+							Post(bind(api.EditReactionOption{}), reqToken(), repo.PostIssueReaction).
+							Delete(bind(api.EditReactionOption{}), reqToken(), repo.DeleteIssueReaction)
 					})
 				}, mustEnableIssuesOrPulls)
 				m.Group("/labels", func() {

routers/api/v1/misc/markdown_test.go

@@ -69,7 +69,6 @@ func TestAPI_RenderGFM(t *testing.T) {
 - Bezier widget (by @r-lyeh) https://github.com/ocornut/imgui/issues/786`,
 		// rendered
 		`<p>Wiki! Enjoy :)</p>
-
 <ul>
 <li><a href="` + AppSubURL + `wiki/Links" rel="nofollow">Links, Language bindings, Engine bindings</a></li>
 <li><a href="` + AppSubURL + `wiki/Tips" rel="nofollow">Tips</a></li>
@@ -88,13 +87,9 @@ Here are some links to the most important topics. You can find the full list of
 `,
 		// rendered
 		`<h2 id="user-content-what-is-wine-staging">What is Wine Staging?</h2>
-
 <p><strong>Wine Staging</strong> on website <a href="http://wine-staging.com" rel="nofollow">wine-staging.com</a>.</p>
-
 <h2 id="user-content-quick-links">Quick Links</h2>
-
 <p>Here are some links to the most important topics. You can find the full list of pages at the sidebar.</p>
-
 <p><a href="` + AppSubURL + `wiki/Configuration" rel="nofollow">Configuration</a>
 <a href="` + AppSubURL + `wiki/raw/images/icon-bug.png" rel="nofollow"><img src="` + AppSubURL + `wiki/raw/images/icon-bug.png" title="icon-bug.png" alt="images/icon-bug.png"/></a></p>
 `,

routers/api/v1/repo/issue.go

@@ -524,8 +524,8 @@ func EditIssue(ctx *context.APIContext, form api.EditIssueOption) {
 		}
 	}
 
-	if err = models.UpdateIssue(issue); err != nil {
-		ctx.Error(http.StatusInternalServerError, "UpdateIssue", err)
+	if err = models.UpdateIssueByAPI(issue); err != nil {
+		ctx.Error(http.StatusInternalServerError, "UpdateIssueByAPI", err)
 		return
 	}
 	if form.State != nil {
@@ -542,7 +542,11 @@ func EditIssue(ctx *context.APIContext, form api.EditIssueOption) {
 	// Refetch from database to assign some automatic values
 	issue, err = models.GetIssueByID(issue.ID)
 	if err != nil {
-		ctx.Error(http.StatusInternalServerError, "GetIssueByID", err)
+		ctx.InternalServerError(err)
+		return
+	}
+	if err = issue.LoadMilestone(); err != nil {
+		ctx.InternalServerError(err)
 		return
 	}
 	ctx.JSON(http.StatusCreated, issue.APIFormat())

routers/api/v1/repo/issue_reaction.go

@@ -41,7 +41,7 @@ func GetIssueCommentReactions(ctx *context.APIContext) {
 	//   required: true
 	// responses:
 	//   "200":
-	//     "$ref": "#/responses/ReactionResponseList"
+	//     "$ref": "#/responses/ReactionList"
 	//   "403":
 	//     "$ref": "#/responses/forbidden"
 
@@ -55,7 +55,7 @@ func GetIssueCommentReactions(ctx *context.APIContext) {
 		return
 	}
 
-	if !ctx.Repo.CanRead(models.UnitTypeIssues) && !ctx.User.IsAdmin {
+	if !ctx.Repo.CanRead(models.UnitTypeIssues) {
 		ctx.Error(http.StatusForbidden, "GetIssueCommentReactions", errors.New("no permission to get reactions"))
 		return
 	}
@@ -71,9 +71,9 @@ func GetIssueCommentReactions(ctx *context.APIContext) {
 		return
 	}
 
-	var result []api.ReactionResponse
+	var result []api.Reaction
 	for _, r := range reactions {
-		result = append(result, api.ReactionResponse{
+		result = append(result, api.Reaction{
 			User:     r.User.APIFormat(),
 			Reaction: r.Type,
 			Created:  r.CreatedUnix.AsTime(),
@@ -114,8 +114,10 @@ func PostIssueCommentReaction(ctx *context.APIContext, form api.EditReactionOpti
 	//   schema:
 	//     "$ref": "#/definitions/EditReactionOption"
 	// responses:
+	//   "200":
+	//     "$ref": "#/responses/Reaction"
 	//   "201":
-	//     "$ref": "#/responses/ReactionResponse"
+	//     "$ref": "#/responses/Reaction"
 	//   "403":
 	//     "$ref": "#/responses/forbidden"
 
@@ -177,7 +179,7 @@ func changeIssueCommentReaction(ctx *context.APIContext, form api.EditReactionOp
 		ctx.Error(http.StatusInternalServerError, "comment.LoadIssue() failed", err)
 	}
 
-	if comment.Issue.IsLocked && !ctx.Repo.CanWrite(models.UnitTypeIssues) && !ctx.User.IsAdmin {
+	if comment.Issue.IsLocked && !ctx.Repo.CanWrite(models.UnitTypeIssues) {
 		ctx.Error(http.StatusForbidden, "ChangeIssueCommentReaction", errors.New("no permission to change reaction"))
 		return
 	}
@@ -188,19 +190,20 @@ func changeIssueCommentReaction(ctx *context.APIContext, form api.EditReactionOp
 		if err != nil {
 			if models.IsErrForbiddenIssueReaction(err) {
 				ctx.Error(http.StatusForbidden, err.Error(), err)
+			} else if models.IsErrReactionAlreadyExist(err) {
+				ctx.JSON(http.StatusOK, api.Reaction{
+					User:     ctx.User.APIFormat(),
+					Reaction: reaction.Type,
+					Created:  reaction.CreatedUnix.AsTime(),
+				})
 			} else {
 				ctx.Error(http.StatusInternalServerError, "CreateCommentReaction", err)
 			}
 			return
 		}
-		_, err = reaction.LoadUser()
-		if err != nil {
-			ctx.Error(http.StatusInternalServerError, "Reaction.LoadUser()", err)
-			return
-		}
 
-		ctx.JSON(http.StatusCreated, api.ReactionResponse{
-			User:     reaction.User.APIFormat(),
+		ctx.JSON(http.StatusCreated, api.Reaction{
+			User:     ctx.User.APIFormat(),
 			Reaction: reaction.Type,
 			Created:  reaction.CreatedUnix.AsTime(),
 		})
@@ -244,7 +247,7 @@ func GetIssueReactions(ctx *context.APIContext) {
 	//   required: true
 	// responses:
 	//   "200":
-	//     "$ref": "#/responses/ReactionResponseList"
+	//     "$ref": "#/responses/ReactionList"
 	//   "403":
 	//     "$ref": "#/responses/forbidden"
 
@@ -258,7 +261,7 @@ func GetIssueReactions(ctx *context.APIContext) {
 		return
 	}
 
-	if !ctx.Repo.CanRead(models.UnitTypeIssues) && !ctx.User.IsAdmin {
+	if !ctx.Repo.CanRead(models.UnitTypeIssues) {
 		ctx.Error(http.StatusForbidden, "GetIssueReactions", errors.New("no permission to get reactions"))
 		return
 	}
@@ -274,9 +277,9 @@ func GetIssueReactions(ctx *context.APIContext) {
 		return
 	}
 
-	var result []api.ReactionResponse
+	var result []api.Reaction
 	for _, r := range reactions {
-		result = append(result, api.ReactionResponse{
+		result = append(result, api.Reaction{
 			User:     r.User.APIFormat(),
 			Reaction: r.Type,
 			Created:  r.CreatedUnix.AsTime(),
@@ -317,8 +320,10 @@ func PostIssueReaction(ctx *context.APIContext, form api.EditReactionOption) {
 	//   schema:
 	//     "$ref": "#/definitions/EditReactionOption"
 	// responses:
+	//   "200":
+	//     "$ref": "#/responses/Reaction"
 	//   "201":
-	//     "$ref": "#/responses/ReactionResponse"
+	//     "$ref": "#/responses/Reaction"
 	//   "403":
 	//     "$ref": "#/responses/forbidden"
 
@@ -375,7 +380,7 @@ func changeIssueReaction(ctx *context.APIContext, form api.EditReactionOption, i
 		return
 	}
 
-	if issue.IsLocked && !ctx.Repo.CanWrite(models.UnitTypeIssues) && !ctx.User.IsAdmin {
+	if issue.IsLocked && !ctx.Repo.CanWrite(models.UnitTypeIssues) {
 		ctx.Error(http.StatusForbidden, "ChangeIssueCommentReaction", errors.New("no permission to change reaction"))
 		return
 	}
@@ -386,19 +391,20 @@ func changeIssueReaction(ctx *context.APIContext, form api.EditReactionOption, i
 		if err != nil {
 			if models.IsErrForbiddenIssueReaction(err) {
 				ctx.Error(http.StatusForbidden, err.Error(), err)
+			} else if models.IsErrReactionAlreadyExist(err) {
+				ctx.JSON(http.StatusOK, api.Reaction{
+					User:     ctx.User.APIFormat(),
+					Reaction: reaction.Type,
+					Created:  reaction.CreatedUnix.AsTime(),
+				})
 			} else {
 				ctx.Error(http.StatusInternalServerError, "CreateCommentReaction", err)
 			}
 			return
 		}
-		_, err = reaction.LoadUser()
-		if err != nil {
-			ctx.Error(http.StatusInternalServerError, "Reaction.LoadUser()", err)
-			return
-		}
 
-		ctx.JSON(http.StatusCreated, api.ReactionResponse{
-			User:     reaction.User.APIFormat(),
+		ctx.JSON(http.StatusCreated, api.Reaction{
+			User:     ctx.User.APIFormat(),
 			Reaction: reaction.Type,
 			Created:  reaction.CreatedUnix.AsTime(),
 		})

routers/api/v1/repo/pull.go

@@ -450,8 +450,8 @@ func EditPullRequest(ctx *context.APIContext, form api.EditPullRequestOption) {
 		}
 	}
 
-	if err = models.UpdateIssue(issue); err != nil {
-		ctx.Error(http.StatusInternalServerError, "UpdateIssue", err)
+	if err = models.UpdateIssueByAPI(issue); err != nil {
+		ctx.Error(http.StatusInternalServerError, "UpdateIssueByAPI", err)
 		return
 	}
 	if form.State != nil {

routers/api/v1/swagger/issue.go

@@ -99,23 +99,16 @@ type swaggerResponseStopWatchList struct {
 	Body []api.StopWatch `json:"body"`
 }
 
-// EditReactionOption
-// swagger:response EditReactionOption
-type swaggerEditReactionOption struct {
+// Reaction
+// swagger:response Reaction
+type swaggerReaction struct {
 	// in:body
-	Body api.EditReactionOption `json:"body"`
+	Body api.Reaction `json:"body"`
 }
 
-// ReactionResponse
-// swagger:response ReactionResponse
-type swaggerReactionResponse struct {
+// ReactionList
+// swagger:response ReactionList
+type swaggerReactionList struct {
 	// in:body
-	Body api.ReactionResponse `json:"body"`
-}
-
-// ReactionResponseList
-// swagger:response ReactionResponseList
-type swaggerReactionResponseList struct {
-	// in:body
-	Body []api.ReactionResponse `json:"body"`
+	Body []api.Reaction `json:"body"`
 }

routers/api/v1/swagger/options.go

@@ -123,4 +123,7 @@ type swaggerParameterBodies struct {
 
 	// in:body
 	RepoTopicOptions api.RepoTopicOptions
+
+	// in:body
+	EditReactionOption api.EditReactionOption
 }

routers/private/hook.go

@@ -113,6 +113,13 @@ func HookPreReceive(ctx *macaron.Context, opts private.HookOptions) {
 					})
 					return
 				}
+				if protectBranch.MergeBlockedByRejectedReview(pr) {
+					log.Warn("Forbidden: User %d cannot push to protected branch: %s in %-v and pr #%d has requested changes", opts.UserID, branchName, repo, pr.Index)
+					ctx.JSON(http.StatusForbidden, map[string]interface{}{
+						"err": fmt.Sprintf("protected branch %s can not be pushed to and pr #%d has requested changes", branchName, opts.ProtectedBranchID),
+					})
+					return
+				}
 			} else if !canPush {
 				log.Warn("Forbidden: User %d cannot push to protected branch: %s in %-v", opts.UserID, branchName, repo)
 				ctx.JSON(http.StatusForbidden, map[string]interface{}{

routers/repo/attachment.go

@@ -6,6 +6,8 @@ package repo
 
 import (
 	"fmt"
+	"net/http"
+	"os"
 	"strings"
 
 	"code.gitea.io/gitea/models"
@@ -85,3 +87,57 @@ func DeleteAttachment(ctx *context.Context) {
 		"uuid": attach.UUID,
 	})
 }
+
+// GetAttachment serve attachements
+func GetAttachment(ctx *context.Context) {
+	attach, err := models.GetAttachmentByUUID(ctx.Params(":uuid"))
+	if err != nil {
+		if models.IsErrAttachmentNotExist(err) {
+			ctx.Error(404)
+		} else {
+			ctx.ServerError("GetAttachmentByUUID", err)
+		}
+		return
+	}
+
+	repository, unitType, err := attach.LinkedRepository()
+	if err != nil {
+		ctx.ServerError("LinkedRepository", err)
+		return
+	}
+
+	if repository == nil { //If not linked
+		if !(ctx.IsSigned && attach.UploaderID == ctx.User.ID) { //We block if not the uploader
+			ctx.Error(http.StatusNotFound)
+			return
+		}
+	} else { //If we have the repository we check access
+		perm, err := models.GetUserRepoPermission(repository, ctx.User)
+		if err != nil {
+			ctx.Error(http.StatusInternalServerError, "GetUserRepoPermission", err.Error())
+			return
+		}
+		if !perm.CanRead(unitType) {
+			ctx.Error(http.StatusNotFound)
+			return
+		}
+	}
+
+	//If we have matched and access to release or issue
+	fr, err := os.Open(attach.LocalPath())
+	if err != nil {
+		ctx.ServerError("Open", err)
+		return
+	}
+	defer fr.Close()
+
+	if err := attach.IncreaseDownloadCount(); err != nil {
+		ctx.ServerError("Update", err)
+		return
+	}
+
+	if err = ServeData(ctx, attach.Name, fr); err != nil {
+		ctx.ServerError("ServeData", err)
+		return
+	}
+}

routers/repo/issue.go

@@ -23,7 +23,6 @@ import (
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/markup"
 	"code.gitea.io/gitea/modules/markup/markdown"
-	"code.gitea.io/gitea/modules/notification"
 	"code.gitea.io/gitea/modules/setting"
 	api "code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/util"
@@ -963,7 +962,8 @@ func ViewIssue(ctx *context.Context) {
 		}
 		if pull.ProtectedBranch != nil {
 			cnt := pull.ProtectedBranch.GetGrantedApprovalsCount(pull)
-			ctx.Data["IsBlockedByApprovals"] = pull.ProtectedBranch.RequiredApprovals > 0 && cnt < pull.ProtectedBranch.RequiredApprovals
+			ctx.Data["IsBlockedByApprovals"] = !pull.ProtectedBranch.HasEnoughApprovals(pull)
+			ctx.Data["IsBlockedByRejection"] = pull.ProtectedBranch.MergeBlockedByRejectedReview(pull)
 			ctx.Data["GrantedApprovals"] = cnt
 		}
 		ctx.Data["IsPullBranchDeletable"] = canDelete && pull.HeadRepo != nil && git.IsBranchExist(pull.HeadRepo.RepoPath(), pull.HeadBranch)
@@ -1177,13 +1177,11 @@ func UpdateIssueAssignee(ctx *context.Context) {
 				return
 			}
 
-			removed, comment, err := issue_service.ToggleAssignee(issue, ctx.User, assigneeID)
+			_, _, err = issue_service.ToggleAssignee(issue, ctx.User, assigneeID)
 			if err != nil {
 				ctx.ServerError("ToggleAssignee", err)
 				return
 			}
-
-			notification.NotifyIssueChangeAssignee(ctx.User, issue, assignee, removed, comment)
 		}
 	}
 	ctx.JSON(200, map[string]interface{}{

routers/repo/setting_protected_branch.go

@@ -244,6 +244,7 @@ func SettingsProtectedBranchPost(ctx *context.Context, f auth.ProtectBranchForm)
 				approvalsWhitelistTeams, _ = base.StringsToInt64s(strings.Split(f.ApprovalsWhitelistTeams, ","))
 			}
 		}
+		protectBranch.BlockOnRejectedReviews = f.BlockOnRejectedReviews
 
 		err = models.UpdateProtectBranch(ctx.Repo.Repository, protectBranch, models.WhitelistOptions{
 			UserIDs:          whitelistUsers,

routers/routes/routes.go

@@ -8,7 +8,6 @@ import (
 	"bytes"
 	"encoding/gob"
 	"net/http"
-	"os"
 	"path"
 	"text/template"
 	"time"
@@ -482,34 +481,7 @@ func RegisterRoutes(m *macaron.Macaron) {
 			m.Get("/following", user.Following)
 		})
 
-		m.Get("/attachments/:uuid", func(ctx *context.Context) {
-			attach, err := models.GetAttachmentByUUID(ctx.Params(":uuid"))
-			if err != nil {
-				if models.IsErrAttachmentNotExist(err) {
-					ctx.Error(404)
-				} else {
-					ctx.ServerError("GetAttachmentByUUID", err)
-				}
-				return
-			}
-
-			fr, err := os.Open(attach.LocalPath())
-			if err != nil {
-				ctx.ServerError("Open", err)
-				return
-			}
-			defer fr.Close()
-
-			if err := attach.IncreaseDownloadCount(); err != nil {
-				ctx.ServerError("Update", err)
-				return
-			}
-
-			if err = repo.ServeData(ctx, attach.Name, fr); err != nil {
-				ctx.ServerError("ServeData", err)
-				return
-			}
-		})
+		m.Get("/attachments/:uuid", repo.GetAttachment)
 	}, ignSignIn)
 
 	m.Group("/attachments", func() {

routers/user/home.go

@@ -389,21 +389,23 @@ func Issues(ctx *context.Context) {
 
 	reposQuery := ctx.Query("repos")
 	var repoIDs []int64
-	if issueReposQueryPattern.MatchString(reposQuery) {
-		// remove "[" and "]" from string
-		reposQuery = reposQuery[1 : len(reposQuery)-1]
-		//for each ID (delimiter ",") add to int to repoIDs
-		for _, rID := range strings.Split(reposQuery, ",") {
-			// Ensure nonempty string entries
-			if rID != "" && rID != "0" {
-				rIDint64, err := strconv.ParseInt(rID, 10, 64)
-				if err == nil {
-					repoIDs = append(repoIDs, rIDint64)
+	if len(reposQuery) != 0 {
+		if issueReposQueryPattern.MatchString(reposQuery) {
+			// remove "[" and "]" from string
+			reposQuery = reposQuery[1 : len(reposQuery)-1]
+			//for each ID (delimiter ",") add to int to repoIDs
+			for _, rID := range strings.Split(reposQuery, ",") {
+				// Ensure nonempty string entries
+				if rID != "" && rID != "0" {
+					rIDint64, err := strconv.ParseInt(rID, 10, 64)
+					if err == nil {
+						repoIDs = append(repoIDs, rIDint64)
+					}
 				}
 			}
+		} else {
+			log.Error("issueReposQueryPattern not match with query")
 		}
-	} else {
-		log.Error("issueReposQueryPattern not match with query")
 	}
 
 	isShowClosed := ctx.Query("state") == "closed"

routers/user/home_test.go

@@ -26,10 +26,10 @@ func TestIssues(t *testing.T) {
 	Issues(ctx)
 	assert.EqualValues(t, http.StatusOK, ctx.Resp.Status())
 
-	assert.EqualValues(t, map[int64]int64{1: 1}, ctx.Data["Counts"])
+	assert.EqualValues(t, map[int64]int64{1: 1, 2: 1}, ctx.Data["Counts"])
 	assert.EqualValues(t, true, ctx.Data["IsShowClosed"])
 	assert.Len(t, ctx.Data["Issues"], 1)
-	assert.Len(t, ctx.Data["Repos"], 1)
+	assert.Len(t, ctx.Data["Repos"], 2)
 }
 
 func TestMilestones(t *testing.T) {