Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

STI breaks arrange scoping #676

Open
wooly opened this issue Sep 14, 2023 · 3 comments
Open

STI breaks arrange scoping #676

wooly opened this issue Sep 14, 2023 · 3 comments

Comments

@wooly
Copy link

wooly commented Sep 14, 2023

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
Copy link
Collaborator

kbrock commented Nov 6, 2023

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
Copy link
Author

wooly commented Nov 6, 2023

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
Copy link
Collaborator

kbrock commented Nov 13, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants