Refactor test code so new test data can be added

Previously many of the tests made assumptions about the global
structure of the test data set: the total number of requests, for
example, or the names of all public bodies. This makes it difficult
to add to the test data. This change is intended to make the test
data easier to extend by eliminating such global assumptions.

4 files changed, 115 insertions, 64 deletions

diff --git a/spec/controllers/admin_public_body_controller_spec.rb b/spec/controllers/admin_public_body_controller_spec.rb
index 2670f2add..6954e22a1 100644
--- a/spec/controllers/admin_public_body_controller_spec.rb
+++ b/spec/controllers/admin_public_body_controller_spec.rb
@@ -25,9 +25,9 @@ describe AdminPublicBodyController, "when administering public bodies" do
     end
 
     it "creates a new public body" do
-        PublicBody.count.should == 2
+        n = PublicBody.count
         post :create, { :public_body => { :name => "New Quango", :short_name => "", :tag_string => "blah", :request_email => 'newquango@localhost', :last_edit_comment => 'From test code' } }
-        PublicBody.count.should == 3
+        PublicBody.count.should == n + 1
     end
 
     it "edits a public body" do
@@ -42,13 +42,19 @@ describe AdminPublicBodyController, "when administering public bodies" do
         pb.name.should == "Renamed"
     end
 
+    it "does not destroy a public body that has associated requests" do
+        id = public_bodies(:humpadink_public_body).id
+        n = PublicBody.count
+        post :destroy, { :id => id }
+        response.should redirect_to(:controller=>'admin_public_body', :action=>'show', :id => id)
+        PublicBody.count.should == n
+    end
+    
     it "destroys a public body" do
-        PublicBody.count.should == 2
-        info_request_events(:badger_outgoing_message_event).destroy
-        outgoing_messages(:badger_outgoing_message).destroy
-        info_requests(:badger_request).destroy
-        post :destroy, { :id => 3 }
-        PublicBody.count.should == 1
+        n = PublicBody.count
+        post :destroy, { :id => public_bodies(:forlorn_public_body).id }
+        response.should redirect_to(:controller=>'admin_public_body', :action=>'list')
+        PublicBody.count.should == n - 1
     end
 
     it "sets a using_admin flag" do
@@ -64,10 +70,10 @@ describe AdminPublicBodyController, "when administering public bodies and paying
 
     it "disallows non-authenticated users to do anything" do
         @request.env["HTTP_AUTHORIZATION"] = ""
-        PublicBody.count.should == 2
+        n = PublicBody.count.should
         post :destroy, { :id => 3 }
         response.code.should == "401"
-        PublicBody.count.should == 2
+        PublicBody.count.should == n
         session[:using_admin].should == nil
     end
 
@@ -76,25 +82,22 @@ describe AdminPublicBodyController, "when administering public bodies and paying
         config['ADMIN_USERNAME'] = ''
         config['ADMIN_PASSWORD'] = ''
         @request.env["HTTP_AUTHORIZATION"] = ""
-        PublicBody.count.should == 2
-        info_request_events(:badger_outgoing_message_event).destroy
-        outgoing_messages(:badger_outgoing_message).destroy
-        info_requests(:badger_request).destroy
-        post :destroy, { :id => 3 }
-        PublicBody.count.should == 1
+
+        n = PublicBody.count
+        post :destroy, { :id => public_bodies(:forlorn_public_body).id }
+        PublicBody.count.should == n - 1
         session[:using_admin].should == 1
     end
+    
     it "skips admin authorisation when no username set" do
         config = MySociety::Config.load_default()
         config['ADMIN_USERNAME'] = ''
         config['ADMIN_PASSWORD'] = 'fuz'
         @request.env["HTTP_AUTHORIZATION"] = ""
-        PublicBody.count.should == 2
-        info_request_events(:badger_outgoing_message_event).destroy
-        outgoing_messages(:badger_outgoing_message).destroy
-        info_requests(:badger_request).destroy
-        post :destroy, { :id => 3 }
-        PublicBody.count.should == 1
+        
+        n = PublicBody.count
+        post :destroy, { :id => public_bodies(:forlorn_public_body).id }
+        PublicBody.count.should == n - 1
         session[:using_admin].should == 1
     end
     it "forces authorisation when password and username set" do
