Fix subtle bug, which caused error with old data and new not-sending-…

…to-auto-annotations code

app/models/info_request.rb

@@ -24,7 +24,7 @@
 # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved.
 # Email: francis@mysociety.org; WWW: http://www.mysociety.org/
 #
-# $Id: info_request.rb,v 1.199 2009-08-18 20:51:26 francis Exp $
+# $Id: info_request.rb,v 1.200 2009-08-19 00:22:49 francis Exp $
 
 require 'digest/sha1'
 require File.join(File.dirname(__FILE__),'../../vendor/plugins/acts_as_xapian/lib/acts_as_xapian')
@@ -555,16 +555,6 @@ def log_event(type, params)
         info_request_event.save!
     end
 
-    # The last comment made, for alerts
-    def get_last_comment_event
-        for e in self.info_request_events.reverse
-            if e.event_type == 'comment'
-                return e
-            end
-        end
-        return nil
-    end
-
     # The last response is the default one people might want to reply to
     def get_last_response_event_id
         for e in self.info_request_events.reverse

app/models/request_mailer.rb

@@ -4,7 +4,7 @@
 # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved.
 # Email: francis@mysociety.org; WWW: http://www.mysociety.org/
 #
-# $Id: request_mailer.rb,v 1.81 2009-08-18 21:32:23 francis Exp $
+# $Id: request_mailer.rb,v 1.82 2009-08-19 00:22:49 francis Exp $
 
 class RequestMailer < ApplicationMailer
 
@@ -367,17 +367,16 @@ def self.alert_comment_on_request()
         for info_request in info_requests
             #STDERR.puts "considering request " + info_request.id.to_s
 
-            # Find the last comment, which we use as id to mark alert done
-            last_comment_event = info_request.get_last_comment_event
-            raise "expected comment event but got none" if last_comment_event.nil?
-
             # Count number of new comments to alert on
             earliest_unalerted_comment_event = nil
+            last_comment_event = nil
             count = 0
             for e in info_request.info_request_events.reverse
                 #STDERR.puts "event " + e.id.to_s + " type " + e.event_type
                 # alert on comments, which were not made by the user who originally made the request
                 if e.event_type == 'comment' && e.comment.user_id != info_request.user_id
+                    last_comment_event = e if last_comment_event.nil?
+
                     alerted_for = e.user_info_request_sent_alerts.find(:first, :conditions => [ "alert_type = 'comment_1' and user_id = ?", info_request.user_id])
                     #STDERR.puts "is comment by other user, alerted_for " + alerted_for.to_s + " comment user " + e.comment.user_id.to_s + " request user " + info_request.user_id.to_s + " body: " + e.comment.body
                     if alerted_for.nil?

spec/controllers/request_controller_spec.rb

@@ -790,7 +790,7 @@ def expect_redirect(status, redirect_path)
     integrate_views
     fixtures :info_requests, :info_request_events, :public_bodies, :users, :incoming_messages, :raw_emails, :outgoing_messages, :comments # all needed as integrating views
 
-    it "should send an alert" do
+    it "should send an alert (once and once only)" do
         # delete ficture comment and make new one, so is in last month (as
         # alerts are only for comments in last month, see
         # RequestMailer.alert_comment_on_request)
@@ -801,9 +801,7 @@ def expect_redirect(status, redirect_path)
 
         # send comment alert
         RequestMailer.alert_comment_on_request
-
         deliveries = ActionMailer::Base.deliveries
-        deliveries.size.should == 1
         mail = deliveries[0]
         mail.body.should =~ /has annotated your/
         mail.to_addrs.to_s.should == info_requests(:fancy_dog_request).user.name_and_email
@@ -814,6 +812,12 @@ def expect_redirect(status, redirect_path)
         # mail_url.should == comment_url(comments(:silly_comment))
 
         #STDERR.puts mail.body
+
+        # check if we send again, no more go out
+        deliveries.clear
+        RequestMailer.alert_comment_on_request
+        deliveries = ActionMailer::Base.deliveries
+        deliveries.size.should == 0
     end
 
     it "should not send an alert when you comment on your own request" do