Database now queried to work out if a notification should be suppressed.

3 files changed, 42 insertions, 45 deletions

diff --git a/lib/mauve/configuration_builders/person.rb b/lib/mauve/configuration_builders/person.rb
index f171d10..9cc09ab 100644
--- a/lib/mauve/configuration_builders/person.rb
+++ b/lib/mauve/configuration_builders/person.rb
@@ -51,10 +51,15 @@ module Mauve
       def suppress_notifications_after(h)
         raise ArgumentError.new("notification_threshold must be specified as e.g. (10 => 1.minute)") unless h.kind_of?(Hash)
 
-        h.each do |k,v|
-          raise ArgumentError.new("notification_threshold must be specified as e.g. (10 => 1.minute)") unless k.is_a?(Integer) and v.is_a?(Integer)
-
-          @result.notification_thresholds[v] = Array.new(k)
+        h.each do |number_of_alerts,in_period|
+          raise ArgumentError.new("notification_threshold must be specified as e.g. (10 => 1.minute)") unless number_of_alerts.is_a?(Integer) and in_period.is_a?(Integer)
+
+          @result.suppress_notifications_after[in_period] = number_of_alerts
+          # History.all(
+          # :limit => number_of_alerts, 
+          # :order => :created_at.desc, 
+          # :type => "notification",
+          # :event.like => '% succeeded')
         end
       end
 
@@ -87,7 +92,7 @@ module Mauve
       #
       # Add a default notification threshold
       #
-      person.notification_thresholds[600] = Array.new(5) if person.notification_thresholds.empty?
+      person.suppress_notifications_after[600] = 5 if person.suppress_notifications_after.empty?
       
       #
       # Add a default notify clause
diff --git a/lib/mauve/notifiers/debug.rb b/lib/mauve/notifiers/debug.rb
index a9afc52..b3e88e2 100644
--- a/lib/mauve/notifiers/debug.rb
+++ b/lib/mauve/notifiers/debug.rb
@@ -69,7 +69,7 @@ module Mauve
         
         deliver_to_queue << [Time.now, self.class, destination, message] if deliver_to_queue
         
-        if  @disable_normal_delivery
+        if @disable_normal_delivery
           true # pretend it happened OK if we're just testing
         else
           send_alert_without_debug(destination, alert, all_alerts, conditions)
diff --git a/lib/mauve/person.rb b/lib/mauve/person.rb
index 3302ecc..1aa27ee 100644
--- a/lib/mauve/person.rb
+++ b/lib/mauve/person.rb
@@ -6,13 +6,12 @@ module Mauve
   class Person 
   
     attr_reader :username, :password, :urgent, :normal, :low, :email, :xmpp, :sms
-    attr_reader :notification_thresholds, :last_pop3_login, :suppressed, :notifications
+    attr_reader :last_pop3_login, :suppressed, :notifications
     attr_reader :notify_when_off_sick, :notify_when_on_holiday
 
     # Set up a new Person
     #
     def initialize(username)
-      @notification_thresholds = nil
       @suppressed = false
       #
       # TODO fix up web login so pop3 can be used as a proxy.
@@ -111,36 +110,45 @@ module Mauve
     # @param [Time] Current time.
     # @return [Boolean] If suppression is needed.
     def should_suppress?(with_notification_at = nil, now = Time.now)
+      #
+      # This is the query we use.  It doesn't get polled until later.
+      #
+      previous_notifications = History.all(:order => :created_at.desc, :user => self.username, :type => "notification", :event.like => '% succeeded', :fields => [:created_at])
+
+      #
+      # Find the latest alert.
+      #
+      if with_notification_at.nil?
+        latest_notification = previous_notifications.first
+        latest = (latest_notification.nil? ? nil : latest_notification.created_at)
+      else
+        latest = with_notification_at
+      end
 
-      return self.notification_thresholds.any? do |period, previous_alert_times|
+      return self.suppress_notifications_after.any? do |period, number|
         #
-        # This is going to work out if we would be suppressed if we send a notification now.
+        # If no notification time has been specified, use the earliest alert time.
         #
-        previous_alert_times = History.all(:user => self.username,
-           :type => "notification", 
-           :created_at.gt => (now - period), 
-           :event.like => '% succeeded')
-
-        if with_notification_at.nil?
-         first = previous_alert_times.first
-         last  = previous_alert_times.last
+        if with_notification_at.nil? or number == 0
+         earliest_notification = previous_notifications[number-1]
         else
-         first = previous_alert_times[1]
-         last  = with_notification_at
+         earliest_notification = previous_notifications[number-2]
         end
-   
-        (first.is_a?(Time) and (now - first) < period) or
-          (last.is_a?(Time) and @suppressed and (now - last) < period) 
+
+        earliest = (earliest_notification.nil? ? nil : earliest_notification.created_at)
+
+        (earliest.is_a?(Time) and (now - earliest) < period) or
+          (latest.is_a?(Time) and @suppressed and (now - latest) < period) 
       end
     end
    
-    # The notification thresholds for this user
     #
-    # @return [Hash]
-    def notification_thresholds
-      @notification_thresholds ||= { } 
+    # 
+    #
+    def suppress_notifications_after 
+      @suppress_notifications_after ||= { } 
     end
- 
+
     # This class implements an instance_eval context to execute the blocks
     # for running a notification block for each person.
     # 
@@ -260,23 +268,7 @@ module Mauve
         ).instance_eval(&__send__(level))
       end
 
-      if [result].flatten.any?
-        # 
-        # Remember that we've sent an alert
-        #
-        #self.notification_thresholds.each do |period, previous_alert_times|
-          #
-          # Hmm.. not sure how to make this thread-safe.
-          #
-        #  self.notification_thresholds[period].push now
-        #  self.notification_thresholds[period].shift
-        #end
-
-
-        return true
-      end
-
-      return false
+      return [result].flatten.any?
     end
    
     #