Fix PR, milestone and label functionality if issue unit is disabled (#2710)

* Fix PR, milestone and label functionality if issue unit is disabled or not assigned to user

* Fix multi-actions in PR page

* Change error message

* Fix comment update and delete functionality in PR

5 files changed, 49 insertions, 42 deletions

diff --git a/routers/repo/issue.go b/routers/repo/issue.go
index 091268116b..c24a4e4360 100644
--- a/routers/repo/issue.go
+++ b/routers/repo/issue.go
@@ -720,11 +720,16 @@ func ViewIssue(ctx *context.Context) {
 func GetActionIssue(ctx *context.Context) *models.Issue {
 	issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index"))
 	if err != nil {
-		if models.IsErrIssueNotExist(err) {
-			ctx.Error(404, "GetIssueByIndex")
-		} else {
-			ctx.Handle(500, "GetIssueByIndex", err)
-		}
+		ctx.NotFoundOrServerError("GetIssueByIndex", models.IsErrIssueNotExist, err)
+		return nil
+	}
+	if issue.IsPull && !ctx.Repo.Repository.UnitEnabled(models.UnitTypePullRequests) ||
+		!issue.IsPull && !ctx.Repo.Repository.UnitEnabled(models.UnitTypeIssues) {
+		ctx.Handle(404, "IssueOrPullRequestUnitNotAllowed", nil)
+		return nil
+	}
+	if err = issue.LoadAttributes(); err != nil {
+		ctx.Handle(500, "LoadAttributes", nil)
 		return nil
 	}
 	return issue
@@ -749,6 +754,19 @@ func getActionIssues(ctx *context.Context) []*models.Issue {
 		ctx.Handle(500, "GetIssuesByIDs", err)
 		return nil
 	}
+	// Check access rights for all issues
+	issueUnitEnabled := ctx.Repo.Repository.UnitEnabled(models.UnitTypeIssues)
+	prUnitEnabled := ctx.Repo.Repository.UnitEnabled(models.UnitTypePullRequests)
+	for _, issue := range issues {
+		if issue.IsPull && !prUnitEnabled || !issue.IsPull && !issueUnitEnabled {
+			ctx.Handle(404, "IssueOrPullRequestUnitNotAllowed", nil)
+			return nil
+		}
+		if err = issue.LoadAttributes(); err != nil {
+			ctx.Handle(500, "LoadAttributes", nil)
+			return nil
+		}
+	}
 	return issues
 }
 
@@ -884,9 +902,8 @@ func UpdateIssueStatus(ctx *context.Context) {
 
 // NewComment create a comment for issue
 func NewComment(ctx *context.Context, form auth.CreateCommentForm) {
-	issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index"))
-	if err != nil {
-		ctx.NotFoundOrServerError("GetIssueByIndex", models.IsErrIssueNotExist, err)
+	issue := GetActionIssue(ctx)
+	if ctx.Written() {
 		return
 	}
 
@@ -913,7 +930,7 @@ func NewComment(ctx *context.Context, form auth.CreateCommentForm) {
 
 			if form.Status == "reopen" && issue.IsPull {
 				pull := issue.PullRequest
-				pr, err = models.GetUnmergedPullRequest(pull.HeadRepoID, pull.BaseRepoID, pull.HeadBranch, pull.BaseBranch)
+				pr, err := models.GetUnmergedPullRequest(pull.HeadRepoID, pull.BaseRepoID, pull.HeadBranch, pull.BaseBranch)
 				if err != nil {
 					if !models.IsErrPullRequestNotExist(err) {
 						ctx.Handle(500, "GetUnmergedPullRequest", err)
@@ -935,7 +952,7 @@ func NewComment(ctx *context.Context, form auth.CreateCommentForm) {
 			if pr != nil {
 				ctx.Flash.Info(ctx.Tr("repo.pulls.open_unmerged_pull_exists", pr.Index))
 			} else {
-				if err = issue.ChangeStatus(ctx.User, ctx.Repo.Repository, form.Status == "close"); err != nil {
+				if err := issue.ChangeStatus(ctx.User, ctx.Repo.Repository, form.Status == "close"); err != nil {
 					log.Error(4, "ChangeStatus: %v", err)
 				} else {
 					log.Trace("Issue [%d] status changed to closed: %v", issue.ID, issue.IsClosed)
@@ -962,7 +979,7 @@ func NewComment(ctx *context.Context, form auth.CreateCommentForm) {
 		return
 	}
 
-	comment, err = models.CreateIssueComment(ctx.User, ctx.Repo.Repository, issue, form.Content, attachments)
+	comment, err := models.CreateIssueComment(ctx.User, ctx.Repo.Repository, issue, form.Content, attachments)
 	if err != nil {
 		ctx.Handle(500, "CreateIssueComment", err)
 		return
@@ -1032,10 +1049,6 @@ func DeleteComment(ctx *context.Context) {
 
 // Milestones render milestones page
 func Milestones(ctx *context.Context) {
-	MustEnableIssues(ctx)
-	if ctx.Written() {
-		return
-	}
 	ctx.Data["Title"] = ctx.Tr("repo.milestones")
 	ctx.Data["PageIsIssueList"] = true
 	ctx.Data["PageIsMilestones"] = true
diff --git a/routers/repo/issue_label.go b/routers/repo/issue_label.go
index 3422677943..9b4da4b500 100644
--- a/routers/repo/issue_label.go
+++ b/routers/repo/issue_label.go
@@ -18,10 +18,6 @@ const (
 
 // Labels render issue's labels page
 func Labels(ctx *context.Context) {
-	MustEnableIssues(ctx)
-	if ctx.Written() {
-		return
-	}
 	ctx.Data["Title"] = ctx.Tr("repo.labels")
 	ctx.Data["PageIsIssueList"] = true
 	ctx.Data["PageIsLabels"] = true
diff --git a/routers/repo/issue_stopwatch.go b/routers/repo/issue_stopwatch.go
index 7e3121da9f..f4392849aa 100644
--- a/routers/repo/issue_stopwatch.go
+++ b/routers/repo/issue_stopwatch.go
@@ -13,11 +13,12 @@ import (
 
 // IssueStopwatch creates or stops a stopwatch for the given issue.
 func IssueStopwatch(c *context.Context) {
-	issueIndex := c.ParamsInt64("index")
-	issue, err := models.GetIssueByIndex(c.Repo.Repository.ID, issueIndex)
-
-	if err != nil {
-		c.Handle(http.StatusInternalServerError, "GetIssueByIndex", err)
+	issue := GetActionIssue(c)
+	if c.Written() {
+		return
+	}
+	if !c.Repo.CanUseTimetracker(issue, c.User) {
+		c.Handle(http.StatusNotFound, "CanUseTimetracker", nil)
 		return
 	}
 
@@ -32,11 +33,12 @@ func IssueStopwatch(c *context.Context) {
 
 // CancelStopwatch cancel the stopwatch
 func CancelStopwatch(c *context.Context) {
-	issueIndex := c.ParamsInt64("index")
-	issue, err := models.GetIssueByIndex(c.Repo.Repository.ID, issueIndex)
-
-	if err != nil {
-		c.Handle(http.StatusInternalServerError, "GetIssueByIndex", err)
+	issue := GetActionIssue(c)
+	if c.Written() {
+		return
+	}
+	if !c.Repo.CanUseTimetracker(issue, c.User) {
+		c.Handle(http.StatusNotFound, "CanUseTimetracker", nil)
 		return
 	}
 
diff --git a/routers/repo/issue_timetrack.go b/routers/repo/issue_timetrack.go
index 4d77ca3cea..d89c67b497 100644
--- a/routers/repo/issue_timetrack.go
+++ b/routers/repo/issue_timetrack.go
@@ -15,14 +15,12 @@ import (
 
 // AddTimeManually tracks time manually
 func AddTimeManually(c *context.Context, form auth.AddTimeManuallyForm) {
-	issueIndex := c.ParamsInt64("index")
-	issue, err := models.GetIssueByIndex(c.Repo.Repository.ID, issueIndex)
-	if err != nil {
-		if models.IsErrIssueNotExist(err) {
-			c.Handle(http.StatusNotFound, "GetIssueByIndex", err)
-			return
-		}
-		c.Handle(http.StatusInternalServerError, "GetIssueByIndex", err)
+	issue := GetActionIssue(c)
+	if c.Written() {
+		return
+	}
+	if !c.Repo.CanUseTimetracker(issue, c.User) {
+		c.Handle(http.StatusNotFound, "CanUseTimetracker", nil)
 		return
 	}
 	url := issue.HTMLURL()
diff --git a/routers/repo/issue_watch.go b/routers/repo/issue_watch.go
index 382798025e..42ffaec5b8 100644
--- a/routers/repo/issue_watch.go
+++ b/routers/repo/issue_watch.go
@@ -21,10 +21,8 @@ func IssueWatch(c *context.Context) {
 		return
 	}
 
-	issueIndex := c.ParamsInt64("index")
-	issue, err := models.GetIssueByIndex(c.Repo.Repository.ID, issueIndex)
-	if err != nil {
-		c.Handle(http.StatusInternalServerError, "GetIssueByIndex", err)
+	issue := GetActionIssue(c)
+	if c.Written() {
 		return
 	}
 
@@ -33,6 +31,6 @@ func IssueWatch(c *context.Context) {
 		return
 	}
 
-	url := fmt.Sprintf("%s/issues/%d", c.Repo.RepoLink, issueIndex)
+	url := fmt.Sprintf("%s/issues/%d", c.Repo.RepoLink, issue.Index)
 	c.Redirect(url, http.StatusSeeOther)
 }