feat: check for cycles when creating or updating a project's parent

pkg/models/error.go

@@ -19,6 +19,7 @@ package models
 import (
 	"fmt"
 	"net/http"
+	"strings"
 
 	"code.vikunja.io/api/pkg/config"
 	"code.vikunja.io/web"
@@ -310,6 +311,42 @@ func (err *ErrProjectCannotBeChildOfItself) HTTPError() web.HTTPError {
 	}
 }
 
+// ErrProjectCannotHaveACyclicRelationship represents an error where a project cannot have a cyclic parent relationship
+type ErrProjectCannotHaveACyclicRelationship struct {
+	ProjectID int64
+	CycleIDs  []int64
+}
+
+// IsErrProjectCannotHaveACyclicRelationship checks if an error is a project is archived error.
+func IsErrProjectCannotHaveACyclicRelationship(err error) bool {
+	_, ok := err.(*ErrProjectCannotHaveACyclicRelationship)
+	return ok
+}
+
+func (err *ErrProjectCannotHaveACyclicRelationship) CycleString() string {
+	var cycle string
+	for _, projectID := range err.CycleIDs {
+		cycle += fmt.Sprintf("%d -> ", projectID)
+	}
+	return strings.TrimSuffix(cycle, " -> ")
+}
+
+func (err *ErrProjectCannotHaveACyclicRelationship) Error() string {
+	return fmt.Sprintf("Project cannot have a cyclic relationship [ProjectID: %d]", err.ProjectID)
+}
+
+// ErrCodeProjectCannotHaveACyclicRelationship holds the unique world-error code of this error
+const ErrCodeProjectCannotHaveACyclicRelationship = 3011
+
+// HTTPError holds the http error description
+func (err *ErrProjectCannotHaveACyclicRelationship) HTTPError() web.HTTPError {
+	return web.HTTPError{
+		HTTPCode: http.StatusPreconditionFailed,
+		Code:     ErrCodeProjectCannotHaveACyclicRelationship,
+		Message:  "This project cannot have a cyclic relationship to a parent project",
+	}
+}
+
 // ==============
 // Task errors
 // ==============

pkg/models/project.go

@@ -627,7 +627,7 @@ func (p *Project) CheckIsArchived(s *xorm.Session) (err error) {
 	return nil
 }
 
-func checkProjectBeforeUpdateOrDelete(s *xorm.Session, project *Project) error {
+func checkProjectBeforeUpdateOrDelete(s *xorm.Session, project *Project) (err error) {
 	if project.ParentProjectID < 0 {
 		return &ErrProjectCannotBelongToAPseudoParentProject{ProjectID: project.ID, ParentProjectID: project.ParentProjectID}
 	}
@@ -640,10 +640,34 @@ func checkProjectBeforeUpdateOrDelete(s *xorm.Session, project *Project) error {
 			}
 		}
 
-		_, err := GetProjectSimpleByID(s, project.ParentProjectID)
+		var parent *Project
+		parent, err = GetProjectSimpleByID(s, project.ParentProjectID)
 		if err != nil {
 			return err
 		}
+
+		// Check if there's a cycle in the parent relation
+		parentsVisited := make(map[int64]bool)
+		parentsVisited[project.ID] = true
+		for {
+			if parent.ParentProjectID == 0 {
+				break
+			}
+
+			// FIXME: Can we do this with better performance?
+			parent, err = GetProjectSimpleByID(s, parent.ParentProjectID)
+			if err != nil {
+				return err
+			}
+
+			if parentsVisited[parent.ID] {
+				return &ErrProjectCannotHaveACyclicRelationship{
+					ProjectID: project.ID,
+				}
+			}
+
+			parentsVisited[parent.ID] = true
+		}
 	}
 
 	// Check if the identifier is unique and not empty