diff options
34 files changed, 681 insertions, 50 deletions
diff --git a/Gemfile.lock b/Gemfile.lock index f17e72d33..2c002e6c2 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -21,9 +21,9 @@ GEM annotate (2.4.0) columnize (0.3.6) fakeweb (1.3.0) - fast_gettext (0.6.7) - gettext (2.1.0) - locale (>= 2.0.5) + fast_gettext (0.6.8) + gettext (2.2.1) + locale json (1.5.4) linecache (0.46) rbx-require-relative (> 0.0.4) @@ -32,8 +32,8 @@ GEM memcache-client (1.8.5) net-http-local (0.1.2) net-purge (0.1.0) - pg (0.11.0) - rack (1.1.0) + pg (0.13.2) + rack (1.1.3) rails (2.3.14) actionmailer (= 2.3.14) actionpack (= 2.3.14) @@ -41,10 +41,10 @@ GEM activeresource (= 2.3.14) activesupport (= 2.3.14) rake (>= 0.8.3) - rake (0.9.2) + rake (0.9.2.2) rbx-require-relative (0.0.9) rdoc (2.4.3) - recaptcha (0.3.1) + recaptcha (0.3.4) rmagick (2.13.1) routing-filter (0.2.4) actionpack @@ -60,11 +60,11 @@ GEM ruby-msg (1.5.0) ruby-ole (>= 1.2.8) vpim (>= 0.360) - ruby-ole (1.2.11.2) + ruby-ole (1.2.11.3) vpim (0.695) will_paginate (2.3.16) xapian-full-alaveteli (1.2.9.4) - xml-simple (1.1.0) + xml-simple (1.1.1) zip (2.0.2) PLATFORMS diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb new file mode 100644 index 000000000..524aa44b7 --- /dev/null +++ b/app/controllers/api_controller.rb @@ -0,0 +1,169 @@ +class ApiController < ApplicationController + before_filter :check_api_key + + def show_request + @request = InfoRequest.find(params[:id]) + raise PermissionDenied if @request.public_body_id != @public_body.id + + @request_data = { + :id => @request.id, + :url => make_url("request", @request.url_title), + :title => @request.title, + + :created_at => @request.created_at, + :updated_at => @request.updated_at, + + :status => @request.calculate_status, + + :public_body_url => make_url("body", @request.public_body.url_name), + :requestor_url => make_url("user", @request.user.url_name), + :request_email => @request.incoming_email, + + :request_text => @request.last_event_forming_initial_request.outgoing_message.body, + } + + render :json => @request_data + end + + def create_request + json = ActiveSupport::JSON.decode(params[:request_json]) + request = InfoRequest.new( + :title => json["title"], + :public_body_id => @public_body.id, + :described_state => "waiting_response", + :external_user_name => json["external_user_name"], + :external_url => json["external_url"] + ) + + outgoing_message = OutgoingMessage.new( + :status => 'ready', + :message_type => 'initial_request', + :body => json["body"], + :last_sent_at => Time.now(), + :what_doing => 'normal_sort', + :info_request => request + ) + request.outgoing_messages << outgoing_message + + # Return an error if the request is invalid + # (Can this ever happen?) + if !request.valid? + render :json => { + 'errors' => request.errors.full_messages + } + return + end + + # Save the request, and add the corresponding InfoRequestEvent + request.save! + request.log_event("sent", + :api => true, + :email => nil, + :outgoing_message_id => outgoing_message.id, + :smtp_message_id => nil + ) + + # Return the URL and ID number. + render :json => { + 'url' => make_url("request", request.url_title), + 'id' => request.id + } + + end + + def add_correspondence + request = InfoRequest.find(params[:id]) + json = ActiveSupport::JSON.decode(params[:correspondence_json]) + attachments = params[:attachments] + + direction = json["direction"] + body = json["body"] + sent_at_str = json["sent_at"] + + errors = [] + + if !request.is_external? + raise ActiveRecord::RecordNotFound.new("Request #{params[:id]} cannot be updated using the API") + end + + if request.public_body_id != @public_body.id + raise ActiveRecord::RecordNotFound.new("You do not own request #{params[:id]}") + end + + if !["request", "response"].include?(direction) + errors << "The direction parameter must be 'request' or 'response'" + end + + if body.nil? + errors << "The 'body' is missing" + elsif body.empty? + errors << "The 'body' is empty" + end + + begin + sent_at = Time.iso8601(sent_at_str) + rescue ArgumentError + errors << "Failed to parse 'sent_at' field as ISO8601 time: #{sent_at_str}" + end + + if direction == "request" && !attachments.nil? + errors << "You cannot attach files to messages in the 'request' direction" + end + + if !errors.empty? + render :json => { "errors" => errors }, :status => 500 + return + end + + if direction == "request" + # In the 'request' direction, i.e. what we (Alaveteli) regard as outgoing + + outgoing_message = OutgoingMessage.new( + :info_request => request, + :status => 'ready', + :message_type => 'followup', + :body => body, + :last_sent_at => sent_at, + :what_doing => 'normal_sort' + ) + request.outgoing_messages << outgoing_message + request.save! + request.log_event("followup_sent", + :api => true, + :email => nil, + :outgoing_message_id => outgoing_message.id, + :smtp_message_id => nil + ) + else + # In the 'response' direction, i.e. what we (Alaveteli) regard as incoming + attachment_hashes = [] + (attachments || []).each_with_index do |attachment, i| + filename = File.basename(attachment.original_filename) + attachment_body = attachment.read + content_type = AlaveteliFileTypes.filename_and_content_to_mimetype(filename, attachment_body) || 'application/octet-stream' + attachment_hashes.push( + :content_type => content_type, + :body => attachment_body, + :filename => filename + ) + end + + mail = RequestMailer.create_external_response(request, body, sent_at, attachment_hashes) + request.receive(mail, mail.encoded, true) + end + + head :no_content + end + + protected + def check_api_key + raise "Missing required parameter 'k'" if params[:k].nil? + @public_body = PublicBody.find_by_api_key(params[:k].gsub(' ', '+')) + raise PermissionDenied if @public_body.nil? + end + + private + def make_url(*args) + "http://" + MySociety::Config.get("DOMAIN", '127.0.0.1:3000') + "/" + args.join("/") + end +end diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb index 2f5b4d643..7f42eeb7e 100644 --- a/app/controllers/request_controller.rb +++ b/app/controllers/request_controller.rb @@ -59,7 +59,7 @@ class RequestController < ApplicationController # Look up by old style numeric identifiers if params[:url_title].match(/^[0-9]+$/) @info_request = InfoRequest.find(params[:url_title].to_i) - redirect_to request_url(@info_request) + redirect_to request_url(@info_request, :format => params[:format]) return end @@ -309,9 +309,11 @@ class RequestController < ApplicationController # See if values were valid or not if !@existing_request.nil? || !@info_request.valid? - # We don't want the error "Outgoing messages is invalid", as the outgoing message - # will be valid for a specific reason which we are displaying anyway. + # We don't want the error "Outgoing messages is invalid", as in this + # case the list of errors will also contain a more specific error + # describing the reason it is invalid. @info_request.errors.delete("outgoing_messages") + render :action => 'new' return end diff --git a/app/helpers/link_to_helper.rb b/app/helpers/link_to_helper.rb index f621721b6..1a86333b6 100755 --- a/app/helpers/link_to_helper.rb +++ b/app/helpers/link_to_helper.rb @@ -96,6 +96,13 @@ module LinkToHelper def user_link_absolute(user) link_to h(user.name), main_url(user_url(user)) end + def request_user_link_absolute(request) + if request.is_external? + request.external_user_name || _("Anonymous user") + else + user_link_absolute(request.user) + end + end def user_or_you_link(user) if @user && user == @user link_to h("you"), user_url(user) diff --git a/app/models/incoming_message.rb b/app/models/incoming_message.rb index 3419956d6..593590fb8 100644 --- a/app/models/incoming_message.rb +++ b/app/models/incoming_message.rb @@ -344,7 +344,7 @@ class IncomingMessage < ActiveRecord::Base # Lotus notes quoting yeuch! def remove_lotus_quoting(text, replacement = "FOLDED_QUOTED_SECTION") text = text.dup - name = Regexp.escape(self.info_request.user.name) + name = Regexp.escape(self.info_request.user_name) # To end of message sections # http://www.whatdotheyknow.com/request/university_investment_in_the_arm diff --git a/app/models/info_request.rb b/app/models/info_request.rb index 3a571daae..d09acbcf6 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -33,7 +33,7 @@ class InfoRequest < ActiveRecord::Base validates_format_of :title, :with => /[a-zA-Z]/, :message => N_("Please write a summary with some text in it"), :if => Proc.new { |info_request| !info_request.title.nil? && !info_request.title.empty? } belongs_to :user - #validates_presence_of :user_id # breaks during construction of new ones :( + validate :must_be_internal_or_external belongs_to :public_body validates_presence_of :public_body_id @@ -104,6 +104,43 @@ class InfoRequest < ActiveRecord::Base errors.add(:described_state, "is not a valid state") if !InfoRequest.enumerate_states.include? described_state end + + # The request must either be internal, in which case it has + # a foreign key reference to a User object and no external_url or external_user_name, + # or else be external in which case it has no user_id but does have an external_url, + # and may optionally also have an external_user_name. + # + # External requests are requests that have been added using the API, whereas internal + # requests are requests made using the site. + def must_be_internal_or_external + # We must permit user_id and external_user_name both to be nil, because the system + # allows a request to be created by a non-logged-in user. + if !user_id.nil? + errors.add(:external_user_name, "must be null for an internal request") if !external_user_name.nil? + errors.add(:external_url, "must be null for an internal request") if !external_url.nil? + end + end + + def is_external? + !external_url.nil? + end + + def user_name + is_external? ? external_user_name : user.name + end + + def user_name_slug + if is_external? + if external_user_name.nil? + fake_slug = "anonymous" + else + fake_slug = external_user_name.parameterize + end + public_body.url_name + "_"+fake_slug + else + user.url_name + end + end @@custom_states_loaded = false begin @@ -232,7 +269,7 @@ public return self.magic_email("request-") end def incoming_name_and_email - return TMail::Address.address_from_name_and_email(self.user.name, self.incoming_email).to_s + return TMail::Address.address_from_name_and_email(self.user_name, self.incoming_email).to_s end # Subject lines for emails about the request @@ -453,7 +490,7 @@ public self.save! end self.info_request_events.each { |event| event.xapian_mark_needs_index } # for the "waiting_classification" index - RequestMailer.deliver_new_response(self, incoming_message) + RequestMailer.deliver_new_response(self, incoming_message) if !is_external? end diff --git a/app/models/info_request_event.rb b/app/models/info_request_event.rb index 9a4f6d9fe..a827d19a4 100644 --- a/app/models/info_request_event.rb +++ b/app/models/info_request_event.rb @@ -118,7 +118,7 @@ class InfoRequestEvent < ActiveRecord::Base :eager_load => [ :outgoing_message, :comment, { :info_request => [ :user, :public_body, :censor_rules ] } ] def requested_by - self.info_request.user.url_name + self.info_request.user_name_slug end def requested_from # acts_as_xapian will detect translated fields via Globalize and add all the diff --git a/app/models/public_body.rb b/app/models/public_body.rb index 267b5d60c..a372de435 100644 --- a/app/models/public_body.rb +++ b/app/models/public_body.rb @@ -17,6 +17,7 @@ # notes :text default(""), not null # first_letter :string(255) not null # publication_scheme :text default(""), not null +# api_key :string(255) not null # # models/public_body.rb: @@ -28,6 +29,7 @@ # $Id: public_body.rb,v 1.160 2009-10-02 22:56:35 francis Exp $ require 'csv' +require 'securerandom' require 'set' class PublicBody < ActiveRecord::Base @@ -87,10 +89,13 @@ class PublicBody < ActiveRecord::Base end end - # Make sure publication_scheme gets the correct default value. - # (This would work automatically, were publication_scheme not a translated attribute) def after_initialize + # Make sure publication_scheme gets the correct default value. + # (This would work automatically, were publication_scheme not a translated attribute) self.publication_scheme = "" if self.publication_scheme.nil? + + # Set an API key if there isn’t one + self.api_key = SecureRandom.base64(32) if self.api_key.nil? end # like find_by_url_name but also search historic url_name if none found @@ -178,7 +183,7 @@ class PublicBody < ActiveRecord::Base end acts_as_versioned - self.non_versioned_columns << 'created_at' << 'updated_at' << 'first_letter' + self.non_versioned_columns << 'created_at' << 'updated_at' << 'first_letter' << 'api_key' class Version attr_accessor :created_at diff --git a/app/models/raw_email.rb b/app/models/raw_email.rb index 1466e5d9c..3bb794684 100644 --- a/app/models/raw_email.rb +++ b/app/models/raw_email.rb @@ -19,13 +19,12 @@ class RawEmail < ActiveRecord::Base has_one :incoming_message - # We keep the old data_text field (which is of type text) for backwards - # compatibility. We use the new data_binary field because only it works - # properly in recent versions of PostgreSQL (get seg faults escaping - # some binary strings). - def directory request_id = self.incoming_message.info_request.id.to_s + if request_id.empty? + raise "Failed to find the id number of the associated request: has it been saved?" + end + if ENV["RAILS_ENV"] == "test" return File.join(Rails.root, 'files/raw_email_test') else @@ -36,7 +35,11 @@ class RawEmail < ActiveRecord::Base end def filepath - File.join(self.directory, self.incoming_message.id.to_s) + incoming_message_id = self.incoming_message.id.to_s + if incoming_message_id.empty? + raise "Failed to find the id number of the associated incoming message: has it been saved?" + end + File.join(self.directory, incoming_message_id) end def data=(d) diff --git a/app/models/request_mailer.rb b/app/models/request_mailer.rb index 1b0bb48b9..03d26f237 100644 --- a/app/models/request_mailer.rb +++ b/app/models/request_mailer.rb @@ -28,6 +28,21 @@ class RequestMailer < ApplicationMailer :filename => attachment_name end end + + # Used when a response is uploaded using the API + def external_response(info_request, body, sent_at, attachments) + @from = blackhole_email + @recipients = info_request.incoming_name_and_email + @body = { :body => body } + + # ActionMailer only works properly when the time is in the local timezone: + # see https://rails.lighthouseapp.com/projects/8994/tickets/3113-actionmailer-only-works-correctly-with-sent_on-times-that-are-in-the-local-time-zone + @sent_on = sent_at.dup.localtime + + attachments.each do |attachment_hash| + attachment attachment_hash + end + end # Incoming message arrived for a request, but new responses have been stopped. def stopped_responses(info_request, email, raw_email_data) @@ -241,7 +256,12 @@ class RequestMailer < ApplicationMailer # Send email alerts for overdue requests def self.alert_overdue_requests() - info_requests = InfoRequest.find(:all, :conditions => [ "described_state = 'waiting_response' and awaiting_description = ?", false ], :include => [ :user ] ) + info_requests = InfoRequest.find(:all, + :conditions => [ + "described_state = 'waiting_response' and awaiting_description = ? and user_id is not null", false + ], + :include => [ :user ] + ) for info_request in info_requests alert_event_id = info_request.last_event_forming_initial_request.id # Only overdue requests diff --git a/app/views/admin_public_body/show.rhtml b/app/views/admin_public_body/show.rhtml index 643ccf5e8..fa17d4027 100644 --- a/app/views/admin_public_body/show.rhtml +++ b/app/views/admin_public_body/show.rhtml @@ -49,7 +49,7 @@ <th>Updated at</th> <% history_columns = PublicBody.content_columns + [] # force dup - history_columns.delete_if {|c| ['created_at', 'updated_at', 'first_letter'].include?(c.name)} + history_columns.delete_if {|c| ['created_at', 'updated_at', 'first_letter', 'api_key'].include?(c.name)} for column in history_columns %> <th><%= column.human_name %></th> <% end %> diff --git a/app/views/admin_request/_some_requests.rhtml b/app/views/admin_request/_some_requests.rhtml index f2b8e7bea..dc11e0f55 100644 --- a/app/views/admin_request/_some_requests.rhtml +++ b/app/views/admin_request/_some_requests.rhtml @@ -12,7 +12,15 @@ <tr class="<%= cycle('odd', 'even') %>"> <td><%= request_both_links(info_request) %></td> <td><%= public_body_both_links(info_request.public_body) %></td> - <td><%= user_both_links(info_request.user) %></td> + <% if info_request.is_external? %> + <% if info_request.external_user_name.nil? %> + <td><i><%= _("Anonymous user") %></i></td> + <% else %> + <td><%= h(info_request.external_user_name) %></td> + <% end %> + <% else %> + <td><%= user_both_links(info_request.user) %></td> + <% end %> <% for column in InfoRequest.content_columns.map { |c| c.name } - [ "title", "url_title" ] %> <td><%=h info_request.send(column) %></td> <% end %> diff --git a/app/views/request/_request_listing_short_via_event.rhtml b/app/views/request/_request_listing_short_via_event.rhtml index cc2a5a162..d93a91070 100644 --- a/app/views/request/_request_listing_short_via_event.rhtml +++ b/app/views/request/_request_listing_short_via_event.rhtml @@ -7,7 +7,7 @@ end %> <p> <%= _('To {{public_body_link_absolute}}',:public_body_link_absolute => public_body_link_absolute(info_request.public_body))%> -<%= _('by {{user_link_absolute}}',:user_link_absolute => user_link_absolute(info_request.user))%> +<%= _('by {{user_link_absolute}}',:user_link_absolute => request_user_link_absolute(info_request))%> <%= simple_date(info_request.created_at) %> </p> </div> diff --git a/app/views/request/_request_listing_via_event.rhtml b/app/views/request/_request_listing_via_event.rhtml index 7a211ed88..e3abfe393 100644 --- a/app/views/request/_request_listing_via_event.rhtml +++ b/app/views/request/_request_listing_via_event.rhtml @@ -17,13 +17,13 @@ end %> </span> <div class="requester"> <% if event.event_type == 'sent' %> - <%= _('Request sent to {{public_body_name}} by {{info_request_user}} on {{date}}.',:public_body_name=>public_body_link_absolute(info_request.public_body),:info_request_user=>user_link_absolute(info_request.user),:date=>simple_date(event.created_at )) %> + <%= _('Request sent to {{public_body_name}} by {{info_request_user}} on {{date}}.',:public_body_name=>public_body_link_absolute(info_request.public_body),:info_request_user=>request_user_link_absolute(info_request),:date=>simple_date(event.created_at )) %> <% elsif event.event_type == 'followup_sent' %> <%=event.display_status %> - <%= _('sent to {{public_body_name}} by {{info_request_user}} on {{date}}.',:public_body_name=>public_body_link_absolute(info_request.public_body),:info_request_user=>user_link_absolute(info_request.user),:date=>simple_date(event.created_at )) %> + <%= _('sent to {{public_body_name}} by {{info_request_user}} on {{date}}.',:public_body_name=>public_body_link_absolute(info_request.public_body),:info_request_user=>request_user_link_absolute(info_request),:date=>simple_date(event.created_at )) %> <% elsif event.event_type == 'response' %> <%=event.display_status %> - <%= _('by {{public_body_name}} to {{info_request_user}} on {{date}}.',:public_body_name=>public_body_link_absolute(info_request.public_body),:info_request_user=>user_link_absolute(info_request.user),:date=>simple_date(event.created_at )) %> + <%= _('by {{public_body_name}} to {{info_request_user}} on {{date}}.',:public_body_name=>public_body_link_absolute(info_request.public_body),:info_request_user=>request_user_link_absolute(info_request),:date=>simple_date(event.created_at )) %> <% elsif event.event_type == 'comment' %> <%= _('Request to {{public_body_name}} by {{info_request_user}}. Annotated by {{event_comment_user}} on {{date}}.',:public_body_name=>public_body_link_absolute(info_request.public_body),:info_request_user=>user_link_absolute(info_request.user),:event_comment_user=>user_link_absolute(event.comment.user),:date=>simple_date(event.created_at)) %> <% else %> diff --git a/app/views/request/_sidebar.rhtml b/app/views/request/_sidebar.rhtml index d6d5e8f12..d20e27e23 100644 --- a/app/views/request/_sidebar.rhtml +++ b/app/views/request/_sidebar.rhtml @@ -23,7 +23,7 @@ <p><%= _('This request has been marked for review by the site administrators, who have not hidden it at this time. If you believe it should be hidden, please <a href="%s">contact us</a>.') % [help_requesting_path] %></p> <% end %> <% else %> - <p><%= _('Requests for personal information and vexatious requests are not considered valid for FOI purposes (<a href="/help/about">read more</a>).') %> + <p><%= _('Requests for personal information and vexatious requests are not considered valid for FOI purposes (<a href="/help/about">read more</a>).') %></p> <p><%= ('If you believe this request is not suitable, you can report it for attention by the site administrators') %></p> <%= link_to _("Report this request"), report_path, :class => "link_button_green", :method => "POST" %> <% end %> diff --git a/app/views/request_mailer/external_response.rhtml b/app/views/request_mailer/external_response.rhtml new file mode 100644 index 000000000..e9858f03f --- /dev/null +++ b/app/views/request_mailer/external_response.rhtml @@ -0,0 +1 @@ +<%=@body%> diff --git a/app/views/request_mailer/new_response_reminder_alert.rhtml b/app/views/request_mailer/new_response_reminder_alert.rhtml index 5f07e8559..86fc71de7 100644 --- a/app/views/request_mailer/new_response_reminder_alert.rhtml +++ b/app/views/request_mailer/new_response_reminder_alert.rhtml @@ -1,4 +1,4 @@ -<%=_('To let us know, follow this link and then select the appropriate box.')%> +<%=_('To let everyone know, follow this link and then select the appropriate box.')%> <%=@url%> diff --git a/config/environments/development.rb b/config/environments/development.rb index cfb727695..f21f27ab6 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -24,5 +24,3 @@ config.action_mailer.delivery_method = :sendmail # so is queued, rather than giv # unintentionally kept references to objects, especially strings. # require 'memory_profiler' # MemoryProfiler.start :string_debug => true, :delay => 10 - -config.gem "gettext", :version => '>=1.9.3', :lib => false diff --git a/config/routes.rb b/config/routes.rb index 814deb760..13ab6669e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -240,6 +240,14 @@ ActionController::Routing::Routes.draw do |map| rule.admin_rule_update '/admin/censor/update/:id', :action => 'update' rule.admin_rule_destroy '/admin/censor/destroy/:censor_rule_id', :action => 'destroy' end + + map.with_options :controller => 'api' do |api| + api.api_create_request '/api/v2/request.json', :action => 'create_request', :conditions => { :method => :post } + + api.api_show_request '/api/v2/request/:id.json', :action => 'show_request', :conditions => { :method => :get } + api.api_add_correspondence '/api/v2/request/:id.json', :action => 'add_correspondence', :conditions => { :method => :post } + end + map.filter('conditionallyprependlocale') # Allow downloading Web Service WSDL as a file with an extension diff --git a/db/migrate/112_add_api_key_to_public_bodies.rb b/db/migrate/112_add_api_key_to_public_bodies.rb new file mode 100644 index 000000000..24961612d --- /dev/null +++ b/db/migrate/112_add_api_key_to_public_bodies.rb @@ -0,0 +1,18 @@ +require "securerandom" + +class AddApiKeyToPublicBodies < ActiveRecord::Migration + def self.up + add_column :public_bodies, :api_key, :string + + PublicBody.find_each do |pb| + pb.api_key = SecureRandom.base64(32) + pb.save! + end + + change_column_null :public_bodies, :api_key, false + end + + def self.down + remove_column :public_bodies, :api_key + end +end diff --git a/db/migrate/113_add_external_fields_to_info_requests.rb b/db/migrate/113_add_external_fields_to_info_requests.rb new file mode 100644 index 000000000..3aea57766 --- /dev/null +++ b/db/migrate/113_add_external_fields_to_info_requests.rb @@ -0,0 +1,22 @@ +class AddExternalFieldsToInfoRequests < ActiveRecord::Migration + def self.up + change_column_null :info_requests, :user_id, true + add_column :info_requests, :external_user_name, :string, :null => true + add_column :info_requests, :external_url, :string, :null => true + + if ActiveRecord::Base.connection.adapter_name == "PostgreSQL" + execute "ALTER TABLE info_requests ADD CONSTRAINT info_requests_external_ck CHECK ( (user_id is null) = (external_url is not null) and (external_user_name is not null or external_url is null) )" + end + end + + def self.down + if ActiveRecord::Base.connection.adapter_name == "PostgreSQL" + execute "ALTER TABLE info_requests DROP CONSTRAINT info_requests_external_ck" + end + + remove_column :info_requests, :external_url + remove_column :info_requests, :external_user_name + + change_column_null :info_requests, :user_id, false + end +end diff --git a/db/migrate/112_add_receive_email_alerts_to_user.rb b/db/migrate/115_add_receive_email_alerts_to_user.rb index 7e06dd275..7e06dd275 100644 --- a/db/migrate/112_add_receive_email_alerts_to_user.rb +++ b/db/migrate/115_add_receive_email_alerts_to_user.rb diff --git a/lib/quiet_opener.rb b/lib/quiet_opener.rb index cb8cf0619..8cedad250 100644 --- a/lib/quiet_opener.rb +++ b/lib/quiet_opener.rb @@ -1,5 +1,6 @@ require 'open-uri' require 'net-purge' +require 'net/http/local' def quietly_try_to_open(url) begin diff --git a/script/handle-mail-replies b/script/handle-mail-replies index 7006b83dd..15454b311 100755 --- a/script/handle-mail-replies +++ b/script/handle-mail-replies @@ -1,4 +1,4 @@ #!/bin/bash cd "`dirname "$0"`" -exec bundle exec ./handle-mail-replies.rb $@ +exec bundle exec ./handle-mail-replies.rb "$@" diff --git a/spec/controllers/api_controller_spec.rb b/spec/controllers/api_controller_spec.rb new file mode 100644 index 000000000..1f65576b6 --- /dev/null +++ b/spec/controllers/api_controller_spec.rb @@ -0,0 +1,263 @@ +require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') + +def normalise_whitespace(s) + s = s.gsub(/^\s+|\s+$/, "") + s = s.gsub(/\s+/, " ") + return s +end + +Spec::Matchers.define :be_equal_modulo_whitespace_to do |expected| + match do |actual| + normalise_whitespace(actual) == normalise_whitespace(expected) + end +end + +describe ApiController, "when using the API" do + it "should check the API key" do + request_data = { + "title" => "Tell me about your chickens", + "body" => "Dear Sir,\n\nI should like to know about your chickens.\n\nYours in faith,\nBob\n", + + "external_url" => "http://www.example.gov.uk/foi/chickens_23", + "external_user_name" => "Bob Smith", + } + + number_of_requests = InfoRequest.count + expect { + post :create_request, :k => "This is not really an API key", :request_json => request_data.to_json + }.to raise_error ApplicationController::PermissionDenied + + InfoRequest.count.should == number_of_requests + end + + it "should create a new request from a POST" do + number_of_requests = InfoRequest.count( + :conditions => [ + "public_body_id = ?", + public_bodies(:geraldine_public_body).id + ] + ) + + request_data = { + "title" => "Tell me about your chickens", + "body" => "Dear Sir,\n\nI should like to know about your chickens.\n\nYours in faith,\nBob\n", + + "external_url" => "http://www.example.gov.uk/foi/chickens_23", + "external_user_name" => "Bob Smith", + } + + post :create_request, :k => public_bodies(:geraldine_public_body).api_key, :request_json => request_data.to_json + response.should be_success + + response.content_type.should == "application/json" + + response_body = ActiveSupport::JSON.decode(response.body) + response_body["errors"].should be_nil + response_body["url"].should =~ /^http/ + + InfoRequest.count(:conditions => [ + "public_body_id = ?", + public_bodies(:geraldine_public_body).id] + ).should == number_of_requests + 1 + + new_request = InfoRequest.find(response_body["id"]) + new_request.user_id.should be_nil + new_request.external_user_name.should == request_data["external_user_name"] + new_request.external_url.should == request_data["external_url"] + + new_request.title.should == request_data["title"] + new_request.last_event_forming_initial_request.outgoing_message.body.should == request_data["body"].strip + + new_request.public_body_id.should == public_bodies(:geraldine_public_body).id + end + + def _create_request + post :create_request, + :k => public_bodies(:geraldine_public_body).api_key, + :request_json => { + "title" => "Tell me about your chickens", + "body" => "Dear Sir,\n\nI should like to know about your chickens.\n\nYours in faith,\nBob\n", + + "external_url" => "http://www.example.gov.uk/foi/chickens_23", + "external_user_name" => "Bob Smith", + }.to_json + response.content_type.should == "application/json" + return ActiveSupport::JSON.decode(response.body)["id"] + end + + it "should add a response to a request" do + # First we need an external request + request_id = info_requests(:external_request).id + + # Initially it has no incoming messages + IncomingMessage.count(:conditions => ["info_request_id = ?", request_id]).should == 0 + + # Now add one + sent_at = "2012-05-28T12:35:39+01:00" + response_body = "Thank you for your request for information, which we are handling in accordance with the Freedom of Information Act 2000. You will receive a response within 20 working days or before the next full moon, whichever is sooner.\n\nYours sincerely,\nJohn Gandermulch,\nExample Council FOI Officer\n" + post :add_correspondence, + :k => public_bodies(:geraldine_public_body).api_key, + :id => request_id, + :correspondence_json => { + "direction" => "response", + "sent_at" => sent_at, + "body" => response_body + }.to_json + + # And make sure it worked + response.should be_success + incoming_messages = IncomingMessage.all(:conditions => ["info_request_id = ?", request_id]) + incoming_messages.count.should == 1 + incoming_message = incoming_messages[0] + + incoming_message.sent_at.should == Time.iso8601(sent_at) + incoming_message.get_main_body_text_folded.should be_equal_modulo_whitespace_to(response_body) + end + + it "should add a followup to a request" do + # First we need an external request + request_id = info_requests(:external_request).id + + # Initially it has one outgoing message + OutgoingMessage.count(:conditions => ["info_request_id = ?", request_id]).should == 1 + + # Add another, as a followup + sent_at = "2012-05-29T12:35:39+01:00" + followup_body = "Pls answer ASAP.\nkthxbye\n" + post :add_correspondence, + :k => public_bodies(:geraldine_public_body).api_key, + :id => request_id, + :correspondence_json => { + "direction" => "request", + "sent_at" => sent_at, + "body" => followup_body + }.to_json + + # Make sure it worked + response.should be_success + followup_messages = OutgoingMessage.all( + :conditions => ["info_request_id = ? and message_type = 'followup'", request_id] + ) + followup_messages.size.should == 1 + followup_message = followup_messages[0] + + followup_message.last_sent_at.should == Time.iso8601(sent_at) + followup_message.body.should == followup_body.strip + end + + it "should not allow internal requests to be updated" do + n_incoming_messages = IncomingMessage.count + n_outgoing_messages = OutgoingMessage.count + + expect { + post :add_correspondence, + :k => public_bodies(:geraldine_public_body).api_key, + :id => info_requests(:naughty_chicken_request).id, + :correspondence_json => { + "direction" => "request", + "sent_at" => Time.now.iso8601, + "body" => "xxx" + }.to_json + }.to raise_error ActiveRecord::RecordNotFound + + IncomingMessage.count.should == n_incoming_messages + OutgoingMessage.count.should == n_outgoing_messages + end + + it "should not allow other people’s requests to be updated" do + request_id = _create_request + n_incoming_messages = IncomingMessage.count + n_outgoing_messages = OutgoingMessage.count + + expect { + post :add_correspondence, + :k => public_bodies(:humpadink_public_body).api_key, + :id => request_id, + :correspondence_json => { + "direction" => "request", + "sent_at" => Time.now.iso8601, + "body" => "xxx" + }.to_json + }.to raise_error ActiveRecord::RecordNotFound + + IncomingMessage.count.should == n_incoming_messages + OutgoingMessage.count.should == n_outgoing_messages + end + + it "should not allow files to be attached to a followup" do + post :add_correspondence, + :k => public_bodies(:geraldine_public_body).api_key, + :id => info_requests(:external_request).id, + :correspondence_json => { + "direction" => "request", + "sent_at" => Time.now.iso8601, + "body" => "Are you joking, or are you serious?" + }.to_json, + :attachments => [ + fixture_file_upload("files/tfl.pdf") + ] + + + # Make sure it worked + response.status.to_i.should == 500 + errors = ActiveSupport::JSON.decode(response.body)["errors"] + errors.should == ["You cannot attach files to messages in the 'request' direction"] + end + + it "should allow files to be attached to a response" do + # First we need an external request + request_id = info_requests(:external_request).id + + # Initially it has no incoming messages + IncomingMessage.count(:conditions => ["info_request_id = ?", request_id]).should == 0 + + # Now add one + sent_at = "2012-05-28T12:35:39+01:00" + response_body = "Thank you for your request for information, which we are handling in accordance with the Freedom of Information Act 2000. You will receive a response within 20 working days or before the next full moon, whichever is sooner.\n\nYours sincerely,\nJohn Gandermulch,\nExample Council FOI Officer\n" + post :add_correspondence, + :k => public_bodies(:geraldine_public_body).api_key, + :id => request_id, + :correspondence_json => { + "direction" => "response", + "sent_at" => sent_at, + "body" => response_body + }.to_json, + :attachments => [ + fixture_file_upload("files/tfl.pdf") + ] + + # And make sure it worked + response.should be_success + incoming_messages = IncomingMessage.all(:conditions => ["info_request_id = ?", request_id]) + incoming_messages.count.should == 1 + incoming_message = incoming_messages[0] + + incoming_message.sent_at.should == Time.iso8601(sent_at) + incoming_message.get_main_body_text_folded.should be_equal_modulo_whitespace_to(response_body) + + # Get the attachment + attachments = incoming_message.get_attachments_for_display + attachments.size.should == 1 + attachment = attachments[0] + + attachment.filename.should == "tfl.pdf" + attachment.body.should == load_file_fixture("tfl.pdf") + end + + it "should show information about a request" do + info_request = info_requests(:naughty_chicken_request) + get :show_request, + :k => public_bodies(:geraldine_public_body).api_key, + :id => info_request.id + + response.should be_success + assigns[:request].id.should == info_request.id + + r = ActiveSupport::JSON.decode(response.body) + r["title"].should == info_request.title + # Let’s not test all the fields here, because it would + # essentially just be a matter of copying the code that + # assigns them and changing assignment to an equality + # check, which does not really test anything at all. + end +end diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb index 01a663bf8..530e9b2c3 100644 --- a/spec/controllers/request_controller_spec.rb +++ b/spec/controllers/request_controller_spec.rb @@ -193,6 +193,7 @@ describe RequestController, "when changing things that appear on the request pag end describe RequestController, "when showing one request" do + integrate_views before(:each) do load_raw_emails_data @@ -209,6 +210,12 @@ describe RequestController, "when showing one request" do response.should render_template('show') end + it "should show the request" do + get :show, :url_title => 'why_do_you_have_such_a_fancy_dog' + response.should be_success + response.body.should include("Why do you have such a fancy dog?") + end + it "should assign the request" do get :show, :url_title => 'why_do_you_have_such_a_fancy_dog' assigns[:info_request].should == info_requests(:fancy_dog_request) @@ -1486,7 +1493,7 @@ describe RequestController, "sending unclassified new response reminder alerts" deliveries = ActionMailer::Base.deliveries deliveries.size.should == 3 # sufficiently late it sends reminders too mail = deliveries[0] - mail.body.should =~ /To let us know/ + mail.body.should =~ /To let everyone know/ mail.to_addrs.first.to_s.should == info_requests(:fancy_dog_request).user.name_and_email mail.body =~ /(http:\/\/.*\/c\/(.*))/ mail_url = $1 @@ -1843,9 +1850,17 @@ describe RequestController, "when showing similar requests" do end -describe RequestController, "when reporting a request" do - integrate_views +describe RequestController, "when reporting a request when not logged in" do + it "should only allow logged-in users to report requests" do + get :report_request, :url_title => info_requests(:badger_request).url_title + post_redirect = PostRedirect.get_last_post_redirect + response.should redirect_to(:controller => 'user', :action => 'signin', :token => post_redirect.token) + end +end +describe RequestController, "when reporting a request (logged in)" do + integrate_views + before do @user = users(:robin_user) session[:user_id] = @user.id @@ -1856,19 +1871,29 @@ describe RequestController, "when reporting a request" do title = ir.url_title get :show, :url_title => title assigns[:info_request].attention_requested.should == false + post :report_request, :url_title => title + response.should redirect_to(:action => :show, :url_title => title) + get :show, :url_title => title + response.should be_success assigns[:info_request].attention_requested.should == true assigns[:info_request].described_state.should == "attention_requested" end it "should not allow a request to be reported twice" do title = info_requests(:badger_request).url_title + post :report_request, :url_title => title + response.should redirect_to(:action => :show, :url_title => title) get :show, :url_title => title + response.should be_success response.body.should include("has been reported") + post :report_request, :url_title => title + response.should redirect_to(:action => :show, :url_title => title) get :show, :url_title => title + response.should be_success response.body.should include("has already been reported") end @@ -1876,10 +1901,14 @@ describe RequestController, "when reporting a request" do title = info_requests(:badger_request).url_title get :show, :url_title => title response.body.should include("Offensive?") + post :report_request, :url_title => title + response.should redirect_to(:action => :show, :url_title => title) + get :show, :url_title => title response.body.should_not include("Offensive?") response.body.should include("This request has been reported") + info_requests(:badger_request).set_described_state("successful") get :show, :url_title => title response.body.should_not include("This request has been reported") diff --git a/spec/fixtures/info_request_events.yml b/spec/fixtures/info_request_events.yml index 3266ec634..e3e8a4460 100644 --- a/spec/fixtures/info_request_events.yml +++ b/spec/fixtures/info_request_events.yml @@ -150,3 +150,13 @@ spam_2_incoming_message_event: described_state: successful calculated_state: successful +external_outgoing_message_event: + id: 914 + params_yaml: "--- \n\ + :outgoing_message_id: 8\n" + outgoing_message_id: 8 + info_request_id: 109 + event_type: sent + created_at: 2009-01-02 02:23:45.6789100 + described_state: waiting_response + calculated_state: waiting_response diff --git a/spec/fixtures/info_requests.yml b/spec/fixtures/info_requests.yml index 33e9a16f2..e4f2287c0 100644 --- a/spec/fixtures/info_requests.yml +++ b/spec/fixtures/info_requests.yml @@ -61,7 +61,7 @@ spam_1_request: title: Cheap v1agra url_title: spam_1 created_at: 2010-01-01 01:23:45.6789100 - created_at: 2010-01-01 01:23:45.6789100 + updated_at: 2010-01-01 01:23:45.6789100 public_body_id: 5 user_id: 5 described_state: successful @@ -72,9 +72,19 @@ spam_2_request: title: Cheap v1agra url_title: spam_2 created_at: 2010-01-01 02:23:45.6789100 - created_at: 2010-01-01 02:23:45.6789100 + updated_at: 2010-01-01 02:23:45.6789100 public_body_id: 6 user_id: 5 described_state: successful awaiting_description: false idhash: 173fd005 +external_request: + id: 109 + title: Balalas + url_title: balalas + external_user_name: Bob Smith + external_url: http://www.example.org/request/balala + public_body_id: 2 + described_state: waiting_response + awaiting_description: false + idhash: a1234567 diff --git a/spec/fixtures/outgoing_messages.yml b/spec/fixtures/outgoing_messages.yml index d33ca4292..32b322bd7 100644 --- a/spec/fixtures/outgoing_messages.yml +++ b/spec/fixtures/outgoing_messages.yml @@ -86,3 +86,14 @@ spam_2_outgoing_message: updated_at: 2007-01-12 02:56:58.586598 what_doing: normal_sort +external_outgoing_message: + id: 8 + info_request_id: 109 + message_type: initial_request + status: sent + body: "I should like to know about balalas." + last_sent_at: 2009-01-12 01:57:58.586598 + created_at: 2009-01-12 01:56:58.586598 + updated_at: 2009-01-12 01:56:58.586598 + what_doing: normal_sort + diff --git a/spec/fixtures/public_bodies.yml b/spec/fixtures/public_bodies.yml index a0893f1e5..367e0fc50 100644 --- a/spec/fixtures/public_bodies.yml +++ b/spec/fixtures/public_bodies.yml @@ -10,6 +10,7 @@ geraldine_public_body: short_name: TGQ url_name: tgq created_at: 2007-10-24 10:51:01.161639 + api_key: 1 humpadink_public_body: name: "Department for Humpadinking" first_letter: D @@ -23,6 +24,7 @@ humpadink_public_body: url_name: dfh created_at: 2007-10-25 10:51:01.161639 notes: An albatross told me!!! + api_key: 2 forlorn_public_body: name: "Department of Loneliness" first_letter: D @@ -36,6 +38,7 @@ forlorn_public_body: url_name: lonely created_at: 2011-01-26 14:11:02.12345 notes: A very lonely public body that no one has corresponded with + api_key: 3 silly_walks_public_body: id: 5 version: 1 @@ -49,6 +52,7 @@ silly_walks_public_body: url_name: msw created_at: 2007-10-25 10:51:01.161639 notes: You know the one. + api_key: 4 sensible_walks_public_body: id: 6 version: 1 @@ -62,3 +66,5 @@ sensible_walks_public_body: last_edit_comment: Another stunning innovation from your friendly national government last_edit_editor: robin created_at: 2008-10-25 10:51:01.161639 + api_key: 5 + diff --git a/spec/models/incoming_message_spec.rb b/spec/models/incoming_message_spec.rb index 6cfd2eb64..c6658905c 100644 --- a/spec/models/incoming_message_spec.rb +++ b/spec/models/incoming_message_spec.rb @@ -120,6 +120,7 @@ describe IncomingMessage, " folding quoted parts of emails" do @user.stub!(:name).and_return("Sir [ Bobble") @info_request = mock_model(InfoRequest) @info_request.stub!(:user).and_return(@user) + @info_request.stub!(:user_name).and_return(@user.name) @incoming_message = IncomingMessage.new() @incoming_message.info_request = @info_request diff --git a/spec/models/xapian_spec.rb b/spec/models/xapian_spec.rb index 81c066184..195b39eee 100644 --- a/spec/models/xapian_spec.rb +++ b/spec/models/xapian_spec.rb @@ -76,13 +76,13 @@ describe PublicBody, " when indexing requests by body they are to" do it "should find requests to the body" do xapian_object = InfoRequest.full_search([InfoRequestEvent], "requested_from:tgq", 'created_at', true, nil, 100, 1) - xapian_object.results.size.should == 4 + xapian_object.results.size.should == PublicBody.find_by_url_name("tgq").info_requests.map(&:info_request_events).flatten.size end it "should update index correctly when URL name of body changes" do # initial search xapian_object = InfoRequest.full_search([InfoRequestEvent], "requested_from:tgq", 'created_at', true, nil, 100, 1) - xapian_object.results.size.should == 4 + xapian_object.results.size.should == PublicBody.find_by_url_name("tgq").info_requests.map(&:info_request_events).flatten.size models_found_before = xapian_object.results.map { |x| x[:model] } # change the URL name of the body @@ -96,7 +96,7 @@ describe PublicBody, " when indexing requests by body they are to" do xapian_object = InfoRequest.full_search([InfoRequestEvent], "requested_from:tgq", 'created_at', true, nil, 100, 1) xapian_object.results.size.should == 0 xapian_object = InfoRequest.full_search([InfoRequestEvent], "requested_from:gq", 'created_at', true, nil, 100, 1) - xapian_object.results.size.should == 4 + xapian_object.results.size.should == PublicBody.find_by_url_name("gq").info_requests.map(&:info_request_events).flatten.size models_found_after = xapian_object.results.map { |x| x[:model] } models_found_before.should == models_found_after @@ -118,7 +118,7 @@ describe PublicBody, " when indexing requests by body they are to" do xapian_object = InfoRequest.full_search([InfoRequestEvent], "requested_from:gq", 'created_at', true, nil, 100, 1) xapian_object.results.size.should == 0 xapian_object = InfoRequest.full_search([InfoRequestEvent], "requested_from:" + body.url_name, 'created_at', true, nil, 100, 1) - xapian_object.results.size.should == 4 + xapian_object.results.size.should == public_bodies(:geraldine_public_body).info_requests.map(&:info_request_events).flatten.size models_found_after = xapian_object.results.map { |x| x[:model] } end end diff --git a/spec/views/public_body/show.rhtml_spec.rb b/spec/views/public_body/show.rhtml_spec.rb index a37d8be0d..1d21f80c4 100644 --- a/spec/views/public_body/show.rhtml_spec.rb +++ b/spec/views/public_body/show.rhtml_spec.rb @@ -102,9 +102,10 @@ def mock_event :info_request => mock_model(InfoRequest, :title => 'Title', :url_title => 'title', - :display_status => 'awaiting_response', - :calculate_status => 'awaiting_response', + :display_status => 'waiting_response', + :calculate_status => 'waiting_response', :public_body => @pb, + :is_external? => false, :user => mock_model(User, :name => 'Test User', :url_name => 'testuser') ), :incoming_message => nil, :is_incoming_message? => false, diff --git a/spec/views/request/list.rhtml_spec.rb b/spec/views/request/list.rhtml_spec.rb index c7067294f..94ece5e76 100644 --- a/spec/views/request/list.rhtml_spec.rb +++ b/spec/views/request/list.rhtml_spec.rb @@ -19,7 +19,8 @@ describe "when listing recent requests" do :display_status => 'awaiting_response', :calculate_status => 'awaiting_response', :public_body => mock_model(PublicBody, :name => 'Test Quango', :url_name => 'testquango'), - :user => mock_model(User, :name => 'Test User', :url_name => 'testuser') + :user => mock_model(User, :name => 'Test User', :url_name => 'testuser'), + :is_external? => false ), :incoming_message => nil, :is_incoming_message? => false, :outgoing_message => nil, :is_outgoing_message? => false, |