Skip to content
This repository was archived by the owner on Nov 6, 2024. It is now read-only.

Add conditionally required validation#5

Open
lutfidemirci wants to merge 2 commits intomasterfrom
add-conditionall-required-validation
Open

Add conditionally required validation#5
lutfidemirci wants to merge 2 commits intomasterfrom
add-conditionall-required-validation

Conversation

@lutfidemirci
Copy link

@lutfidemirci lutfidemirci commented Dec 23, 2020

Nodes within schema can not accept required option. This option makes the node conditionally required. Currently we do not allow a node to be conditionally required and optional in the same time.

schema = NxtSchema.schema(:contact) do
  required(:first_name, :String)
  required(:last_name, :String)
  node(:email, :String, required: ->(node) { node.up[:last_name].input == 'Superstar' })
end

schema.apply(input: {
  first_name: 'Andy',
  last_name: 'Superstar'
}).valid? # => false

schema.apply(input: {
  first_name: 'Andy',
  last_name: 'Other'
}).valid? # => true

@lutfidemirci lutfidemirci requested a review from getand December 23, 2020 08:12
Copy link
Contributor

@getand getand left a comment

Choose a reason for hiding this comment

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

A few suggestions on naming. What I would like to have though is a test that proves an error is raised in case both optional and required option are given. Otherwise looks good.

has_required_option = options.key?(:required)
has_optional_option = options.key?(:optional)

raise Errors::InvalidOptions, "Can't make a node optional and required in the same time" if has_required_option && has_optional_option
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
raise Errors::InvalidOptions, "Can't make a node optional and required in the same time" if has_required_option && has_optional_option
raise Errors::InvalidOptions, "Can't make a node optional and required at the same time" if has_required_option && has_optional_option

end

def resolve_required_or_optional_option
has_required_option = options.key?(:required)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
has_required_option = options.key?(:required)
required_option_provided = options.key?(:required)


def resolve_required_or_optional_option
has_required_option = options.key?(:required)
has_optional_option = options.key?(:optional)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
has_optional_option = options.key?(:optional)
optional_option_provided = options.key?(:optional)


if required.respond_to?(:call)
# When a node is conditionally required we make it optional and add a validator to the parent to check
# that it's there when the option apply.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# that it's there when the option apply.
# that it's there when the option applies.

@getand
Copy link
Contributor

getand commented Dec 23, 2020

Also please do not forget to update the readme with an example. ❤️

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants