validate Problem#last_notice_at; ensure that it isn't nil

resolves the apparent sorting fail on problems by last notice timestamp

validate Problem#last_notice_at; ensure that it isn't nil
resolves the apparent sorting fail on problems by last notice timestamp
Robert Lail · Nathan Broadbent
1 parent e4c691d2
Showing 6 changed files with 40 additions and 17 deletions Show diff stats
app/helpers/errs_helper.rb
app/models/problem.rb
app/views/errs/_table.html.haml
app/views/errs/show.html.haml
db/migrate/20120829034812_ensure_that_problems_last_notice_at_is_not_nil.rb
spec/models/problem_spec.rb
 module ErrsHelper
-  def last_notice_at(problem)
-    problem.last_notice_at || problem.created_at
-  end
-
   def err_confirm
     Errbit::Config.confirm_resolve_err === false ? nil : 'Seriously?'
   end
@@ -6,8 +6,8 @@ class Problem
   include Mongoid::Document
   include Mongoid::Timestamps
-  field :last_notice_at, :type => DateTime
-  field :first_notice_at, :type => DateTime
+  field :last_notice_at, :type => DateTime, :default => Proc.new { Time.now }
+  field :first_notice_at, :type => DateTime, :default => Proc.new { Time.now }
   field :last_deploy_at, :type => Time
   field :resolved, :type => Boolean, :default => false
   field :resolved_at, :type => Time
@@ -45,6 +45,8 @@ class Problem
   scope :unresolved, where(:resolved => false)
   scope :ordered, order_by(:last_notice_at.desc)
   scope :for_apps, lambda {|apps| where(:app_id.in => apps.all.map(&:id))}
+  
+  validates_presence_of :last_notice_at, :first_notice_at
   def self.in_env(env)
@@ -132,17 +134,22 @@ class Problem
   end
   def cache_notice_attributes(notice=nil)
-    notice ||= notices.first
-    attrs = {:last_notice_at => notices.order_by([:created_at, :asc]).last.try(:created_at), :first_notice_at => notices.order_by([:created_at, :asc]).first.try(:created_at)}
+    first_notice = notices.order_by([:created_at, :asc]).first
+    last_notice = notices.order_by([:created_at, :asc]).last
+    notice ||= first_notice
+    
+    attrs = {}
+    attrs[:first_notice_at] = first_notice.created_at if first_notice
+    attrs[:last_notice_at] = last_notice.created_at if last_notice
     attrs.merge!(
-      :message => notice.message,
+      :message     => notice.message,
       :environment => notice.environment_name,
       :error_class => notice.error_class,
-      :where => notice.where,
+      :where       => notice.where,
       :messages    => attribute_count_increase(:messages, notice.message),
       :hosts       => attribute_count_increase(:hosts, notice.host),
       :user_agents => attribute_count_increase(:user_agents, notice.user_agent_string)
-      ) if notice
+    ) if notice
     update_attributes!(attrs)
   end
@@ -33,7 +33,7 @@
                 - if comment.user
                   %em.commenter= (Errbit::Config.user_has_username ? comment.user.username : comment.user.email).to_s << ":"
                 %em= truncate(comment.body, :length => 100, :separator => ' ')
-          %td.latest #{time_ago_in_words(last_notice_at problem)} ago
+          %td.latest #{time_ago_in_words(problem.last_notice_at)} ago
           %td.deploy= problem.last_deploy_at ? problem.last_deploy_at.to_s(:micro) : 'n/a'
           %td.count= link_to problem.notices_count, app_err_path(problem.app, problem)
           - if any_issue_links
@@ -10,7 +10,7 @@
   %strong Environment:
   = @problem.environment
   %strong Last Notice:
-  = last_notice_at(@problem).to_s(:precise)
+  = @problem.last_notice_at.to_s(:precise)
 - content_for :action_bar do
   - if @problem.unresolved?
     %span= link_to 'resolve', resolve_app_err_path(@app, @problem), :method => :put, :data => { :confirm => err_confirm }, :class => 'resolve'
@@ -0,0 +1,23 @@
+class EnsureThatProblemsLastNoticeAtIsNotNil < Mongoid::Migration
+  def self.up
+    Problem.where("$or" => [{:last_notice_at => nil}, {:first_notice_at => nil}]).each do |problem|
+      first_notice = problem.notices.order_by([:created_at, :asc]).first
+      
+      # Destroy problems with no notices
+      if first_notice.nil?
+        problem.destroy
+        next
+      end
+      
+      last_notice = problem.notices.order_by([:created_at, :asc]).last
+      
+      problem.update_attributes!({
+        :first_notice_at => first_notice.created_at,
+        :last_notice_at => last_notice.created_at
+      })
+    end
+  end
+
+  def self.down
+  end
+end
@@ -24,14 +24,13 @@ describe Problem do
       end
     end
   end
+  
   context '#last_notice_at' do
     it "returns the created_at timestamp of the latest notice" do
       err = Fabricate(:err)
       problem = err.problem
       problem.should_not be_nil
-      problem.last_notice_at.should be_nil
-
       notice1 = Fabricate(:notice, :err => err)
       problem.last_notice_at.should == notice1.created_at
@@ -46,8 +45,6 @@ describe Problem do
       problem = err.problem
       problem.should_not be_nil
-      problem.first_notice_at.should be_nil
-
       notice1 = Fabricate(:notice, :err => err)
       problem.first_notice_at.should == notice1.created_at
	@@ -33,7 +33,7 @@		@@ -33,7 +33,7 @@
33	- if comment.user	33	- if comment.user
34	%em.commenter= (Errbit::Config.user_has_username ? comment.user.username : comment.user.email).to_s << ":"	34	%em.commenter= (Errbit::Config.user_has_username ? comment.user.username : comment.user.email).to_s << ":"
35	%em= truncate(comment.body, :length => 100, :separator => ' ')	35	%em= truncate(comment.body, :length => 100, :separator => ' ')
36	- %td.latest #{time_ago_in_words(last_notice_at problem)} ago	36	+ %td.latest #{time_ago_in_words(problem.last_notice_at)} ago
37	%td.deploy= problem.last_deploy_at ? problem.last_deploy_at.to_s(:micro) : 'n/a'	37	%td.deploy= problem.last_deploy_at ? problem.last_deploy_at.to_s(:micro) : 'n/a'
38	%td.count= link_to problem.notices_count, app_err_path(problem.app, problem)	38	%td.count= link_to problem.notices_count, app_err_path(problem.app, problem)
39	- if any_issue_links	39	- if any_issue_links
	@@ -10,7 +10,7 @@		@@ -10,7 +10,7 @@
10	%strong Environment:	10	%strong Environment:
11	= @problem.environment	11	= @problem.environment
12	%strong Last Notice:	12	%strong Last Notice:
13	- = last_notice_at(@problem).to_s(:precise)	13	+ = @problem.last_notice_at.to_s(:precise)
14	- content_for :action_bar do	14	- content_for :action_bar do
15	- if @problem.unresolved?	15	- if @problem.unresolved?
16	%span= link_to 'resolve', resolve_app_err_path(@app, @problem), :method => :put, :data => { :confirm => err_confirm }, :class => 'resolve'	16	%span= link_to 'resolve', resolve_app_err_path(@app, @problem), :method => :put, :data => { :confirm => err_confirm }, :class => 'resolve'
@@ -0,0 +1,23 @@		@@ -0,0 +1,23 @@
	1	+class EnsureThatProblemsLastNoticeAtIsNotNil < Mongoid::Migration
	2	+ def self.up
	3	+ Problem.where("$or" => [{:last_notice_at => nil}, {:first_notice_at => nil}]).each do \|problem\|
	4	+ first_notice = problem.notices.order_by([:created_at, :asc]).first
	5	+
	6	+ # Destroy problems with no notices
	7	+ if first_notice.nil?
	8	+ problem.destroy
	9	+ next
	10	+ end
	11	+
	12	+ last_notice = problem.notices.order_by([:created_at, :asc]).last
	13	+
	14	+ problem.update_attributes!({
	15	+ :first_notice_at => first_notice.created_at,
	16	+ :last_notice_at => last_notice.created_at
	17	+ })
	18	+ end
	19	+ end
	20	+
	21	+ def self.down
	22	+ end
	23	+end