From a220a073045894171cc3071241834a9f07146e00 Mon Sep 17 00:00:00 2001
From: James Mead <james@floehopper.org>
Date: Mon, 29 Feb 2016 15:04:23 +0100
Subject: [PATCH 01/17] Convert permitted_next_nodes into attribute reader
Nothing seems to be relying on the "writer" behaviour of this method.
Furthermore, I think that this kind of dual-purpose method (both reader and
writer) are confusing.
So by converting the method into an attribute reader, we're simplifying the
code.
---
lib/smart_answer/question/base.rb | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/lib/smart_answer/question/base.rb b/lib/smart_answer/question/base.rb
index c6bc8841c3a..3f4df6b8195 100644
--- a/lib/smart_answer/question/base.rb
+++ b/lib/smart_answer/question/base.rb
@@ -3,6 +3,8 @@ 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 ||= []
@@ -30,10 +32,6 @@ 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)
From 2098dd2aea29ca4d40f403aac3d646f8d8a21e7e Mon Sep 17 00:00:00 2001
From: James Mead <james@floehopper.org>
Date: Mon, 29 Feb 2016 15:27:36 +0100
Subject: [PATCH 02/17] Remove unused permitted_next_node? method
This method does not appear to be used anywhere.
---
lib/smart_answer/question/base.rb | 4 ----
1 file changed, 4 deletions(-)
diff --git a/lib/smart_answer/question/base.rb b/lib/smart_answer/question/base.rb
index 3f4df6b8195..8b6294593b8 100644
--- a/lib/smart_answer/question/base.rb
+++ b/lib/smart_answer/question/base.rb
@@ -75,10 +75,6 @@ def question?
end
private
- def permitted_next_node?(next_node)
- @permitted_next_nodes.include?(next_node)
- end
-
def validate!(current_state, input)
@validations.each do |message, predicate|
if !current_state.instance_exec(input, &predicate)
From f1dddeba36b2408071595a250c6c3d3f02e0c4fb Mon Sep 17 00:00:00 2001
From: James Mead <james@floehopper.org>
Date: Mon, 29 Feb 2016 15:30:09 +0100
Subject: [PATCH 03/17] Rename default_next_node_function to next_node_block
The name `default_next_node_function` confused me for two reasons:
1. The instance variable doesn't just store the *default* value; it also stores
non-default values.
2. It's more common to talk about methods and blocks in Ruby rather than
functions. In this case, the instance variable stores a reference to a block.
---
lib/smart_answer/question/base.rb | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/lib/smart_answer/question/base.rb b/lib/smart_answer/question/base.rb
index 8b6294593b8..4e8c24e0127 100644
--- a/lib/smart_answer/question/base.rb
+++ b/lib/smart_answer/question/base.rb
@@ -8,7 +8,7 @@ class NextNodeUndefined < StandardError; end
def initialize(flow, name, options = {}, &block)
@save_input_as = nil
@validations ||= []
- @default_next_node_function ||= lambda {|_|}
+ @next_node_block ||= lambda {|_|}
@permitted_next_nodes = []
super
end
@@ -19,10 +19,10 @@ def next_node(next_node = nil, permitted: [], &block)
raise "You must specify the permitted next nodes"
end
@permitted_next_nodes += permitted
- @default_next_node_function = block
+ @next_node_block = block
elsif next_node
@permitted_next_nodes = [next_node]
- @default_next_node_function = proc { next_node }
+ @next_node_block = proc { next_node }
else
raise ArgumentError
end
@@ -34,7 +34,7 @@ def validate(message = nil, &block)
def next_node_for(current_state, input)
validate!(current_state, input)
- next_node = next_node_from_default_function(current_state, input)
+ next_node = next_node_from_next_node_block(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
unless @permitted_next_nodes.include?(next_node)
@@ -87,8 +87,8 @@ def validate!(current_state, input)
end
end
- def next_node_from_default_function(current_state, input)
- current_state.instance_exec(input, &@default_next_node_function)
+ def next_node_from_next_node_block(current_state, input)
+ current_state.instance_exec(input, &@next_node_block)
end
end
end
From cbe0a7a9e79dd3c9c3e2eaaab6a1ea9ceff6980d Mon Sep 17 00:00:00 2001
From: James Mead <james@floehopper.org>
Date: Mon, 29 Feb 2016 15:33:40 +0100
Subject: [PATCH 04/17] Inline next_node_from_next_node_block method
I don't think this method was adding anything to the code.
---
lib/smart_answer/question/base.rb | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/lib/smart_answer/question/base.rb b/lib/smart_answer/question/base.rb
index 4e8c24e0127..42d968f24b9 100644
--- a/lib/smart_answer/question/base.rb
+++ b/lib/smart_answer/question/base.rb
@@ -34,7 +34,7 @@ def validate(message = nil, &block)
def next_node_for(current_state, input)
validate!(current_state, input)
- next_node = next_node_from_next_node_block(current_state, input)
+ next_node = current_state.instance_exec(input, &@next_node_block)
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
unless @permitted_next_nodes.include?(next_node)
@@ -86,10 +86,6 @@ def validate!(current_state, input)
end
end
end
-
- def next_node_from_next_node_block(current_state, input)
- current_state.instance_exec(input, &@next_node_block)
- end
end
end
end
From 4f7766a6aef3650237b5cdd557a66a9c7826ee2d Mon Sep 17 00:00:00 2001
From: James Mead <james@floehopper.org>
Date: Mon, 29 Feb 2016 15:38:17 +0100
Subject: [PATCH 05/17] Remove unnecessary blank line from top of test
---
test/unit/question_base_test.rb | 1 -
1 file changed, 1 deletion(-)
diff --git a/test/unit/question_base_test.rb b/test/unit/question_base_test.rb
index 2a0ab341541..6b3a8778dea 100644
--- a/test/unit/question_base_test.rb
+++ b/test/unit/question_base_test.rb
@@ -1,4 +1,3 @@
-
require 'ostruct'
require_relative '../test_helper'
From e17825bb60bdca0d8ff536704d36c528320f3005 Mon Sep 17 00:00:00 2001
From: James Mead <james@floehopper.org>
Date: Mon, 29 Feb 2016 15:38:46 +0100
Subject: [PATCH 06/17] Remove unused require statement from top of test
---
test/unit/question_base_test.rb | 1 -
1 file changed, 1 deletion(-)
diff --git a/test/unit/question_base_test.rb b/test/unit/question_base_test.rb
index 6b3a8778dea..b4cd3c9c05e 100644
--- a/test/unit/question_base_test.rb
+++ b/test/unit/question_base_test.rb
@@ -1,4 +1,3 @@
-require 'ostruct'
require_relative '../test_helper'
class QuestionBaseTest < ActiveSupport::TestCase
From 8a0fd948c1cdbaf04283c9f194e72ca0b90d9275 Mon Sep 17 00:00:00 2001
From: James Mead <james@floehopper.org>
Date: Mon, 29 Feb 2016 15:51:08 +0100
Subject: [PATCH 07/17] Improve argument checking for calls to next_node
* Consistently use `ArgumentError` if `next_node` is called with incorrect
arguments.
* Add unit test coverage for the two possible errors.
* Add/improve error messages.
---
lib/smart_answer/question/base.rb | 4 ++--
test/unit/question_base_test.rb | 22 +++++++++++++++++++++-
2 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/lib/smart_answer/question/base.rb b/lib/smart_answer/question/base.rb
index 42d968f24b9..038fac31f5f 100644
--- a/lib/smart_answer/question/base.rb
+++ b/lib/smart_answer/question/base.rb
@@ -16,7 +16,7 @@ def initialize(flow, name, options = {}, &block)
def next_node(next_node = nil, permitted: [], &block)
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
@next_node_block = block
@@ -24,7 +24,7 @@ def next_node(next_node = nil, permitted: [], &block)
@permitted_next_nodes = [next_node]
@next_node_block = proc { next_node }
else
- raise ArgumentError
+ raise ArgumentError, 'You must specify a block or a single next node key'
end
end
diff --git a/test/unit/question_base_test.rb b/test/unit/question_base_test.rb
index b4cd3c9c05e..45f1c36c286 100644
--- a/test/unit/question_base_test.rb
+++ b/test/unit/question_base_test.rb
@@ -15,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
@@ -24,6 +35,15 @@ 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 "State is carried over on a state transition" do
q = SmartAnswer::Question::Base.new(nil, :example) {
next_node :done
From b4ea958aed891b0c9013372d555ee6d967a6e7de Mon Sep 17 00:00:00 2001
From: James Mead <james@floehopper.org>
Date: Mon, 29 Feb 2016 16:18:57 +0100
Subject: [PATCH 08/17] Simplify instance var assignments in Question::Base
There's nowhere else these instance variables could've been assigned prior to
the class' constructor, so it doesn't seem necessary to include the `||` check
to see whether they are already assigned.
---
lib/smart_answer/question/base.rb | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/smart_answer/question/base.rb b/lib/smart_answer/question/base.rb
index 038fac31f5f..8ee2a486fec 100644
--- a/lib/smart_answer/question/base.rb
+++ b/lib/smart_answer/question/base.rb
@@ -7,8 +7,8 @@ class NextNodeUndefined < StandardError; end
def initialize(flow, name, options = {}, &block)
@save_input_as = nil
- @validations ||= []
- @next_node_block ||= lambda {|_|}
+ @validations = []
+ @next_node_block = lambda {|_|}
@permitted_next_nodes = []
super
end
From 9bb238691d0150c6f186064a97a7b50981fe6aaf Mon Sep 17 00:00:00 2001
From: James Mead <james@floehopper.org>
Date: Mon, 29 Feb 2016 16:22:31 +0100
Subject: [PATCH 09/17] Use lambda vs proc in Question::Base
* Calls to `lambda` produce less surprising procs than calls to `proc` or
`Proc.new` (in terms of their return behaviour).
* Procs produced by calls to `lambda` are stricter about the number of arguments
they are passed and so we have to declare a dummy input argument like we do in
the constructor.
* We were already using `lambda` to set the default value of `@next_node_block`
in the constructor and it seems better to be consistent.
---
lib/smart_answer/question/base.rb | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/smart_answer/question/base.rb b/lib/smart_answer/question/base.rb
index 8ee2a486fec..8395edf5154 100644
--- a/lib/smart_answer/question/base.rb
+++ b/lib/smart_answer/question/base.rb
@@ -22,7 +22,7 @@ def next_node(next_node = nil, permitted: [], &block)
@next_node_block = block
elsif next_node
@permitted_next_nodes = [next_node]
- @next_node_block = proc { next_node }
+ @next_node_block = lambda { |_| next_node }
else
raise ArgumentError, 'You must specify a block or a single next node key'
end
From f78895199b9743b1d18968eff875ba7719746972 Mon Sep 17 00:00:00 2001
From: James Mead <james@floehopper.org>
Date: Mon, 29 Feb 2016 16:37:26 +0100
Subject: [PATCH 10/17] Improve test coverage when next_node not called
There are two ways that a `NextNodeUndefined` exception can be raised:
1. The call to `next_node` does not return a truth-y value
2. No call to `next_node` is made
The second of these was not being unit tested.
---
test/unit/question_base_test.rb | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/test/unit/question_base_test.rb b/test/unit/question_base_test.rb
index 45f1c36c286..c1f23d8b2ba 100644
--- a/test/unit/question_base_test.rb
+++ b/test/unit/question_base_test.rb
@@ -198,4 +198,19 @@ class QuestionBaseTest < ActiveSupport::TestCase
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
From ed85398e5ceb526455c606d0285f048aa6e09e79 Mon Sep 17 00:00:00 2001
From: James Mead <james@floehopper.org>
Date: Mon, 29 Feb 2016 16:40:25 +0100
Subject: [PATCH 11/17] Make next_node_block default return value more explicit
In the case where no call to `next_node` is made for a question, the code which
raises `NextNodeUndefined` relies on the default value for `@next_node_block`
returning a false-y value when invoked. This makes that more explicit.
---
lib/smart_answer/question/base.rb | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/smart_answer/question/base.rb b/lib/smart_answer/question/base.rb
index 8395edf5154..95892d0f089 100644
--- a/lib/smart_answer/question/base.rb
+++ b/lib/smart_answer/question/base.rb
@@ -8,7 +8,7 @@ class NextNodeUndefined < StandardError; end
def initialize(flow, name, options = {}, &block)
@save_input_as = nil
@validations = []
- @next_node_block = lambda {|_|}
+ @next_node_block = lambda { |_| nil }
@permitted_next_nodes = []
super
end
From c8128e7aca5ac13be84587f89db0f8538844afe8 Mon Sep 17 00:00:00 2001
From: James Mead <james@floehopper.org>
Date: Mon, 29 Feb 2016 16:43:08 +0100
Subject: [PATCH 12/17] Convert single-line conditional into multi-line
I think this improves the readability, because the line is so long it's easy to
miss the conditional. Also it's consistent with the `if` statement immediately
below it.
---
lib/smart_answer/question/base.rb | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/lib/smart_answer/question/base.rb b/lib/smart_answer/question/base.rb
index 95892d0f089..4c7f39ac424 100644
--- a/lib/smart_answer/question/base.rb
+++ b/lib/smart_answer/question/base.rb
@@ -36,7 +36,9 @@ def next_node_for(current_state, input)
validate!(current_state, input)
next_node = current_state.instance_exec(input, &@next_node_block)
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
+ unless next_node
+ raise NextNodeUndefined.new("Next node undefined. Node: #{current_state.current_node}. Responses: #{responses_and_input}")
+ 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
From 578b848d31645e59a973cc0cecd33430b76d01d2 Mon Sep 17 00:00:00 2001
From: James Mead <james@floehopper.org>
Date: Mon, 29 Feb 2016 16:59:59 +0100
Subject: [PATCH 13/17] Don't allow multiple calls to next_node per question
I'm pretty sure we don't have any places where `next_node` is called multiple
times per question and I can't see any sensible justification for allowing it.
Constraining the DSL so that it can only be called once means that we can
simplify the code and make it easier to reason about.
With this guard condition in place, there's no longer any need to add the
`permitted_next_nodes` to any stored in a previous call.
---
lib/smart_answer/question/base.rb | 5 ++++-
test/unit/question_base_test.rb | 10 ++++++++++
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/lib/smart_answer/question/base.rb b/lib/smart_answer/question/base.rb
index 4c7f39ac424..a29e61b80fb 100644
--- a/lib/smart_answer/question/base.rb
+++ b/lib/smart_answer/question/base.rb
@@ -14,11 +14,14 @@ def initialize(flow, name, options = {}, &block)
end
def next_node(next_node = nil, permitted: [], &block)
+ if @permitted_next_nodes.any?
+ raise 'Multiple calls to next_node are not allowed'
+ end
if block_given?
unless permitted.any?
raise ArgumentError, 'You must specify at least one permitted next node'
end
- @permitted_next_nodes += permitted
+ @permitted_next_nodes = permitted
@next_node_block = block
elsif next_node
@permitted_next_nodes = [next_node]
diff --git a/test/unit/question_base_test.rb b/test/unit/question_base_test.rb
index c1f23d8b2ba..707aa63229d 100644
--- a/test/unit/question_base_test.rb
+++ b/test/unit/question_base_test.rb
@@ -44,6 +44,16 @@ class QuestionBaseTest < ActiveSupport::TestCase
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
From ce1208cc5bf7607920f7ebf39eec805ed221bf88 Mon Sep 17 00:00:00 2001
From: James Mead <james@floehopper.org>
Date: Tue, 1 Mar 2016 12:20:13 +0100
Subject: [PATCH 14/17] More direct detection of multiple calls to next_node
Previously we were relying on detecting a side-effect of `next_node` having
been called previously. Now we rely on whether or not `@next_node_block` has
been set which is much more direct and explicit.
---
lib/smart_answer/question/base.rb | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/lib/smart_answer/question/base.rb b/lib/smart_answer/question/base.rb
index a29e61b80fb..51ed973e01d 100644
--- a/lib/smart_answer/question/base.rb
+++ b/lib/smart_answer/question/base.rb
@@ -8,13 +8,13 @@ class NextNodeUndefined < StandardError; end
def initialize(flow, name, options = {}, &block)
@save_input_as = nil
@validations = []
- @next_node_block = lambda { |_| nil }
+ @default_next_node_block = lambda { |_| nil }
@permitted_next_nodes = []
super
end
def next_node(next_node = nil, permitted: [], &block)
- if @permitted_next_nodes.any?
+ if @next_node_block.present?
raise 'Multiple calls to next_node are not allowed'
end
if block_given?
@@ -37,7 +37,7 @@ def validate(message = nil, &block)
def next_node_for(current_state, input)
validate!(current_state, input)
- next_node = current_state.instance_exec(input, &@next_node_block)
+ next_node = current_state.instance_exec(input, &next_node_block)
responses_and_input = current_state.responses + [input]
unless next_node
raise NextNodeUndefined.new("Next node undefined. Node: #{current_state.current_node}. Responses: #{responses_and_input}")
@@ -80,6 +80,10 @@ def question?
end
private
+ def next_node_block
+ @next_node_block || @default_next_node_block
+ end
+
def validate!(current_state, input)
@validations.each do |message, predicate|
if !current_state.instance_exec(input, &predicate)
From cb1b459fb612384ab3343591e35e69d49a5a0fba Mon Sep 17 00:00:00 2001
From: James Mead <james@floehopper.org>
Date: Sat, 5 Mar 2016 11:56:00 +0100
Subject: [PATCH 15/17] Move local variable inside only block where it's used
---
lib/smart_answer/question/base.rb | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/smart_answer/question/base.rb b/lib/smart_answer/question/base.rb
index 51ed973e01d..8b7efa80f51 100644
--- a/lib/smart_answer/question/base.rb
+++ b/lib/smart_answer/question/base.rb
@@ -38,8 +38,8 @@ def validate(message = nil, &block)
def next_node_for(current_state, input)
validate!(current_state, input)
next_node = current_state.instance_exec(input, &next_node_block)
- responses_and_input = current_state.responses + [input]
unless next_node
+ responses_and_input = current_state.responses + [input]
raise NextNodeUndefined.new("Next node undefined. Node: #{current_state.current_node}. Responses: #{responses_and_input}")
end
unless @permitted_next_nodes.include?(next_node)
From 704866355095fd3fbb42bf047ddb9b26f5f9af4f Mon Sep 17 00:00:00 2001
From: James Mead <james@floehopper.org>
Date: Sat, 5 Mar 2016 11:58:33 +0100
Subject: [PATCH 16/17] Build exception message separately to avoid long line
Also added full-stop to second sentence for consistency.
---
lib/smart_answer/question/base.rb | 4 +++-
test/unit/question_base_test.rb | 4 ++--
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/lib/smart_answer/question/base.rb b/lib/smart_answer/question/base.rb
index 8b7efa80f51..d638d68b0d4 100644
--- a/lib/smart_answer/question/base.rb
+++ b/lib/smart_answer/question/base.rb
@@ -40,7 +40,9 @@ def next_node_for(current_state, input)
next_node = current_state.instance_exec(input, &next_node_block)
unless next_node
responses_and_input = current_state.responses + [input]
- raise NextNodeUndefined.new("Next node undefined. Node: #{current_state.current_node}. Responses: #{responses_and_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})"
diff --git a/test/unit/question_base_test.rb b/test/unit/question_base_test.rb
index 707aa63229d..1b69409fa6c 100644
--- a/test/unit/question_base_test.rb
+++ b/test/unit/question_base_test.rb
@@ -205,7 +205,7 @@ 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
@@ -220,7 +220,7 @@ 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
end
From f441f7c38b1e99ecdfebfaa91d4be33720648d09 Mon Sep 17 00:00:00 2001
From: James Mead <james@floehopper.org>
Date: Mon, 7 Mar 2016 17:02:45 +0100
Subject: [PATCH 17/17] Update/improve documentation for next_node
Most of these changes document behaviour that existed prior to this
pull request, although some of the error messages have changed slightly.
The only additional behaviour that was added in this pull request is the
check which prevents multiple calls to `next_node`.
---
doc/next-node-rules.md | 41 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 40 insertions(+), 1 deletion(-)
diff --git a/doc/next-node-rules.md b/doc/next-node-rules.md
index 9f47f6ab93b..23a0722e803 100644
--- a/doc/next-node-rules.md
+++ b/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
```