Skip to content

Validate string in list - #30

Open
cnavas88 wants to merge 2 commits into
lob:masterfrom
admanmedia:develop
Open

cnavas88 wants to merge 2 commits into
lob:masterfrom
admanmedia:develop

Conversation

@cnavas88

Copy link
Copy Markdown
  • Adding "included" in String validator.
  • Validate that a string is included in a list.
  • Testing Included function.
  • Documentation included function.

@dmlittle dmlittle left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cnavas88 thanks for the PR! I have some comments on the implementation and naming convention of this feature but this is something we'd love to support! @mgartner might have more thoughts on this issue.

Comment thread lib/litmus/type/string.ex
* `:trim` - Removes additional whitespace at the front and end of a string.
Allowed values are `true` and `false`. The default is `false`.

* `:included` - Specifies a option list.

@dmlittle dmlittle Jul 23, 2019

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this feature but I think I'd prefer having the name option be a list called :valid since it specifies the list of valid strings. This would make the using this feature like this:

schema = %{
  "casing" => %Litmus.Type.String{
    valid: ["lowercase", "uppercase"]
  }
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dmlittle i think you tpyo'd included instead of valid 😆

Comment thread lib/litmus/type/string.ex
if Map.get(params, field) in included do
{:ok, params}
else
{:error, "#{field} isn't into the list."}

@dmlittle dmlittle Jul 23, 2019

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might be helpful to return the valid values in the error to provide a more helpful message. Taking inspiration from Joi we could do something like this:

defp included(%__MODULE__{valid: valid}, field, params) do
  if Map.get(params, field) in valid do
    {:ok, params}
  else
    {:error, "#{field} must be one of #{valid}"}
  end
end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants