coolaj86
/
gitroast
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

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
This commit is contained in:
Lauris BH 2017-10-16 10:55:43 +03:00 committed by GitHub
parent a75d5c72bb
commit c1b0c9e7c4
7 changed files with 68 additions and 66 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
routers/repo/issue.go
View File

@ -720,11 +720,16 @@ func ViewIssue(ctx *context.Context) {
func GetActionIssue(ctx *context.Context) *models.Issue { func GetActionIssue(ctx *context.Context) *models.Issue {
issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index"))
if err != nil { if err != nil {
if models.IsErrIssueNotExist(err) { ctx.NotFoundOrServerError("GetIssueByIndex", models.IsErrIssueNotExist, err)
ctx.Error(404, "GetIssueByIndex") return nil
} else {
ctx.Handle(500, "GetIssueByIndex", err)
} }
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 nil
} }
return issue return issue
@ -749,6 +754,19 @@ func getActionIssues(ctx *context.Context) []*models.Issue {
ctx.Handle(500, "GetIssuesByIDs", err) ctx.Handle(500, "GetIssuesByIDs", err)
return nil 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 return issues
} }
@ -884,9 +902,8 @@ func UpdateIssueStatus(ctx *context.Context) {
// NewComment create a comment for issue // NewComment create a comment for issue
func NewComment(ctx *context.Context, form auth.CreateCommentForm) { func NewComment(ctx *context.Context, form auth.CreateCommentForm) {
issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) issue := GetActionIssue(ctx)
if err != nil { if ctx.Written() {
ctx.NotFoundOrServerError("GetIssueByIndex", models.IsErrIssueNotExist, err)
return return
} }
@ -913,7 +930,7 @@ func NewComment(ctx *context.Context, form auth.CreateCommentForm) {
if form.Status == "reopen" && issue.IsPull { if form.Status == "reopen" && issue.IsPull {
pull := issue.PullRequest 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 err != nil {
if !models.IsErrPullRequestNotExist(err) { if !models.IsErrPullRequestNotExist(err) {
ctx.Handle(500, "GetUnmergedPullRequest", err) ctx.Handle(500, "GetUnmergedPullRequest", err)
@ -935,7 +952,7 @@ func NewComment(ctx *context.Context, form auth.CreateCommentForm) {
if pr != nil { if pr != nil {
ctx.Flash.Info(ctx.Tr("repo.pulls.open_unmerged_pull_exists", pr.Index)) ctx.Flash.Info(ctx.Tr("repo.pulls.open_unmerged_pull_exists", pr.Index))
} else { } 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) log.Error(4, "ChangeStatus: %v", err)
} else { } else {
log.Trace("Issue [%d] status changed to closed: %v", issue.ID, issue.IsClosed) 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 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 { if err != nil {
ctx.Handle(500, "CreateIssueComment", err) ctx.Handle(500, "CreateIssueComment", err)
return return
@ -1032,10 +1049,6 @@ func DeleteComment(ctx *context.Context) {
// Milestones render milestones page // Milestones render milestones page
func Milestones(ctx *context.Context) { func Milestones(ctx *context.Context) {
MustEnableIssues(ctx)
if ctx.Written() {
return
}
ctx.Data["Title"] = ctx.Tr("repo.milestones") ctx.Data["Title"] = ctx.Tr("repo.milestones")
ctx.Data["PageIsIssueList"] = true ctx.Data["PageIsIssueList"] = true
ctx.Data["PageIsMilestones"] = true ctx.Data["PageIsMilestones"] = true

4
routers/repo/issue_label.go
View File

@ -18,10 +18,6 @@ const (
// Labels render issue's labels page // Labels render issue's labels page
func Labels(ctx *context.Context) { func Labels(ctx *context.Context) {
MustEnableIssues(ctx)
if ctx.Written() {
return
}
ctx.Data["Title"] = ctx.Tr("repo.labels") ctx.Data["Title"] = ctx.Tr("repo.labels")
ctx.Data["PageIsIssueList"] = true ctx.Data["PageIsIssueList"] = true
ctx.Data["PageIsLabels"] = true ctx.Data["PageIsLabels"] = true

22
routers/repo/issue_stopwatch.go
View File

@ -13,11 +13,12 @@ import (
// IssueStopwatch creates or stops a stopwatch for the given issue. // IssueStopwatch creates or stops a stopwatch for the given issue.
func IssueStopwatch(c *context.Context) { func IssueStopwatch(c *context.Context) {
issueIndex := c.ParamsInt64("index") issue := GetActionIssue(c)
issue, err := models.GetIssueByIndex(c.Repo.Repository.ID, issueIndex) if c.Written() {
return
if err != nil { }
c.Handle(http.StatusInternalServerError, "GetIssueByIndex", err) if !c.Repo.CanUseTimetracker(issue, c.User) {
c.Handle(http.StatusNotFound, "CanUseTimetracker", nil)
return return
} }
@ -32,11 +33,12 @@ func IssueStopwatch(c *context.Context) {
// CancelStopwatch cancel the stopwatch // CancelStopwatch cancel the stopwatch
func CancelStopwatch(c *context.Context) { func CancelStopwatch(c *context.Context) {
issueIndex := c.ParamsInt64("index") issue := GetActionIssue(c)
issue, err := models.GetIssueByIndex(c.Repo.Repository.ID, issueIndex) if c.Written() {
return
if err != nil { }
c.Handle(http.StatusInternalServerError, "GetIssueByIndex", err) if !c.Repo.CanUseTimetracker(issue, c.User) {
c.Handle(http.StatusNotFound, "CanUseTimetracker", nil)
return return
} }

10
routers/repo/issue_timetrack.go
View File

@ -15,14 +15,12 @@ import (
// AddTimeManually tracks time manually // AddTimeManually tracks time manually
func AddTimeManually(c *context.Context, form auth.AddTimeManuallyForm) { func AddTimeManually(c *context.Context, form auth.AddTimeManuallyForm) {
issueIndex := c.ParamsInt64("index") issue := GetActionIssue(c)
issue, err := models.GetIssueByIndex(c.Repo.Repository.ID, issueIndex) if c.Written() {
if err != nil {
if models.IsErrIssueNotExist(err) {
c.Handle(http.StatusNotFound, "GetIssueByIndex", err)
return return
} }
c.Handle(http.StatusInternalServerError, "GetIssueByIndex", err) if !c.Repo.CanUseTimetracker(issue, c.User) {
c.Handle(http.StatusNotFound, "CanUseTimetracker", nil)
return return
} }
url := issue.HTMLURL() url := issue.HTMLURL()

8
routers/repo/issue_watch.go
View File

@ -21,10 +21,8 @@ func IssueWatch(c *context.Context) {
return return
} }
issueIndex := c.ParamsInt64("index") issue := GetActionIssue(c)
issue, err := models.GetIssueByIndex(c.Repo.Repository.ID, issueIndex) if c.Written() {
if err != nil {
c.Handle(http.StatusInternalServerError, "GetIssueByIndex", err)
return return
} }
@ -33,6 +31,6 @@ func IssueWatch(c *context.Context) {
return 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) c.Redirect(url, http.StatusSeeOther)
} }

33
routers/routes/routes.go
View File

@ -474,12 +474,13 @@ func RegisterRoutes(m *macaron.Macaron) {
m.Get("/:username/:reponame/action/:action", reqSignIn, context.RepoAssignment(), repo.Action) m.Get("/:username/:reponame/action/:action", reqSignIn, context.RepoAssignment(), repo.Action)
m.Group("/:username/:reponame", func() { 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.Group("/issues", func() {
m.Combo("/new").Get(context.RepoRef(), repo.NewIssue). m.Combo("/new").Get(context.RepoRef(), repo.NewIssue).
Post(bindIgnErr(auth.CreateIssueForm{}), repo.NewIssuePost) 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.Group("/:index", func() {
m.Post("/title", repo.UpdateIssueTitle) m.Post("/title", repo.UpdateIssueTitle)
m.Post("/content", repo.UpdateIssueContent) m.Post("/content", repo.UpdateIssueContent)
@ -491,30 +492,24 @@ func RegisterRoutes(m *macaron.Macaron) {
m.Post("/toggle", repo.IssueStopwatch) m.Post("/toggle", repo.IssueStopwatch)
m.Post("/cancel", repo.CancelStopwatch) 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("/labels", reqRepoWriter, repo.UpdateIssueLabel)
m.Post("/milestone", repo.UpdateIssueMilestone, reqRepoWriter) m.Post("/milestone", reqRepoWriter, repo.UpdateIssueMilestone)
m.Post("/assignee", repo.UpdateIssueAssignee, reqRepoWriter) m.Post("/assignee", reqRepoWriter, repo.UpdateIssueAssignee)
m.Post("/status", repo.UpdateIssueStatus, reqRepoWriter) m.Post("/status", reqRepoWriter, repo.UpdateIssueStatus)
}, context.CheckUnit(models.UnitTypeIssues)) })
m.Group("/comments/:id", func() { m.Group("/comments/:id", func() {
m.Post("", repo.UpdateCommentContent) m.Post("", repo.UpdateCommentContent)
m.Post("/delete", repo.DeleteComment) m.Post("/delete", repo.DeleteComment)
}, context.CheckUnit(models.UnitTypeIssues)) }, context.CheckAnyUnit(models.UnitTypeIssues, models.UnitTypePullRequests))
m.Group("/labels", func() { m.Group("/labels", func() {
m.Post("/new", bindIgnErr(auth.CreateLabelForm{}), repo.NewLabel) m.Post("/new", bindIgnErr(auth.CreateLabelForm{}), repo.NewLabel)
m.Post("/edit", bindIgnErr(auth.CreateLabelForm{}), repo.UpdateLabel) m.Post("/edit", bindIgnErr(auth.CreateLabelForm{}), repo.UpdateLabel)
m.Post("/delete", repo.DeleteLabel) m.Post("/delete", repo.DeleteLabel)
m.Post("/initialize", bindIgnErr(auth.InitializeLabelsForm{}), repo.InitializeLabels) 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.Group("/milestones", func() {
m.Combo("/new").Get(repo.NewMilestone). m.Combo("/new").Get(repo.NewMilestone).
Post(bindIgnErr(auth.CreateMilestoneForm{}), repo.NewMilestonePost) 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.Post("/:id/edit", bindIgnErr(auth.CreateMilestoneForm{}), repo.EditMilestonePost)
m.Get("/:id/:action", repo.ChangeMilestonStatus) m.Get("/:id/:action", repo.ChangeMilestonStatus)
m.Post("/delete", repo.DeleteMilestone) 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). m.Combo("/compare/*", repo.MustAllowPulls, repo.SetEditorconfigIfExists).
Get(repo.CompareAndPullRequest). Get(repo.CompareAndPullRequest).
@ -593,8 +588,8 @@ func RegisterRoutes(m *macaron.Macaron) {
m.Group("", func() { m.Group("", func() {
m.Get("/^:type(issues|pulls)$", repo.RetrieveLabels, repo.Issues) m.Get("/^:type(issues|pulls)$", repo.RetrieveLabels, repo.Issues)
m.Get("/^:type(issues|pulls)$/:index", repo.ViewIssue) m.Get("/^:type(issues|pulls)$/:index", repo.ViewIssue)
m.Get("/labels/", repo.RetrieveLabels, repo.Labels) m.Get("/labels/", context.CheckAnyUnit(models.UnitTypeIssues, models.UnitTypePullRequests), repo.RetrieveLabels, repo.Labels)
m.Get("/milestones", repo.Milestones) m.Get("/milestones", context.CheckAnyUnit(models.UnitTypeIssues, models.UnitTypePullRequests), repo.Milestones)
}, context.RepoRef()) }, context.RepoRef())
m.Group("/wiki", func() { m.Group("/wiki", func() {

10
templates/repo/issue/list.tmpl
View File

@ -103,8 +103,8 @@
</div> </div>
<div class="issue-actions"> <div class="issue-actions">
<div class="ui basic status buttons"> <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 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="{{$.Link}}/status">{{.i18n.Tr "repo.issues.action_close"}}</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>
<div class="ui secondary filter menu floated right"> <div class="ui secondary filter menu floated right">
@ -116,7 +116,7 @@
</span> </span>
<div class="menu"> <div class="menu">
{{range .Labels}} {{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}} <span class="octicon {{if eq $.SelectLabels .ID}}octicon-check{{end}}"></span><span class="label color" style="background-color: {{.Color}}"></span> {{.Name | Sanitize}}
</div> </div>
{{end}} {{end}}
@ -134,7 +134,7 @@
{{.i18n.Tr "repo.issues.action_milestone_no_select"}} {{.i18n.Tr "repo.issues.action_milestone_no_select"}}
</div> </div>
{{range .Milestones}} {{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}} {{.Name | Sanitize}}
</div> </div>
{{end}} {{end}}
@ -152,7 +152,7 @@
{{.i18n.Tr "repo.issues.action_assignee_no_select"}} {{.i18n.Tr "repo.issues.action_assignee_no_select"}}
</div> </div>
{{range .Assignees}} {{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}} <img src="{{.RelAvatarLink}}"> {{.Name}}
</div> </div>
{{end}} {{end}}