-
Notifications
You must be signed in to change notification settings - Fork 18
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add Money.Validate.validate_money/3 to support ecto changeset validat…
…ions
- Loading branch information
Showing
5 changed files
with
175 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,104 @@ | ||
defmodule Money.Validate do | ||
@moduledoc """ | ||
Implements Ecto validations for the `t:Money` type based upon the | ||
`Money.Ecto.Composite.Type` type. | ||
""" | ||
|
||
@money_validators %{ | ||
less_than: "must be less than %{money}", | ||
greater_than: "must be greater than %{money}", | ||
less_than_or_equal_to: "must be less than or equal to %{money}", | ||
greater_than_or_equal_to: "must be greater than or equal to %{money}", | ||
equal_to: "must be equal to %{money}", | ||
not_equal_to: "must be not equal to %{money}" | ||
} | ||
|
||
@doc """ | ||
Validates the properties of a `t:Money`. | ||
This function, including its options, is designed to | ||
mirror the function `Ecto.Changeset.validate_number/3`. | ||
## Options | ||
* `:less_than` | ||
* `:greater_than` | ||
* `:less_than_or_equal_to` | ||
* `:greater_than_or_equal_to` | ||
* `:equal_to` | ||
* `:not_equal_to` | ||
* `:message` - the message on failure, defaults to one of: | ||
* "must be less than %{money}" | ||
* "must be greater than %{money}" | ||
* "must be less than or equal to %{money}" | ||
* "must be greater than or equal to %{money}" | ||
* "must be equal to %{money}" | ||
* "must be not equal to %{money}" | ||
## Examples | ||
validate_money(changeset, :value, less_than: Money.new(:USD, 200)) | ||
validate_money(changeset, :value, less_than_or_equal_to: Money.new(:USD, 200) | ||
validate_money(changeset, :value, less_than_or_equal_to: Money.new(:USD, 100)) | ||
validate_money(changeset, :value, greater_than: Money.new(:USD, 50)) | ||
validate_money(changeset, :value, greater_than_or_equal_to: Money.new(:USD, 50)) | ||
validate_money(changeset, :value, greater_than_or_equal_to: Money.new(:USD, 100)) | ||
""" | ||
@spec validate_money(Ecto.Changeset.t(), atom, Keyword.t) :: Ecto.Changeset.t() | ||
def validate_money(changeset, field, opts) do | ||
Ecto.Changeset.validate_change changeset, field, {:money, opts}, fn | ||
field, value -> | ||
{message, opts} = Keyword.pop(opts, :message) | ||
Enum.find_value opts, [], fn {spec_key, target_value} -> | ||
case Map.fetch(@money_validators, spec_key) do | ||
{:ok, default_message} -> | ||
validate_money(field, value, message || default_message, | ||
spec_key, target_value) | ||
:error -> | ||
supported_options = @money_validators |> Map.keys() | ||
|
||
raise ArgumentError, """ | ||
unknown option #{inspect spec_key} given to validate_money/3 | ||
The supported options are: | ||
#{supported_options} | ||
""" | ||
end | ||
end | ||
end | ||
end | ||
|
||
defp validate_money(field, %Money{} = value, message, spec_key, %Money{} = target_value) do | ||
result = Money.compare(value, target_value) | ||
|
||
case money_compare(result, spec_key) do | ||
true -> | ||
nil | ||
|
||
false -> | ||
[{field, {message, validation: :money, kind: spec_key, money: target_value}}] | ||
|
||
{:error, {_exception, reason}} -> | ||
[{field, {reason, validation: :money, kind: spec_key, money: target_value}}] | ||
end | ||
end | ||
|
||
defp validate_money(_field, value, _message, _spec_key, %Money{} = _target_value) do | ||
raise ArgumentError, "expected value to be of type Money, got: #{inspect value}" | ||
end | ||
|
||
defp validate_money(_field, %Money{} = _value, _message, _spec_key, target_value) do | ||
raise ArgumentError, "expected target_value to be of type Money, got: #{inspect target_value}" | ||
end | ||
|
||
defp validate_money(_field, value, _message, _spec_key, target_value) do | ||
raise ArgumentError, "expected value and target_value to be of type Money, " <> | ||
"got value: #{inspect value} and target_value: #{target_value}" | ||
end | ||
|
||
defp money_compare(:lt, spec), do: spec in [:less_than, :less_than_or_equal_to, :not_equal_to] | ||
defp money_compare(:gt, spec), do: spec in [:greater_than, :greater_than_or_equal_to, :not_equal_to] | ||
defp money_compare(:eq, spec), do: spec in [:equal_to, :less_than_or_equal_to, :greater_than_or_equal_to] | ||
defp money_compare(other, _spec), do: other | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
defmodule Money.Sql.Mixfile do | ||
use Mix.Project | ||
|
||
@version "1.6.0" | ||
@version "1.7.0-dev" | ||
|
||
def project do | ||
[ | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,49 @@ | ||
defmodule Money.Changeset.Test do | ||
use ExUnit.Case | ||
import Money.Validate | ||
import Money.ValidationSupport | ||
|
||
test "Changeset default currency" do | ||
changeset = Organization.changeset(%Organization{}, %{payroll: "0"}) | ||
assert changeset.changes.payroll == Money.new(:JPY, 0) | ||
end | ||
|
||
test "money positive validation" do | ||
assert validate_money(test_changeset(), :value, less_than: Money.new(:USD, 200)).valid? | ||
assert validate_money(test_changeset(), :value, less_than_or_equal_to: Money.new(:USD, 200)).valid? | ||
assert validate_money(test_changeset(), :value, less_than_or_equal_to: Money.new(:USD, 100)).valid? | ||
|
||
assert validate_money(test_changeset(), :value, greater_than: Money.new(:USD, 50)).valid? | ||
assert validate_money(test_changeset(), :value, greater_than_or_equal_to: Money.new(:USD, 50)).valid? | ||
assert validate_money(test_changeset(), :value, greater_than_or_equal_to: Money.new(:USD, 100)).valid? | ||
|
||
assert validate_money(test_changeset(), :value, equal_to: Money.new(:USD, 100)).valid? | ||
|
||
assert validate_money(test_changeset(), :value, greater_than: Money.new(:USD, 50), less_than: Money.new(:USD, 200)).valid? | ||
end | ||
|
||
test "money negative validation" do | ||
refute validate_money(test_changeset(), :value, less_than: Money.new(:AUD, 200)).valid? | ||
|
||
assert validate_money(test_changeset(), :value, less_than: Money.new(:USD, 50)).errors == | ||
[value: {"must be less than %{money}", [validation: :money, kind: :less_than, money: Money.new(:USD, 50)]}] | ||
end | ||
|
||
test "Non-money changeset and comparison values" do | ||
assert validate_money(test_changeset(), :value, less_than: Money.new(:AUD, 200)).errors == | ||
[value: {"Cannot compare monies with different currencies. Received :USD and :AUD.", | ||
[validation: :money, kind: :less_than, money: Money.new(:AUD, 200)]}] | ||
|
||
assert_raise ArgumentError, ~r/expected target_value to be of type Money/, fn -> | ||
validate_money(test_changeset(), :value, less_than: 200) | ||
end | ||
|
||
assert_raise ArgumentError, ~r/expected value to be of type Money/, fn -> | ||
validate_money(non_money_changeset(), :employee_count, less_than: Money.new(:USD, 200)) | ||
end | ||
|
||
assert_raise ArgumentError, ~r/expected value and target_value to be of type Money/, fn -> | ||
validate_money(non_money_changeset(), :employee_count, less_than: 200) | ||
end | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
defmodule Money.ValidationSupport do | ||
import Ecto.Changeset | ||
|
||
def test_changeset do | ||
params = %{"value" => "100"} | ||
|
||
changeset = | ||
%Organization{} | ||
|> cast(params, [:value]) | ||
end | ||
|
||
def non_money_changeset do | ||
params = %{"employee_count" => "100"} | ||
|
||
changeset = | ||
%Organization{} | ||
|> cast(params, [:employee_count]) | ||
end | ||
end |
cd2547d
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kipcole9 I‘d be more explicit and name it ˋvalidate_amountˋ and allow it to match against decimal values, ignoring currency. Then there could be a ˋvalidate_currencyˋ which works like ˋvalidate_inclusionˋ to validate the currency part of the field. This should be quite a bit more flexible than the current approach and prevent overlapping concerns of errors about currency being produced by a amount validating function. Comparing to another money struct directly seems useful to start, but is likely a pain to handle the error cases of.
cd2547d
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything I've done so far in this library has been to keep the integrity of the
Money
struct - the primary principle being that money has no meaning without both the currency and the amount. So I feel like I'm being more explicit by insisting (in this function) that the comparison is always between twoMoney
structs, not the opaque elements. Its not a general purpose validation - it's a very specific in its focus only onMoney
.So in this case I'm feeling like the proposed flexibility is a compromise to the goals of the library. Perhaps I'm being overly conservative?
cd2547d
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can see where you‘re coming from, but say the currency is a dropdown in the form and allows selection out of a set of currencies and the amount may not be negative. For this it‘s not possible to construct a valid money value to compare to in advance. Only by looking at the submitted currency that would be possible making the code more complex. E.g. in one of my projects people may submit allowances and they may be in multiple currencies, as they might cross borders between individual allowances.
Making sure money can never be compared across currencies is certainly a valid concern, but for form validation it‘s not always a comparison to another money value, but also lower level constraints like: this needs to be a positive value in eur or chf.
cd2547d
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good points definitely, and I see the use case. I just thought that the existing
validate_number
andvalidation_inclusion
already take care of these particular requirements? More than happy to add other money validations but its been a while since I've done any UI-related work so maybe not seeing the gap properly.cd2547d
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The gap is that ecto changeset api works on fields, not on internals of field values. Amount and currency are internals of a money field when using one of the custom ecto types to storing money.
cd2547d
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yes, of course. Got it. OK, I have a path forward.