intercom
Handle hyphens in `locale` when fetching Articles with `translated_content`
Certain locales include hyphens, which causes trouble (i.e. a SyntaxError) when dynamically defining method names.
This change converts the hyphen to an underscore for safety.
Why?
Without this, calling e.g. intercom_client.articles.all.to_a will raise a SyntaxError if the articles have translated content, as it tries to define a method with a hyphen for certain locales (e.g. pt-BR).
How?
The update checks for - in attributes and, if present, replaces with _ before defining accessors and using the setter.
I experimented with pushing this lower (into the DynamicAccessors) but the ApiResource#initialize_property immediately calls set_property using the same attribute name, so we'd then end up having to do this sanitization in multiple places which felt weird.
Other thoughts
When writing the specs I was tempted to add a new directory, e.g. spec/fixtures/articles/, and store the sample article JSON there vs. pasting it directly in spec_helper. For simplicity/consistency I mimicked the current approach but wanted to share my instinct in case useful input 😅.
I also considered trimming the test article hash down to just a couple representative locales to keep it shorter (i.e. a local without a hyphen and a locale with a hyphen), but I thought it better to keep the representation as a mirror of what is actually returned.
Regarding the spec failure:
/usr/local/bundle/gems/mocha-1.16.1/lib/mocha/integration/mini_test/adapter.rb:26:in `included': uninitialized constant MiniTest (NameError)
It looks like mocha < 2.1.x is not compatible with minitest >= 5.19.x: https://github.com/freerange/mocha/issues/614
This is fixed in mocha 2.1.0.
So there needs to be a change to the gemspec to constrain one (or both) of these dependencies to ensure compatibility. If the decision is to pin minitest < 5.19, note that you'll encounter another compatibility problem, this time with the m gem, where m < 1.6.1 is incompatible with minitest >= 5.16.x: https://github.com/qrush/m/issues/93
This is fixed in [email protected] as mentioned here.
For example, here's what I did locally to get specs working: