From a57bcf11034886b1ac0f4efab83cc1f36ef11c3a Mon Sep 17 00:00:00 2001
From: kolaente <konrad@kola-entertainments.de>
Date: Sun, 26 Nov 2017 18:41:23 +0100
Subject: [PATCH] Added more specific error when creating a new dependency if
 it already exists

---
 models/issue_dependency.go           | 28 +++++++++++++++++-----------
 options/locale/locale_en-US.ini      |  1 +
 routers/repo/issue_dependency_add.go |  6 +++++-
 3 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/models/issue_dependency.go b/models/issue_dependency.go
index 8cbfcbd93..5eb3e7494 100644
--- a/models/issue_dependency.go
+++ b/models/issue_dependency.go
@@ -30,17 +30,17 @@ const (
 )
 
 // CreateIssueDependency creates a new dependency for an issue
-func CreateIssueDependency(user *User, issue, dep *Issue) (exists bool, err error) {
+func CreateIssueDependency(user *User, issue, dep *Issue) (exists, circular bool, err error) {
 	sess := x.NewSession()
 
 	// Check if it aleready exists
-	exists, err = issueDepExists(x, issue.ID, dep.ID)
+	exists, circular, err = issueDepExists(x, issue.ID, dep.ID)
 	if err != nil {
-		return exists, err
+		return
 	}
 
 	// If it not exists, create it, otherwise show an error message
-	if !exists {
+	if !exists && !circular {
 		newIssueDependency := &IssueDependency{
 			UserID:       user.ID,
 			IssueID:      issue.ID,
@@ -48,20 +48,20 @@ func CreateIssueDependency(user *User, issue, dep *Issue) (exists bool, err erro
 		}
 
 		if _, err := x.Insert(newIssueDependency); err != nil {
-			return exists, err
+			return exists, circular, err
 		}
 
 		// Add comment referencing the new dependency
 		if _, err = createIssueDependencyComment(sess, user, issue, dep, true); err != nil {
-			return exists, err
+			return
 		}
 
 		// Create a new comment for the dependent issue
 		if _, err = createIssueDependencyComment(sess, user, dep, issue, true); err != nil {
-			return exists, err
+			return
 		}
 	}
-	return exists, nil
+	return exists, circular, nil
 }
 
 // RemoveIssueDependency removes a dependency from an issue
@@ -70,7 +70,7 @@ func RemoveIssueDependency(user *User, issue *Issue, dep *Issue, depType Depende
 
 	// Check if it exists
 	var exists bool
-	if exists, err = issueDepExists(x, issue.ID, dep.ID); err != nil {
+	if exists, _, err = issueDepExists(x, issue.ID, dep.ID); err != nil {
 		return err
 	}
 
@@ -106,9 +106,15 @@ func RemoveIssueDependency(user *User, issue *Issue, dep *Issue, depType Depende
 }
 
 // Check if the dependency already exists
-func issueDepExists(e Engine, issueID int64, depID int64) (exists bool, err error) {
+func issueDepExists(e Engine, issueID int64, depID int64) (exists, circular bool, err error) {
 
-	exists, err = e.Where("(issue_id = ? AND dependency_id = ?) OR (issue_id = ? AND dependency_id = ?)", issueID, depID, depID, issueID).Exist(&IssueDependency{})
+	// Check if the dependency exists
+	exists, err = e.Where("(issue_id = ? AND dependency_id = ?)", issueID, depID).Exist(&IssueDependency{})
+
+	// If not, check for circular dependencies
+	if !exists {
+		circular, err = e.Where("issue_id = ? AND dependency_id = ?", depID, issueID).Exist(&IssueDependency{})
+	}
 
 	return
 }
diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini
index 7ce995f31..af4fec4c6 100644
--- a/options/locale/locale_en-US.ini
+++ b/options/locale/locale_en-US.ini
@@ -746,6 +746,7 @@ issues.dependency.setting = Issues can have dependencies
 issues.dependency.add_error_same_issue = You cannot make an issue depend on itself!
 issues.dependency.add_error_dep_not_exist = Dependend issue does not exist!
 issues.dependency.add_error_dep_exists = Dependency already exists!
+issues.dependency.add_error_cannot_create_circular = You cannot create a dependency whith two issues blocking each other!
 issues.dependency.add_error_dep_not_same_repo = Both issues must be in the same repo!
 
 pulls.desc = Pulls management your code review and merge requests
diff --git a/routers/repo/issue_dependency_add.go b/routers/repo/issue_dependency_add.go
index c113bdbcc..ddcf53625 100644
--- a/routers/repo/issue_dependency_add.go
+++ b/routers/repo/issue_dependency_add.go
@@ -57,7 +57,7 @@ func AddDependency(c *context.Context) {
 		c.Flash.Error(c.Tr("repo.issues.dependency.add_error_same_issue"))
 	} else {
 
-		exists, err := models.CreateIssueDependency(c.User, issue, dep)
+		exists, circular, err := models.CreateIssueDependency(c.User, issue, dep)
 		if err != nil {
 			c.Handle(http.StatusInternalServerError, "CreateOrUpdateIssueDependency", err)
 			return
@@ -66,6 +66,10 @@ func AddDependency(c *context.Context) {
 		if exists {
 			c.Flash.Error(c.Tr("repo.issues.dependency.add_error_dep_exists"))
 		}
+
+		if circular {
+			c.Flash.Error(c.Tr("repo.issues.dependency.add_error_cannot_create_circular"))
+		}
 	}
 
 	url := fmt.Sprintf("%s/issues/%d", c.Repo.RepoLink, issueIndex)