dsander/huginn — pull request #4 by cantino (2014-08-09 21:36:50+00:00)
Title: Omniauth refactors

Hey @dsander, here are some minor changes to your branch. Mostly just me thinking in code, so feel free to challenge any of them, especially allowing unknown providers.

I also have a few questions / thoughts:

1) I don't fully understand what the `.oauthable-form` class JavaScript logic is doing.

2) Your upgrade code makes sense. Would it also be good to allow backwards support in the Twitter agents like this:

```ruby
ENV['TWITTER_OAUTH_KEY'] || options['consumer_key'].presence || credential('twitter_consumer_key')
```

Or is that unnecessary? The same thing could be done in the BasecampAgent to avoid breaking on upgrade.

3) What is the use case for granting other Huginn users access to your data on a given service? I don't think I fully understand this.

4) Should we use JSONSerializedField for `serialize :options, Hash` in Service? (Maybe there's no benefit at all.)

5) Based on the presence of `@@provider_to_env_map` (now `PROVIDER_TO_ENV_MAP`) and `provider_specific_options`, perhaps we should be using subclasses of Service to handle differences? Maybe not needed yet?

6) In your upgrade migration, what if a user's Huginn instance is using multiple Twitter applications? (Seems unlikely, but it's possible.) We could check that `agents.map { |agent| twitter_consumer_key(agent) }.uniq.length == 1` and warn otherwise?