Fix for adding children with << operator when order_is_numeric? #85
Conversation
First try to fix broken << operator. Some tests still fail in sqlite, but fixes appear to work when manually tested in PostgreSQL. Checking in so I can clone on a CentOS box where PostgreSQL tests can be run.
…ll I narrow down the problems.
…/Rails 3.2 on all databases
|
NOTE: Travis tests for MySQL and PostgreSQL and Rails 4.1beta1 failed, but the failure was in the second execution after all tests had first passed. Failures appear to be in the code that creates the tables, not in implementation. |
|
Thanks for all this work! I'll pull into a new branch and see if I can determine what's breaking. |
|
Hey there, so did you have a chance to look at Do you think we should make I think if you're confused by the current API, that's proof that there's some work to be done here. |
| @@ -42,7 +42,7 @@ def find_or_create_at_same_time(name) | |||
| end | |||
| max_wait_time = @lock.synchronize { @wake_times.max } | |||
| sleep_time = max_wait_time - Time.now.to_f | |||
| $stderr.puts "sleeping for #{sleep_time}" | |||
| # $stderr.puts "sleeping for #{sleep_time}" | |||
There was a problem hiding this comment.
Yeah, I know it's janky, but I've fought with Travis enough with this issue to want to keep the stderr message in there.
There was a problem hiding this comment.
No worries. It just looked real odd and made test output hard to read.
|
As for your earlier question, I think the real issue was that nil was still allowed as a value for order_value, even if order_is_numeric? returns true. That certainly caused the << function to fail (and it's documented in the README as a valid way to add children). There could have been other side effects from those nil order_values. For instance, the .root method should pick the first of the available roots, but PostgreSQL in particular doesn't always return records ordered by ID by default (though MySQL seems to). This means you could get different root notes from the root method. My fix assures that if order_by_numeric? is true then the order_value column is never saved with a nil value and minimizes the number of records that must be re-written when a new record is inserted since any record prior to the inserted record doesn't need to be re-written. |
Added tests for using << operator to add children when order_is_numeric? is true. (Proved bug)
Fix for Issue #84 where the order_value was left as nil when order_is_numeric? is true.
Fix for bad SQL statements when parent_id is nil (tree roots). Caused ordering problems for roots because root nodes wouldn't be matched by reorder_with_parent_id for PostgreSQL or MySQL.