stefankroes/ancestry

STI does not work with counter cache

mattvague opened this issue ยท 12 comments

It looks like the counter cache feature of ancestry does not work when using different STI subclasses in your tree. For example, let's say you have a setup like

class Node
  has_ancestry counter_cache: true
end

class SomeNodeType < Node
end

class SomeOtherNodeType < Node
end

root = SomeNodeType.create
edge = SomeOtherNodeType.create(parent: SomeNodeType)

Calling root.children_count will return 0 where it should have returned 1. I dug into it and I believe it's due to calling self.class.increment_counter instead of self.ancestry_base_class.increment_counter here.

@kbrock Is this intentional or is this something you'd accept a PR to change?

Thanks!

My proposed fix is here: goodproblems@e17cf5e

This is great. PR would be gladly accepted. Do you need help with a spec?

Do you think

return if defined?(@_trigger_destroy_callback) && !@_trigger_destroy_callback
without the same guard in the increment is a problem as well? (Just noticed this while looking at your code)

This is great. PR would be gladly accepted. Do you need help with a spec?

Yeah that would be great actually as I was having some trouble getting ancestry suite running due to mysql gem issues on my M1 Mac. I added a test that should in theory work, but let me know what you think. PR is here: #550

Do you think ... without the same guard in the increment is a problem as well? (Just noticed this while looking at your code)

That's a good question, I'm not familiar with that part of rails or ancestry so I'd have to look into it more. Spontaneously I would say that I can't see a reason to have that on decrement but not increment. Probably best to see if you can write a failing test that replicates the issue?

@kbrock One more thing: there were a few other ideas that I wanted to discuss for ancestry but I think issues are not the right place. Any idea what it would take to get GitHub Discussions enabled?

@kbrock Sorry to bug you again, but just wondering if you had a chance to look at #550? My main thing is just not knowing if that test is good enough or even if it passes since I wasn't able to run the suite. Cheers!

sorry.

  1. please share mysql gem issues. my other mac is an m1 and I'd like to have the answer fo any issues that creep up.
  2. make sure you are installing the mysql2 driver and not mysql. I did support both up until rails 4or 5ish, and had to let it go because I couldn't get it compiled.

thanks for the ping and reminder.

@kbrock Ok! Finally figured out the magic options to get mysql2 installing:

bundle config build.mysql2 --srcdir=$(brew --prefix)/Cellar/mysql/YOUR_MYSQL_VERSION_HERE/include

(that's assuming you've already installed mysql with home-brew brew install mysql)

@mattvague take another look at the sti_support_test.rb file - I think you should more easily be able to copy those fixed examples

Gah. Now I'm having similar problems with sqlite gem ๐Ÿ™„ Anyways, will make the suggested changes and push to my PR

@mattvague just get it to work for one of them. my setup is pretty solid and I should be able to ensure it is working

thanks for all the help here