I am working on a personal project and I have hit a wall. I know I am writing bad code and I really want to refactor the code below. The application has three tables on the same page. Each table contains data from a has-many-though relationship. In essence I have an employees page which contains three tables of employee licenses that all expire in grouped intervals:
ALL EMPLOYEE PAGE
- Employees with licenses Expiring in 30 days
- Employees with licenses Expiring in 30-90 days
- Employees with licenses Expiring in 90 days
All three of these tables are independently paginated and I am allowing the user to enter a search term and search across all three tables.
However I have over 1200 licenses so the page is taking forever to load. How can I optimize this functionality? Any help would be greatly appreciated.
Model
def self.emp_lic_small
self.all.map{|se| se.employee_licenses.less_than_thirty}.flatten
end
def self.emp_lic_medium
self.all.map{|se| se.employee_licenses.between_thirty_and_ninty}.flatten
end
def self.emp_lic_large
self.all.map{|se| se.employee_licenses.greater_than_ninty}.flatten
end
Controller
@small_employee_licenses = SiteEmployee.search(params[:search]).emp_lic_small.paginate(:page => params[:small_lic], :per_page => 20)
@medium_employee_licenses = SiteEmployee.search(params[:search]).emp_lic_medium.paginate(:page => params[:med_lic], :per_page => 20)
@large_employee_licenses = SiteEmployee.search(params[:search]).emp_lic_large.paginate(:page => params[:large_lic], :per_page => 20)
View
<div class="panel panel-danger">
<div class="panel-heading"><strong>Employee Licenses Expiring in Less Than 30 Days</strong></div>
<table class="table">
<thead>
<th class="text-center">Employee Name</th>
<th class="text-center">Employed By</th>
<th class="text-center">License Name</th>
<th class="text-center">Expiration Date</th>
<th class="text-center">Obtained?</th>
</tr>
</thead>
<tbody>
<% if @small_employee_licenses.present? %>
<% @small_employee_licenses.each do |e| %>
<tr>
<td class="text-center"><%= link_to e.site_employee.to_s, site_employee_path(e.site_employee)%></td>
<td class="text-center"><%= link_to e.site_employee.site.name, site_path(e.site_employee.site)%></td>
<td class="text-center"><%= e.license.name %></td>
<td class="text-center"><%= e.expiration_date.strftime("%m/%d/%Y") %></td>
<td class="text-center"><%= e.obtained? ? "Yes" : "No" %></td>
</tr>
<%end%>
<% else %>
<tr><td colspan="3">There are currently no Licenses due in the next 30 days.</td></tr>
<% end %>
</tbody>
</table>
</div>
<%= will_paginate @small_employee_licenses, param_name:'small_lic' unless @small_employee_licenses.blank? %>
<div class="panel panel-warning">
<div class="panel-heading"><strong>Employee Licenses Expiring in 30-90 Days</strong></div>
<table class="table">
<thead>
<th class="text-center">Employee Name</th>
<th class="text-center">Employed By</th>
<th class="text-center">License Name</th>
<th class="text-center">Expiration Date</th>
<th class="text-center">Obtained?</th>
</tr>
</thead>
<tbody>
<% if @medium_employee_licenses.present? %>
<% @medium_employee_licenses.each do |e| %>
<tr>
<td class="text-center"><%= link_to e.site_employee.to_s, site_employee_path(e.site_employee)%></td>
<td class="text-center"><%= link_to e.site_employee.site.name, site_path(e.site_employee.site)%></td>
<td class="text-center"><%= e.license.name %></td>
<td class="text-center"><%= e.expiration_date.strftime("%m/%d/%Y") %></td>
<td class="text-center"><%= e.obtained? ? "Yes" : "No" %></td>
</tr>
<%end%>
<% else %>
<tr><td colspan="3">There are currently no Licenses due in the next 30 days.</td></tr>
<% end %>
</tbody>
</table>
</div>
<%= will_paginate @medium_employee_licenses, param_name:'med_lic' unless @medium_employee_licenses.blank? %>
<div class="panel panel-success">
<div class="panel-heading"><strong>Employee Licenses Expiring in 30-90 Days</strong></div>
<table class="table">
<thead>
<th class="text-center">Employee Name</th>
<th class="text-center">Employed By</th>
<th class="text-center">License Name</th>
<th class="text-center">Expiration Date</th>
<th class="text-center">Obtained?</th>
</tr>
</thead>
<tbody>
<% if @large_employee_licenses.present? %>
<% @large_employee_licenses.each do |e| %>
<tr>
<td class="text-center"><%= link_to e.site_employee.to_s, site_employee_path(e.site_employee)%></td>
<td class="text-center"><%= link_to e.site_employee.site.name, site_path(e.site_employee.site)%></td>
<td class="text-center"><%= e.license.name %></td>
<td class="text-center"><%= e.expiration_date.strftime("%m/%d/%Y") %></td>
<td class="text-center"><%= e.obtained? ? "Yes" : "No" %></td>
</tr>
<%end%>
<% else %>
<tr><td colspan="3">There are currently no Licenses due in the next 30 days.</td></tr>
<% end %>
</tbody>
</table>
</div>
<%= will_paginate @large_employee_licenses, param_name:'large_lic' unless @large_employee_licenses.blank? %>
all
records. I can't tell from the code exactly what you're scoping that to – what model is this from? And what'sse
? Are the lists scoped to something before searching? – but you're looking for licenses, but seem to be going the long way of finding licenses through something. It also implies that the pagination happens late - after a lot of stuff has been loaded. So the pagination is the act of throwing a lot of it away, rather than loading what you need. But your question doesn't have enough detail, so I'm not sure. \$\endgroup\$has_many
, etc, for the tables in the example?) \$\endgroup\$