Use symbols instead of SQL strings for reorder (for Rails 5.2)

[jhawthorn]

Fixes #290

Passing strings to order/reorder is deprecated in Rails 5.2.

DEPRECATION WARNING: Dangerous query method (method whose arguments are used as raw SQL) called with non-attribute argument(s): "\"content_blocks\".\"position\" DESC". Non-attribute arguments will be disallowed in Rails 6.0. This method should not be called with user-provided values, such as request parameters or model attributes. Known-safe values can be passed by wrapping them in Arel.sql().

There was some concern in #290 about having issues with ambiguous column names. That shouldn't ever be an issue when using symbols (rails will quot it properly), and we already have a test for it :tada:!

[jhawthorn]

Looks like this doesn't agree with Rails 3.2, which does very silly things when passed symbols.

Do we want to continue Supporting Rails 3.2. If so, we'll probably have to add conditionals on each of these reorder calls

[brendon]

That's a hard one. Having been in a place where longer term support for older rails versions was a blessing, I'd like to keep support. But perhaps it's time to go Rails 4.0+? @swanandp, what do you think? You could use a new method instead of position_column, then put the conditional on that method to either return the symbol or the quoted string for Rails 3.2.

[Petercopter]

@jhawthorn @brendon The symbol fork works for me on Rails 5.2. The forest of deprecation warnings in the CI has been reduced drastically. 👍

[brendon]

Ha @Petercopter! Glad it's a good fix. @jhawthorn, did you want to try abstracting the position_column for reordering purposes into a method that either returns a symbol or the table name and column depending on Rails 3.2?

[brendon]

I'm happy with that now. I've merged this. We'll need to add 5.2 to the test suite and make sure everything else passes.

[Petercopter]

Fix looks good to me. Thanks for your help resolving this! 🍺 🍸 🍹

[brendon]

Yes thanks @jhawthorn for your work on this! :)