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

Fixed /admin/users #95

Merged
merged 5 commits into from
May 12, 2014
Merged

Fixed /admin/users #95

merged 5 commits into from
May 12, 2014

Conversation

alter
Copy link
Contributor

@alter alter commented May 4, 2014

ArgumentError in Admin::UsersController#index
wrong number of arguments (1 for 0)

Rails 4 has depricated "all" method, now it's default behaviour

POC: http://stackoverflow.com/questions/18203712/rails-4-relationall-deprecation

ArgumentError in Admin::UsersController#index
wrong number of arguments (1 for 0)

Rails 4 has depricated "all" method, now it's default behaviour
end
people.email AS email").joins(:person).load.order("people.last_name ASC")

end

Choose a reason for hiding this comment

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

end at 11, 1 is not aligned with def at 4, 2

people.last_name AS last_name,
people.first_name AS first_name,
people.public_name AS public_name,
people.email AS email")
people.email AS email').
joins(:person).load.order('people.last_name ASC')

Choose a reason for hiding this comment

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

Line is too long. [83/80]

@kalabiyau
Copy link
Member

I would go even further - because rails does not require you to select columns - the whole select can be like

@users = User.joins(:person).order('people.last_name ASC')

and to User model user.rb after line 18

delegate :last_name, :first_name, :public_name, :to => :person

@@ -17,6 +17,8 @@ class User < ActiveRecord::Base
before_save :setup_role
before_create :create_person

delegate :last_name, :first_name, :public_name, :email, to: :person
Copy link
Member

Choose a reason for hiding this comment

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

@alter please remove :email here as it should be user's method, not delegated to person

kalabiyau added a commit that referenced this pull request May 12, 2014
@kalabiyau kalabiyau merged commit 9090cc0 into openSUSE:master May 12, 2014
cycomachead added a commit to cycomachead/osem that referenced this pull request Mar 10, 2022
….5.0

Update codecov: 0.2.11 → 0.5.0 (major)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants