diff --git a/models/action.go b/models/action.go index f8c8c713e..66fe71b77 100644 --- a/models/action.go +++ b/models/action.go @@ -470,7 +470,12 @@ func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit) err } // Check for dependencies, if there aren't any, close it - if IssueNoDependenciesLeft(issue) { + noDeps, err := IssueNoDependenciesLeft(issue) + if err != nil { + return err + } + + if noDeps { if err = issue.ChangeStatus(doer, repo, true); err != nil { return err } diff --git a/models/issue_dependency.go b/models/issue_dependency.go index 5237803c8..7a475f517 100644 --- a/models/issue_dependency.go +++ b/models/issue_dependency.go @@ -52,16 +52,12 @@ func CreateIssueDependency(user *User, issue, dep *Issue) (exists bool, err erro } // Add comment referencing the new dependency - _, err = createIssueDependencyComment(sess, user, issue, dep, true) - - if err != nil { + if _, err = createIssueDependencyComment(sess, user, issue, dep, true); err != nil { return exists, err } // Create a new comment for the dependent issue - _, err = createIssueDependencyComment(sess, user, dep, issue, true) - - if err != nil { + if _, err = createIssueDependencyComment(sess, user, dep, issue, true); err != nil { return exists, err } } @@ -73,8 +69,8 @@ func RemoveIssueDependency(user *User, issue *Issue, dep *Issue, depType Depende sess := x.NewSession() // Check if it exists - exists, err := issueDepExists(x, issue.ID, dep.ID) - if err != nil { + var exists bool + if exists, err = issueDepExists(x, issue.ID, dep.ID); err != nil { return err } @@ -92,22 +88,17 @@ func RemoveIssueDependency(user *User, issue *Issue, dep *Issue, depType Depende return } - _, err := x.Delete(&issueDepToDelete) - if err != nil { + if _, err := x.Delete(&issueDepToDelete); err != nil { return err } // Add comment referencing the removed dependency - _, err = createIssueDependencyComment(sess, user, issue, dep, false) - - if err != nil { + if _, err = createIssueDependencyComment(sess, user, issue, dep, false); err != nil { return err } // Create a new comment for the dependent issue - _, err = createIssueDependencyComment(sess, user, dep, issue, false) - - if err != nil { + if _, err = createIssueDependencyComment(sess, user, dep, issue, false); err != nil { return err } } @@ -119,11 +110,7 @@ func issueDepExists(e Engine, issueID int64, depID int64) (exists bool, err erro exists, err = e.Where("(issue_id = ? AND dependency_id = ?) OR (issue_id = ? AND dependency_id = ?)", issueID, depID, depID, issueID).Exist(&IssueDependency{}) - if err != nil { - return exists, err - } - - return exists, nil + return } // IssueDependencyIssue custom type for mysql join @@ -138,7 +125,7 @@ func (IssueDependencyIssue) TableName() string { } // IssueNoDependenciesLeft checks if issue can be closed -func IssueNoDependenciesLeft(issue *Issue) bool { +func IssueNoDependenciesLeft(issue *Issue) (bool, error) { exists, err := x. Join("INNER", "issue", "issue.id = issue_dependency.dependency_id"). @@ -146,9 +133,5 @@ func IssueNoDependenciesLeft(issue *Issue) bool { And("issue.is_closed = ?", "0"). Exist(&IssueDependencyIssue{}) - if err != nil { - return false - } - - return !exists + return !exists, err } diff --git a/models/migrations/v49.go b/models/migrations/v49.go index 21ed4760b..36756d319 100644 --- a/models/migrations/v49.go +++ b/models/migrations/v49.go @@ -24,9 +24,7 @@ func addIssueDependencyTables(x *xorm.Engine) (err error) { UpdatedUnix int64 `xorm:"updated"` } - err = x.Sync(new(IssueDependency)) - - if err != nil { + if err = x.Sync(new(IssueDependency)); err != nil { return fmt.Errorf("Error creating issue_dependency_table column definition: %v", err) } diff --git a/models/models.go b/models/models.go index 836a14db5..082100e6c 100644 --- a/models/models.go +++ b/models/models.go @@ -117,6 +117,7 @@ func init() { new(TrackedTime), new(DeletedBranch), new(RepoIndexerStatus), + new(IssueDependency), ) gonicNames := []string{"SSL", "UID"} diff --git a/models/repo.go b/models/repo.go index 1f8f1541e..de6256937 100644 --- a/models/repo.go +++ b/models/repo.go @@ -2457,8 +2457,7 @@ func (repo *Repository) GetUserFork(userID int64) (*Repository, error) { func (repo *Repository) getBlockedByDependencies(e Engine, issueID int64) (_ []*IssueDependencyIssue, err error) { var issueDeps []*IssueDependencyIssue - err = x.Join("INNER", "issue", "issue.id = issue_dependency.dependency_id").Where("issue_id = ?", issueID).Find(&issueDeps) - if err != nil { + if err = x.Join("INNER", "issue", "issue.id = issue_dependency.dependency_id").Where("issue_id = ?", issueID).Find(&issueDeps); err != nil { return issueDeps, err } @@ -2466,11 +2465,10 @@ func (repo *Repository) getBlockedByDependencies(e Engine, issueID int64) (_ []* } // Get Blocking Dependencies, aka all issues this issue blocks. -func (repo *Repository) getBlockingDependencies(e Engine, issueID int64) (_ []*IssueDependencyIssue, err error) { +func (repo *Repository) getBlockingDependencies(e Engine, issueID int64) ([]*IssueDependencyIssue, error) { var issueDeps []*IssueDependencyIssue - err = x.Join("INNER", "issue", "issue.id = issue_dependency.issue_id").Where("dependency_id = ?", issueID).Find(&issueDeps) - if err != nil { + if err := x.Join("INNER", "issue", "issue.id = issue_dependency.issue_id").Where("dependency_id = ?", issueID).Find(&issueDeps); err != nil { return issueDeps, err } diff --git a/routers/repo/issue.go b/routers/repo/issue.go index 441e039dc..0f164cfa4 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -937,7 +937,12 @@ func NewComment(ctx *context.Context, form auth.CreateCommentForm) { !(issue.IsPull && issue.PullRequest.HasMerged) { // Check for open dependencies - if form.Status == "close" && !models.IssueNoDependenciesLeft(issue) { + noDeps, err := models.IssueNoDependenciesLeft(issue) + if err != nil { + return + } + + if form.Status == "close" && !noDeps { if issue.IsPull { ctx.Flash.Error("You need to close all issues blocking this pull request before you can merge it!") ctx.Redirect(fmt.Sprintf("%s/pulls/%d", ctx.Repo.RepoLink, issue.Index)) diff --git a/routers/repo/issue_dependency_add.go b/routers/repo/issue_dependency_add.go index 394a013db..c113bdbcc 100644 --- a/routers/repo/issue_dependency_add.go +++ b/routers/repo/issue_dependency_add.go @@ -16,8 +16,6 @@ import ( // AddDependency adds new dependencies func AddDependency(c *context.Context) { - // TODO: should should an issue only have dependencies in it's own repo? - depID, err := strconv.ParseInt(c.Req.PostForm.Get("newDependency"), 10, 64) if err != nil { c.Handle(http.StatusBadRequest, "issue ID is not int", err) diff --git a/routers/repo/issue_dependency_remove.go b/routers/repo/issue_dependency_remove.go index 6b29fc5a9..475c07f48 100644 --- a/routers/repo/issue_dependency_remove.go +++ b/routers/repo/issue_dependency_remove.go @@ -50,8 +50,7 @@ func RemoveDependency(c *context.Context) { return } - err = models.RemoveIssueDependency(c.User, issue, dep, depType) - if err != nil { + if err = models.RemoveIssueDependency(c.User, issue, dep, depType); err != nil { c.Handle(http.StatusInternalServerError, "CreateOrUpdateIssueDependency", err) return } diff --git a/routers/repo/pull.go b/routers/repo/pull.go index 280c5eff1..0cdd7ff2a 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -495,7 +495,12 @@ func MergePullRequest(ctx *context.Context) { pr.Issue = issue pr.Issue.Repo = ctx.Repo.Repository - if !models.IssueNoDependenciesLeft(issue) { + noDeps, err := models.IssueNoDependenciesLeft(issue) + if err != nil { + return + } + + if !noDeps { ctx.Flash.Error("You need to close all issues blocking this pull request before you can merge it!") ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index)) return diff --git a/templates/repo/issue/view_content/sidebar.tmpl b/templates/repo/issue/view_content/sidebar.tmpl index 03bcdcf7e..92717c7dc 100644 --- a/templates/repo/issue/view_content/sidebar.tmpl +++ b/templates/repo/issue/view_content/sidebar.tmpl @@ -193,97 +193,97 @@ {{if .IsSigned}} {{if .IssueDependenciesEnabled}} -
+
-
- {{.i18n.Tr "repo.issues.dependency.title"}} -
- {{if .BlockedByDependencies}} - - {{.i18n.Tr "repo.issues.dependency.blocked_by_short"}}: - -
- {{range .BlockedByDependencies}} -
-
- - - - {{if .IsClosed}} -
- +
+ {{.i18n.Tr "repo.issues.dependency.title"}} +
+ {{if .BlockedByDependencies}} + + {{.i18n.Tr "repo.issues.dependency.blocked_by_short"}}: + +
+ {{range .BlockedByDependencies}} +
+
+ + + + {{if .IsClosed}} +
+ +
+ {{else}} +
+ +
+ {{end}} +
+
#{{.Index}}
+ {{.Title}} +
+ {{end}}
- {{else}} -
- -
- {{end}} -
-
#{{.Index}}
- {{.Title}} -
- {{end}} -
- {{end}} + {{end}} - {{if .BlockingDependencies}} - - {{.i18n.Tr "repo.issues.dependency.blocks_short"}}: - -
- {{range .BlockingDependencies}} -
-
- - - - {{if .IsClosed}} -
- + {{if .BlockingDependencies}} + + {{.i18n.Tr "repo.issues.dependency.blocks_short"}}: + +
+ {{range .BlockingDependencies}} +
+
+ + + + {{if .IsClosed}} +
+ +
+ {{else}} +
+ +
+ {{end}} +
+
#{{.Index}}
+ {{.Title}} +
+ {{end}}
- {{else}} -
- -
- {{end}} -
-
#{{.Index}}
- {{.Title}} -
- {{end}} -
- {{end}} + {{end}} - {{if (and (not .BlockedByDependencies) (not .BlockingDependencies))}} -

{{.i18n.Tr "repo.issues.dependency.no_dependencies"}}

- {{end}} -
-
- {{$.CsrfTokenHtml}} -
-
- - -
{{.i18n.Tr "repo.issues.dependency.add"}}
-
+ {{if (and (not .BlockedByDependencies) (not .BlockingDependencies))}} +

{{.i18n.Tr "repo.issues.dependency.no_dependencies"}}

+ {{end}} +
+ + {{$.CsrfTokenHtml}} +
+
+ + +
{{.i18n.Tr "repo.issues.dependency.add"}}
+
+
+
+
-
- +
- -
-
+