Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Authorization endpoints to API which lists accounts. #101

Merged
merged 3 commits into from
May 1, 2015

Conversation

cqr
Copy link
Contributor

@cqr cqr commented Apr 22, 2015

resolves #99

end

included do
extend ActiveModel::Naming unless (method(:model_name) rescue nil)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid using rescue in its modifier form.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 98.11% when pulling 46754d3 on feat/authorization into a2eb026 on master.


class Api::AuthorizationsController < Api::BaseController
def resource
Authorization.new(User.find(prx_auth_token.user_id))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might DRY this out a bit and put User.find(prx_auth_token.user_id) into a find or get user method.

@kookster
Copy link
Member

@chrisrhoden I had only minor feedback on this, LGTM.
If you want to DRY up that User.find method, fine, if not, fine - I'll merge as is.

# consider if there is a better way to do this - decorate model instead?
attr_accessor :is_root_resource

def is_root_resource

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename is_root_resource to root_resource?.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 98.12% when pulling e0705a8 on feat/authorization into a2eb026 on master.

# encoding: utf-8

class Api::AuthorizationsController < Api::BaseController
private

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep a blank line before and after private.

@cqr
Copy link
Contributor Author

cqr commented May 1, 2015

@kookster let me know if I missed something

@coveralls
Copy link

Coverage Status

Coverage increased (+0.06%) to 98.12% when pulling 29e268a on feat/authorization into a2eb026 on master.

kookster added a commit that referenced this pull request May 1, 2015
Add Authorization endpoints to API which lists accounts.
@kookster kookster merged commit ad455ba into master May 1, 2015
@kookster kookster deleted the feat/authorization branch May 1, 2015 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide list of accounts for an autorized session
4 participants