Merge branch 'develop' of ssh://git.mysociety.org/data/git/public/alaveteli into develop

4 files changed, 119 insertions, 36 deletions

diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb
index 718c31e6f..409a432eb 100644
--- a/app/controllers/api_controller.rb
+++ b/app/controllers/api_controller.rb
@@ -72,7 +72,12 @@ class ApiController < ApplicationController
     end
     
     def add_correspondence
-        request = InfoRequest.find(params[:id])
+        request = InfoRequest.find_by_id(params[:id])
+        if request.nil?
+            render :json => { "errors" => ["Could not find request #{params[:id]}"] }, :status => 404
+            return
+        end
+        
         json = ActiveSupport::JSON.decode(params[:correspondence_json])
         attachments = params[:attachments]
         
@@ -83,11 +88,13 @@ class ApiController < ApplicationController
         errors = []
         
         if !request.is_external?
-            raise ActiveRecord::RecordNotFound.new("Request #{params[:id]} cannot be updated using the API")
+            render :json => { "errors" => ["Request #{params[:id]} cannot be updated using the API"] }, :status => 500
+            return
         end
         
         if request.public_body_id != @public_body.id
-            raise ActiveRecord::RecordNotFound.new("You do not own request #{params[:id]}")
+            render :json => { "errors" => ["You do not own request #{params[:id]}"] }, :status => 500
+            return
         end
         
         if !["request", "response"].include?(direction)
@@ -160,17 +167,41 @@ class ApiController < ApplicationController
         feed_type = params[:feed_type]
         raise PermissionDenied.new("#{@public_body.id} != #{params[:id]}") if @public_body.id != params[:id].to_i
         
-        @events = InfoRequestEvent.find_by_sql([
-          %(select info_request_events.*
-            from info_requests
-            join info_request_events on info_requests.id = info_request_events.info_request_id
-            where info_requests.public_body_id = ?
-            and info_request_events.event_type in (
-              'sent', 'followup_sent', 'resent', 'followup_resent'
-            )
-            order by info_request_events.created_at desc
-          ), @public_body.id
-        ])
+        since_date_str = params[:since_date]
+        if since_date_str.nil?
+            @events = InfoRequestEvent.find_by_sql([
+              %(select info_request_events.*
+                from info_requests
+                join info_request_events on info_requests.id = info_request_events.info_request_id
+                where info_requests.public_body_id = ?
+                and info_request_events.event_type in (
+                  'sent', 'followup_sent', 'resent', 'followup_resent'
+                )
+                order by info_request_events.created_at desc
+              ), @public_body.id
+            ])
+        else
+            begin
+              since_date = Date.strptime(since_date_str, "%Y-%m-%d")
+            rescue ArgumentError
+              render :json => {"errors" => [
+                      "Parameter since_date must be in format yyyy-mm-dd (not '#{since_date_str}')" ] },
+                  :status => 500
+              return
+            end
+            @events = InfoRequestEvent.find_by_sql([
+              %(select info_request_events.*
+                from info_requests
+                join info_request_events on info_requests.id = info_request_events.info_request_id
+                where info_requests.public_body_id = ?
+                and info_request_events.event_type in (
+                  'sent', 'followup_sent', 'resent', 'followup_resent'
+                )
+                and info_request_events.created_at >= ?
+                order by info_request_events.created_at desc
+              ), @public_body.id, since_date
+            ])
+        end
         if feed_type == "atom"
             render :template => "api/request_events.atom", :layout => false
         elsif feed_type == "json"
diff --git a/config/environment.rb b/config/environment.rb
index f788845a9..6234ae5c1 100644
--- a/config/environment.rb
+++ b/config/environment.rb
@@ -32,7 +32,7 @@ require File.join(File.dirname(__FILE__), '../lib/old_rubygems_patch')
 
 
 # Application version
-ALAVETELI_VERSION = '0.6.5'
+ALAVETELI_VERSION = '0.6.5.1'
 
 Rails::Initializer.run do |config|
   # Load intial mySociety config
diff --git a/config/packages b/config/packages
index d059d2906..3cffcb0f9 100644
--- a/config/packages
+++ b/config/packages
@@ -9,7 +9,7 @@ irb
 wv
 poppler-utils
 pdftk (>> 1.41+dfsg-1) | pdftk (<< 1.41+dfsg-1) # that version has a non-functionining uncompress option
-gs-gpl
+ghostscript
 catdoc
 links
 elinks
diff --git a/spec/controllers/api_controller_spec.rb b/spec/controllers/api_controller_spec.rb
index 98751a93a..ded9a040a 100644
--- a/spec/controllers/api_controller_spec.rb
+++ b/spec/controllers/api_controller_spec.rb
@@ -149,16 +149,19 @@ describe ApiController, "when using the API" 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
+        request_id = info_requests(:naughty_chicken_request).id
+        post :add_correspondence,
+            :k => public_bodies(:geraldine_public_body).api_key,
+            :id => request_id,
+            :correspondence_json => {
+                "direction" => "request",
+                "sent_at" => Time.now.iso8601,
+                "body" => "xxx"
+            }.to_json
+        
+        response.status.should == "500 Internal Server Error"
+        ActiveSupport::JSON.decode(response.body)["errors"].should == [
+          "Request #{request_id} cannot be updated using the API"]
         
         IncomingMessage.count.should == n_incoming_messages
         OutgoingMessage.count.should == n_outgoing_messages
@@ -169,16 +172,18 @@ describe ApiController, "when using the API" do
         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
+        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
+        
+        response.status.should == "500 Internal Server Error"
+        ActiveSupport::JSON.decode(response.body)["errors"].should == [
+          "You do not own request #{request_id}"]
         
         IncomingMessage.count.should == n_incoming_messages
         OutgoingMessage.count.should == n_outgoing_messages
@@ -314,4 +319,51 @@ describe ApiController, "when using the API" do
         response.should be_success
         assigns[:event_data].should == [first_event]
     end
+    
+    it "should honour the since_date parameter for the Atom feed" do
+        get :body_request_events,
+            :id => public_bodies(:humpadink_public_body).id,
+            :k => public_bodies(:humpadink_public_body).api_key,
+            :since_date => "2010-01-01",
+            :feed_type => "atom"
+        
+        response.should be_success
+        response.should render_template("api/request_events.atom")
+        assigns[:events].size.should > 0
+        assigns[:events].each do |event|
+            event.created_at.should >= Date.new(2010, 1, 1)
+        end
+    end
+    
+    it "should return a JSON 404 error for non-existent requests" do
+        request_id = 123459876 # Let's hope this doesn't exist!
+        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
+        response.status.should == "404 Not Found"
+        ActiveSupport::JSON.decode(response.body)["errors"].should == ["Could not find request 123459876"]
+    end
+    
+    it "should return a JSON 500 error if we try to add correspondence to a request we don't own" do
+        request_id = info_requests(:naughty_chicken_request).id
+        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
+        response.status.should == "500 Internal Server Error"
+        ActiveSupport::JSON.decode(response.body)["errors"].should == ["Request #{request_id} cannot be updated using the API"]
+    end
 end