@@ -102,14 +105,11 @@ describe AdminPublicBodyController, "when administering public bodies and paying
         config['ADMIN_USERNAME'] = 'biz'
         config['ADMIN_PASSWORD'] = 'fuz'
         @request.env["HTTP_AUTHORIZATION"] = ""
-        PublicBody.count.should == 2
+        n = PublicBody.count.should
         basic_auth_login(@request, "baduser", "badpassword")
-        info_request_events(:badger_outgoing_message_event).destroy
-        outgoing_messages(:badger_outgoing_message).destroy
-        info_requests(:badger_request).destroy
-        post :destroy, { :id => 3 }
+        post :destroy, { :id => public_bodies(:forlorn_public_body).id }
         response.code.should == "401"
-        PublicBody.count.should == 2
+        PublicBody.count.should == n
         session[:using_admin].should == nil
     end
 
@@ -179,12 +179,9 @@ describe AdminPublicBodyController, "when administering public bodies with i18n"
     end
 
     it "destroy a public body" do
-        PublicBody.count.should == 2
-        info_request_events(:badger_outgoing_message_event).destroy
-        outgoing_messages(:badger_outgoing_message).destroy
-        info_requests(:badger_request).destroy
-        post :destroy, { :id => 3 }
-        PublicBody.count.should == 1
+        n = PublicBody.count
+        post :destroy, { :id => public_bodies(:forlorn_public_body).id }
+        PublicBody.count.should == n - 1
     end
 
 end
@@ -207,23 +204,23 @@ describe AdminPublicBodyController, "when creating public bodies with i18n" do
 
 
     it "creates a new public body in one locale" do
-        PublicBody.count.should == 2
+        n = PublicBody.count
         post :create, { :public_body => { :name => "New Quango", :short_name => "", :tag_string => "blah", :request_email => 'newquango@localhost', :last_edit_comment => 'From test code' } }
-        PublicBody.count.should == 3
+        PublicBody.count.should == n + 1
 
         body = PublicBody.find_by_name("New Quango")
         response.should redirect_to(:controller=>'admin_public_body', :action=>'show', :id=>body.id)
     end
 
     it "creates a new public body with multiple locales" do
-        PublicBody.count.should == 2
+        n = PublicBody.count
         post :create, { 
             :public_body => { 
                 :name => "New Quango", :short_name => "", :tag_string => "blah", :request_email => 'newquango@localhost', :last_edit_comment => 'From test code',
                 :translated_versions => [{ :locale => "es", :name => "Mi Nuevo Quango", :short_name => "", :request_email => 'newquango@localhost' }]
                 }
         }
-        PublicBody.count.should == 3
+        PublicBody.count.should == n + 1
         
         body = PublicBody.find_by_name("New Quango")
         body.translations.map {|t| t.locale.to_s}.sort.should == ["en", "es"]
diff --git a/spec/controllers/public_body_controller_spec.rb b/spec/controllers/public_body_controller_spec.rb
index 131412a90..4b657849a 100644
--- a/spec/controllers/public_body_controller_spec.rb
+++ b/spec/controllers/public_body_controller_spec.rb
@@ -26,13 +26,23 @@ describe PublicBodyController, "when showing a body" do
         assigns[:public_body].should == public_bodies(:humpadink_public_body)
     end
 
-    it "should assign the requests" do
+    it "should assign the requests (1)" do
         get :show, :url_name => "tgq", :view => 'all'
-        assigns[:xapian_requests].results.count.should == 2
+        assigns[:xapian_requests].results.map{|x|x[:model].info_request}.should =~ InfoRequest.all(
+            :conditions => ["public_body_id = ?", public_bodies(:geraldine_public_body).id])
+    end
+    
+    it "should assign the requests (2)" do
         get :show, :url_name => "tgq", :view => 'successful'
