diff options
-rw-r--r-- | app/controllers/admin_request_controller.rb | 25 | ||||
-rw-r--r-- | app/controllers/request_controller.rb | 33 | ||||
-rw-r--r-- | app/models/info_request_batch.rb | 8 | ||||
-rw-r--r-- | app/models/outgoing_message.rb | 70 | ||||
-rw-r--r-- | config/application.rb | 1 | ||||
-rw-r--r-- | config/general.yml-example | 3 | ||||
-rw-r--r-- | config/initializers/alaveteli.rb | 1 | ||||
-rw-r--r-- | lib/alaveteli_localization.rb | 1 | ||||
-rw-r--r-- | lib/routing_filters.rb | 8 | ||||
-rw-r--r-- | spec/controllers/request_controller_spec.rb | 20 | ||||
-rw-r--r-- | spec/factories/outgoing_messages.rb | 10 | ||||
-rw-r--r-- | spec/integration/localisation_spec.rb | 29 | ||||
-rw-r--r-- | spec/models/info_request_spec.rb | 11 |
13 files changed, 164 insertions, 56 deletions
diff --git a/app/controllers/admin_request_controller.rb b/app/controllers/admin_request_controller.rb index 21120e4ad..8f023bf12 100644 --- a/app/controllers/admin_request_controller.rb +++ b/app/controllers/admin_request_controller.rb @@ -37,7 +37,30 @@ class AdminRequestController < AdminController def resend @outgoing_message = OutgoingMessage.find(params[:outgoing_message_id]) - @outgoing_message.resend_message + @outgoing_message.prepare_message_for_resend + + mail_message = case @outgoing_message.message_type + when 'initial_request' + OutgoingMailer.initial_request( + @outgoing_message.info_request, + @outgoing_message + ).deliver + when 'followup' + OutgoingMailer.followup( + @outgoing_message.info_request, + @outgoing_message, + @outgoing_message.incoming_message_followup + ).deliver + else + raise "Message id #{id} has type '#{message_type}' which cannot be resent" + end + + @outgoing_message.record_email_delivery( + mail_message.to_addrs.join(', '), + mail_message.message_id, + 'resent' + ) + flash[:notice] = "Outgoing message resent" redirect_to admin_request_show_url(@outgoing_message.info_request) end diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb index 3fa0ef0ce..9e2c291dc 100644 --- a/app/controllers/request_controller.rb +++ b/app/controllers/request_controller.rb @@ -365,8 +365,21 @@ class RequestController < ApplicationController end # This automatically saves dependent objects, such as @outgoing_message, in the same transaction @info_request.save! - # TODO: send_message needs the database id, so we send after saving, which isn't ideal if the request broke here. - @outgoing_message.send_message + + # TODO: Sending the message needs the database id, so we send after + # saving, which isn't ideal if the request broke here. + if @outgoing_message.sendable? + mail_message = OutgoingMailer.initial_request( + @outgoing_message.info_request, + @outgoing_message + ).deliver + + @outgoing_message.record_email_delivery( + mail_message.to_addrs.join(', '), + mail_message.message_id + ) + end + flash[:notice] = _("<p>Your {{law_used_full}} request has been <strong>sent on its way</strong>!</p> <p><strong>We will email you</strong> when there is a response, or after {{late_number_of_days}} working days if the authority still hasn't replied by then.</p> @@ -668,13 +681,27 @@ class RequestController < ApplicationController end # Send a follow up message - @outgoing_message.send_message + @outgoing_message.sendable? + + mail_message = OutgoingMailer.followup( + @outgoing_message.info_request, + @outgoing_message, + @outgoing_message.incoming_message_followup + ).deliver + + @outgoing_message.record_email_delivery( + mail_message.to_addrs.join(', '), + mail_message.message_id + ) + @outgoing_message.save! + if @outgoing_message.what_doing == 'internal_review' flash[:notice] = _("Your internal review request has been sent on its way.") else flash[:notice] = _("Your follow up message has been sent on its way.") end + redirect_to request_url(@info_request) end else diff --git a/app/models/info_request_batch.rb b/app/models/info_request_batch.rb index d7c5eb9af..8a5ebeaba 100644 --- a/app/models/info_request_batch.rb +++ b/app/models/info_request_batch.rb @@ -46,7 +46,13 @@ class InfoRequestBatch < ActiveRecord::Base self.sent_at = Time.now self.save! end - created.each{ |info_request| info_request.outgoing_messages.first.send_message } + created.each do |info_request| + outgoing_message = info_request.outgoing_messages.first + + outgoing_message.sendable? + mail_message = OutgoingMailer.initial_request(outgoing_message.info_request, outgoing_message).deliver + outgoing_message.record_email_delivery(mail_message.to_addrs.join(', '), mail_message.message_id) + end return unrequestable end diff --git a/app/models/outgoing_message.rb b/app/models/outgoing_message.rb index 9424113fc..fa83c7381 100644 --- a/app/models/outgoing_message.rb +++ b/app/models/outgoing_message.rb @@ -171,57 +171,42 @@ class OutgoingMessage < ActiveRecord::Base MySociety::Validate.contains_postcode?(body) end - # Deliver outgoing message - # Note: You can test this from script/console with, say: - # InfoRequest.find(1).outgoing_messages[0].send_message - def send_message(log_event_type = 'sent') + def record_email_delivery(to_addrs, message_id, log_event_type = 'sent') + self.last_sent_at = Time.now + self.status = 'sent' + save! + + log_event_type = "followup_#{ log_event_type }" if message_type == 'followup' + + info_request.log_event(log_event_type, { :email => to_addrs, + :outgoing_message_id => id, + :smtp_message_id => message_id }) + set_info_request_described_state + end + + def sendable? if status == 'ready' if message_type == 'initial_request' - self.last_sent_at = Time.now - self.status = 'sent' - self.save! - - mail_message = OutgoingMailer.initial_request(info_request, self).deliver - self.info_request.log_event(log_event_type, { - :email => mail_message.to_addrs.join(", "), - :outgoing_message_id => self.id, - :smtp_message_id => mail_message.message_id - }) - self.info_request.set_described_state('waiting_response') + return true elsif message_type == 'followup' - self.last_sent_at = Time.now - self.status = 'sent' - self.save! - - mail_message = OutgoingMailer.followup(info_request, self, incoming_message_followup).deliver - self.info_request.log_event('followup_' + log_event_type, { - :email => mail_message.to_addrs.join(", "), - :outgoing_message_id => self.id, - :smtp_message_id => mail_message.message_id - }) - if info_request.described_state == 'waiting_clarification' - self.info_request.set_described_state('waiting_response') - end - if what_doing == 'internal_review' - self.info_request.set_described_state('internal_review') - end + return true else - raise "Message id #{id} has type '#{message_type}' which send_message can't handle" + raise "Message id #{id} has type '#{message_type}' which cannot be sent" end elsif status == 'sent' raise "Message id #{id} has already been sent" else - raise "Message id #{id} not in state for send_message" + raise "Message id #{id} not in state for sending" end end # An admin function - def resend_message + def prepare_message_for_resend if ['initial_request', 'followup'].include?(message_type) and status == 'sent' self.status = 'ready' - send_message('resent') else - raise "Message id #{id} has type '#{message_type}' status '#{status}' which resend_message can't handle" + raise "Message id #{id} has type '#{message_type}' status " \ + "'#{status}' which prepare_message_for_resend can't handle" end end @@ -303,6 +288,19 @@ class OutgoingMessage < ActiveRecord::Base private + def set_info_request_described_state + if message_type == 'initial_request' + info_request.set_described_state('waiting_response') + elsif message_type == 'followup' + if info_request.described_state == 'waiting_clarification' + info_request.set_described_state('waiting_response') + end + if what_doing == 'internal_review' + info_request.set_described_state('internal_review') + end + end + end + def set_default_letter self.body = get_default_message if body.nil? end diff --git a/config/application.rb b/config/application.rb index a514daf3a..ed4f07819 100644 --- a/config/application.rb +++ b/config/application.rb @@ -61,7 +61,6 @@ module Alaveteli config.action_dispatch.rack_cache = nil config.after_initialize do |app| - require 'routing_filters.rb' # Add a catch-all route to force routing errors to be handled by the application, # rather than by middleware. app.routes.append{ match '*path', :to => 'general#not_found' } diff --git a/config/general.yml-example b/config/general.yml-example index a80784712..ac96b5e50 100644 --- a/config/general.yml-example +++ b/config/general.yml-example @@ -1,6 +1,9 @@ # general.yml-example: # Example values for the "general" config file. # +# Documentation on configuring Alaveteli is available at +# http://alaveteli.org/docs/customising/ +# # Configuration parameters, in YAML syntax. # # Copy this file to one called "general.yml" in the same directory. Or diff --git a/config/initializers/alaveteli.rb b/config/initializers/alaveteli.rb index 9a151e3e8..3a1220326 100644 --- a/config/initializers/alaveteli.rb +++ b/config/initializers/alaveteli.rb @@ -55,6 +55,7 @@ require 'date_quarter' require 'public_body_csv' require 'category_and_heading_migrator' require 'public_body_categories' +require 'routing_filters' AlaveteliLocalization.set_locales(AlaveteliConfiguration::available_locales, AlaveteliConfiguration::default_locale) diff --git a/lib/alaveteli_localization.rb b/lib/alaveteli_localization.rb index 6daab124a..2b6978c92 100644 --- a/lib/alaveteli_localization.rb +++ b/lib/alaveteli_localization.rb @@ -7,6 +7,7 @@ class AlaveteliLocalization I18n.locale = default_locale I18n.available_locales = available_locales.map { |locale_name| locale_name.to_sym } I18n.default_locale = default_locale + RoutingFilter::Conditionallyprependlocale.locales = available_locales end def set_default_text_domain(name, path) diff --git a/lib/routing_filters.rb b/lib/routing_filters.rb index a9a62b8db..5b5da6870 100644 --- a/lib/routing_filters.rb +++ b/lib/routing_filters.rb @@ -22,5 +22,13 @@ module RoutingFilter prepend_segment!(result, locale) if prepend_locale?(locale) end end + + # Reset the locale pattern when the locales are set. + class << self + def locales=(locales) + @@locales_pattern = nil + @@locales = locales.map(&:to_sym) + end + end end end diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb index f7c935af3..6c0f4573e 100644 --- a/spec/controllers/request_controller_spec.rb +++ b/spec/controllers/request_controller_spec.rb @@ -1827,7 +1827,15 @@ describe RequestController, "when sending a followup message" do # make the followup session[:user_id] = users(:bob_smith_user).id - post :show_response, :outgoing_message => { :body => "What a useless response! You suck.", :what_doing => 'normal_sort' }, :id => info_requests(:fancy_dog_request).id, :incoming_message_id => incoming_messages(:useless_incoming_message), :submitted_followup => 1 + + post :show_response, + :outgoing_message => { + :body => "What a useless response! You suck.", + :what_doing => 'normal_sort' + }, + :id => info_requests(:fancy_dog_request).id, + :incoming_message_id => incoming_messages(:useless_incoming_message), + :submitted_followup => 1 # check it worked deliveries = ActionMailer::Base.deliveries @@ -1982,7 +1990,15 @@ describe RequestController, "sending overdue request alerts" do :info_request_id => chicken_request.id, :body => 'Some text', :what_doing => 'normal_sort') - outgoing_message.send_message + + outgoing_message.sendable? + mail_message = OutgoingMailer.followup( + outgoing_message.info_request, + outgoing_message, + outgoing_message.incoming_message_followup + ).deliver + outgoing_message.record_email_delivery(mail_message.to_addrs.join(', '), mail_message.message_id) + outgoing_message.save! chicken_request = InfoRequest.find(chicken_request.id) diff --git a/spec/factories/outgoing_messages.rb b/spec/factories/outgoing_messages.rb index 3887d3f48..e11cbdfb9 100644 --- a/spec/factories/outgoing_messages.rb +++ b/spec/factories/outgoing_messages.rb @@ -10,7 +10,9 @@ FactoryGirl.define do body 'Some information please' what_doing 'normal_sort' end + end + factory :internal_review_request do ignore do status 'ready' @@ -18,6 +20,7 @@ FactoryGirl.define do body 'I want a review' what_doing 'internal_review' end + end # FIXME: This here because OutgoingMessage has an after_initialize, @@ -30,9 +33,14 @@ FactoryGirl.define do :message_type => message_type, :body => body, :what_doing => what_doing }) } + after_create do |outgoing_message| - outgoing_message.send_message + outgoing_message.sendable? + outgoing_message.record_email_delivery( + 'test@example.com', + 'ogm-14+537f69734b97c-1ebd@localhost') end + end end diff --git a/spec/integration/localisation_spec.rb b/spec/integration/localisation_spec.rb index 4f6b61ae1..037603ad5 100644 --- a/spec/integration/localisation_spec.rb +++ b/spec/integration/localisation_spec.rb @@ -24,14 +24,29 @@ describe "when generating urls" do response.should_not contain @home_link_regex end - it 'should redirect requests for a public body in a locale to the canonical name in that locale' do - get('/es/body/dfh') - response.should redirect_to "/es/body/edfh" - end + context 'when handling public body requests' do + + before do + AlaveteliLocalization.set_locales(available_locales='es en', default_locale='en') + body = FactoryGirl.create(:public_body, :short_name => 'english_short') + I18n.with_locale(:es) do + body.short_name = 'spanish_short' + body.save! + end + end + + it 'should redirect requests for a public body in a locale to the + canonical name in that locale' do + get('/es/body/english_short') + response.should redirect_to "/es/body/spanish_short" + end - it 'should remember a filter view when redirecting a public body request to the canonical name' do - get('/es/body/tgq/successful') - response.should redirect_to "/es/body/etgq/successful" + it 'should remember a filter view when redirecting a public body + request to the canonical name' do + AlaveteliLocalization.set_locales(available_locales='es en', default_locale='en') + get('/es/body/english_short/successful') + response.should redirect_to "/es/body/spanish_short/successful" + end end describe 'when there is more than one locale' do diff --git a/spec/models/info_request_spec.rb b/spec/models/info_request_spec.rb index afb8e0949..9ad616ea5 100644 --- a/spec/models/info_request_spec.rb +++ b/spec/models/info_request_spec.rb @@ -848,9 +848,11 @@ describe InfoRequest do context "a series of events on a request" do it "should have sensible events after the initial request has been made" do # An initial request is sent - # The logic that changes the status when a message is sent is mixed up - # in OutgoingMessage#send_message. So, rather than extract it (or call it) - # let's just duplicate what it does here for the time being. + # FIXME: The logic that changes the status when a message + # is sent is mixed up in + # OutgoingMessage#record_email_delivery. So, rather than + # extract it (or call it) let's just duplicate what it does + # here for the time being. request.log_event('sent', {}) request.set_described_state('waiting_response') @@ -919,7 +921,8 @@ describe InfoRequest do request.log_event("status_update", {}) request.set_described_state("waiting_response") # A normal follow up is sent - # This is normally done in OutgoingMessage#send_message + # This is normally done in + # OutgoingMessage#record_email_delivery request.log_event('followup_sent', {}) request.set_described_state('waiting_response') |