Skip to content

Commit d58dfaa

Browse files
committed
improve error reports categories query on postgres
postgres can't/won't use the index for a DISTINCT query, so be a bit more explicit with it to get it to use the index test plan: * go to /error_reports * it should load fast, there should be a categories dropdown, and it should have a few things in it besides all categories (assuming you have some error reports) Change-Id: If7c71cd669e7eadc1792eefe1fb6e85a08c054ed Reviewed-on: https://gerrit.instructure.com/8190 Reviewed-by: Brian Palmer <brianp@instructure.com> Tested-by: Hudson <hudson@instructure.com>
1 parent 947e651 commit d58dfaa

4 files changed

Lines changed: 28 additions & 4 deletions

File tree

app/models/error_report.rb

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,20 @@ def self.useful_http_env_stuff_from_request(request)
137137
stuff
138138
end
139139

140+
def self.categories
141+
if ActiveRecord::Base.configurations[RAILS_ENV]['adapter'] == 'postgresql'
142+
categories = []
143+
category = ErrorReport.minimum(:category)
144+
while category
145+
categories << category
146+
category = ErrorReport.minimum(:category, :conditions => [ 'category>?', category ])
147+
end
148+
categories
149+
else
150+
ErrorReport.find(:all, :select => 'DISTINCT category', :conditions => 'category IS NOT NULL', :order => 'category').map(&:category)
151+
end
152+
end
153+
140154
on_send_to_external do |error_report|
141155
config = Canvas::Plugin.find('error_reporting').try(:settings) || {}
142156

app/models/user.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ class User < ActiveRecord::Base
2020
# this has to be before include COntext to prevent a circular dependency in Course
2121
def self.sortable_name_order_by_clause(table = nil)
2222
col = table ? "#{table}.sortable_name" : 'sortable_name'
23-
connection_pool.spec.config[:adapter] == 'postgresql' ? "LOWER(#{col})" : col
23+
ActiveRecord::Base.configurations[RAILS_ENV]['adapter'] == 'postgresql' ? "LOWER(#{col})" : col
2424
end
2525

2626
include Context

app/views/errors/index.html.erb

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,14 @@
33
add_crumb t("#crumbs.error_reports", "Error Reports")
44
%>
55

6-
<% unless params[:action] == 'show' %>
76
<% form_for @current_user, :url => '/error_reports', :html => { :method => :get } do |f| %>
87
<%= t :message_contains, "Message contains" %>
98
<input type="text" name="message" value="<%= @message %>" style="width: 250px;"/>
109
<select name="category">
11-
<%= options_for_select(([[t(:all_categories, " - All Categories -"), nil]] + ErrorReport.all(:select => 'DISTINCT category').map(&:category)).compact, params[:category]) %>
10+
<%= options_for_select([[t(:all_categories, " - All Categories -"), nil]] + ErrorReport.categories, params[:category]) %>
1211
</select>
1312
<button type="submit" class="button"><%= t "#buttons.search", "Search" %></button>
1413
<% end %>
15-
<% end %>
1614

1715
<% content_for :pagination do %>
1816
<% if params[:action] == 'index' %>

spec/models/error_report_spec.rb

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,4 +51,16 @@
5151
it "should not fail with invalid UTF-8" do
5252
ErrorReport.log_error('my error', :message => "he\xffllo")
5353
end
54+
55+
it "should return categories" do
56+
ErrorReport.categories.should == []
57+
ErrorReport.create! { |r| r.category = 'bob' }
58+
ErrorReport.categories.should == ['bob']
59+
ErrorReport.create! { |r| r.category = 'bob' }
60+
ErrorReport.categories.should == ['bob']
61+
ErrorReport.create! { |r| r.category = 'george' }
62+
ErrorReport.categories.should == ['bob', 'george']
63+
ErrorReport.create! { |r| r.category = 'fred' }
64+
ErrorReport.categories.should == ['bob', 'fred', 'george']
65+
end
5466
end

0 commit comments

Comments
 (0)