aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--app/controllers/admin_request_controller.rb25
-rw-r--r--app/controllers/request_controller.rb33
-rw-r--r--app/models/info_request_batch.rb8
-rw-r--r--app/models/outgoing_message.rb70
-rw-r--r--config/application.rb1
-rw-r--r--config/general.yml-example3
-rw-r--r--config/initializers/alaveteli.rb1
-rw-r--r--lib/alaveteli_localization.rb1
-rw-r--r--lib/routing_filters.rb8
-rw-r--r--spec/controllers/request_controller_spec.rb20
-rw-r--r--spec/factories/outgoing_messages.rb10
-rw-r--r--spec/integration/localisation_spec.rb29
-rw-r--r--spec/models/info_request_spec.rb11
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')