Refactor SmartAnswer::Question::Base

doc/next-node-rules.md

@@ -18,15 +18,54 @@ next_node(permitted: permitted_next_nodes) do |response|
 end
 ```
 
+## Shortcut
+
+If the next node for a question is always the same then you can call `next_node` with a single node key. This will automatically add the specified node key to the list of permitted next nodes i.e. there is no need to specify the `:permitted` option.
+
+```ruby
+# For example
+next_node :red
+```
+
 ## Errors
 
+### Multiple calls to next_node are not allowed
+
+Occurs if `next_node` is called more than once within a single question block.
+
+```ruby
+# For example
+next_node :red
+next_node :green
+```
+
+### ArgumentError: You must specify at least one permitted next node
+
+Occurs if the list of permitted next nodes is empty.
+
+```ruby
+# For example
+next_node(permitted: []) do
+  :red
+end
+```
+
+### ArgumentError: You must specify a block or a single next node key
+
+Occurs if `next_node` is called without a block and with no arguments.
+
+```ruby
+# For example
+next_node
+```
+
 ### Next node undefined
 
 Occurs if the `next_node` block returns something "falsey" (e.g. `nil`).
 
 ```ruby
 # For example
-next_node(permitted: []) do
+next_node(permitted: [:red]) do
   nil
 end
 ```

lib/smart_answer/question/base.rb

@@ -3,42 +3,47 @@ module Question
     class Base < Node
       class NextNodeUndefined < StandardError; end
 
+      attr_reader :permitted_next_nodes
+
       def initialize(flow, name, options = {}, &block)
         @save_input_as = nil
-        @validations ||= []
-        @default_next_node_function ||= lambda {|_|}
+        @validations = []
+        @default_next_node_block = lambda { |_| nil }
         @permitted_next_nodes = []
         super
       end
 
       def next_node(next_node = nil, permitted: [], &block)
+        if @next_node_block.present?
+          raise 'Multiple calls to next_node are not allowed'
+        end
         if block_given?
           unless permitted.any?
-            raise "You must specify the permitted next nodes"
+            raise ArgumentError, 'You must specify at least one permitted next node'
           end
-          @permitted_next_nodes += permitted
-          @default_next_node_function = block
+          @permitted_next_nodes = permitted
+          @next_node_block = block
         elsif next_node
           @permitted_next_nodes = [next_node]
-          @default_next_node_function = proc { next_node }
+          @next_node_block = lambda { |_| next_node }
         else
-          raise ArgumentError
+          raise ArgumentError, 'You must specify a block or a single next node key'
         end
       end
 
       def validate(message = nil, &block)
         @validations << [message, block]
       end
 
-      def permitted_next_nodes(*args)
-        @permitted_next_nodes += args
-      end
-
       def next_node_for(current_state, input)
         validate!(current_state, input)
-        next_node = next_node_from_default_function(current_state, input)
-        responses_and_input = current_state.responses + [input]
-        raise NextNodeUndefined.new("Next node undefined. Node: #{current_state.current_node}. Responses: #{responses_and_input}") unless next_node
+        next_node = current_state.instance_exec(input, &next_node_block)
+        unless next_node
+          responses_and_input = current_state.responses + [input]
+          message = "Next node undefined. Node: #{current_state.current_node}."
+          message << " Responses: #{responses_and_input}."
+          raise NextNodeUndefined.new(message)
+        end
         unless @permitted_next_nodes.include?(next_node)
           raise "Next node (#{next_node}) not in list of permitted next nodes (#{@permitted_next_nodes.to_sentence})"
         end
@@ -77,8 +82,8 @@ def question?
       end
 
     private
-      def permitted_next_node?(next_node)
-        @permitted_next_nodes.include?(next_node)
+      def next_node_block
+        @next_node_block || @default_next_node_block
       end
 
       def validate!(current_state, input)
@@ -92,10 +97,6 @@ def validate!(current_state, input)
           end
         end
       end
-
-      def next_node_from_default_function(current_state, input)
-        current_state.instance_exec(input, &@default_next_node_function)
-      end
     end
   end
 end

test/unit/question_base_test.rb

@@ -1,5 +1,3 @@
-
-require 'ostruct'
 require_relative '../test_helper'
 
 class QuestionBaseTest < ActiveSupport::TestCase
@@ -17,7 +15,18 @@ class QuestionBaseTest < ActiveSupport::TestCase
     assert_equal expected_message, exception.message
   end
 
-  test 'permitted next nodes can be supplied to next_node' do
+  test 'permitted next nodes must be supplied if next_node is called with block' do
+    e = assert_raises(ArgumentError) do
+      SmartAnswer::Question::Base.new(nil, :example) {
+        next_node do
+          :done
+        end
+      }
+    end
+    assert_equal 'You must specify at least one permitted next node', e.message
+  end
+
+  test 'permitted next nodes supplied to next_node are stored' do
     q = SmartAnswer::Question::Base.new(nil, :example) {
       next_node(permitted: [:done]) do
         :done
@@ -26,6 +35,25 @@ class QuestionBaseTest < ActiveSupport::TestCase
     assert_equal [:done], q.permitted_next_nodes
   end
 
+  test 'single next node key must be supplied if next_node called without block' do
+    e = assert_raises(ArgumentError) do
+      SmartAnswer::Question::Base.new(nil, :example) {
+        next_node
+      }
+    end
+    assert_equal 'You must specify a block or a single next node key', e.message
+  end
+
+  test 'multiple calls to next_node are not allowed' do
+    e = assert_raises do
+      SmartAnswer::Question::Base.new(nil, :example) {
+        next_node :outcome_one
+        next_node :outcome_two
+      }
+    end
+    assert_equal 'Multiple calls to next_node are not allowed', e.message
+  end
+
   test "State is carried over on a state transition" do
     q = SmartAnswer::Question::Base.new(nil, :example) {
       next_node :done
@@ -177,7 +205,22 @@ class QuestionBaseTest < ActiveSupport::TestCase
     error = assert_raises(SmartAnswer::Question::Base::NextNodeUndefined) do
       q.next_node_for(initial_state, responses[1])
     end
-    expected_message = "Next node undefined. Node: #{question_name}. Responses: #{responses}"
+    expected_message = "Next node undefined. Node: #{question_name}. Responses: #{responses}."
+    assert_equal expected_message, error.message
+  end
+
+  test "error if next_node was not called for question" do
+    question_name = :example
+    responses = [:blue, :red]
+    q = SmartAnswer::Question::Base.new(nil, question_name) {
+      # no call to next_node
+    }
+    initial_state = SmartAnswer::State.new(q.name)
+    initial_state.responses << responses[0]
+    error = assert_raises(SmartAnswer::Question::Base::NextNodeUndefined) do
+      q.next_node_for(initial_state, responses[1])
+    end
+    expected_message = "Next node undefined. Node: #{question_name}. Responses: #{responses}."
     assert_equal expected_message, error.message
   end
 end