stefankroes / ancestry

Organise ActiveRecord model into a tree structure
MIT License
3.71k stars 458 forks source link

Defining a maximal depth #540

Open edouard-per-angusta opened 3 years ago

edouard-per-angusta commented 3 years ago

Hi! I try to validate a maximal depth, but it seems that Rails validations are skipped when cached depth is updated after a parent has been updated.

For example, given I want this maximal depth to be 1:

Node.has_ancestry(cache_depth: true)
Node.validates(:ancestry_depth, numericality: { less_than_or_equal_to: 1 })

a = Node.create!            # ancestry_depth == 0
b = Node.create!(parent: a) # ancestry_depth == 1
c = Node.create!(parent: b) # error, as expected

x = Node.create!            # ancestry_depth == 0
a.update!(parent: x)        # it works, but then b.ancestry_depth == 2, which is not allowed

For now I've added a validation on update like:

def validate_descendants_ancestry_depth
  return unless ancestry_changed?
  return if descendants.maximum(:ancestry_depth) + ancestry_depth - ancestry_depth_was <= 1

  errors.add(...)
end

But I was wondering if there was a built-in system for that? Like something checking if there is any other ActiveModel::Validations::NumericalityValidator on the ancestry_depth attribute?

Thanks! :)

kbrock commented 3 years ago

wonderful edge case.

there is no way to determine that you have kids (without a query) I guess there is a way to know that you are a new node - so we could skip the check in that instance

Could also only add this validation in has_ancestry when the depth validation is added.

This just feels like an expensive check.

Curious, are you running this on postgres or mysql? Probably doesn't make a difference, but we approach them slightly differently.