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

diff --git a/routers/repo/issue.go b/routers/repo/issue.go
index 091268116bf4f4f05d0f37bc1cc7e0606a360df9..c24a4e4360d415759ecd60d79630ec6f25f5059b 100644 (file)
--- 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 3422677943e6b10580d76f65e82b63c862a5119c..9b4da4b5000cd65da217ec68a00adea5ad4692d2 100644 (file)
--- 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 7e3121da9f7e5f23fcb53f25bd7434a49d8981a2..f4392849aa9f86f74c936a008b825899590dc587 100644 (file)
--- 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 4d77ca3cea21014b9b0f5d18074d53e84cc999f2..d89c67b497da3810f69b72cb49f793b3777c0113 100644 (file)
--- 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 382798025e86392bf50f3ee7962d232dcb9265bc..42ffaec5b8ac2dc0bf37e26f8e3c2445d60597f7 100644 (file)
--- 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)
 }

diff --git a/routers/routes/routes.go b/routers/routes/routes.go
index 9d9e65643326cc7104507d3050e73c783414d655..a6f73aaedd9a9f82e19ea6f2a5ef6a32d64bd205 100644 (file)
--- a/routers/routes/routes.go
+++ b/routers/routes/routes.go
@@ -474,12 +474,13 @@ func RegisterRoutes(m *macaron.Macaron) {
        m.Get("/:username/:reponame/action/:action", reqSignIn, context.RepoAssignment(), repo.Action)
 
        m.Group("/:username/:reponame", func() {
-               // FIXME: should use different URLs but mostly same logic for comments of issue and pull reuqest.
-               // So they can apply their own enable/disable logic on routers.
                m.Group("/issues", func() {
                        m.Combo("/new").Get(context.RepoRef(), repo.NewIssue).
                                Post(bindIgnErr(auth.CreateIssueForm{}), repo.NewIssuePost)
-
+               }, context.CheckUnit(models.UnitTypeIssues))
+               // FIXME: should use different URLs but mostly same logic for comments of issue and pull reuqest.
+               // So they can apply their own enable/disable logic on routers.
+               m.Group("/issues", func() {
                        m.Group("/:index", func() {
                                m.Post("/title", repo.UpdateIssueTitle)
                                m.Post("/content", repo.UpdateIssueContent)
@@ -491,30 +492,24 @@ func RegisterRoutes(m *macaron.Macaron) {
                                                m.Post("/toggle", repo.IssueStopwatch)
                                                m.Post("/cancel", repo.CancelStopwatch)
                                        })
-
-                               }, func(ctx *context.Context) {
-                                       if !ctx.Repo.CanUseTimetracker(repo.GetActionIssue(ctx), ctx.User) {
-                                               ctx.Handle(404, ctx.Req.RequestURI, nil)
-                                               return
-                                       }
                                })
                        })
 
-                       m.Post("/labels", repo.UpdateIssueLabel, reqRepoWriter)
-                       m.Post("/milestone", repo.UpdateIssueMilestone, reqRepoWriter)
-                       m.Post("/assignee", repo.UpdateIssueAssignee, reqRepoWriter)
-                       m.Post("/status", repo.UpdateIssueStatus, reqRepoWriter)
-               }, context.CheckUnit(models.UnitTypeIssues))
+                       m.Post("/labels", reqRepoWriter, repo.UpdateIssueLabel)
+                       m.Post("/milestone", reqRepoWriter, repo.UpdateIssueMilestone)
+                       m.Post("/assignee", reqRepoWriter, repo.UpdateIssueAssignee)
+                       m.Post("/status", reqRepoWriter, repo.UpdateIssueStatus)
+               })
                m.Group("/comments/:id", func() {
                        m.Post("", repo.UpdateCommentContent)
                        m.Post("/delete", repo.DeleteComment)
-               }, context.CheckUnit(models.UnitTypeIssues))
+               }, context.CheckAnyUnit(models.UnitTypeIssues, models.UnitTypePullRequests))
                m.Group("/labels", func() {
                        m.Post("/new", bindIgnErr(auth.CreateLabelForm{}), repo.NewLabel)
                        m.Post("/edit", bindIgnErr(auth.CreateLabelForm{}), repo.UpdateLabel)
                        m.Post("/delete", repo.DeleteLabel)
                        m.Post("/initialize", bindIgnErr(auth.InitializeLabelsForm{}), repo.InitializeLabels)
-               }, reqRepoWriter, context.RepoRef(), context.CheckUnit(models.UnitTypeIssues))
+               }, reqRepoWriter, context.RepoRef(), context.CheckAnyUnit(models.UnitTypeIssues, models.UnitTypePullRequests))
                m.Group("/milestones", func() {
                        m.Combo("/new").Get(repo.NewMilestone).
                                Post(bindIgnErr(auth.CreateMilestoneForm{}), repo.NewMilestonePost)
@@ -522,7 +517,7 @@ func RegisterRoutes(m *macaron.Macaron) {
                        m.Post("/:id/edit", bindIgnErr(auth.CreateMilestoneForm{}), repo.EditMilestonePost)
                        m.Get("/:id/:action", repo.ChangeMilestonStatus)
                        m.Post("/delete", repo.DeleteMilestone)
-               }, reqRepoWriter, context.RepoRef(), context.CheckUnit(models.UnitTypeIssues))
+               }, reqRepoWriter, context.RepoRef(), context.CheckAnyUnit(models.UnitTypeIssues, models.UnitTypePullRequests))
 
                m.Combo("/compare/*", repo.MustAllowPulls, repo.SetEditorconfigIfExists).
                        Get(repo.CompareAndPullRequest).
@@ -593,8 +588,8 @@ func RegisterRoutes(m *macaron.Macaron) {
                m.Group("", func() {
                        m.Get("/^:type(issues|pulls)$", repo.RetrieveLabels, repo.Issues)
                        m.Get("/^:type(issues|pulls)$/:index", repo.ViewIssue)
-                       m.Get("/labels/", repo.RetrieveLabels, repo.Labels)
-                       m.Get("/milestones", repo.Milestones)
+                       m.Get("/labels/", context.CheckAnyUnit(models.UnitTypeIssues, models.UnitTypePullRequests), repo.RetrieveLabels, repo.Labels)
+                       m.Get("/milestones", context.CheckAnyUnit(models.UnitTypeIssues, models.UnitTypePullRequests), repo.Milestones)
                }, context.RepoRef())
 
                m.Group("/wiki", func() {

diff --git a/templates/repo/issue/list.tmpl b/templates/repo/issue/list.tmpl
index 5c5a5749769008c876dd754b02d340e67f4de466..711d70f2a0c2a34842119436598c1997818cda8f 100644 (file)
--- a/templates/repo/issue/list.tmpl
+++ b/templates/repo/issue/list.tmpl
@@ -103,8 +103,8 @@
                </div>
                <div class="issue-actions">
                        <div class="ui basic status buttons">
-                               <div class="ui green active basic button issue-action" data-action="open" data-url="{{$.Link}}/status">{{.i18n.Tr "repo.issues.action_open"}}</div>
-                               <div class="ui red active basic button issue-action" data-action="close" data-url="{{$.Link}}/status">{{.i18n.Tr "repo.issues.action_close"}}</div>
+                               <div class="ui green active basic button issue-action" data-action="open" data-url="{{$.RepoLink}}/issues/status">{{.i18n.Tr "repo.issues.action_open"}}</div>
+                               <div class="ui red active basic button issue-action" data-action="close" data-url="{{$.RepoLink}}/issues/status">{{.i18n.Tr "repo.issues.action_close"}}</div>
                        </div>
 
                        <div class="ui secondary filter menu floated right">
@@ -116,7 +116,7 @@
                                        </span>
                                        <div class="menu">
                                                {{range .Labels}}
-                                                       <div class="item issue-action" data-action="toggle" data-element-id="{{.ID}}" data-url="{{$.Link}}/labels">
+                                                       <div class="item issue-action" data-action="toggle" data-element-id="{{.ID}}" data-url="{{$.RepoLink}}/issues/labels">
                                                                <span class="octicon {{if eq $.SelectLabels .ID}}octicon-check{{end}}"></span><span class="label color" style="background-color: {{.Color}}"></span> {{.Name | Sanitize}}
                                                        </div>
                                                {{end}}
@@ -134,7 +134,7 @@
                                                  {{.i18n.Tr "repo.issues.action_milestone_no_select"}}
                                                </div>
                                                {{range .Milestones}}
-                                                       <div class="item issue-action" data-element-id="{{.ID}}" data-url="{{$.Link}}/milestone">
+                                                       <div class="item issue-action" data-element-id="{{.ID}}" data-url="{{$.RepoLink}}/issues/milestone">
                                                                {{.Name | Sanitize}}
                                                        </div>
                                                {{end}}
@@ -152,7 +152,7 @@
                                                        {{.i18n.Tr "repo.issues.action_assignee_no_select"}}
                                                </div>
                                                {{range .Assignees}}
-                                                       <div class="item issue-action" data-element-id="{{.ID}}" data-url="{{$.Link}}/assignee">
+                                                       <div class="item issue-action" data-element-id="{{.ID}}" data-url="{{$.RepoLink}}/issues/assignee">
                                                                <img src="{{.RelAvatarLink}}"> {{.Name}}
                                                        </div>
                                                {{end}}