3
\$\begingroup\$

I have this code that creates (or updates) a user from an omniauth hash parameter:

  def self.find_or_create_from_auth_hash(auth_hash)
    uid = auth_hash['uid']
    provider = auth_hash['provider']

    user = find_or_create_by(uid: uid, provider: provider)

    user.name = auth_hash['info']['name'] || ''
    user.email = auth_hash['info']['email']
    user.nickname = auth_hash['info']['nickname']
    user.avatar = auth_hash['info']['image'] || ''

    user.access_token = auth_hash['credentials']['token']

    user.location = auth_hash['extra']['raw_info']['location'] || ''
    user.company = auth_hash['extra']['raw_info']['company'] || ''
    user.member_since = auth_hash['extra']['raw_info']['created_at'] || ''

    user if user.save
  end

It is working, but it looks unpleasant. What can be changed in this piece of code, making it easier to read and/or beautiful?

\$\endgroup\$
  • 1
    \$\begingroup\$ Welcome to CodeReiview! Your question is on topic, but state only the code purpose in the title. \$\endgroup\$ – Caridorc Jun 21 '15 at 14:30
  • \$\begingroup\$ what do you recommend? \$\endgroup\$ – Luiz K. Jun 21 '15 at 14:32
  • \$\begingroup\$ Creating user profile from omniauth hash \$\endgroup\$ – Caridorc Jun 21 '15 at 14:33
6
\$\begingroup\$

The "default to empty string" bit should probably be done in the model. I.e. in a before_validation callback or similar.

You can also use Hash#slice to pluck out certain keys:

user = find_or_create_by(auth_hash.slice(%w(uid provider)))

user.assign_attributes(auth_hash['info'].slice(%w(name email nickname))
user.avatar = auth_hash['info']['image']

user.access_token = auth_hash['credentials']['token']

extra = auth_hash['extra']['raw_info']

user.assign_attributes(extra['raw_info'].slice(%w(location company)))
user.member_since = extra['raw_info']['created_at']

Alternatively, you could rewrite the keys (e.g. in a loop or using transform_keys) before using assign_attributes:

translations = { # could be a constant somewhere
  'image' => 'avatar',
  'token' => 'access_token',
  'created_at' => 'member_since'
}

attributes = auth_hash['info'].slice(%w(name email nickname image))
attributes.merge!(auth_hash['extra']['raw_info'].slice(%w(location company created_at)))
attributes['token'] = auth_hash['credentials']['token']

attributes.transform_keys! { |key| translations[key] || key }

user.assign_attributes(attributes)
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ that is pretty awesome! \$\endgroup\$ – Luiz K. Jun 21 '15 at 21:04
3
\$\begingroup\$

Perhaps use assign_attributes? Or use another implementation / call send yourself.

Otherwise, if you provide access to the attributes of user via [] the assignments could be done in a loop.

That said, you can at least remove the duplicated parts:

def self.find_or_create_from_auth_hash(auth_hash)
  uid = auth_hash['uid']
  provider = auth_hash['provider']

  user = find_or_create_by(uid: uid, provider: provider)

  info = auth_hash['info']

  user.name = info['name'] || ''
  user.email = info['email']
  user.nickname = info['nickname']
  user.avatar = info['image'] || ''

  user.access_token = auth_hash['credentials']['token']

  extra = auth_hash['extra']['raw_info']

  user.location = raw_info['location'] || ''
  user.company = raw_info['company'] || ''
  user.member_since = raw_info['created_at'] || ''

  user if user.save
end
| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.