Refactor custom states loading (use class methods where appropriate, simplify main code at the cost (small) of test legibility)

5 files changed, 32 insertions, 45 deletions

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index b339134eb..44e4880c2 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -12,25 +12,13 @@ class RequestController < ApplicationController
     before_filter :check_read_only, :only => [ :new, :show_response, :describe_state, :upload_response ]
     protect_from_forgery :only => [ :new, :show_response, :describe_state, :upload_response ] # See ActionController::RequestForgeryProtection for details
     
-    def load_custom_states
-        @@custom_states_loaded = false
+    @@custom_states_loaded = false
+    begin
         if !ENV["RAILS_ENV"] == "test"
-            load_custom_states!
-        end
-    end
-
-    def load_custom_states!
-        begin
-            # InfoRequestCustomStates may be `require`d in a theme
-            # plugin, or by a test
-            RequestController.send(:include, RequestControllerCustomStates)
+            include RequestControllerCustomStates
             @@custom_states_loaded = true
-        rescue NameError
         end
-    end
-
-    def initialize
-        self.load_custom_states
+    rescue NameError
     end
 
     def show
diff --git a/app/models/info_request.rb b/app/models/info_request.rb
index c51e0c546..f7a8f58a2 100644
--- a/app/models/info_request.rb
+++ b/app/models/info_request.rb
@@ -80,7 +80,7 @@ class InfoRequest < ActiveRecord::Base
         'blackhole' # just dump them
     ]
 
-    def enumerate_states
+    def self.enumerate_states
         states = [ 
         'waiting_response',
         'waiting_clarification', 
@@ -96,15 +96,25 @@ class InfoRequest < ActiveRecord::Base
         ]
 
         if @@custom_states_loaded
-            states += self.theme_extra_states
+            states += InfoRequest.theme_extra_states
         end
         states
     end
 
     def must_be_valid_state
         errors.add(:described_state, "is not a valid state") if 
-            !self.enumerate_states.include? described_state
+            !InfoRequest.enumerate_states.include? described_state
     end
+
+    @@custom_states_loaded = false
+    begin
+        if !ENV["RAILS_ENV"] == "test"
+            include InfoRequestCustomStates
+            @@custom_states_loaded = true
+        end
+    rescue NameError
+    end
+
     # only check on create, so existing models with mixed case are allowed
     def validate_on_create
         if !self.title.nil? && !MySociety::Validate.uses_mixed_capitals(self.title, 10)
@@ -121,7 +131,6 @@ class InfoRequest < ActiveRecord::Base
     OLD_AGE_IN_DAYS = 21.days
 
     def after_initialize
-        self.load_custom_states
         if self.described_state.nil?
             self.described_state = 'waiting_response'
         end
@@ -131,23 +140,6 @@ class InfoRequest < ActiveRecord::Base
         end
     end
 
-    def load_custom_states
-        @@custom_states_loaded = false
-        if !ENV["RAILS_ENV"] == "test"
-            load_custom_states!
-        end
-    end
-
-    def load_custom_states!
-        begin
-            # InfoRequestCustomStates may be `require`d in a theme
-            # plugin, or by a test
-            InfoRequest.send(:include, InfoRequestCustomStates)
-            @@custom_states_loaded = true
-        rescue NameError
-        end
-    end
-
     def visible_comments
         self.comments.find(:all, :conditions => 'visible')
     end
diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb
index 64f3f8061..4943e4a6c 100644
--- a/spec/controllers/request_controller_spec.rb
+++ b/spec/controllers/request_controller_spec.rb
@@ -719,7 +719,8 @@ describe RequestController, "when classifying an information request" do
 
         describe "when using custom statuses from the theme" do
             InfoRequest.send(:require, File.expand_path(File.join(File.dirname(__FILE__), '..', 'models', 'customstates')))
-
+            InfoRequest.send(:include, InfoRequestCustomStates)
+            InfoRequest.class_eval('@@custom_states_loaded = true')
             it "knows about extended states" do
                 Time.stub!(:now).and_return(Time.utc(2007, 11, 10, 00, 01)) 
                 post_status('deadline_extended')
diff --git a/spec/models/customstates.rb b/spec/models/customstates.rb
index de8d04ffb..406d4ead9 100644
--- a/spec/models/customstates.rb
+++ b/spec/models/customstates.rb
@@ -1,4 +1,9 @@
 module InfoRequestCustomStates
+
+    def self.included(base)
+        base.extend(ClassMethods)
+    end
+
     # Mixin methods for InfoRequest
     def theme_display_status(status)
         if status == 'deadline_extended'
@@ -10,12 +15,6 @@ module InfoRequestCustomStates
         end
     end
 
-    def theme_extra_states
-        return ['deadline_extended',
-                'wrong_response']
-    end
-
-
     def theme_calculate_status
         return 'waiting_classification' if self.awaiting_description
         waiting_response = self.described_state == "waiting_response" || self.described_state == "deadline_extended"
@@ -41,6 +40,12 @@ module InfoRequestCustomStates
         return Holiday.due_date_from(self.date_response_required_by, 15)
     end
 
+    module ClassMethods 
+        def theme_extra_states
+            return ['deadline_extended',
+                    'wrong_response']
+        end
+    end
 end
 
 module RequestControllerCustomStates
diff --git a/spec/models/info_request_spec.rb b/spec/models/info_request_spec.rb
index c13eda8cb..ab1ef1ce0 100644
--- a/spec/models/info_request_spec.rb
+++ b/spec/models/info_request_spec.rb
@@ -166,8 +166,9 @@ describe InfoRequest do
 
         before do
             InfoRequest.send(:require, File.expand_path(File.dirname(__FILE__) + '/customstates'))
+            InfoRequest.send(:include, InfoRequestCustomStates)
+            InfoRequest.class_eval('@@custom_states_loaded = true')
             @ir = info_requests(:naughty_chicken_request)
-            @ir.load_custom_states!
         end
 
         it "rejects invalid states" do