4
\$\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\$
3
  • 1
    \$\begingroup\$ Welcome to CodeReiview! Your question is on topic, but state only the code purpose in the title. \$\endgroup\$ Commented Jun 21, 2015 at 14:30
  • \$\begingroup\$ what do you recommend? \$\endgroup\$ Commented Jun 21, 2015 at 14:32
  • \$\begingroup\$ Creating user profile from omniauth hash \$\endgroup\$ Commented Jun 21, 2015 at 14:33

2 Answers 2

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)
\$\endgroup\$
1
  • \$\begingroup\$ that is pretty awesome! \$\endgroup\$ Commented Jun 21, 2015 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
\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.