Use since_event_id to restrict query, not just data displayed.

Allow the same params (since_event_id, since_date) for both formats.
There doesn't seem to be a particular reason not to treat them the same.

2 files changed, 36 insertions, 17 deletions

diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb
index 4473139d1..6f83d89d6 100644
--- a/app/controllers/api_controller.rb
+++ b/app/controllers/api_controller.rb
@@ -191,10 +191,17 @@ class ApiController < ApplicationController
         raise PermissionDenied.new("#{@public_body.id} != #{params[:id]}") if @public_body.id != params[:id].to_i
 
         since_date_str = params[:since_date]
+        since_event_id = params[:since_event_id]
+
         event_type_clause = "event_type in ('sent', 'followup_sent', 'resent', 'followup_resent')"
-        if since_date_str.nil?
-            where_params = event_type_clause
-         else
+
+        @events = InfoRequestEvent.where(event_type_clause) \
+            .joins(:info_request) \
+            .where("public_body_id = ?", @public_body.id) \
+            .includes([{:info_request => :user}, :outgoing_message]) \
+            .order('info_request_events.created_at DESC')
+
+        if since_date_str
             begin
                 since_date = Date.strptime(since_date_str, "%Y-%m-%d")
             rescue ArgumentError
@@ -203,26 +210,29 @@ class ApiController < ApplicationController
                        :status => 500
                 return
             end
-            event_type_clause << " AND info_request_events.created_at >= ?"
-            where_params = [event_type_clause, since_date]
+            @events = @events.where("info_request_events.created_at >= ?", since_date)
+        end
+
+        # We take a "since" parameter that allows the client
+        # to restrict to events more recent than a certain other event
+        if since_event_id
+            begin
+                event = InfoRequestEvent.find(since_event_id)
+            rescue ActiveRecord::RecordNotFound
+                render :json => {"errors" => [
+                                    "Event ID #{since_event_id} not found" ] },
+                       :status => 500
+                return
+            end
+            @events = @events.where("info_request_events.created_at > ?", event.created_at)
         end
-        @events = InfoRequestEvent.where(where_params) \
-            .joins(:info_request) \
-            .where("public_body_id = ?", @public_body.id) \
-            .includes([{:info_request => :user}, :outgoing_message]) \
-            .order('info_request_events.created_at DESC')
+
 
         if feed_type == "atom"
             render :template => "api/request_events", :formats => ['atom'], :layout => false
         elsif feed_type == "json"
-            # For the JSON feed, we take a "since" parameter that allows the client
-            # to restrict to events more recent than a certain other event
-            if params[:since_event_id]
-                @since_event_id = params[:since_event_id].to_i
-            end
             @event_data = []
             @events.each do |event|
-                break if event.id == @since_event_id
 
                 request = event.info_request
                 this_event = {
diff --git a/spec/controllers/api_controller_spec.rb b/spec/controllers/api_controller_spec.rb
index 7b1d73e8a..323ef4cd4 100644
--- a/spec/controllers/api_controller_spec.rb
+++ b/spec/controllers/api_controller_spec.rb
@@ -514,7 +514,7 @@ describe ApiController, "when using the API" do
             assigns[:event_data].should == [first_event]
         end
 
-        it 'should honour the since_date parameter for the Atom feed' do
+        it 'should honour the since_date parameter' do
             get :body_request_events,
                 :id => public_bodies(:humpadink_public_body).id,
                 :k => public_bodies(:humpadink_public_body).api_key,
@@ -527,6 +527,15 @@ describe ApiController, "when using the API" do
             assigns[:events].each do |event|
                 event.created_at.should >= Date.new(2010, 1, 1)
             end
+
+            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 => 'json'
+            assigns[:events].each do |event|
+                event.created_at.should >= Date.new(2010, 1, 1)
+            end
         end
     end
 end