diff options
-rw-r--r-- | app/models/info_request.rb | 34 | ||||
-rw-r--r-- | app/models/request_mailer.rb | 68 | ||||
-rw-r--r-- | app/models/user_info_request_sent_alert.rb | 3 | ||||
-rw-r--r-- | app/views/request/show.rhtml | 2 | ||||
-rw-r--r-- | app/views/request_mailer/comment_on_alert.rhtml | 6 | ||||
-rw-r--r-- | app/views/request_mailer/comment_on_alert_plural.rhtml | 6 | ||||
-rw-r--r-- | config/crontab.ugly | 3 | ||||
-rwxr-xr-x | script/alert-comment-on-request | 7 | ||||
-rw-r--r-- | spec/controllers/request_controller_spec.rb | 42 | ||||
-rw-r--r-- | todo.txt | 8 |
10 files changed, 159 insertions, 20 deletions
diff --git a/app/models/info_request.rb b/app/models/info_request.rb index 8e303eed2..5cc4afabf 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -23,7 +23,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.131 2008-08-26 16:03:36 francis Exp $ +# $Id: info_request.rb,v 1.132 2008-08-29 09:44:31 francis Exp $ require 'digest/sha1' require File.join(File.dirname(__FILE__),'../../vendor/plugins/acts_as_xapian/lib/acts_as_xapian') @@ -496,6 +496,16 @@ public 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 @@ -505,25 +515,21 @@ public end return nil end - - # The last response is the default one people might want to reply to def get_last_response_event - info_request_event_id = get_last_response_event_id - if info_request_event_id.nil? - return nil - else - return InfoRequestEvent.find(info_request_event_id) + for e in self.info_request_events.reverse + if e.event_type == 'response' + 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 = self.get_last_response_event_id - if event_id.nil? + last_response_event = self.get_last_response_event + if last_response_event.nil? return nil + else + return last_response_event.incoming_message end - e = self.info_request_events.find(event_id) - return e.incoming_message end # The last outgoing message diff --git a/app/models/request_mailer.rb b/app/models/request_mailer.rb index 5e663ce90..72c826eac 100644 --- a/app/models/request_mailer.rb +++ b/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.43 2008-08-26 23:43:42 francis Exp $ +# $Id: request_mailer.rb,v 1.44 2008-08-29 09:44:31 francis Exp $ class RequestMailer < ApplicationMailer @@ -125,6 +125,20 @@ class RequestMailer < ApplicationMailer @body = { :incoming_message => incoming_message, :info_request => info_request, :url => url } end + # Tell the requester that they need to clarify their request + def comment_on_alert(info_request, comment) + @from = contact_from_name_and_email + @recipients = info_request.user.name_and_email + @subject = "Somebody added a note to your FOI request - " + info_request.title + @body = { :comment => comment, :info_request => info_request, :url => main_url(comment_url(comment)) } + end + def comment_on_alert_plural(info_request, count, earliest_unalerted_comment) + @from = contact_from_name_and_email + @recipients = info_request.user.name_and_email + @subject = "Some notes have been added to your FOI request - " + info_request.title + @body = { :count => @count, :info_request => info_request, :url => main_url(comment_url(earliest_unalerted_comment)) } + end + # Class function, called by script/mailin with all incoming responses. # [ This is a copy (Monkeypatch!) of function from action_mailer/base.rb, @@ -245,6 +259,58 @@ class RequestMailer < ApplicationMailer end end + # Send email alert to request submitter for new comments on the request. + def self.alert_comment_on_request() + #STDERR.puts "alert_comment_on_request" + # We only check comments made in the last month - this means if the + # cron jobs broke fro more than a month events would be lost, but no + # matter. I suspect the performance gain will be needed (with an index on updated_at) + info_requests = InfoRequest.find(:all, :conditions => [ "(select count(*) from info_request_events where event_type = 'comment' and info_request_events.info_request_id = info_requests.id and updated_at > ?) > 0", Time.now() - 1.month ], :include => [ { :info_request_events => :user_info_request_sent_alerts } ], :order => "info_requests.id" ) + 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 + count = 0 + for e in info_request.info_request_events.reverse + if e.event_type == 'comment' + alerted_for = e.user_info_request_sent_alerts.find(:first, :conditions => [ "alert_type = 'comment_1' and user_id = ?", info_request.user_id]) + if alerted_for.nil? + count = count + 1 + earliest_unalerted_comment_event = e + else + break + end + end + end + + # Alert needs sending if there are new comments + if count > 0 + store_sent = UserInfoRequestSentAlert.new + store_sent.info_request = info_request + store_sent.user = info_request.user + store_sent.alert_type = 'comment_1' + store_sent.info_request_event_id = last_comment_event.id + if count > 1 + STDERR.puts "sending multiple comment on request alert to info_request " + info_request.id.to_s + " user " + info_request.user_id.to_s + " count " + count.to_s + RequestMailer.deliver_comment_on_alert_plural(info_request, count, earliest_unalerted_comment_event.comment) + elsif count == 1 + STDERR.puts "sending comment on request alert to info_request " + info_request.id.to_s + " user " + info_request.user_id.to_s + " event " + last_comment_event.id.to_s + RequestMailer.deliver_comment_on_alert(info_request, last_comment_event.comment) + else + raise "internal error" + end + store_sent.save! + STDERR.puts "sent " + info_request.user.email + end + end + end + + end diff --git a/app/models/user_info_request_sent_alert.rb b/app/models/user_info_request_sent_alert.rb index af3c6e981..7ae76edd1 100644 --- a/app/models/user_info_request_sent_alert.rb +++ b/app/models/user_info_request_sent_alert.rb @@ -17,7 +17,7 @@ # Copyright (c) 2008 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org; WWW: http://www.mysociety.org/ # -# $Id: user_info_request_sent_alert.rb,v 1.20 2008-08-09 15:19:01 francis Exp $ +# $Id: user_info_request_sent_alert.rb,v 1.21 2008-08-29 09:44:31 francis Exp $ class UserInfoRequestSentAlert < ActiveRecord::Base belongs_to :user @@ -28,6 +28,7 @@ class UserInfoRequestSentAlert < ActiveRecord::Base 'new_response_reminder_1', # reminder user to classify the recent response 'new_response_reminder_2', # repeat reminder user to classify the recent response 'not_clarified_1', # reminder that user has to explain part of the request + 'comment_1', # tell user that info request has a new comment ] end diff --git a/app/views/request/show.rhtml b/app/views/request/show.rhtml index 0d6d72f22..c070faf97 100644 --- a/app/views/request/show.rhtml +++ b/app/views/request/show.rhtml @@ -101,7 +101,6 @@ <%= render :partial => 'correspondence', :locals => { :info_request_event => info_request_event } %> <% end %> -<!-- <div id="comment_container"> <h2>Add an annotation</h2> @@ -151,7 +150,6 @@ <%= render :partial => 'comment/comment_form', :locals => { } %> </div> - --> </div> diff --git a/app/views/request_mailer/comment_on_alert.rhtml b/app/views/request_mailer/comment_on_alert.rhtml new file mode 100644 index 000000000..b1650318d --- /dev/null +++ b/app/views/request_mailer/comment_on_alert.rhtml @@ -0,0 +1,6 @@ +<%=@comment.user.name%> has annotated your <%=@info_request.law_used_short%> +request. Follow this link to see what they wrote. + +<%=@url%> + +-- the WhatDoTheyKnow team diff --git a/app/views/request_mailer/comment_on_alert_plural.rhtml b/app/views/request_mailer/comment_on_alert_plural.rhtml new file mode 100644 index 000000000..ee12ba975 --- /dev/null +++ b/app/views/request_mailer/comment_on_alert_plural.rhtml @@ -0,0 +1,6 @@ +There are <%=@count%> new annotations on your <%=@info_request.law_used_short%> +request. Follow this link to see what they wrote. + +<%=@url%> + +-- the WhatDoTheyKnow team diff --git a/config/crontab.ugly b/config/crontab.ugly index f80804448..dd5024ba8 100644 --- a/config/crontab.ugly +++ b/config/crontab.ugly @@ -4,7 +4,7 @@ # Copyright (c) 2008 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org. WWW: http://www.mysociety.org/ # -# $Id: crontab.ugly,v 1.15 2008-06-13 10:16:58 francis Exp $ +# $Id: crontab.ugly,v 1.16 2008-08-29 09:44:32 francis Exp $ PATH=/usr/local/bin:/usr/bin:/bin MAILTO=team@whatdotheyknow.com @@ -18,6 +18,7 @@ MAILTO=team@whatdotheyknow.com # Once an hour 39 * * * * !!(*= $user *)!! run-with-lockfile -n /data/vhost/!!(*= $vhost *)!!/alert-overdue-requests.lock /data/vhost/!!(*= $vhost *)!!/mysociety/foi/script/alert-overdue-requests || echo "stalled?" +09 * * * * !!(*= $user *)!! run-with-lockfile -n /data/vhost/!!(*= $vhost *)!!/alert-comment-on-request.lock /data/vhost/!!(*= $vhost *)!!/mysociety/foi/script/alert-comment-on-request || echo "stalled?" # Once a day, early morning 23 4 * * * !!(*= $user *)!! run-with-lockfile -n /data/vhost/!!(*= $vhost *)!!/delete-old-post-redirects.lock /data/vhost/!!(*= $vhost *)!!/mysociety/foi/script/delete-old-post-redirects || echo "stalled?" diff --git a/script/alert-comment-on-request b/script/alert-comment-on-request new file mode 100755 index 000000000..006ba15f6 --- /dev/null +++ b/script/alert-comment-on-request @@ -0,0 +1,7 @@ +#!/bin/bash + +LOC=`dirname $0` + +$LOC/runner 'RequestMailer.alert_comment_on_request' + + diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb index e5e514c89..b63dbbe2f 100644 --- a/spec/controllers/request_controller_spec.rb +++ b/spec/controllers/request_controller_spec.rb @@ -390,6 +390,48 @@ describe RequestController, "clarification required alerts" do end +describe RequestController, "comment alerts" do + integrate_views + fixtures :info_requests, :info_request_events, :public_bodies, :users, :incoming_messages, :outgoing_messages, :comments # all needed as integrating views + + it "should send an alert" do + 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 + mail.body =~ /(http:\/\/.*)/ + mail_url = $1 + + # XXX check mail_url here somehow, can't call comment_url like this: + # mail_url.should == comment_url(comments(:silly_comment)) + + #STDERR.puts mail.body + end + + it "should send an alert when there are two new comments" do + new_comment = info_requests(:fancy_dog_request).add_comment('Not as daft as this one', users(:bob_smith_user)) + + RequestMailer.alert_comment_on_request + + deliveries = ActionMailer::Base.deliveries + deliveries.size.should == 1 + mail = deliveries[0] + mail.body.should =~ /There are new annotations/ + mail.to_addrs.to_s.should == info_requests(:fancy_dog_request).user.name_and_email + mail.body =~ /(http:\/\/.*)/ + mail_url = $1 + + # XXX check mail_url here somehow, can't call comment_url like this: + # mail_url.should == comment_url(comments(:silly_comment)) + + #STDERR.puts mail.body + end + +end + @@ -27,7 +27,6 @@ doing file RESPONSE/Internal documents/Briefing with Contact Islington/Contact I Search for other extensions that we have now Comments interleaved with body - - Check CSS class/id names used in all comment HTML - Annotation thing too far down when much sidebar e.g. http://localhost:3000/request/heya#outgoing-199 - Email people when comments on their own request @@ -47,6 +46,7 @@ Internal review status/marker? http://www.whatdotheyknow.com/request/crime_investigation_following_al http://www.whatdotheyknow.com/request/determination_of_pricing_for_pro#incoming-2902 my own veolia request + http://www.whatdotheyknow.com/request/online_petitions_documents_from (search for it!) Request withdrawn by user status/marker? @@ -62,12 +62,18 @@ Clear out all the need admin attention requests Clear out all the need classifying requests Admin: + Fix two things from Tony + Add accelerator keys e.g. save public body with S Have internal links to different parts of request page Somehow fold up the enormous pages on many admin pages Make it easy to go from pages to admin page (perhaps via link as in PB?) Replace "deep" with admin_... URLs +is_owning_user is wrong name +move stuff from request_controller_spec.rb to request_mailer_spec.rb +Remove get_last_response_event_id etc. + Later ===== |