Is there a better way to do this? I'm kinda ending up having a lot of if statements.
if params[:custom_url] || params[:user_id]
if !params[:user_id].blank?
@user = User.find(params[:user_id]).first
@url = @user.custom_url
else
@url = params[:custom_url]
@user = User.where(:custom_url => @url).first
end
end
I typically chain scopes together like this:
Model:
def self.by_user_and_url(user_id, custom_url)
by_user(user_id).by_custom_url(custom_url)
end
def self.by_user(user_id)
user_id ? where(user_id: user_id) : scoped
end
def self.by_custom_url(custom_url)
custom_url ? where(custom_url: custom_url) : scoped
end
Controller:
if params[:custom_url].present? || params[:user_id].present?
@user = User.by_user_and_url(params[:user_id], params[:custom_url]).first
end
I'm not sure why you had two instance variables. The url can be pulled from the @user instance variable if you need it in your view.
Not a big fan of case..when:
@user = User.find_first(params[:user_id]) if params[:user_id]
@user ||= User.find_by_custom_url(@url)
@url = params[:custom_url] || @user.custom_url
if params[:user_id].present?
\$\endgroup\$
!params[:user_id].blank?, you can writeparams[:user_id].present?which reads more nicely. \$\endgroup\$