Move One Category, Watch the Tree Loop
A validation that only rejects self-parenting looks sufficient until a re-parent to your own descendant quietly forms a cycle in the category tree.
TL;DR
Moving A under B seemed fine but created a loop because B was already A's child. The old validation only blocked self-parenting, so the fix now walks up ancestors until it hits a root, a missing parent, or the original category. It uses the standard wrapped-error check and limits the walk to ten levels to handle legacy cycles safely.
Two dropdowns. Category A, parent changed to category B. Save. The form returns a validation error, and the log shows a message I had never seen the week before: changing the parent would form a category cycle. All I did was put A under B, the way you move a folder in a file manager.
The funny part: this was never a visible bug. validateCategory already checked two things that seem right: parent_id must not equal the category's own id, and the candidate parent must exist in the database. Both pass for A under B. What they don't catch: B happens to be a child of A. Once A gets parent B, the chain loops, A to B, B back to A.
A parent_id column is a linked list in disguise
A data model that only stores parent_id per row is a linked list pretending to be a tree. No database constraint stops two rows from pointing at each other. MySQL 8.0 even ships recursive CTEs with cycle avoidance to handle this at the query level [9], but this project's production database is still on 5.7, where that feature doesn't exist. So the guard has to live in the application layer.
The fix: starting from the candidate parent, walk upward through GetCategoryByID repeatedly, following each ancestor's parent_id. Two clean exits: hitting ErrCategoryNotFound means a broken chain, so the candidate parent is invalid [12]. An ancestor with no parent means we reached a legitimate root, safe. And midway, if an ancestor's id equals the category being edited, that's the cycle, rejected with ErrValidation.
ancestorID := *req.ParentID
for i := 0; i < maxCategoryDepth; i++ {
parent, err := s.repo.GetCategoryByID(ctx, ancestorID)
if errors.Is(err, ErrCategoryNotFound) {
return ErrValidation // candidate parent is not valid
}
if parent.ID == excludeID {
return ErrValidation // cycle: ancestor == the category being edited
}
if !parent.ParentID.Valid {
break // reached a legitimate root
}
ancestorID = parent.ParentID.Int64
}Notice the check uses errors.Is, not ==. Since Go 1.13, an error can wrap another error and errors.Is walks the wrap chain until it finds a match [11]. Error mapping without it can break at exactly the wrong spot.
A depth cap for data that was already broken
The easiest thing to miss: what if legacy rows already contain a cycle from before this guard existed? Walking ancestors would loop forever, and the validation meant to prevent cycles becomes one itself. That's why the walk is capped at maxCategoryDepth = 10. Way above any sane category depth, short enough to fail fast.
One test covers three cases: a cycling re-parent is rejected, self-parent stays rejected, and a legal re-parent (B stays under A) still goes through. Without the third one, it's far too easy for a new guard to block everyday operations.
Now when I move a category in the CMS and it gets stuck, I get a clear validation message instead of a vanished tree. A one-level guard always looks sufficient, until the day it isn't.
Sources