-        assigns[:xapian_requests].results.count.should == 0
+        assigns[:xapian_requests].results.map{|x|x[:model].info_request}.should =~ InfoRequest.all(
+            :conditions => ["described_state = ? and public_body_id = ?",
+                "successful", public_bodies(:geraldine_public_body).id])
+    end
+    
+    it "should assign the requests (3)" do
         get :show, :url_name => "dfh", :view => 'all'
-        assigns[:xapian_requests].results.count.should == 1
+        assigns[:xapian_requests].results.map{|x|x[:model].info_request}.should =~ InfoRequest.all(
+            :conditions => ["public_body_id = ?", public_bodies(:humpadink_public_body).id])
     end
 
     it "should assign the body using different locale from that used for url_name" do
@@ -81,16 +91,16 @@ describe PublicBodyController, "when listing bodies" do
 
     it "should list all bodies from default locale, even when there are no translations for selected locale" do
         PublicBody.with_locale(:en) do
-            english_only = PublicBody.new(:name => 'English only',
+            @english_only = PublicBody.new(:name => 'English only',
                                           :short_name => 'EO',
                                           :request_email => 'english@flourish.org',
                                           :last_edit_editor => 'test',
                                           :last_edit_comment => '')
-            english_only.save
+            @english_only.save
         end
         PublicBody.with_locale(:es) do
             get :list
-            assigns[:public_bodies].length.should == 3
+            assigns[:public_bodies].include?(@english_only).should == true
         end
     end
 
@@ -99,7 +109,7 @@ describe PublicBodyController, "when listing bodies" do
 
         response.should render_template('list')
 
-        assigns[:public_bodies].should == [ public_bodies(:humpadink_public_body), public_bodies(:geraldine_public_body) ]
+        assigns[:public_bodies].should == PublicBody.all(:order => "name", :conditions => "id <> #{PublicBody.internal_admin_body.id}")
         assigns[:tag].should == "all"
         assigns[:description].should == ""
     end
@@ -135,11 +145,11 @@ describe PublicBodyController, "when listing bodies" do
 
         get :list, :tag => "other"
         response.should render_template('list')
-        assigns[:public_bodies].should == [ public_bodies(:geraldine_public_body) ]
+        assigns[:public_bodies].should =~ PublicBody.all(:conditions => "id not in (#{public_bodies(:humpadink_public_body).id}, #{PublicBody.internal_admin_body.id})")
         
         get :list
         response.should render_template('list')
-        assigns[:public_bodies].count.should == 2
+        assigns[:public_bodies].should =~ PublicBody.all(:conditions => "id <> #{PublicBody.internal_admin_body.id}")
 
     end
 
diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb
index 055c9b3d4..a77d1ac27 100644
--- a/spec/controllers/request_controller_spec.rb
+++ b/spec/controllers/request_controller_spec.rb
@@ -23,20 +23,42 @@ describe RequestController, "when listing recent requests" do
 
     it "should filter requests" do
         get :list, :view => 'all'
-        assigns[:list_results].size.should == 3
+        assigns[:list_results].map(&:info_request).should =~ InfoRequest.all
+        
         # default sort order is the request with the most recently created event first
-        assigns[:list_results][0].info_request.id.should == 104
+        assigns[:list_results].map(&:info_request).should == InfoRequest.all(
+            :order => "(select max(info_request_events.created_at) from info_request_events where info_request_events.info_request_id = info_requests.id) DESC")
+
         get :list, :view => 'successful'
-        assigns[:list_results].size.should == 0
+        assigns[:list_results].map(&:info_request).should =~ InfoRequest.all(
+            :conditions => "id in (
+                select info_request_id
+                from info_request_events
+                where not exists (
+                    select *
+                    from info_request_events later_events
+                    where later_events.created_at > info_request_events.created_at
+                    and later_events.info_request_id = info_request_events.info_request_id
+                )
+                and info_request_events.described_state in ('successful', 'partially_successful')
+            )")
     end
 
     it "should filter requests by date" do
+        # The semantics of the search are that it finds any InfoRequest
+        # that has any InfoRequestEvent created in the specified range
+        
         get :list, :view => 'all', :request_date_before => '13/10/2007'
-        assigns[:list_results].size.should == 1
+        assigns[:list_results].map(&:info_request).should =~ InfoRequest.all(
+            :conditions => "id in (select info_request_id from info_request_events where created_at < '2007-10-13'::date)")
+        
         get :list, :view => 'all', :request_date_after => '13/10/2007'
-        assigns[:list_results].size.should == 3
+        assigns[:list_results].map(&:info_request).should =~ InfoRequest.all(
+            :conditions => "id in (select info_request_id from info_request_events where created_at > '2007-10-13'::date)")
+        
         get :list, :view => 'all', :request_date_after => '13/10/2007', :request_date_before => '01/11/2007'
-        assigns[:list_results].size.should == 1
+        assigns[:list_results].map(&:info_request).should =~ InfoRequest.all(
+            :conditions => "id in (select info_request_id from info_request_events where created_at between '2007-10-13'::date and '2007-11-01'::date)")
     end
 
     it "should make a sane-sized cache tag" do
@@ -46,13 +68,32 @@ describe RequestController, "when listing recent requests" do
 
     it "should list internal_review requests as unresolved ones" do
         get :list, :view => 'awaiting'
-        assigns[:list_results].size.should == 0
+        
+        # This doesn’t precisely duplicate the logic of the actual
+        # query, but it is close enough to give the same result with
+        # the current set of test data.
+        assigns[:list_results].should =~ InfoRequestEvent.all(
+            :conditions => "described_state in (
+                    'waiting_response', 'waiting_clarification',
+                    'internal_review', 'gone_postal', 'error_message', 'requires_admin'
+                ) and not exists (
+                    select *
+                    from info_request_events later_events
+                    where later_events.created_at > info_request_events.created_at
+                    and later_events.info_request_id = info_request_events.info_request_id
+                )")
+        
+        
+        get :list, :view => 'awaiting'
+        assigns[:list_results].map(&:info_request).include?(info_requests(:fancy_dog_request)).should == false
+        
         event = info_request_events(:useless_incoming_message_event)
-        event.calculated_state = "internal_review"
+        event.described_state = event.calculated_state = "internal_review"
         event.save!
         rebuild_xapian_index
+        
         get :list, :view => 'awaiting'
-        assigns[:list_results].size.should == 1
+        assigns[:list_results].map(&:info_request).include?(info_requests(:fancy_dog_request)).should == true
     end
 
     it "should assign the first page of results" do
diff --git a/spec/controllers/user_controller_spec.rb b/spec/controllers/user_controller_spec.rb
index a90fa18df..81d3ff2e5 100644
--- a/spec/controllers/user_controller_spec.rb
+++ b/spec/controllers/user_controller_spec.rb
@@ -46,16 +46,19 @@ describe UserController, "when showing a user" do
 
     it "should search the user's contributions" do
         get :show, :url_name => "bob_smith"
-        assigns[:xapian_requests].results.count.should == 3
+        assigns[:xapian_requests].results.map{|x|x[:model].info_request}.should =~ InfoRequest.all(:conditions => "user_id = #{users(:bob_smith_user).id}")
+        
         get :show, :url_name => "bob_smith", :user_query => "money"
-        assigns[:xapian_requests].results.count.should == 1
+        assigns[:xapian_requests].results.map{|x|x[:model].info_request}.should == [info_requests(:naughty_chicken_request)]
     end
 
-# Error handling not quite good enough for this yet
-#    it "should not show unconfirmed users" do
-#        get :show, :url_name => "silly_emnameem"
-#        assigns[:display_users].should == [ users(:silly_name_user) ]
-#    end
+    it "should not show unconfirmed users" do
+        begin
+            get :show, :url_name => "unconfirmed_user"
+        rescue => e
+        end
+        e.should be_an_instance_of(ActiveRecord::RecordNotFound)
+    end
 
 end