class LoginAttempt < ActiveRecord::Base
belongs_to :user
def self.search(params)
conditions = Hash.new()
if params[:result] and !params[:result].empty?
conditions[:result] = params[:result]
end
if params[:login] and !params[:login].empty?
user = User.find_by_login(params[:login])
conditions[:user_id] = user.id
end
if params[:name] and !params[:name].empty?
user = User.find_by_name(params[:name])
conditions[:user_id] = user.id
end
if params[:email] and !params[:email].empty?
user = User.find_by_email(params[:email])
conditions[:user_id] = user.id
end
if params[:start_date] and !params[:start_date].empty?
conditions['created_at > ?'] = params[:start_date]
end
if params[:end_date] and !params[:end_date].empty?
conditions['creted_at < ?'] = params[:end_date]
end
find(:all, :conditions => conditions)
end
end
Refactorings
No refactoring yet !
s
August 1, 2009, August 01, 2009 09:07, permalink
No rating.
Login to rate!
I've refactored it, but I suspect that the entire approach this is a part of may be "wrong".
class LoginAttempt < ActiveRecord::Base
belongs_to :user
def self.search(params, conditions = {})
conditions[:result] = params[:result] unless params[:result].nil? || params[:result].empty?
conditions[:user_id] = find_user(params).id
conditions['created_at > ?'] = params[:start_date] unless params[:start_date].nil? || params[:start_date].empty?
conditions['created_at < ?'] = params[:end_date] unless params[:end_date].nil? || params[:end_date].empty?
find(:all, :conditions => conditions)
end
private
# really this method belongs as User.find_by_params or something like that
def self.find_user(params)
# really the .nil? and .empty? checks belong in the User.find_by_* methods
user = User.find_by_login(params[:login]) unless params[:login].nil? || params[:login].empty?
user ||= User.find_by_name(params[:name]) unless params[:name].nil? || params[:name].empty?
user ||= User.find_by_email(params[:email]) unless params[:email].nil? || params[:email].empty?
user
end
end
jose cortinas
August 1, 2009, August 01, 2009 17:18, permalink
1 rating.
Login to rate!
You need to look into the searchlogic gem, all of this could be turned into one line and a search form.
philrosenstein.myopenid.com
August 2, 2009, August 02, 2009 02:02, permalink
No rating.
Login to rate!
searchlogic is just what the doctor ordered! Now the model is empty except for the belongs_to association.
Here is what the controller and view look like so far.
class LoginAttemptsController < ApplicationController
before_filter :login_required
require_role :admin
def index
@search = LoginAttempt.search(params[:search])
@login_attempts = @search.all(:include => :user).paginate :page => params[:page]
@login_attempts_count = @search.count
end
end
<h2>Login History</h2>
<% form_for @search do |f| %>
<fieldset>
<legend>Search Users</legend>
<div class="field">
Login<br />
<%= f.text_field :user_login_like, :size => 20 %>
</div>
<div class="field">
IP<br />
<%= f.text_field :remote_ip_like, :size => 20 %>
</div>
<div class="field">
Result<br />
<%= f.text_field :result_equals, :size => 20 %>
</div>
<div class="field">
<br />
<%= f.submit "Search" %>
</div>
</fieldset>
<% end %>
<%= @login_attempts_count %> login attempts found
<br />
<br />
<table>
<tr>
<th><%= order @search, :by => :login, :as => "Login" %></th>
<th><%= order @search, :by => :remote_ip, :as => "IP" %></th>
<th><%= order @search, :by => :result, :as => "Result" %></th>
</tr>
<% @login_attempts.each do |login_attempt| %>
<tr>
<td><%= login_attempt.user ? login_attempt.user.login : "-" %></td>
<td><%= login_attempt.remote_ip %></td>
<td><%= login_attempt.result %></td>
</tr>
<% end %>
</table>
<%= will_paginate @login_attempts %>
Muke Tever
August 2, 2009, August 02, 2009 06:11, permalink
No rating.
Login to rate!
I haven't used searchlogic, so I can't answer those; this is just a sort of cleanup refactor of what you were doing to begin with ..
class LoginAttempt < ActiveRecord::Base
belongs_to :user
def self.search(params)
conditions = {
:result => params[:result],
:user_id => find_user_id_by_params(params),
'created_at > ?' => params[:start_date],
'created_at < ?' => params[:end_date]
}.delete_if {|k, v| v.blank?}
find(:all, :conditions => conditions)
end
# You still need to define behaviour for if no user is returned
# You could generalize this if the params you use are subject to variation
def find_user_id_by_params(params)
case
when params[:login].present? then User.find_by_login(params[:login]).id
when params[:name].present? then User.find_by_name(params[:name]).id
when params[:email].present? then User.find_by_email(params[:email]).id
end
end
end
Alexandre Grégoire
August 2, 2009, August 02, 2009 14:41, permalink
No rating.
Login to rate!
You should also check out Squirrel (http://www.thoughtbot.com/projects/squirrel/) from Thoughtbot.
I made these search filters for a report of user login attempts. There must be a better way to get the results matching attributes in the parent class (user). Also I know what I'm doing right now returns an error if there is no matching user record.