From 9011894a2975d9d112dc3db453739e13261c0716 Mon Sep 17 00:00:00 2001
From: kolaente <k@knt.li>
Date: Mon, 3 Apr 2023 11:46:08 +0200
Subject: [PATCH] feat: check for cycles when creating or updating a project's
 parent

---
 pkg/models/error.go   | 37 +++++++++++++++++++++++++++++++++++++
 pkg/models/project.go | 28 ++++++++++++++++++++++++++--
 2 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/pkg/models/error.go b/pkg/models/error.go
index 6a80bc811..425f02a55 100644
--- a/pkg/models/error.go
+++ b/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
 // ==============
diff --git a/pkg/models/project.go b/pkg/models/project.go
index d2b14f2fe..2c6482426 100644
--- a/pkg/models/project.go
+++ b/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