Merge pull request #95 from WeTransfer/gh-94-rule-value-fix

Values should be of the specified type in Rules

test/models/rule_test.exs

@@ -2,7 +2,7 @@ defmodule Xperiments.RuleTest do
   use Xperiments.ModelCase
   alias Xperiments.Rule
 
-  @valid_attrs %{parameter: "lang", type: "string", operator: "==", value: "eu"}
+  @valid_attrs %{"parameter" => "lang", "type" => "string", "operator" => "==", "value" => "eu"}
 
   test "changest with valid attributes" do
     changeset = Rule.changeset(%Rule{}, @valid_attrs)
@@ -15,30 +15,37 @@ defmodule Xperiments.RuleTest do
   end
 
   test "validation of types and opertators" do
-    changeset = Rule.changeset(%Rule{}, %{@valid_attrs | operator: "!"})
+    changeset = Rule.changeset(%Rule{}, %{@valid_attrs | "operator" => "!"})
     refute changeset.valid?
-    changeset = Rule.changeset(%Rule{}, %{@valid_attrs | type: "bad_type"})
+    changeset = Rule.changeset(%Rule{}, %{@valid_attrs | "type" => "bad_type"})
     refute changeset.valid?
   end
 
   test "validate operators for specific types" do
-    chset = Rule.changeset(%Rule{}, %{@valid_attrs | operator: ">="})
+    chset = Rule.changeset(%Rule{}, %{@valid_attrs | "operator" => ">="})
     refute chset.valid?
     assert hd(chset.errors) == {:type, {"string types must have only '==' and '!=' operators", []}}
     # boolean
-    chset = Rule.changeset(%Rule{}, %{@valid_attrs | type: "boolean", operator: "!="})
+    chset = Rule.changeset(%Rule{}, %{@valid_attrs | "type" => "boolean", "operator" => "!="})
     refute chset.valid?
     assert hd(chset.errors) == {:type, {"boolean type must have only '==' operator and value must be 'true' or 'false'", []}}
   end
 
   test "downcase of a parameter when saving" do
-    chset = Rule.changeset(%Rule{}, %{@valid_attrs | parameter: "Lang"})
+    chset = Rule.changeset(%Rule{}, %{@valid_attrs | "parameter" => "Lang"})
     assert chset.valid?
     assert chset.changes[:parameter] == "lang"
   end
 
   test "validation for value if type set to 'number'" do
-    chset = Rule.changeset(%Rule{}, %{@valid_attrs | type: "number", value: "str"})
+    chset = Rule.changeset(%Rule{}, %{@valid_attrs | "type" => "number", "value" => "str"})
     refute chset.valid?
   end
+
+  test "validaiton for valuer as a number" do
+    chset = Rule.changeset(%Rule{}, %{@valid_attrs | "type" => "number", "value" => 3})
+    assert chset.valid?
+    # chset = Rule.changeset(%Rule{}, %{@valid_attrs | type: "number"})
+    # refute chset.valid?
+  end
 end

web/models/rule.ex

@@ -21,6 +21,7 @@ defmodule Xperiments.Rule do
   @allowed_params ~w(parameter type operator value)a
 
   def changeset(struct, params \\ %{}) do
+    params = make_value_as_string(params)
     struct
     |> cast(params, @allowed_params)
     |> validate_required(@allowed_params)
@@ -32,6 +33,12 @@ defmodule Xperiments.Rule do
     |> downcase_parameter()
   end
 
+  def make_value_as_string(%{"value" => val} = params) when not is_nil(val) and is_number(val) do
+    Map.update!(params, "value", &to_string(&1))
+  end
+  def make_value_as_string(params), do: params
+
+
   def validate_string_type(chset) do
     changes = chset.changes
     if changes[:type] == "string" and not changes[:operator] in ["==", "!="] do
@@ -51,7 +58,16 @@ defmodule Xperiments.Rule do
   end
 
   def validate_number_for_type_number(chset) do
-    if chset.changes[:type] == "number" and not is_number(chset.changes[:value]) do
+    string_integer_validation = fn
+      str when is_bitstring(str) ->
+        case Integer.parse(str) do
+          {_num, _} -> true
+          :error -> false
+        end
+      num when is_number(num) -> true
+      _ -> false
+    end
+    if chset.changes[:type] == "number" and not string_integer_validation.(chset.changes[:value]) do
       add_error(chset, :value, "must be a number")
     else
       chset

web/static/js/component/forms/createexperiment/addrule.es6

@@ -83,7 +83,11 @@ export default class AddRule extends Form {
   }
 
   handleValue(value) {
-    this.props.setValue(value);
+    if (this.props.rule.type && this.props.rule.type === 'number') {
+      this.props.setValue(parseInt(value));
+    } else {
+      this.props.setValue(parseInt(value));
+    }
     this.unsetError('value');
   }