search

stefankroes / ancestry

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

Fix materialized_path2 #597

Closed kshnurov closed 1 year ago

kshnurov commented 1 year ago

materialized_path2 was completely broken from the beginning. This PR fixes it and renames strategy to ancestry_format since it's not really a strategy, it doesn't change any behavior.

kshnurov commented 1 year ago

@kbrock these parameters were previously hard-coded, which is definitely not good. Now they are used both in other modules (like the materialized_path2 or materialized_path_pg) and application code (like the provided migration).

It probably makes sense to change them to cattr_reader and make some of them private, but other than that it's just defining some variables instead of constants. What's the other option?

kshnurov commented 1 year ago

@kbrock here's a take on cattr_accessor: https://github.com/kshnurov/ancestry/tree/refactor-options-storage It has breaking changes: all writers are removed (you shouldn't change ancestry options on-the-fly, that'll lead to unexpected behavior) and all instance accessors are removed.

kshnurov commented 1 year ago

@kbrock how about releasing 4.3 on RubyGems? :)

kshnurov commented 1 year ago

@kbrock 4.2 is still the latest version on RubyGems, do you have access to release a new version?

kbrock commented 1 year ago

@kshnurov Yes, I have access.

moving to issue https://github.com/stefankroes/ancestry/issues/603