Unscope select to avoid PG::UndefinedFunction

[donv]

Having a default_scope with a select breaks since acts_as_list 0.9.5. The error type is discussed here:

rails/rails#19146

Unscoping the select scope fixes the problem.

[swanandp]

@donv Thanks for contributing. Can you please add a failing test case to demonstrate the problem?

[brendon]

Hi @donv, should we be using count(:all) instead?

[donv]

Yes, using count(:all) is probably a better solution, but it was a bigger change, so I tried for the smallest change I could see 😄 .

[brendon]

Thanks @donv, I think count(:all) is probably the best way forward. What do you think @swanandp?

If you could switch to that method and provide a failing test that would be great :D

[brendon]

Hi @donv, would you mind doing that fix with the test? :)

[donv]

Hi @brendon !

No, I don't mind 😄 . Will put it on my plan for next week or the week after. Lots of things to do 😅 .

[brendon]

Thanks @donv, we really appreciate it :)

[brendon]

@donv: ping! :D

[donv]

Thanks for the ping. My plan has slipped :) Will get to it.

[donv]

Starting on this now.

[donv]

I have added a failing test that becomes green with the current change.

[donv]

I am trying to implement the count(:all) idea we had, but it proves complex since we expect to have the position_column available in many places. Having a default scope with a select results in the position column not being present in the resulting records.

I have tried a few ways to add the position_column to the query, but then we mess up the default select of all columns.

I now think my original idea is the best, to just unscope :select.

What do you think?

[brendon]

Unscoping a default :select seems like an ok solution. As you say, it's unworkable not to have the position column selected. No damage can be done by unscoping the :select? There were side effects to unscoping a default :order which is why we now only unscope the :where.

@swanandp, did you have any feedback? :)

[brendon]

@donv, someone has proposed the unscope :select solution in a seperate pull request so I think we'll merge this one. Can you update from master just so we can get a green suite then we should be good to go :)

[donv]

@brendon Rebased and green tests.

[brendon]

Thanks @donv, I'm happy with this. I'll release a new gem for you.