Commit d123d0a82aff66630edbf87705cc740f9c1d10ff
1 parent
44b8346b
Exists in
master
and in
1 other branch
Order errs/notices base on last notice time
Showing
4 changed files
with
16 additions
and
8 deletions
Show diff stats
app/controllers/errs_controller.rb
| 1 | class ErrsController < ApplicationController | 1 | class ErrsController < ApplicationController |
| 2 | 2 | ||
| 3 | def index | 3 | def index |
| 4 | - @errs = Err.unresolved.paginate(:page => params[:page]) | 4 | + @errs = Err.unresolved.ordered.paginate(:page => params[:page]) |
| 5 | end | 5 | end |
| 6 | 6 | ||
| 7 | def show | 7 | def show |
| 8 | @project = Project.find(params[:project_id]) | 8 | @project = Project.find(params[:project_id]) |
| 9 | @err = @project.errs.find(params[:id]) | 9 | @err = @project.errs.find(params[:id]) |
| 10 | - @notices = @err.notices.paginate(:page => (params[:notice] || @err.notices.count), :per_page => 1) | 10 | + @notices = @err.notices.ordered.paginate(:page => (params[:notice] || @err.notices.count), :per_page => 1) |
| 11 | @notice = @notices.first | 11 | @notice = @notices.first |
| 12 | end | 12 | end |
| 13 | 13 |
app/models/err.rb
| @@ -7,6 +7,7 @@ class Err | @@ -7,6 +7,7 @@ class Err | ||
| 7 | field :action | 7 | field :action |
| 8 | field :environment | 8 | field :environment |
| 9 | field :fingerprint | 9 | field :fingerprint |
| 10 | + field :last_notice_at, :type => DateTime | ||
| 10 | field :resolved, :type => Boolean, :default => false | 11 | field :resolved, :type => Boolean, :default => false |
| 11 | 12 | ||
| 12 | referenced_in :project | 13 | referenced_in :project |
| @@ -16,6 +17,7 @@ class Err | @@ -16,6 +17,7 @@ class Err | ||
| 16 | 17 | ||
| 17 | scope :resolved, where(:resolved => true) | 18 | scope :resolved, where(:resolved => true) |
| 18 | scope :unresolved, where(:resolved => false) | 19 | scope :unresolved, where(:resolved => false) |
| 20 | + scope :ordered, order_by(:last_notice_at.desc) | ||
| 19 | 21 | ||
| 20 | def self.for(attrs) | 22 | def self.for(attrs) |
| 21 | project = attrs.delete(:project) | 23 | project = attrs.delete(:project) |
| @@ -30,10 +32,6 @@ class Err | @@ -30,10 +32,6 @@ class Err | ||
| 30 | !resolved? | 32 | !resolved? |
| 31 | end | 33 | end |
| 32 | 34 | ||
| 33 | - def last_notice_at | ||
| 34 | - notices.last.try(:created_at) | ||
| 35 | - end | ||
| 36 | - | ||
| 37 | def where | 35 | def where |
| 38 | where = component.dup | 36 | where = component.dup |
| 39 | where << "##{action}" if action.present? | 37 | where << "##{action}" if action.present? |
app/models/notice.rb
| @@ -12,10 +12,13 @@ class Notice | @@ -12,10 +12,13 @@ class Notice | ||
| 12 | 12 | ||
| 13 | embedded_in :err, :inverse_of => :notices | 13 | embedded_in :err, :inverse_of => :notices |
| 14 | 14 | ||
| 15 | + after_create :cache_last_notice_at | ||
| 15 | after_create :deliver_notification, :if => :should_notify? | 16 | after_create :deliver_notification, :if => :should_notify? |
| 16 | 17 | ||
| 17 | validates_presence_of :backtrace, :server_environment, :notifier | 18 | validates_presence_of :backtrace, :server_environment, :notifier |
| 18 | 19 | ||
| 20 | + scope :ordered, order_by(:created_at.asc) | ||
| 21 | + | ||
| 19 | def self.from_xml(hoptoad_xml) | 22 | def self.from_xml(hoptoad_xml) |
| 20 | hoptoad_notice = Hoptoad::V2.parse_xml(hoptoad_xml) | 23 | hoptoad_notice = Hoptoad::V2.parse_xml(hoptoad_xml) |
| 21 | project = Project.find_by_api_key!(hoptoad_notice['api-key']) | 24 | project = Project.find_by_api_key!(hoptoad_notice['api-key']) |
| @@ -61,6 +64,10 @@ class Notice | @@ -61,6 +64,10 @@ class Notice | ||
| 61 | Mailer.error_notification(self).deliver | 64 | Mailer.error_notification(self).deliver |
| 62 | end | 65 | end |
| 63 | 66 | ||
| 67 | + def cache_last_notice_at | ||
| 68 | + err.update_attributes(:last_notice_at => created_at) | ||
| 69 | + end | ||
| 70 | + | ||
| 64 | protected | 71 | protected |
| 65 | 72 | ||
| 66 | def should_notify? | 73 | def should_notify? |
spec/controllers/errs_controller_spec.rb
| @@ -9,7 +9,9 @@ describe ErrsController do | @@ -9,7 +9,9 @@ describe ErrsController do | ||
| 9 | it "gets a paginated list of unresolved errors" do | 9 | it "gets a paginated list of unresolved errors" do |
| 10 | errors = WillPaginate::Collection.new(1,30) | 10 | errors = WillPaginate::Collection.new(1,30) |
| 11 | 3.times { errors << Factory(:err) } | 11 | 3.times { errors << Factory(:err) } |
| 12 | - Err.should_receive(:unresolved).and_return(mock('proxy', :paginate => errors)) | 12 | + Err.should_receive(:unresolved).and_return( |
| 13 | + mock('proxy', :ordered => mock('proxy', :paginate => errors)) | ||
| 14 | + ) | ||
| 13 | get :index | 15 | get :index |
| 14 | assigns(:errs).should == errors | 16 | assigns(:errs).should == errors |
| 15 | end | 17 | end |
| @@ -33,7 +35,8 @@ describe ErrsController do | @@ -33,7 +35,8 @@ describe ErrsController do | ||
| 33 | it "paginates the notices, 1 at a time" do | 35 | it "paginates the notices, 1 at a time" do |
| 34 | Project.stub(:find).with(project.id).and_return(project) | 36 | Project.stub(:find).with(project.id).and_return(project) |
| 35 | project.errs.stub(:find).with(err.id).and_return(err) | 37 | project.errs.stub(:find).with(err.id).and_return(err) |
| 36 | - err.notices.should_receive(:paginate).with(:page => 3, :per_page => 1). | 38 | + err.notices.should_receive(:ordered).and_return(proxy = stub('proxy')) |
| 39 | + proxy.should_receive(:paginate).with(:page => 3, :per_page => 1). | ||
| 37 | and_return(WillPaginate::Collection.new(1,1) << err.notices.first) | 40 | and_return(WillPaginate::Collection.new(1,1) << err.notices.first) |
| 38 | get :show, :project_id => project.id, :id => err.id | 41 | get :show, :project_id => project.id, :id => err.id |
| 39 | end | 42 | end |