Add columns managed by Globalize gem

[peterfication]

Globalize hooks into the model and removes the translated columns
from the klass.columns. This commit checks if globalize is
hooked into the model and adds the necessary columns to the
annotation array.

[peterfication]

@ctran did you have to chance so far to have a look at it?

[dvdoliveira]

Hey @ctran any chance that this PR could be reviewed and included in the next release?
I'm also using annotate_models with the globalize gem and noticed the same issue reported by @peterfication here.

[peterfication]

@drwl you seem to be actively working on this project. Can you have a look at the idea of this PR? I will rebase or restructure the code but only if there is a chance that it gets merged.

[ctran]

Yes, we'll review and merge it.

[peterfication]

I rebase and restructured my added code a little bit. I'm still missing tests so this will be the next step. Is it the right approach to mock all methods from the globalize gem that are being used by my code?

[peterfication]

And I'm wondering whether this should be added to the Rubocop config:

Metrics/BlockLength:
  Exclude:
    - 'spec/**/*.rb'

wdyt?

[ctran]

I'm ok with that.

…

On Tue, Dec 17, 2019, 23:48 Peter Gundel ***@***.***> wrote: And I'm wondering whether this should be added to the Rubocop config: Metrics/BlockLength: Exclude: - 'spec/**/*.rb' wdyt? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#602?email_source=notifications&email_token=AAAAD25AOFCDIPPA3DNPLSLQZHITPA5CNFSM4GYEX57KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHFHUNA#issuecomment-566917684>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAAD2ZQPUJQWUJZKLKLS4TQZHITPANCNFSM4GYEX57A> .

[drwl]

@peterfication thanks for the PR! Gonna merge it in

[bartoszkopinski]

Why is it including the ID column as well? How do I disable globalize annotations? It looks confusing since the columns aren't really there.

[peterfication]

(1) Which ID column do you mean? It shouldn't include the one from globalize. If this is the case for your setup, it's a bug. Can you please share the versions you use (Rails, globalize, annotate_models)?

(2) I'm very sorry for not picking a sensible default here. There should have been an option to enable it and the default should have been false. @drwl should I add such an option with the default false?

[cddso]

I noticed the same behavior with table names that consist of multiple words (e.g. MyCar). Here a .downcase is used, but this results in mycar instead of my_car.

foreign_column_name = [
        klass.translation_class.to_s
             .gsub('::Translation', '').gsub('::', '_')
             .downcase,
        '_id'
      ].join.to_sym

Globalize uses a different method to determine the foreign key.

t.references table_name.sub(/^#{table_name_prefix}/, '').singularize,

Defined here: https://github.com/globalize/globalize/blob/master/lib/globalize/active_record/migration.rb#L82

Perhaps a solution is to use a similar approach and use the table name of the class instead of the table name of the translation class. Alternative might be to use .underscore over .downcase but this can potentially also result in incorrect names where capital letters are not necessarily a breakpoint (ABChart as model could have a_b_chart or ab_chart as table name)