search

brendon / acts_as_list

An ActiveRecord plugin for managing lists.
http://brendon.github.io/acts_as_list/
MIT License
2.04k stars 355 forks source link

Add quoted table names to some columns #221

Closed jpalumickas closed 8 years ago

jpalumickas commented 8 years ago

When I'm using custom acts_as_list_list with joins I have an error: column reference "id" is ambiguous. We need to specify table name in some places to prevent this error.

jpalumickas commented 8 years ago

@brendon can you take a look ?

brendon commented 8 years ago

Looks good and the tests are passing so that's good too :) My only advice would be to add a test that triggers the problem so that if someone decides to refactor that stuff in the future it won't break. Also, do we need to be so explicit on all of those queries, or is there a logical subset that are affected (e.g. methods that can be accessed via the join relationship). I'd assume some only acts on the individual model instance and wouldn't invoke the join?

Lastly, it's probably be an idea to DRY up the quoted stuff as another method. quoted_position_column_with_table_name? or something like that :D

jpalumickas commented 8 years ago

Yep, we need to add probably everywhere in case (like mine) someone will overwrite some gem methods. Agree with adding another method, but this I think is too long, because it's not reduce anything for the current situation.

brendon commented 8 years ago

Oh yes I agree, that was just to help explain what the method would be for :) It'd be great if there was an AR method that one could pass the column name and it returned a full quoted and table-namespaces column identifier.

jpalumickas commented 8 years ago

Added a quoted_position_column_with_table_name method 😉

brendon commented 8 years ago

Haha, thanks man. It's a terrible method name. :D One to refactor one day I guess :)

jpalumickas commented 8 years ago

Updated, can you merge if it's ok and make a release ? 😉

brendon commented 8 years ago

Hi @jpalumickas, thanks for that. I've merged this and released 0.7.7 for you :)

Have a great day!

jpalumickas commented 8 years ago

Thanks!