stefankroes / ancestry

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

STI breaks arrange scoping #676

Open wooly opened 11 months ago

wooly commented 11 months ago

I've found what I think is a bug with the arrange method when used with a chained scope in an STI environment (a bit of an obscure one, I realise!).

Say we have a Node with a sub-class of Fruit. An additional class of Group is also added to exhibit the error. A Group has a has_many association to both nodes and fruits.

When you call arrange with the base class of node, the query to fetch the nodes is correct. However, when you call arrange with the sub-class of node, the query to fetch the nodes fetches everything and filters in-memory.

I've made an example rails app exhibiting the behaviour here: https://github.com/wooly/ancestry_arrange_error/tree/main

If you create the db with rails db:create then run rails runner scripts/strange.rb, you'll see the issue.

# Calling Group.first.fruits.arrange is missing WHERE type='Fruit' AND group_id='x'
D, [2023-09-15T08:20:43.638659 #66825] DEBUG -- :   Node Load (0.1ms)  SELECT "nodes".* FROM "nodes"

# Calling Group.first.nodes.arrange generates the expected SQL
D, [2023-09-15T08:20:43.639383 #66825] DEBUG -- :   Node Load (0.1ms)  SELECT "nodes".* FROM "nodes" WHERE "nodes"."group_id" = ?  [["group_id", 11]]

Any ideas?

kbrock commented 9 months ago

Ancestry does ignore the base class used for the queries. And I understand how this could seem strange and it could be different from what you want and expect.

The issue is when you fetch a Fruit and ask for the path of this fruit. The parent is possibly/probably Group. If we queried just on Fruit, the parents would not come back. So we use the top level STI class.

Are there particular cases where you expect it to ignore the class used for the query and cases where you expect it to respect the class used for the query?

wooly commented 9 months ago

I feel like for our app, we would always want to respect the class, since we have a restriction on parents being of the same STI type, so the trees are always made up of unique types.

In our app we have a base Group class, with sub-classes for the different types of group (there are around 10 different types of group). These group trees are per-customer, so there will likely be a fair few records in the future, as some of these trees can contain hundreds of records and some customers are using all 10 trees.

This issue is appearing when we select the tree to display to the user. We only want to select a tree a) for a particular customer and b) for a particular sub-type and arrange that for display.

Does that give some context on what we're trying to achieve?

kbrock commented 9 months ago

thanks @wooly

All of the STI examples I've seen have a class and the children are a type in the same class list. So Node with children Folder and File. (not saying this is the only way, just saying what I've seen). So basically any node in the STI tree can be returned. And we've (even developers before I joined the project) had to go to great lengths to strip the type our of the query.

My day job does have a generic hierarchy type class but it breaks down and I am moving away from this, albeit slowly.

Why are you using STI at all? Does it just buy you not having to define 10 db tables? It sounds like the classes are all different.

#  Group.first.fruit.arrange # dropping the :type from the where clause
Fruit.where(:group_id => Group.first.id).arrange

class Group # als
  has_many :fruit, -> { where(:type => 'Fruit') }
end

And instead, it is returning every hierarchy